On 1/2/2024 3:25 PM, Bjorn Helgaas wrote:
From: Bjorn Helgaas bhelgaas@google.com
This reverts commit 08d0cc5f34265d1a1e3031f319f594bd1970976c.
Michael reported that when attempting to resume from suspend to RAM on ASUS mini PC PN51-BB757MDE1 (DMI model: MINIPC PN51-E1), 08d0cc5f3426 ("PCI/ASPM: Remove pcie_aspm_pm_state_change()") caused a 12-second delay with no output, followed by a reboot.
Workarounds include:
- Reverting 08d0cc5f3426 ("PCI/ASPM: Remove pcie_aspm_pm_state_change()")
- Booting with "pcie_aspm=off"
- Booting with "pcie_aspm.policy=performance"
- "echo 0 | sudo tee /sys/bus/pci/devices/0000:03:00.0/link/l1_aspm" before suspending
- Connecting a USB flash drive
Did you find the root cause? Is this issue specific to that particular device? If yes, can we do a quirk?
Fixes: 08d0cc5f3426 ("PCI/ASPM: Remove pcie_aspm_pm_state_change()") Reported-by: Michael Schaller michael@5challer.de Link: https://lore.kernel.org/r/76c61361-b8b4-435f-a9f1-32b716763d62@5challer.de Signed-off-by: Bjorn Helgaas bhelgaas@google.com Cc: stable@vger.kernel.org
drivers/pci/pci.c | 6 ++++++ drivers/pci/pci.h | 2 ++ drivers/pci/pcie/aspm.c | 19 +++++++++++++++++++ 3 files changed, 27 insertions(+)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 55bc3576a985..bdbf8a94b4d0 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1335,6 +1335,9 @@ static int pci_set_full_power_state(struct pci_dev *dev) pci_restore_bars(dev); }
- if (dev->bus->self)
pcie_aspm_pm_state_change(dev->bus->self);- return 0;
} @@ -1429,6 +1432,9 @@ static int pci_set_low_power_state(struct pci_dev *dev, pci_power_t state) pci_power_name(dev->current_state), pci_power_name(state));
- if (dev->bus->self)
pcie_aspm_pm_state_change(dev->bus->self);- return 0;
} diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 5ecbcf041179..f43873049d52 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -569,10 +569,12 @@ int pcie_retrain_link(struct pci_dev *pdev, bool use_lt); #ifdef CONFIG_PCIEASPM 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); #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) { } #endif diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index 50b04ae5c394..8715e951c491 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -1008,6 +1008,25 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev) up_read(&pci_bus_sem); } +/* @pdev: the root port or switch downstream port */ +void pcie_aspm_pm_state_change(struct pci_dev *pdev) +{
- struct pcie_link_state *link = pdev->link_state;
- if (aspm_disabled || !link)
return;- /*
* Devices changed PM state, we should recheck if latency* meets all functions' requirement*/- down_read(&pci_bus_sem);
- mutex_lock(&aspm_lock);
- pcie_update_aspm_capable(link->root);
- pcie_config_aspm_path(link);
- mutex_unlock(&aspm_lock);
- up_read(&pci_bus_sem);
+}
void pcie_aspm_powersave_config_link(struct pci_dev *pdev) { struct pcie_link_state *link = pdev->link_state;