[PATCH] PCI/ASPM: Disable ASPM when save/restore PCI state
From: Victor Ding <hidden>
Date: 2021-01-28 12:29:39
Also in:
linux-pci, lkml
Subsystem:
pci subsystem, the rest · Maintainers:
Bjorn Helgaas, Linus Torvalds
Certain PCIe devices (e.g. GL9750) have high penalties (e.g. high Port T_POWER_ON) when exiting L1 but enter L1 aggressively. As a result, such devices enter and exit L1 frequently during pci_save_state and pci_restore_state; eventually causing poor suspend/resume performance. Based on the observation that PCI accesses dominance pci_save_state/ pci_restore_state plus these accesses are fairly close to each other, the actual time the device could stay in low power states is negligible. Therefore, the little power-saving benefit from ASPM during suspend/resume does not overweight the performance degradation caused by high L1 exit penalties. Therefore, this patch proposes to disable ASPM during a suspend/resume process. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=211187 Signed-off-by: Victor Ding <redacted> --- drivers/pci/pci.c | 18 +++++++++++++++--- drivers/pci/pci.h | 6 ++++++ drivers/pci/pcie/aspm.c | 26 ++++++++++++++++++++++++++ include/linux/pci.h | 1 + 4 files changed, 48 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 32011b7b4c04..a925a7075063 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c@@ -1542,6 +1542,10 @@ static void pci_restore_ltr_state(struct pci_dev *dev) int pci_save_state(struct pci_dev *dev) { int i; + + pcie_save_aspm_control(dev); + pcie_disable_aspm(dev); + /* XXX: 100% dword access ok here? */ for (i = 0; i < 16; i++) { pci_read_config_dword(dev, i * 4, &dev->saved_config_space[i]);
@@ -1552,18 +1556,22 @@ int pci_save_state(struct pci_dev *dev) i = pci_save_pcie_state(dev); if (i != 0) - return i; + goto Exit; i = pci_save_pcix_state(dev); if (i != 0) - return i; + goto Exit; pci_save_ltr_state(dev); pci_save_aspm_l1ss_state(dev); pci_save_dpc_state(dev); pci_save_aer_state(dev); pci_save_ptm_state(dev); - return pci_save_vc_state(dev); + i = pci_save_vc_state(dev); + +Exit: + pcie_restore_aspm_control(dev); + return i; } EXPORT_SYMBOL(pci_save_state);
@@ -1661,6 +1669,8 @@ void pci_restore_state(struct pci_dev *dev) if (!dev->state_saved) return; + pcie_disable_aspm(dev); + /* * Restore max latencies (in the LTR capability) before enabling * LTR itself (in the PCIe capability).
@@ -1689,6 +1699,8 @@ void pci_restore_state(struct pci_dev *dev) pci_enable_acs(dev); pci_restore_iov_state(dev); + pcie_restore_aspm_control(dev); + dev->state_saved = false; } EXPORT_SYMBOL(pci_restore_state);
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index a81459159f6d..e074a0cbe73c 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h@@ -584,6 +584,9 @@ void pcie_aspm_pm_state_change(struct pci_dev *pdev); void pcie_aspm_powersave_config_link(struct pci_dev *pdev); void pci_save_aspm_l1ss_state(struct pci_dev *dev); void pci_restore_aspm_l1ss_state(struct pci_dev *dev); +void pcie_save_aspm_control(struct pci_dev *dev); +void pcie_restore_aspm_control(struct pci_dev *dev); +void pcie_disable_aspm(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) { }
@@ -591,6 +594,9 @@ static inline void pcie_aspm_pm_state_change(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 *dev) { } static inline void pci_restore_aspm_l1ss_state(struct pci_dev *dev) { } +static inline void pcie_save_aspm_control(struct pci_dev *dev) { } +static inline void pcie_restore_aspm_control(struct pci_dev *dev) { } +static inline void pcie_disable_aspm(struct pci_dev *pdev) { } #endif #ifdef CONFIG_PCIE_ECRC
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index a08e7d6dc248..519b9f1b067a 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c@@ -105,6 +105,12 @@ static const char *policy_str[] = { #define LINK_RETRAIN_TIMEOUT HZ +void pcie_disable_aspm(struct pci_dev *pdev) +{ + pcie_capability_clear_and_set_word(pdev, PCI_EXP_LNKCTL, + PCI_EXP_LNKCTL_ASPMC, 0); +} + static int policy_to_aspm_state(struct pcie_link_state *link) { switch (aspm_policy) {
@@ -680,6 +686,26 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist) } } +void pcie_save_aspm_control(struct pci_dev *dev) +{ + u16 lnkctl; + + if (!pci_is_pcie(dev)) + return; + + pci_read_config_word(dev, PCI_EXP_LNKCTL, &lnkctl); + dev->saved_aspm_ctl = lnkctl & PCI_EXP_LNKCTL_ASPMC; +} + +void pcie_restore_aspm_control(struct pci_dev *dev) +{ + if (!pci_is_pcie(dev)) + return; + + pcie_capability_clear_and_set_word(dev, PCI_EXP_LNKCTL, + PCI_EXP_LNKCTL_ASPMC, dev->saved_aspm_ctl); +} + /* Configure the ASPM L1 substates */ static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state) {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index b32126d26997..a21bfd6e3f89 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h@@ -387,6 +387,7 @@ struct pci_dev { unsigned int ltr_path:1; /* Latency Tolerance Reporting supported from root to here */ u16 l1ss; /* L1SS Capability pointer */ + u16 saved_aspm_ctl; /* ASPM Control saved at suspend time */ #endif unsigned int eetlp_prefix_path:1; /* End-to-End TLP Prefix */
--
2.30.0.280.ga3ce27912f-goog