Revert "PCI/ASPM: Save L1 PM Substates Capability for suspend/resume"

commit 5c168d03e9f875e7d1162b83e577da9f9ea568a8 upstream.

This reverts commit efcd698d7e52737575b5ce9c51d9ea0d4f6a8b22.

Tasev Nikola and Mark Enriquez reported that resume from suspend was broken
in v6.1-rc1.  Tasev bisected to 98c9c12c8ad7 ("PCI/PTM: Cache PTM
Capability offset"), but we can't figure out how that could be related.

Mark saw the same symptoms and bisected to efcd698d7e52 ("PCI/ASPM: Save L1
PM Substates Capability for suspend/resume"), which does have a connection:
it restores L1 Substates configuration while ASPM L1 may be enabled:

  pci_restore_state
    pci_restore_aspm_l1ss_state
      aspm_program_l1ss
        pci_write_config_dword(PCI_L1SS_CTL1, ctl1)         # L1SS restore
    pci_restore_pcie_state
      pcie_capability_write_word(PCI_EXP_LNKCTL, cap[i++])  # L1 restore

which is a problem because PCIe r6.0, sec 5.5.4, requires that:

  If setting either or both of the enable bits for ASPM L1 PM
  Substates, both ports must be configured as described in this
  section while ASPM L1 is disabled.

Separately, Thomas Witt reported that 79181434adc3 ("PCI/ASPM: Refactor L1
PM Substates Control Register programming") broke suspend/resume, and it
depends on efcd698d7e52.

Revert efcd698d7e52 ("PCI/ASPM: Save L1 PM Substates Capability for
suspend/resume") to fix the resume issue and enable revert of 79181434adc3
to fix the issue Thomas reported.

Note that reverting efcd698d7e52 means L1 Substates config may be lost on
suspend/resume.  As far as we know the system will use more power but will
still *work* correctly.

Fixes: efcd698d7e52 ("PCI/ASPM: Save L1 PM Substates Capability for suspend/resume")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=216782
Link: https://bugzilla.kernel.org/show_bug.cgi?id=216877
Reported-by: Tasev Nikola <tasev.stefanoska@skynet.be>
Reported-by: Mark Enriquez <enriquezmark36@gmail.com>
Reported-by: Thomas Witt <kernel@witt.link>
Tested-by: Mark Enriquez <enriquezmark36@gmail.com>
Tested-by: Thomas Witt <kernel@witt.link>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Cc: stable@vger.kernel.org	# v6.1+
Cc: Vidya Sagar <vidyas@nvidia.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index ab615ab4e44097119ae451ee2608ebd11e090244..6d81df459b2f075da6b306631afc916f87e34ac7 100644 (file)
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1665,7 +1665,6 @@ int pci_save_state(struct pci_dev *dev)
                return i;
 
        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);
@@ -1772,7 +1771,6 @@ void pci_restore_state(struct pci_dev *dev)
         * LTR itself (in the PCIe capability).
         */
        pci_restore_ltr_state(dev);
-       pci_restore_aspm_l1ss_state(dev);
 
        pci_restore_pcie_state(dev);
        pci_restore_pasid_state(dev);
@@ -3465,11 +3463,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 b1ebb7ab8805170d224b15b292be52b092090457..ce169b12a8f6d658b9dd2d684de124606d19bcf6 100644 (file)
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -565,14 +565,10 @@ 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 *dev);
-void pci_restore_aspm_l1ss_state(struct pci_dev *dev);
 #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 *dev) { }
-static inline void pci_restore_aspm_l1ss_state(struct pci_dev *dev) { }
 #endif
 
 #ifdef CONFIG_PCIE_ECRC

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 53a1fa306e1ee7b70f20581dbca928792dbe6384..915cbd939dd9c2a46ad73922ce6125d3ccd32a97 100644 (file)
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -757,43 +757,6 @@ 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 *dev)
-{
-       struct pci_cap_saved_state *save_state;
-       u16 l1ss = dev->l1ss;
-       u32 *cap;
-
-       if (!l1ss)
-               return;
-
-       save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_L1SS);
-       if (!save_state)
-               return;
-
-       cap = (u32 *)&save_state->cap.data[0];
-       pci_read_config_dword(dev, l1ss + PCI_L1SS_CTL2, cap++);
-       pci_read_config_dword(dev, l1ss + PCI_L1SS_CTL1, cap++);
-}
-
-void pci_restore_aspm_l1ss_state(struct pci_dev *dev)
-{
-       struct pci_cap_saved_state *save_state;
-       u32 *cap, ctl1, ctl2;
-       u16 l1ss = dev->l1ss;
-
-       if (!l1ss)
-               return;
-
-       save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_L1SS);
-       if (!save_state)
-               return;
-
-       cap = (u32 *)&save_state->cap.data[0];
-       ctl2 = *cap++;
-       ctl1 = *cap;
-       aspm_program_l1ss(dev, ctl1, ctl2);
-}
-
 static void pcie_config_aspm_dev(struct pci_dev *pdev, u32 val)
 {
        pcie_capability_clear_and_set_word(pdev, PCI_EXP_LNKCTL,