Skip to content

Commit a7152be

Browse files
committed
Revert "PCI/ASPM: Save L1 PM Substates Capability for suspend/resume"
This reverts commit 4ff116d. Tasev Nikola and Mark Enriquez reported that resume from suspend was broken in v6.1-rc1. Tasev bisected to a47126e ("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 4ff116d ("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 5e85eba ("PCI/ASPM: Refactor L1 PM Substates Control Register programming") broke suspend/resume, and it depends on 4ff116d. Revert 4ff116d ("PCI/ASPM: Save L1 PM Substates Capability for suspend/resume") to fix the resume issue and enable revert of 5e85eba to fix the issue Thomas reported. Note that reverting 4ff116d 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: 4ff116d ("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 <[email protected]> Reported-by: Mark Enriquez <[email protected]> Reported-by: Thomas Witt <[email protected]> Tested-by: Mark Enriquez <[email protected]> Tested-by: Thomas Witt <[email protected]> Signed-off-by: Bjorn Helgaas <[email protected]> Cc: [email protected] # v6.1+ Cc: Vidya Sagar <[email protected]>
1 parent 581e43e commit a7152be

File tree

3 files changed

+0
-48
lines changed

3 files changed

+0
-48
lines changed

drivers/pci/pci.c

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1665,7 +1665,6 @@ int pci_save_state(struct pci_dev *dev)
16651665
return i;
16661666

16671667
pci_save_ltr_state(dev);
1668-
pci_save_aspm_l1ss_state(dev);
16691668
pci_save_dpc_state(dev);
16701669
pci_save_aer_state(dev);
16711670
pci_save_ptm_state(dev);
@@ -1772,7 +1771,6 @@ void pci_restore_state(struct pci_dev *dev)
17721771
* LTR itself (in the PCIe capability).
17731772
*/
17741773
pci_restore_ltr_state(dev);
1775-
pci_restore_aspm_l1ss_state(dev);
17761774

17771775
pci_restore_pcie_state(dev);
17781776
pci_restore_pasid_state(dev);
@@ -3465,11 +3463,6 @@ void pci_allocate_cap_save_buffers(struct pci_dev *dev)
34653463
if (error)
34663464
pci_err(dev, "unable to allocate suspend buffer for LTR\n");
34673465

3468-
error = pci_add_ext_cap_save_buffer(dev, PCI_EXT_CAP_ID_L1SS,
3469-
2 * sizeof(u32));
3470-
if (error)
3471-
pci_err(dev, "unable to allocate suspend buffer for ASPM-L1SS\n");
3472-
34733466
pci_allocate_vc_save_buffers(dev);
34743467
}
34753468

drivers/pci/pci.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -566,14 +566,10 @@ bool pcie_wait_for_link(struct pci_dev *pdev, bool active);
566566
void pcie_aspm_init_link_state(struct pci_dev *pdev);
567567
void pcie_aspm_exit_link_state(struct pci_dev *pdev);
568568
void pcie_aspm_powersave_config_link(struct pci_dev *pdev);
569-
void pci_save_aspm_l1ss_state(struct pci_dev *dev);
570-
void pci_restore_aspm_l1ss_state(struct pci_dev *dev);
571569
#else
572570
static inline void pcie_aspm_init_link_state(struct pci_dev *pdev) { }
573571
static inline void pcie_aspm_exit_link_state(struct pci_dev *pdev) { }
574572
static inline void pcie_aspm_powersave_config_link(struct pci_dev *pdev) { }
575-
static inline void pci_save_aspm_l1ss_state(struct pci_dev *dev) { }
576-
static inline void pci_restore_aspm_l1ss_state(struct pci_dev *dev) { }
577573
#endif
578574

579575
#ifdef CONFIG_PCIE_ECRC

drivers/pci/pcie/aspm.c

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -757,43 +757,6 @@ static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state)
757757
PCI_L1SS_CTL1_L1SS_MASK, val);
758758
}
759759

760-
void pci_save_aspm_l1ss_state(struct pci_dev *dev)
761-
{
762-
struct pci_cap_saved_state *save_state;
763-
u16 l1ss = dev->l1ss;
764-
u32 *cap;
765-
766-
if (!l1ss)
767-
return;
768-
769-
save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_L1SS);
770-
if (!save_state)
771-
return;
772-
773-
cap = (u32 *)&save_state->cap.data[0];
774-
pci_read_config_dword(dev, l1ss + PCI_L1SS_CTL2, cap++);
775-
pci_read_config_dword(dev, l1ss + PCI_L1SS_CTL1, cap++);
776-
}
777-
778-
void pci_restore_aspm_l1ss_state(struct pci_dev *dev)
779-
{
780-
struct pci_cap_saved_state *save_state;
781-
u32 *cap, ctl1, ctl2;
782-
u16 l1ss = dev->l1ss;
783-
784-
if (!l1ss)
785-
return;
786-
787-
save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_L1SS);
788-
if (!save_state)
789-
return;
790-
791-
cap = (u32 *)&save_state->cap.data[0];
792-
ctl2 = *cap++;
793-
ctl1 = *cap;
794-
aspm_program_l1ss(dev, ctl1, ctl2);
795-
}
796-
797760
static void pcie_config_aspm_dev(struct pci_dev *pdev, u32 val)
798761
{
799762
pcie_capability_clear_and_set_word(pdev, PCI_EXP_LNKCTL,

0 commit comments

Comments
 (0)