linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch 1/2] Enable link power management for ata drivers
       [not found] <20070924215140.966161778@intel.com>
@ 2007-09-24 22:13 ` Kristen Carlson Accardi
  2007-09-24 23:12   ` roel
  2007-10-25  5:21   ` Jeff Garzik
  2007-09-24 22:14 ` [patch 2/2] Enable Aggressive Link Power management for AHCI controllers Kristen Carlson Accardi
  1 sibling, 2 replies; 11+ messages in thread
From: Kristen Carlson Accardi @ 2007-09-24 22:13 UTC (permalink / raw)
  To: jgarzik; +Cc: linux-ide, linux-kernel, axboe, akpm, Kristen Carlson Accardi

Device Initiated Power Management, which is defined
in SATA 2.5 can be enabled for disks which support it.
This patch enables DIPM when the user sets the link
power management policy to "min_power".

Additionally, libata drivers can define a function 
(enable_pm) that will perform hardware specific actions to 
enable whatever power management policy the user set up 
for Host Initiated Power management (HIPM).
This power management policy will be activated after all 
disks have been enumerated and intialized.  Drivers should 
also define disable_pm, which will turn off link power 
management, but not change link power management policy.

Signed-off-by:  Kristen Carlson Accardi <kristen.c.accardi@intel.com>
---
 Documentation/scsi/link_power_management_policy.txt |   19 +
 drivers/ata/libata-core.c                           |  194 +++++++++++++++++++-
 drivers/ata/libata-eh.c                             |    5 
 drivers/ata/libata-scsi.c                           |   83 ++++++++
 include/linux/ata.h                                 |    7 
 include/linux/libata.h                              |   25 ++
 6 files changed, 322 insertions(+), 11 deletions(-)

Index: libata-dev/drivers/ata/libata-scsi.c
===================================================================
--- libata-dev.orig/drivers/ata/libata-scsi.c	2007-09-24 13:43:10.000000000 -0700
+++ libata-dev/drivers/ata/libata-scsi.c	2007-09-24 13:46:22.000000000 -0700
@@ -110,6 +110,78 @@ static struct scsi_transport_template at
 };
 
 
+static const struct {
+	enum link_pm	value;
+	char		*name;
+} link_pm_policy[] = {
+	{ NOT_AVAILABLE, "max_performance" },
+	{ MIN_POWER, "min_power" },
+	{ MAX_PERFORMANCE, "max_performance" },
+	{ MEDIUM_POWER, "medium_power" },
+};
+
+const char *ata_scsi_link_pm_policy(enum link_pm policy)
+{
+	int i;
+	char *name = NULL;
+
+	for (i = 0; i < ARRAY_SIZE(link_pm_policy); i++) {
+		if (link_pm_policy[i].value == policy) {
+			name = link_pm_policy[i].name;
+			break;
+		}
+	}
+	return name;
+}
+
+static ssize_t store_link_pm_policy(struct class_device *class_dev,
+	const char *buf, size_t count)
+{
+	struct Scsi_Host *shost = class_to_shost(class_dev);
+	struct ata_port *ap = ata_shost_to_port(shost);
+	enum link_pm policy = 0;
+	int i;
+
+	/*
+ 	 * we are skipping array location 0 on purpose - this
+ 	 * is because a value of NOT_AVAILABLE is displayed
+ 	 * to the user as max_performance, but when the user
+ 	 * writes "max_performance", they actually want the
+ 	 * value to match MAX_PERFORMANCE.
+ 	 */
+	for (i = 1; i < ARRAY_SIZE(link_pm_policy); i++) {
+		const int len = strlen(link_pm_policy[i].name);
+		if (strncmp(link_pm_policy[i].name, buf, len) == 0 &&
+		   buf[len] == '\n') {
+			policy = link_pm_policy[i].value;
+			break;
+		}
+	}
+	if (!policy)
+		return -EINVAL;
+
+	if (ata_scsi_set_link_pm_policy(ap, policy))
+		return -EINVAL;
+	return count;
+}
+
+static ssize_t
+show_link_pm_policy(struct class_device *class_dev, char *buf)
+{
+	struct Scsi_Host *shost = class_to_shost(class_dev);
+	struct ata_port *ap = ata_shost_to_port(shost);
+	const char *policy =
+		ata_scsi_link_pm_policy(ap->pm_policy);
+
+	if (!policy)
+		return -EINVAL;
+
+	return snprintf(buf, 23, "%s\n", policy);
+}
+CLASS_DEVICE_ATTR(link_power_management_policy, S_IRUGO | S_IWUSR,
+		show_link_pm_policy, store_link_pm_policy);
+EXPORT_SYMBOL_GPL(class_device_attr_link_power_management_policy);
+
 static void ata_scsi_invalid_field(struct scsi_cmnd *cmd,
 				   void (*done)(struct scsi_cmnd *))
 {
@@ -3041,6 +3113,17 @@ void ata_scsi_simulate(struct ata_device
 	}
 }
 
+int ata_scsi_set_link_pm_policy(struct ata_port *ap,
+		enum link_pm policy)
+{
+	ap->pm_policy = policy;
+	ap->link.eh_info.action |= ATA_EHI_LPM;
+	ap->link.eh_info.flags |= ATA_EHI_NO_AUTOPSY;
+	ata_port_schedule_eh(ap);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ata_scsi_set_link_pm_policy);
+
 int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht)
 {
 	int i, rc;
Index: libata-dev/include/linux/libata.h
===================================================================
--- libata-dev.orig/include/linux/libata.h	2007-09-24 13:43:10.000000000 -0700
+++ libata-dev/include/linux/libata.h	2007-09-24 13:47:57.000000000 -0700
@@ -140,6 +140,8 @@ enum {
 	ATA_DFLAG_ACPI_PENDING	= (1 << 5), /* ACPI resume action pending */
 	ATA_DFLAG_ACPI_FAILED	= (1 << 6), /* ACPI on devcfg has failed */
 	ATA_DFLAG_AN		= (1 << 7), /* device supports AN */
+	ATA_DFLAG_HIPM		= (1 << 8), /* device supports HIPM */
+	ATA_DFLAG_DIPM		= (1 << 9), /* device supports DIPM */
 	ATA_DFLAG_CFG_MASK	= (1 << 12) - 1,
 
 	ATA_DFLAG_PIO		= (1 << 12), /* device limited to PIO mode */
@@ -181,6 +183,7 @@ enum {
 	ATA_FLAG_NO_IORDY	= (1 << 16), /* controller lacks iordy */
 	ATA_FLAG_ACPI_SATA	= (1 << 17), /* need native SATA ACPI layout */
 	ATA_FLAG_AN		= (1 << 18), /* controller supports AN */
+	ATA_FLAG_IPM		= (1 << 19), /* driver can handle IPM */
 
 	/* The following flag belongs to ap->pflags but is kept in
 	 * ap->flags because it's referenced in many LLDs and will be
@@ -283,6 +286,7 @@ enum {
 	ATA_EHI_RESUME_LINK	= (1 << 1),  /* resume link (reset modifier) */
 	ATA_EHI_NO_AUTOPSY	= (1 << 2),  /* no autopsy */
 	ATA_EHI_QUIET		= (1 << 3),  /* be quiet */
+	ATA_EHI_LPM		= (1 << 4),  /* link power management action */
 
 	ATA_EHI_DID_SOFTRESET	= (1 << 16), /* already soft-reset this port */
 	ATA_EHI_DID_HARDRESET	= (1 << 17), /* already soft-reset this port */
@@ -308,6 +312,7 @@ enum {
 	ATA_HORKAGE_NONCQ	= (1 << 2),	/* Don't use NCQ */
 	ATA_HORKAGE_MAX_SEC_128	= (1 << 3),	/* Limit max sects to 128 */
 	ATA_HORKAGE_BROKEN_HPA	= (1 << 4),	/* Broken HPA */
+	ATA_HORKAGE_IPM		= (1 << 5), 	/* LPM problems */
 };
 
 enum hsm_task_states {
@@ -347,6 +352,18 @@ typedef int (*ata_reset_fn_t)(struct ata
 			      unsigned long deadline);
 typedef void (*ata_postreset_fn_t)(struct ata_link *link, unsigned int *classes);
 
+/*
+ * host pm policy: If you alter this, you also need to alter libata-scsi.c
+ * (for the ascii descriptions)
+ */
+enum link_pm {
+	NOT_AVAILABLE,
+	MIN_POWER,
+	MAX_PERFORMANCE,
+	MEDIUM_POWER,
+};
+extern struct class_device_attribute class_device_attr_link_power_management_policy;
+
 struct ata_ioports {
 	void __iomem		*cmd_addr;
 	void __iomem		*data_addr;
@@ -585,6 +602,7 @@ struct ata_port {
 
 	pm_message_t		pm_mesg;
 	int			*pm_result;
+	enum link_pm		pm_policy;
 
 	struct timer_list	fastdrain_timer;
 	unsigned long		fastdrain_cnt;
@@ -647,7 +665,8 @@ struct ata_port_operations {
 
 	int (*port_suspend) (struct ata_port *ap, pm_message_t mesg);
 	int (*port_resume) (struct ata_port *ap);
-
+	int (*enable_pm) (struct ata_port *ap, enum link_pm policy);
+	int (*disable_pm) (struct ata_port *ap);
 	int (*port_start) (struct ata_port *ap);
 	void (*port_stop) (struct ata_port *ap);
 
@@ -854,7 +873,9 @@ extern int ata_cable_40wire(struct ata_p
 extern int ata_cable_80wire(struct ata_port *ap);
 extern int ata_cable_sata(struct ata_port *ap);
 extern int ata_cable_unknown(struct ata_port *ap);
-
+extern int ata_scsi_set_link_pm_policy(struct ata_port *ap, enum link_pm);
+extern int ata_dev_enable_pm(struct ata_device *dev, enum link_pm policy);
+extern void ata_dev_disable_pm(struct ata_device *dev);
 /*
  * Timing helpers
  */
Index: libata-dev/drivers/ata/libata-core.c
===================================================================
--- libata-dev.orig/drivers/ata/libata-core.c	2007-09-24 13:43:10.000000000 -0700
+++ libata-dev/drivers/ata/libata-core.c	2007-09-24 13:46:22.000000000 -0700
@@ -68,7 +68,8 @@ const unsigned long sata_deb_timing_long
 static unsigned int ata_dev_init_params(struct ata_device *dev,
 					u16 heads, u16 sectors);
 static unsigned int ata_dev_set_xfermode(struct ata_device *dev);
-static unsigned int ata_dev_set_AN(struct ata_device *dev, u8 enable);
+static unsigned int ata_dev_set_feature(struct ata_device *dev,
+					u8 enable, u8 feature);
 static void ata_dev_xfermask(struct ata_device *dev);
 static unsigned long ata_dev_blacklisted(const struct ata_device *dev);
 
@@ -615,6 +616,129 @@ void ata_dev_disable(struct ata_device *
 	}
 }
 
+static int ata_dev_set_dipm(struct ata_device *dev, enum link_pm policy)
+{
+	struct ata_link *link = dev->link;
+	struct ata_port *ap = link->ap;
+	u32 scontrol;
+
+	/*
+	 * disallow DIPM for drivers which haven't set
+	 * ATA_FLAG_IPM.  This is because when DIPM is enabled,
+	 * phy ready will be set in the interrupt status on
+	 * state changes, which will cause some drivers to
+	 * think there are errors - additionally drivers will
+	 * need to disable hot plug.
+	 */
+	if (!(ap->flags & ATA_FLAG_IPM) || !ata_dev_enabled(dev)) {
+		ap->pm_policy = NOT_AVAILABLE;
+		return -EINVAL;
+	}
+
+	/*
+	 * For DIPM, we will only enable it for the
+	 * min_power setting.
+	 *
+	 * Why?  Because Disks are too stupid to know that
+	 * If the host rejects a request to go to SLUMBER
+	 * they should retry at PARTIAL, and instead it
+	 * just would give up.  So, for medium_power to
+	 * work at all, we need to only allow HIPM.
+	 */
+	sata_scr_read(link, SCR_CONTROL, &scontrol);
+
+	switch (policy) {
+	case MIN_POWER:
+		/* no restrictions on IPM transitions */
+		scontrol &= ~(0x3 << 8);
+		sata_scr_write(link, SCR_CONTROL, scontrol);
+
+		/* enable DIPM */
+		if (dev->flags & ATA_DFLAG_DIPM)
+			ata_dev_set_feature(dev, SETFEATURES_SATA_ENABLE,
+						SATA_DIPM);
+		break;
+	case MEDIUM_POWER:
+		/* allow IPM to PARTIAL */
+		scontrol &= ~(0x1 << 8);
+		scontrol |= (0x2 << 8);
+		sata_scr_write(link, SCR_CONTROL, scontrol);
+
+		/* disable DIPM */
+		if (ata_dev_enabled(dev) && (dev->flags & ATA_DFLAG_DIPM))
+			ata_dev_set_feature(dev, SETFEATURES_SATA_DISABLE,
+						SATA_DIPM);
+		break;
+	case NOT_AVAILABLE:
+	case MAX_PERFORMANCE:
+		/* disable all IPM transitions */
+		scontrol |= (0x3 << 8);
+		sata_scr_write(link, SCR_CONTROL, scontrol);
+
+		/* disable DIPM */
+		if (ata_dev_enabled(dev) && (dev->flags & ATA_DFLAG_DIPM))
+			ata_dev_set_feature(dev, SETFEATURES_SATA_DISABLE,
+						SATA_DIPM);
+		break;
+	}
+	return 0;
+}
+
+/**
+ *	ata_dev_enable_pm - enable SATA interface power management
+ *	@device - device to enable ipm for
+ *	@policy - the link power management policy
+ *
+ *	Enable SATA Interface power management.  This will enable
+ *	Device Interface Power Management (DIPM) for min_power
+ * 	policy, and then call driver specific callbacks for
+ *	enabling Host Initiated Power management.
+ *
+ *	Locking: Caller.
+ *	Returns: -EINVAL if IPM is not supported, 0 otherwise.
+ */
+int ata_dev_enable_pm(struct ata_device *dev, enum link_pm policy)
+{
+	int rc = 0;
+	struct ata_port *ap = dev->link->ap;
+
+	/* set HIPM first, then DIPM */
+	if (ap->ops->enable_pm)
+		rc = ap->ops->enable_pm(ap, policy);
+	if (rc)
+		goto enable_pm_out;
+	rc = ata_dev_set_dipm(dev, policy);
+
+enable_pm_out:
+	if (rc)
+		ap->pm_policy = MAX_PERFORMANCE;
+	else
+		ap->pm_policy = policy;
+	return rc;
+}
+
+/**
+ *	ata_dev_disable_pm - disable SATA interface power management
+ *	@device - device to enable ipm for
+ *
+ *	Disable SATA Interface power management.  This will disable
+ *	Device Interface Power Management (DIPM) without changing
+ * 	policy,  call driver specific callbacks for disabling Host
+ * 	Initiated Power management.
+ *
+ *	Locking: Caller.
+ *	Returns: void
+ */
+void ata_dev_disable_pm(struct ata_device *dev)
+{
+	struct ata_port *ap = dev->link->ap;
+
+	ata_dev_set_dipm(dev, MAX_PERFORMANCE);
+	if (ap->ops->disable_pm)
+		ap->ops->disable_pm(ap);
+}
+
+
 /**
  *	ata_devchk - PATA device presence detection
  *	@ap: ATA channel to examine
@@ -2029,7 +2153,8 @@ int ata_dev_configure(struct ata_device 
 		if ((ap->flags & ATA_FLAG_AN) && ata_id_has_AN(id)) {
 			int err;
 			/* issue SET feature command to turn this on */
-			err = ata_dev_set_AN(dev, SETFEATURES_SATA_ENABLE);
+			err = ata_dev_set_feature(dev, SETFEATURES_SATA_ENABLE,
+						SATA_AN);
 			if (err)
 				ata_dev_printk(dev, KERN_ERR,
 						"unable to set AN, err %x\n",
@@ -2057,6 +2182,13 @@ int ata_dev_configure(struct ata_device 
 	if (dev->flags & ATA_DFLAG_LBA48)
 		dev->max_sectors = ATA_MAX_SECTORS_LBA48;
 
+	if (!(dev->horkage & ATA_HORKAGE_IPM)) {
+		if (ata_id_has_hipm(dev->id))
+			dev->flags |= ATA_DFLAG_HIPM;
+		if (ata_id_has_dipm(dev->id))
+			dev->flags |= ATA_DFLAG_DIPM;
+	}
+
 	if (dev->horkage & ATA_HORKAGE_DIAGNOSTIC) {
 		/* Let the user know. We don't want to disallow opens for
 		   rescue purposes, or in case the vendor is just a blithering
@@ -2082,6 +2214,13 @@ int ata_dev_configure(struct ata_device 
 		dev->max_sectors = min_t(unsigned int, ATA_MAX_SECTORS_128,
 					 dev->max_sectors);
 
+	if (ata_dev_blacklisted(dev) & ATA_HORKAGE_IPM) {
+		dev->horkage |= ATA_HORKAGE_IPM;
+
+		/* reset link pm_policy for this port to no pm */
+		ap->pm_policy = MAX_PERFORMANCE;
+	}
+
 	if (ap->ops->dev_config)
 		ap->ops->dev_config(dev);
 
@@ -4048,15 +4187,14 @@ static unsigned int ata_dev_set_xfermode
 	DPRINTK("EXIT, err_mask=%x\n", err_mask);
 	return err_mask;
 }
-
 /**
- *	ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES
+ *	ata_dev_set_feature - Issue SET FEATURES - SATA FEATURES
  *	@dev: Device to which command will be sent
  *	@enable: Whether to enable or disable the feature
+ *	@feature: The sector count represents the feature to set
  *
  *	Issue SET FEATURES - SATA FEATURES command to device @dev
- *	on port @ap with sector count set to indicate Asynchronous
- *	Notification feature
+ *	on port @ap with sector count
  *
  *	LOCKING:
  *	PCI/etc. bus probe sem.
@@ -4064,7 +4202,8 @@ static unsigned int ata_dev_set_xfermode
  *	RETURNS:
  *	0 on success, AC_ERR_* mask otherwise.
  */
-static unsigned int ata_dev_set_AN(struct ata_device *dev, u8 enable)
+static unsigned int ata_dev_set_feature(struct ata_device *dev, u8 enable,
+					u8 feature)
 {
 	struct ata_taskfile tf;
 	unsigned int err_mask;
@@ -4077,7 +4216,7 @@ static unsigned int ata_dev_set_AN(struc
 	tf.feature = enable;
 	tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
 	tf.protocol = ATA_PROT_NODATA;
-	tf.nsect = SATA_AN;
+	tf.nsect = feature;
 
 	err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0);
 
@@ -6031,6 +6170,32 @@ int ata_flush_cache(struct ata_device *d
 	return 0;
 }
 
+static void ata_host_disable_link_pm(struct ata_host *host)
+{
+	int i;
+	struct ata_link *link;
+	struct ata_port *ap;
+	struct ata_device *dev;
+
+	for (i = 0; i < host->n_ports; i++) {
+		ap = host->ports[i];
+		ata_port_for_each_link(link, ap) {
+			ata_link_for_each_dev(dev, link)
+				ata_dev_disable_pm(dev);
+		}
+	}
+}
+
+static void ata_host_enable_link_pm(struct ata_host *host)
+{
+	int i;
+
+	for (i = 0; i < host->n_ports; i++) {
+		struct ata_port *ap = host->ports[i];
+		ata_scsi_set_link_pm_policy(ap, ap->pm_policy);
+	}
+}
+
 #ifdef CONFIG_PM
 static int ata_host_request_pm(struct ata_host *host, pm_message_t mesg,
 			       unsigned int action, unsigned int ehi_flags,
@@ -6101,6 +6266,12 @@ int ata_host_suspend(struct ata_host *ho
 {
 	int rc;
 
+	/*
+ 	 * disable link pm on all ports before requesting
+ 	 * any pm activity
+ 	 */
+	ata_host_disable_link_pm(host);
+
 	rc = ata_host_request_pm(host, mesg, 0, ATA_EHI_QUIET, 1);
 	if (rc == 0)
 		host->dev->power.power_state = mesg;
@@ -6123,6 +6294,9 @@ void ata_host_resume(struct ata_host *ho
 	ata_host_request_pm(host, PMSG_ON, ATA_EH_SOFTRESET,
 			    ATA_EHI_NO_AUTOPSY | ATA_EHI_QUIET, 0);
 	host->dev->power.power_state = PMSG_ON;
+
+	/* reenable link pm */
+	ata_host_enable_link_pm(host);
 }
 #endif
 
@@ -6663,6 +6837,7 @@ int ata_host_register(struct ata_host *h
 		struct ata_port *ap = host->ports[i];
 
 		ata_scsi_scan_host(ap, 1);
+		ata_scsi_set_link_pm_policy(ap, ap->pm_policy);
 	}
 
 	return 0;
@@ -7059,7 +7234,8 @@ const struct ata_port_info ata_dummy_por
  * likely to change as new drivers are added and updated.
  * Do not depend on ABI/API stability.
  */
-
+EXPORT_SYMBOL_GPL(ata_dev_enable_pm);
+EXPORT_SYMBOL_GPL(ata_dev_disable_pm);
 EXPORT_SYMBOL_GPL(sata_deb_timing_normal);
 EXPORT_SYMBOL_GPL(sata_deb_timing_hotplug);
 EXPORT_SYMBOL_GPL(sata_deb_timing_long);
Index: libata-dev/include/linux/ata.h
===================================================================
--- libata-dev.orig/include/linux/ata.h	2007-09-24 13:43:10.000000000 -0700
+++ libata-dev/include/linux/ata.h	2007-09-24 13:46:22.000000000 -0700
@@ -235,6 +235,7 @@ enum {
 
 	/* SETFEATURE Sector counts for SATA features */
 	SATA_AN			= 0x05,  /* Asynchronous Notification */
+	SATA_DIPM		= 0x03,  /* Device Initiated Power Management */
 
 	/* ATAPI stuff */
 	ATAPI_PKT_DMA		= (1 << 0),
@@ -367,6 +368,12 @@ struct ata_taskfile {
 	  ((u64) (id)[(n) + 0]) )
 
 #define ata_id_cdb_intr(id)	(((id)[0] & 0x60) == 0x20)
+#define ata_id_has_hipm(id)	\
+	( (((id)[76] != 0x0000) && ((id)[76] != 0xffff)) && \
+	  ((id)[76] & (1 << 9)) )
+#define ata_id_has_dipm(id)	\
+	( (((id)[76] != 0x0000) && ((id)[76] != 0xffff)) && \
+	  ((id)[78] & (1 << 3)) )
 
 static inline int ata_id_has_fua(const u16 *id)
 {
Index: libata-dev/Documentation/scsi/link_power_management_policy.txt
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ libata-dev/Documentation/scsi/link_power_management_policy.txt	2007-09-24 13:46:22.000000000 -0700
@@ -0,0 +1,19 @@
+This parameter allows the user to set the link (interface) power management.
+There are 3 possible options:
+
+Value			Effect
+----------------------------------------------------------------------------
+min_power		Tell the controller to try to make the link use the
+			least possible power when possible.  This may
+			sacrifice some performance due to increased latency
+			when coming out of lower power states.
+
+max_performance		Generally, this means no power management.  Tell
+			the controller to have performance be a priority
+			over power management.
+
+medium_power		Tell the controller to enter a lower power state
+			when possible, but do not enter the lowest power
+			state, thus improving latency over min_power setting.
+
+
Index: libata-dev/drivers/ata/libata-eh.c
===================================================================
--- libata-dev.orig/drivers/ata/libata-eh.c	2007-09-24 13:43:10.000000000 -0700
+++ libata-dev/drivers/ata/libata-eh.c	2007-09-24 13:46:22.000000000 -0700
@@ -2400,6 +2400,11 @@ static int ata_eh_recover(struct ata_por
 			ehc->i.flags &= ~ATA_EHI_SETMODE;
 		}
 
+		if (ehc->i.action & ATA_EHI_LPM) {
+			ata_link_for_each_dev(dev, link)
+				ata_dev_enable_pm(dev, ap->pm_policy);
+		}
+
 		/* this link is okay now */
 		ehc->i.flags = 0;
 		continue;

-- 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [patch 2/2] Enable Aggressive Link Power management for AHCI controllers.
       [not found] <20070924215140.966161778@intel.com>
  2007-09-24 22:13 ` [patch 1/2] Enable link power management for ata drivers Kristen Carlson Accardi
@ 2007-09-24 22:14 ` Kristen Carlson Accardi
  2007-10-25  5:34   ` Jeff Garzik
  1 sibling, 1 reply; 11+ messages in thread
From: Kristen Carlson Accardi @ 2007-09-24 22:14 UTC (permalink / raw)
  To: jgarzik; +Cc: linux-ide, linux-kernel, axboe, akpm, Kristen Carlson Accardi

This patch will set the correct bits to turn on Aggressive
Link Power Management (ALPM) for the ahci driver.  This
will cause the controller and disk to negotiate a lower
power state for the link when there is no activity (see
the AHCI 1.x spec for details).  This feature is mutually
exclusive with Hot Plug, so when ALPM is enabled, Hot Plug
is disabled.  ALPM will be enabled by default, but it is
settable via the scsi host syfs interface.  Possible 
settings for this feature are:

Setting		Effect
----------------------------------------------------------
min_power	ALPM is enabled, and link set to enter 
		lowest power state (SLUMBER) when idle
		Hot plug not allowed.

max_performance	ALPM is disabled, Hot Plug is allowed

medium_power	ALPM is enabled, and link set to enter
		second lowest power state (PARTIAL) when
		idle.  Hot plug not allowed.

Signed-off-by:  Kristen Carlson Accardi <kristen.c.accardi@intel.com>

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

Index: libata-dev/drivers/ata/ahci.c
===================================================================
--- libata-dev.orig/drivers/ata/ahci.c	2007-09-24 13:43:10.000000000 -0700
+++ libata-dev/drivers/ata/ahci.c	2007-09-24 13:48:59.000000000 -0700
@@ -48,6 +48,9 @@
 #define DRV_NAME	"ahci"
 #define DRV_VERSION	"2.3"
 
+static int ahci_enable_alpm(struct ata_port *ap,
+		enum link_pm policy);
+static int ahci_disable_alpm(struct ata_port *ap);
 
 enum {
 	AHCI_PCI_BAR		= 5,
@@ -97,6 +100,7 @@ enum {
 	/* HOST_CAP bits */
 	HOST_CAP_SSC		= (1 << 14), /* Slumber capable */
 	HOST_CAP_CLO		= (1 << 24), /* Command List Override support */
+	HOST_CAP_ALPM		= (1 << 26), /* Aggressive Link PM support */
 	HOST_CAP_SSS		= (1 << 27), /* Staggered Spin-up */
 	HOST_CAP_SNTF		= (1 << 29), /* SNotification register */
 	HOST_CAP_NCQ		= (1 << 30), /* Native Command Queueing */
@@ -152,6 +156,8 @@ enum {
 				  PORT_IRQ_PIOS_FIS | PORT_IRQ_D2H_REG_FIS,
 
 	/* PORT_CMD bits */
+	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_LIST_ON	= (1 << 15), /* cmd list DMA engine running */
 	PORT_CMD_FIS_ON		= (1 << 14), /* FIS DMA engine running */
@@ -177,7 +183,7 @@ enum {
 
 	AHCI_FLAG_COMMON		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 					  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
-					  ATA_FLAG_ACPI_SATA,
+					  ATA_FLAG_ACPI_SATA | ATA_FLAG_IPM,
 	AHCI_LFLAG_COMMON		= ATA_LFLAG_SKIP_D2H_BSY,
 };
 
@@ -242,6 +248,11 @@ static int ahci_pci_device_suspend(struc
 static int ahci_pci_device_resume(struct pci_dev *pdev);
 #endif
 
+static struct class_device_attribute *ahci_shost_attrs[] = {
+	&class_device_attr_link_power_management_policy,
+	NULL
+};
+
 static struct scsi_host_template ahci_sht = {
 	.module			= THIS_MODULE,
 	.name			= DRV_NAME,
@@ -259,6 +270,7 @@ static struct scsi_host_template ahci_sh
 	.slave_configure	= ata_scsi_slave_config,
 	.slave_destroy		= ata_scsi_slave_destroy,
 	.bios_param		= ata_std_bios_param,
+	.shost_attrs		= ahci_shost_attrs,
 };
 
 static const struct ata_port_operations ahci_ops = {
@@ -286,6 +298,8 @@ static const struct ata_port_operations 
 	.port_suspend		= ahci_port_suspend,
 	.port_resume		= ahci_port_resume,
 #endif
+	.enable_pm		= ahci_enable_alpm,
+	.disable_pm		= ahci_disable_alpm,
 
 	.port_start		= ahci_port_start,
 	.port_stop		= ahci_port_stop,
@@ -767,6 +781,130 @@ static void ahci_power_up(struct ata_por
 	writel(cmd | PORT_CMD_ICC_ACTIVE, port_mmio + PORT_CMD);
 }
 
+static int ahci_disable_alpm(struct ata_port *ap)
+{
+	void __iomem *port_mmio = ahci_port_base(ap);
+	u32 cmd;
+	struct ahci_port_priv *pp = ap->private_data;
+
+	/* IPM bits should be disabled by libata-core */
+	/* get the existing command bits */
+	cmd = readl(port_mmio + PORT_CMD);
+
+	/* disable ALPM and ASP */
+	cmd &= ~PORT_CMD_ASP;
+	cmd &= ~PORT_CMD_ALPE;
+
+	/* force the interface back to active */
+	cmd |= PORT_CMD_ICC_ACTIVE;
+
+	/* write out new cmd value */
+	writel(cmd, port_mmio + PORT_CMD);
+	cmd = readl(port_mmio + PORT_CMD);
+
+	/* wait 10ms to be sure we've come out of any low power state */
+	msleep(10);
+
+	/* clear out any PhyRdy stuff from interrupt status */
+	writel(PORT_IRQ_PHYRDY, port_mmio + PORT_IRQ_STAT);
+
+	/* go ahead and clean out PhyRdy Change from Serror too */
+	ahci_scr_write(ap, SCR_ERROR, ((1 << 16) | (1 << 18)));
+
+	/*
+ 	 * Clear flag to indicate that we should ignore all PhyRdy
+ 	 * state changes
+ 	 */
+	ap->flags &= ~AHCI_FLAG_NO_HOTPLUG;
+
+	/*
+ 	 * Enable interrupts on Phy Ready.
+ 	 */
+	pp->intr_mask |= PORT_IRQ_PHYRDY;
+	writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
+
+	/*
+ 	 * don't change the link pm policy - we can be called
+ 	 * just to turn of link pm temporarily
+ 	 */
+	return 0;
+}
+
+static int ahci_enable_alpm(struct ata_port *ap,
+	enum link_pm policy)
+{
+	struct ahci_host_priv *hpriv = ap->host->private_data;
+	void __iomem *port_mmio = ahci_port_base(ap);
+	u32 cmd;
+	struct ahci_port_priv *pp = ap->private_data;
+	u32 asp;
+
+	/* Make sure the host is capable of link power management */
+	if (!(hpriv->cap & HOST_CAP_ALPM))
+		return -EINVAL;
+
+	switch (policy) {
+	case MAX_PERFORMANCE:
+	case NOT_AVAILABLE:
+		/*
+ 		 * if we came here with NOT_AVAILABLE,
+ 		 * it just means this is the first time we
+ 		 * have tried to enable - default to max performance,
+ 		 * and let the user go to lower power modes on request.
+ 		 */
+		ahci_disable_alpm(ap);
+		return 0;
+	case MIN_POWER:
+		/* configure HBA to enter SLUMBER */
+		asp = PORT_CMD_ASP;
+		break;
+	case MEDIUM_POWER:
+		/* configure HBA to enter PARTIAL */
+		asp = 0;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/*
+ 	 * Disable interrupts on Phy Ready. This keeps us from
+ 	 * getting woken up due to spurious phy ready interrupts
+	 * TBD - Hot plug should be done via polling now, is
+	 * that even supported?
+ 	 */
+	pp->intr_mask &= ~PORT_IRQ_PHYRDY;
+	writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
+
+	/*
+ 	 * Set a flag to indicate that we should ignore all PhyRdy
+ 	 * state changes since these can happen now whenever we
+ 	 * change link state
+ 	 */
+	ap->flags |= AHCI_FLAG_NO_HOTPLUG;
+
+	/* get the existing command bits */
+	cmd = readl(port_mmio + PORT_CMD);
+
+	/*
+ 	 * Set ASP based on Policy
+ 	 */
+	cmd |= asp;
+
+	/*
+ 	 * Setting this bit will instruct the HBA to aggressively
+ 	 * enter a lower power link state when it's appropriate and
+ 	 * based on the value set above for ASP
+ 	 */
+	cmd |= PORT_CMD_ALPE;
+
+	/* write out new cmd value */
+	writel(cmd, port_mmio + PORT_CMD);
+	cmd = readl(port_mmio + PORT_CMD);
+
+	/* IPM bits should be set by libata-core */
+	return 0;
+}
+
 #ifdef CONFIG_PM
 static void ahci_power_down(struct ata_port *ap)
 {
@@ -1348,6 +1486,17 @@ static void ahci_port_intr(struct ata_po
 	status = readl(port_mmio + PORT_IRQ_STAT);
 	writel(status, port_mmio + PORT_IRQ_STAT);
 
+	/* If we are getting PhyRdy, this is
+ 	 * just a power state change, we should
+ 	 * clear out this, plus the PhyRdy/Comm
+ 	 * Wake bits from Serror
+ 	 */
+	if ((ap->flags & AHCI_FLAG_NO_HOTPLUG) &&
+		(status & PORT_IRQ_PHYRDY)) {
+		status &= ~PORT_IRQ_PHYRDY;
+		ahci_scr_write(ap, SCR_ERROR, ((1 << 16) | (1 << 18)));
+	}
+
 	if (unlikely(status & PORT_IRQ_ERROR)) {
 		ahci_error_intr(ap, status);
 		return;
@@ -1882,6 +2031,9 @@ static int ahci_init_one(struct pci_dev 
 		ata_port_pbar_desc(ap, AHCI_PCI_BAR,
 				   0x100 + ap->port_no * 0x80, "port");
 
+		/* set initial link pm policy */
+		ap->pm_policy = NOT_AVAILABLE;
+
 		/* standard SATA port setup */
 		if (hpriv->port_map & (1 << i))
 			ap->ioaddr.cmd_addr = port_mmio;

-- 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch 1/2] Enable link power management for ata drivers
  2007-09-24 22:13 ` [patch 1/2] Enable link power management for ata drivers Kristen Carlson Accardi
@ 2007-09-24 23:12   ` roel
  2007-09-24 23:28     ` Davide Libenzi
  2007-09-24 23:41     ` Kristen Carlson Accardi
  2007-10-25  5:21   ` Jeff Garzik
  1 sibling, 2 replies; 11+ messages in thread
From: roel @ 2007-09-24 23:12 UTC (permalink / raw)
  To: Kristen Carlson Accardi; +Cc: jgarzik, linux-ide, linux-kernel, axboe, akpm

Kristen Carlson Accardi wrote:
> Device Initiated Power Management, which is defined
> in SATA 2.5 can be enabled for disks which support it.
> This patch enables DIPM when the user sets the link
> power management policy to "min_power".
> 
> Additionally, libata drivers can define a function 
> (enable_pm) that will perform hardware specific actions to 
> enable whatever power management policy the user set up 
> for Host Initiated Power management (HIPM).
> This power management policy will be activated after all 
> disks have been enumerated and intialized.  Drivers should 
> also define disable_pm, which will turn off link power 
> management, but not change link power management policy.
> 
> Signed-off-by:  Kristen Carlson Accardi <kristen.c.accardi@intel.com>
> ---
>  Documentation/scsi/link_power_management_policy.txt |   19 +
>  drivers/ata/libata-core.c                           |  194 +++++++++++++++++++-
>  drivers/ata/libata-eh.c                             |    5 
>  drivers/ata/libata-scsi.c                           |   83 ++++++++
>  include/linux/ata.h                                 |    7 
>  include/linux/libata.h                              |   25 ++
>  6 files changed, 322 insertions(+), 11 deletions(-)
> 
> Index: libata-dev/drivers/ata/libata-scsi.c
> ===================================================================
> --- libata-dev.orig/drivers/ata/libata-scsi.c	2007-09-24 13:43:10.000000000 -0700
> +++ libata-dev/drivers/ata/libata-scsi.c	2007-09-24 13:46:22.000000000 -0700
> @@ -110,6 +110,78 @@ static struct scsi_transport_template at
>  };
>  
>  
> +static const struct {
> +	enum link_pm	value;
> +	char		*name;
> +} link_pm_policy[] = {
> +	{ NOT_AVAILABLE, "max_performance" },
> +	{ MIN_POWER, "min_power" },
> +	{ MAX_PERFORMANCE, "max_performance" },
> +	{ MEDIUM_POWER, "medium_power" },
> +};
> +
> +const char *ata_scsi_link_pm_policy(enum link_pm policy)
> +{
> +	int i;
> +	char *name = NULL;
> +
> +	for (i = 0; i < ARRAY_SIZE(link_pm_policy); i++) {
> +		if (link_pm_policy[i].value == policy) {
> +			name = link_pm_policy[i].name;
> +			break;
> +		}
> +	}
> +	return name;
> +}
> +
> +static ssize_t store_link_pm_policy(struct class_device *class_dev,
> +	const char *buf, size_t count)
> +{
> +	struct Scsi_Host *shost = class_to_shost(class_dev);
> +	struct ata_port *ap = ata_shost_to_port(shost);
> +	enum link_pm policy = 0;
> +	int i;
> +
> +	/*
> + 	 * we are skipping array location 0 on purpose - this
> + 	 * is because a value of NOT_AVAILABLE is displayed
> + 	 * to the user as max_performance, but when the user
> + 	 * writes "max_performance", they actually want the
> + 	 * value to match MAX_PERFORMANCE.
> + 	 */
> +	for (i = 1; i < ARRAY_SIZE(link_pm_policy); i++) {
> +		const int len = strlen(link_pm_policy[i].name);
> +		if (strncmp(link_pm_policy[i].name, buf, len) == 0 &&
> +		   buf[len] == '\n') {
> +			policy = link_pm_policy[i].value;
> +			break;
> +		}
> +	}
> +	if (!policy)
> +		return -EINVAL;
> +
> +	if (ata_scsi_set_link_pm_policy(ap, policy))
> +		return -EINVAL;
> +	return count;
> +}
> +
> +static ssize_t
> +show_link_pm_policy(struct class_device *class_dev, char *buf)
> +{
> +	struct Scsi_Host *shost = class_to_shost(class_dev);
> +	struct ata_port *ap = ata_shost_to_port(shost);
> +	const char *policy =
> +		ata_scsi_link_pm_policy(ap->pm_policy);
> +
> +	if (!policy)
> +		return -EINVAL;
> +
> +	return snprintf(buf, 23, "%s\n", policy);
> +}
> +CLASS_DEVICE_ATTR(link_power_management_policy, S_IRUGO | S_IWUSR,
> +		show_link_pm_policy, store_link_pm_policy);
> +EXPORT_SYMBOL_GPL(class_device_attr_link_power_management_policy);
> +
>  static void ata_scsi_invalid_field(struct scsi_cmnd *cmd,
>  				   void (*done)(struct scsi_cmnd *))
>  {
> @@ -3041,6 +3113,17 @@ void ata_scsi_simulate(struct ata_device
>  	}
>  }
>  
> +int ata_scsi_set_link_pm_policy(struct ata_port *ap,
> +		enum link_pm policy)
> +{
> +	ap->pm_policy = policy;
> +	ap->link.eh_info.action |= ATA_EHI_LPM;
> +	ap->link.eh_info.flags |= ATA_EHI_NO_AUTOPSY;
> +	ata_port_schedule_eh(ap);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(ata_scsi_set_link_pm_policy);
> +
>  int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht)
>  {
>  	int i, rc;
> Index: libata-dev/include/linux/libata.h
> ===================================================================
> --- libata-dev.orig/include/linux/libata.h	2007-09-24 13:43:10.000000000 -0700
> +++ libata-dev/include/linux/libata.h	2007-09-24 13:47:57.000000000 -0700
> @@ -140,6 +140,8 @@ enum {
>  	ATA_DFLAG_ACPI_PENDING	= (1 << 5), /* ACPI resume action pending */
>  	ATA_DFLAG_ACPI_FAILED	= (1 << 6), /* ACPI on devcfg has failed */
>  	ATA_DFLAG_AN		= (1 << 7), /* device supports AN */
> +	ATA_DFLAG_HIPM		= (1 << 8), /* device supports HIPM */
> +	ATA_DFLAG_DIPM		= (1 << 9), /* device supports DIPM */
>  	ATA_DFLAG_CFG_MASK	= (1 << 12) - 1,
>  
>  	ATA_DFLAG_PIO		= (1 << 12), /* device limited to PIO mode */
> @@ -181,6 +183,7 @@ enum {
>  	ATA_FLAG_NO_IORDY	= (1 << 16), /* controller lacks iordy */
>  	ATA_FLAG_ACPI_SATA	= (1 << 17), /* need native SATA ACPI layout */
>  	ATA_FLAG_AN		= (1 << 18), /* controller supports AN */
> +	ATA_FLAG_IPM		= (1 << 19), /* driver can handle IPM */
>  
>  	/* The following flag belongs to ap->pflags but is kept in
>  	 * ap->flags because it's referenced in many LLDs and will be
> @@ -283,6 +286,7 @@ enum {
>  	ATA_EHI_RESUME_LINK	= (1 << 1),  /* resume link (reset modifier) */
>  	ATA_EHI_NO_AUTOPSY	= (1 << 2),  /* no autopsy */
>  	ATA_EHI_QUIET		= (1 << 3),  /* be quiet */
> +	ATA_EHI_LPM		= (1 << 4),  /* link power management action */
>  
>  	ATA_EHI_DID_SOFTRESET	= (1 << 16), /* already soft-reset this port */
>  	ATA_EHI_DID_HARDRESET	= (1 << 17), /* already soft-reset this port */
> @@ -308,6 +312,7 @@ enum {
>  	ATA_HORKAGE_NONCQ	= (1 << 2),	/* Don't use NCQ */
>  	ATA_HORKAGE_MAX_SEC_128	= (1 << 3),	/* Limit max sects to 128 */
>  	ATA_HORKAGE_BROKEN_HPA	= (1 << 4),	/* Broken HPA */
> +	ATA_HORKAGE_IPM		= (1 << 5), 	/* LPM problems */
>  };
>  
>  enum hsm_task_states {
> @@ -347,6 +352,18 @@ typedef int (*ata_reset_fn_t)(struct ata
>  			      unsigned long deadline);
>  typedef void (*ata_postreset_fn_t)(struct ata_link *link, unsigned int *classes);
>  
> +/*
> + * host pm policy: If you alter this, you also need to alter libata-scsi.c
> + * (for the ascii descriptions)
> + */
> +enum link_pm {
> +	NOT_AVAILABLE,
> +	MIN_POWER,
> +	MAX_PERFORMANCE,
> +	MEDIUM_POWER,
> +};
> +extern struct class_device_attribute class_device_attr_link_power_management_policy;
> +
>  struct ata_ioports {
>  	void __iomem		*cmd_addr;
>  	void __iomem		*data_addr;
> @@ -585,6 +602,7 @@ struct ata_port {
>  
>  	pm_message_t		pm_mesg;
>  	int			*pm_result;
> +	enum link_pm		pm_policy;
>  
>  	struct timer_list	fastdrain_timer;
>  	unsigned long		fastdrain_cnt;
> @@ -647,7 +665,8 @@ struct ata_port_operations {
>  
>  	int (*port_suspend) (struct ata_port *ap, pm_message_t mesg);
>  	int (*port_resume) (struct ata_port *ap);
> -
> +	int (*enable_pm) (struct ata_port *ap, enum link_pm policy);
> +	int (*disable_pm) (struct ata_port *ap);
>  	int (*port_start) (struct ata_port *ap);
>  	void (*port_stop) (struct ata_port *ap);
>  
> @@ -854,7 +873,9 @@ extern int ata_cable_40wire(struct ata_p
>  extern int ata_cable_80wire(struct ata_port *ap);
>  extern int ata_cable_sata(struct ata_port *ap);
>  extern int ata_cable_unknown(struct ata_port *ap);
> -
> +extern int ata_scsi_set_link_pm_policy(struct ata_port *ap, enum link_pm);
> +extern int ata_dev_enable_pm(struct ata_device *dev, enum link_pm policy);
> +extern void ata_dev_disable_pm(struct ata_device *dev);
>  /*
>   * Timing helpers
>   */
> Index: libata-dev/drivers/ata/libata-core.c
> ===================================================================
> --- libata-dev.orig/drivers/ata/libata-core.c	2007-09-24 13:43:10.000000000 -0700
> +++ libata-dev/drivers/ata/libata-core.c	2007-09-24 13:46:22.000000000 -0700
> @@ -68,7 +68,8 @@ const unsigned long sata_deb_timing_long
>  static unsigned int ata_dev_init_params(struct ata_device *dev,
>  					u16 heads, u16 sectors);
>  static unsigned int ata_dev_set_xfermode(struct ata_device *dev);
> -static unsigned int ata_dev_set_AN(struct ata_device *dev, u8 enable);
> +static unsigned int ata_dev_set_feature(struct ata_device *dev,
> +					u8 enable, u8 feature);
>  static void ata_dev_xfermask(struct ata_device *dev);
>  static unsigned long ata_dev_blacklisted(const struct ata_device *dev);
>  
> @@ -615,6 +616,129 @@ void ata_dev_disable(struct ata_device *
>  	}
>  }
>  
> +static int ata_dev_set_dipm(struct ata_device *dev, enum link_pm policy)
> +{
> +	struct ata_link *link = dev->link;
> +	struct ata_port *ap = link->ap;
> +	u32 scontrol;
> +
> +	/*
> +	 * disallow DIPM for drivers which haven't set
> +	 * ATA_FLAG_IPM.  This is because when DIPM is enabled,
> +	 * phy ready will be set in the interrupt status on
> +	 * state changes, which will cause some drivers to
> +	 * think there are errors - additionally drivers will
> +	 * need to disable hot plug.
> +	 */
> +	if (!(ap->flags & ATA_FLAG_IPM) || !ata_dev_enabled(dev)) {

	if (!((ap->flags & ATA_FLAG_IPM) && ata_dev_enabled(dev))) {

> +		ap->pm_policy = NOT_AVAILABLE;
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * For DIPM, we will only enable it for the
> +	 * min_power setting.
> +	 *
> +	 * Why?  Because Disks are too stupid to know that
> +	 * If the host rejects a request to go to SLUMBER
> +	 * they should retry at PARTIAL, and instead it
> +	 * just would give up.  So, for medium_power to
> +	 * work at all, we need to only allow HIPM.
> +	 */
> +	sata_scr_read(link, SCR_CONTROL, &scontrol);
> +
> +	switch (policy) {
> +	case MIN_POWER:
> +		/* no restrictions on IPM transitions */
> +		scontrol &= ~(0x3 << 8);
> +		sata_scr_write(link, SCR_CONTROL, scontrol);
> +
> +		/* enable DIPM */
> +		if (dev->flags & ATA_DFLAG_DIPM)
> +			ata_dev_set_feature(dev, SETFEATURES_SATA_ENABLE,
> +						SATA_DIPM);
> +		break;
> +	case MEDIUM_POWER:
> +		/* allow IPM to PARTIAL */
> +		scontrol &= ~(0x1 << 8);
> +		scontrol |= (0x2 << 8);
> +		sata_scr_write(link, SCR_CONTROL, scontrol);
> +
> +		/* disable DIPM */
> +		if (ata_dev_enabled(dev) && (dev->flags & ATA_DFLAG_DIPM))
> +			ata_dev_set_feature(dev, SETFEATURES_SATA_DISABLE,
> +						SATA_DIPM);
> +		break;
> +	case NOT_AVAILABLE:
> +	case MAX_PERFORMANCE:
> +		/* disable all IPM transitions */
> +		scontrol |= (0x3 << 8);
> +		sata_scr_write(link, SCR_CONTROL, scontrol);
> +
> +		/* disable DIPM */
> +		if (ata_dev_enabled(dev) && (dev->flags & ATA_DFLAG_DIPM))
> +			ata_dev_set_feature(dev, SETFEATURES_SATA_DISABLE,
> +						SATA_DIPM);
> +		break;
> +	}
> +	return 0;
> +}
> +
> +/**
> + *	ata_dev_enable_pm - enable SATA interface power management
> + *	@device - device to enable ipm for
> + *	@policy - the link power management policy
> + *
> + *	Enable SATA Interface power management.  This will enable
> + *	Device Interface Power Management (DIPM) for min_power
> + * 	policy, and then call driver specific callbacks for
> + *	enabling Host Initiated Power management.
> + *
> + *	Locking: Caller.
> + *	Returns: -EINVAL if IPM is not supported, 0 otherwise.
> + */
> +int ata_dev_enable_pm(struct ata_device *dev, enum link_pm policy)
> +{
> +	int rc = 0;
> +	struct ata_port *ap = dev->link->ap;
> +
> +	/* set HIPM first, then DIPM */
> +	if (ap->ops->enable_pm)
> +		rc = ap->ops->enable_pm(ap, policy);
> +	if (rc)
> +		goto enable_pm_out;
> +	rc = ata_dev_set_dipm(dev, policy);
> +
> +enable_pm_out:
> +	if (rc)
> +		ap->pm_policy = MAX_PERFORMANCE;
> +	else
> +		ap->pm_policy = policy;
> +	return rc;
> +}
> +
> +/**
> + *	ata_dev_disable_pm - disable SATA interface power management
> + *	@device - device to enable ipm for
> + *
> + *	Disable SATA Interface power management.  This will disable
> + *	Device Interface Power Management (DIPM) without changing
> + * 	policy,  call driver specific callbacks for disabling Host
> + * 	Initiated Power management.
> + *
> + *	Locking: Caller.
> + *	Returns: void
> + */
> +void ata_dev_disable_pm(struct ata_device *dev)
> +{
> +	struct ata_port *ap = dev->link->ap;
> +
> +	ata_dev_set_dipm(dev, MAX_PERFORMANCE);
> +	if (ap->ops->disable_pm)
> +		ap->ops->disable_pm(ap);
> +}
> +
> +
>  /**
>   *	ata_devchk - PATA device presence detection
>   *	@ap: ATA channel to examine
> @@ -2029,7 +2153,8 @@ int ata_dev_configure(struct ata_device 
>  		if ((ap->flags & ATA_FLAG_AN) && ata_id_has_AN(id)) {
>  			int err;
>  			/* issue SET feature command to turn this on */
> -			err = ata_dev_set_AN(dev, SETFEATURES_SATA_ENABLE);
> +			err = ata_dev_set_feature(dev, SETFEATURES_SATA_ENABLE,
> +						SATA_AN);
>  			if (err)
>  				ata_dev_printk(dev, KERN_ERR,
>  						"unable to set AN, err %x\n",
> @@ -2057,6 +2182,13 @@ int ata_dev_configure(struct ata_device 
>  	if (dev->flags & ATA_DFLAG_LBA48)
>  		dev->max_sectors = ATA_MAX_SECTORS_LBA48;
>  
> +	if (!(dev->horkage & ATA_HORKAGE_IPM)) {
> +		if (ata_id_has_hipm(dev->id))
> +			dev->flags |= ATA_DFLAG_HIPM;
> +		if (ata_id_has_dipm(dev->id))
> +			dev->flags |= ATA_DFLAG_DIPM;
> +	}
> +
>  	if (dev->horkage & ATA_HORKAGE_DIAGNOSTIC) {
>  		/* Let the user know. We don't want to disallow opens for
>  		   rescue purposes, or in case the vendor is just a blithering
> @@ -2082,6 +2214,13 @@ int ata_dev_configure(struct ata_device 
>  		dev->max_sectors = min_t(unsigned int, ATA_MAX_SECTORS_128,
>  					 dev->max_sectors);
>  
> +	if (ata_dev_blacklisted(dev) & ATA_HORKAGE_IPM) {
> +		dev->horkage |= ATA_HORKAGE_IPM;
> +
> +		/* reset link pm_policy for this port to no pm */
> +		ap->pm_policy = MAX_PERFORMANCE;
> +	}
> +
>  	if (ap->ops->dev_config)
>  		ap->ops->dev_config(dev);
>  
> @@ -4048,15 +4187,14 @@ static unsigned int ata_dev_set_xfermode
>  	DPRINTK("EXIT, err_mask=%x\n", err_mask);
>  	return err_mask;
>  }
> -
>  /**
> - *	ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES
> + *	ata_dev_set_feature - Issue SET FEATURES - SATA FEATURES
>   *	@dev: Device to which command will be sent
>   *	@enable: Whether to enable or disable the feature
> + *	@feature: The sector count represents the feature to set
>   *
>   *	Issue SET FEATURES - SATA FEATURES command to device @dev
> - *	on port @ap with sector count set to indicate Asynchronous
> - *	Notification feature
> + *	on port @ap with sector count
>   *
>   *	LOCKING:
>   *	PCI/etc. bus probe sem.
> @@ -4064,7 +4202,8 @@ static unsigned int ata_dev_set_xfermode
>   *	RETURNS:
>   *	0 on success, AC_ERR_* mask otherwise.
>   */
> -static unsigned int ata_dev_set_AN(struct ata_device *dev, u8 enable)
> +static unsigned int ata_dev_set_feature(struct ata_device *dev, u8 enable,
> +					u8 feature)
>  {
>  	struct ata_taskfile tf;
>  	unsigned int err_mask;
> @@ -4077,7 +4216,7 @@ static unsigned int ata_dev_set_AN(struc
>  	tf.feature = enable;
>  	tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
>  	tf.protocol = ATA_PROT_NODATA;
> -	tf.nsect = SATA_AN;
> +	tf.nsect = feature;
>  
>  	err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0);
>  
> @@ -6031,6 +6170,32 @@ int ata_flush_cache(struct ata_device *d
>  	return 0;
>  }
>  
> +static void ata_host_disable_link_pm(struct ata_host *host)
> +{
> +	int i;
> +	struct ata_link *link;
> +	struct ata_port *ap;
> +	struct ata_device *dev;
> +
> +	for (i = 0; i < host->n_ports; i++) {
> +		ap = host->ports[i];
> +		ata_port_for_each_link(link, ap) {
> +			ata_link_for_each_dev(dev, link)
> +				ata_dev_disable_pm(dev);
> +		}
> +	}
> +}
> +
> +static void ata_host_enable_link_pm(struct ata_host *host)
> +{
> +	int i;
> +
> +	for (i = 0; i < host->n_ports; i++) {
> +		struct ata_port *ap = host->ports[i];
> +		ata_scsi_set_link_pm_policy(ap, ap->pm_policy);
> +	}
> +}
> +
>  #ifdef CONFIG_PM
>  static int ata_host_request_pm(struct ata_host *host, pm_message_t mesg,
>  			       unsigned int action, unsigned int ehi_flags,
> @@ -6101,6 +6266,12 @@ int ata_host_suspend(struct ata_host *ho
>  {
>  	int rc;
>  
> +	/*
> + 	 * disable link pm on all ports before requesting
> + 	 * any pm activity
> + 	 */
> +	ata_host_disable_link_pm(host);
> +
>  	rc = ata_host_request_pm(host, mesg, 0, ATA_EHI_QUIET, 1);
>  	if (rc == 0)
>  		host->dev->power.power_state = mesg;
> @@ -6123,6 +6294,9 @@ void ata_host_resume(struct ata_host *ho
>  	ata_host_request_pm(host, PMSG_ON, ATA_EH_SOFTRESET,
>  			    ATA_EHI_NO_AUTOPSY | ATA_EHI_QUIET, 0);
>  	host->dev->power.power_state = PMSG_ON;
> +
> +	/* reenable link pm */
> +	ata_host_enable_link_pm(host);
>  }
>  #endif
>  
> @@ -6663,6 +6837,7 @@ int ata_host_register(struct ata_host *h
>  		struct ata_port *ap = host->ports[i];
>  
>  		ata_scsi_scan_host(ap, 1);
> +		ata_scsi_set_link_pm_policy(ap, ap->pm_policy);
>  	}
>  
>  	return 0;
> @@ -7059,7 +7234,8 @@ const struct ata_port_info ata_dummy_por
>   * likely to change as new drivers are added and updated.
>   * Do not depend on ABI/API stability.
>   */
> -
> +EXPORT_SYMBOL_GPL(ata_dev_enable_pm);
> +EXPORT_SYMBOL_GPL(ata_dev_disable_pm);
>  EXPORT_SYMBOL_GPL(sata_deb_timing_normal);
>  EXPORT_SYMBOL_GPL(sata_deb_timing_hotplug);
>  EXPORT_SYMBOL_GPL(sata_deb_timing_long);
> Index: libata-dev/include/linux/ata.h
> ===================================================================
> --- libata-dev.orig/include/linux/ata.h	2007-09-24 13:43:10.000000000 -0700
> +++ libata-dev/include/linux/ata.h	2007-09-24 13:46:22.000000000 -0700
> @@ -235,6 +235,7 @@ enum {
>  
>  	/* SETFEATURE Sector counts for SATA features */
>  	SATA_AN			= 0x05,  /* Asynchronous Notification */
> +	SATA_DIPM		= 0x03,  /* Device Initiated Power Management */
>  
>  	/* ATAPI stuff */
>  	ATAPI_PKT_DMA		= (1 << 0),
> @@ -367,6 +368,12 @@ struct ata_taskfile {
>  	  ((u64) (id)[(n) + 0]) )
>  
>  #define ata_id_cdb_intr(id)	(((id)[0] & 0x60) == 0x20)
> +#define ata_id_has_hipm(id)	\
> +	( (((id)[76] != 0x0000) && ((id)[76] != 0xffff)) && \
> +	  ((id)[76] & (1 << 9)) )
		^
		|
are you sure this
should be 76?

we can also change the first statement a bit:
	(!(((id)[76] == 0x0000) || ((id)[76] == 0xffff)) && \


> +#define ata_id_has_dipm(id)	\
> +	( (((id)[76] != 0x0000) && ((id)[76] != 0xffff)) && \

and:
	(!(((id)[76] == 0x0000) || ((id)[76] == 0xffff)) && \

> +	  ((id)[78] & (1 << 3)) )
>  
>  static inline int ata_id_has_fua(const u16 *id)
>  {
> Index: libata-dev/Documentation/scsi/link_power_management_policy.txt
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ libata-dev/Documentation/scsi/link_power_management_policy.txt	2007-09-24 13:46:22.000000000 -0700
> @@ -0,0 +1,19 @@
> +This parameter allows the user to set the link (interface) power management.
> +There are 3 possible options:
> +
> +Value			Effect
> +----------------------------------------------------------------------------
> +min_power		Tell the controller to try to make the link use the
> +			least possible power when possible.  This may
> +			sacrifice some performance due to increased latency
> +			when coming out of lower power states.
> +
> +max_performance		Generally, this means no power management.  Tell
> +			the controller to have performance be a priority
> +			over power management.
> +
> +medium_power		Tell the controller to enter a lower power state
> +			when possible, but do not enter the lowest power
> +			state, thus improving latency over min_power setting.
> +
> +
> Index: libata-dev/drivers/ata/libata-eh.c
> ===================================================================
> --- libata-dev.orig/drivers/ata/libata-eh.c	2007-09-24 13:43:10.000000000 -0700
> +++ libata-dev/drivers/ata/libata-eh.c	2007-09-24 13:46:22.000000000 -0700
> @@ -2400,6 +2400,11 @@ static int ata_eh_recover(struct ata_por
>  			ehc->i.flags &= ~ATA_EHI_SETMODE;
>  		}
>  
> +		if (ehc->i.action & ATA_EHI_LPM) {
> +			ata_link_for_each_dev(dev, link)
> +				ata_dev_enable_pm(dev, ap->pm_policy);
> +		}
> +
>  		/* this link is okay now */
>  		ehc->i.flags = 0;
>  		continue;
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch 1/2] Enable link power management for ata drivers
  2007-09-24 23:12   ` roel
@ 2007-09-24 23:28     ` Davide Libenzi
  2007-09-25  0:00       ` roel
  2007-09-24 23:41     ` Kristen Carlson Accardi
  1 sibling, 1 reply; 11+ messages in thread
From: Davide Libenzi @ 2007-09-24 23:28 UTC (permalink / raw)
  To: roel
  Cc: Kristen Carlson Accardi, jgarzik, linux-ide,
	Linux Kernel Mailing List, axboe, Andrew Morton

On Tue, 25 Sep 2007, roel wrote:

> > +	if (!(ap->flags & ATA_FLAG_IPM) || !ata_dev_enabled(dev)) {
> 
> 	if (!((ap->flags & ATA_FLAG_IPM) && ata_dev_enabled(dev))) {

int foo(int i, int j) {
        
        return !(i & 8) || !j;
}

int moo(int i, int j) {
        
        return !((i & 8) && j);
}


gcc -O2 -S:


.globl foo
        .type   foo, @function
foo:
        shrl    $3, %edi
        xorl    $1, %edi
        testl   %esi, %esi
        sete    %al
        orl     %eax, %edi
        andl    $1, %edi
        movl    %edi, %eax
        ret
.globl moo
        .type   moo, @function
moo:
        shrl    $3, %edi
        xorl    $1, %edi
        testl   %esi, %esi
        sete    %al
        orl     %eax, %edi
        andl    $1, %edi
        movl    %edi, %eax
        ret



- Davide

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch 1/2] Enable link power management for ata drivers
  2007-09-24 23:12   ` roel
  2007-09-24 23:28     ` Davide Libenzi
@ 2007-09-24 23:41     ` Kristen Carlson Accardi
  2007-09-24 23:59       ` Jeff Garzik
  1 sibling, 1 reply; 11+ messages in thread
From: Kristen Carlson Accardi @ 2007-09-24 23:41 UTC (permalink / raw)
  To: roel; +Cc: jgarzik, linux-ide, linux-kernel, axboe, akpm

On Tue, 25 Sep 2007 01:12:32 +0200
roel <12o3l@tiscali.nl> wrote:

> >  #define ata_id_cdb_intr(id)	(((id)[0] & 0x60) == 0x20)
> > +#define ata_id_has_hipm(id)	\
> > +	( (((id)[76] != 0x0000) && ((id)[76] != 0xffff)) && \
> > +	  ((id)[76] & (1 << 9)) )
> 		^
> 		|
> are you sure this
> should be 76?

Yes.

> 
> we can also change the first statement a bit:
> 	(!(((id)[76] == 0x0000) || ((id)[76] == 0xffff)) && \
> 
> 
> > +#define ata_id_has_dipm(id)	\
> > +	( (((id)[76] != 0x0000) && ((id)[76] != 0xffff)) && \
> 
> and:
> 	(!(((id)[76] == 0x0000) || ((id)[76] == 0xffff)) && \

I feel this is equivalent functionality and not as readable.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch 1/2] Enable link power management for ata drivers
  2007-09-24 23:41     ` Kristen Carlson Accardi
@ 2007-09-24 23:59       ` Jeff Garzik
  2007-09-25  9:50         ` Alan Cox
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff Garzik @ 2007-09-24 23:59 UTC (permalink / raw)
  To: Kristen Carlson Accardi; +Cc: roel, linux-ide, linux-kernel, akpm, Alan

Kristen Carlson Accardi wrote:
> On Tue, 25 Sep 2007 01:12:32 +0200
> roel <12o3l@tiscali.nl> wrote:
> 
>>>  #define ata_id_cdb_intr(id)	(((id)[0] & 0x60) == 0x20)
>>> +#define ata_id_has_hipm(id)	\
>>> +	( (((id)[76] != 0x0000) && ((id)[76] != 0xffff)) && \
>>> +	  ((id)[76] & (1 << 9)) )
>> 		^
>> 		|
>> are you sure this
>> should be 76?
> 
> Yes.
> 
>> we can also change the first statement a bit:
>> 	(!(((id)[76] == 0x0000) || ((id)[76] == 0xffff)) && \
>>
>>
>>> +#define ata_id_has_dipm(id)	\
>>> +	( (((id)[76] != 0x0000) && ((id)[76] != 0xffff)) && \
>> and:
>> 	(!(((id)[76] == 0x0000) || ((id)[76] == 0xffff)) && \
> 
> I feel this is equivalent functionality and not as readable.

Poke around for Alan Cox's cleanup of this area of linux/ata.h.

It converts several macros to inline functions (encouraged), and also 
illustrates a nice, clean way of testing an ID word's validity. 
[obviously the final implementation varies, depending on that ID word's 
history]

Alan or Andrew, got a copy somewhere?  My feeble search skills don't 
seem to turn it up at the moment, even though I had a copy in my hands 
quite recently.

	Jeff



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch 1/2] Enable link power management for ata drivers
  2007-09-24 23:28     ` Davide Libenzi
@ 2007-09-25  0:00       ` roel
  0 siblings, 0 replies; 11+ messages in thread
From: roel @ 2007-09-25  0:00 UTC (permalink / raw)
  To: Davide Libenzi
  Cc: Kristen Carlson Accardi, jgarzik, linux-ide,
	Linux Kernel Mailing List, axboe, Andrew Morton

Davide Libenzi wrote:
> On Tue, 25 Sep 2007, roel wrote:
> 
>>> +	if (!(ap->flags & ATA_FLAG_IPM) || !ata_dev_enabled(dev)) {
>> 	if (!((ap->flags & ATA_FLAG_IPM) && ata_dev_enabled(dev))) {
> 
> int foo(int i, int j) {
>         
>         return !(i & 8) || !j;
> }
> 
> int moo(int i, int j) {
>         
>         return !((i & 8) && j);
> }
> 
> 
> gcc -O2 -S:
> 
> 
> .globl foo
>         .type   foo, @function
> foo:
>         shrl    $3, %edi
>         xorl    $1, %edi
>         testl   %esi, %esi
>         sete    %al
>         orl     %eax, %edi
>         andl    $1, %edi
>         movl    %edi, %eax
>         ret
> .globl moo
>         .type   moo, @function
> moo:
>         shrl    $3, %edi
>         xorl    $1, %edi
>         testl   %esi, %esi
>         sete    %al
>         orl     %eax, %edi
>         andl    $1, %edi
>         movl    %edi, %eax
>         ret

Indeed, no difference, except for the eye. 

do you not consider it an improvement or do you not want to change it?
or
don't you consider it an improvement and want to change it?

Never mind. Disregard if you please.

> - Davide


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch 1/2] Enable link power management for ata drivers
  2007-09-24 23:59       ` Jeff Garzik
@ 2007-09-25  9:50         ` Alan Cox
  2007-09-26  2:45           ` Jeff Garzik
  0 siblings, 1 reply; 11+ messages in thread
From: Alan Cox @ 2007-09-25  9:50 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Kristen Carlson Accardi, roel, linux-ide, linux-kernel, akpm

> It converts several macros to inline functions (encouraged), and also 
> illustrates a nice, clean way of testing an ID word's validity. 
> [obviously the final implementation varies, depending on that ID word's 
> history]

Its in -mm and I thought you put a copy in your tree after I said it
hadn't upset anything in -mm ?

Alan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch 1/2] Enable link power management for ata drivers
  2007-09-25  9:50         ` Alan Cox
@ 2007-09-26  2:45           ` Jeff Garzik
  0 siblings, 0 replies; 11+ messages in thread
From: Jeff Garzik @ 2007-09-26  2:45 UTC (permalink / raw)
  To: Alan Cox; +Cc: Kristen Carlson Accardi, roel, linux-ide, linux-kernel, akpm

Alan Cox wrote:
>> It converts several macros to inline functions (encouraged), and also 
>> illustrates a nice, clean way of testing an ID word's validity. 
>> [obviously the final implementation varies, depending on that ID word's 
>> history]
> 
> Its in -mm and I thought you put a copy in your tree after I said it
> hadn't upset anything in -mm ?

it didn't apply straightaway to my tree for whatever reason, IIRC



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch 1/2] Enable link power management for ata drivers
  2007-09-24 22:13 ` [patch 1/2] Enable link power management for ata drivers Kristen Carlson Accardi
  2007-09-24 23:12   ` roel
@ 2007-10-25  5:21   ` Jeff Garzik
  1 sibling, 0 replies; 11+ messages in thread
From: Jeff Garzik @ 2007-10-25  5:21 UTC (permalink / raw)
  To: Kristen Carlson Accardi; +Cc: linux-ide, linux-kernel, axboe, akpm, Tejun Heo

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

applied as the attached two patches to jgarzik/libata-dev.git#alpm

open issues:

1) need to check ata_dev_set_feature() return value in 
ata_dev_set_dipm() and do something useful with it

2) as the name implies, this probably better belongs in ata_link.

3) however, the feature is tightly coupled to the host controller.  in 
theory PMP -might- do this, but I think its unlikely.  as such I was OK 
with the present arrangement.

4) there has been some discussion of software-initiated device/link 
power management, but I think this should go in, in parallel with those 
discussions.  ALPM
	* is quite self-contained
	* gives a noticable power savings

I'm definitely interested in seeing somebody pursue software-initiated 
link PM as well...

	Jeff




[-- Attachment #2: patch.1 --]
[-- Type: text/plain, Size: 3467 bytes --]

commit 218f3d30e60f32394738372c594d063f8e43ee6d
Author: Jeff Garzik <jeff@garzik.org>
Date:   Thu Oct 25 00:33:27 2007 -0400

    [libata] Create internal helper ata_dev_set_feature()
    
    Signed-off-by: Jeff Garzik <jgarzik@redhat.com>

 drivers/ata/libata-core.c |   26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

218f3d30e60f32394738372c594d063f8e43ee6d
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 2d147b5..294eee3 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -68,7 +68,8 @@ const unsigned long sata_deb_timing_long[]		= { 100, 2000, 5000 };
 static unsigned int ata_dev_init_params(struct ata_device *dev,
 					u16 heads, u16 sectors);
 static unsigned int ata_dev_set_xfermode(struct ata_device *dev);
-static unsigned int ata_dev_set_AN(struct ata_device *dev, u8 enable);
+static unsigned int ata_dev_set_feature(struct ata_device *dev,
+					u8 enable, u8 feature);
 static void ata_dev_xfermask(struct ata_device *dev);
 static unsigned long ata_dev_blacklisted(const struct ata_device *dev);
 
@@ -1799,13 +1800,7 @@ int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class,
 		 * SET_FEATURES spin-up subcommand before it will accept
 		 * anything other than the original IDENTIFY command.
 		 */
-		ata_tf_init(dev, &tf);
-		tf.command = ATA_CMD_SET_FEATURES;
-		tf.feature = SETFEATURES_SPINUP;
-		tf.protocol = ATA_PROT_NODATA;
-		tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
-		err_mask = ata_exec_internal(dev, &tf, NULL,
-					     DMA_NONE, NULL, 0, 0);
+		err_mask = ata_dev_set_feature(dev, SETFEATURES_SPINUP, 0);
 		if (err_mask && id[2] != 0x738c) {
 			rc = -EIO;
 			reason = "SPINUP failed";
@@ -2075,7 +2070,8 @@ int ata_dev_configure(struct ata_device *dev)
 			unsigned int err_mask;
 
 			/* issue SET feature command to turn this on */
-			err_mask = ata_dev_set_AN(dev, SETFEATURES_SATA_ENABLE);
+			err_mask = ata_dev_set_feature(dev,
+					SETFEATURES_SATA_ENABLE, SATA_AN);
 			if (err_mask)
 				ata_dev_printk(dev, KERN_ERR,
 					"failed to enable ATAPI AN "
@@ -4181,15 +4177,14 @@ static unsigned int ata_dev_set_xfermode(struct ata_device *dev)
 	DPRINTK("EXIT, err_mask=%x\n", err_mask);
 	return err_mask;
 }
-
 /**
- *	ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES
+ *	ata_dev_set_feature - Issue SET FEATURES - SATA FEATURES
  *	@dev: Device to which command will be sent
  *	@enable: Whether to enable or disable the feature
+ *	@feature: The sector count represents the feature to set
  *
  *	Issue SET FEATURES - SATA FEATURES command to device @dev
- *	on port @ap with sector count set to indicate Asynchronous
- *	Notification feature
+ *	on port @ap with sector count
  *
  *	LOCKING:
  *	PCI/etc. bus probe sem.
@@ -4197,7 +4192,8 @@ static unsigned int ata_dev_set_xfermode(struct ata_device *dev)
  *	RETURNS:
  *	0 on success, AC_ERR_* mask otherwise.
  */
-static unsigned int ata_dev_set_AN(struct ata_device *dev, u8 enable)
+static unsigned int ata_dev_set_feature(struct ata_device *dev, u8 enable,
+					u8 feature)
 {
 	struct ata_taskfile tf;
 	unsigned int err_mask;
@@ -4210,7 +4206,7 @@ static unsigned int ata_dev_set_AN(struct ata_device *dev, u8 enable)
 	tf.feature = enable;
 	tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
 	tf.protocol = ATA_PROT_NODATA;
-	tf.nsect = SATA_AN;
+	tf.nsect = feature;
 
 	err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, 0);
 

[-- Attachment #3: patch.2 --]
[-- Type: text/plain, Size: 16517 bytes --]

commit 01fdf95f877e2a3f767248a5896e08a9e8787971
Author: Kristen Carlson Accardi <kristen.c.accardi@intel.com>
Date:   Thu Oct 25 00:58:59 2007 -0400

    [libata] Link power management infrastructure
    
    Device Initiated Power Management, which is defined
    in SATA 2.5 can be enabled for disks which support it.
    This patch enables DIPM when the user sets the link
    power management policy to "min_power".
    
    Additionally, libata drivers can define a function
    (enable_pm) that will perform hardware specific actions to
    enable whatever power management policy the user set up
    for Host Initiated Power management (HIPM).
    This power management policy will be activated after all
    disks have been enumerated and intialized.  Drivers should
    also define disable_pm, which will turn off link power
    management, but not change link power management policy.
    
    Documentation/scsi/link_power_management_policy.txt has additional
    information.
    
    Signed-off-by:  Kristen Carlson Accardi <kristen.c.accardi@intel.com>
    Signed-off-by: Jeff Garzik <jgarzik@redhat.com>

 Documentation/scsi/link_power_management_policy.txt |   19 +
 drivers/ata/libata-core.c                           |  196 +++++++++++++++++++-
 drivers/ata/libata-eh.c                             |    4 
 drivers/ata/libata-scsi.c                           |   68 ++++++
 drivers/ata/libata.h                                |    2 
 include/linux/ata.h                                 |   21 ++
 include/linux/libata.h                              |   21 ++
 7 files changed, 329 insertions(+), 2 deletions(-)

01fdf95f877e2a3f767248a5896e08a9e8787971
diff --git a/Documentation/scsi/link_power_management_policy.txt b/Documentation/scsi/link_power_management_policy.txt
new file mode 100644
index 0000000..d18993d
--- /dev/null
+++ b/Documentation/scsi/link_power_management_policy.txt
@@ -0,0 +1,19 @@
+This parameter allows the user to set the link (interface) power management.
+There are 3 possible options:
+
+Value			Effect
+----------------------------------------------------------------------------
+min_power		Tell the controller to try to make the link use the
+			least possible power when possible.  This may
+			sacrifice some performance due to increased latency
+			when coming out of lower power states.
+
+max_performance		Generally, this means no power management.  Tell
+			the controller to have performance be a priority
+			over power management.
+
+medium_power		Tell the controller to enter a lower power state
+			when possible, but do not enter the lowest power
+			state, thus improving latency over min_power setting.
+
+
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 294eee3..a89ab56 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -620,6 +620,177 @@ void ata_dev_disable(struct ata_device *dev)
 	}
 }
 
+static int ata_dev_set_dipm(struct ata_device *dev, enum link_pm policy)
+{
+	struct ata_link *link = dev->link;
+	struct ata_port *ap = link->ap;
+	u32 scontrol;
+	unsigned int err_mask;
+	int rc;
+
+	/*
+	 * disallow DIPM for drivers which haven't set
+	 * ATA_FLAG_IPM.  This is because when DIPM is enabled,
+	 * phy ready will be set in the interrupt status on
+	 * state changes, which will cause some drivers to
+	 * think there are errors - additionally drivers will
+	 * need to disable hot plug.
+	 */
+	if (!(ap->flags & ATA_FLAG_IPM) || !ata_dev_enabled(dev)) {
+		ap->pm_policy = NOT_AVAILABLE;
+		return -EINVAL;
+	}
+
+	/*
+	 * For DIPM, we will only enable it for the
+	 * min_power setting.
+	 *
+	 * Why?  Because Disks are too stupid to know that
+	 * If the host rejects a request to go to SLUMBER
+	 * they should retry at PARTIAL, and instead it
+	 * just would give up.  So, for medium_power to
+	 * work at all, we need to only allow HIPM.
+	 */
+	rc = sata_scr_read(link, SCR_CONTROL, &scontrol);
+	if (rc)
+		return rc;
+
+	switch (policy) {
+	case MIN_POWER:
+		/* no restrictions on IPM transitions */
+		scontrol &= ~(0x3 << 8);
+		rc = sata_scr_write(link, SCR_CONTROL, scontrol);
+		if (rc)
+			return rc;
+
+		/* enable DIPM */
+		if (dev->flags & ATA_DFLAG_DIPM)
+			err_mask = ata_dev_set_feature(dev,
+					SETFEATURES_SATA_ENABLE, SATA_DIPM);
+		break;
+	case MEDIUM_POWER:
+		/* allow IPM to PARTIAL */
+		scontrol &= ~(0x1 << 8);
+		scontrol |= (0x2 << 8);
+		rc = sata_scr_write(link, SCR_CONTROL, scontrol);
+		if (rc)
+			return rc;
+
+		/* disable DIPM */
+		if (ata_dev_enabled(dev) && (dev->flags & ATA_DFLAG_DIPM))
+			err_mask = ata_dev_set_feature(dev,
+					SETFEATURES_SATA_DISABLE, SATA_DIPM);
+		break;
+	case NOT_AVAILABLE:
+	case MAX_PERFORMANCE:
+		/* disable all IPM transitions */
+		scontrol |= (0x3 << 8);
+		rc = sata_scr_write(link, SCR_CONTROL, scontrol);
+		if (rc)
+			return rc;
+
+		/* disable DIPM */
+		if (ata_dev_enabled(dev) && (dev->flags & ATA_DFLAG_DIPM))
+			err_mask = ata_dev_set_feature(dev,
+					SETFEATURES_SATA_DISABLE, SATA_DIPM);
+		break;
+	}
+
+	/* FIXME: handle SET FEATURES failure */
+	(void) err_mask;
+
+	return 0;
+}
+
+/**
+ *	ata_dev_enable_pm - enable SATA interface power management
+ *	@device - device to enable ipm for
+ *	@policy - the link power management policy
+ *
+ *	Enable SATA Interface power management.  This will enable
+ *	Device Interface Power Management (DIPM) for min_power
+ * 	policy, and then call driver specific callbacks for
+ *	enabling Host Initiated Power management.
+ *
+ *	Locking: Caller.
+ *	Returns: -EINVAL if IPM is not supported, 0 otherwise.
+ */
+void ata_dev_enable_pm(struct ata_device *dev, enum link_pm policy)
+{
+	int rc = 0;
+	struct ata_port *ap = dev->link->ap;
+
+	/* set HIPM first, then DIPM */
+	if (ap->ops->enable_pm)
+		rc = ap->ops->enable_pm(ap, policy);
+	if (rc)
+		goto enable_pm_out;
+	rc = ata_dev_set_dipm(dev, policy);
+
+enable_pm_out:
+	if (rc)
+		ap->pm_policy = MAX_PERFORMANCE;
+	else
+		ap->pm_policy = policy;
+	return /* rc */;	/* hopefully we can use 'rc' eventually */
+}
+
+/**
+ *	ata_dev_disable_pm - disable SATA interface power management
+ *	@device - device to enable ipm for
+ *
+ *	Disable SATA Interface power management.  This will disable
+ *	Device Interface Power Management (DIPM) without changing
+ * 	policy,  call driver specific callbacks for disabling Host
+ * 	Initiated Power management.
+ *
+ *	Locking: Caller.
+ *	Returns: void
+ */
+static void ata_dev_disable_pm(struct ata_device *dev)
+{
+	struct ata_port *ap = dev->link->ap;
+
+	ata_dev_set_dipm(dev, MAX_PERFORMANCE);
+	if (ap->ops->disable_pm)
+		ap->ops->disable_pm(ap);
+}
+
+void ata_lpm_schedule(struct ata_port *ap, enum link_pm policy)
+{
+	ap->pm_policy = policy;
+	ap->link.eh_info.action |= ATA_EHI_LPM;
+	ap->link.eh_info.flags |= ATA_EHI_NO_AUTOPSY;
+	ata_port_schedule_eh(ap);
+}
+
+static void ata_lpm_enable(struct ata_host *host)
+{
+	struct ata_link *link;
+	struct ata_port *ap;
+	struct ata_device *dev;
+	int i;
+
+	for (i = 0; i < host->n_ports; i++) {
+		ap = host->ports[i];
+		ata_port_for_each_link(link, ap) {
+			ata_link_for_each_dev(dev, link)
+				ata_dev_disable_pm(dev);
+		}
+	}
+}
+
+static void ata_lpm_disable(struct ata_host *host)
+{
+	int i;
+
+	for (i = 0; i < host->n_ports; i++) {
+		struct ata_port *ap = host->ports[i];
+		ata_lpm_schedule(ap, ap->pm_policy);
+	}
+}
+
+
 /**
  *	ata_devchk - PATA device presence detection
  *	@ap: ATA channel to examine
@@ -2101,6 +2272,13 @@ int ata_dev_configure(struct ata_device *dev)
 	if (dev->flags & ATA_DFLAG_LBA48)
 		dev->max_sectors = ATA_MAX_SECTORS_LBA48;
 
+	if (!(dev->horkage & ATA_HORKAGE_IPM)) {
+		if (ata_id_has_hipm(dev->id))
+			dev->flags |= ATA_DFLAG_HIPM;
+		if (ata_id_has_dipm(dev->id))
+			dev->flags |= ATA_DFLAG_DIPM;
+	}
+
 	if (dev->horkage & ATA_HORKAGE_DIAGNOSTIC) {
 		/* Let the user know. We don't want to disallow opens for
 		   rescue purposes, or in case the vendor is just a blithering
@@ -2126,6 +2304,13 @@ int ata_dev_configure(struct ata_device *dev)
 		dev->max_sectors = min_t(unsigned int, ATA_MAX_SECTORS_128,
 					 dev->max_sectors);
 
+	if (ata_dev_blacklisted(dev) & ATA_HORKAGE_IPM) {
+		dev->horkage |= ATA_HORKAGE_IPM;
+
+		/* reset link pm_policy for this port to no pm */
+		ap->pm_policy = MAX_PERFORMANCE;
+	}
+
 	if (ap->ops->dev_config)
 		ap->ops->dev_config(dev);
 
@@ -6292,6 +6477,12 @@ int ata_host_suspend(struct ata_host *host, pm_message_t mesg)
 {
 	int rc;
 
+	/*
+	 * disable link pm on all ports before requesting
+	 * any pm activity
+	 */
+	ata_lpm_enable(host);
+
 	rc = ata_host_request_pm(host, mesg, 0, ATA_EHI_QUIET, 1);
 	if (rc == 0)
 		host->dev->power.power_state = mesg;
@@ -6314,6 +6505,9 @@ void ata_host_resume(struct ata_host *host)
 	ata_host_request_pm(host, PMSG_ON, ATA_EH_SOFTRESET,
 			    ATA_EHI_NO_AUTOPSY | ATA_EHI_QUIET, 0);
 	host->dev->power.power_state = PMSG_ON;
+
+	/* reenable link pm */
+	ata_lpm_disable(host);
 }
 #endif
 
@@ -6856,6 +7050,7 @@ int ata_host_register(struct ata_host *host, struct scsi_host_template *sht)
 		struct ata_port *ap = host->ports[i];
 
 		ata_scsi_scan_host(ap, 1);
+		ata_lpm_schedule(ap, ap->pm_policy);
 	}
 
 	return 0;
@@ -7252,7 +7447,6 @@ const struct ata_port_info ata_dummy_port_info = {
  * likely to change as new drivers are added and updated.
  * Do not depend on ABI/API stability.
  */
-
 EXPORT_SYMBOL_GPL(sata_deb_timing_normal);
 EXPORT_SYMBOL_GPL(sata_deb_timing_hotplug);
 EXPORT_SYMBOL_GPL(sata_deb_timing_long);
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 93e2b54..2cae530 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -2607,6 +2607,10 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
 			ehc->i.flags &= ~ATA_EHI_SETMODE;
 		}
 
+		if (ehc->i.action & ATA_EHI_LPM)
+			ata_link_for_each_dev(dev, link)
+				ata_dev_enable_pm(dev, ap->pm_policy);
+
 		/* this link is okay now */
 		ehc->i.flags = 0;
 		continue;
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index f5d5420..2111faa 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -110,6 +110,74 @@ static struct scsi_transport_template ata_scsi_transport_template = {
 };
 
 
+static const struct {
+	enum link_pm	value;
+	const char	*name;
+} link_pm_policy[] = {
+	{ NOT_AVAILABLE, "max_performance" },
+	{ MIN_POWER, "min_power" },
+	{ MAX_PERFORMANCE, "max_performance" },
+	{ MEDIUM_POWER, "medium_power" },
+};
+
+const char *ata_scsi_lpm_get(enum link_pm policy)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(link_pm_policy); i++)
+		if (link_pm_policy[i].value == policy)
+			return link_pm_policy[i].name;
+
+	return NULL;
+}
+
+static ssize_t ata_scsi_lpm_put(struct class_device *class_dev,
+	const char *buf, size_t count)
+{
+	struct Scsi_Host *shost = class_to_shost(class_dev);
+	struct ata_port *ap = ata_shost_to_port(shost);
+	enum link_pm policy = 0;
+	int i;
+
+	/*
+	 * we are skipping array location 0 on purpose - this
+	 * is because a value of NOT_AVAILABLE is displayed
+	 * to the user as max_performance, but when the user
+	 * writes "max_performance", they actually want the
+	 * value to match MAX_PERFORMANCE.
+	 */
+	for (i = 1; i < ARRAY_SIZE(link_pm_policy); i++) {
+		const int len = strlen(link_pm_policy[i].name);
+		if (strncmp(link_pm_policy[i].name, buf, len) == 0 &&
+		   buf[len] == '\n') {
+			policy = link_pm_policy[i].value;
+			break;
+		}
+	}
+	if (!policy)
+		return -EINVAL;
+
+	ata_lpm_schedule(ap, policy);
+	return count;
+}
+
+static ssize_t
+ata_scsi_lpm_show(struct class_device *class_dev, char *buf)
+{
+	struct Scsi_Host *shost = class_to_shost(class_dev);
+	struct ata_port *ap = ata_shost_to_port(shost);
+	const char *policy =
+		ata_scsi_lpm_get(ap->pm_policy);
+
+	if (!policy)
+		return -EINVAL;
+
+	return snprintf(buf, 23, "%s\n", policy);
+}
+CLASS_DEVICE_ATTR(link_power_management_policy, S_IRUGO | S_IWUSR,
+		ata_scsi_lpm_show, ata_scsi_lpm_put);
+EXPORT_SYMBOL_GPL(class_device_attr_link_power_management_policy);
+
 static void ata_scsi_invalid_field(struct scsi_cmnd *cmd,
 				   void (*done)(struct scsi_cmnd *))
 {
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 90df58a..0e6cf3a 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -101,6 +101,8 @@ extern int sata_link_init_spd(struct ata_link *link);
 extern int ata_task_ioctl(struct scsi_device *scsidev, void __user *arg);
 extern int ata_cmd_ioctl(struct scsi_device *scsidev, void __user *arg);
 extern struct ata_port *ata_port_alloc(struct ata_host *host);
+extern void ata_dev_enable_pm(struct ata_device *dev, enum link_pm policy);
+extern void ata_lpm_schedule(struct ata_port *ap, enum link_pm);
 
 /* libata-acpi.c */
 #ifdef CONFIG_ATA_ACPI
diff --git a/include/linux/ata.h b/include/linux/ata.h
index 8263a7b..bf84032 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -235,6 +235,7 @@ enum {
 
 	/* SETFEATURE Sector counts for SATA features */
 	SATA_AN			= 0x05,  /* Asynchronous Notification */
+	SATA_DIPM		= 0x03,  /* Device Initiated Power Management */
 
 	/* ATAPI stuff */
 	ATAPI_PKT_DMA		= (1 << 0),
@@ -377,6 +378,26 @@ struct ata_taskfile {
 
 #define ata_id_cdb_intr(id)	(((id)[0] & 0x60) == 0x20)
 
+static inline bool ata_id_has_hipm(const u16 *id)
+{
+	u16 val = id[76];
+
+	if (val == 0 || val == 0xffff)
+		return false;
+
+	return val & (1 << 9);
+}
+
+static inline bool ata_id_has_dipm(const u16 *id)
+{
+	u16 val = id[78];
+
+	if (val == 0 || val == 0xffff)
+		return false;
+
+	return val & (1 << 3);
+}
+
 static inline int ata_id_has_fua(const u16 *id)
 {
 	if ((id[84] & 0xC000) != 0x4000)
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 6fd24e0..ededd26 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -133,6 +133,8 @@ enum {
 	ATA_DFLAG_ACPI_PENDING	= (1 << 5), /* ACPI resume action pending */
 	ATA_DFLAG_ACPI_FAILED	= (1 << 6), /* ACPI on devcfg has failed */
 	ATA_DFLAG_AN		= (1 << 7), /* AN configured */
+	ATA_DFLAG_HIPM		= (1 << 8), /* device supports HIPM */
+	ATA_DFLAG_DIPM		= (1 << 9), /* device supports DIPM */
 	ATA_DFLAG_CFG_MASK	= (1 << 12) - 1,
 
 	ATA_DFLAG_PIO		= (1 << 12), /* device limited to PIO mode */
@@ -185,6 +187,7 @@ enum {
 	ATA_FLAG_ACPI_SATA	= (1 << 17), /* need native SATA ACPI layout */
 	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 */
 
 	/* The following flag belongs to ap->pflags but is kept in
 	 * ap->flags because it's referenced in many LLDs and will be
@@ -294,6 +297,7 @@ enum {
 	ATA_EHI_RESUME_LINK	= (1 << 1),  /* resume link (reset modifier) */
 	ATA_EHI_NO_AUTOPSY	= (1 << 2),  /* no autopsy */
 	ATA_EHI_QUIET		= (1 << 3),  /* be quiet */
+	ATA_EHI_LPM		= (1 << 4),  /* link power management action */
 
 	ATA_EHI_DID_SOFTRESET	= (1 << 16), /* already soft-reset this port */
 	ATA_EHI_DID_HARDRESET	= (1 << 17), /* already soft-reset this port */
@@ -325,6 +329,7 @@ enum {
 	ATA_HORKAGE_BROKEN_HPA	= (1 << 4),	/* Broken HPA */
 	ATA_HORKAGE_SKIP_PM	= (1 << 5),	/* Skip PM operations */
 	ATA_HORKAGE_HPA_SIZE	= (1 << 6),	/* native size off by one */
+	ATA_HORKAGE_IPM		= (1 << 7),	/* Link PM problems */
 
 	 /* DMA mask for user DMA control: User visible values; DO NOT
 	    renumber */
@@ -370,6 +375,18 @@ typedef int (*ata_reset_fn_t)(struct ata_link *link, unsigned int *classes,
 			      unsigned long deadline);
 typedef void (*ata_postreset_fn_t)(struct ata_link *link, unsigned int *classes);
 
+/*
+ * host pm policy: If you alter this, you also need to alter libata-scsi.c
+ * (for the ascii descriptions)
+ */
+enum link_pm {
+	NOT_AVAILABLE,
+	MIN_POWER,
+	MAX_PERFORMANCE,
+	MEDIUM_POWER,
+};
+extern struct class_device_attribute class_device_attr_link_power_management_policy;
+
 struct ata_ioports {
 	void __iomem		*cmd_addr;
 	void __iomem		*data_addr;
@@ -616,6 +633,7 @@ struct ata_port {
 
 	pm_message_t		pm_mesg;
 	int			*pm_result;
+	enum link_pm		pm_policy;
 
 	struct timer_list	fastdrain_timer;
 	unsigned long		fastdrain_cnt;
@@ -683,7 +701,8 @@ struct ata_port_operations {
 
 	int (*port_suspend) (struct ata_port *ap, pm_message_t mesg);
 	int (*port_resume) (struct ata_port *ap);
-
+	int (*enable_pm) (struct ata_port *ap, enum link_pm policy);
+	void (*disable_pm) (struct ata_port *ap);
 	int (*port_start) (struct ata_port *ap);
 	void (*port_stop) (struct ata_port *ap);
 

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [patch 2/2] Enable Aggressive Link Power management for AHCI controllers.
  2007-09-24 22:14 ` [patch 2/2] Enable Aggressive Link Power management for AHCI controllers Kristen Carlson Accardi
@ 2007-10-25  5:34   ` Jeff Garzik
  0 siblings, 0 replies; 11+ messages in thread
From: Jeff Garzik @ 2007-10-25  5:34 UTC (permalink / raw)
  To: Kristen Carlson Accardi; +Cc: linux-ide, linux-kernel, axboe, akpm

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

Kristen Carlson Accardi wrote:
> This patch will set the correct bits to turn on Aggressive
> Link Power Management (ALPM) for the ahci driver.  This
> will cause the controller and disk to negotiate a lower
> power state for the link when there is no activity (see
> the AHCI 1.x spec for details).  This feature is mutually
> exclusive with Hot Plug, so when ALPM is enabled, Hot Plug
> is disabled.  ALPM will be enabled by default, but it is
> settable via the scsi host syfs interface.  Possible 
> settings for this feature are:
> 
> Setting		Effect
> ----------------------------------------------------------
> min_power	ALPM is enabled, and link set to enter 
> 		lowest power state (SLUMBER) when idle
> 		Hot plug not allowed.
> 
> max_performance	ALPM is disabled, Hot Plug is allowed
> 
> medium_power	ALPM is enabled, and link set to enter
> 		second lowest power state (PARTIAL) when
> 		idle.  Hot plug not allowed.
> 
> Signed-off-by:  Kristen Carlson Accardi <kristen.c.accardi@intel.com>

applied, with minor changes (attached)

Please check libata-dev.git#alpm and make sure it still works as 
expected (after it mirrors out to git.kernel.org)



[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 8653 bytes --]

commit 0fdf9763a76f4cce208b7139e555019217765e81
Author: Kristen Carlson Accardi <kristen.c.accardi@intel.com>
Date:   Thu Oct 25 01:33:26 2007 -0400

    [libata] AHCI: add hw link power management support
    
    This patch will set the correct bits to turn on Aggressive
    Link Power Management (ALPM) for the ahci driver.  This
    will cause the controller and disk to negotiate a lower
    power state for the link when there is no activity (see
    the AHCI 1.x spec for details).  This feature is mutually
    exclusive with Hot Plug, so when ALPM is enabled, Hot Plug
    is disabled.  ALPM will be enabled by default, but it is
    settable via the scsi host syfs interface.  Possible
    settings for this feature are:
    
    Setting         Effect
    ----------------------------------------------------------
    min_power       ALPM is enabled, and link set to enter
                    lowest power state (SLUMBER) when idle
                    Hot plug not allowed.
    
    max_performance ALPM is disabled, Hot Plug is allowed
    
    medium_power    ALPM is enabled, and link set to enter
                    second lowest power state (PARTIAL) when
                    idle.  Hot plug not allowed.
    
    Signed-off-by:  Kristen Carlson Accardi <kristen.c.accardi@intel.com>
    Signed-off-by: Jeff Garzik <jgarzik@redhat.com>

 drivers/ata/ahci.c |  157 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 155 insertions(+), 2 deletions(-)

0fdf9763a76f4cce208b7139e555019217765e81
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 95229e7..2b4faca 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -48,6 +48,9 @@
 #define DRV_NAME	"ahci"
 #define DRV_VERSION	"3.0"
 
+static int ahci_enable_alpm(struct ata_port *ap,
+		enum link_pm policy);
+static void ahci_disable_alpm(struct ata_port *ap);
 
 enum {
 	AHCI_PCI_BAR		= 5,
@@ -98,6 +101,7 @@ enum {
 	HOST_CAP_SSC		= (1 << 14), /* Slumber capable */
 	HOST_CAP_PMP		= (1 << 17), /* Port Multiplier support */
 	HOST_CAP_CLO		= (1 << 24), /* Command List Override support */
+	HOST_CAP_ALPM		= (1 << 26), /* Aggressive Link PM support */
 	HOST_CAP_SSS		= (1 << 27), /* Staggered Spin-up */
 	HOST_CAP_SNTF		= (1 << 29), /* SNotification register */
 	HOST_CAP_NCQ		= (1 << 30), /* Native Command Queueing */
@@ -154,6 +158,8 @@ enum {
 				  PORT_IRQ_PIOS_FIS | PORT_IRQ_D2H_REG_FIS,
 
 	/* PORT_CMD bits */
+	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_PMP		= (1 << 17), /* PMP attached */
 	PORT_CMD_LIST_ON	= (1 << 15), /* cmd list DMA engine running */
@@ -177,13 +183,14 @@ enum {
 	AHCI_HFLAG_MV_PATA		= (1 << 4), /* PATA port */
 	AHCI_HFLAG_NO_MSI		= (1 << 5), /* no PCI MSI */
 	AHCI_HFLAG_NO_PMP		= (1 << 6), /* no PMP */
+	AHCI_HFLAG_NO_HOTPLUG		= (1 << 7), /* ignore PxSERR.DIAG.N */
 
 	/* ap->flags bits */
-	AHCI_FLAG_NO_HOTPLUG		= (1 << 24), /* ignore PxSERR.DIAG.N */
 
 	AHCI_FLAG_COMMON		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 					  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
-					  ATA_FLAG_ACPI_SATA | ATA_FLAG_AN,
+					  ATA_FLAG_ACPI_SATA | ATA_FLAG_AN |
+					  ATA_FLAG_IPM,
 	AHCI_LFLAG_COMMON		= ATA_LFLAG_SKIP_D2H_BSY,
 };
 
@@ -252,6 +259,11 @@ static int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg);
 static int ahci_pci_device_resume(struct pci_dev *pdev);
 #endif
 
+static struct class_device_attribute *ahci_shost_attrs[] = {
+	&class_device_attr_link_power_management_policy,
+	NULL
+};
+
 static struct scsi_host_template ahci_sht = {
 	.module			= THIS_MODULE,
 	.name			= DRV_NAME,
@@ -269,6 +281,7 @@ static struct scsi_host_template ahci_sht = {
 	.slave_configure	= ata_scsi_slave_config,
 	.slave_destroy		= ata_scsi_slave_destroy,
 	.bios_param		= ata_std_bios_param,
+	.shost_attrs		= ahci_shost_attrs,
 };
 
 static const struct ata_port_operations ahci_ops = {
@@ -300,6 +313,8 @@ static const struct ata_port_operations ahci_ops = {
 	.port_suspend		= ahci_port_suspend,
 	.port_resume		= ahci_port_resume,
 #endif
+	.enable_pm		= ahci_enable_alpm,
+	.disable_pm		= ahci_disable_alpm,
 
 	.port_start		= ahci_port_start,
 	.port_stop		= ahci_port_stop,
@@ -800,6 +815,130 @@ static void ahci_power_up(struct ata_port *ap)
 	writel(cmd | PORT_CMD_ICC_ACTIVE, port_mmio + PORT_CMD);
 }
 
+static void ahci_disable_alpm(struct ata_port *ap)
+{
+	struct ahci_host_priv *hpriv = ap->host->private_data;
+	void __iomem *port_mmio = ahci_port_base(ap);
+	u32 cmd;
+	struct ahci_port_priv *pp = ap->private_data;
+
+	/* IPM bits should be disabled by libata-core */
+	/* get the existing command bits */
+	cmd = readl(port_mmio + PORT_CMD);
+
+	/* disable ALPM and ASP */
+	cmd &= ~PORT_CMD_ASP;
+	cmd &= ~PORT_CMD_ALPE;
+
+	/* force the interface back to active */
+	cmd |= PORT_CMD_ICC_ACTIVE;
+
+	/* write out new cmd value */
+	writel(cmd, port_mmio + PORT_CMD);
+	cmd = readl(port_mmio + PORT_CMD);
+
+	/* wait 10ms to be sure we've come out of any low power state */
+	msleep(10);
+
+	/* clear out any PhyRdy stuff from interrupt status */
+	writel(PORT_IRQ_PHYRDY, port_mmio + PORT_IRQ_STAT);
+
+	/* go ahead and clean out PhyRdy Change from Serror too */
+	ahci_scr_write(ap, SCR_ERROR, ((1 << 16) | (1 << 18)));
+
+	/*
+ 	 * Clear flag to indicate that we should ignore all PhyRdy
+ 	 * state changes
+ 	 */
+	hpriv->flags &= ~AHCI_HFLAG_NO_HOTPLUG;
+
+	/*
+ 	 * Enable interrupts on Phy Ready.
+ 	 */
+	pp->intr_mask |= PORT_IRQ_PHYRDY;
+	writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
+
+	/*
+ 	 * don't change the link pm policy - we can be called
+ 	 * just to turn of link pm temporarily
+ 	 */
+}
+
+static int ahci_enable_alpm(struct ata_port *ap,
+	enum link_pm policy)
+{
+	struct ahci_host_priv *hpriv = ap->host->private_data;
+	void __iomem *port_mmio = ahci_port_base(ap);
+	u32 cmd;
+	struct ahci_port_priv *pp = ap->private_data;
+	u32 asp;
+
+	/* Make sure the host is capable of link power management */
+	if (!(hpriv->cap & HOST_CAP_ALPM))
+		return -EINVAL;
+
+	switch (policy) {
+	case MAX_PERFORMANCE:
+	case NOT_AVAILABLE:
+		/*
+ 		 * if we came here with NOT_AVAILABLE,
+ 		 * it just means this is the first time we
+ 		 * have tried to enable - default to max performance,
+ 		 * and let the user go to lower power modes on request.
+ 		 */
+		ahci_disable_alpm(ap);
+		return 0;
+	case MIN_POWER:
+		/* configure HBA to enter SLUMBER */
+		asp = PORT_CMD_ASP;
+		break;
+	case MEDIUM_POWER:
+		/* configure HBA to enter PARTIAL */
+		asp = 0;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/*
+ 	 * Disable interrupts on Phy Ready. This keeps us from
+ 	 * getting woken up due to spurious phy ready interrupts
+	 * TBD - Hot plug should be done via polling now, is
+	 * that even supported?
+ 	 */
+	pp->intr_mask &= ~PORT_IRQ_PHYRDY;
+	writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
+
+	/*
+ 	 * Set a flag to indicate that we should ignore all PhyRdy
+ 	 * state changes since these can happen now whenever we
+ 	 * change link state
+ 	 */
+	hpriv->flags |= AHCI_HFLAG_NO_HOTPLUG;
+
+	/* get the existing command bits */
+	cmd = readl(port_mmio + PORT_CMD);
+
+	/*
+ 	 * Set ASP based on Policy
+ 	 */
+	cmd |= asp;
+
+	/*
+ 	 * Setting this bit will instruct the HBA to aggressively
+ 	 * enter a lower power link state when it's appropriate and
+ 	 * based on the value set above for ASP
+ 	 */
+	cmd |= PORT_CMD_ALPE;
+
+	/* write out new cmd value */
+	writel(cmd, port_mmio + PORT_CMD);
+	cmd = readl(port_mmio + PORT_CMD);
+
+	/* IPM bits should be set by libata-core */
+	return 0;
+}
+
 #ifdef CONFIG_PM
 static void ahci_power_down(struct ata_port *ap)
 {
@@ -1426,6 +1565,17 @@ static void ahci_port_intr(struct ata_port *ap)
 	if (unlikely(resetting))
 		status &= ~PORT_IRQ_BAD_PMP;
 
+	/* If we are getting PhyRdy, this is
+ 	 * just a power state change, we should
+ 	 * clear out this, plus the PhyRdy/Comm
+ 	 * Wake bits from Serror
+ 	 */
+	if ((hpriv->flags & AHCI_HFLAG_NO_HOTPLUG) &&
+		(status & PORT_IRQ_PHYRDY)) {
+		status &= ~PORT_IRQ_PHYRDY;
+		ahci_scr_write(ap, SCR_ERROR, ((1 << 16) | (1 << 18)));
+	}
+
 	if (unlikely(status & PORT_IRQ_ERROR)) {
 		ahci_error_intr(ap, status);
 		return;
@@ -2015,6 +2165,9 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		ata_port_pbar_desc(ap, AHCI_PCI_BAR,
 				   0x100 + ap->port_no * 0x80, "port");
 
+		/* set initial link pm policy */
+		ap->pm_policy = NOT_AVAILABLE;
+
 		/* standard SATA port setup */
 		if (hpriv->port_map & (1 << i))
 			ap->ioaddr.cmd_addr = port_mmio;

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2007-10-25  5:34 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20070924215140.966161778@intel.com>
2007-09-24 22:13 ` [patch 1/2] Enable link power management for ata drivers Kristen Carlson Accardi
2007-09-24 23:12   ` roel
2007-09-24 23:28     ` Davide Libenzi
2007-09-25  0:00       ` roel
2007-09-24 23:41     ` Kristen Carlson Accardi
2007-09-24 23:59       ` Jeff Garzik
2007-09-25  9:50         ` Alan Cox
2007-09-26  2:45           ` Jeff Garzik
2007-10-25  5:21   ` Jeff Garzik
2007-09-24 22:14 ` [patch 2/2] Enable Aggressive Link Power management for AHCI controllers Kristen Carlson Accardi
2007-10-25  5:34   ` Jeff Garzik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).