[PATCH 0/4][SRU][B][C][OEM-B]SATA device is not going to DEVSLP

classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/4][SRU][B][C][OEM-B]SATA device is not going to DEVSLP

AceLan Kao
BugLink: https://bugs.launchpad.net/bugs/1781533

[Impact]
Any of the platforms we’ve been seeing SATA problems not going to deepest
state leads to other devices not getting there during long idle or s2idle.
And it also prevents the system from entering deeper PC state other than
PC3.

[Fix]
Suggested from Intel and Dell to contains the following 4 commits,
and all of 4 commits are in v4.19-rc1
https://patchwork.kernel.org/patch/10502285/
https://patchwork.kernel.org/patch/10502287/
https://patchwork.kernel.org/patch/10535781/
https://patchwork.kernel.org/patch/10535783/

[Test]
Verified the power consumption on some new platforms, it improves the
SATA HDD power consumption around 0.5w during long idle.

[Regression Potential]
Low, the DEVSLP function is already validated when shipped with SLP_S0
support.

Srinivas Pandruvada (4):
  ata: ahci: Support state with min power but Partial low power state
  ata: ahci: Enable DEVSLP by default on x86 with SLP_S0
  ata: libahci: Correct setting of DEVSLP register
  ata: libahci: Allow reconfigure of DEVSLP register

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

--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/4][SRU][OEM-B] ata: libahci: Correct setting of DEVSLP register

AceLan Kao
From: Srinivas Pandruvada <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1781533

We have seen that on some platforms, SATA device never show any DEVSLP
residency. This prevent power gating of SATA IP, which prevent system
to transition to low power mode in systems with SLP_S0 aka modern
standby systems. The PHY logic is off only in DEVSLP not in slumber.
Reference:
https://www.intel.com/content/dam/www/public/us/en/documents/datasheets
/332995-skylake-i-o-platform-datasheet-volume-1.pdf
Section 28.7.6.1

Here driver is trying to do read-modify-write the devslp register. But
not resetting the bits for which this driver will modify values (DITO,
MDAT and DETO). So simply reset those bits before updating to new values.

Signed-off-by: Srinivas Pandruvada <[hidden email]>
Reviewed-by: Rafael J. Wysocki <[hidden email]>
Reviewed-by: Hans de Goede <[hidden email]>
Signed-off-by: Tejun Heo <[hidden email]>
(cherry picked from commit 2dbb3ec29a6c069035857a2fc4c24e80e5dfe3cc)
Signed-off-by: AceLan Kao <[hidden email]>
---
 drivers/ata/libahci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index edaba13edeb9..0a2938e4c834 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -2149,6 +2149,8 @@ static void ahci_set_aggressive_devslp(struct ata_port *ap, bool sleep)
  deto = 20;
  }
 
+ /* Make dito, mdat, deto bits to 0s */
+ devslp &= ~GENMASK_ULL(24, 2);
  devslp |= ((dito << PORT_DEVSLP_DITO_OFFSET) |
    (mdat << PORT_DEVSLP_MDAT_OFFSET) |
    (deto << PORT_DEVSLP_DETO_OFFSET) |
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/4][SRU][OEM-B] ata: libahci: Allow reconfigure of DEVSLP register

AceLan Kao
In reply to this post by AceLan Kao
From: Srinivas Pandruvada <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1781533

There are two modes in which DEVSLP can be entered. The OS initiated or
hardware autonomous.

In hardware autonomous mode, BIOS configures the AHCI controller and the
device to enable DEVSLP. But they may not be ideal for all cases. So in
this case, OS should be able to reconfigure DEVSLP register.

Currently if the DEVSLP is already enabled, we can't set again as it will
simply return. There are some systems where the firmware is setting high
DITO by default, in this case we can't modify here to correct settings.
With the default in several seconds, we are not able to transition to
DEVSLP.

This change will allow reconfiguration of devslp register if DITO is
different.

Signed-off-by: Srinivas Pandruvada <[hidden email]>
Reviewed-by: Hans de Goede <[hidden email]>
Signed-off-by: Tejun Heo <[hidden email]>
(cherry picked from commit 11c291461b6ea8d1195a96d6bba6673a94aacebc)
Signed-off-by: AceLan Kao <[hidden email]>
---
 drivers/ata/libahci.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 0a2938e4c834..a1342fa12859 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -2094,7 +2094,7 @@ static void ahci_set_aggressive_devslp(struct ata_port *ap, bool sleep)
  struct ahci_host_priv *hpriv = ap->host->private_data;
  void __iomem *port_mmio = ahci_port_base(ap);
  struct ata_device *dev = ap->link.device;
- u32 devslp, dm, dito, mdat, deto;
+ u32 devslp, dm, dito, mdat, deto, dito_conf;
  int rc;
  unsigned int err_mask;
 
@@ -2118,8 +2118,15 @@ static void ahci_set_aggressive_devslp(struct ata_port *ap, bool sleep)
  return;
  }
 
- /* device sleep was already enabled */
- if (devslp & PORT_DEVSLP_ADSE)
+ dm = (devslp & PORT_DEVSLP_DM_MASK) >> PORT_DEVSLP_DM_OFFSET;
+ dito = devslp_idle_timeout / (dm + 1);
+ if (dito > 0x3ff)
+ dito = 0x3ff;
+
+ dito_conf = (devslp >> PORT_DEVSLP_DITO_OFFSET) & 0x3FF;
+
+ /* device sleep was already enabled and same dito */
+ if ((devslp & PORT_DEVSLP_ADSE) && (dito_conf == dito))
  return;
 
  /* set DITO, MDAT, DETO and enable DevSlp, need to stop engine first */
@@ -2127,11 +2134,6 @@ static void ahci_set_aggressive_devslp(struct ata_port *ap, bool sleep)
  if (rc)
  return;
 
- dm = (devslp & PORT_DEVSLP_DM_MASK) >> PORT_DEVSLP_DM_OFFSET;
- dito = devslp_idle_timeout / (dm + 1);
- if (dito > 0x3ff)
- dito = 0x3ff;
-
  /* Use the nominal value 10 ms if the read MDAT is zero,
  * the nominal value of DETO is 20 ms.
  */
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 3/4][SRU][OEM-B] ata: ahci: Support state with min power but Partial low power state

AceLan Kao
In reply to this post by AceLan Kao
From: Srinivas Pandruvada <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1781533

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-and-reviewed-by: Hans de Goede <[hidden email]>
Signed-off-by: Srinivas Pandruvada <[hidden email]>
Signed-off-by: Tejun Heo <[hidden email]>
(cherry picked from commit a5ec5a7bfd1f28d1905499641c9f589be36808c1)
Signed-off-by: AceLan Kao <[hidden email]>
---
 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 a1342fa12859..c5ef90eebbc9 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -790,6 +790,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);
@@ -800,7 +802,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 d8d45072e4ad..0d005563c923 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3974,6 +3974,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 89a9d4a2efc8..fdfad63cca23 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 ed9826b21c5e..c72849ef1866 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -523,7 +523,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


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 4/4][SRU][OEM-B] ata: ahci: Enable DEVSLP by default on x86 with SLP_S0

AceLan Kao
In reply to this post by AceLan Kao
From: Srinivas Pandruvada <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1781533

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 <[hidden email]>
Reviewed-by: Hans de Goede <[hidden email]>
Signed-off-by: Tejun Heo <[hidden email]>
(cherry picked from commit b1a9585cc396cac5a9e5a09b2721f3b8568e62d0)
Signed-off-by: AceLan Kao <[hidden email]>
---
 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 6389c88b3500..8e2d099caed3 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -608,7 +608,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");
 
@@ -1549,6 +1549,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;
@@ -1746,10 +1777,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


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/5][SRU][B] ahci: Allow setting a default LPM policy for mobile chipsets

AceLan Kao
In reply to this post by AceLan Kao
From: Hans de Goede <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1781533

On many laptops setting a different LPM policy then unknown /
max_performance can lead to power-savings of 1.0 - 1.5 Watts (when idle).

Modern ultrabooks idle around 6W (at 50% screen brightness), 1.0 - 1.5W
is a significant chunk of this.

There are some performance / latency costs to enabling LPM by default,
so it is desirable to make it possible to set a different LPM policy
for mobile / laptop variants of chipsets / "South Bridges" vs their
desktop / server counterparts. Also enabling LPM by default is not
entirely without risk of regressions. At least min_power is known to
cause issues with some disks, including some reports of data corruption.

This commits adds a new ahci.mobile_lpm_policy kernel cmdline option,
which defaults to a new SATA_MOBILE_LPM_POLICY Kconfig option so that
Linux distributions can choose to set a LPM policy for mobile chipsets
by default.

The reason to have both a kernel cmdline option and a Kconfig default
value for it, is to allow easy overriding of the default to allow
trouble-shooting without needing to rebuild the kernel.

Signed-off-by: Hans de Goede <[hidden email]>
Signed-off-by: Tejun Heo <[hidden email]>
(backported from commit ebb82e3c79d2a956366d0848304a53648bd6350b)
Signed-off-by: AceLan Kao <[hidden email]>
---
 drivers/ata/Kconfig | 19 +++++++++
 drivers/ata/ahci.c  | 97 ++++++++++++++++++++++++++-------------------
 drivers/ata/ahci.h  |  3 ++
 3 files changed, 78 insertions(+), 41 deletions(-)

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index cb5339166563..b3fad5663aeb 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -92,6 +92,25 @@ config SATA_AHCI
 
   If unsure, say N.
 
+config SATA_MOBILE_LPM_POLICY
+ int "Default SATA Link Power Management policy for mobile chipsets"
+ range 0 4
+ default 0
+ depends on SATA_AHCI
+ help
+  Select the Default SATA Link Power Management (LPM) policy to use
+  for mobile / laptop variants of chipsets / "South Bridges".
+
+  The value set has the following meanings:
+ 0 => Keep firmware settings
+ 1 => Maximum performance
+ 2 => Medium power
+ 3 => Medium power with Device Initiated PM enabled
+ 4 => Minimum power
+
+  Note "Minimum power" is known to cause issues, including disk
+  corruption, with some disks and should not be used.
+
 config SATA_AHCI_PLATFORM
  tristate "Platform AHCI SATA support"
  help
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index e3e69f5ec157..6389c88b3500 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -65,6 +65,7 @@ enum board_ids {
  /* board IDs by feature in alphabetical order */
  board_ahci,
  board_ahci_ign_iferr,
+ board_ahci_mobile,
  board_ahci_nomsi,
  board_ahci_noncq,
  board_ahci_nosntf,
@@ -140,6 +141,13 @@ static const struct ata_port_info ahci_port_info[] = {
  .udma_mask = ATA_UDMA6,
  .port_ops = &ahci_ops,
  },
+ [board_ahci_mobile] = {
+ AHCI_HFLAGS (AHCI_HFLAG_IS_MOBILE),
+ .flags = AHCI_FLAG_COMMON,
+ .pio_mask = ATA_PIO4,
+ .udma_mask = ATA_UDMA6,
+ .port_ops = &ahci_ops,
+ },
  [board_ahci_nomsi] = {
  AHCI_HFLAGS (AHCI_HFLAG_NO_MSI),
  .flags = AHCI_FLAG_COMMON,
@@ -252,13 +260,13 @@ static const struct pci_device_id ahci_pci_tbl[] = {
  { PCI_VDEVICE(INTEL, 0x2924), board_ahci }, /* ICH9 */
  { PCI_VDEVICE(INTEL, 0x2925), board_ahci }, /* ICH9 */
  { PCI_VDEVICE(INTEL, 0x2927), board_ahci }, /* ICH9 */
- { PCI_VDEVICE(INTEL, 0x2929), board_ahci }, /* ICH9M */
- { PCI_VDEVICE(INTEL, 0x292a), board_ahci }, /* ICH9M */
- { PCI_VDEVICE(INTEL, 0x292b), board_ahci }, /* ICH9M */
- { PCI_VDEVICE(INTEL, 0x292c), board_ahci }, /* ICH9M */
- { PCI_VDEVICE(INTEL, 0x292f), board_ahci }, /* ICH9M */
+ { PCI_VDEVICE(INTEL, 0x2929), board_ahci_mobile }, /* ICH9M */
+ { PCI_VDEVICE(INTEL, 0x292a), board_ahci_mobile }, /* ICH9M */
+ { PCI_VDEVICE(INTEL, 0x292b), board_ahci_mobile }, /* ICH9M */
+ { PCI_VDEVICE(INTEL, 0x292c), board_ahci_mobile }, /* ICH9M */
+ { PCI_VDEVICE(INTEL, 0x292f), board_ahci_mobile }, /* ICH9M */
  { PCI_VDEVICE(INTEL, 0x294d), board_ahci }, /* ICH9 */
- { PCI_VDEVICE(INTEL, 0x294e), board_ahci }, /* ICH9M */
+ { PCI_VDEVICE(INTEL, 0x294e), board_ahci_mobile }, /* ICH9M */
  { PCI_VDEVICE(INTEL, 0x502a), board_ahci }, /* Tolapai */
  { PCI_VDEVICE(INTEL, 0x502b), board_ahci }, /* Tolapai */
  { PCI_VDEVICE(INTEL, 0x3a05), board_ahci }, /* ICH10 */
@@ -268,9 +276,9 @@ static const struct pci_device_id ahci_pci_tbl[] = {
  { PCI_VDEVICE(INTEL, 0x3b23), board_ahci }, /* PCH AHCI */
  { PCI_VDEVICE(INTEL, 0x3b24), board_ahci }, /* PCH RAID */
  { PCI_VDEVICE(INTEL, 0x3b25), board_ahci }, /* PCH RAID */
- { PCI_VDEVICE(INTEL, 0x3b29), board_ahci }, /* PCH M AHCI */
+ { PCI_VDEVICE(INTEL, 0x3b29), board_ahci_mobile }, /* PCH M AHCI */
  { PCI_VDEVICE(INTEL, 0x3b2b), board_ahci }, /* PCH RAID */
- { PCI_VDEVICE(INTEL, 0x3b2c), board_ahci }, /* PCH M RAID */
+ { PCI_VDEVICE(INTEL, 0x3b2c), board_ahci_mobile }, /* PCH M RAID */
  { PCI_VDEVICE(INTEL, 0x3b2f), board_ahci }, /* PCH AHCI */
  { PCI_VDEVICE(INTEL, 0x19b0), board_ahci }, /* DNV AHCI */
  { PCI_VDEVICE(INTEL, 0x19b1), board_ahci }, /* DNV AHCI */
@@ -293,9 +301,9 @@ static const struct pci_device_id ahci_pci_tbl[] = {
  { PCI_VDEVICE(INTEL, 0x19cE), board_ahci }, /* DNV AHCI */
  { PCI_VDEVICE(INTEL, 0x19cF), board_ahci }, /* DNV AHCI */
  { PCI_VDEVICE(INTEL, 0x1c02), board_ahci }, /* CPT AHCI */
- { PCI_VDEVICE(INTEL, 0x1c03), board_ahci }, /* CPT M AHCI */
+ { PCI_VDEVICE(INTEL, 0x1c03), board_ahci_mobile }, /* CPT M AHCI */
  { PCI_VDEVICE(INTEL, 0x1c04), board_ahci }, /* CPT RAID */
- { PCI_VDEVICE(INTEL, 0x1c05), board_ahci }, /* CPT M RAID */
+ { PCI_VDEVICE(INTEL, 0x1c05), board_ahci_mobile }, /* CPT M RAID */
  { PCI_VDEVICE(INTEL, 0x1c06), board_ahci }, /* CPT RAID */
  { PCI_VDEVICE(INTEL, 0x1c07), board_ahci }, /* CPT RAID */
  { PCI_VDEVICE(INTEL, 0x1d02), board_ahci }, /* PBG AHCI */
@@ -304,28 +312,28 @@ static const struct pci_device_id ahci_pci_tbl[] = {
  { PCI_VDEVICE(INTEL, 0x2826), board_ahci }, /* PBG RAID */
  { PCI_VDEVICE(INTEL, 0x2323), board_ahci }, /* DH89xxCC AHCI */
  { PCI_VDEVICE(INTEL, 0x1e02), board_ahci }, /* Panther Point AHCI */
- { PCI_VDEVICE(INTEL, 0x1e03), board_ahci }, /* Panther Point M AHCI */
+ { PCI_VDEVICE(INTEL, 0x1e03), board_ahci_mobile }, /* Panther M AHCI */
  { PCI_VDEVICE(INTEL, 0x1e04), board_ahci }, /* Panther Point RAID */
  { PCI_VDEVICE(INTEL, 0x1e05), board_ahci }, /* Panther Point RAID */
  { PCI_VDEVICE(INTEL, 0x1e06), board_ahci }, /* Panther Point RAID */
- { PCI_VDEVICE(INTEL, 0x1e07), board_ahci }, /* Panther Point M RAID */
+ { PCI_VDEVICE(INTEL, 0x1e07), board_ahci_mobile }, /* Panther M RAID */
  { PCI_VDEVICE(INTEL, 0x1e0e), board_ahci }, /* Panther Point RAID */
  { PCI_VDEVICE(INTEL, 0x8c02), board_ahci }, /* Lynx Point AHCI */
- { PCI_VDEVICE(INTEL, 0x8c03), board_ahci }, /* Lynx Point M AHCI */
+ { PCI_VDEVICE(INTEL, 0x8c03), board_ahci_mobile }, /* Lynx M AHCI */
  { PCI_VDEVICE(INTEL, 0x8c04), board_ahci }, /* Lynx Point RAID */
- { PCI_VDEVICE(INTEL, 0x8c05), board_ahci }, /* Lynx Point M RAID */
+ { PCI_VDEVICE(INTEL, 0x8c05), board_ahci_mobile }, /* Lynx M RAID */
  { PCI_VDEVICE(INTEL, 0x8c06), board_ahci }, /* Lynx Point RAID */
- { PCI_VDEVICE(INTEL, 0x8c07), board_ahci }, /* Lynx Point M RAID */
+ { PCI_VDEVICE(INTEL, 0x8c07), board_ahci_mobile }, /* Lynx M RAID */
  { PCI_VDEVICE(INTEL, 0x8c0e), board_ahci }, /* Lynx Point RAID */
- { PCI_VDEVICE(INTEL, 0x8c0f), board_ahci }, /* Lynx Point M RAID */
- { PCI_VDEVICE(INTEL, 0x9c02), board_ahci }, /* Lynx Point-LP AHCI */
- { PCI_VDEVICE(INTEL, 0x9c03), board_ahci }, /* Lynx Point-LP AHCI */
- { PCI_VDEVICE(INTEL, 0x9c04), board_ahci }, /* Lynx Point-LP RAID */
- { PCI_VDEVICE(INTEL, 0x9c05), board_ahci }, /* Lynx Point-LP RAID */
- { PCI_VDEVICE(INTEL, 0x9c06), board_ahci }, /* Lynx Point-LP RAID */
- { PCI_VDEVICE(INTEL, 0x9c07), board_ahci }, /* Lynx Point-LP RAID */
- { PCI_VDEVICE(INTEL, 0x9c0e), board_ahci }, /* Lynx Point-LP RAID */
- { PCI_VDEVICE(INTEL, 0x9c0f), board_ahci }, /* Lynx Point-LP RAID */
+ { PCI_VDEVICE(INTEL, 0x8c0f), board_ahci_mobile }, /* Lynx M RAID */
+ { PCI_VDEVICE(INTEL, 0x9c02), board_ahci_mobile }, /* Lynx LP AHCI */
+ { PCI_VDEVICE(INTEL, 0x9c03), board_ahci_mobile }, /* Lynx LP AHCI */
+ { PCI_VDEVICE(INTEL, 0x9c04), board_ahci_mobile }, /* Lynx LP RAID */
+ { PCI_VDEVICE(INTEL, 0x9c05), board_ahci_mobile }, /* Lynx LP RAID */
+ { PCI_VDEVICE(INTEL, 0x9c06), board_ahci_mobile }, /* Lynx LP RAID */
+ { PCI_VDEVICE(INTEL, 0x9c07), board_ahci_mobile }, /* Lynx LP RAID */
+ { PCI_VDEVICE(INTEL, 0x9c0e), board_ahci_mobile }, /* Lynx LP RAID */
+ { PCI_VDEVICE(INTEL, 0x9c0f), board_ahci_mobile }, /* Lynx LP RAID */
  { PCI_VDEVICE(INTEL, 0x1f22), board_ahci }, /* Avoton AHCI */
  { PCI_VDEVICE(INTEL, 0x1f23), board_ahci }, /* Avoton AHCI */
  { PCI_VDEVICE(INTEL, 0x1f24), board_ahci }, /* Avoton RAID */
@@ -353,26 +361,26 @@ static const struct pci_device_id ahci_pci_tbl[] = {
  { PCI_VDEVICE(INTEL, 0x8d66), board_ahci }, /* Wellsburg RAID */
  { PCI_VDEVICE(INTEL, 0x8d6e), board_ahci }, /* Wellsburg RAID */
  { PCI_VDEVICE(INTEL, 0x23a3), board_ahci }, /* Coleto Creek AHCI */
- { PCI_VDEVICE(INTEL, 0x9c83), board_ahci }, /* Wildcat Point-LP AHCI */
- { PCI_VDEVICE(INTEL, 0x9c85), board_ahci }, /* Wildcat Point-LP RAID */
- { PCI_VDEVICE(INTEL, 0x9c87), board_ahci }, /* Wildcat Point-LP RAID */
- { PCI_VDEVICE(INTEL, 0x9c8f), board_ahci }, /* Wildcat Point-LP RAID */
+ { PCI_VDEVICE(INTEL, 0x9c83), board_ahci_mobile }, /* Wildcat LP AHCI */
+ { PCI_VDEVICE(INTEL, 0x9c85), board_ahci_mobile }, /* Wildcat LP RAID */
+ { PCI_VDEVICE(INTEL, 0x9c87), board_ahci_mobile }, /* Wildcat LP RAID */
+ { PCI_VDEVICE(INTEL, 0x9c8f), board_ahci_mobile }, /* Wildcat LP RAID */
  { PCI_VDEVICE(INTEL, 0x8c82), board_ahci }, /* 9 Series AHCI */
- { PCI_VDEVICE(INTEL, 0x8c83), board_ahci }, /* 9 Series M AHCI */
+ { PCI_VDEVICE(INTEL, 0x8c83), board_ahci_mobile }, /* 9 Series M AHCI */
  { PCI_VDEVICE(INTEL, 0x8c84), board_ahci }, /* 9 Series RAID */
- { PCI_VDEVICE(INTEL, 0x8c85), board_ahci }, /* 9 Series M RAID */
+ { PCI_VDEVICE(INTEL, 0x8c85), board_ahci_mobile }, /* 9 Series M RAID */
  { PCI_VDEVICE(INTEL, 0x8c86), board_ahci }, /* 9 Series RAID */
- { PCI_VDEVICE(INTEL, 0x8c87), board_ahci }, /* 9 Series M RAID */
+ { PCI_VDEVICE(INTEL, 0x8c87), board_ahci_mobile }, /* 9 Series M RAID */
  { PCI_VDEVICE(INTEL, 0x8c8e), board_ahci }, /* 9 Series RAID */
- { PCI_VDEVICE(INTEL, 0x8c8f), board_ahci }, /* 9 Series M RAID */
- { PCI_VDEVICE(INTEL, 0x9d03), board_ahci }, /* Sunrise Point-LP AHCI */
- { PCI_VDEVICE(INTEL, 0x9d05), board_ahci }, /* Sunrise Point-LP RAID */
- { PCI_VDEVICE(INTEL, 0x9d07), board_ahci }, /* Sunrise Point-LP RAID */
+ { PCI_VDEVICE(INTEL, 0x8c8f), board_ahci_mobile }, /* 9 Series M RAID */
+ { PCI_VDEVICE(INTEL, 0x9d03), board_ahci_mobile }, /* Sunrise LP AHCI */
+ { PCI_VDEVICE(INTEL, 0x9d05), board_ahci_mobile }, /* Sunrise LP RAID */
+ { PCI_VDEVICE(INTEL, 0x9d07), board_ahci_mobile }, /* Sunrise LP RAID */
  { PCI_VDEVICE(INTEL, 0xa102), board_ahci }, /* Sunrise Point-H AHCI */
- { PCI_VDEVICE(INTEL, 0xa103), board_ahci }, /* Sunrise Point-H M AHCI */
+ { PCI_VDEVICE(INTEL, 0xa103), board_ahci_mobile }, /* Sunrise M AHCI */
  { PCI_VDEVICE(INTEL, 0xa105), board_ahci }, /* Sunrise Point-H RAID */
  { PCI_VDEVICE(INTEL, 0xa106), board_ahci }, /* Sunrise Point-H RAID */
- { PCI_VDEVICE(INTEL, 0xa107), board_ahci }, /* Sunrise Point-H M RAID */
+ { PCI_VDEVICE(INTEL, 0xa107), board_ahci_mobile }, /* Sunrise M RAID */
  { PCI_VDEVICE(INTEL, 0xa10f), board_ahci }, /* Sunrise Point-H RAID */
  { PCI_VDEVICE(INTEL, 0x2822), board_ahci }, /* Lewisburg RAID*/
  { PCI_VDEVICE(INTEL, 0x2823), board_ahci }, /* Lewisburg AHCI*/
@@ -387,10 +395,10 @@ static const struct pci_device_id ahci_pci_tbl[] = {
  { PCI_VDEVICE(INTEL, 0xa252), board_ahci }, /* Lewisburg RAID*/
  { PCI_VDEVICE(INTEL, 0xa256), board_ahci }, /* Lewisburg RAID*/
  { PCI_VDEVICE(INTEL, 0xa356), board_ahci }, /* Cannon Lake PCH-H RAID */
- { PCI_VDEVICE(INTEL, 0x0f22), board_ahci }, /* Bay Trail AHCI */
- { PCI_VDEVICE(INTEL, 0x0f23), board_ahci }, /* Bay Trail AHCI */
- { PCI_VDEVICE(INTEL, 0x22a3), board_ahci }, /* Cherry Trail AHCI */
- { PCI_VDEVICE(INTEL, 0x5ae3), board_ahci }, /* Apollo Lake AHCI */
+ { PCI_VDEVICE(INTEL, 0x0f22), board_ahci_mobile }, /* Bay Trail AHCI */
+ { PCI_VDEVICE(INTEL, 0x0f23), board_ahci_mobile }, /* Bay Trail AHCI */
+ { PCI_VDEVICE(INTEL, 0x22a3), board_ahci_mobile }, /* Cherry Tr. AHCI */
+ { PCI_VDEVICE(INTEL, 0x5ae3), board_ahci_mobile }, /* ApolloLake AHCI */
 
  /* JMicron 360/1/3/5/6, match class to avoid IDE function */
  { PCI_VENDOR_ID_JMICRON, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
@@ -600,6 +608,9 @@ 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;
+module_param(mobile_lpm_policy, int, 0644);
+MODULE_PARM_DESC(mobile_lpm_policy, "Default LPM policy for mobile chipsets");
 
 static void ahci_pci_save_initial_config(struct pci_dev *pdev,
  struct ahci_host_priv *hpriv)
@@ -1735,6 +1746,10 @@ 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;
 
  /* disabled/not-implemented port */
  if (!(hpriv->port_map & (1 << i)))
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 9cc196b4474a..824bd399f02e 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -251,6 +251,9 @@ enum {
  AHCI_HFLAG_YES_ALPM = (1 << 23), /* force ALPM cap on */
  AHCI_HFLAG_NO_WRITE_TO_RO = (1 << 24), /* don't write to read
  only registers */
+ AHCI_HFLAG_IS_MOBILE = (1 << 25), /* mobile chipset, use
+ SATA_MOBILE_LPM_POLICY
+ as default lpm_policy */
 
  /* ap->flags bits */
 
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/5][SRU][B] ata: libahci: Correct setting of DEVSLP register

AceLan Kao
In reply to this post by AceLan Kao
From: Srinivas Pandruvada <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1781533

We have seen that on some platforms, SATA device never show any DEVSLP
residency. This prevent power gating of SATA IP, which prevent system
to transition to low power mode in systems with SLP_S0 aka modern
standby systems. The PHY logic is off only in DEVSLP not in slumber.
Reference:
https://www.intel.com/content/dam/www/public/us/en/documents/datasheets
/332995-skylake-i-o-platform-datasheet-volume-1.pdf
Section 28.7.6.1

Here driver is trying to do read-modify-write the devslp register. But
not resetting the bits for which this driver will modify values (DITO,
MDAT and DETO). So simply reset those bits before updating to new values.

Signed-off-by: Srinivas Pandruvada <[hidden email]>
Reviewed-by: Rafael J. Wysocki <[hidden email]>
Reviewed-by: Hans de Goede <[hidden email]>
Signed-off-by: Tejun Heo <[hidden email]>
(cherry picked from commit 2dbb3ec29a6c069035857a2fc4c24e80e5dfe3cc)
Signed-off-by: AceLan Kao <[hidden email]>
---
 drivers/ata/libahci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index edaba13edeb9..0a2938e4c834 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -2149,6 +2149,8 @@ static void ahci_set_aggressive_devslp(struct ata_port *ap, bool sleep)
  deto = 20;
  }
 
+ /* Make dito, mdat, deto bits to 0s */
+ devslp &= ~GENMASK_ULL(24, 2);
  devslp |= ((dito << PORT_DEVSLP_DITO_OFFSET) |
    (mdat << PORT_DEVSLP_MDAT_OFFSET) |
    (deto << PORT_DEVSLP_DETO_OFFSET) |
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 3/5][SRU][B] ata: libahci: Allow reconfigure of DEVSLP register

AceLan Kao
In reply to this post by AceLan Kao
From: Srinivas Pandruvada <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1781533

There are two modes in which DEVSLP can be entered. The OS initiated or
hardware autonomous.

In hardware autonomous mode, BIOS configures the AHCI controller and the
device to enable DEVSLP. But they may not be ideal for all cases. So in
this case, OS should be able to reconfigure DEVSLP register.

Currently if the DEVSLP is already enabled, we can't set again as it will
simply return. There are some systems where the firmware is setting high
DITO by default, in this case we can't modify here to correct settings.
With the default in several seconds, we are not able to transition to
DEVSLP.

This change will allow reconfiguration of devslp register if DITO is
different.

Signed-off-by: Srinivas Pandruvada <[hidden email]>
Reviewed-by: Hans de Goede <[hidden email]>
Signed-off-by: Tejun Heo <[hidden email]>
(cherry picked from commit 11c291461b6ea8d1195a96d6bba6673a94aacebc)
Signed-off-by: AceLan Kao <[hidden email]>
---
 drivers/ata/libahci.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 0a2938e4c834..a1342fa12859 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -2094,7 +2094,7 @@ static void ahci_set_aggressive_devslp(struct ata_port *ap, bool sleep)
  struct ahci_host_priv *hpriv = ap->host->private_data;
  void __iomem *port_mmio = ahci_port_base(ap);
  struct ata_device *dev = ap->link.device;
- u32 devslp, dm, dito, mdat, deto;
+ u32 devslp, dm, dito, mdat, deto, dito_conf;
  int rc;
  unsigned int err_mask;
 
@@ -2118,8 +2118,15 @@ static void ahci_set_aggressive_devslp(struct ata_port *ap, bool sleep)
  return;
  }
 
- /* device sleep was already enabled */
- if (devslp & PORT_DEVSLP_ADSE)
+ dm = (devslp & PORT_DEVSLP_DM_MASK) >> PORT_DEVSLP_DM_OFFSET;
+ dito = devslp_idle_timeout / (dm + 1);
+ if (dito > 0x3ff)
+ dito = 0x3ff;
+
+ dito_conf = (devslp >> PORT_DEVSLP_DITO_OFFSET) & 0x3FF;
+
+ /* device sleep was already enabled and same dito */
+ if ((devslp & PORT_DEVSLP_ADSE) && (dito_conf == dito))
  return;
 
  /* set DITO, MDAT, DETO and enable DevSlp, need to stop engine first */
@@ -2127,11 +2134,6 @@ static void ahci_set_aggressive_devslp(struct ata_port *ap, bool sleep)
  if (rc)
  return;
 
- dm = (devslp & PORT_DEVSLP_DM_MASK) >> PORT_DEVSLP_DM_OFFSET;
- dito = devslp_idle_timeout / (dm + 1);
- if (dito > 0x3ff)
- dito = 0x3ff;
-
  /* Use the nominal value 10 ms if the read MDAT is zero,
  * the nominal value of DETO is 20 ms.
  */
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 4/5][SRU][B] ata: ahci: Support state with min power but Partial low power state

AceLan Kao
In reply to this post by AceLan Kao
From: Srinivas Pandruvada <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1781533

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-and-reviewed-by: Hans de Goede <[hidden email]>
Signed-off-by: Srinivas Pandruvada <[hidden email]>
Signed-off-by: Tejun Heo <[hidden email]>
(cherry picked from commit a5ec5a7bfd1f28d1905499641c9f589be36808c1)
Signed-off-by: AceLan Kao <[hidden email]>
---
 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 a1342fa12859..c5ef90eebbc9 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -790,6 +790,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);
@@ -800,7 +802,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 d8d45072e4ad..0d005563c923 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3974,6 +3974,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 89a9d4a2efc8..fdfad63cca23 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 ed9826b21c5e..c72849ef1866 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -523,7 +523,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


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 5/5][SRU][B] ata: ahci: Enable DEVSLP by default on x86 with SLP_S0

AceLan Kao
In reply to this post by AceLan Kao
From: Srinivas Pandruvada <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1781533

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 <[hidden email]>
Reviewed-by: Hans de Goede <[hidden email]>
Signed-off-by: Tejun Heo <[hidden email]>
(cherry picked from commit b1a9585cc396cac5a9e5a09b2721f3b8568e62d0)
Signed-off-by: AceLan Kao <[hidden email]>
---
 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 6389c88b3500..8e2d099caed3 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -608,7 +608,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");
 
@@ -1549,6 +1549,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;
@@ -1746,10 +1777,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


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2][SRU][C] ata: ahci: Support state with min power but Partial low power state

AceLan Kao
In reply to this post by AceLan Kao
From: Srinivas Pandruvada <[hidden email]>

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-and-reviewed-by: Hans de Goede <[hidden email]>
Signed-off-by: Srinivas Pandruvada <[hidden email]>
Signed-off-by: Tejun Heo <[hidden email]>
(cherry picked from commit a5ec5a7bfd1f28d1905499641c9f589be36808c1)
Signed-off-by: AceLan Kao <[hidden email]>
---
 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 704a761f94b2..11a1da0079bd 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -801,6 +801,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);
@@ -811,7 +813,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 22a2bc5f25ce..27fa6b7d8028 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3970,6 +3970,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 aad1b01447de..a396234ead47 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 bc4f87cbe7f4..3c2aa9bf37b1 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -523,7 +523,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


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2][SRU][C] ata: ahci: Enable DEVSLP by default on x86 with SLP_S0

AceLan Kao
In reply to this post by AceLan Kao
From: Srinivas Pandruvada <[hidden email]>

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 <[hidden email]>
Reviewed-by: Hans de Goede <[hidden email]>
Signed-off-by: Tejun Heo <[hidden email]>
(cherry picked from commit b1a9585cc396cac5a9e5a09b2721f3b8568e62d0)
Signed-off-by: AceLan Kao <[hidden email]>
---
 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 b2b9eba1d214..021ce46e2e57 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -610,7 +610,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");
 
@@ -1604,6 +1604,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;
@@ -1807,10 +1838,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


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2][RESEND][SRU][C] ata: ahci: Support state with min power but Partial low power state

AceLan Kao
In reply to this post by AceLan Kao
From: Srinivas Pandruvada <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1781533

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-and-reviewed-by: Hans de Goede <[hidden email]>
Signed-off-by: Srinivas Pandruvada <[hidden email]>
Signed-off-by: Tejun Heo <[hidden email]>
(cherry picked from commit a5ec5a7bfd1f28d1905499641c9f589be36808c1)
Signed-off-by: AceLan Kao <[hidden email]>
---
 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 704a761f94b2..11a1da0079bd 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -801,6 +801,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);
@@ -811,7 +813,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 22a2bc5f25ce..27fa6b7d8028 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3970,6 +3970,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 aad1b01447de..a396234ead47 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 bc4f87cbe7f4..3c2aa9bf37b1 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -523,7 +523,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


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2][RESEND][SRU][C] ata: ahci: Enable DEVSLP by default on x86 with SLP_S0

AceLan Kao
From: Srinivas Pandruvada <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1781533

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 <[hidden email]>
Reviewed-by: Hans de Goede <[hidden email]>
Signed-off-by: Tejun Heo <[hidden email]>
(cherry picked from commit b1a9585cc396cac5a9e5a09b2721f3b8568e62d0)
Signed-off-by: AceLan Kao <[hidden email]>
---
 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 b2b9eba1d214..021ce46e2e57 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -610,7 +610,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");
 
@@ -1604,6 +1604,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;
@@ -1807,10 +1838,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


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/4][SRU][B][C][OEM-B]SATA device is not going to DEVSLP

Kai-Heng Feng
In reply to this post by AceLan Kao


> On Nov 28, 2018, at 5:19 PM, AceLan Kao <[hidden email]> wrote:
>
> BugLink: https://bugs.launchpad.net/bugs/1781533
>
> [Impact]
> Any of the platforms we’ve been seeing SATA problems not going to deepest
> state leads to other devices not getting there during long idle or s2idle.
> And it also prevents the system from entering deeper PC state other than
> PC3.
>
> [Fix]
> Suggested from Intel and Dell to contains the following 4 commits,
> and all of 4 commits are in v4.19-rc1
> https://patchwork.kernel.org/patch/10502285/
> https://patchwork.kernel.org/patch/10502287/
> https://patchwork.kernel.org/patch/10535781/
> https://patchwork.kernel.org/patch/10535783/
>
> [Test]
> Verified the power consumption on some new platforms, it improves the
> SATA HDD power consumption around 0.5w during long idle.
>
> [Regression Potential]
> Low, the DEVSLP function is already validated when shipped with SLP_S0
> support.
>
> Srinivas Pandruvada (4):
>  ata: ahci: Support state with min power but Partial low power state
>  ata: ahci: Enable DEVSLP by default on x86 with SLP_S0
>  ata: libahci: Correct setting of DEVSLP register
>  ata: libahci: Allow reconfigure of DEVSLP register
>
> drivers/ata/ahci.c        | 38 +++++++++++++++++++++++++++++++++-----
> drivers/ata/libahci.c     | 25 ++++++++++++++++---------
> drivers/ata/libata-core.c |  1 +
> drivers/ata/libata-scsi.c |  1 +
> include/linux/libata.h    |  3 ++-
> 5 files changed, 53 insertions(+), 15 deletions(-)

Acked-by: Kai-Heng Feng <[hidden email]>

>
> --
> 2.17.1
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACKed: [PATCH 0/4][SRU][B][C][OEM-B]SATA device is not going to DEVSLP

Aaron Ma
In reply to this post by AceLan Kao
This series is:
Acked-by: Aaron Ma <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED[OEM-B]: [PATCH 0/4][SRU][B][C][OEM-B]SATA device is not going to DEVSLP

AceLan Kao
In reply to this post by AceLan Kao
Applied on oem kernel 4.15.0-1029.34

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/Cmnt: [PATCH 0/4][SRU][B][C][OEM-B]SATA device is not going to DEVSLP

Stefan Bader-2
In reply to this post by AceLan Kao
On 28.11.18 10:19, AceLan Kao wrote:

> BugLink: https://bugs.launchpad.net/bugs/1781533
>
> [Impact]
> Any of the platforms we’ve been seeing SATA problems not going to deepest
> state leads to other devices not getting there during long idle or s2idle.
> And it also prevents the system from entering deeper PC state other than
> PC3.
>
> [Fix]
> Suggested from Intel and Dell to contains the following 4 commits,
> and all of 4 commits are in v4.19-rc1
> https://patchwork.kernel.org/patch/10502285/
> https://patchwork.kernel.org/patch/10502287/
> https://patchwork.kernel.org/patch/10535781/
> https://patchwork.kernel.org/patch/10535783/
>
> [Test]
> Verified the power consumption on some new platforms, it improves the
> SATA HDD power consumption around 0.5w during long idle.
>
> [Regression Potential]
> Low, the DEVSLP function is already validated when shipped with SLP_S0
> support.
>
> Srinivas Pandruvada (4):
>   ata: ahci: Support state with min power but Partial low power state
>   ata: ahci: Enable DEVSLP by default on x86 with SLP_S0
>   ata: libahci: Correct setting of DEVSLP register
>   ata: libahci: Allow reconfigure of DEVSLP register
>
>  drivers/ata/ahci.c        | 38 +++++++++++++++++++++++++++++++++-----
>  drivers/ata/libahci.c     | 25 ++++++++++++++++---------
>  drivers/ata/libata-core.c |  1 +
>  drivers/ata/libata-scsi.c |  1 +
>  include/linux/libata.h    |  3 ++-
>  5 files changed, 53 insertions(+), 15 deletions(-)
>
From looking at this I take it that the default from the new config is to leave
things as they were if not changed by command line. At least if SLP_S0 only
exists with newer hardware which than gets validated using the new support.

Just some notes for
a) probably needs an updateconfigs run and the new config default added to a
commit after the set has been applied in bionic.
b) some care when committing as some re-send was done as a re-ply to the same
thread for cosmic (might be easy to miss the way thunderbird shows that to me).

otherwise:

Acked-by: Stefan Bader <[hidden email]>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

APPLIED[B/C]: [PATCH 0/4][SRU][B][C][OEM-B]SATA device is not going to DEVSLP

Kleber Souza
In reply to this post by AceLan Kao
On 11/28/18 10:19 AM, AceLan Kao wrote:

> BugLink: https://bugs.launchpad.net/bugs/1781533
>
> [Impact]
> Any of the platforms we’ve been seeing SATA problems not going to deepest
> state leads to other devices not getting there during long idle or s2idle.
> And it also prevents the system from entering deeper PC state other than
> PC3.
>
> [Fix]
> Suggested from Intel and Dell to contains the following 4 commits,
> and all of 4 commits are in v4.19-rc1
> https://patchwork.kernel.org/patch/10502285/
> https://patchwork.kernel.org/patch/10502287/
> https://patchwork.kernel.org/patch/10535781/
> https://patchwork.kernel.org/patch/10535783/
>
> [Test]
> Verified the power consumption on some new platforms, it improves the
> SATA HDD power consumption around 0.5w during long idle.
>
> [Regression Potential]
> Low, the DEVSLP function is already validated when shipped with SLP_S0
> support.
>
> Srinivas Pandruvada (4):
>   ata: ahci: Support state with min power but Partial low power state
>   ata: ahci: Enable DEVSLP by default on x86 with SLP_S0
>   ata: libahci: Correct setting of DEVSLP register
>   ata: libahci: Allow reconfigure of DEVSLP register
>
>  drivers/ata/ahci.c        | 38 +++++++++++++++++++++++++++++++++-----
>  drivers/ata/libahci.c     | 25 ++++++++++++++++---------
>  drivers/ata/libata-core.c |  1 +
>  drivers/ata/libata-scsi.c |  1 +
>  include/linux/libata.h    |  3 ++-
>  5 files changed, 53 insertions(+), 15 deletions(-)
>
Applied to bionic/master-next and cosmic/master-next branches.

As Stefan mentioned, a new commit to set the default
CONFIG_SATA_MOBILE_LPM_POLICY=0 on
debian.master/config/config.common.ubuntu was needed.

The Cosmic common config already had an entry for that, but set to =3.
That was set as requested on LP: #1759547 ("Use med_power_with_dipm SATA
LPM to save more power for mobile platforms"), if that default value
would also make more sense than =0 on Bionic please let us know.


Thanks,
Kleber


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team