Re: SATA powersave patches

Re: SATA powersave patches

[Pavel Machek]

Hi!

(cc-ed to the list)

> >>>Do you have SATA powersave patches somewhere handy? I found out that
> >>>SATA eats way too much power.
> >>Hello, Pavel.
> >>
> >>Do you mean link powersaving?  Or disk spindown?
> >
> >Link powersaving... disk spindown should be handled by laptop_mode +
> >hdparm -y, right?
> 
> Yes for disk spindown.  Link powersaving is in the following git tree 
> but it's ~2 months old.
> 
> http://htj.dyndns.org/git/?p=libata-tj.git;a=shortlog;h=powersave
> git://htj.dyndns.org/libata-tj.git powersave

Thanks... I got it to work (on 2 the old tree, I was not able to
forward-port it), but power savings were not too big (~0.1W, maybe).

I'm getting huge (~1W) savings by powering down SATA controller, as in
ahci_pci_device_suspend(). It would be great to be able to power SATA
controller down, then power it back up when it is needed... I tried
following hack, but could not get it to work. Any ideas?

								Pavel

diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index 3e98334..b6e9f55 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -222,7 +222,7 @@ static void ahci_thaw(struct ata_port *a
 static void ahci_error_handler(struct ata_port *ap);
 static void ahci_post_internal_cmd(struct ata_queued_cmd *qc);
 static int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t state);
-static int ahci_pci_device_resume(struct pci_dev *pdev);
+ int ahci_pci_device_resume(struct pci_dev *pdev);
 static int ahci_port_standby(void __iomem *port_mmio, u32 cap);
 static int ahci_port_spinup(void __iomem *port_mmio, u32 cap);
 static int ahci_port_suspend(struct ata_port *ap, pm_message_t state);
@@ -1082,6 +1082,9 @@ static unsigned int ahci_fill_sg(struct 
 	return n_sg;
 }
 
+int suspended;
+struct pci_dev *mydev;
+
 int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t state)
 {
 	struct ata_host_set *host_set = dev_get_drvdata(&pdev->dev);
@@ -1089,6 +1092,11 @@ int ahci_pci_device_suspend(struct pci_d
 	u32 tmp;
 	int i;
 
+	if (suspended)
+		return;
+	suspended = 1;
+	mydev = pdev;
+
 	/* First suspend all ports */
 	for (i = 0; i < host_set->n_ports; i++) {
 		struct ata_port *ap;
@@ -1121,6 +1129,10 @@ int ahci_pci_device_resume(struct pci_de
 	struct ata_port *ap;
 	u32 i, tmp, irq_stat;
 
+	if (!suspended)
+		return;
+	suspended = 0;
+
 	tmp = readl(mmio + HOST_CTL);
 	if (!(tmp & HOST_AHCI_EN)) {
 		tmp |= HOST_AHCI_EN;
diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
index e92c31d..89fb997 100644
--- a/drivers/scsi/libata-scsi.c
+++ b/drivers/scsi/libata-scsi.c
@@ -2822,6 +2822,14 @@ int ata_scsi_queuecmd(struct scsi_cmnd *
 	struct Scsi_Host *shost = scsidev->host;
 	int rc = 0;
 
+	extern int suspended;
+	extern int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t state);
+	extern int ahci_pci_device_resume(struct pci_dev *pdev);
+	extern struct pci_dev *mydev;
+
+	if (suspended)
+		ahci_pci_device_resume(mydev);
+
 	ap = ata_shost_to_port(shost);
 
 	spin_unlock(shost->host_lock);




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

Re: SATA powersave patches

[Tejun Heo]

Hello, Pavel Machek.

Pavel Machek wrote:
> Thanks... I got it to work (on 2 the old tree, I was not able to
> forward-port it), but power savings were not too big (~0.1W, maybe).
> 
> I'm getting huge (~1W) savings by powering down SATA controller, as in
> ahci_pci_device_suspend().

Yeah, it only turns off SATA PHY, so it doesn't result in huge saving. 
IIRC, it was somewhere around 5 percent on my notebook w/ static 
linksave mode (turning PHY off on empty port).  But link powersaving 
introduces virtually no recognizable delay, so it's nice to have.

Can you check if there is any difference between [D/H]IPS and static? 
ICH6M on my notebook can't do DIPS/HIPS, so I couldn't compare them 
against static.

> It would be great to be able to power SATA
> controller down, then power it back up when it is needed... I tried
> following hack, but could not get it to work. Any ideas?

1. One way to do it would be by dynamic power management.  It would be 
nice to have wake-up mechanism at the block layer.  Idle timer can run 
in the block layer or it can be implemented in the userland.

ATM, this implies that the attached devices are powered down too 
(spindown).  As spinning up takes quite some time, we can implement 
another level of dynamic PM w/ shorter delay to wake up - drives are not 
spinned down but controllers are powered down completely.

In any case, channel reset and following revalidation are necessary on 
wake up - if the device is still spinning, this shouldn't take too long 
but it will introduce noticeable delay - probably under or around a sec.

2. Another hacky way would be implementing it as an extension of link 
powersaving.  I don't think this is a good idea tho.  Waking up a 
controller usually involves link reset which in turn requires 
revalidation and reconfiguration of attached device, which should be 
done from exception handler.

The reason why your hack doesn't work is probably this reason.  You need 
to pass the command to EH and tell it to perform full wake-up sequence 
and retry the command.

So, I think option #1 is the way to go - implementing leveled dynamic 
power management infrastructure and adding support in the block layer. 
What do you think?

Thanks.

-- 
tejun

Re: SATA powersave patches

[Pavel Machek]

Hi!

> >Thanks... I got it to work (on 2 the old tree, I was not able to
> >forward-port it), but power savings were not too big (~0.1W, maybe).
> >
> >I'm getting huge (~1W) savings by powering down SATA controller, as in
> >ahci_pci_device_suspend().
> 
> Yeah, it only turns off SATA PHY, so it doesn't result in huge saving. 
> IIRC, it was somewhere around 5 percent on my notebook w/ static 
> linksave mode (turning PHY off on empty port).  But link powersaving 
> introduces virtually no recognizable delay, so it's nice to have.

Yes, any powersavings without cost are a good idea. 

> Can you check if there is any difference between [D/H]IPS and static? 
> ICH6M on my notebook can't do DIPS/HIPS, so I couldn't compare them 
> against static.

What is D/HIPS? I could not find anything relevant..

> >It would be great to be able to power SATA
> >controller down, then power it back up when it is needed... I tried
> >following hack, but could not get it to work. Any ideas?
> 
> 1. One way to do it would be by dynamic power management.  It would be 
> nice to have wake-up mechanism at the block layer.  Idle timer can run 
> in the block layer or it can be implemented in the userland.
> 
> ATM, this implies that the attached devices are powered down too 
> (spindown).  As spinning up takes quite some time, we can implement 

For now, powering down controller when disks are spinned down would be
very nice first step.

When I forced disk to be spinned down (with power/state file)
controller actually survived power down/power up... unfortunately with
so long delay (~30 sec) that it is not usable in practice.

> So, I think option #1 is the way to go - implementing leveled dynamic 
> power management infrastructure and adding support in the block layer. 
> What do you think?

Would be nice :-).
								Pavel

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

Re: SATA powersave patches

[Tejun Heo]

Hello,

Pavel Machek wrote:
>> Can you check if there is any difference between [D/H]IPS and static? 
>> ICH6M on my notebook can't do DIPS/HIPS, so I couldn't compare them 
>> against static.
> 
> What is D/HIPS? I could not find anything relevant..

D/HIPS stand for device/host initiated power saving.  These modes use 
two SATA link powersaving state (partial and slumber).  Static mode 
simply turns off PHY on unoccupied port using SControl register.  So, if 
you have an access to a notebook which has a SATA dock which support 
link powersaving, you can test it by...

* set link powersaving mode to HIPS/static. (mode 4)

* w/ device inserted, leave it idle for 15 seconds and record power 
consumption level (link should be in slumber state).

* pull out the device, wait for libata to detach the device and record 
power consumption level (libata should have turned off PHY after 
detaching the device).

I wanna know whether there is any difference in the amount of power 
saved between slumber and off states.

>>> It would be great to be able to power SATA
>>> controller down, then power it back up when it is needed... I tried
>>> following hack, but could not get it to work. Any ideas?
>> 1. One way to do it would be by dynamic power management.  It would be 
>> nice to have wake-up mechanism at the block layer.  Idle timer can run 
>> in the block layer or it can be implemented in the userland.
>>
>> ATM, this implies that the attached devices are powered down too 
>> (spindown).  As spinning up takes quite some time, we can implement 
> 
> For now, powering down controller when disks are spinned down would be
> very nice first step.

Yeap.

> When I forced disk to be spinned down (with power/state file)
> controller actually survived power down/power up... unfortunately with
> so long delay (~30 sec) that it is not usable in practice.

Can you describe what you've done in more detail?  Do you have dmesg of 
the 30sec wait?

>> So, I think option #1 is the way to go - implementing leveled dynamic 
>> power management infrastructure and adding support in the block layer. 
>> What do you think?
> 
> Would be nice :-).

So, do you think we're ready for another PM infrastructure update?  :-P

-- 
tejun

Re: SATA powersave patches

[Pavel Machek]

Hi!

First, sorry for long reply time. I had too many horses and not enough
time.

> >>Can you check if there is any difference between [D/H]IPS and static? 
> >>ICH6M on my notebook can't do DIPS/HIPS, so I couldn't compare them 
> >>against static.
> >
> >What is D/HIPS? I could not find anything relevant..
> 
> D/HIPS stand for device/host initiated power saving.  These modes use 
> two SATA link powersaving state (partial and slumber).  Static mode 
> simply turns off PHY on unoccupied port using SControl register.  So, if 
> you have an access to a notebook which has a SATA dock which support 
> link powersaving, you can test it by...
> 
> * set link powersaving mode to HIPS/static. (mode 4)
> 
> * w/ device inserted, leave it idle for 15 seconds and record power 
> consumption level (link should be in slumber state).
> 
> * pull out the device, wait for libata to detach the device and record 
> power consumption level (libata should have turned off PHY after 
> detaching the device).
> 
> I wanna know whether there is any difference in the amount of power 
> saved between slumber and off states.

I'm probably doing something wrong, but...

I'm on commit 

commit 9a7b050525f7d70d2ed62affb691b9d4ca2b82d2
tree b8195e5625dc5bad6757b0dddec0dacf416a0779
parent 50c3086de212ce56eaa2bf284586fb021615b5e1
author Tejun Heo <htejun@gmail.com> Mon, 16 Oct 2006 07:24:57 +0900
committer Tejun Heo <htejun@gmail.com> Mon, 16 Oct 2006 07:24:57 +0900

    [PATCH] sata_sil24: implement PORT_RST

    As DEV_RST (hardreset) sometimes fail to recover the controller
    (especially after PMP DMA CS errata).  In such cases, perform
PORT_RST
    prior to DEV_RST.

    Signed-off-by: Tejun Heo <htejun@gmail.com>


(2.6.19-rc1)

and I do not see powersave tunable:

root@amd:/sys/module# ls libata/parameters/
ata_probe_timeout         atapi_enabled hotplug_polling_interval
atapi_dmadir              fua

...how do I pull working version?

> >>So, I think option #1 is the way to go - implementing leveled dynamic 
> >>power management infrastructure and adding support in the block layer. 
> >>What do you think?
> >
> >Would be nice :-).
> 
> So, do you think we're ready for another PM infrastructure update?  :-P

Well... things are pretty quiet in that area just now... So yes.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: SATA powersave patches

[Tejun Heo]

Pavel Machek wrote:
> I'm probably doing something wrong, but...
> 
> I'm on commit 
> 
> commit 9a7b050525f7d70d2ed62affb691b9d4ca2b82d2
> tree b8195e5625dc5bad6757b0dddec0dacf416a0779
> parent 50c3086de212ce56eaa2bf284586fb021615b5e1
> author Tejun Heo <htejun@gmail.com> Mon, 16 Oct 2006 07:24:57 +0900
> committer Tejun Heo <htejun@gmail.com> Mon, 16 Oct 2006 07:24:57 +0900
> 
>     [PATCH] sata_sil24: implement PORT_RST
> 
>     As DEV_RST (hardreset) sometimes fail to recover the controller
>     (especially after PMP DMA CS errata).  In such cases, perform
> PORT_RST
>     prior to DEV_RST.
> 
>     Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> (2.6.19-rc1)
> 
> and I do not see powersave tunable:
> 
> root@amd:/sys/module# ls libata/parameters/
> ata_probe_timeout         atapi_enabled hotplug_polling_interval
> atapi_dmadir              fua
> 
> ...how do I pull working version?

I haven't updated link powersave patch yet.  Core implementation was 
agreed on but interface hasn't been decided yet.  Maybe it's about time 
to add /sys/class/ata_{host|device}/.  So, the patchset is pushed back 
for the time being.

>>>> So, I think option #1 is the way to go - implementing leveled dynamic 
>>>> power management infrastructure and adding support in the block layer. 
>>>> What do you think?
>>> Would be nice :-).
>> So, do you think we're ready for another PM infrastructure update?  :-P
> 
> Well... things are pretty quiet in that area just now... So yes.

If I understood correctly, the high power consumption of ahci controller 
can be solved by dynamically turning off command processing while the 
controller is idle, which fits nicely into link powersaving, right?  So, 
I think full-fledged leveled dynamic PM would be an overkill for this 
particular problem, but then again, maybe the correct way to implement 
link powersaving is via leveled dynamic PM.  I'll give it more thoughts.

Thanks.

-- 
tejun

Re: SATA powersave patches

[Pavel Machek]

Hi!

> >and I do not see powersave tunable:
> >
> >root@amd:/sys/module# ls libata/parameters/
> >ata_probe_timeout         atapi_enabled 
> >hotplug_polling_interval
> >atapi_dmadir              fua
> >
> >...how do I pull working version?
> 
> I haven't updated link powersave patch yet.  Core 
> implementation was agreed on but interface hasn't been 
> decided yet.  Maybe it's about time to add 
> /sys/class/ata_{host|device}/.  So, the patchset is 
> pushed back for the time being.

/sys/class/ata* would probably be ok.

> >Well... things are pretty quiet in that area just 
> >now... So yes.
> 
> If I understood correctly, the high power consumption of 
> ahci controller can be solved by dynamically turning off 
> command processing while the controller is idle, which 
> fits nicely into link powersaving, right?  So, I think 
> full-fledged leveled dynamic PM would be an overkill for 
> this particular problem, but then again, maybe the 

It is single bit, and it should not even need a timeout, AFAICT, so
perhaps we should just fix it (no need for dynamic PM layers). It
probably does not even need to be configurable...

-- 
Thanks for all the (sleeping) penguins.

Re: SATA powersave patches

[Tejun Heo]

Hello,

Pavel Machek wrote:
>> If I understood correctly, the high power consumption of 
>> ahci controller can be solved by dynamically turning off 
>> command processing while the controller is idle, which 
>> fits nicely into link powersaving, right?  So, I think 
>> full-fledged leveled dynamic PM would be an overkill for 
>> this particular problem, but then again, maybe the 
> 
> It is single bit, and it should not even need a timeout, AFAICT, so
> perhaps we should just fix it (no need for dynamic PM layers). It
> probably does not even need to be configurable...

I think this has been discussed in linux-ide recently but just to add my 
2 cents.  ALPE and ASP can cause quite some problems.  Many devices 
don't implement link powermanagement mode properly and some locks up 
completely (recent LG dvd-rams, only physical power removal/reapply can 
recover it) while others (wd raptors) spin down on slumber.  If you turn 
on ALPE and ASP on such devices, all hell will break loose.

Even for devices which implement partial and slumber modes properly, I'm 
not very comfortable with ALPE.  It puts the link into power save mode 
as soon as there is no command pending.  IMHO, that's way too 
aggressive.  It's not like being that aggressive will bring any 
noticeable difference in power consumption.  Disk idle period is too 
long for that level of aggressiveness to be effective.

Thanks.

-- 
tejun

Re: SATA powersave patches

[Pavel Machek]

Hi!

> >>If I understood correctly, the high power consumption of 
> >>ahci controller can be solved by dynamically turning off 
> >>command processing while the controller is idle, which 
> >>fits nicely into link powersaving, right?  So, I think 
> >>full-fledged leveled dynamic PM would be an overkill for 
> >>this particular problem, but then again, maybe the 
> >
> >It is single bit, and it should not even need a timeout, AFAICT, so
> >perhaps we should just fix it (no need for dynamic PM layers). It
> >probably does not even need to be configurable...
> 
> I think this has been discussed in linux-ide recently but just to add my 
> 2 cents.  ALPE and ASP can cause quite some problems.  Many devices 
> don't implement link powermanagement mode properly and some locks up 

Ok, so it needs to be optional :-). I played with the code a little
bit, but did not find combination that both worked and saved power :-(.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html