2022-05-09 08:43:22

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH v1 00/11] PCI/PM: Rework powering up PCI devices

Hi All,

This patch set replaces patches [4-9/9] from the series at

https://lore.kernel.org/linux-pm/4419002.LvFx2qVVIh@kreacher/T/#mf7ed30e7cf114b131e6067e4e10c28e59d661cb4

which had to be dropped, because they were problematic:

https://lore.kernel.org/linux-pm/4419002.LvFx2qVVIh@kreacher/T/#ma71172f00a95708f0cd4d21741bcc248d394caf1

This time there are more patches making smaller changes each and I've done my
best to avoid making any changes without a good enough motivation.

Please refer to the patch changelogs for details.

Thanks!





2022-05-09 09:45:02

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH v1 08/11] PCI/PM: Do not restore BARs if device is not in D0

From: Rafael J. Wysocki <[email protected]>

Do not attempt to restore the device's BARs in
pci_set_full_power_state() if the actual current
power state of the device is not D0.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/pci/pci.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)

Index: linux-pm/drivers/pci/pci.c
===================================================================
--- linux-pm.orig/drivers/pci/pci.c
+++ linux-pm/drivers/pci/pci.c
@@ -1273,25 +1273,25 @@ static int pci_set_full_power_state(stru

pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
dev->current_state = pmcsr & PCI_PM_CTRL_STATE_MASK;
- if (dev->current_state != PCI_D0)
+ if (dev->current_state != PCI_D0) {
pci_info_ratelimited(dev, "Refused to change power state from %s to D0\n",
pci_power_name(dev->current_state));
-
- /*
- * According to section 5.4.1 of the "PCI BUS POWER MANAGEMENT
- * INTERFACE SPECIFICATION, REV. 1.2", a device transitioning
- * from D3hot to D0 _may_ perform an internal reset, thereby
- * going to "D0 Uninitialized" rather than "D0 Initialized".
- * For example, at least some versions of the 3c905B and the
- * 3c556B exhibit this behaviour.
- *
- * At least some laptop BIOSen (e.g. the Thinkpad T21) leave
- * devices in a D3hot state at boot. Consequently, we need to
- * restore at least the BARs so that the device will be
- * accessible to its driver.
- */
- if (ret > 0)
+ } else if (ret > 0) {
+ /*
+ * According to section 5.4.1 of the "PCI BUS POWER MANAGEMENT
+ * INTERFACE SPECIFICATION, REV. 1.2", a device transitioning
+ * from D3hot to D0 _may_ perform an internal reset, thereby
+ * going to "D0 Uninitialized" rather than "D0 Initialized".
+ * For example, at least some versions of the 3c905B and the
+ * 3c556B exhibit this behaviour.
+ *
+ * At least some laptop BIOSen (e.g. the Thinkpad T21) leave
+ * devices in a D3hot state at boot. Consequently, we need to
+ * restore at least the BARs so that the device will be
+ * accessible to its driver.
+ */
pci_restore_bars(dev);
+ }

if (dev->bus->self)
pcie_aspm_pm_state_change(dev->bus->self);




2022-05-09 09:45:45

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH v1 11/11] PCI/PM: Replace pci_set_power_state() in pci_pm_thaw_noirq()

From: Rafael J. Wysocki <[email protected]>

Calling pci_set_power_state() to put the given device into D0 in
pci_pm_thaw_noirq() may cause it to restore the device's BARs, which
is redundant before calling pci_restore_state(), so replace it with
a direct pci_power_up() call followed by pci_update_current_state()
if it returns a nonzeor value, in analogy with
pci_pm_default_resume_early().

Avoid code duplication by introducing a wrapper function to contain
the repeating pattern and calling it in both places.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/pci/pci-driver.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

Index: linux-pm/drivers/pci/pci-driver.c
===================================================================
--- linux-pm.orig/drivers/pci/pci-driver.c
+++ linux-pm/drivers/pci/pci-driver.c
@@ -551,10 +551,15 @@ static void pci_pm_default_resume(struct
pci_enable_wake(pci_dev, PCI_D0, false);
}

-static void pci_pm_default_resume_early(struct pci_dev *pci_dev)
+static void pci_pm_power_up_and_verify_state(struct pci_dev *pci_dev)
{
pci_power_up(pci_dev);
pci_update_current_state(pci_dev, PCI_D0);
+}
+
+static void pci_pm_default_resume_early(struct pci_dev *pci_dev)
+{
+ pci_pm_power_up_and_verify_state(pci_dev);
pci_restore_state(pci_dev);
pci_pme_restore(pci_dev);
}
@@ -1079,7 +1084,7 @@ static int pci_pm_thaw_noirq(struct devi
* in case the driver's "freeze" callbacks put it into a low-power
* state.
*/
- pci_set_power_state(pci_dev, PCI_D0);
+ pci_pm_power_up_and_verify_state(pci_dev);
pci_restore_state(pci_dev);

if (pci_has_legacy_pm_support(pci_dev))