From 64dbb2d707444f691539fb12aacf81797786c10b Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Tue, 5 Mar 2024 15:15:25 -0600 Subject: [PATCH] PCI/ASPM: Disable L1 before configuring L1 Substates Per PCIe r6.1, sec 5.5.4, L1 must be disabled while setting ASPM L1 PM Substates enable bits. Previously this was enforced by clearing PCI_EXP_LNKCTL_ASPMC before calling pci_restore_aspm_l1ss_state(). Move the L1 (and L0s, although that doesn't seem required) disable into pci_restore_aspm_l1ss_state() itself so it's closer to the code that depends on it. Link: https://lore.kernel.org/r/20240223213733.GA115410@bhelgaas Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 16 +++------------- drivers/pci/pcie/aspm.c | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 4ea98665172d..5a4b501a3f41 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1633,13 +1633,14 @@ static void pci_restore_pcie_state(struct pci_dev *dev) { int i = 0; struct pci_cap_saved_state *save_state; - u16 *cap, lnkctl; + u16 *cap; /* * Restore max latencies (in the LTR capability) before enabling * LTR itself in PCI_EXP_DEVCTL2. */ pci_restore_ltr_state(dev); + pci_restore_aspm_l1ss_state(dev); save_state = pci_find_saved_cap(dev, PCI_CAP_ID_EXP); if (!save_state) @@ -1654,23 +1655,12 @@ 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 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_LNKCTL, cap[i++]); 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_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) diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index 977eca893b2a..1379b8decdf1 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -105,6 +105,7 @@ void pci_restore_aspm_l1ss_state(struct pci_dev *pdev) 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; + u16 clnkctl, plnkctl; /* * In case BIOS enabled L1.2 when resuming, we need to disable it first @@ -129,6 +130,17 @@ void pci_restore_aspm_l1ss_state(struct pci_dev *pdev) pl_ctl2 = *cap++; pl_ctl1 = *cap; + /* Make sure L0s/L1 are disabled before updating L1SS config */ + pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &clnkctl); + pcie_capability_read_word(parent, PCI_EXP_LNKCTL, &plnkctl); + if (FIELD_GET(PCI_EXP_LNKCTL_ASPMC, clnkctl) || + FIELD_GET(PCI_EXP_LNKCTL_ASPMC, plnkctl)) { + pcie_capability_write_word(pdev, PCI_EXP_LNKCTL, + clnkctl & ~PCI_EXP_LNKCTL_ASPMC); + pcie_capability_write_word(parent, PCI_EXP_LNKCTL, + plnkctl & ~PCI_EXP_LNKCTL_ASPMC); + } + /* * Disable L1.2 on this downstream endpoint device first, followed * by the upstream @@ -161,6 +173,13 @@ void pci_restore_aspm_l1ss_state(struct pci_dev *pdev) pci_write_config_dword(pdev, pdev->l1ss + PCI_L1SS_CTL1, cl_ctl1 | cl_l1_2_enable); } + + /* Restore L0s/L1 if they were enabled */ + if (FIELD_GET(PCI_EXP_LNKCTL_ASPMC, clnkctl) || + FIELD_GET(PCI_EXP_LNKCTL_ASPMC, plnkctl)) { + pcie_capability_write_word(parent, PCI_EXP_LNKCTL, clnkctl); + pcie_capability_write_word(pdev, PCI_EXP_LNKCTL, plnkctl); + } } #ifdef CONFIG_PCIEASPM