* [PATCH 0/5] Always build aspm.c
@ 2024-01-28 23:32 David E. Box
2024-01-28 23:32 ` [PATCH 1/5] PCI: " David E. Box
` (5 more replies)
0 siblings, 6 replies; 8+ messages in thread
From: David E. Box @ 2024-01-28 23:32 UTC (permalink / raw)
To: mika.westerberg, david.e.box, ilpo.jarvinen, bhelgaas, rjw
Cc: tasev.stefanoska, enriquezmark36, kernel, wse, vidyas,
kai.heng.feng, sathyanarayanan.kuppuswamy, ricky_wu,
mario.limonciello, linux-pci, linux-kernel
Here is the series to always build aspm.c, add back L1SS save/restore, and
consolidate all ASPM related code in aspm.c.
Patch 1 - Moves all PCI core functions running under CONFIG_PCIEASPM into
aspm.c and changes they Makefile to always build it. No functional changes.
Patch 2 - Creates a separate function to save the L1SS offset and places it
outside of CONFIG_PCIEASPM in aspm.c so that the offset is available for
later use by the L1SS save/restore code which needs to run when
CONFIG_PCIEASPM=n.
Patch 3 - Updated L1 Substate save/restore patch from previous V5 [1].
Patch 4 - Moves the LTR save/restore state functions into aspm.c outside of
CONFIG_PCIEASPM.
Patch 5 - Moves the LTR save/restore state calls into
pci_save/restore_pcie_state().
The series does not continue any of the ASPM robustness changes proposed by
Ilpo [2]. But if we think it's worth combining with this series I can
add it and help continue the work.
[1] https://lore.kernel.org/linux-pci/20231221011250.191599-1-david.e.box@linux.intel.com/
[2] https://lore.kernel.org/linux-pci/20230918131103.24119-1-ilpo.jarvinen@linux.intel.com/
David E. Box (5):
PCI: Always build aspm.c
PCI: Create function to save L1SS offset
PCI/ASPM: Add back L1 PM Substate save and restore
PCI: Move pci_save/restore_ltr_state() to aspm.c
PCI: Call LTR save/restore state from PCIe save/restore
drivers/pci/pci.c | 91 ++++----------
drivers/pci/pci.h | 10 +-
drivers/pci/pcie/Makefile | 2 +-
drivers/pci/pcie/aspm.c | 257 ++++++++++++++++++++++++++++++++++++--
drivers/pci/probe.c | 62 +--------
include/linux/pci.h | 4 +-
6 files changed, 283 insertions(+), 143 deletions(-)
base-commit: 0dd3ee31125508cd67f7e7172247f05b7fd1753a
--
2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/5] PCI: Always build aspm.c
2024-01-28 23:32 [PATCH 0/5] Always build aspm.c David E. Box
@ 2024-01-28 23:32 ` David E. Box
2024-01-28 23:32 ` [PATCH 2/5] PCI: Create function to save L1SS offset David E. Box
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: David E. Box @ 2024-01-28 23:32 UTC (permalink / raw)
To: mika.westerberg, david.e.box, ilpo.jarvinen, bhelgaas, rjw
Cc: tasev.stefanoska, enriquezmark36, kernel, wse, vidyas,
kai.heng.feng, sathyanarayanan.kuppuswamy, ricky_wu,
mario.limonciello, linux-pci, linux-kernel
Some ASPM related tasks, such as save and restore of LTR and L1SS
capabilities, still need to be performed when CONFIG_PCIASPM=n. To prepare
for these changes, wrap the current code in aspm.c with an ifdef and always
build. Also move pci_configure_ltr() and pci_bridge_reconfigure_ltr() into
aspm.c since they only build when CONFIG_PCIEASPM is set.
Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
drivers/pci/pci.c | 18 ---------
drivers/pci/pci.h | 5 ++-
drivers/pci/pcie/Makefile | 2 +-
drivers/pci/pcie/aspm.c | 78 +++++++++++++++++++++++++++++++++++++++
drivers/pci/probe.c | 61 ------------------------------
5 files changed, 83 insertions(+), 81 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index bdbf8a94b4d0..71229ec39e88 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1563,24 +1563,6 @@ static int pci_save_pcie_state(struct pci_dev *dev)
return 0;
}
-void pci_bridge_reconfigure_ltr(struct pci_dev *dev)
-{
-#ifdef CONFIG_PCIEASPM
- struct pci_dev *bridge;
- u32 ctl;
-
- bridge = pci_upstream_bridge(dev);
- if (bridge && bridge->ltr_path) {
- pcie_capability_read_dword(bridge, PCI_EXP_DEVCTL2, &ctl);
- if (!(ctl & PCI_EXP_DEVCTL2_LTR_EN)) {
- pci_dbg(bridge, "re-enabling LTR\n");
- pcie_capability_set_word(bridge, PCI_EXP_DEVCTL2,
- PCI_EXP_DEVCTL2_LTR_EN);
- }
- }
-#endif
-}
-
static void pci_restore_pcie_state(struct pci_dev *dev)
{
int i = 0;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index f43873049d52..6771862de921 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -97,7 +97,6 @@ void pci_msi_init(struct pci_dev *dev);
void pci_msix_init(struct pci_dev *dev);
bool pci_bridge_d3_possible(struct pci_dev *dev);
void pci_bridge_d3_update(struct pci_dev *dev);
-void pci_bridge_reconfigure_ltr(struct pci_dev *dev);
int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type);
static inline void pci_wakeup_event(struct pci_dev *dev)
@@ -571,11 +570,15 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev);
void pcie_aspm_exit_link_state(struct pci_dev *pdev);
void pcie_aspm_pm_state_change(struct pci_dev *pdev);
void pcie_aspm_powersave_config_link(struct pci_dev *pdev);
+void pci_configure_ltr(struct pci_dev *pdev);
+void pci_bridge_reconfigure_ltr(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_pm_state_change(struct pci_dev *pdev) { }
static inline void pcie_aspm_powersave_config_link(struct pci_dev *pdev) { }
+static inline void pci_configure_ltr(struct pci_dev *pdev) { }
+static inline void pci_bridge_reconfigure_ltr(struct pci_dev *pdev) { }
#endif
#ifdef CONFIG_PCIE_ECRC
diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
index 8de4ed5f98f1..6461aa93fe76 100644
--- a/drivers/pci/pcie/Makefile
+++ b/drivers/pci/pcie/Makefile
@@ -6,7 +6,7 @@ pcieportdrv-y := portdrv.o rcec.o
obj-$(CONFIG_PCIEPORTBUS) += pcieportdrv.o
-obj-$(CONFIG_PCIEASPM) += aspm.o
+obj-y += aspm.o
obj-$(CONFIG_PCIEAER) += aer.o err.o
obj-$(CONFIG_PCIEAER_INJECT) += aer_inject.o
obj-$(CONFIG_PCIE_PME) += pme.o
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 060f4b3c8698..6d077e237a65 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -24,6 +24,8 @@
#include "../pci.h"
+#ifdef CONFIG_PCIEASPM
+
#ifdef MODULE_PARAM_PREFIX
#undef MODULE_PARAM_PREFIX
#endif
@@ -943,6 +945,81 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
up_read(&pci_bus_sem);
}
+void pci_bridge_reconfigure_ltr(struct pci_dev *pdev)
+{
+ struct pci_dev *bridge;
+ u32 ctl;
+
+ bridge = pci_upstream_bridge(pdev);
+ if (bridge && bridge->ltr_path) {
+ pcie_capability_read_dword(bridge, PCI_EXP_DEVCTL2, &ctl);
+ if (!(ctl & PCI_EXP_DEVCTL2_LTR_EN)) {
+ pci_dbg(bridge, "re-enabling LTR\n");
+ pcie_capability_set_word(bridge, PCI_EXP_DEVCTL2,
+ PCI_EXP_DEVCTL2_LTR_EN);
+ }
+ }
+}
+
+void pci_configure_ltr(struct pci_dev *pdev)
+{
+ struct pci_host_bridge *host = pci_find_host_bridge(pdev->bus);
+ struct pci_dev *bridge;
+ u32 cap, ctl;
+
+ if (!pci_is_pcie(pdev))
+ return;
+
+ /* Read L1 PM substate capabilities */
+ pdev->l1ss = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_L1SS);
+
+ pcie_capability_read_dword(pdev, PCI_EXP_DEVCAP2, &cap);
+ if (!(cap & PCI_EXP_DEVCAP2_LTR))
+ return;
+
+ pcie_capability_read_dword(pdev, PCI_EXP_DEVCTL2, &ctl);
+ if (ctl & PCI_EXP_DEVCTL2_LTR_EN) {
+ if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT) {
+ pdev->ltr_path = 1;
+ return;
+ }
+
+ bridge = pci_upstream_bridge(pdev);
+ if (bridge && bridge->ltr_path)
+ pdev->ltr_path = 1;
+
+ return;
+ }
+
+ if (!host->native_ltr)
+ return;
+
+ /*
+ * Software must not enable LTR in an Endpoint unless the Root
+ * Complex and all intermediate Switches indicate support for LTR.
+ * PCIe r4.0, sec 6.18.
+ */
+ if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT) {
+ pcie_capability_set_word(pdev, PCI_EXP_DEVCTL2,
+ PCI_EXP_DEVCTL2_LTR_EN);
+ pdev->ltr_path = 1;
+ return;
+ }
+
+ /*
+ * If we're configuring a hot-added device, LTR was likely
+ * disabled in the upstream bridge, so re-enable it before enabling
+ * it in the new device.
+ */
+ bridge = pci_upstream_bridge(pdev);
+ if (bridge && bridge->ltr_path) {
+ pci_bridge_reconfigure_ltr(pdev);
+ pcie_capability_set_word(pdev, PCI_EXP_DEVCTL2,
+ PCI_EXP_DEVCTL2_LTR_EN);
+ pdev->ltr_path = 1;
+ }
+}
+
/* Recheck latencies and update aspm_capable for links under the root */
static void pcie_update_aspm_capable(struct pcie_link_state *root)
{
@@ -1447,3 +1524,4 @@ bool pcie_aspm_support_enabled(void)
{
return aspm_support_enabled;
}
+#endif /* CONFIG_PCIEASPM */
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ed6b7f48736a..0b8c2c9cc9dd 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2159,67 +2159,6 @@ static void pci_configure_relaxed_ordering(struct pci_dev *dev)
}
}
-static void pci_configure_ltr(struct pci_dev *dev)
-{
-#ifdef CONFIG_PCIEASPM
- struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
- struct pci_dev *bridge;
- u32 cap, ctl;
-
- if (!pci_is_pcie(dev))
- return;
-
- /* Read L1 PM substate capabilities */
- dev->l1ss = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_L1SS);
-
- pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
- if (!(cap & PCI_EXP_DEVCAP2_LTR))
- return;
-
- pcie_capability_read_dword(dev, PCI_EXP_DEVCTL2, &ctl);
- if (ctl & PCI_EXP_DEVCTL2_LTR_EN) {
- if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) {
- dev->ltr_path = 1;
- return;
- }
-
- bridge = pci_upstream_bridge(dev);
- if (bridge && bridge->ltr_path)
- dev->ltr_path = 1;
-
- return;
- }
-
- if (!host->native_ltr)
- return;
-
- /*
- * Software must not enable LTR in an Endpoint unless the Root
- * Complex and all intermediate Switches indicate support for LTR.
- * PCIe r4.0, sec 6.18.
- */
- if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) {
- pcie_capability_set_word(dev, PCI_EXP_DEVCTL2,
- PCI_EXP_DEVCTL2_LTR_EN);
- dev->ltr_path = 1;
- return;
- }
-
- /*
- * If we're configuring a hot-added device, LTR was likely
- * disabled in the upstream bridge, so re-enable it before enabling
- * it in the new device.
- */
- bridge = pci_upstream_bridge(dev);
- if (bridge && bridge->ltr_path) {
- pci_bridge_reconfigure_ltr(dev);
- pcie_capability_set_word(dev, PCI_EXP_DEVCTL2,
- PCI_EXP_DEVCTL2_LTR_EN);
- dev->ltr_path = 1;
- }
-#endif
-}
-
static void pci_configure_eetlp_prefix(struct pci_dev *dev)
{
#ifdef CONFIG_PCI_PASID
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/5] PCI: Create function to save L1SS offset
2024-01-28 23:32 [PATCH 0/5] Always build aspm.c David E. Box
2024-01-28 23:32 ` [PATCH 1/5] PCI: " David E. Box
@ 2024-01-28 23:32 ` David E. Box
2024-01-28 23:32 ` [PATCH 3/5] PCI/ASPM: Add back L1 PM Substate save and restore David E. Box
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: David E. Box @ 2024-01-28 23:32 UTC (permalink / raw)
To: mika.westerberg, david.e.box, ilpo.jarvinen, bhelgaas, rjw
Cc: tasev.stefanoska, enriquezmark36, kernel, wse, vidyas,
kai.heng.feng, sathyanarayanan.kuppuswamy, ricky_wu,
mario.limonciello, linux-pci, linux-kernel
The offset for the L1 Substate Capability register is not saved in pci_dev
until pci_configure_ltr() which only builds with CONFIG_PCIEASPM. Instead,
create a separate function to retrieve the offset so that it is always
available. This offset will be used to save and restore the L1SS registers
even when PCIEASPM=n.
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
drivers/pci/pci.h | 1 +
drivers/pci/pcie/aspm.c | 9 ++++++---
drivers/pci/probe.c | 1 +
include/linux/pci.h | 4 ++--
4 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 6771862de921..b48e8e4f360f 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -97,6 +97,7 @@ void pci_msi_init(struct pci_dev *dev);
void pci_msix_init(struct pci_dev *dev);
bool pci_bridge_d3_possible(struct pci_dev *dev);
void pci_bridge_d3_update(struct pci_dev *dev);
+void pci_aspm_get_l1ss(struct pci_dev *pdev);
int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type);
static inline void pci_wakeup_event(struct pci_dev *dev)
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 6d077e237a65..93718b733af3 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -24,6 +24,12 @@
#include "../pci.h"
+void pci_aspm_get_l1ss(struct pci_dev *pdev)
+{
+ /* Read L1 PM substate capabilities */
+ pdev->l1ss = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_L1SS);
+}
+
#ifdef CONFIG_PCIEASPM
#ifdef MODULE_PARAM_PREFIX
@@ -970,9 +976,6 @@ void pci_configure_ltr(struct pci_dev *pdev)
if (!pci_is_pcie(pdev))
return;
- /* Read L1 PM substate capabilities */
- pdev->l1ss = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_L1SS);
-
pcie_capability_read_dword(pdev, PCI_EXP_DEVCAP2, &cap);
if (!(cap & PCI_EXP_DEVCAP2_LTR))
return;
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 0b8c2c9cc9dd..e39ad912ce8c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2208,6 +2208,7 @@ static void pci_configure_device(struct pci_dev *dev)
pci_configure_mps(dev);
pci_configure_extended_tags(dev, NULL);
pci_configure_relaxed_ordering(dev);
+ pci_aspm_get_l1ss(dev);
pci_configure_ltr(dev);
pci_configure_eetlp_prefix(dev);
pci_configure_serr(dev);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index dea043bc1e38..dfc4b525c7a1 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -390,9 +390,9 @@ struct pci_dev {
unsigned int d3hot_delay; /* D3hot->D0 transition time in ms */
unsigned int d3cold_delay; /* D3cold->D0 transition time in ms */
-#ifdef CONFIG_PCIEASPM
- struct pcie_link_state *link_state; /* ASPM link state */
u16 l1ss; /* L1SS Capability pointer */
+#ifdef CONFIG_PCIEASPM
+ struct pcie_link_state *link_state; /* ASPM link state */
unsigned int ltr_path:1; /* Latency Tolerance Reporting
supported from root to here */
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/5] PCI/ASPM: Add back L1 PM Substate save and restore
2024-01-28 23:32 [PATCH 0/5] Always build aspm.c David E. Box
2024-01-28 23:32 ` [PATCH 1/5] PCI: " David E. Box
2024-01-28 23:32 ` [PATCH 2/5] PCI: Create function to save L1SS offset David E. Box
@ 2024-01-28 23:32 ` David E. Box
2024-01-28 23:32 ` [PATCH 4/5] PCI: Move pci_save/restore_ltr_state() to aspm.c David E. Box
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: David E. Box @ 2024-01-28 23:32 UTC (permalink / raw)
To: mika.westerberg, david.e.box, ilpo.jarvinen, bhelgaas, rjw
Cc: tasev.stefanoska, enriquezmark36, kernel, wse, vidyas,
kai.heng.feng, sathyanarayanan.kuppuswamy, ricky_wu,
mario.limonciello, linux-pci, linux-kernel
Commit 4ff116d0d5fd ("PCI/ASPM: Save L1 PM Substates Capability for
suspend/resume") was reverted due to a regression that caused resume from
suspend to fail on certain systems. However, lack of this feature is now
causing systems to 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 following PCIe r6.0.1, sec 5.5.4
more closely by:
1) Do not restore ASPM configuration in pci_restore_pcie_state() but
do that after PCIe capability is restored in pci_restore_aspm_state()
following PCIe r6.0, sec 5.5.4.
2) If BIOS reenables L1SS on us, particularly L1.2, we need to clear the
enables in the right order, downstream before upstream. Defer
restoring the L1SS config until we are at the downstream component.
Then update the config for both ends of the link in the prescribed
order.
3) Program ASPM L1 PM substate configuration before L1 enables.
4) Program ASPM L1 PM substate enables last after rest of the fields
in the capability are programmed.
Reported-by: Koba Ko <koba.ko@canonical.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217321
Link: https://bugzilla.kernel.org/show_bug.cgi?id=216782
Link: https://bugzilla.kernel.org/show_bug.cgi?id=216877
Cc: Tasev Nikola <tasev.stefanoska@skynet.be>
Cc: Mark Enriquez <enriquezmark36@gmail.com>
Cc: Thomas Witt <kernel@witt.link>
Cc: Werner Sembach <wse@tuxedocomputers.com>
Cc: Vidya Sagar <vidyas@nvidia.com>
Co-developed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Co-developed-by: David E. Box <david.e.box@linux.intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
V1 - Move aspm save/restore calls to pci_save/restore_pcie_state()
- Move aspm save/restore functions outside of CONFIG_PCIEASPM so that
L1SS state is always managed.
- pcie_link_state is now unavailable when CONFIG_PCIEASPM=n, so
use pcie_downstream_port() to test for the link during restore.
- Comment and code cleanup suggested by Bjorn and Mika
Previous history before new series:
v5: https://lore.kernel.org/linux-pci/20231221011250.191599-1-david.e.box@linux.intel.com/
v4: https://lore.kernel.org/linux-pci/20231002070044.2299644-1-mika.westerberg@linux.intel.com/
v3: https://lore.kernel.org/linux-pci/20230925074636.2893747-1-mika.westerberg@linux.intel.com/
v2: https://lore.kernel.org/linux-pci/20230911073352.3472918-1-mika.westerberg@linux.intel.com/
v1: https://lore.kernel.org/linux-pci/20230627062442.54008-1-mika.westerberg@linux.intel.com/
drivers/pci/pci.c | 19 +++++-
drivers/pci/pci.h | 2 +
drivers/pci/pcie/aspm.c | 136 ++++++++++++++++++++++++++++++++++++----
3 files changed, 144 insertions(+), 13 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 71229ec39e88..0a8613e77dab 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1560,6 +1560,8 @@ static int pci_save_pcie_state(struct pci_dev *dev)
pcie_capability_read_word(dev, PCI_EXP_LNKCTL2, &cap[i++]);
pcie_capability_read_word(dev, PCI_EXP_SLTCTL2, &cap[i++]);
+ pci_save_aspm_state(dev);
+
return 0;
}
@@ -1567,7 +1569,7 @@ static void pci_restore_pcie_state(struct pci_dev *dev)
{
int i = 0;
struct pci_cap_saved_state *save_state;
- u16 *cap;
+ u16 *cap, val;
save_state = pci_find_saved_cap(dev, PCI_CAP_ID_EXP);
if (!save_state)
@@ -1582,12 +1584,20 @@ static void pci_restore_pcie_state(struct pci_dev *dev)
cap = (u16 *)&save_state->cap.data[0];
pcie_capability_write_word(dev, PCI_EXP_DEVCTL, cap[i++]);
- pcie_capability_write_word(dev, PCI_EXP_LNKCTL, cap[i++]);
+
+ /*
+ * Restore only the LNKCTL register with the ASPM control field
+ * clear. ASPM will be restored in pci_restore_aspm_state().
+ */
+ val = cap[i++] & ~PCI_EXP_LNKCTL_ASPMC;
+ pcie_capability_write_word(dev, PCI_EXP_LNKCTL, val);
pcie_capability_write_word(dev, PCI_EXP_SLTCTL, cap[i++]);
pcie_capability_write_word(dev, PCI_EXP_RTCTL, cap[i++]);
pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, cap[i++]);
pcie_capability_write_word(dev, PCI_EXP_LNKCTL2, cap[i++]);
pcie_capability_write_word(dev, PCI_EXP_SLTCTL2, cap[i++]);
+
+ pci_restore_aspm_state(dev);
}
static int pci_save_pcix_state(struct pci_dev *dev)
@@ -3497,6 +3507,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 b48e8e4f360f..7b14cdbe2e69 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -98,6 +98,8 @@ void pci_msix_init(struct pci_dev *dev);
bool pci_bridge_d3_possible(struct pci_dev *dev);
void pci_bridge_d3_update(struct pci_dev *dev);
void pci_aspm_get_l1ss(struct pci_dev *pdev);
+void pci_save_aspm_state(struct pci_dev *pdev);
+void pci_restore_aspm_state(struct pci_dev *pdev);
int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type);
static inline void pci_wakeup_event(struct pci_dev *dev)
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 93718b733af3..3185058e9c41 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -30,6 +30,131 @@ void pci_aspm_get_l1ss(struct pci_dev *pdev)
pdev->l1ss = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_L1SS);
}
+static void pci_clear_and_set_dword(struct pci_dev *pdev, int pos,
+ u32 clear, u32 set)
+{
+ u32 val;
+
+ pci_read_config_dword(pdev, pos, &val);
+ val &= ~clear;
+ val |= set;
+ pci_write_config_dword(pdev, pos, val);
+}
+
+void pci_save_aspm_state(struct pci_dev *pdev)
+{
+ struct pci_cap_saved_state *save_state;
+ u16 l1ss = pdev->l1ss;
+ u32 *cap;
+
+ /*
+ * Save L1 substate configuration. The ASPM L0s/L1 configuration
+ * is already saved in pci_save_pcie_state().
+ */
+ if (!l1ss)
+ return;
+
+ save_state = pci_find_saved_ext_cap(pdev, PCI_EXT_CAP_ID_L1SS);
+ if (!save_state)
+ return;
+
+ cap = &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++);
+}
+
+static void pcie_restore_aspm_l1ss(struct pci_dev *pdev)
+{
+ struct pci_cap_saved_state *pl_save_state, *cl_save_state;
+ struct pci_dev *parent = pdev->bus->self;
+ u32 *cap, pl_ctl1, pl_ctl2, pl_l1_2_enable;
+ u32 cl_ctl1, cl_ctl2, cl_l1_2_enable;
+
+ /*
+ * In case BIOS enabled L1.2 after resume, we need to disable it first
+ * on the downstream component before the upstream. So, don't attempt to
+ * restore either until we are at the downstream component.
+ */
+ if (pcie_downstream_port(pdev) || !parent)
+ return;
+
+ if (!pdev->l1ss || !parent->l1ss)
+ return;
+
+ cl_save_state = pci_find_saved_ext_cap(pdev, PCI_EXT_CAP_ID_L1SS);
+ pl_save_state = pci_find_saved_ext_cap(parent, PCI_EXT_CAP_ID_L1SS);
+ if (!cl_save_state || !pl_save_state)
+ return;
+
+ cap = &cl_save_state->cap.data[0];
+ cl_ctl2 = *cap++;
+ cl_ctl1 = *cap;
+ cap = &pl_save_state->cap.data[0];
+ pl_ctl2 = *cap++;
+ pl_ctl1 = *cap;
+
+
+ /*
+ * Disable L1.2 on this downstream endpoint device first, followed
+ * by the upstream
+ */
+ pci_clear_and_set_dword(pdev, pdev->l1ss + PCI_L1SS_CTL1,
+ PCI_L1SS_CTL1_L1_2_MASK, 0);
+ pci_clear_and_set_dword(parent, parent->l1ss + PCI_L1SS_CTL1,
+ PCI_L1SS_CTL1_L1_2_MASK, 0);
+
+ /*
+ * 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.
+ */
+ pl_l1_2_enable = pl_ctl1 & PCI_L1SS_CTL1_L1_2_MASK;
+ pl_ctl1 &= ~PCI_L1SS_CTL1_L1_2_MASK;
+ cl_l1_2_enable = cl_ctl1 & PCI_L1SS_CTL1_L1_2_MASK;
+ cl_ctl1 &= ~PCI_L1SS_CTL1_L1_2_MASK;
+
+ /* Write back without enables first (above we cleared them in ctl1) */
+ pci_write_config_dword(parent, parent->l1ss + PCI_L1SS_CTL2, pl_ctl2);
+ pci_write_config_dword(pdev, pdev->l1ss + PCI_L1SS_CTL2, cl_ctl2);
+ pci_write_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1, pl_ctl1);
+ pci_write_config_dword(pdev, pdev->l1ss + PCI_L1SS_CTL1, cl_ctl1);
+
+
+ /* Then write back the enables */
+ if (pl_l1_2_enable || cl_l1_2_enable) {
+ pci_write_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1,
+ pl_ctl1 | pl_l1_2_enable);
+ pci_write_config_dword(pdev, pdev->l1ss + PCI_L1SS_CTL1,
+ cl_ctl1 | cl_l1_2_enable);
+ }
+}
+
+void pci_restore_aspm_state(struct pci_dev *pdev)
+{
+ struct pci_cap_saved_state *save_state;
+ u16 *cap, val;
+
+ save_state = pci_find_saved_cap(pdev, PCI_CAP_ID_EXP);
+
+ if (!save_state)
+ return;
+
+ cap = (u16 *)&save_state->cap.data[0];
+ /* Must match the ordering in pci_save/restore_pcie_state() */
+ val = cap[1] & PCI_EXP_LNKCTL_ASPMC;
+ if (!val)
+ return;
+
+ /*
+ * We restore L1 substate configuration first before enabling L1
+ * as the PCIe spec 6.0 sec 5.5.4 suggests.
+ */
+ pcie_restore_aspm_l1ss(pdev);
+
+ /* Re-enable L0s/L1 */
+ pcie_capability_set_word(pdev, PCI_EXP_LNKCTL, val);
+}
+
#ifdef CONFIG_PCIEASPM
#ifdef MODULE_PARAM_PREFIX
@@ -434,17 +559,6 @@ static void pcie_aspm_check_latency(struct pci_dev *endpoint)
}
}
-static void pci_clear_and_set_dword(struct pci_dev *pdev, int pos,
- u32 clear, u32 set)
-{
- u32 val;
-
- pci_read_config_dword(pdev, pos, &val);
- val &= ~clear;
- val |= set;
- pci_write_config_dword(pdev, pos, val);
-}
-
/* Calculate L1.2 PM substate timing parameters */
static void aspm_calc_l12_info(struct pcie_link_state *link,
u32 parent_l1ss_cap, u32 child_l1ss_cap)
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/5] PCI: Move pci_save/restore_ltr_state() to aspm.c
2024-01-28 23:32 [PATCH 0/5] Always build aspm.c David E. Box
` (2 preceding siblings ...)
2024-01-28 23:32 ` [PATCH 3/5] PCI/ASPM: Add back L1 PM Substate save and restore David E. Box
@ 2024-01-28 23:32 ` David E. Box
2024-01-28 23:32 ` [PATCH 5/5] PCI: Save and restore LTR state from pci_save/restore_pcie_state() David E. Box
2024-01-29 23:46 ` [PATCH 0/5] Always build aspm.c Bjorn Helgaas
5 siblings, 0 replies; 8+ messages in thread
From: David E. Box @ 2024-01-28 23:32 UTC (permalink / raw)
To: mika.westerberg, david.e.box, ilpo.jarvinen, bhelgaas, rjw
Cc: tasev.stefanoska, enriquezmark36, kernel, wse, vidyas,
kai.heng.feng, sathyanarayanan.kuppuswamy, ricky_wu,
mario.limonciello, linux-pci, linux-kernel
Since the LTR Capability is linked with ASPM and only enabled when
CONFIG_PCIEASPM is set, move the save/restore code to aspm.c
Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
drivers/pci/pci.c | 40 ----------------------------------------
drivers/pci/pci.h | 2 ++
drivers/pci/pcie/aspm.c | 40 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 42 insertions(+), 40 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 0a8613e77dab..61e56e040510 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1636,46 +1636,6 @@ static void pci_restore_pcix_state(struct pci_dev *dev)
pci_write_config_word(dev, pos + PCI_X_CMD, cap[i++]);
}
-static void pci_save_ltr_state(struct pci_dev *dev)
-{
- int ltr;
- struct pci_cap_saved_state *save_state;
- u32 *cap;
-
- if (!pci_is_pcie(dev))
- return;
-
- ltr = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_LTR);
- if (!ltr)
- return;
-
- save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_LTR);
- if (!save_state) {
- pci_err(dev, "no suspend buffer for LTR; ASPM issues possible after resume\n");
- return;
- }
-
- /* Some broken devices only support dword access to LTR */
- cap = &save_state->cap.data[0];
- pci_read_config_dword(dev, ltr + PCI_LTR_MAX_SNOOP_LAT, cap);
-}
-
-static void pci_restore_ltr_state(struct pci_dev *dev)
-{
- struct pci_cap_saved_state *save_state;
- int ltr;
- u32 *cap;
-
- save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_LTR);
- ltr = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_LTR);
- if (!save_state || !ltr)
- return;
-
- /* Some broken devices only support dword access to LTR */
- cap = &save_state->cap.data[0];
- pci_write_config_dword(dev, ltr + PCI_LTR_MAX_SNOOP_LAT, *cap);
-}
-
/**
* pci_save_state - save the PCI configuration space of a device before
* suspending
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 7b14cdbe2e69..98f54b48f013 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -100,6 +100,8 @@ void pci_bridge_d3_update(struct pci_dev *dev);
void pci_aspm_get_l1ss(struct pci_dev *pdev);
void pci_save_aspm_state(struct pci_dev *pdev);
void pci_restore_aspm_state(struct pci_dev *pdev);
+void pci_save_ltr_state(struct pci_dev *dev);
+void pci_restore_ltr_state(struct pci_dev *dev);
int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type);
static inline void pci_wakeup_event(struct pci_dev *dev)
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 3185058e9c41..f7712d8453a4 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -155,6 +155,46 @@ void pci_restore_aspm_state(struct pci_dev *pdev)
pcie_capability_set_word(pdev, PCI_EXP_LNKCTL, val);
}
+void pci_save_ltr_state(struct pci_dev *dev)
+{
+ int ltr;
+ struct pci_cap_saved_state *save_state;
+ u32 *cap;
+
+ if (!pci_is_pcie(dev))
+ return;
+
+ ltr = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_LTR);
+ if (!ltr)
+ return;
+
+ save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_LTR);
+ if (!save_state) {
+ pci_err(dev, "no suspend buffer for LTR; ASPM issues possible after resume\n");
+ return;
+ }
+
+ /* Some broken devices only support dword access to LTR */
+ cap = &save_state->cap.data[0];
+ pci_read_config_dword(dev, ltr + PCI_LTR_MAX_SNOOP_LAT, cap);
+}
+
+void pci_restore_ltr_state(struct pci_dev *dev)
+{
+ struct pci_cap_saved_state *save_state;
+ int ltr;
+ u32 *cap;
+
+ save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_LTR);
+ ltr = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_LTR);
+ if (!save_state || !ltr)
+ return;
+
+ /* Some broken devices only support dword access to LTR */
+ cap = &save_state->cap.data[0];
+ pci_write_config_dword(dev, ltr + PCI_LTR_MAX_SNOOP_LAT, *cap);
+}
+
#ifdef CONFIG_PCIEASPM
#ifdef MODULE_PARAM_PREFIX
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 5/5] PCI: Save and restore LTR state from pci_save/restore_pcie_state()
2024-01-28 23:32 [PATCH 0/5] Always build aspm.c David E. Box
` (3 preceding siblings ...)
2024-01-28 23:32 ` [PATCH 4/5] PCI: Move pci_save/restore_ltr_state() to aspm.c David E. Box
@ 2024-01-28 23:32 ` David E. Box
2024-01-29 23:46 ` [PATCH 0/5] Always build aspm.c Bjorn Helgaas
5 siblings, 0 replies; 8+ messages in thread
From: David E. Box @ 2024-01-28 23:32 UTC (permalink / raw)
To: mika.westerberg, david.e.box, ilpo.jarvinen, bhelgaas, rjw
Cc: tasev.stefanoska, enriquezmark36, kernel, wse, vidyas,
kai.heng.feng, sathyanarayanan.kuppuswamy, ricky_wu,
mario.limonciello, linux-pci, linux-kernel
ASPM state is saved and restored from pci_save/restore_pcie_state().
Since the LTR Capability is linked with ASPM, move the LTR save and
restore calls there as well.
Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
drivers/pci/pci.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 61e56e040510..78c3c9d82b3b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1561,6 +1561,7 @@ static int pci_save_pcie_state(struct pci_dev *dev)
pcie_capability_read_word(dev, PCI_EXP_SLTCTL2, &cap[i++]);
pci_save_aspm_state(dev);
+ pci_save_ltr_state(dev);
return 0;
}
@@ -1571,6 +1572,12 @@ static void pci_restore_pcie_state(struct pci_dev *dev)
struct pci_cap_saved_state *save_state;
u16 *cap, val;
+ /*
+ * Restore max latencies (in the LTR capability) before enabling
+ * LTR itself (in the PCIe capability).
+ */
+ pci_restore_ltr_state(dev);
+
save_state = pci_find_saved_cap(dev, PCI_CAP_ID_EXP);
if (!save_state)
return;
@@ -1660,7 +1667,6 @@ int pci_save_state(struct pci_dev *dev)
if (i != 0)
return i;
- pci_save_ltr_state(dev);
pci_save_dpc_state(dev);
pci_save_aer_state(dev);
pci_save_ptm_state(dev);
@@ -1761,12 +1767,6 @@ void pci_restore_state(struct pci_dev *dev)
if (!dev->state_saved)
return;
- /*
- * Restore max latencies (in the LTR capability) before enabling
- * LTR itself (in the PCIe capability).
- */
- pci_restore_ltr_state(dev);
-
pci_restore_pcie_state(dev);
pci_restore_pasid_state(dev);
pci_restore_pri_state(dev);
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 0/5] Always build aspm.c
2024-01-28 23:32 [PATCH 0/5] Always build aspm.c David E. Box
` (4 preceding siblings ...)
2024-01-28 23:32 ` [PATCH 5/5] PCI: Save and restore LTR state from pci_save/restore_pcie_state() David E. Box
@ 2024-01-29 23:46 ` Bjorn Helgaas
2024-02-23 20:35 ` Bjorn Helgaas
5 siblings, 1 reply; 8+ messages in thread
From: Bjorn Helgaas @ 2024-01-29 23:46 UTC (permalink / raw)
To: David E. Box
Cc: mika.westerberg, ilpo.jarvinen, bhelgaas, rjw, tasev.stefanoska,
enriquezmark36, kernel, wse, vidyas, kai.heng.feng,
sathyanarayanan.kuppuswamy, ricky_wu, mario.limonciello,
linux-pci, linux-kernel
On Sun, Jan 28, 2024 at 03:32:07PM -0800, David E. Box wrote:
> Here is the series to always build aspm.c, add back L1SS save/restore, and
> consolidate all ASPM related code in aspm.c.
>
> Patch 1 - Moves all PCI core functions running under CONFIG_PCIEASPM into
> aspm.c and changes they Makefile to always build it. No functional changes.
>
> Patch 2 - Creates a separate function to save the L1SS offset and places it
> outside of CONFIG_PCIEASPM in aspm.c so that the offset is available for
> later use by the L1SS save/restore code which needs to run when
> CONFIG_PCIEASPM=n.
>
> Patch 3 - Updated L1 Substate save/restore patch from previous V5 [1].
>
> Patch 4 - Moves the LTR save/restore state functions into aspm.c outside of
> CONFIG_PCIEASPM.
>
> Patch 5 - Moves the LTR save/restore state calls into
> pci_save/restore_pcie_state().
>
> The series does not continue any of the ASPM robustness changes proposed by
> Ilpo [2]. But if we think it's worth combining with this series I can
> add it and help continue the work.
>
> [1] https://lore.kernel.org/linux-pci/20231221011250.191599-1-david.e.box@linux.intel.com/
> [2] https://lore.kernel.org/linux-pci/20230918131103.24119-1-ilpo.jarvinen@linux.intel.com/
>
> David E. Box (5):
> PCI: Always build aspm.c
> PCI: Create function to save L1SS offset
> PCI/ASPM: Add back L1 PM Substate save and restore
> PCI: Move pci_save/restore_ltr_state() to aspm.c
> PCI: Call LTR save/restore state from PCIe save/restore
>
> drivers/pci/pci.c | 91 ++++----------
> drivers/pci/pci.h | 10 +-
> drivers/pci/pcie/Makefile | 2 +-
> drivers/pci/pcie/aspm.c | 257 ++++++++++++++++++++++++++++++++++++--
> drivers/pci/probe.c | 62 +--------
> include/linux/pci.h | 4 +-
> 6 files changed, 283 insertions(+), 143 deletions(-)
>
>
> base-commit: 0dd3ee31125508cd67f7e7172247f05b7fd1753a
Rebased to v6.8-rc1 and applied to pci/aspm for v6.9, thanks!
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/5] Always build aspm.c
2024-01-29 23:46 ` [PATCH 0/5] Always build aspm.c Bjorn Helgaas
@ 2024-02-23 20:35 ` Bjorn Helgaas
0 siblings, 0 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2024-02-23 20:35 UTC (permalink / raw)
To: David E. Box
Cc: mika.westerberg, ilpo.jarvinen, bhelgaas, rjw, tasev.stefanoska,
enriquezmark36, kernel, wse, vidyas, kai.heng.feng,
sathyanarayanan.kuppuswamy, ricky_wu, mario.limonciello,
linux-pci, linux-kernel
On Mon, Jan 29, 2024 at 05:46:27PM -0600, Bjorn Helgaas wrote:
> On Sun, Jan 28, 2024 at 03:32:07PM -0800, David E. Box wrote:
> > Here is the series to always build aspm.c, add back L1SS save/restore, and
> > consolidate all ASPM related code in aspm.c.
> >
> > Patch 1 - Moves all PCI core functions running under CONFIG_PCIEASPM into
> > aspm.c and changes they Makefile to always build it. No functional changes.
> >
> > Patch 2 - Creates a separate function to save the L1SS offset and places it
> > outside of CONFIG_PCIEASPM in aspm.c so that the offset is available for
> > later use by the L1SS save/restore code which needs to run when
> > CONFIG_PCIEASPM=n.
> >
> > Patch 3 - Updated L1 Substate save/restore patch from previous V5 [1].
> >
> > Patch 4 - Moves the LTR save/restore state functions into aspm.c outside of
> > CONFIG_PCIEASPM.
> >
> > Patch 5 - Moves the LTR save/restore state calls into
> > pci_save/restore_pcie_state().
> >
> > The series does not continue any of the ASPM robustness changes proposed by
> > Ilpo [2]. But if we think it's worth combining with this series I can
> > add it and help continue the work.
> >
> > [1] https://lore.kernel.org/linux-pci/20231221011250.191599-1-david.e.box@linux.intel.com/
> > [2] https://lore.kernel.org/linux-pci/20230918131103.24119-1-ilpo.jarvinen@linux.intel.com/
> >
> > David E. Box (5):
> > PCI: Always build aspm.c
> > PCI: Create function to save L1SS offset
> > PCI/ASPM: Add back L1 PM Substate save and restore
> > PCI: Move pci_save/restore_ltr_state() to aspm.c
> > PCI: Call LTR save/restore state from PCIe save/restore
> >
> > drivers/pci/pci.c | 91 ++++----------
> > drivers/pci/pci.h | 10 +-
> > drivers/pci/pcie/Makefile | 2 +-
> > drivers/pci/pcie/aspm.c | 257 ++++++++++++++++++++++++++++++++++++--
> > drivers/pci/probe.c | 62 +--------
> > include/linux/pci.h | 4 +-
> > 6 files changed, 283 insertions(+), 143 deletions(-)
> >
> >
> > base-commit: 0dd3ee31125508cd67f7e7172247f05b7fd1753a
>
> Rebased to v6.8-rc1 and applied to pci/aspm for v6.9, thanks!
I reworked this to accommodate Vidya's "Update saved buffers with
latest ASPM configuration" patch:
https://lore.kernel.org/r/20230125133830.20620-1-vidyas@nvidia.com
It ended up being more work than I expected. The interdiff is below,
and I put this and Vidya's patch on a pci/aspm-rework branch
temporarily so we can take a look before saying it's done:
https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=aspm-rework
The main changes are:
- Rename pci_save_aspm_state() to pci_save_aspm_l1ss_state() (this
is the main thing for Vidya's patch).
- Rename pcie_restore_aspm_l1ss() to pci_restore_aspm_l1ss_state()
to match.
- Move the PCI_EXP_LNKCTL_ASPMC from pci_restore_aspm_state() to
pci_restore_pcie_state() so both writes are in the same place.
- Rename pci_aspm_get_l1ss() to pci_configure_aspm_l1ss() and add
the save_buffer there as well.
- Split [1/5] into two patches: move pci_configure_ltr() and
pci_bridge_reconfigure_ltr() to aspm.c, and build aspm.c
unconditionally.
- Squash [2/5] and [3/5] since [2/5] didn't add any functionality
itself so they seem like a single logical change.
Let me know if you spot anything that I broke.
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 00139fad1827..4ea98665172d 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1623,7 +1623,7 @@ static int pci_save_pcie_state(struct pci_dev *dev)
pcie_capability_read_word(dev, PCI_EXP_LNKCTL2, &cap[i++]);
pcie_capability_read_word(dev, PCI_EXP_SLTCTL2, &cap[i++]);
- pci_save_aspm_state(dev);
+ pci_save_aspm_l1ss_state(dev);
pci_save_ltr_state(dev);
return 0;
@@ -1633,11 +1633,11 @@ static void pci_restore_pcie_state(struct pci_dev *dev)
{
int i = 0;
struct pci_cap_saved_state *save_state;
- u16 *cap, val;
+ u16 *cap, lnkctl;
/*
* Restore max latencies (in the LTR capability) before enabling
- * LTR itself (in the PCIe capability).
+ * LTR itself in PCI_EXP_DEVCTL2.
*/
pci_restore_ltr_state(dev);
@@ -1655,19 +1655,22 @@ static void pci_restore_pcie_state(struct pci_dev *dev)
cap = (u16 *)&save_state->cap.data[0];
pcie_capability_write_word(dev, PCI_EXP_DEVCTL, cap[i++]);
- /*
- * Restore only the LNKCTL register with the ASPM control field
- * clear. ASPM will be restored in pci_restore_aspm_state().
- */
- val = cap[i++] & ~PCI_EXP_LNKCTL_ASPMC;
- pcie_capability_write_word(dev, PCI_EXP_LNKCTL, val);
+ /* Restore LNKCTL register with ASPM control field clear */
+ lnkctl = cap[i++];
+ pcie_capability_write_word(dev, PCI_EXP_LNKCTL,
+ lnkctl & ~PCI_EXP_LNKCTL_ASPMC);
+
pcie_capability_write_word(dev, PCI_EXP_SLTCTL, cap[i++]);
pcie_capability_write_word(dev, PCI_EXP_RTCTL, cap[i++]);
pcie_capability_write_word(dev, PCI_EXP_DEVCTL2, cap[i++]);
pcie_capability_write_word(dev, PCI_EXP_LNKCTL2, cap[i++]);
pcie_capability_write_word(dev, PCI_EXP_SLTCTL2, cap[i++]);
- pci_restore_aspm_state(dev);
+ pci_restore_aspm_l1ss_state(dev);
+
+ /* Restore ASPM control after restoring L1SS state */
+ pcie_capability_set_word(dev, PCI_EXP_LNKCTL,
+ lnkctl & PCI_EXP_LNKCTL_ASPMC);
}
static int pci_save_pcix_state(struct pci_dev *dev)
@@ -3532,11 +3535,6 @@ 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 ecceb690fbbb..eca5938deb07 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -97,11 +97,6 @@ void pci_msi_init(struct pci_dev *dev);
void pci_msix_init(struct pci_dev *dev);
bool pci_bridge_d3_possible(struct pci_dev *dev);
void pci_bridge_d3_update(struct pci_dev *dev);
-void pci_aspm_get_l1ss(struct pci_dev *pdev);
-void pci_save_aspm_state(struct pci_dev *pdev);
-void pci_restore_aspm_state(struct pci_dev *pdev);
-void pci_save_ltr_state(struct pci_dev *dev);
-void pci_restore_ltr_state(struct pci_dev *dev);
int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type);
static inline void pci_wakeup_event(struct pci_dev *dev)
@@ -572,6 +567,14 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
bool pcie_wait_for_link(struct pci_dev *pdev, bool active);
int pcie_retrain_link(struct pci_dev *pdev, bool use_lt);
+
+/* ASPM-related functionality we need even without CONFIG_PCIEASPM */
+void pci_save_ltr_state(struct pci_dev *dev);
+void pci_restore_ltr_state(struct pci_dev *dev);
+void pci_configure_aspm_l1ss(struct pci_dev *dev);
+void pci_save_aspm_l1ss_state(struct pci_dev *dev);
+void pci_restore_aspm_l1ss_state(struct pci_dev *dev);
+
#ifdef CONFIG_PCIEASPM
void pcie_aspm_init_link_state(struct pci_dev *pdev);
void pcie_aspm_exit_link_state(struct pci_dev *pdev);
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 60716fbf40a9..977eca893b2a 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -24,126 +24,6 @@
#include "../pci.h"
-void pci_aspm_get_l1ss(struct pci_dev *pdev)
-{
- /* Read L1 PM substate capabilities */
- pdev->l1ss = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_L1SS);
-}
-
-void pci_save_aspm_state(struct pci_dev *pdev)
-{
- struct pci_cap_saved_state *save_state;
- u16 l1ss = pdev->l1ss;
- u32 *cap;
-
- /*
- * Save L1 substate configuration. The ASPM L0s/L1 configuration
- * is already saved in pci_save_pcie_state().
- */
- if (!l1ss)
- return;
-
- save_state = pci_find_saved_ext_cap(pdev, PCI_EXT_CAP_ID_L1SS);
- if (!save_state)
- return;
-
- cap = &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++);
-}
-
-static void pcie_restore_aspm_l1ss(struct pci_dev *pdev)
-{
- struct pci_cap_saved_state *pl_save_state, *cl_save_state;
- struct pci_dev *parent = pdev->bus->self;
- u32 *cap, pl_ctl1, pl_ctl2, pl_l1_2_enable;
- u32 cl_ctl1, cl_ctl2, cl_l1_2_enable;
-
- /*
- * In case BIOS enabled L1.2 after resume, we need to disable it first
- * on the downstream component before the upstream. So, don't attempt to
- * restore either until we are at the downstream component.
- */
- if (pcie_downstream_port(pdev) || !parent)
- return;
-
- if (!pdev->l1ss || !parent->l1ss)
- return;
-
- cl_save_state = pci_find_saved_ext_cap(pdev, PCI_EXT_CAP_ID_L1SS);
- pl_save_state = pci_find_saved_ext_cap(parent, PCI_EXT_CAP_ID_L1SS);
- if (!cl_save_state || !pl_save_state)
- return;
-
- cap = &cl_save_state->cap.data[0];
- cl_ctl2 = *cap++;
- cl_ctl1 = *cap;
- cap = &pl_save_state->cap.data[0];
- pl_ctl2 = *cap++;
- pl_ctl1 = *cap;
-
-
- /*
- * Disable L1.2 on this downstream endpoint device first, followed
- * by the upstream
- */
- pci_clear_and_set_config_dword(pdev, pdev->l1ss + PCI_L1SS_CTL1,
- PCI_L1SS_CTL1_L1_2_MASK, 0);
- pci_clear_and_set_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1,
- PCI_L1SS_CTL1_L1_2_MASK, 0);
-
- /*
- * 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.
- */
- pl_l1_2_enable = pl_ctl1 & PCI_L1SS_CTL1_L1_2_MASK;
- pl_ctl1 &= ~PCI_L1SS_CTL1_L1_2_MASK;
- cl_l1_2_enable = cl_ctl1 & PCI_L1SS_CTL1_L1_2_MASK;
- cl_ctl1 &= ~PCI_L1SS_CTL1_L1_2_MASK;
-
- /* Write back without enables first (above we cleared them in ctl1) */
- pci_write_config_dword(parent, parent->l1ss + PCI_L1SS_CTL2, pl_ctl2);
- pci_write_config_dword(pdev, pdev->l1ss + PCI_L1SS_CTL2, cl_ctl2);
- pci_write_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1, pl_ctl1);
- pci_write_config_dword(pdev, pdev->l1ss + PCI_L1SS_CTL1, cl_ctl1);
-
-
- /* Then write back the enables */
- if (pl_l1_2_enable || cl_l1_2_enable) {
- pci_write_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1,
- pl_ctl1 | pl_l1_2_enable);
- pci_write_config_dword(pdev, pdev->l1ss + PCI_L1SS_CTL1,
- cl_ctl1 | cl_l1_2_enable);
- }
-}
-
-void pci_restore_aspm_state(struct pci_dev *pdev)
-{
- struct pci_cap_saved_state *save_state;
- u16 *cap, val;
-
- save_state = pci_find_saved_cap(pdev, PCI_CAP_ID_EXP);
-
- if (!save_state)
- return;
-
- cap = (u16 *)&save_state->cap.data[0];
- /* Must match the ordering in pci_save/restore_pcie_state() */
- val = cap[1] & PCI_EXP_LNKCTL_ASPMC;
- if (!val)
- return;
-
- /*
- * We restore L1 substate configuration first before enabling L1
- * as the PCIe spec 6.1 sec 5.5.4 suggests.
- */
- pcie_restore_aspm_l1ss(pdev);
-
- /* Re-enable L0s/L1 */
- pcie_capability_set_word(pdev, PCI_EXP_LNKCTL, val);
-}
-
void pci_save_ltr_state(struct pci_dev *dev)
{
int ltr;
@@ -184,6 +64,105 @@ void pci_restore_ltr_state(struct pci_dev *dev)
pci_write_config_dword(dev, ltr + PCI_LTR_MAX_SNOOP_LAT, *cap);
}
+void pci_configure_aspm_l1ss(struct pci_dev *pdev)
+{
+ int rc;
+
+ pdev->l1ss = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_L1SS);
+
+ rc = pci_add_ext_cap_save_buffer(pdev, PCI_EXT_CAP_ID_L1SS,
+ 2 * sizeof(u32));
+ if (rc)
+ pci_err(pdev, "unable to allocate ASPM L1SS save buffer (%pe)\n",
+ ERR_PTR(rc));
+}
+
+void pci_save_aspm_l1ss_state(struct pci_dev *pdev)
+{
+ struct pci_cap_saved_state *save_state;
+ u16 l1ss = pdev->l1ss;
+ u32 *cap;
+
+ /*
+ * Save L1 substate configuration. The ASPM L0s/L1 configuration
+ * in PCI_EXP_LNKCTL_ASPMC is saved by pci_save_pcie_state().
+ */
+ if (!l1ss)
+ return;
+
+ save_state = pci_find_saved_ext_cap(pdev, PCI_EXT_CAP_ID_L1SS);
+ if (!save_state)
+ return;
+
+ cap = &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++);
+}
+
+void pci_restore_aspm_l1ss_state(struct pci_dev *pdev)
+{
+ struct pci_cap_saved_state *pl_save_state, *cl_save_state;
+ struct pci_dev *parent = pdev->bus->self;
+ u32 *cap, pl_ctl1, pl_ctl2, pl_l1_2_enable;
+ u32 cl_ctl1, cl_ctl2, cl_l1_2_enable;
+
+ /*
+ * In case BIOS enabled L1.2 when resuming, we need to disable it first
+ * on the downstream component before the upstream. So, don't attempt to
+ * restore either until we are at the downstream component.
+ */
+ if (pcie_downstream_port(pdev) || !parent)
+ return;
+
+ if (!pdev->l1ss || !parent->l1ss)
+ return;
+
+ cl_save_state = pci_find_saved_ext_cap(pdev, PCI_EXT_CAP_ID_L1SS);
+ pl_save_state = pci_find_saved_ext_cap(parent, PCI_EXT_CAP_ID_L1SS);
+ if (!cl_save_state || !pl_save_state)
+ return;
+
+ cap = &cl_save_state->cap.data[0];
+ cl_ctl2 = *cap++;
+ cl_ctl1 = *cap;
+ cap = &pl_save_state->cap.data[0];
+ pl_ctl2 = *cap++;
+ pl_ctl1 = *cap;
+
+ /*
+ * Disable L1.2 on this downstream endpoint device first, followed
+ * by the upstream
+ */
+ pci_clear_and_set_config_dword(pdev, pdev->l1ss + PCI_L1SS_CTL1,
+ PCI_L1SS_CTL1_L1_2_MASK, 0);
+ pci_clear_and_set_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1,
+ PCI_L1SS_CTL1_L1_2_MASK, 0);
+
+ /*
+ * 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.
+ */
+ pl_l1_2_enable = pl_ctl1 & PCI_L1SS_CTL1_L1_2_MASK;
+ pl_ctl1 &= ~PCI_L1SS_CTL1_L1_2_MASK;
+ cl_l1_2_enable = cl_ctl1 & PCI_L1SS_CTL1_L1_2_MASK;
+ cl_ctl1 &= ~PCI_L1SS_CTL1_L1_2_MASK;
+
+ /* Write back without enables first (above we cleared them in ctl1) */
+ pci_write_config_dword(parent, parent->l1ss + PCI_L1SS_CTL2, pl_ctl2);
+ pci_write_config_dword(pdev, pdev->l1ss + PCI_L1SS_CTL2, cl_ctl2);
+ pci_write_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1, pl_ctl1);
+ pci_write_config_dword(pdev, pdev->l1ss + PCI_L1SS_CTL1, cl_ctl1);
+
+ /* Then write back the enables */
+ if (pl_l1_2_enable || cl_l1_2_enable) {
+ pci_write_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1,
+ pl_ctl1 | pl_l1_2_enable);
+ pci_write_config_dword(pdev, pdev->l1ss + PCI_L1SS_CTL1,
+ cl_ctl1 | cl_l1_2_enable);
+ }
+}
+
#ifdef CONFIG_PCIEASPM
#ifdef MODULE_PARAM_PREFIX
@@ -1676,4 +1655,5 @@ bool pcie_aspm_support_enabled(void)
{
return aspm_support_enabled;
}
+
#endif /* CONFIG_PCIEASPM */
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index b5ccf5a16dc1..1434bf495db3 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2258,8 +2258,8 @@ static void pci_configure_device(struct pci_dev *dev)
pci_configure_mps(dev);
pci_configure_extended_tags(dev, NULL);
pci_configure_relaxed_ordering(dev);
- pci_aspm_get_l1ss(dev);
pci_configure_ltr(dev);
+ pci_configure_aspm_l1ss(dev);
pci_configure_eetlp_prefix(dev);
pci_configure_serr(dev);
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-02-23 20:35 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-28 23:32 [PATCH 0/5] Always build aspm.c David E. Box
2024-01-28 23:32 ` [PATCH 1/5] PCI: " David E. Box
2024-01-28 23:32 ` [PATCH 2/5] PCI: Create function to save L1SS offset David E. Box
2024-01-28 23:32 ` [PATCH 3/5] PCI/ASPM: Add back L1 PM Substate save and restore David E. Box
2024-01-28 23:32 ` [PATCH 4/5] PCI: Move pci_save/restore_ltr_state() to aspm.c David E. Box
2024-01-28 23:32 ` [PATCH 5/5] PCI: Save and restore LTR state from pci_save/restore_pcie_state() David E. Box
2024-01-29 23:46 ` [PATCH 0/5] Always build aspm.c Bjorn Helgaas
2024-02-23 20:35 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).