[PATCH 1/6] PCI/PM: Respect pci_dev->skip_bus_pm in the .poweroff() path

Ville Syrjala ville.syrjala at linux.intel.com
Wed Sep 25 14:45:21 UTC 2024


From: Ville Syrjälä <ville.syrjala at linux.intel.com>

On some older laptops i915 needs to leave the GPU in
D0 when hibernating the system, or else the BIOS
hangs somewhere. Currently that is achieved by calling
pci_save_state() ahead of time, which then skips the
whole pci_prepare_to_sleep() stuff.

It feels to me that this approach could lead to unintended
side effects as it causes the pci code to deviate from the
standard path in various ways. In order to keep i915
behaviour more standard it seems preferrable to use
pci_dev->skip_bus_pm here. Duplicate the relevant logic
from pci_pm_suspend_noirq() in pci_pm_poweroff_noirq().

It also looks like the current code is may put the parent
bridge into D3 despite leaving the device in D0. Though
perhaps the host bridge (which is where the integrated
GPU lives) always has subordinates, which would make
this a non-issue for i915. But maybe this could be a
problem for other devices. Utilizing skip_bus_pm will
make the behaviour of leaving the bridge in D0 a bit
more explicit if nothing else.

Cc: Bjorn Helgaas <bhelgaas at google.com>
Cc: "Rafael J. Wysocki" <rafael at kernel.org>
Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
Cc: linux-pci at vger.kernel.org
Cc: intel-gfx at lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
 drivers/pci/pci-driver.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index f412ef73a6e4..ef436895939c 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -1142,6 +1142,8 @@ static int pci_pm_poweroff(struct device *dev)
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
 
+	pci_dev->skip_bus_pm = false;
+
 	if (pci_has_legacy_pm_support(pci_dev))
 		return pci_legacy_suspend(dev, PMSG_HIBERNATE);
 
@@ -1206,9 +1208,21 @@ static int pci_pm_poweroff_noirq(struct device *dev)
 			return error;
 	}
 
-	if (!pci_dev->state_saved && !pci_has_subordinate(pci_dev))
+	if (!pci_dev->state_saved && !pci_dev->skip_bus_pm &&
+	    !pci_has_subordinate(pci_dev))
 		pci_prepare_to_sleep(pci_dev);
 
+	if (pci_dev->current_state == PCI_D0) {
+		pci_dev->skip_bus_pm = true;
+		/*
+		 * Per PCI PM r1.2, table 6-1, a bridge must be in D0 if any
+		 * downstream device is in D0, so avoid changing the power state
+		 * of the parent bridge by setting the skip_bus_pm flag for it.
+		 */
+		if (pci_dev->bus->self)
+			pci_dev->bus->self->skip_bus_pm = true;
+	}
+
 	/*
 	 * The reason for doing this here is the same as for the analogous code
 	 * in pci_pm_suspend_noirq().
-- 
2.44.2



More information about the Intel-gfx mailing list