Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Ville Syrjala <ville.syrjala@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	linux-pci@vger.kernel.org
Subject: [PATCH 1/6] PCI/PM: Respect pci_dev->skip_bus_pm in the .poweroff() path
Date: Wed, 25 Sep 2024 17:45:21 +0300	[thread overview]
Message-ID: <20240925144526.2482-2-ville.syrjala@linux.intel.com> (raw)
In-Reply-To: <20240925144526.2482-1-ville.syrjala@linux.intel.com>

From: Ville Syrjälä <ville.syrjala@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@google.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: linux-pci@vger.kernel.org
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@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


  reply	other threads:[~2024-09-25 14:45 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-25 14:45 [PATCH 0/6] drm/i915/pm: Clean up the hibernate vs. PCI D3 quirk Ville Syrjala
2024-09-25 14:45 ` Ville Syrjala [this message]
2024-09-25 19:28   ` [PATCH 1/6] PCI/PM: Respect pci_dev->skip_bus_pm in the .poweroff() path Bjorn Helgaas
2024-09-26 16:03     ` Ville Syrjälä
2024-09-30 19:50       ` Bjorn Helgaas
2024-10-01 13:12         ` Ville Syrjälä
2024-10-23 15:31     ` Ville Syrjälä
2024-09-25 14:45 ` [PATCH 2/6] drm/i915/pm: Hoist pci_save_state()+pci_set_power_state() to the end of pm _late() hook Ville Syrjala
2024-09-26 14:43   ` Rodrigo Vivi
2024-09-26 15:41     ` Ville Syrjälä
2024-09-26 16:40       ` Rodrigo Vivi
2024-09-25 14:45 ` [PATCH 3/6] drm/i915/pm: Simplify pm hook documentation Ville Syrjala
2024-09-26 14:45   ` Rodrigo Vivi
2024-09-26 15:38     ` Ville Syrjälä
2024-09-26 16:10       ` Rodrigo Vivi
2024-09-25 14:45 ` [PATCH 4/6] drm/i915/pm: Move the hibernate+D3 quirk stuff into noirq() pm hooks Ville Syrjala
2024-09-25 19:28   ` Bjorn Helgaas
2024-09-25 14:45 ` [PATCH 5/6] drm/i915/pm: Do pci_restore_state() in switcheroo resume hook Ville Syrjala
2024-09-26 14:48   ` Rodrigo Vivi
2024-09-26 15:36     ` Ville Syrjälä
2024-09-26 16:27       ` Rodrigo Vivi
2024-09-25 14:45 ` [PATCH 6/6] drm/i915/pm: Use pci_dev->skip_bus_pm for hibernate vs. D3 workaround Ville Syrjala
2024-09-25 19:28   ` Bjorn Helgaas
2024-09-27  3:01 ` ✓ Fi.CI.BAT: success for drm/i915/pm: Clean up the hibernate vs. PCI D3 quirk Patchwork
2024-09-27 23:44 ` ✗ Fi.CI.IGT: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240925144526.2482-2-ville.syrjala@linux.intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=bhelgaas@google.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=rodrigo.vivi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox