Commit a7152be79b62 ("Revert "PCI/ASPM: Save L1 PM Substates Capability
for suspend/resume"") reverted saving and restoring of ASPM L1 Substates
due to a regression that caused resume from suspend to fail on certain
systems. However, we never added this capability back and this is now
causing systems fail to enter low power CPU states, drawing more power
from the battery.
The original revert mentioned that we restore L1 PM substate configuration
even though ASPM L1 may already be enabled. This is due the fact that
the pci_restore_aspm_l1ss_state() was called before
pci_restore_pcie_state().
Try to enable this functionality again by:
1) Moving the restore happen after PCIe capability is restored
following PCIe r6.0, sec 5.5.4.
2) Following the PCIe spec more closely to restore L1 PM substate
configuration (program enable bits last).
3) Adding denylist that skips the restoring on the ASUS system where
the original regression happened, just in case.
Reported-by: Koba Ko <koba.ko@xxxxxxxxxxxxx>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217321
Cc: Tasev Nikola <tasev.stefanoska@xxxxxxxxx>
Cc: Mark Enriquez <enriquezmark36@xxxxxxxxx>
Cc: Thomas Witt <kernel@xxxxxxxxx>
Signed-off-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
---
Hi,
There are several systems out there that are not entering lower package
C-states after the first suspend/resume cycle because we do not restore
L1SS configuration as can be seen in the linked bugzilla entry. Although
they are functioning fine they still empty the battery faster than the
user may expect.
Apologies sending this out during merge window but I will be starting my
vacation next week so wanted to get this out before, and possibly
incorporate any changes during the week still.
drivers/pci/pci.c | 7 ++++
drivers/pci/pci.h | 4 +++
drivers/pci/pcie/aspm.c | 76 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 87 insertions(+)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 578bf0d3ec3c..bf30561aa32c 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1679,6 +1679,7 @@ int pci_save_state(struct pci_dev *dev)
pci_save_dpc_state(dev);
pci_save_aer_state(dev);
pci_save_ptm_state(dev);
+ pci_save_aspm_l1ss_state(dev);
return pci_save_vc_state(dev);
}
EXPORT_SYMBOL(pci_save_state);
@@ -1790,6 +1791,7 @@ void pci_restore_state(struct pci_dev *dev)
pci_restore_vc_state(dev);
pci_restore_rebar_state(dev);
pci_restore_dpc_state(dev);
+ pci_restore_aspm_l1ss_state(dev);
pci_restore_ptm_state(dev);
pci_aer_clear_status(dev);
@@ -3474,6 +3476,11 @@ void pci_allocate_cap_save_buffers(struct pci_dev *dev)
if (error)
pci_err(dev, "unable to allocate suspend buffer for LTR\n");
+ error = pci_add_ext_cap_save_buffer(dev, PCI_EXT_CAP_ID_L1SS,
+ 2 * sizeof(u32));
+ if (error)
+ pci_err(dev, "unable to allocate suspend buffer for ASPM-L1SS\n");
+
pci_allocate_vc_save_buffers(dev);
}
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index d09e8f39e429..f34b55701d63 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -560,10 +560,14 @@ bool pcie_wait_for_link(struct pci_dev *pdev, bool active);
void pcie_aspm_init_link_state(struct pci_dev *pdev);
void pcie_aspm_exit_link_state(struct pci_dev *pdev);
void pcie_aspm_powersave_config_link(struct pci_dev *pdev);
+void pci_save_aspm_l1ss_state(struct pci_dev *pdev);
+void pci_restore_aspm_l1ss_state(struct pci_dev *pdev);
#else
static inline void pcie_aspm_init_link_state(struct pci_dev *pdev) { }
static inline void pcie_aspm_exit_link_state(struct pci_dev *pdev) { }
static inline void pcie_aspm_powersave_config_link(struct pci_dev *pdev) { }
+static inline void pci_save_aspm_l1ss_state(struct pci_dev *pdev) { }
+static inline void pci_restore_aspm_l1ss_state(struct pci_dev *pdev) { }
#endif
#ifdef CONFIG_PCIE_ECRC
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 66d7514ca111..52e1a69f818a 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -7,6 +7,7 @@
* Copyright (C) Shaohua Li (shaohua.li@xxxxxxxxx)
*/
+#include <linux/dmi.h>
#include <linux/kernel.h>
#include <linux/math.h>
#include <linux/module.h>
@@ -751,6 +752,81 @@ static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state)
PCI_L1SS_CTL1_L1SS_MASK, val);
}
+void pci_save_aspm_l1ss_state(struct pci_dev *pdev)
+{
+ struct pci_cap_saved_state *save_state;
+ u16 l1ss = pdev->l1ss;
+ u32 *cap;
+
+ if (!l1ss)
+ return;
+
+ save_state = pci_find_saved_ext_cap(pdev, PCI_EXT_CAP_ID_L1SS);
+ if (!save_state)
+ return;
+
+ cap = (u32 *)&save_state->cap.data[0];
+ pci_read_config_dword(pdev, l1ss + PCI_L1SS_CTL2, cap++);
+ pci_read_config_dword(pdev, l1ss + PCI_L1SS_CTL1, cap++);
+}
+
+/*
+ * Do not restore L1 substates for the below systems even if BIOS has
+ * enabled it initially. This breaks resume from suspend otherwise on
+ * these.
+ */
+static const struct dmi_system_id aspm_l1ss_denylist[] = {
+ {
+ /* https://bugzilla.kernel.org/show_bug.cgi?id=216782 */
+ .ident = "ASUS UX305FA",
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "ASUSTeK COMPUTER INC."),
+ DMI_MATCH(DMI_BOARD_NAME, "UX305FA"),
+ },
+ },
+ { }
+};
+
+void pci_restore_aspm_l1ss_state(struct pci_dev *pdev)
+{
+ struct pci_cap_saved_state *save_state;
+ u32 *cap, ctl1, ctl2, l1_2_enable;
+ u16 l1ss = pdev->l1ss;
+
+ if (!l1ss)
+ return;
+
+ if (dmi_check_system(aspm_l1ss_denylist)) {
+ pci_dbg(pdev, "skipping restoring L1 substates on this system\n");
+ return;
+ }
+
+ save_state = pci_find_saved_ext_cap(pdev, PCI_EXT_CAP_ID_L1SS);
+ if (!save_state)
+ return;
+
+ cap = (u32 *)&save_state->cap.data[0];
+ ctl2 = *cap++;
+ ctl1 = *cap;
+
+ /*
+ * In addition, Common_Mode_Restore_Time and LTR_L1.2_THRESHOLD
+ * in PCI_L1SS_CTL1 must be programmed *before* setting the L1.2
+ * enable bits, even though they're all in PCI_L1SS_CTL1.
+ */
+ l1_2_enable = ctl1 & PCI_L1SS_CTL1_L1_2_MASK;
+ ctl1 &= ~PCI_L1SS_CTL1_L1_2_MASK;
+
+ /* Write back without enables first (above we cleared them in ctl1) */
+ pci_write_config_dword(pdev, l1ss + PCI_L1SS_CTL1, ctl1);
+ pci_write_config_dword(pdev, l1ss + PCI_L1SS_CTL2, ctl2);
+
+ /* Then write back the enables */
+ if (l1_2_enable)
+ pci_write_config_dword(pdev, l1ss + PCI_L1SS_CTL1,
+ ctl1 | l1_2_enable);
+}
+
static void pcie_config_aspm_dev(struct pci_dev *pdev, u32 val)
{
pcie_capability_clear_and_set_word(pdev, PCI_EXP_LNKCTL,
--
2.40.1
