AHCI power saving (was Re: Ten hours on X60s)

AHCI power saving (was Re: Ten hours on X60s)

[Pavel Machek]

Hi!

> There are a couple of bits for turning on the hardware's
> power-saving. It makes me think it might save me about a watt, but the
> effect could be entirely psychological.

No, I did not mean _those_ bits. This made little or difference for
me... (100mW or so, definitely not watt).

doing ahci_pci_device_{suspend,resume} should definitely do the trick,
and ahci_{start,stop}_engine might be enough.

> Here is the patch. It is not correct and ready for general use,
> because you are supposed to check whether the AHCI chipset supports
> the feature.

> So there is no example code for sending the AHCI chipset to S3 and
> bringing it back? I thought you said there was before, but I can't
> find it!

ahci_pci_device_{suspend,resume} seems to be the code...

Here's the port of your patch to recent -git.
								Pavel

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index cef2e70..82a8a44 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -148,6 +148,8 @@ enum {
 				  PORT_IRQ_PIOS_FIS | PORT_IRQ_D2H_REG_FIS,
 
 	/* PORT_CMD bits */
+	PORT_CMD_ALPE		= (1 << 27), /* Aggressive Link Power Management Enable */
+	PORT_CMD_ASP		= (1 << 26), /* Aggressive entrance to Slumber or Partial power management states */
 	PORT_CMD_ATAPI		= (1 << 24), /* Device is ATAPI */
 	PORT_CMD_LIST_ON	= (1 << 15), /* cmd list DMA engine running */
 	PORT_CMD_FIS_ON		= (1 << 14), /* FIS DMA engine running */
@@ -486,7 +488,7 @@ static void ahci_power_up(void __iomem *
 	}
 
 	/* wake up link */
-	writel(cmd | PORT_CMD_ICC_ACTIVE, port_mmio + PORT_CMD);
+	writel(cmd | PORT_CMD_ICC_ACTIVE | PORT_CMD_ALPE | PORT_CMD_ASP, port_mmio + PORT_CMD);
 }
 
 static void ahci_power_down(void __iomem *port_mmio, u32 cap)

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: AHCI power saving (was Re: Ten hours on X60s)

[Jeff Garzik]

Pavel Machek wrote:
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -148,6 +148,8 @@ enum {
>  				  PORT_IRQ_PIOS_FIS | PORT_IRQ_D2H_REG_FIS,
>  
>  	/* PORT_CMD bits */
> +	PORT_CMD_ALPE		= (1 << 27), /* Aggressive Link Power Management Enable */
> +	PORT_CMD_ASP		= (1 << 26), /* Aggressive entrance to Slumber or Partial power management states */
>  	PORT_CMD_ATAPI		= (1 << 24), /* Device is ATAPI */
>  	PORT_CMD_LIST_ON	= (1 << 15), /* cmd list DMA engine running */
>  	PORT_CMD_FIS_ON		= (1 << 14), /* FIS DMA engine running */
> @@ -486,7 +488,7 @@ static void ahci_power_up(void __iomem *
>  	}
>  
>  	/* wake up link */
> -	writel(cmd | PORT_CMD_ICC_ACTIVE, port_mmio + PORT_CMD);
> +	writel(cmd | PORT_CMD_ICC_ACTIVE | PORT_CMD_ALPE | PORT_CMD_ASP, port_mmio + PORT_CMD);


Therein lies a key problem.  Turning on all of AHCI's aggressive power 
management features DOES save a lot of power.  But at the same time, it 
shortens the life of your hard drive, particularly hard drives that are 
really PATA, but have a PATA<->SATA bridge glued on the drive to enable 
connection to SATA controllers.

	Jeff

Re: AHCI power saving (was Re: Ten hours on X60s)

[Andi Kleen]

> Therein lies a key problem.  Turning on all of AHCI's aggressive power 
> management features DOES save a lot of power.  But at the same time, it 
> shortens the life of your hard drive, particularly hard drives that are 
> really PATA, but have a PATA<->SATA bridge glued on the drive to enable 
> connection to SATA controllers.

How does it shorten its life?

-Andi

Re: AHCI power saving (was Re: Ten hours on X60s)

[Jeff Garzik]

Andi Kleen wrote:
>> Therein lies a key problem.  Turning on all of AHCI's aggressive power 
>> management features DOES save a lot of power.  But at the same time, it 
>> shortens the life of your hard drive, particularly hard drives that are 
>> really PATA, but have a PATA<->SATA bridge glued on the drive to enable 
>> connection to SATA controllers.
> 
> How does it shorten its life?

Parks your hard drive heads many thousands of times more often than it 
does without the aggressive PM features.

	Jeff

Re: AHCI power saving (was Re: Ten hours on X60s)

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 781 bytes --]

On Mon, 13 Nov 2006 10:48:53 EST, Jeff Garzik said:
> Andi Kleen wrote:
> >> Therein lies a key problem.  Turning on all of AHCI's aggressive power 
> >> management features DOES save a lot of power.  But at the same time, it 
> >> shortens the life of your hard drive, particularly hard drives that are 
> >> really PATA, but have a PATA<->SATA bridge glued on the drive to enable 
> >> connection to SATA controllers.
> > 
> > How does it shorten its life?
> 
> Parks your hard drive heads many thousands of times more often than it 
> does without the aggressive PM features.

I've heard tell that the controller chip chews almost as much power as
the drive motor does - is there an option that says "Controller take a
snooze till we wake you up, but leave the drive spinning"?

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: AHCI power saving (was Re: Ten hours on X60s)

[Mark Lord]

Jeff Garzik wrote:
> Andi Kleen wrote:
>
>> How does it shorten its life?
> 
> Parks your hard drive heads many thousands of times more often than it 
> does without the aggressive PM features.

Spinning-down would definitely shorten the drive lifespan.  Does it do that?

Parking heads is more like just doing some extra (long) seeks.
Is this documented somewhere as being a life-shortening action?

Cheers

Re: AHCI power saving (was Re: Ten hours on X60s)

[Pavel Machek]

On Mon 2006-11-13 12:58:10, Mark Lord wrote:
> Jeff Garzik wrote:
> >Andi Kleen wrote:
> >
> >>How does it shorten its life?
> >
> >Parks your hard drive heads many thousands of times more often than it 
> >does without the aggressive PM features.
> 
> Spinning-down would definitely shorten the drive lifespan.  Does it do that?

Not on my machine.

> Parking heads is more like just doing some extra (long) seeks.
> Is this documented somewhere as being a life-shortening action?

I do not see any effects after applying that patch.
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

HD head unloads (was: Re: AHCI power saving)

[Henrique de Moraes Holschuh]

On Mon, 13 Nov 2006, Pavel Machek wrote:
> On Mon 2006-11-13 12:58:10, Mark Lord wrote:
> > Jeff Garzik wrote:
> > >Andi Kleen wrote:
> > >
> > >>How does it shorten its life?
> > >
> > >Parks your hard drive heads many thousands of times more often than it 
> > >does without the aggressive PM features.
> > 
> > Spinning-down would definitely shorten the drive lifespan.  Does it do that?
> 
> Not on my machine.

Heck, given just how much a ThinkPad T43 BIOS will attempt to do it for you,
consider yourself lucky if the X60 behaves differently.  When I thought of
monitoring the head unload counter through SMART on mine, my HD was already
beyond 14k unloads... and the notebook had been powered up less than 100
times :p

The BIOS likes to set the drive APM mode to something other than "off", and
in many drives (well, Hitachi ones at least), that means the drive will be
happy to unload heads every chance it gets, so as to be able to power off
the head assembly motion drive.

> > Parking heads is more like just doing some extra (long) seeks.

Long seeks don't lift the head assembly off the plates, head unloads do.
And head unloads will also power down some stuff in laptop HDs, seeks don't
do that either.

And even old-style parking places the heads on a different surface than the
data area.  That's a lot different from seeks no matter how one looks at it.

> > Is this documented somewhere as being a life-shortening action?

Yes, although not often with that many words.

For example, a Hitachi Travelstar 5k100 is rated for 600k load/unload
cycles, and 20k emergency load/unload cycles (each emergency unload counts
as 30 normal unloads, but the tech docs say it is about 100 times more
stressfull to the drive).  It is in the public drive datasheet, along with
other important information, such as that the drive needs to spin down often
(no less than once every 48h) or its lifespan will be shortened.

A typical desktop HD can probably survive a lot less head load/unload
cycles and spin up/down cycles than that.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: HD head unloads (was: Re: AHCI power saving)

[Jan Engelhardt]

On Nov 14 2006 01:43, Henrique de Moraes Holschuh wrote:
>On Mon, 13 Nov 2006, Pavel Machek wrote:
>>On Mon 2006-11-13 12:58:10, Mark Lord wrote:
>>>Jeff Garzik wrote:
>>>>Andi Kleen wrote:
>>>>
>>>>>How does it shorten its life?
>>>>
>>>>Parks your hard drive heads many thousands of times more often than it 
>>>>does without the aggressive PM features.
>>> 
>>>Spinning-down would definitely shorten the drive lifespan.  Does 
>>>it do that?
>> 
>>Not on my machine.
>
>Heck, given just how much a ThinkPad T43 BIOS will attempt to do it for you,
>consider yourself lucky if the X60 behaves differently.  When I thought of
>monitoring the head unload counter through SMART on mine, my HD was already
>beyond 14k unloads... and the notebook had been powered up less than 100
>times :p

Let me jump in here. Short info: Toshiba MK2003GAH 1.8" 20GB 
PATA harddisk, in a Sony Vaio U3 (x86, gray-blue PhoenixBIOS).
If idle for more than 5 secs, unloads. Even when not inside any OS, 
which really sets me off.
    So I wrote a quick workaround hack for Linux, http://tinyurl.com/y3qs6g
It reads a predefined amount of bytes (just as much to not cause 
slowdown yet still cause it to not unload) from the disk at fixed 
intervals.

>The BIOS likes to set the drive APM mode to something other than "off", and
>in many drives (well, Hitachi ones at least), that means the drive will be
>happy to unload heads every chance it gets, so as to be able to power off
>the head assembly motion drive.
>
>> > Parking heads is more like just doing some extra (long) seeks.
>
>Long seeks don't lift the head assembly off the plates, head unloads do.
>And head unloads will also power down some stuff in laptop HDs, seeks don't
>do that either.

Parking heads is worse than a seek - it takes more time to reload it 
than to seek to the other side.

>And even old-style parking places the heads on a different surface than the
>data area.  That's a lot different from seeks no matter how one looks at it.
>
>> > Is this documented somewhere as being a life-shortening action?
>
>Yes, although not often with that many words.
>
>For example, a Hitachi Travelstar 5k100 is rated for 600k load/unload
>cycles, and 20k emergency load/unload cycles (each emergency unload counts
>as 30 normal unloads, but the tech docs say it is about 100 times more
>stressfull to the drive).  It is in the public drive datasheet, along with
>other important information, such as that the drive needs to spin down often
>(no less than once every 48h) or its lifespan will be shortened.
>
>A typical desktop HD can probably survive a lot less head load/unload
>cycles and spin up/down cycles than that.
>
>-- 
>  "One disk to rule them all, One disk to find them. One disk to bring
>  them all and in the darkness grind them. In the Land of Redmond
>  where the shadows lie." -- The Silicon Valley Tarot
>  Henrique Holschuh
>-

	-`J'
-- 

Re: HD head unloads

[Mark Lord]

Jan Engelhardt wrote:
>
> Let me jump in here. Short info: Toshiba MK2003GAH 1.8" 20GB 
> PATA harddisk, in a Sony Vaio U3 (x86, gray-blue PhoenixBIOS).
> If idle for more than 5 secs, unloads. Even when not inside any OS, 
> which really sets me off.
>     So I wrote a quick workaround hack for Linux, http://tinyurl.com/y3qs6g
> It reads a predefined amount of bytes (just as much to not cause 
> slowdown yet still cause it to not unload) from the disk at fixed 
> intervals.

Thanks for the info.
Jan, in your specific case, can you not "fix it" properly with:

    hdparm -B255 /dev/?d?

(fill in your drive device there).

???

Re: HD head unloads

[Jan Engelhardt]

On Nov 14 2006 13:06, Mark Lord wrote:
> Jan Engelhardt wrote:
>> 
>> Let me jump in here. Short info: Toshiba MK2003GAH 1.8" 20GB PATA
>> harddisk, in a Sony Vaio U3 (x86, gray-blue PhoenixBIOS).
>> If idle for more than 5 secs, unloads. Even when not inside any OS, which
>> really sets me off.
>>    So I wrote a quick workaround hack for Linux, http://tinyurl.com/y3qs6g
>> It reads a predefined amount of bytes (just as much to not cause slowdown
>> yet still cause it to not unload) from the disk at fixed intervals.
>
> Thanks for the info.
> Jan, in your specific case, can you not "fix it" properly with:
>
>   hdparm -B255 /dev/?d?

No not really. The unload threshold only raises up to about 15 seconds.


	-`J'
-- 

Re: HD head unloads

[Jan Engelhardt]

On Nov 15 2006 11:58, Jan Engelhardt wrote:
>On Nov 14 2006 13:06, Mark Lord wrote:
>> Jan Engelhardt wrote:
>>> 
>>> Let me jump in here. Short info: Toshiba MK2003GAH 1.8" 20GB PATA
>>> harddisk, in a Sony Vaio U3 (x86, gray-blue PhoenixBIOS).
>>> If idle for more than 5 secs, unloads. Even when not inside any OS, which
>>> really sets me off.
>>>    So I wrote a quick workaround hack for Linux, http://tinyurl.com/y3qs6g
>>> It reads a predefined amount of bytes (just as much to not cause slowdown
>>> yet still cause it to not unload) from the disk at fixed intervals.
>>
>> Thanks for the info.
>> Jan, in your specific case, can you not "fix it" properly with:
>>
>>   hdparm -B255 /dev/?d?
>
>No not really. The unload threshold only raises up to about 15 seconds.

-B254 was more promising: unload timeout was like 2
minutes. What's more: upon every reboot, the power
management value is set back to 128.
    I guess I'll stay with thkd.ko for more time.

	-`J'
-- 

Re: AHCI power saving (was Re: Ten hours on X60s)

[Tejun Heo]

Mark Lord wrote:
> Jeff Garzik wrote:
>> Andi Kleen wrote:
>>
>>> How does it shorten its life?
>>
>> Parks your hard drive heads many thousands of times more often than it 
>> does without the aggressive PM features.
> 
> Spinning-down would definitely shorten the drive lifespan.  Does it do 
> that?
> 
> Parking heads is more like just doing some extra (long) seeks.
> Is this documented somewhere as being a life-shortening action?

I wrote this in the other thread but writing here too for documentation 
purpose.

* HL-DT-ST DVD-RAM GSA-H30N locks up completely on slumber.  Physical 
power removal and reapply is the only to recover it.

* Some WD raptors spin down (yeap, that's right, it spins down) on slumber.

Wonderful world of ATA.  :-P

-- 
tejun

Re: AHCI power saving (was Re: Ten hours on X60s)

[Pavel Machek]

Hi!

> >>>How does it shorten its life?
> >>
> >>Parks your hard drive heads many thousands of times more often than it 
> >>does without the aggressive PM features.
> >
> >Spinning-down would definitely shorten the drive lifespan.  Does it do 
> >that?
> >
> >Parking heads is more like just doing some extra (long) seeks.
> >Is this documented somewhere as being a life-shortening action?
> 
> I wrote this in the other thread but writing here too for documentation 
> purpose.
> 
> * HL-DT-ST DVD-RAM GSA-H30N locks up completely on slumber.  Physical 
> power removal and reapply is the only to recover it.
> 
> * Some WD raptors spin down (yeap, that's right, it spins down) on slumber.
> 
> Wonderful world of ATA.  :-P

Do you have some patches to play with? I tried this to get links down,
and it did not save much (if any) power.

(Last hunk is actually tiny cleanup...)
								Pavel

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index cef2e70..13ef1c5 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -50,6 +50,7 @@ #include <asm/io.h>
 #define DRV_NAME	"ahci"
 #define DRV_VERSION	"2.0"
 
+#define POWER_SAVE
 
 enum {
 	AHCI_PCI_BAR		= 5,
@@ -148,6 +149,8 @@ enum {
 				  PORT_IRQ_PIOS_FIS | PORT_IRQ_D2H_REG_FIS,
 
 	/* PORT_CMD bits */
+	PORT_CMD_ALPE		= (1 << 27), /* Aggressive Link Power Management Enable */
+	PORT_CMD_ASP		= (1 << 26), /* Aggressive entrance to Slumber or Partial power management states */
 	PORT_CMD_ATAPI		= (1 << 24), /* Device is ATAPI */
 	PORT_CMD_LIST_ON	= (1 << 15), /* cmd list DMA engine running */
 	PORT_CMD_FIS_ON		= (1 << 14), /* FIS DMA engine running */
@@ -407,6 +410,9 @@ static void ahci_start_engine(void __iom
 	tmp |= PORT_CMD_START;
 	writel(tmp, port_mmio + PORT_CMD);
 	readl(port_mmio + PORT_CMD); /* flush */
+#ifdef POWER_SAVE_V
+	printk("Starting engine\n");
+#endif
 }
 
 static int ahci_stop_engine(void __iomem *port_mmio)
@@ -422,6 +428,9 @@ static int ahci_stop_engine(void __iomem
 	/* setting HBA to idle */
 	tmp &= ~PORT_CMD_START;
 	writel(tmp, port_mmio + PORT_CMD);
+#ifdef POWER_SAVE_V
+	printk("Stopping engine\n");
+#endif
 
 	/* wait for engine to stop. This could be as long as 500 msec */
 	tmp = ata_wait_register(port_mmio + PORT_CMD,
@@ -486,7 +495,7 @@ static void ahci_power_up(void __iomem *
 	}
 
 	/* wake up link */
-	writel(cmd | PORT_CMD_ICC_ACTIVE, port_mmio + PORT_CMD);
+	writel(cmd | PORT_CMD_ICC_ACTIVE | PORT_CMD_ALPE | PORT_CMD_ASP, port_mmio + PORT_CMD);
 }
 
 static void ahci_power_down(void __iomem *port_mmio, u32 cap)
@@ -917,6 +926,14 @@ static void ahci_qc_prep(struct ata_queu
 	const u32 cmd_fis_len = 5; /* five dwords */
 	unsigned int n_elem;
 
+#ifdef POWER_SAVE
+	struct ahci_host_priv *hpriv = ap->host->private_data;
+	void __iomem *mmio = ap->host->mmio_base;
+	void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no);
+
+	ahci_port_resume(ap);
+#endif
+
 	/*
 	 * Fill in command table information.  First, the header,
 	 * a SATA Register - Host to Device command FIS.
@@ -1029,8 +1046,17 @@ static void ahci_host_intr(struct ata_po
 		qc_active = readl(port_mmio + PORT_CMD_ISSUE);
 
 	rc = ata_qc_complete_multiple(ap, qc_active, NULL);
-	if (rc > 0)
+	if (rc > 0) {
+#ifdef POWER_SAVE
+		struct ahci_host_priv *hpriv = ap->host->private_data;
+		void __iomem *mmio = ap->host->mmio_base;
+		void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no);
+
+		hpriv->cap &= ~HOST_CAP_SSC;
+		ahci_port_suspend(ap, PMSG_SUSPEND);
+#endif
 		return;
+	}
 	if (rc < 0) {
 		ehi->err_mask |= AC_ERR_HSM;
 		ehi->action |= ATA_EH_SOFTRESET;
@@ -1367,7 +1393,7 @@ static int ahci_host_init(struct ata_pro
 
 	hpriv->cap = readl(mmio + HOST_CAP);
 	hpriv->port_map = readl(mmio + HOST_PORTS_IMPL);
-	probe_ent->n_ports = (hpriv->cap & 0x1f) + 1;
+	probe_ent->n_ports = 1; /* (hpriv->cap & 0x1f) + 1; */
 
 	VPRINTK("cap 0x%x  port_map 0x%x  n_ports %d\n",
 		hpriv->cap, hpriv->port_map, probe_ent->n_ports);
@@ -1543,12 +1569,11 @@ static int ahci_init_one (struct pci_dev
 	}
 	base = (unsigned long) mmio_base;
 
-	hpriv = kmalloc(sizeof(*hpriv), GFP_KERNEL);
+	hpriv = kzalloc(sizeof(*hpriv), GFP_KERNEL);
 	if (!hpriv) {
 		rc = -ENOMEM;
 		goto err_out_iounmap;
 	}
-	memset(hpriv, 0, sizeof(*hpriv));
 
 	probe_ent->sht		= ahci_port_info[board_idx].sht;
 	probe_ent->port_flags	= ahci_port_info[board_idx].flags;


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: AHCI power saving (was Re: Ten hours on X60s)

[Alan]

On Mon, 13 Nov 2006 10:32:24 -0500
Jeff Garzik <jeff@garzik.org> wrote:

> Therein lies a key problem.  Turning on all of AHCI's aggressive power 
> management features DOES save a lot of power.  But at the same time, it 
> shortens the life of your hard drive, particularly hard drives that are 
> really PATA, but have a PATA<->SATA bridge glued on the drive to enable 
> connection to SATA controllers.

We already detect PATA over SATA so we can trivially address that (the
knobble check). Similarly we can turn ALPME on using a timer so it kicks
in after long idle periods only, or tie it to laptop mode.

For a laptop type environment with SATA not bridged PATA turning it on is
clearly the right thing to do.

Alan

Re: AHCI power saving (was Re: Ten hours on X60s)

[Pavel Machek]

Hi!

> >--- a/drivers/ata/ahci.c
> >+++ b/drivers/ata/ahci.c
> >@@ -148,6 +148,8 @@ enum {
> > 				  PORT_IRQ_PIOS_FIS | PORT_IRQ_D2H_REG_FIS,
> > 
> > 	/* PORT_CMD bits */
> >+	PORT_CMD_ALPE		= (1 << 27), /* Aggressive Link Power 
> >Management Enable */
> >+	PORT_CMD_ASP		= (1 << 26), /* Aggressive entrance to 
> >Slumber or Partial power management states */
> > 	PORT_CMD_ATAPI		= (1 << 24), /* Device is ATAPI */
> > 	PORT_CMD_LIST_ON	= (1 << 15), /* cmd list DMA engine running 
> > 	*/
> > 	PORT_CMD_FIS_ON		= (1 << 14), /* FIS DMA engine running */
> >@@ -486,7 +488,7 @@ static void ahci_power_up(void __iomem *
> > 	}
> > 
> > 	/* wake up link */
> >-	writel(cmd | PORT_CMD_ICC_ACTIVE, port_mmio + PORT_CMD);
> >+	writel(cmd | PORT_CMD_ICC_ACTIVE | PORT_CMD_ALPE | PORT_CMD_ASP, 
> >port_mmio + PORT_CMD);
> 
> 
> Therein lies a key problem.  Turning on all of AHCI's aggressive power 
> management features DOES save a lot of power.  But at the same time, it 
> shortens the life of your hard drive, particularly hard drives that are 
> really PATA, but have a PATA<->SATA bridge glued on the drive to enable 
> connection to SATA controllers.

Well, it would be useful to do on notebooks. I believe notebook hard
drives are okay with this kind of use... Heck, I've seen notebooks
with 5 seconds of spindown time.

Anyway, I tried stopping the DMA engine when no requests are
processed. I expected to see that 1W power saving, but nothing.

ahci_stop_engine does ...

        tmp = readl(port_mmio + PORT_CMD);

        /* check if the HBA is idle */
        if ((tmp & (PORT_CMD_START | PORT_CMD_LIST_ON)) == 0)
                return 0;

        /* setting HBA to idle */
        tmp &= ~PORT_CMD_START;
        writel(tmp, port_mmio + PORT_CMD);
        printk("Stopping engine\n");

... is PORT_CMD_START that bit that was expected to save 1W?

								Pavel

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index cef2e70..060d4c9 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -50,6 +50,7 @@ #include <asm/io.h>
 #define DRV_NAME	"ahci"
 #define DRV_VERSION	"2.0"
 
+#define POWER_SAVE
 
 enum {
 	AHCI_PCI_BAR		= 5,
@@ -148,6 +149,8 @@ enum {
 				  PORT_IRQ_PIOS_FIS | PORT_IRQ_D2H_REG_FIS,
 
 	/* PORT_CMD bits */
+	PORT_CMD_ALPE		= (1 << 27), /* Aggressive Link Power Management Enable */
+	PORT_CMD_ASP		= (1 << 26), /* Aggressive entrance to Slumber or Partial power management states */
 	PORT_CMD_ATAPI		= (1 << 24), /* Device is ATAPI */
 	PORT_CMD_LIST_ON	= (1 << 15), /* cmd list DMA engine running */
 	PORT_CMD_FIS_ON		= (1 << 14), /* FIS DMA engine running */
@@ -422,6 +425,7 @@ static int ahci_stop_engine(void __iomem
 	/* setting HBA to idle */
 	tmp &= ~PORT_CMD_START;
 	writel(tmp, port_mmio + PORT_CMD);
+	printk("Stopping engine\n");
 
 	/* wait for engine to stop. This could be as long as 500 msec */
 	tmp = ata_wait_register(port_mmio + PORT_CMD,
@@ -486,7 +490,7 @@ static void ahci_power_up(void __iomem *
 	}
 
 	/* wake up link */
-	writel(cmd | PORT_CMD_ICC_ACTIVE, port_mmio + PORT_CMD);
+	writel(cmd | PORT_CMD_ICC_ACTIVE | PORT_CMD_ALPE | PORT_CMD_ASP, port_mmio + PORT_CMD);
 }
 
 static void ahci_power_down(void __iomem *port_mmio, u32 cap)
@@ -917,6 +921,13 @@ static void ahci_qc_prep(struct ata_queu
 	const u32 cmd_fis_len = 5; /* five dwords */
 	unsigned int n_elem;
 
+#ifdef POWER_SAVE
+	void __iomem *mmio = ap->host->mmio_base;
+	void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no);
+
+	ahci_start_engine(port_mmio);
+#endif
+
 	/*
 	 * Fill in command table information.  First, the header,
 	 * a SATA Register - Host to Device command FIS.
@@ -1029,8 +1040,15 @@ static void ahci_host_intr(struct ata_po
 		qc_active = readl(port_mmio + PORT_CMD_ISSUE);
 
 	rc = ata_qc_complete_multiple(ap, qc_active, NULL);
-	if (rc > 0)
+	if (rc > 0) {
+#ifdef POWER_SAVE
+		void __iomem *mmio = ap->host->mmio_base;
+		void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no);
+
+		ahci_stop_engine(port_mmio);
+#endif
 		return;
+	}
 	if (rc < 0) {
 		ehi->err_mask |= AC_ERR_HSM;
 		ehi->action |= ATA_EH_SOFTRESET;


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: AHCI power saving

[John Fremlin]

Jeff Garzik <jeff@garzik.org> writes:

> Pavel Machek wrote:
>> --- a/drivers/ata/ahci.c
>> +++ b/drivers/ata/ahci.c
>> @@ -148,6 +148,8 @@ enum {
>>  				  PORT_IRQ_PIOS_FIS | PORT_IRQ_D2H_REG_FIS,
>>   	/* PORT_CMD bits */
>> +	PORT_CMD_ALPE		= (1 << 27), /* Aggressive Link Power Management Enable */
>> +	PORT_CMD_ASP		= (1 << 26), /* Aggressive entrance to Slumber or Partial power management states */
>>  	PORT_CMD_ATAPI		= (1 << 24), /* Device is ATAPI */
>>  	PORT_CMD_LIST_ON	= (1 << 15), /* cmd list DMA engine running */
>>  	PORT_CMD_FIS_ON		= (1 << 14), /* FIS DMA engine running */
>> @@ -486,7 +488,7 @@ static void ahci_power_up(void __iomem *
>>  	}
>>   	/* wake up link */
>> -	writel(cmd | PORT_CMD_ICC_ACTIVE, port_mmio + PORT_CMD);
>> +	writel(cmd | PORT_CMD_ICC_ACTIVE | PORT_CMD_ALPE | PORT_CMD_ASP, port_mmio + PORT_CMD);
>
>
> Therein lies a key problem.  Turning on all of AHCI's aggressive
> power management features DOES save a lot of power.  But at the same
> time, it shortens the life of your hard drive, particularly hard
> drives that are really PATA, but have a PATA<->SATA bridge glued on
> the drive to enable connection to SATA controllers.

Are you sure? I thought that these bits will only affect the SATA
communication between the chipset on the motherboard and the chipset
on the drive and are not related to the drive's spinning at all.

Please note that the patch is incorrect because not all AHCI chipsets
support these PM features. It would probably be best to make it
optional: it might slow down drive access, but does certainly save the
environment a little.