ahci: Dont start port DMA engines unless a device is present

If a port doesn't support hot plug, there's no reason to keep the phy powered on unoccupied ports.

Quoted 2 lines · Kristen Carlson Accardi

If a port doesn't support hot plug, there's no reason to keep the phy powered on unoccupied ports.

Would it be possible to add some writable sysfs attribute for this too?

A lot of ports support hot plug, but the users knows they will never actually hot plug anything there. Exposing it to sysfs would make sense so someone could make a conscious decision to save these watts by sacrifying hotplug-support.

-Andi

According to the AHCI spec, you should not set PxCMD.ST unless a functional device is present, as determined by PxTFD.STS.BSY=0, PxTFD.STS.DRQ=0, and PxSSTS.DET = 3.

Signed-off-by Kristen Carlson Accardi <[email protected]>

Index: linux-2.6.25-rc3/drivers/ata/ahci.c
===================================================================

drivers/ata/ahci.c +21 -0

@@ -783,11 +783,40 @@ static int ahci_scr_write(struct ata_por
 	return -EINVAL;
 }
 
+static int ahci_is_device_present(struct ata_port *ap)
+{
+	void __iomem *port_mmio = ahci_port_base(ap);
+	u8 status;
+
+	/* Make sure PxTFD.STS.BSY and PxTFD.STS.DRQ are 0 */
+	status = ahci_check_status(ap);
+	if (status & (ATA_BUSY | ATA_DRQ))
+		return 0;
+
+	/* Make sure PxSSTS.DET is 3h */
+	status = readl(port_mmio + PORT_SCR_STAT);
+	status &= 0xf;
+	if ((status & 0xf) != 3)
+		return 0;
+	return 1;
+}
+
 static void ahci_start_engine(struct ata_port *ap)
 {
 	void __iomem *port_mmio = ahci_port_base(ap);
 	u32 tmp;
 
+	/*
+	 * See AHCI specification 1.2 section 10.1.2
+	 *

If a port doesn't support hot plug, there's no reason to keep the phy powered on unoccupied ports.

Signed-off-by Kristen Carlson Accardi <[email protected]>

Index: linux-2.6.25-rc3/drivers/ata/ahci.c
===================================================================

drivers/ata/ahci.c +1 -0

@@ -52,6 +52,7 @@
 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,

Quoted 18 lines · Kristen Carlson Accardi

If a port doesn't support hot plug, there's no reason to keep the phy powered on unoccupied ports.

Signed-off-by Kristen Carlson Accardi <[email protected]>

Index: linux-2.6.25-rc3/drivers/ata/ahci.c
===================================================================

drivers/ata/ahci.c +1 -0

@@ -52,6 +52,7 @@
 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,

drivers/ata/ahci.c enum { · line 167

Quoted 4 lines

drivers/ata/ahci.c +1 -0

@@ -163,6 +164,7 @@ 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_HPCP		= (1 << 18), /* port is hot plug capable */

Under which conditions is this bit set?

The conclusion reached by this patch seems correct, but I am not sure about the premise...

I was under the impression that AHCI ports were hotplug capable, from libata's point of view, simply due to the fundamentals of SATA.

Thinking about the bigger pictures, powering off the phy is something we want to do in a lot more cases than this, but there is a stumbling
block: we wander into the realm of policy.

For most users most of the time, empty SATA ports are needlessly powered. The problem is that, at any given moment, a device may be hot-plugged, so we must be ready for that. We need some way for the user to let the driver know that they will not be hotplugging anything anytime soon, permitting power savings to be enabled.

A compromise solution that avoids adding a userspace "knob" has also been proposed (by Tejun, I think?): power up the phy every N seconds, check for device, power down phy if nothing. That should provide some power savings, though not as much as with a "knob" switched to "hotplug:
off"

Jeff

On Sat, 12 Apr 2008 00:28:13 -0400
Jeff Garzik <[email protected]> wrote:

Quoted 5 lines

Thinking about the bigger pictures, powering off the phy is something we want to do in a lot more cases than this, but there is a stumbling block: we wander into the realm of policy.

For most users most of the time, empty SATA ports are needlessly powered. The problem is that, at any given moment, a device may be hot-plugged, so we must be ready for that. We need some way for the user to let the driver know that they will not be hotplugging anything anytime soon, permitting power savings to be enabled.

A compromise solution that avoids adding a userspace "knob" has also been proposed (by Tejun, I think?): power up the phy every N seconds, check for device, power down phy if nothing. That should provide some power savings, though not as much as with a "knob" switched to "hotplug: off"

I think that for the common case - we should use HPCP to determine if
the suggested use of the port is for hot plug. I can see your point
about wanting to give the user the option to just disregard the intended
use of the port and do what they want, but I say we don't make that
the default behavior. And, I don't like the idea of adding another
wakeup in the driver to do polling - seems like 99% of users are
going to be just fine with a knob - and that they should only
have to use the knob to override the default (or how bout a module param?). I don't think we should compromise power for a feature that most people are unlikely to use (if HPCP is not set).

Quoted 2 lines · Kristen Carlson Accardi

On Sat, 12 Apr 2008 00:28:13 -0400
Jeff Garzik <[email protected]> wrote:

..

Quoted 10 lines

Quoted 10 lines

Thinking about the bigger pictures, powering off the phy is something we want to do in a lot more cases than this, but there is a stumbling
block: we wander into the realm of policy.

For most users most of the time, empty SATA ports are needlessly powered. The problem is that, at any given moment, a device may be hot-plugged, so we must be ready for that. We need some way for the user to let the driver know that they will not be hotplugging anything anytime soon, permitting power savings to be enabled.

Quoted 5 lines

Quoted 5 lines

A compromise solution that avoids adding a userspace "knob" has also been proposed (by Tejun, I think?): power up the phy every N seconds, check for device, power down phy if nothing. That should provide some power savings, though not as much as with a "knob" switched to "hotplug:
off"

..

How long does a poll actually take, in real time? Probably a hundred milliseconds or two, to see if the PHY syncs up?

So with a knob set to, say 2 seconds (sysfs), then this would save 90% of the power of a permanent "off". That's pretty good, and setting that same knob to "infinity" (-1 ?) would achieve 100%. Definitely a good way to go, IMHO.

Quoted 10 lines

I think that for the common case - we should use HPCP to determine if the suggested use of the port is for hot plug. I can see your point about wanting to give the user the option to just disregard the intended use of the port and do what they want, but I say we don't make that the default behavior. And, I don't like the idea of adding another wakeup in the driver to do polling - seems like 99% of users are going to be just fine with a knob - and that they should only have to use the knob to override the default (or how bout a module param?). I don't think we should compromise power for a feature that most people are unlikely to use (if HPCP is not set).

..

I worry that this is far too x86 / vendor specific.
Most of the SATA ports I have here, for example, probably lack this HPCP bit even on x86, and on other arch's it likely doesn't exist at all, right?

For arch/system/device that does have HPCP implemented, then sure, it could be used to automatically tweak the sysfs knob. But from userspace perhaps, rather than kernel ?

Thanks