Re: [ugly patch] Save .15W-.5W by AHCI powersaving

2008-02-26 Thread Mark Lord

Pavel Machek wrote:

Hi!


This is a patch (very ugly, assumes you have just one disk) to bring
powersaving to AHCI. You need Alan's SCSI autosuspend (attached) patch
as a base.

It saves .5W compared to config with disk spinning, and even .15W
compared to hdparm -y... on my thinkpad x60 anyway.

..

There was a discussion of this here today.


Real-life discussion, or something I could read? :-).


It makes good use of AHCI-specific features.

Has it been tested with a Port-Multiplier yet?


I do not know what port-multiplier is, sorry. But it was not really
tested. It is not expected to work on any other config than notebook
very similar to mine.


This is cool enough that we really ought to do a hardware-independent
version, so that all SATA interfaces could benefit.  Especially ata_piix,
but others too.


Well, it seems like it is 10 lines per driver once Alan's SCSI
autosuspend patches are in...

..

Cool (literally)!

I think I might have gotten your patch confused in my mind
with another AHCI patch, which uses features of the chip itself
to automatically negotiate/change link power status on the fly
(no s/w needed, other than to turn it on).

That one is very ACPI specific, though.


-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [ugly patch] Save .15W-.5W by AHCI powersaving

2008-02-26 Thread Matthew Garrett
On Mon, Feb 25, 2008 at 05:42:58PM -0500, Jeff Garzik wrote:

 BTW we can also save power by allowing the user to choose to disable 
 hotplugging support.  Then we can power down PHYs that are not in use.
 
 That requires the addition of some policy controls, because it is 
 user-specific whether or not to waste power waiting for a plug-in event.

For AHCI, if you've enabled link power management then you've already 
disabled hotplug. We might as well power down unused phys in that case. 
Note that laptop bays still seem to tend to use platform-specific 
hotplug notification, even when they're sata - we'll get the hotplug 
notify for them even if the phy's powered down, so that case also needs 
to be handled.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[ugly patch] Save .15W-.5W by AHCI powersaving

2008-02-25 Thread Pavel Machek
Hi!

This is a patch (very ugly, assumes you have just one disk) to bring
powersaving to AHCI. You need Alan's SCSI autosuspend (attached) patch
as a base.

It saves .5W compared to config with disk spinning, and even .15W
compared to hdparm -y... on my thinkpad x60 anyway.

It is also mandatory first step towards sleepy linux ;-).

Pavel
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 29e71bd..0197b1f 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -259,8 +260,8 @@ static void ahci_fill_cmd_slot(struct ah
   u32 opts);
 #ifdef CONFIG_PM
 static int ahci_port_suspend(struct ata_port *ap, pm_message_t mesg);
-static int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg);
-static int ahci_pci_device_resume(struct pci_dev *pdev);
+int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg);
+int ahci_pci_device_resume(struct pci_dev *pdev);
 #endif
 
 static struct class_device_attribute *ahci_shost_attrs[] = {
@@ -268,6 +269,35 @@ static struct class_device_attribute *ah
NULL
 };
 
+struct pci_dev *my_pdev;
+int autosuspend_enabled;
+
+/* The host and its devices are all idle so we can autosuspend */
+static int autosuspend(struct Scsi_Host *host)
+{
+   if (my_pdev  autosuspend_enabled) {
+   printk(ahci: should autosuspend\n);
+   ahci_pci_device_suspend(my_pdev, PMSG_SUSPEND);
+   return 0;
+   } 
+   printk(ahci: autosuspend disabled\n);
+   return -EINVAL;
+}
+
+/* The host needs to be autoresumed */
+static int autoresume(struct Scsi_Host *host)
+{
+   if (my_pdev  autosuspend_enabled) {
+   printk(ahci: should autoresume\n);
+   ahci_pci_device_resume(my_pdev);
+   return 0;
+   }
+   printk(ahci: autoresume disabled\n);
+   return -EINVAL;
+}
+
+
+
 static struct scsi_host_template ahci_sht = {
.module = THIS_MODULE,
.name   = DRV_NAME,
@@ -286,6 +322,8 @@ static struct scsi_host_template ahci_sh
.slave_destroy  = ata_scsi_slave_destroy,
.bios_param = ata_std_bios_param,
.shost_attrs= ahci_shost_attrs,
+   .autosuspend= autosuspend,
+   .autoresume = autoresume,
 };
 
 static const struct ata_port_operations ahci_ops = {
@@ -2300,8 +2356,12 @@ static int ahci_init_one(struct pci_dev 
ahci_print_info(host);
 
pci_set_master(pdev);
-   return ata_host_activate(host, pdev-irq, ahci_interrupt, IRQF_SHARED,
+
+   rc = ata_host_activate(host, pdev-irq, ahci_interrupt, IRQF_SHARED,
 ahci_sht);
+   pci_save_state(pdev);
+   my_pdev = pdev;
+   return rc;
 }
 
 static int __init ahci_init(void)
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 4e31071..5c40ac2 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -380,7 +381,7 @@ enum scsi_eh_timer_return ata_scsi_timed
  * Inherited from SCSI layer (none, can sleep)
  *
  * RETURNS:
- * Zero.
+ * Nothing.
  */
 void ata_scsi_error(struct Scsi_Host *host)
 {
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index c838e65..0edc25e 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -484,6 +484,8 @@ static int scsi_try_host_reset(struct sc
if (scsi_autoresume_host(shost) != 0)
return FAILED;
 
+   rtn = shost-hostt-eh_host_reset_handler(scmd);
+
if (rtn == SUCCESS) {
if (!shost-hostt-skip_settle_delay)
ssleep(HOST_RESET_SETTLE_TIME);
@@ -1577,7 +1579,11 @@ int scsi_error_handler(void *data)
 * what we need to do to get it up and online again (if we can).
 * If we fail, we end up taking the thing offline.
 */
+#if 0
+   /* libata uses scsi_error_handler to suspend its parts; we 
deadlock
+  if we try to autoresume here */
autoresume_rc = scsi_autoresume_host(shost);
+#endif
if (shost-transportt-eh_strategy_handler)
shost-transportt-eh_strategy_handler(shost);
else
@@ -1591,8 +1597,10 @@ int scsi_error_handler(void *data)
 * which are still online.
 */
scsi_restart_operations(shost);
+#if 0
if (autoresume_rc == 0)
scsi_autosuspend_host(shost);
+#endif
set_current_state(TASK_INTERRUPTIBLE);
}
__set_current_state(TASK_RUNNING);
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 233feee..3c598e0 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -67,6 +67,8 @@ #undef SP
 
 static struct kmem_cache *scsi_bidi_sdb_cache;
 
+void scsi_run_queue(struct 

Re: [ugly patch] Save .15W-.5W by AHCI powersaving

2008-02-25 Thread Mark Lord

Pavel Machek wrote:

Hi!

This is a patch (very ugly, assumes you have just one disk) to bring
powersaving to AHCI. You need Alan's SCSI autosuspend (attached) patch
as a base.

It saves .5W compared to config with disk spinning, and even .15W
compared to hdparm -y... on my thinkpad x60 anyway.

..

There was a discussion of this here today.
It makes good use of AHCI-specific features.

Has it been tested with a Port-Multiplier yet?

This is cool enough that we really ought to do a hardware-independent
version, so that all SATA interfaces could benefit.  Especially ata_piix,
but others too.

Cheers
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [ugly patch] Save .15W-.5W by AHCI powersaving

2008-02-25 Thread Pavel Machek
Hi!

 This is a patch (very ugly, assumes you have just one disk) to bring
 powersaving to AHCI. You need Alan's SCSI autosuspend (attached) patch
 as a base.

 It saves .5W compared to config with disk spinning, and even .15W
 compared to hdparm -y... on my thinkpad x60 anyway.
 ..

 There was a discussion of this here today.

Real-life discussion, or something I could read? :-).

 It makes good use of AHCI-specific features.

 Has it been tested with a Port-Multiplier yet?

I do not know what port-multiplier is, sorry. But it was not really
tested. It is not expected to work on any other config than notebook
very similar to mine.

 This is cool enough that we really ought to do a hardware-independent
 version, so that all SATA interfaces could benefit.  Especially ata_piix,
 but others too.

Well, it seems like it is 10 lines per driver once Alan's SCSI
autosuspend patches are in...
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [ugly patch] Save .15W-.5W by AHCI powersaving

2008-02-25 Thread Jeff Garzik

Mark Lord wrote:

Pavel Machek wrote:

This is a patch (very ugly, assumes you have just one disk) to bring
powersaving to AHCI. You need Alan's SCSI autosuspend (attached) patch
as a base.

It saves .5W compared to config with disk spinning, and even .15W
compared to hdparm -y... on my thinkpad x60 anyway.

..

There was a discussion of this here today.
It makes good use of AHCI-specific features.

Has it been tested with a Port-Multiplier yet?

This is cool enough that we really ought to do a hardware-independent
version, so that all SATA interfaces could benefit.  Especially ata_piix,
but others too.


BTW we can also save power by allowing the user to choose to disable 
hotplugging support.  Then we can power down PHYs that are not in use.


That requires the addition of some policy controls, because it is 
user-specific whether or not to waste power waiting for a plug-in event.


Jeff



-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html