* Re: SATA powersave patches [not found] ` <4501655F.5000103@gmail.com> @ 2006-09-10 22:48 ` Pavel Machek 2006-09-11 10:24 ` Tejun Heo 0 siblings, 1 reply; 9+ messages in thread From: Pavel Machek @ 2006-09-10 22:48 UTC (permalink / raw) To: Tejun Heo; +Cc: kernel list, axboe 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: SATA powersave patches 2006-09-10 22:48 ` SATA powersave patches Pavel Machek @ 2006-09-11 10:24 ` Tejun Heo 2006-09-18 10:05 ` Pavel Machek 0 siblings, 1 reply; 9+ messages in thread From: Tejun Heo @ 2006-09-11 10:24 UTC (permalink / raw) To: Pavel Machek; +Cc: kernel list, axboe 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: SATA powersave patches 2006-09-11 10:24 ` Tejun Heo @ 2006-09-18 10:05 ` Pavel Machek 2006-09-18 10:38 ` Tejun Heo 0 siblings, 1 reply; 9+ messages in thread From: Pavel Machek @ 2006-09-18 10:05 UTC (permalink / raw) To: Tejun Heo; +Cc: kernel list, axboe 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: SATA powersave patches 2006-09-18 10:05 ` Pavel Machek @ 2006-09-18 10:38 ` Tejun Heo 2006-11-06 13:57 ` Pavel Machek 0 siblings, 1 reply; 9+ messages in thread From: Tejun Heo @ 2006-09-18 10:38 UTC (permalink / raw) To: Pavel Machek; +Cc: kernel list, axboe 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: SATA powersave patches 2006-09-18 10:38 ` Tejun Heo @ 2006-11-06 13:57 ` Pavel Machek 2006-11-06 17:44 ` Tejun Heo 0 siblings, 1 reply; 9+ messages in thread From: Pavel Machek @ 2006-11-06 13:57 UTC (permalink / raw) To: Tejun Heo; +Cc: kernel list, axboe 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: SATA powersave patches 2006-11-06 13:57 ` Pavel Machek @ 2006-11-06 17:44 ` Tejun Heo 2006-11-12 18:39 ` Pavel Machek 0 siblings, 1 reply; 9+ messages in thread From: Tejun Heo @ 2006-11-06 17:44 UTC (permalink / raw) To: Pavel Machek; +Cc: kernel list, axboe 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: SATA powersave patches 2006-11-06 17:44 ` Tejun Heo @ 2006-11-12 18:39 ` Pavel Machek 2006-11-21 4:30 ` Tejun Heo 0 siblings, 1 reply; 9+ messages in thread From: Pavel Machek @ 2006-11-12 18:39 UTC (permalink / raw) To: Tejun Heo; +Cc: kernel list, axboe 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: SATA powersave patches 2006-11-12 18:39 ` Pavel Machek @ 2006-11-21 4:30 ` Tejun Heo 2006-11-21 23:07 ` Pavel Machek 0 siblings, 1 reply; 9+ messages in thread From: Tejun Heo @ 2006-11-21 4:30 UTC (permalink / raw) To: Pavel Machek; +Cc: kernel list, axboe 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: SATA powersave patches 2006-11-21 4:30 ` Tejun Heo @ 2006-11-21 23:07 ` Pavel Machek 0 siblings, 0 replies; 9+ messages in thread From: Pavel Machek @ 2006-11-21 23:07 UTC (permalink / raw) To: Tejun Heo; +Cc: kernel list, axboe 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2006-11-21 23:07 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20060908110346.GC920@elf.ucw.cz>
[not found] ` <45015767.1090002@gmail.com>
[not found] ` <20060908123537.GB17640@elf.ucw.cz>
[not found] ` <4501655F.5000103@gmail.com>
2006-09-10 22:48 ` SATA powersave patches Pavel Machek
2006-09-11 10:24 ` Tejun Heo
2006-09-18 10:05 ` Pavel Machek
2006-09-18 10:38 ` Tejun Heo
2006-11-06 13:57 ` Pavel Machek
2006-11-06 17:44 ` Tejun Heo
2006-11-12 18:39 ` Pavel Machek
2006-11-21 4:30 ` Tejun Heo
2006-11-21 23:07 ` Pavel Machek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox