ata: ahci: power off unused ports

drivers/ata/ahci.c static int ahci_pci_device_resume(struct · line 1919

Quoted 11 lines · Kristen Carlson Accardi

drivers/ata/ahci.c +11 -0

+static int ahci_is_hotplug_capable(struct ata_port *ap)
+{
+	void __iomem *port_mmio = ahci_port_base(ap);
+	u8 cmd;
+
+	if (!ahci_power_save)
+		return 1;
+
+	cmd = readl(port_mmio + PORT_CMD);
+	return ((cmd & PORT_CMD_HPCP) || (cmd & PORT_CMD_ESP));
+}

Just out of interest, is this set on laptop bays? (Are any of those even native SATA yet?)

Quoted 2 lines · On Fri, 9 May 2008 00:37:13 +0100 Matthew Garrett <[email protected]> wrote:

Just out of interest, is this set on laptop bays? (Are any of those even native SATA yet?)

I haven't seen any yet that are. Most of them are PATA. I expect that will change in the future.

Quoted 5 lines · Kristen Carlson Accardi

Quoted 2 lines · On Fri, 9 May 2008 00:37:13 +0100 Matthew Garrett <[email protected]> wrote:

Just out of interest, is this set on laptop bays? (Are any of those even native SATA yet?)

Quoted 2 lines

I haven't seen any yet that are. Most of them are PATA. I expect that will change in the future.

Yeah. My vague concern would be that on laptops, we can probably power down the port even if it's declared as being hotplug capable - we'll probably get an out of band signal from ACPI anyway. Not an issue at the moment, but possibly something we'll hit in future.

Just out of interest, is this set on laptop bays? (Are any of those even native SATA yet?)

Quoted 2 lines · Matthew Garrett

Just out of interest, is this set on laptop bays? (Are any of those even native SATA yet?)

My wife's Asus R1F does in fact have SATA in use for the drive bay and allows swapping between the DVD-RW and a second HD (or another battery if she had one) in that bay. I would think many laptops do that now.

Genuinely SATA, or PATA bridged to SATA?

Quoted 1 line · Matthew Garrett

Genuinely SATA, or PATA bridged to SATA?

SATA drives using intel 945GM chipset which has two native SATA ports.

power off unused ports

If the port isn't either a drive bay or an external SATA port, do not keep the phy powered on if the port is unoccupied. This saves .75 watts on most laptops. A module parameter can be used to override this behavior and allow the phy's to be powered up regardless of whether it has externally accessible SATA ports.

Signed-off-by Kristen Carlson Accardi <[email protected]>

And a more general note: I still believe we should have a follow-up feature to this one, to enable polling for newly inserted drives.

That would allow powering down idle ports to save money/planet/whatever, but still with hotplug capability. The polling interval should be tunable in /sys, with a default of, say, once every couple of seconds.

Thanks for working on this stuff.

power off unused ports

If the port isn't either a drive bay or an external SATA port, do not keep the phy powered on if the port is unoccupied. This saves .75 watts on most laptops. A module parameter can be used to override this behavior and allow the phy's to be powered up regardless of whether it has externally accessible SATA ports.

Signed-off-by Kristen Carlson Accardi <[email protected]>

Quoted 9 lines · Kristen Carlson Accardi

power off unused ports

If the port isn't either a drive bay or an external SATA port, do not keep the phy powered on if the port is unoccupied. This saves .75 watts on most laptops. A module parameter can be used to override this behavior and allow the phy's to be powered up regardless of whether it has externally accessible SATA ports.

Signed-off-by Kristen Carlson Accardi <[email protected]>

Hi,

I'm wondering what's the status of this patch?

if this has hit a subsystem tree yet? 0.75 watts power savings sounds really cool, but it doesn't seem to be in Linus's mainline yet. Has it hit a subsystem tree yet, or is there some issue that still needs to be resolved before it can go it?

Thanks, regards,

- Ted

As far as I know the patch has gone nowhere. I believe that Jeff wanted something more flexible than the module parameter that I provided to override the BIOS options. I am not working on this, I figured he had a pretty firm idea what he wanted so he was better equipped to write the patch.

Quoted 6 lines · Kristen Carlson Accardi

As far as I know the patch has gone nowhere. I believe that Jeff wanted something more flexible than the module parameter that I provided to override the BIOS options. I am not working on this, I figured he had a pretty firm idea what he wanted so he was better equipped to write the patch.

Thanks Kristen,

Can you say which laptops you had tested this on where it saved power? (Did you test any Thinkpads, in particular?) I'm wondering if it's worth trying to forward port your patch as a private mod to my kernel; 30 to 40 minutes of extra battery life is nothing to sneeze at!

- Ted

Quoted 6 lines

Quoted 5 lines

As far as I know the patch has gone nowhere. I believe that Jeff wanted something more flexible than the module parameter that I provided to override the BIOS options. I am not working on this, I figured he had a pretty firm idea what he wanted so he was better equipped to write the patch.

Quoted 9 lines

Thanks Kristen,

Can you say which laptops you had tested this on where it saved power? (Did you test any Thinkpads, in particular?) I'm wondering if it's worth trying to forward port your patch as a private mod to my kernel; 30 to 40 minutes of extra battery life is nothing to sneeze at!

- Ted

I tested this on an Intel mobile software development platform with a newer mobile ICH - the power savings were measured at the actual component (via probes on the ICH), so I did not measure the power savings at the wall socket, although I would expect the power savings to be even greater on the other side of the power supply. So in short, yes, I think it's worth it to give it a try - the patch is pretty unintrusive, so it should be that difficult a port to do.

Hi!

Quoted 13 lines

Quoted 13 lines

Quoted 4 lines

As far as I know the patch has gone nowhere. I believe that Jeff wanted something more flexible than the module parameter that I provided to override the BIOS options. I am not working on this,
I figured he had a pretty firm idea what he wanted so he was better
equipped to write the patch.

Thanks Kristen,

Can you say which laptops you had tested this on where it saved power? (Did you test any Thinkpads, in particular?) I'm wondering if it's worth trying to forward port your patch as a private mod to my kernel; 30 to 40 minutes of extra battery life is nothing to sneeze at!

- Ted

Quoted 4 lines

I tested this on an Intel mobile software development platform with a newer mobile ICH - the power savings were measured at the actual component (via probes on the ICH), so I did not measure the power savings at the wall socket, although I would expect the power savings to be even greater on the other side of the power supply. So in short,
yes, I think it's worth it to give it a try - the patch is pretty
unintrusive, so it should be that difficult a port to do.

Can you repost the patch? I believe we should push it and only add complex enable/disable functionality if someone needs it...

If you are talking about SATA -- incorrect.

The patch deals with policy, and the user MUST have the ability to control this stuff. Otherwise you create a situation where the user might be denied hotplug use in valid cases, or similar negative situations.

Quoted 3 lines

If you are talking about SATA -- incorrect.

The patch deals with policy, and the user MUST have the ability to control this stuff. Otherwise you create a situation where the user might be denied hotplug use in valid cases, or similar negative situations.

The policy isn't however complicated. Tejun added the stuff for forcing cable type and mode on setup and has therefore written all the per device setup code we might need. Alternatively a single

	foo=1/0

option has been fine for acpi and will do fine for this. Total additional cost - 1 line.

The key requirement is per-port control. Ideally via hdparm or another userspace tool, but kernel command line (module options) or sysfs would be just fine too. And agreed, the minimal you need is simply 1/0 for the port's policy.

Quoted 1 line

The key requirement is per-port control. Ideally via hdparm or another userspace tool, but kernel command line (module options) or sysfs would be just fine too. And agreed, the minimal you need is simply 1/0 for the port's policy.

We don't need it per port Jeff, you are being quite silly here. Right now its permanently foo=0 for all ports and nobody has suffered anything too horrible so being able to turn it off for all ports is clearly quite sufficient for the neat future.

Quoted 4 lines

We don't need it per port Jeff, you are being quite silly here. Right now its permanently foo=0 for all ports and nobody has suffered anything too horrible so being able to turn it off for all ports is clearly quite sufficient for the neat future.

Er, huh? foo=0 means hotplug continues to work on the unused ports. Nobody has suffered because we default to enabling all the goodies.

Quoted 9 lines · Alan Cox

Quoted 4 lines

The key requirement is per-port control. Ideally via hdparm or another userspace tool, but kernel command line (module options) or sysfs would be just fine too. And agreed, the minimal you need is simply 1/0 for the port's policy.

We don't need it per port Jeff, you are being quite silly here. Right now its permanently foo=0 for all ports and nobody has suffered anything too horrible so being able to turn it off for all ports is clearly quite sufficient for the neat future.

Er, huh? foo=0 means hotplug continues to work on the unused ports. Nobody has suffered because we default to enabling all the goodies.

Jeff

Quoted 8 lines · Alan Cox

Quoted 3 lines

If you are talking about SATA -- incorrect.

The patch deals with policy, and the user MUST have the ability to control this stuff. Otherwise you create a situation where the user might be denied hotplug use in valid cases, or similar negative situations.

The policy isn't however complicated. Tejun added the stuff for forcing cable type and mode on setup and has therefore written all the per device setup code we might need. Alternatively a single

Quoted 4 lines

    foo=1/0

option has been fine for acpi and will do fine for this. Total additional cost - 1 line.

The key requirement is per-port control. Ideally via hdparm or another userspace tool, but kernel command line (module options) or sysfs would be just fine too. And agreed, the minimal you need is simply 1/0 for the port's policy.

For the power-off of unused ports, the current patch still sounds extremely vendor-specific (Intel).

Does it actually work (demonstrate, please) on any other hardware ?

Quoted 5 lines · Mark Lord

For the power-off of unused ports, the current patch still sounds extremely vendor-specific (Intel).

Does it actually work (demonstrate, please) on any other hardware ?

Ding... correct. We need something per-port that's not Intel specific.

Jeff

Quoted 4 lines · Mark Lord

For the power-off of unused ports, the current patch still sounds extremely vendor-specific (Intel).

Does it actually work (demonstrate, please) on any other hardware ?

Ding... correct. We need something per-port that's not Intel specific.

Jeff

Can you tell me what part of this patch is vendor specific? Maybe I am misunderstanding what you are saying. Here's that patch again in case you need to look at it again.

Quoted 3 lines · Kristen Carlson Accardi

Can you tell me what part of this patch is vendor specific? Maybe I am misunderstanding what you are saying. Here's that patch again in case you need to look at it again.

It's specific to AHCI, and does not cover other controllers.

Certainly PORT_CMD_HPCP must be, but the concept itself is not at all AHCI-specific.

Jeff

Here you can see that it would be very easy for another driver to just add code to set the NO_HOTPLUG flag and then this code will work for them as well, since we power off the phy using DET which is specified by SATA.

here's that code:

Quoted 6 lines · Kristen Carlson Accardi

Here you can see that it would be very easy for another driver to just add code to set the NO_HOTPLUG flag and then this code will work for them as well, since we power off the phy using DET which is specified by SATA.

here's that code:

Agreed. The main discussion in this sub-thread is more about user interface. The user interface in this case, a module option, is specific to AHCI.

Surely you can see how it is a bit silly to force each driver writer to create the same user interface in each driver, to support a generic concept.

Jeff

Quoted 7 lines · On Mon, 02 Jun 2008 14:15:27 -0400 Jeff Garzik <[email protected]> wrote:

Quoted 5 lines · Kristen Carlson Accardi

Here you can see that it would be very easy for another driver to just add code to set the NO_HOTPLUG flag and then this code will work for them as well, since we power off the phy using DET which is specified by SATA.

Quoted 10 lines

Quoted 2 lines

here's that code:

Agreed. The main discussion in this sub-thread is more about user interface. The user interface in this case, a module option, is specific to AHCI.

Surely you can see how it is a bit silly to force each driver writer to create the same user interface in each driver, to support a generic concept.

Jeff

Not all drivers will need a user interface to turn off hotplug I would think. At any rate - I would think it'd be better to let driver writers decide how they want their drivers to behave wrt hotplug and power instead of forcing a generic policy on everyone.

This patch would provide users of AHCI controllers a way to save power now, while you work on the grand scheme for polling/turning on off hotplug via sysfs. It's an interim solution that impacts nobody but ahci users and is can be easily integrated into whatever solution you eventually work out.

Quoted 5 lines · Kristen Carlson Accardi

Not all drivers will need a user interface to turn off hotplug I would think. At any rate - I would think it'd be better to let driver writers decide how they want their drivers to behave wrt hotplug and power instead of forcing a generic policy on everyone.

Quoted 5 lines

This patch would provide users of AHCI controllers a way to save power now, while you work on the grand scheme for polling/turning on off hotplug via sysfs. It's an interim solution that impacts nobody but ahci users and is can be easily integrated into whatever solution you eventually work out.

I like the patch - it seems that it will help a lot of users out near term in a very positive way while we iterate on the broader solution,

Quoted 11 lines · Ric Wheeler

Quoted 10 lines · Kristen Carlson Accardi

Not all drivers will need a user interface to turn off hotplug I would think. At any rate - I would think it'd be better to let driver writers decide how they want their drivers to behave wrt hotplug and power instead of forcing a generic policy on everyone.

This patch would provide users of AHCI controllers a way to save power now, while you work on the grand scheme for polling/turning on off hotplug via sysfs. It's an interim solution that impacts nobody but ahci users and is can be easily integrated into whatever solution you eventually work out.

Quoted 1 line

I like the patch - it seems that it will help a lot of users out near term in a very positive way while we iterate on the broader solution,

A better patch would enable the _possibility_ of power savings on non-AHCI chips, and not add a one-off AHCI-specific user interface that must be supported for years to come.

Jeff

Quoted 15 lines · Jeff Garzik

Quoted 13 lines · Ric Wheeler

Quoted 10 lines · Kristen Carlson Accardi

Not all drivers will need a user interface to turn off hotplug I would think. At any rate - I would think it'd be better to let driver writers decide how they want their drivers to behave wrt hotplug and power instead of forcing a generic policy on everyone.

This patch would provide users of AHCI controllers a way to save power now, while you work on the grand scheme for polling/turning on off hotplug via sysfs. It's an interim solution that impacts nobody but ahci users and is can be easily integrated into whatever solution you eventually work out.

I like the patch - it seems that it will help a lot of users out near term in a very positive way while we iterate on the broader solution,

Quoted 3 lines

A better patch would enable the _possibility_ of power savings on non-AHCI chips, and not add a one-off AHCI-specific user interface that must be supported for years to come.

Jeff

I think that the patch does that first part pretty reasonably for all chips as Kristen explained before.

If the user interface is the only obstacle and the base code seems to be flexible enough for any device to take advantage of, it would still seem to be a positive step forward to put in that base functionality (even without the module param) to enable power saving.

It is always good to get the complete solution in (how hard can the user interface be to code once we agree on what it should be ;-)), but this seems to be a good first step.

A better patch would enable the _possibility_ of power savings on non-AHCI chips, and not add a one-off AHCI-specific user interface that must be supported for years to come.

Quoted 6 lines · On Mon, 02 Jun 2008 14:15:27 -0400 Jeff Garzik <[email protected]> wrote:

Quoted 5 lines · Kristen Carlson Accardi

Here you can see that it would be very easy for another driver to just add code to set the NO_HOTPLUG flag and then this code will work for them as well, since we power off the phy using DET which is specified by SATA.
here's that code:

Quoted 7 lines

Agreed. The main discussion in this sub-thread is more about user interface. The user interface in this case, a module option, is specific to AHCI.

Surely you can see how it is a bit silly to force each driver writer to create the same user interface in each driver, to support a generic concept.

Jeff

Not all drivers will need a user interface to turn off hotplug I would think. At any rate - I would think it'd be better to let driver writers decide how they want their drivers to behave wrt hotplug and power instead of forcing a generic policy on everyone.

This patch would provide users of AHCI controllers a way to save power now, while you work on the grand scheme for polling/turning on off hotplug via sysfs. It's an interim solution that impacts nobody but ahci users and is can be easily integrated into whatever solution you eventually work out.

Quoted 24 lines · Kristen Carlson Accardi

Quoted 6 lines · On Mon, 02 Jun 2008 14:15:27 -0400 Jeff Garzik <[email protected]> wrote:

Quoted 5 lines · Kristen Carlson Accardi

Here you can see that it would be very easy for another driver to just add code to set the NO_HOTPLUG flag and then this code will work for them as well, since we power off the phy using DET which is specified by SATA.
here's that code:

Quoted 10 lines

Agreed. The main discussion in this sub-thread is more about user interface. The user interface in this case, a module option, is specific to AHCI.

Surely you can see how it is a bit silly to force each driver writer to create the same user interface in each driver, to support a generic concept.

Jeff

Not all drivers will need a user interface to turn off hotplug I would think. At any rate - I would think it'd be better to let driver writers decide how they want their drivers to behave wrt hotplug and power instead of forcing a generic policy on everyone.

Quoted 5 lines

This patch would provide users of AHCI controllers a way to save power now, while you work on the grand scheme for polling/turning on off hotplug via sysfs. It's an interim solution that impacts nobody but ahci users and is can be easily integrated into whatever solution you eventually work out.

It's a one-off driver-specific interface that will have to be supported even after the same feature is available via libata core... that's not the path to scalable, sustainable engineering.

I know user interfaces are annoying because you have to think about chips other than your own, but that's life. Other hardware vendors have to do it too.

Letting each driver have a different user interface is /unfriendly/ to both developers users. It's easiest for Intel kernel developers, but that is not our target audience :)

The biggest power savings for the largest amount of users can be had if you take a moment and figure out what's best for Linux, rather than what is best for Intel.

Because you can be damned sure SATA users with non-AHCI chips want this power savings too... let's not put roadblocks to that in place in the beginning (by adding one-off interfaces).

Quoted 14 lines · Jeff Garzik

Quoted 13 lines · Alan Cox

Quoted 6 lines

If you are talking about SATA -- incorrect.

The patch deals with policy, and the user MUST have the ability to control this stuff. Otherwise you create a situation where the user might be denied hotplug use in valid cases, or similar negative situations.

The policy isn't however complicated. Tejun added the stuff for forcing cable type and mode on setup and has therefore written all the per device setup code we might need. Alternatively a single

    foo=1/0

Quoted 7 lines

Quoted 2 lines

option has been fine for acpi and will do fine for this. Total additional cost - 1 line.

The key requirement is per-port control. Ideally via hdparm or another userspace tool, but kernel command line (module options) or sysfs would be just fine too. And agreed, the minimal you need is simply 1/0 for the port's policy.

..

Btw.. hdparm-8.7 (unreleased) can grok /sys now, so that interface is as good as any from a userspace viewpoint now.

For the power-off of unused ports, the current patch still sounds extremely vendor-specific (Intel).

Does it actually work (demonstrate, please) on any other hardware ?

Quoted 16 lines · On Mon, 02 Jun 2008 09:03:04 -0400 Mark Lord <[email protected]> wrote:

Quoted 13 lines · Jeff Garzik

Quoted 11 lines · Alan Cox

Quoted 3 lines

If you are talking about SATA -- incorrect.

The patch deals with policy, and the user MUST have the ability to control this stuff. Otherwise you create a situation where the user might be denied hotplug use in valid cases, or similar negative situations.

The policy isn't however complicated. Tejun added the stuff for forcing cable type and mode on setup and has therefore written all the per device setup code we might need. Alternatively a single

    foo=1/0

option has been fine for acpi and will do fine for this. Total additional cost - 1 line.

The key requirement is per-port control. Ideally via hdparm or another userspace tool, but kernel command line (module options) or sysfs would be just fine too. And agreed, the minimal you need is simply 1/0 for the port's policy.

..

Quoted 5 lines

Btw.. hdparm-8.7 (unreleased) can grok /sys now, so that interface is as good as any from a userspace viewpoint now.

For the power-off of unused ports, the current patch still sounds extremely vendor-specific (Intel).

Wrong - this patch is implemented according to the AHCI spec and has absolutely nothing vendor specific in it.