* [PATCH v4 0/2] Unfiy PCI suspend and hibernate paths
@ 2026-09-16 5:10 Mario Limonciello
2026-09-16 5:10 ` [PATCH v4 1/2] PCI/PM: Split out code from pci_pm_suspend_noirq() into helper Mario Limonciello
2026-09-16 5:10 ` [PATCH v4 2/2] PCI: Put PCIe bridges with downstream devices into D3 at hibernate Mario Limonciello
0 siblings, 2 replies; 8+ messages in thread
From: Mario Limonciello @ 2026-09-16 5:10 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: open list:PCI SUBSYSTEM, open list, Rafael J . Wysocki,
Lukas Wunner, linux-pm, Mario Limonciello
Many cycles ago I sent out a kernel series for using the S4 paths when
the system goes to S5. Some parts of it got merged, and Rafael suggested
to split the other parts into smaller pieces across multiple kernel cycles
to make bisecting easier.
This series attempts to unify the PCI suspend and hibernate paths so that
PCIe bridges with downstream devices are put into D3 at hibernate the same
way they already are at S3 suspend. I have been carrying it, rebasing it
and personally using it for a while now.
v4:
- Change pci_pm_suspend_noirq_common() to bool (Rafael)
- Drop patch 2/3 from v3, mostly dead code (Sashiko)
- Rework patch 3/3 from v3 (Sashiko)
Mario Limonciello (AMD) (2):
PCI/PM: Split out code from pci_pm_suspend_noirq() into helper
PCI: Put PCIe bridges with downstream devices into D3 at hibernate
drivers/pci/pci-driver.c | 86 ++++++++++++++++++++++++++--------------
1 file changed, 56 insertions(+), 30 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v4 1/2] PCI/PM: Split out code from pci_pm_suspend_noirq() into helper
2026-09-16 5:10 [PATCH v4 0/2] Unfiy PCI suspend and hibernate paths Mario Limonciello
@ 2026-09-16 5:10 ` Mario Limonciello
2026-09-16 5:15 ` sashiko-bot
2026-09-16 13:49 ` Rafael J. Wysocki (Intel)
2026-09-16 5:10 ` [PATCH v4 2/2] PCI: Put PCIe bridges with downstream devices into D3 at hibernate Mario Limonciello
1 sibling, 2 replies; 8+ messages in thread
From: Mario Limonciello @ 2026-09-16 5:10 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: open list:PCI SUBSYSTEM, open list, Rafael J . Wysocki,
Lukas Wunner, linux-pm, Mario Limonciello (AMD), Eric Naim
From: "Mario Limonciello (AMD)" <superm1@kernel.org>
In order to unify suspend and hibernate codepaths without code duplication
the common code should be in common helpers. Move it from
pci_pm_suspend_noirq() into a helper. No intended functional changes.
Tested-by: Eric Naim <dnaim@cachyos.org>
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
v4:
* Make pci_pm_suspend_noirq_common() bool instead (Rafael)
---
drivers/pci/pci-driver.c | 77 +++++++++++++++++++++++++---------------
1 file changed, 49 insertions(+), 28 deletions(-)
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index e16aa59dd7ac8..8334214f8c1ed 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -818,6 +818,52 @@ static void pci_pm_complete(struct device *dev)
#endif /* !CONFIG_PM_SLEEP */
+#if defined(CONFIG_SUSPEND)
+/**
+ * pci_pm_suspend_noirq_common - prepare a device to enter a low-power state
+ * @pci_dev: pci device
+ *
+ * Save the device state and decide whether bus-level power management should
+ * skipped. Returns true if bus-level power management should be skipped,
+ * false otherwise.
+ */
+static bool pci_pm_suspend_noirq_common(struct pci_dev *pci_dev)
+{
+ if (!pci_dev->state_saved) {
+ pci_save_state(pci_dev);
+
+ /*
+ * If the device is a bridge with a child in D0 below it,
+ * it needs to stay in D0, so check skip_bus_pm to avoid
+ * putting it into a low-power state in that case.
+ */
+ if (!pci_dev->skip_bus_pm && pci_power_manageable(pci_dev))
+ pci_prepare_to_sleep(pci_dev);
+ }
+
+ pci_dbg(pci_dev, "PCI PM: Sleep power state: %s\n",
+ pci_power_name(pci_dev->current_state));
+
+ 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;
+ }
+
+ if (pci_dev->skip_bus_pm && pm_suspend_no_platform()) {
+ pci_dbg(pci_dev, "PCI PM: Skipped\n");
+ return true;
+ }
+
+ return false;
+}
+#endif /* CONFIG_SUSPEND */
+
#ifdef CONFIG_SUSPEND
static void pcie_pme_root_status_cleanup(struct pci_dev *pci_dev)
{
@@ -907,6 +953,7 @@ static int pci_pm_suspend_noirq(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
+ bool skip_bus_pm;
if (dev_pm_skip_suspend(dev))
return 0;
@@ -937,36 +984,10 @@ static int pci_pm_suspend_noirq(struct device *dev)
}
}
- if (!pci_dev->state_saved) {
- pci_save_state(pci_dev);
-
- /*
- * If the device is a bridge with a child in D0 below it,
- * it needs to stay in D0, so check skip_bus_pm to avoid
- * putting it into a low-power state in that case.
- */
- if (!pci_dev->skip_bus_pm && pci_power_manageable(pci_dev))
- pci_prepare_to_sleep(pci_dev);
- }
+ skip_bus_pm = pci_pm_suspend_noirq_common(pci_dev);
- pci_dbg(pci_dev, "PCI PM: Suspend power state: %s\n",
- pci_power_name(pci_dev->current_state));
-
- 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;
- }
-
- if (pci_dev->skip_bus_pm && pm_suspend_no_platform()) {
- pci_dbg(pci_dev, "PCI PM: Skipped\n");
+ if (skip_bus_pm)
goto Fixup;
- }
set_unknown:
pci_pm_set_unknown_state(pci_dev);
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v4 2/2] PCI: Put PCIe bridges with downstream devices into D3 at hibernate
2026-09-16 5:10 [PATCH v4 0/2] Unfiy PCI suspend and hibernate paths Mario Limonciello
2026-09-16 5:10 ` [PATCH v4 1/2] PCI/PM: Split out code from pci_pm_suspend_noirq() into helper Mario Limonciello
@ 2026-09-16 5:10 ` Mario Limonciello
2026-09-16 5:23 ` sashiko-bot
2026-09-16 13:56 ` Rafael J. Wysocki (Intel)
1 sibling, 2 replies; 8+ messages in thread
From: Mario Limonciello @ 2026-09-16 5:10 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: open list:PCI SUBSYSTEM, open list, Rafael J . Wysocki,
Lukas Wunner, linux-pm, Mario Limonciello (AMD), AceLan Kao,
Kai-Heng Feng, Mark Pearson, Denis Benato, Merthan Karakaş,
Eric Naim
From: "Mario Limonciello (AMD)" <superm1@kernel.org>
During S3 suspend, pci_pm_suspend_noirq() puts PCIe bridges with
downstream devices into a low-power state (D3hot or D3cold) when the
platform allows it. The hibernate poweroff_noirq path never did this:
it only called pci_prepare_to_sleep() for devices with no subordinate,
so bridges with active children were left in D0.
The platform firmware leaves bridges alone when the system enters S4.
This prevents being able to meet various energy certification criteria
for different parts of the world, particularly in designs with a dGPU.
Align the hibernate flow with suspend by making pci_pm_poweroff_noirq()
use pci_pm_suspend_noirq_common() instead of the open-coded
pci_prepare_to_sleep() call. This reuses exactly the logic the S3
suspend path uses, including the skip_bus_pm handling that keeps a
bridge in D0 when a downstream device must stay in D0 (e.g. a
configured wakeup source) and the pm_suspend_no_platform() bus-PM skip.
No restore-side change is needed: the hibernation image is snapshotted
during the freeze phase, before poweroff_noirq runs, so the powered-down
bridge state is never part of the image, and resuming from it goes
through a full boot that re-enumerates and retrains the PCIe links.
Because the poweroff_noirq path now mirrors the already-shipping
suspend_noirq path and is guarded identically, bridges that must remain
in D0 are unaffected; only bridges that S3 suspend would have powered
down are now also powered down at hibernate.
Cc: AceLan Kao <acelan.kao@canonical.com>
Cc: Kai-Heng Feng <kaihengf@nvidia.com>
Cc: Mark Pearson <mpearson-lenovo@squebb.ca>
Cc: Denis Benato <benato.denis96@gmail.com>
Cc: Merthan Karakaş <m3rthn.k@gmail.com>
Tested-by: Eric Naim <dnaim@cachyos.org>
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
drivers/pci/pci-driver.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 8334214f8c1ed..3f80c8e6e0e52 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -818,7 +818,7 @@ static void pci_pm_complete(struct device *dev)
#endif /* !CONFIG_PM_SLEEP */
-#if defined(CONFIG_SUSPEND)
+#if defined(CONFIG_SUSPEND) || defined(CONFIG_HIBERNATE_CALLBACKS)
/**
* pci_pm_suspend_noirq_common - prepare a device to enter a low-power state
* @pci_dev: pci device
@@ -862,7 +862,7 @@ static bool pci_pm_suspend_noirq_common(struct pci_dev *pci_dev)
return false;
}
-#endif /* CONFIG_SUSPEND */
+#endif /* CONFIG_SUSPEND || CONFIG_HIBERNATE_CALLBACKS */
#ifdef CONFIG_SUSPEND
static void pcie_pme_root_status_cleanup(struct pci_dev *pci_dev)
@@ -1217,6 +1217,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);
@@ -1259,6 +1261,7 @@ static int pci_pm_poweroff_noirq(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
+ bool skip_bus_pm;
if (dev_pm_skip_suspend(dev))
return 0;
@@ -1280,8 +1283,9 @@ static int pci_pm_poweroff_noirq(struct device *dev)
return error;
}
- if (!pci_dev->state_saved && !pci_has_subordinate(pci_dev))
- pci_prepare_to_sleep(pci_dev);
+ skip_bus_pm = pci_pm_suspend_noirq_common(pci_dev);
+ if (skip_bus_pm)
+ goto Fixup;
/*
* The reason for doing this here is the same as for the analogous code
@@ -1290,6 +1294,7 @@ static int pci_pm_poweroff_noirq(struct device *dev)
if (pci_dev->class == PCI_CLASS_SERIAL_USB_EHCI)
pci_write_config_word(pci_dev, PCI_COMMAND, 0);
+Fixup:
pci_fixup_device(pci_fixup_suspend_late, pci_dev);
return 0;
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v4 1/2] PCI/PM: Split out code from pci_pm_suspend_noirq() into helper
2026-09-16 5:10 ` [PATCH v4 1/2] PCI/PM: Split out code from pci_pm_suspend_noirq() into helper Mario Limonciello
@ 2026-09-16 5:15 ` sashiko-bot
2026-09-16 13:49 ` Rafael J. Wysocki (Intel)
1 sibling, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-09-16 5:15 UTC (permalink / raw)
To: Mario Limonciello; +Cc: linux-pci
> From: "Mario Limonciello (AMD)" <superm1@kernel.org>
>
> In order to unify suspend and hibernate codepaths without code duplication
> the common code should be in common helpers. Move it from
> pci_pm_suspend_noirq() into a helper. No intended functional changes.
>
> Tested-by: Eric Naim <dnaim@cachyos.org>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260916051034.180226-1-mario.limonciello@amd.com?part=1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 2/2] PCI: Put PCIe bridges with downstream devices into D3 at hibernate
2026-09-16 5:10 ` [PATCH v4 2/2] PCI: Put PCIe bridges with downstream devices into D3 at hibernate Mario Limonciello
@ 2026-09-16 5:23 ` sashiko-bot
2026-09-16 5:36 ` Mario Limonciello
2026-09-16 13:56 ` Rafael J. Wysocki (Intel)
1 sibling, 1 reply; 8+ messages in thread
From: sashiko-bot @ 2026-09-16 5:23 UTC (permalink / raw)
To: Mario Limonciello; +Cc: linux-pci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci: config space corruption on hibernation abort due to missing D3 guard
--
commit 99f9dafa720163cc3dfac6ce20da883f888fb896
Author: Mario Limonciello (AMD) <superm1@kernel.org>
PCI: Put PCIe bridges with downstream devices into D3 at hibernate
During S3 suspend, pci_pm_suspend_noirq() puts PCIe bridges with
downstream devices into a low-power state. This commit aligns the
hibernate poweroff_noirq flow to use pci_pm_suspend_noirq_common(),
ensuring bridges with active children are also put into D3.
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
[ ... ]
> @@ -1280,8 +1283,9 @@ static int pci_pm_poweroff_noirq(struct device *dev)
> return error;
> }
>
> - if (!pci_dev->state_saved && !pci_has_subordinate(pci_dev))
> - pci_prepare_to_sleep(pci_dev);
> + skip_bus_pm = pci_pm_suspend_noirq_common(pci_dev);
[Severity: High]
Could this lead to configuration space corruption if a hibernation aborts?
If a driver's poweroff_noirq callback puts the device into a low-power
state (like D3) but fails to save its state, calling
pci_pm_suspend_noirq_common() here will unconditionally invoke
pci_save_state().
Because the device is already in D3, reading the configuration registers
in pci_save_state() will populate saved_config_space with 0xFFFFFFFF
garbage.
If the hibernation process subsequently aborts, the PM core executes
pci_pm_restore_noirq(), which will blindly write this 0xFFFFFFFF garbage
back to the device's configuration registers (BARs, command registers, etc).
This will likely cause a system crash upon subsequent memory-mapped I/O
access.
Does pci_pm_poweroff_noirq() need the same explicit D3 safety guard that
the S3 suspend_noirq path uses to prevent reading config space from a
powered-down device?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260916051034.180226-1-mario.limonciello@amd.com?part=2
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 2/2] PCI: Put PCIe bridges with downstream devices into D3 at hibernate
2026-09-16 5:23 ` sashiko-bot
@ 2026-09-16 5:36 ` Mario Limonciello
0 siblings, 0 replies; 8+ messages in thread
From: Mario Limonciello @ 2026-09-16 5:36 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-pci
On 9/16/26 00:23, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] pci: config space corruption on hibernation abort due to missing D3 guard
> --
>
> commit 99f9dafa720163cc3dfac6ce20da883f888fb896
> Author: Mario Limonciello (AMD) <superm1@kernel.org>
>
> PCI: Put PCIe bridges with downstream devices into D3 at hibernate
>
> During S3 suspend, pci_pm_suspend_noirq() puts PCIe bridges with
> downstream devices into a low-power state. This commit aligns the
> hibernate poweroff_noirq flow to use pci_pm_suspend_noirq_common(),
> ensuring bridges with active children are also put into D3.
>
>> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
>> --- a/drivers/pci/pci-driver.c
>> +++ b/drivers/pci/pci-driver.c
> [ ... ]
>> @@ -1280,8 +1283,9 @@ static int pci_pm_poweroff_noirq(struct device *dev)
>> return error;
>> }
>>
>> - if (!pci_dev->state_saved && !pci_has_subordinate(pci_dev))
>> - pci_prepare_to_sleep(pci_dev);
>> + skip_bus_pm = pci_pm_suspend_noirq_common(pci_dev);
>
> [Severity: High]
> Could this lead to configuration space corruption if a hibernation aborts?
>
> If a driver's poweroff_noirq callback puts the device into a low-power
> state (like D3) but fails to save its state, calling
> pci_pm_suspend_noirq_common() here will unconditionally invoke
> pci_save_state().
That seems wrong. It's gated by if (!pci_dev->state_saved) in the helper..
>
> Because the device is already in D3, reading the configuration registers
> in pci_save_state() will populate saved_config_space with 0xFFFFFFFF
> garbage.
I think this is true; but only for D3cold, not D3hot.
I'm not sure how exactly this state could actually happen.
>
> If the hibernation process subsequently aborts, the PM core executes
> pci_pm_restore_noirq(), which will blindly write this 0xFFFFFFFF garbage
> back to the device's configuration registers (BARs, command registers, etc).
> This will likely cause a system crash upon subsequent memory-mapped I/O
> access.
>
I guess this is reasonable (pci_pm_default_resume_early() ->
pci_restore_state()).
I suppose I didn't really consider a poweroff abort, which is what would
be relevant in this case.
> Does pci_pm_poweroff_noirq() need the same explicit D3 safety guard that
> the S3 suspend_noirq path uses to prevent reading config space from a
> powered-down device?
>
This does sound reasonable at least for the 3rd case. I'm still not
sold on the other two that Sashiko called out.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 1/2] PCI/PM: Split out code from pci_pm_suspend_noirq() into helper
2026-09-16 5:10 ` [PATCH v4 1/2] PCI/PM: Split out code from pci_pm_suspend_noirq() into helper Mario Limonciello
2026-09-16 5:15 ` sashiko-bot
@ 2026-09-16 13:49 ` Rafael J. Wysocki (Intel)
1 sibling, 0 replies; 8+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-09-16 13:49 UTC (permalink / raw)
To: Mario Limonciello
Cc: Bjorn Helgaas, open list:PCI SUBSYSTEM, open list,
Rafael J . Wysocki, Lukas Wunner, linux-pm,
Mario Limonciello (AMD), Eric Naim
On Wed, Sep 16, 2026 at 7:10 AM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> From: "Mario Limonciello (AMD)" <superm1@kernel.org>
>
> In order to unify suspend and hibernate codepaths without code duplication
> the common code should be in common helpers. Move it from
> pci_pm_suspend_noirq() into a helper. No intended functional changes.
>
> Tested-by: Eric Naim <dnaim@cachyos.org>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
Reviewed-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>
> ---
> v4:
> * Make pci_pm_suspend_noirq_common() bool instead (Rafael)
> ---
> drivers/pci/pci-driver.c | 77 +++++++++++++++++++++++++---------------
> 1 file changed, 49 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index e16aa59dd7ac8..8334214f8c1ed 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -818,6 +818,52 @@ static void pci_pm_complete(struct device *dev)
>
> #endif /* !CONFIG_PM_SLEEP */
>
> +#if defined(CONFIG_SUSPEND)
> +/**
> + * pci_pm_suspend_noirq_common - prepare a device to enter a low-power state
> + * @pci_dev: pci device
> + *
> + * Save the device state and decide whether bus-level power management should
> + * skipped. Returns true if bus-level power management should be skipped,
> + * false otherwise.
> + */
> +static bool pci_pm_suspend_noirq_common(struct pci_dev *pci_dev)
> +{
> + if (!pci_dev->state_saved) {
> + pci_save_state(pci_dev);
> +
> + /*
> + * If the device is a bridge with a child in D0 below it,
> + * it needs to stay in D0, so check skip_bus_pm to avoid
> + * putting it into a low-power state in that case.
> + */
> + if (!pci_dev->skip_bus_pm && pci_power_manageable(pci_dev))
> + pci_prepare_to_sleep(pci_dev);
> + }
> +
> + pci_dbg(pci_dev, "PCI PM: Sleep power state: %s\n",
> + pci_power_name(pci_dev->current_state));
> +
> + 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;
> + }
> +
> + if (pci_dev->skip_bus_pm && pm_suspend_no_platform()) {
> + pci_dbg(pci_dev, "PCI PM: Skipped\n");
> + return true;
> + }
> +
> + return false;
> +}
> +#endif /* CONFIG_SUSPEND */
> +
> #ifdef CONFIG_SUSPEND
> static void pcie_pme_root_status_cleanup(struct pci_dev *pci_dev)
> {
> @@ -907,6 +953,7 @@ static int pci_pm_suspend_noirq(struct device *dev)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
> const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> + bool skip_bus_pm;
>
> if (dev_pm_skip_suspend(dev))
> return 0;
> @@ -937,36 +984,10 @@ static int pci_pm_suspend_noirq(struct device *dev)
> }
> }
>
> - if (!pci_dev->state_saved) {
> - pci_save_state(pci_dev);
> -
> - /*
> - * If the device is a bridge with a child in D0 below it,
> - * it needs to stay in D0, so check skip_bus_pm to avoid
> - * putting it into a low-power state in that case.
> - */
> - if (!pci_dev->skip_bus_pm && pci_power_manageable(pci_dev))
> - pci_prepare_to_sleep(pci_dev);
> - }
> + skip_bus_pm = pci_pm_suspend_noirq_common(pci_dev);
>
> - pci_dbg(pci_dev, "PCI PM: Suspend power state: %s\n",
> - pci_power_name(pci_dev->current_state));
> -
> - 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;
> - }
> -
> - if (pci_dev->skip_bus_pm && pm_suspend_no_platform()) {
> - pci_dbg(pci_dev, "PCI PM: Skipped\n");
> + if (skip_bus_pm)
> goto Fixup;
> - }
>
> set_unknown:
> pci_pm_set_unknown_state(pci_dev);
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 2/2] PCI: Put PCIe bridges with downstream devices into D3 at hibernate
2026-09-16 5:10 ` [PATCH v4 2/2] PCI: Put PCIe bridges with downstream devices into D3 at hibernate Mario Limonciello
2026-09-16 5:23 ` sashiko-bot
@ 2026-09-16 13:56 ` Rafael J. Wysocki (Intel)
1 sibling, 0 replies; 8+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-09-16 13:56 UTC (permalink / raw)
To: Mario Limonciello
Cc: Bjorn Helgaas, open list:PCI SUBSYSTEM, open list,
Rafael J . Wysocki, Lukas Wunner, linux-pm,
Mario Limonciello (AMD), AceLan Kao, Kai-Heng Feng, Mark Pearson,
Denis Benato, Merthan Karakaş, Eric Naim
On Wed, Sep 16, 2026 at 7:10 AM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> From: "Mario Limonciello (AMD)" <superm1@kernel.org>
>
> During S3 suspend, pci_pm_suspend_noirq() puts PCIe bridges with
> downstream devices into a low-power state (D3hot or D3cold) when the
> platform allows it. The hibernate poweroff_noirq path never did this:
> it only called pci_prepare_to_sleep() for devices with no subordinate,
> so bridges with active children were left in D0.
>
> The platform firmware leaves bridges alone when the system enters S4.
Well, that depends. IIRC there were platforms on which it actually
removed power from them unless wakeup was enabled in the past.
> This prevents being able to meet various energy certification criteria
> for different parts of the world, particularly in designs with a dGPU.
Fair enough.
> Align the hibernate flow with suspend by making pci_pm_poweroff_noirq()
> use pci_pm_suspend_noirq_common() instead of the open-coded
> pci_prepare_to_sleep() call. This reuses exactly the logic the S3
> suspend path uses, including the skip_bus_pm handling that keeps a
> bridge in D0 when a downstream device must stay in D0 (e.g. a
> configured wakeup source) and the pm_suspend_no_platform() bus-PM skip.
And that's key here, so the patch subject should be something like
"PCI: Align hibernate poweroff flow with suspend flow for bridges".
> No restore-side change is needed: the hibernation image is snapshotted
> during the freeze phase, before poweroff_noirq runs, so the powered-down
> bridge state is never part of the image, and resuming from it goes
> through a full boot that re-enumerates and retrains the PCIe links.
>
> Because the poweroff_noirq path now mirrors the already-shipping
> suspend_noirq path and is guarded identically, bridges that must remain
> in D0 are unaffected; only bridges that S3 suspend would have powered
> down are now also powered down at hibernate.
>
> Cc: AceLan Kao <acelan.kao@canonical.com>
> Cc: Kai-Heng Feng <kaihengf@nvidia.com>
> Cc: Mark Pearson <mpearson-lenovo@squebb.ca>
> Cc: Denis Benato <benato.denis96@gmail.com>
> Cc: Merthan Karakaş <m3rthn.k@gmail.com>
> Tested-by: Eric Naim <dnaim@cachyos.org>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
With the above addressed
Acked-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>
> ---
> drivers/pci/pci-driver.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 8334214f8c1ed..3f80c8e6e0e52 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -818,7 +818,7 @@ static void pci_pm_complete(struct device *dev)
>
> #endif /* !CONFIG_PM_SLEEP */
>
> -#if defined(CONFIG_SUSPEND)
> +#if defined(CONFIG_SUSPEND) || defined(CONFIG_HIBERNATE_CALLBACKS)
> /**
> * pci_pm_suspend_noirq_common - prepare a device to enter a low-power state
> * @pci_dev: pci device
> @@ -862,7 +862,7 @@ static bool pci_pm_suspend_noirq_common(struct pci_dev *pci_dev)
>
> return false;
> }
> -#endif /* CONFIG_SUSPEND */
> +#endif /* CONFIG_SUSPEND || CONFIG_HIBERNATE_CALLBACKS */
>
> #ifdef CONFIG_SUSPEND
> static void pcie_pme_root_status_cleanup(struct pci_dev *pci_dev)
> @@ -1217,6 +1217,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);
>
> @@ -1259,6 +1261,7 @@ static int pci_pm_poweroff_noirq(struct device *dev)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
> const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> + bool skip_bus_pm;
>
> if (dev_pm_skip_suspend(dev))
> return 0;
> @@ -1280,8 +1283,9 @@ static int pci_pm_poweroff_noirq(struct device *dev)
> return error;
> }
>
> - if (!pci_dev->state_saved && !pci_has_subordinate(pci_dev))
> - pci_prepare_to_sleep(pci_dev);
> + skip_bus_pm = pci_pm_suspend_noirq_common(pci_dev);
> + if (skip_bus_pm)
> + goto Fixup;
>
> /*
> * The reason for doing this here is the same as for the analogous code
> @@ -1290,6 +1294,7 @@ static int pci_pm_poweroff_noirq(struct device *dev)
> if (pci_dev->class == PCI_CLASS_SERIAL_USB_EHCI)
> pci_write_config_word(pci_dev, PCI_COMMAND, 0);
>
> +Fixup:
> pci_fixup_device(pci_fixup_suspend_late, pci_dev);
>
> return 0;
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-09-16 13:57 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-16 5:10 [PATCH v4 0/2] Unfiy PCI suspend and hibernate paths Mario Limonciello
2026-09-16 5:10 ` [PATCH v4 1/2] PCI/PM: Split out code from pci_pm_suspend_noirq() into helper Mario Limonciello
2026-09-16 5:15 ` sashiko-bot
2026-09-16 13:49 ` Rafael J. Wysocki (Intel)
2026-09-16 5:10 ` [PATCH v4 2/2] PCI: Put PCIe bridges with downstream devices into D3 at hibernate Mario Limonciello
2026-09-16 5:23 ` sashiko-bot
2026-09-16 5:36 ` Mario Limonciello
2026-09-16 13:56 ` Rafael J. Wysocki (Intel)
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.