[patch 1/2] allow user to power off unused ports via sysfs

[patch 1/2] allow user to power off unused ports via sysfs

[kristen.c.accardi]

This patch expands the definition of link power management to 
include the notion of simply powering the entire port off.  
We add a new valid value for link_power_management_policy:

power_off:  phy is not on at all.
min_power:  driver uses minimum possible power. hotplug
            may or may not be available.
medium_power: best power/performance tradeoff.  hotplug
            may or may not be available
max_performance: max performance without regard to power
            hot plug is available.

Additionally - this patch modifies the libata-core to
power off the port automatically at init time if the
driver marks the port as not hotpluggable.

The user may not power off a port if it is occupied.
If the port is powered off, and a new drive is plugged
in, the drive will not be detected until the port is
powered back up by setting the link_power_management_policy
to "max_performance".

Signed-off-by:  Kristen Carlson Accardi <kristen.c.accardi@intel.com>
---
 drivers/ata/libata-core.c |   58 +++++++++++++++++++++++++++++++++++++++++++++-
 drivers/ata/libata-scsi.c |    1 
 include/linux/libata.h    |    2 +
 3 files changed, 60 insertions(+), 1 deletion(-)

Index: linux-ahci-phy/drivers/ata/libata-core.c
===================================================================
--- linux-ahci-phy.orig/drivers/ata/libata-core.c	2008-06-27 13:32:51.000000000 -0700
+++ linux-ahci-phy/drivers/ata/libata-core.c	2008-07-01 16:33:00.000000000 -0700
@@ -162,6 +162,19 @@ MODULE_DESCRIPTION("Library module for A
 MODULE_LICENSE("GPL");
 MODULE_VERSION(DRV_VERSION);
 
+static void ata_phy_offline(struct ata_link *link)
+{
+	u32 scontrol;
+	int rc;
+
+	/* set DET to 4 */
+	rc = sata_scr_read(link, SCR_CONTROL, &scontrol);
+	if (rc)
+		return;
+	scontrol &= ~0xf;
+	scontrol |= (1 << 2);
+	sata_scr_write(link, SCR_CONTROL, scontrol);
+}
 
 /**
  *	ata_force_cbl - force cable type according to libata.force
@@ -953,6 +966,7 @@ static int ata_dev_set_dipm(struct ata_d
 		 * disallow all transitions which effectively
 		 * disable DIPM anyway.
 		 */
+	default:
 		break;
 	}
 
@@ -980,6 +994,22 @@ void ata_dev_enable_pm(struct ata_device
 	int rc = 0;
 	struct ata_port *ap = dev->link->ap;
 
+	/*
+	 * if we don't have a device attached, all we can
+	 * do is power off
+	 */
+	if (!ata_dev_enabled(dev) && policy == POWER_OFF) {
+		ap->flags |= ATA_FLAG_NO_HOTPLUG;
+		ata_phy_offline(&ap->link);
+		goto enable_pm_out;
+	}
+
+	/*
+         * if there's no device attached, we are done
+	 */
+	if (!ata_dev_enabled(dev))
+		return;
+
 	/* set HIPM first, then DIPM */
 	if (ap->ops->enable_pm)
 		rc = ap->ops->enable_pm(ap, policy);
@@ -1020,10 +1050,24 @@ static void ata_dev_disable_pm(struct at
 
 void ata_lpm_schedule(struct ata_port *ap, enum link_pm policy)
 {
-	ap->pm_policy = policy;
 	ap->link.eh_info.action |= ATA_EH_LPM;
+
+	/*
+	 * if we are coming from a state of OFF to ON, we need
+	 * to reset the link and scan for devices in case someone
+	 * plugged something in while we were off.
+	 * Clear the NO_HOTPLUG flag because we are leaving the
+	 * phy on now.
+	 */
+	if (ap->pm_policy == POWER_OFF && policy != POWER_OFF) {
+		ap->flags &= ~ATA_FLAG_NO_HOTPLUG;
+		ap->link.eh_info.probe_mask |= ATA_ALL_DEVICES;
+		ap->link.eh_info.flags |= ATA_EH_RESET;
+	}
 	ap->link.eh_info.flags |= ATA_EHI_NO_AUTOPSY;
+	ap->pm_policy = policy;
 	ata_port_schedule_eh(ap);
+	ata_port_wait_eh(ap);
 }
 
 #ifdef CONFIG_PM
@@ -2678,6 +2722,7 @@ void ata_port_disable(struct ata_port *a
 	ap->link.device[0].class = ATA_DEV_NONE;
 	ap->link.device[1].class = ATA_DEV_NONE;
 	ap->flags |= ATA_FLAG_DISABLED;
+	ata_phy_offline(&ap->link);
 }
 
 /**
@@ -5614,6 +5659,8 @@ int ata_host_register(struct ata_host *h
 		if (ap->ops->error_handler) {
 			struct ata_eh_info *ehi = &ap->link.eh_info;
 			unsigned long flags;
+			int device_attached = 0;
+			struct ata_device *dev;
 
 			ata_port_probe(ap);
 
@@ -5632,6 +5679,15 @@ int ata_host_register(struct ata_host *h
 
 			/* wait for EH to finish */
 			ata_port_wait_eh(ap);
+			ata_link_for_each_dev(dev, &ap->link)
+				if (ata_dev_enabled(dev))
+					device_attached++;
+			if (!device_attached &&
+			    (ap->flags & ATA_FLAG_NO_HOTPLUG)) {
+				/* no device present, disable port */
+				ap->pm_policy = POWER_OFF;
+				ata_port_disable(ap);
+			}
 		} else {
 			DPRINTK("ata%u: bus probe begin\n", ap->print_id);
 			rc = ata_bus_probe(ap);
Index: linux-ahci-phy/include/linux/libata.h
===================================================================
--- linux-ahci-phy.orig/include/linux/libata.h	2008-06-27 13:32:57.000000000 -0700
+++ linux-ahci-phy/include/linux/libata.h	2008-06-27 13:38:11.000000000 -0700
@@ -190,6 +190,7 @@ enum {
 	ATA_FLAG_AN		= (1 << 18), /* controller supports AN */
 	ATA_FLAG_PMP		= (1 << 19), /* controller supports PMP */
 	ATA_FLAG_IPM		= (1 << 20), /* driver can handle IPM */
+	ATA_FLAG_NO_HOTPLUG	= (1 << 21), /* port doesn't support HP */
 
 	/* The following flag belongs to ap->pflags but is kept in
 	 * ap->flags because it's referenced in many LLDs and will be
@@ -439,6 +440,7 @@ enum link_pm {
 	MIN_POWER,
 	MAX_PERFORMANCE,
 	MEDIUM_POWER,
+	POWER_OFF,
 };
 extern struct device_attribute dev_attr_link_power_management_policy;
 
Index: linux-ahci-phy/drivers/ata/libata-scsi.c
===================================================================
--- linux-ahci-phy.orig/drivers/ata/libata-scsi.c	2008-06-27 13:32:51.000000000 -0700
+++ linux-ahci-phy/drivers/ata/libata-scsi.c	2008-06-27 13:38:11.000000000 -0700
@@ -122,6 +122,7 @@ static const struct {
 	{ MIN_POWER, "min_power" },
 	{ MAX_PERFORMANCE, "max_performance" },
 	{ MEDIUM_POWER, "medium_power" },
+	{ POWER_OFF, "power_off" },
 };
 
 static const char *ata_scsi_lpm_get(enum link_pm policy)

-- 

Re: [patch 1/2] allow user to power off unused ports via sysfs

[Jeff Garzik]

kristen.c.accardi@intel.com wrote:
> This patch expands the definition of link power management to 
> include the notion of simply powering the entire port off.  
> We add a new valid value for link_power_management_policy:
> 
> power_off:  phy is not on at all.
> min_power:  driver uses minimum possible power. hotplug
>             may or may not be available.
> medium_power: best power/performance tradeoff.  hotplug
>             may or may not be available
> max_performance: max performance without regard to power
>             hot plug is available.
> 
> Additionally - this patch modifies the libata-core to
> power off the port automatically at init time if the
> driver marks the port as not hotpluggable.
> 
> The user may not power off a port if it is occupied.
> If the port is powered off, and a new drive is plugged
> in, the drive will not be detected until the port is
> powered back up by setting the link_power_management_policy
> to "max_performance".
> 
> Signed-off-by:  Kristen Carlson Accardi <kristen.c.accardi@intel.com>
> ---
>  drivers/ata/libata-core.c |   58 +++++++++++++++++++++++++++++++++++++++++++++-
>  drivers/ata/libata-scsi.c |    1 
>  include/linux/libata.h    |    2 +
>  3 files changed, 60 insertions(+), 1 deletion(-)

Looks good to me, ACK.

I'm apply soonish, unless there are other complaints.

Re: [patch 1/2] allow user to power off unused ports via sysfs

[Jeff Garzik]

kristen.c.accardi@intel.com wrote:
> This patch expands the definition of link power management to 
> include the notion of simply powering the entire port off.  
> We add a new valid value for link_power_management_policy:
> 
> power_off:  phy is not on at all.
> min_power:  driver uses minimum possible power. hotplug
>             may or may not be available.
> medium_power: best power/performance tradeoff.  hotplug
>             may or may not be available
> max_performance: max performance without regard to power
>             hot plug is available.
> 
> Additionally - this patch modifies the libata-core to
> power off the port automatically at init time if the
> driver marks the port as not hotpluggable.
> 
> The user may not power off a port if it is occupied.
> If the port is powered off, and a new drive is plugged
> in, the drive will not be detected until the port is
> powered back up by setting the link_power_management_policy
> to "max_performance".
> 
> Signed-off-by:  Kristen Carlson Accardi <kristen.c.accardi@intel.com>
> ---
>  drivers/ata/libata-core.c |   58 +++++++++++++++++++++++++++++++++++++++++++++-
>  drivers/ata/libata-scsi.c |    1 
>  include/linux/libata.h    |    2 +
>  3 files changed, 60 insertions(+), 1 deletion(-)
> 
> Index: linux-ahci-phy/drivers/ata/libata-core.c
> ===================================================================
> --- linux-ahci-phy.orig/drivers/ata/libata-core.c	2008-06-27 13:32:51.000000000 -0700
> +++ linux-ahci-phy/drivers/ata/libata-core.c	2008-07-01 16:33:00.000000000 -0700
> @@ -162,6 +162,19 @@ MODULE_DESCRIPTION("Library module for A
>  MODULE_LICENSE("GPL");
>  MODULE_VERSION(DRV_VERSION);
>  
> +static void ata_phy_offline(struct ata_link *link)
> +{
> +	u32 scontrol;
> +	int rc;
> +
> +	/* set DET to 4 */
> +	rc = sata_scr_read(link, SCR_CONTROL, &scontrol);
> +	if (rc)
> +		return;
> +	scontrol &= ~0xf;
> +	scontrol |= (1 << 2);
> +	sata_scr_write(link, SCR_CONTROL, scontrol);
> +}
>  
>  /**
>   *	ata_force_cbl - force cable type according to libata.force
> @@ -953,6 +966,7 @@ static int ata_dev_set_dipm(struct ata_d
>  		 * disallow all transitions which effectively
>  		 * disable DIPM anyway.
>  		 */
> +	default:
>  		break;
>  	}
>  
> @@ -980,6 +994,22 @@ void ata_dev_enable_pm(struct ata_device
>  	int rc = 0;
>  	struct ata_port *ap = dev->link->ap;
>  
> +	/*
> +	 * if we don't have a device attached, all we can
> +	 * do is power off
> +	 */
> +	if (!ata_dev_enabled(dev) && policy == POWER_OFF) {
> +		ap->flags |= ATA_FLAG_NO_HOTPLUG;
> +		ata_phy_offline(&ap->link);
> +		goto enable_pm_out;
> +	}
> +
> +	/*
> +         * if there's no device attached, we are done
> +	 */
> +	if (!ata_dev_enabled(dev))
> +		return;
> +
>  	/* set HIPM first, then DIPM */
>  	if (ap->ops->enable_pm)
>  		rc = ap->ops->enable_pm(ap, policy);
> @@ -1020,10 +1050,24 @@ static void ata_dev_disable_pm(struct at
>  
>  void ata_lpm_schedule(struct ata_port *ap, enum link_pm policy)
>  {
> -	ap->pm_policy = policy;
>  	ap->link.eh_info.action |= ATA_EH_LPM;
> +
> +	/*
> +	 * if we are coming from a state of OFF to ON, we need
> +	 * to reset the link and scan for devices in case someone
> +	 * plugged something in while we were off.
> +	 * Clear the NO_HOTPLUG flag because we are leaving the
> +	 * phy on now.
> +	 */
> +	if (ap->pm_policy == POWER_OFF && policy != POWER_OFF) {
> +		ap->flags &= ~ATA_FLAG_NO_HOTPLUG;
> +		ap->link.eh_info.probe_mask |= ATA_ALL_DEVICES;
> +		ap->link.eh_info.flags |= ATA_EH_RESET;
> +	}
>  	ap->link.eh_info.flags |= ATA_EHI_NO_AUTOPSY;
> +	ap->pm_policy = policy;
>  	ata_port_schedule_eh(ap);
> +	ata_port_wait_eh(ap);
>  }
>  
>  #ifdef CONFIG_PM

just a question... why wait for EH?


> @@ -2678,6 +2722,7 @@ void ata_port_disable(struct ata_port *a
>  	ap->link.device[0].class = ATA_DEV_NONE;
>  	ap->link.device[1].class = ATA_DEV_NONE;
>  	ap->flags |= ATA_FLAG_DISABLED;
> +	ata_phy_offline(&ap->link);
>  }
>  
>  /**

do we really want to unconditionally assume SATA here?

[patch 2/2] set default of ahci driver to power off unused ports

[kristen.c.accardi]

If the port isn't either a drive bay or an external SATA port, 
mark the port not-hotpluggable.  This will cause libata to 
power off the phy if it 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 <kristen.c.accardi@intel.com>

---
 drivers/ata/ahci.c |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Index: linux-ahci-phy/drivers/ata/ahci.c
===================================================================
--- linux-ahci-phy.orig/drivers/ata/ahci.c	2008-07-01 16:27:56.000000000 -0700
+++ linux-ahci-phy/drivers/ata/ahci.c	2008-07-01 16:35:03.000000000 -0700
@@ -53,9 +53,13 @@ static int ahci_skip_host_reset;
 module_param_named(skip_host_reset, ahci_skip_host_reset, int, 0444);
 MODULE_PARM_DESC(skip_host_reset, "skip global host reset (0=don't skip, 1=skip)");
 
+static int ahci_power_save = 1;
+module_param_named(power_save, ahci_power_save, int, 0444);
+MODULE_PARM_DESC(power_save, "Power off unused ports (0=don't power off, 1=power off)");
 static int ahci_enable_alpm(struct ata_port *ap,
 		enum link_pm policy);
 static void ahci_disable_alpm(struct ata_port *ap);
+static int ahci_is_hotplug_capable(struct ata_port *ap);
 
 enum {
 	AHCI_PCI_BAR		= 5,
@@ -168,6 +172,8 @@ enum {
 	PORT_CMD_ASP		= (1 << 27), /* Aggressive Slumber/Partial */
 	PORT_CMD_ALPE		= (1 << 26), /* Aggressive Link PM enable */
 	PORT_CMD_ATAPI		= (1 << 24), /* Device is ATAPI */
+	PORT_CMD_ESP		= (1 << 21), /* External SATA Port */
+	PORT_CMD_HPCP		= (1 << 18), /* port is hot plug capable */
 	PORT_CMD_PMP		= (1 << 17), /* PMP attached */
 	PORT_CMD_LIST_ON	= (1 << 15), /* cmd list DMA engine running */
 	PORT_CMD_FIS_ON		= (1 << 14), /* FIS DMA engine running */
@@ -1998,6 +2004,18 @@ static int ahci_pci_device_resume(struct
 }
 #endif
 
+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));
+}
+
 static int ahci_port_start(struct ata_port *ap)
 {
 	struct device *dev = ap->host->dev;
@@ -2049,6 +2067,9 @@ static int ahci_port_start(struct ata_po
 
 	ap->private_data = pp;
 
+	/* set some flags based on port capabilities */
+	if (!ahci_is_hotplug_capable(ap))
+		ap->flags |= ATA_FLAG_NO_HOTPLUG;
 	/* engage engines, captain */
 	return ahci_port_resume(ap);
 }

-- 

Re: [patch 2/2] set default of ahci driver to power off unused ports

[Jeff Garzik]

kristen.c.accardi@intel.com wrote:
> If the port isn't either a drive bay or an external SATA port, 
> mark the port not-hotpluggable.

That's just flat out incorrect, though.

We've been hotplugging SATA cables since SATA began, when the _most 
common_ hotplug case was where a port was neither a drive bay nor eSATA. 
  Part of the beauty of SATA is being able to just 'yank the cable'.

PORT_CMD_ESP and PORT_CMD_HPCP are positive indicators of hotplug 
capability, but are not exclusively such.

So, this patch would create regressions for people in servers or 
desktops or development systems that wish to retain their _existing_ 
ability to hotplug.

One guaranteed way to avoid regressions is to default ahci_power_save to 
zero, but that's then not helpful to Linux users in general, few of 
which will know of this module option and turn it on.

This is actually wandering into system-wide power policy :/

	Jeff

Re: [patch 1/2] allow user to power off unused ports via sysfs

[Jeffrey W. Baker]

I tried this patch and it doesn't work on ICH8 because it violates AHCI
protocol.

kristen.c.accardi@intel.com wrote:
> --- linux-ahci-phy.orig/drivers/ata/libata-core.c	2008-06-27 13:32:51.000000000 -0700
> +++ linux-ahci-phy/drivers/ata/libata-core.c	2008-07-01 16:33:00.000000000 -0700
> @@ -162,6 +162,19 @@ MODULE_DESCRIPTION("Library module for A
>  MODULE_LICENSE("GPL");
>  MODULE_VERSION(DRV_VERSION);
>  
> +static void ata_phy_offline(struct ata_link *link)
> +{
> +	u32 scontrol;
> +	int rc;
> +
> +	/* set DET to 4 */
> +	rc = sata_scr_read(link, SCR_CONTROL, &scontrol);
> +	if (rc)
> +		return;
> +	scontrol &= ~0xf;
> +	scontrol |= (1 << 2);
> +	sata_scr_write(link, SCR_CONTROL, scontrol);
> +}

On AHCI, you can't change SCTL.DET when the port is running.  This
version of ata_phy_offline changes SCTL.DET without regard to CMD.ST.

I would propose something more like this:

static void ata_port_offline(struct ata_port *ap)
{
        u32 scontrol;
        int rc;
        struct ata_link *link = &ap->link;

        if (ap->ops->port_stop)
                ap->ops->port_stop(ap);

        rc = sata_scr_read(link, SCR_CONTROL, &scontrol);
        if (rc)
                return;

        scontrol &= ~0xf;
        scontrol |= (1<<2);

        sata_scr_write(link, SCR_CONTROL, scontrol);
}

I tried the above on ICH8 and by poking around in /dev/mem I believe it
works.  After I set the ALPM to power_off in sysfs, the port shows
CMD.ST == 0, CMD.FRE == 0, and SSTS.DET == 4.  So I think this is the
way to go, at least with respect to AHCI.  I can't say whether this
makes sense generally for the other users of libata-core.

The problem is I didn't save any power this way :(  ThinkPad X61t was
using minimum 7.6W on a one-minute average before power_off, and same
power after power_off.  I'd been led to believe that disabling these
ports would have a substantial power saving effect.

-jwb

Re: [patch 1/2] allow user to power off unused ports via sysfs

[Kristen Carlson Accardi]

On Sun, 28 Sep 2008 14:58:34 -0700
"Jeffrey W. Baker" <jwbaker@gmail.com> wrote:

> 
> The problem is I didn't save any power this way :(  ThinkPad X61t was
> using minimum 7.6W on a one-minute average before power_off, and same
> power after power_off.  I'd been led to believe that disabling these
> ports would have a substantial power saving effect.
> 
> -jwb
> 

Hi Jeffrey - the reason you are not seeing any power savings is 
that this patch isn't actually powering off the ports.  I swear this
used to work... but I confirmed today it is not doing what it is
supposed to do.  I'll send out a reworked patch as soon as I figure
out what the problem is.

Kristen

Re: [patch 1/2] allow user to power off unused ports via sysfs

[Kristen Carlson Accardi]

On Mon, 29 Sep 2008 16:49:13 -0700
Kristen Carlson Accardi <kristen.c.accardi@intel.com> wrote:

> On Sun, 28 Sep 2008 14:58:34 -0700
> "Jeffrey W. Baker" <jwbaker@gmail.com> wrote:
> 
> > 
> > The problem is I didn't save any power this way :(  ThinkPad X61t was
> > using minimum 7.6W on a one-minute average before power_off, and same
> > power after power_off.  I'd been led to believe that disabling these
> > ports would have a substantial power saving effect.
> > 
> > -jwb
> > 
> 
> Hi Jeffrey - the reason you are not seeing any power savings is 
> that this patch isn't actually powering off the ports.  I swear this
> used to work... but I confirmed today it is not doing what it is
> supposed to do.  I'll send out a reworked patch as soon as I figure
> out what the problem is.
> 
> Kristen
> 

Hi - I took another look at this patch - and I was wrong, it is 
working just fine for me.  the reason I thought it wasn't working
was because I was trying to power off a dummy port (doh).  I
confirmed with a power meter that you should see .2 Watts per
port saved when powering down an unused port.  Just keep in mind
that you can't power off an occupied port, and don't do like I
did and try to power off a dummy port.