All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] ata: ahci: Enable DEVSLP by default on SLP_S0 support
@ 2018-07-27 20:47 Srinivas Pandruvada
  2018-07-27 20:47 ` [PATCH v2 1/2] ata: ahci: Support state with min power but Partial low power state Srinivas Pandruvada
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Srinivas Pandruvada @ 2018-07-27 20:47 UTC (permalink / raw)
  To: tj, hdegoede, rjw
  Cc: alan.cox, linux-ide, linux-kernel, linux-pm, Mario.Limonciello,
	Srinivas Pandruvada

One of the requirement for modern x86 system to enter lowest power mode
(SLP_S0) is SATA IP block to be off. This is true even during when
platform is suspended to idle and not only in opportunistic (runtime)
suspend.
This series is to enable DEVSLP by default.

v2:
 As suggested by Hans, take care of the module param is same as default
 LPM policy

-current (non-rfc) series
Implemented suggestions from Hans
- no override of policy set via module param
- Override lpm policy per host not global policy
- changed the new policy name to "min_power_with_partial"

rfc-v2
- As suggested by Hans, it is possible to have ASP with DEVSLP, so
add a new state.
- Removed usage of mem_sleep_current, instead just rely on low power
idle flag. Don't feel good to EXPORT from core suspend code.
- Depending host policy to decide if we enable DEVSLP by default.

Srinivas Pandruvada (2):
  ata: ahci: Support state with min power but Partial low power state
  ata: ahci: Enable DEVSLP by default on x86 with SLP_S0

 drivers/ata/ahci.c        | 38 +++++++++++++++++++++++++++++++++-----
 drivers/ata/libahci.c     |  5 ++++-
 drivers/ata/libata-core.c |  1 +
 drivers/ata/libata-scsi.c |  1 +
 include/linux/libata.h    |  3 ++-
 5 files changed, 41 insertions(+), 7 deletions(-)

-- 
2.17.1

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

* [PATCH v2 1/2] ata: ahci: Support state with min power but Partial low power state
  2018-07-27 20:47 [PATCH v2 0/2] ata: ahci: Enable DEVSLP by default on SLP_S0 support Srinivas Pandruvada
@ 2018-07-27 20:47 ` Srinivas Pandruvada
  2018-07-27 20:47 ` [PATCH v2 2/2] ata: ahci: Enable DEVSLP by default on x86 with SLP_S0 Srinivas Pandruvada
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Srinivas Pandruvada @ 2018-07-27 20:47 UTC (permalink / raw)
  To: tj, hdegoede, rjw
  Cc: alan.cox, linux-ide, linux-kernel, linux-pm, Mario.Limonciello,
	Srinivas Pandruvada

Currently when min_power policy is selected, the partial low power state
is not entered and link will try aggressively enter to only slumber state.
Add a new policy which still enable DEVSLP but also try to enter partial
low power state. This policy is presented as "min_power_with_partial".

For information the difference between partial and slumber
Partial – PHY logic is powered up, and in a reduced power state. The link
PM exit latency to active state maximum is 10 ns.
Slumber – PHY logic is powered up, and in a reduced power state. The link
PM exit latency to active state maximum is 10 ms.
Devslp – PHY logic is powered down. The link PM exit latency from this
state to active state maximum is 20 ms, unless otherwise specified by
DETO.

Suggested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/ata/libahci.c     | 5 ++++-
 drivers/ata/libata-core.c | 1 +
 drivers/ata/libata-scsi.c | 1 +
 include/linux/libata.h    | 3 ++-
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 4c49c7c2a42c..94dcec2a8a5b 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -800,6 +800,8 @@ static int ahci_set_lpm(struct ata_link *link, enum ata_lpm_policy policy,
 			cmd |= PORT_CMD_ALPE;
 			if (policy == ATA_LPM_MIN_POWER)
 				cmd |= PORT_CMD_ASP;
+			else if (policy == ATA_LPM_MIN_POWER_WITH_PARTIAL)
+				cmd &= ~PORT_CMD_ASP;
 
 			/* write out new cmd value */
 			writel(cmd, port_mmio + PORT_CMD);
@@ -810,7 +812,8 @@ static int ahci_set_lpm(struct ata_link *link, enum ata_lpm_policy policy,
 	if ((hpriv->cap2 & HOST_CAP2_SDS) &&
 	    (hpriv->cap2 & HOST_CAP2_SADM) &&
 	    (link->device->flags & ATA_DFLAG_DEVSLP)) {
-		if (policy == ATA_LPM_MIN_POWER)
+		if (policy == ATA_LPM_MIN_POWER ||
+		    policy == ATA_LPM_MIN_POWER_WITH_PARTIAL)
 			ahci_set_aggressive_devslp(ap, true);
 		else
 			ahci_set_aggressive_devslp(ap, false);
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 27d15ed7fa3d..773b5f8eed1e 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3967,6 +3967,7 @@ int sata_link_scr_lpm(struct ata_link *link, enum ata_lpm_policy policy,
 		scontrol |= (0x6 << 8);
 		break;
 	case ATA_LPM_MED_POWER_WITH_DIPM:
+	case ATA_LPM_MIN_POWER_WITH_PARTIAL:
 	case ATA_LPM_MIN_POWER:
 		if (ata_link_nr_enabled(link) > 0)
 			/* no restrictions on LPM transitions */
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 6a91d04351d9..4648bd5cee15 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -110,6 +110,7 @@ static const char *ata_lpm_policy_names[] = {
 	[ATA_LPM_MAX_POWER]		= "max_performance",
 	[ATA_LPM_MED_POWER]		= "medium_power",
 	[ATA_LPM_MED_POWER_WITH_DIPM]	= "med_power_with_dipm",
+	[ATA_LPM_MIN_POWER_WITH_PARTIAL] = "min_power_with_partial",
 	[ATA_LPM_MIN_POWER]		= "min_power",
 };
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 8b8946dd63b9..975543f8c1ff 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -522,7 +522,8 @@ enum ata_lpm_policy {
 	ATA_LPM_MAX_POWER,
 	ATA_LPM_MED_POWER,
 	ATA_LPM_MED_POWER_WITH_DIPM, /* Med power + DIPM as win IRST does */
-	ATA_LPM_MIN_POWER,
+	ATA_LPM_MIN_POWER_WITH_PARTIAL, /* Min Power + partial and slumber */
+	ATA_LPM_MIN_POWER, /* Min power + no partial (slumber only) */
 };
 
 enum ata_lpm_hints {
-- 
2.17.1

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

* [PATCH v2 2/2] ata: ahci: Enable DEVSLP by default on x86 with SLP_S0
  2018-07-27 20:47 [PATCH v2 0/2] ata: ahci: Enable DEVSLP by default on SLP_S0 support Srinivas Pandruvada
  2018-07-27 20:47 ` [PATCH v2 1/2] ata: ahci: Support state with min power but Partial low power state Srinivas Pandruvada
@ 2018-07-27 20:47 ` Srinivas Pandruvada
  2018-07-29  9:25 ` [PATCH v2 0/2] ata: ahci: Enable DEVSLP by default on SLP_S0 support Hans de Goede
  2018-07-30 14:10 ` Tejun Heo
  3 siblings, 0 replies; 5+ messages in thread
From: Srinivas Pandruvada @ 2018-07-27 20:47 UTC (permalink / raw)
  To: tj, hdegoede, rjw
  Cc: alan.cox, linux-ide, linux-kernel, linux-pm, Mario.Limonciello,
	Srinivas Pandruvada

One of the requirement for modern x86 system to enter lowest power mode
(SLP_S0) is SATA IP block to be off. This is true even during when
platform is suspended to idle and not only in opportunistic (runtime)
suspend.

Several of these system don't have traditional ACPI S3, so it is
important that they enter SLP_S0 state, to avoid draining battery even
during suspend. So it is important that out of the box Linux installation
reach this state.

SATA IP block doesn't get turned off till SATA is in DEVSLP mode. Here
user has to either use scsi-host sysfs or tools like powertop to set
the sata-host link_power_management_policy to min_power.

This change sets by default link power management policy to min_power
with partial (preferred) or slumber support on idle for some platforms.

To avoid regressions, the following conditions are used:
- User didn't override the policy from module parameter
- The kernel config is already set to use med_power_with_dipm or deeper
- System is a SLP_S0 capable using ACPI low power idle flag
This combination will make sure that systems are fairly recent and
since getting shipped with SLP_S0 support, the DEVSLP function
is already validated.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/ata/ahci.c | 38 +++++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 738fb22978dd..b14dccda0154 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -609,7 +609,7 @@ static int marvell_enable = 1;
 module_param(marvell_enable, int, 0644);
 MODULE_PARM_DESC(marvell_enable, "Marvell SATA via AHCI (1 = enabled)");
 
-static int mobile_lpm_policy = CONFIG_SATA_MOBILE_LPM_POLICY;
+static int mobile_lpm_policy = -1;
 module_param(mobile_lpm_policy, int, 0644);
 MODULE_PARM_DESC(mobile_lpm_policy, "Default LPM policy for mobile chipsets");
 
@@ -1550,6 +1550,37 @@ static int ahci_init_msi(struct pci_dev *pdev, unsigned int n_ports,
 	return pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX);
 }
 
+static void ahci_update_initial_lpm_policy(struct ata_port *ap,
+					   struct ahci_host_priv *hpriv)
+{
+	int policy = CONFIG_SATA_MOBILE_LPM_POLICY;
+
+
+	/* Ignore processing for non mobile platforms */
+	if (!(hpriv->flags & AHCI_HFLAG_IS_MOBILE))
+		return;
+
+	/* user modified policy via module param */
+	if (mobile_lpm_policy != -1) {
+		policy = mobile_lpm_policy;
+		goto update_policy;
+	}
+
+#ifdef CONFIG_ACPI
+	if (policy > ATA_LPM_MED_POWER &&
+	    (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0)) {
+		if (hpriv->cap & HOST_CAP_PART)
+			policy = ATA_LPM_MIN_POWER_WITH_PARTIAL;
+		else if (hpriv->cap & HOST_CAP_SSC)
+			policy = ATA_LPM_MIN_POWER;
+	}
+#endif
+
+update_policy:
+	if (policy >= ATA_LPM_UNKNOWN && policy <= ATA_LPM_MIN_POWER)
+		ap->target_lpm_policy = policy;
+}
+
 static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	unsigned int board_id = ent->driver_data;
@@ -1747,10 +1778,7 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		if (ap->flags & ATA_FLAG_EM)
 			ap->em_message_type = hpriv->em_msg_type;
 
-		if ((hpriv->flags & AHCI_HFLAG_IS_MOBILE) &&
-		    mobile_lpm_policy >= ATA_LPM_UNKNOWN &&
-		    mobile_lpm_policy <= ATA_LPM_MIN_POWER)
-			ap->target_lpm_policy = mobile_lpm_policy;
+		ahci_update_initial_lpm_policy(ap, hpriv);
 
 		/* disabled/not-implemented port */
 		if (!(hpriv->port_map & (1 << i)))
-- 
2.17.1

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

* Re: [PATCH v2 0/2] ata: ahci: Enable DEVSLP by default on SLP_S0 support
  2018-07-27 20:47 [PATCH v2 0/2] ata: ahci: Enable DEVSLP by default on SLP_S0 support Srinivas Pandruvada
  2018-07-27 20:47 ` [PATCH v2 1/2] ata: ahci: Support state with min power but Partial low power state Srinivas Pandruvada
  2018-07-27 20:47 ` [PATCH v2 2/2] ata: ahci: Enable DEVSLP by default on x86 with SLP_S0 Srinivas Pandruvada
@ 2018-07-29  9:25 ` Hans de Goede
  2018-07-30 14:10 ` Tejun Heo
  3 siblings, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2018-07-29  9:25 UTC (permalink / raw)
  To: Srinivas Pandruvada, tj, rjw
  Cc: alan.cox, linux-ide, linux-kernel, linux-pm, Mario.Limonciello

Hi,

On 27-07-18 22:47, Srinivas Pandruvada wrote:
> One of the requirement for modern x86 system to enter lowest power mode
> (SLP_S0) is SATA IP block to be off. This is true even during when
> platform is suspended to idle and not only in opportunistic (runtime)
> suspend.
> This series is to enable DEVSLP by default.
> 
> v2:
>   As suggested by Hans, take care of the module param is same as default
>   LPM policy

Thanks.

Series looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Note we need to keep an eye out for this causing regressions with
some disks / firmware-versions.

Regards,

Hans




> 
> -current (non-rfc) series
> Implemented suggestions from Hans
> - no override of policy set via module param
> - Override lpm policy per host not global policy
> - changed the new policy name to "min_power_with_partial"
> 
> rfc-v2
> - As suggested by Hans, it is possible to have ASP with DEVSLP, so
> add a new state.
> - Removed usage of mem_sleep_current, instead just rely on low power
> idle flag. Don't feel good to EXPORT from core suspend code.
> - Depending host policy to decide if we enable DEVSLP by default.
> 
> Srinivas Pandruvada (2):
>    ata: ahci: Support state with min power but Partial low power state
>    ata: ahci: Enable DEVSLP by default on x86 with SLP_S0
> 
>   drivers/ata/ahci.c        | 38 +++++++++++++++++++++++++++++++++-----
>   drivers/ata/libahci.c     |  5 ++++-
>   drivers/ata/libata-core.c |  1 +
>   drivers/ata/libata-scsi.c |  1 +
>   include/linux/libata.h    |  3 ++-
>   5 files changed, 41 insertions(+), 7 deletions(-)
> 

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

* Re: [PATCH v2 0/2] ata: ahci: Enable DEVSLP by default on SLP_S0 support
  2018-07-27 20:47 [PATCH v2 0/2] ata: ahci: Enable DEVSLP by default on SLP_S0 support Srinivas Pandruvada
                   ` (2 preceding siblings ...)
  2018-07-29  9:25 ` [PATCH v2 0/2] ata: ahci: Enable DEVSLP by default on SLP_S0 support Hans de Goede
@ 2018-07-30 14:10 ` Tejun Heo
  3 siblings, 0 replies; 5+ messages in thread
From: Tejun Heo @ 2018-07-30 14:10 UTC (permalink / raw)
  To: Srinivas Pandruvada
  Cc: hdegoede, rjw, alan.cox, linux-ide, linux-kernel, linux-pm,
	Mario.Limonciello

On Fri, Jul 27, 2018 at 01:47:01PM -0700, Srinivas Pandruvada wrote:
> One of the requirement for modern x86 system to enter lowest power mode
> (SLP_S0) is SATA IP block to be off. This is true even during when
> platform is suspended to idle and not only in opportunistic (runtime)
> suspend.
> This series is to enable DEVSLP by default.

Applied to 1-2 to libata/for-4.19.

Thanks.

-- 
tejun

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

end of thread, other threads:[~2018-07-30 14:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-27 20:47 [PATCH v2 0/2] ata: ahci: Enable DEVSLP by default on SLP_S0 support Srinivas Pandruvada
2018-07-27 20:47 ` [PATCH v2 1/2] ata: ahci: Support state with min power but Partial low power state Srinivas Pandruvada
2018-07-27 20:47 ` [PATCH v2 2/2] ata: ahci: Enable DEVSLP by default on x86 with SLP_S0 Srinivas Pandruvada
2018-07-29  9:25 ` [PATCH v2 0/2] ata: ahci: Enable DEVSLP by default on SLP_S0 support Hans de Goede
2018-07-30 14:10 ` Tejun Heo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.