* [PATCH v3 0/3] Unfiy PCI suspend and hibernate paths
@ 2026-08-31 20:55 Mario Limonciello
2026-08-31 20:55 ` [PATCH v3 1/3] PCI/PM: Split out code from pci_pm_suspend_noirq() into helper Mario Limonciello
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Mario Limonciello @ 2026-08-31 20:55 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.
Since v2, the two changes that had major push back on have been dropped
(see the changelog below), and the remaining patches have been reworked to
minimize churn and to explain the concrete motivation behind them.
v3:
- Drop "PCI/PM: Disable device wakeups when halting or powering off
system" (was 2/6). It would break Wake-on-LAN / Remote Start from S5,
has a user-visible sysfs side effect if the poweroff transition is
rolled back, and runs counter to ACPI r6.5 sec 16.1.5 ("OSPM does not
disable wake events before setting the SLP_EN bit when entering the S5
system state"). (Rafael, Bjorn)
- Drop "PCI/PM: Use pci_power_manageable() in pci_pm_poweroff_noirq()"
(was 5/6). It only touched the line that the final patch replaces, and
the shared helper already selects bridges via pci_power_manageable(), so
it was superseded churn. (Rafael)
- Drop "PCI: Stop setting cached power state to unknown on unbind"
(was 1/6) from the series; it has been merged.
(Reviewed-by: Rafael J. Wysocki)
- Split-out-helper patch: keep pci_pm_set_unknown_state() in the callers
so pci_pm_suspend_noirq()'s set_unknown: label is preserved and no label
churn is needed; fix the kernel-doc comment for the new helper. (Rafael,
Sashiko)
- Rewrite the changelog of "Run bridge power up actions as part of restore
phase" to explain why it is needed: it is the restore-side counterpart
to putting bridges into D3cold at hibernate, mirroring
pci_pm_resume_noirq(). (Rafael)
- Rewrite the changelog of "Put PCIe bridges with downstream devices into
D3 at hibernate" to describe the concrete problem (firmware not powering
the bridges down at S4 on affected platforms, and the resulting power
draw) and to argue safety (it mirrors the shipping S3 suspend_noirq path
and is guarded identically). (Rafael)
Mario Limonciello (AMD) (3):
PCI/PM: Split out code from pci_pm_suspend_noirq() into helper
PCI/PM: Run bridge power up actions as part of restore phase
PCI: Put PCIe bridges with downstream devices into D3 at hibernate
drivers/pci/pci-driver.c | 89 ++++++++++++++++++++++++++--------------
1 file changed, 59 insertions(+), 30 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 1/3] PCI/PM: Split out code from pci_pm_suspend_noirq() into helper
2026-08-31 20:55 [PATCH v3 0/3] Unfiy PCI suspend and hibernate paths Mario Limonciello
@ 2026-08-31 20:55 ` Mario Limonciello
2026-09-04 13:03 ` Rafael J. Wysocki (Intel)
2026-08-31 20:55 ` [PATCH v3 2/3] PCI/PM: Run bridge power up actions as part of restore phase Mario Limonciello
2026-08-31 20:55 ` [PATCH v3 3/3] PCI: Put PCIe bridges with downstream devices into D3 at hibernate Mario Limonciello
2 siblings, 1 reply; 7+ messages in thread
From: Mario Limonciello @ 2026-08-31 20:55 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>
---
drivers/pci/pci-driver.c | 75 +++++++++++++++++++++++++---------------
1 file changed, 47 insertions(+), 28 deletions(-)
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index e16aa59dd7ac8..e3d352c7b31d4 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -818,6 +818,50 @@ 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
+ * @skip_bus_pm: pointer to a boolean indicating whether to skip bus PM
+ *
+ * Save the device state and decide whether bus-level power management should
+ * be skipped, reporting the decision back to the caller via @skip_bus_pm.
+ */
+static void pci_pm_suspend_noirq_common(struct pci_dev *pci_dev, bool *skip_bus_pm)
+{
+ 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");
+ *skip_bus_pm = true;
+ }
+}
+#endif /* CONFIG_SUSPEND */
+
#ifdef CONFIG_SUSPEND
static void pcie_pme_root_status_cleanup(struct pci_dev *pci_dev)
{
@@ -907,6 +951,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 = false;
if (dev_pm_skip_suspend(dev))
return 0;
@@ -937,36 +982,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);
- }
-
- pci_dbg(pci_dev, "PCI PM: Suspend power state: %s\n",
- pci_power_name(pci_dev->current_state));
+ pci_pm_suspend_noirq_common(pci_dev, &skip_bus_pm);
- 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] 7+ messages in thread
* [PATCH v3 2/3] PCI/PM: Run bridge power up actions as part of restore phase
2026-08-31 20:55 [PATCH v3 0/3] Unfiy PCI suspend and hibernate paths Mario Limonciello
2026-08-31 20:55 ` [PATCH v3 1/3] PCI/PM: Split out code from pci_pm_suspend_noirq() into helper Mario Limonciello
@ 2026-08-31 20:55 ` Mario Limonciello
2026-09-04 13:09 ` Rafael J. Wysocki (Intel)
2026-08-31 20:55 ` [PATCH v3 3/3] PCI: Put PCIe bridges with downstream devices into D3 at hibernate Mario Limonciello
2 siblings, 1 reply; 7+ messages in thread
From: Mario Limonciello @ 2026-08-31 20:55 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>
pci_pm_resume_noirq() calls pci_pm_bridge_power_up_actions() when a
bridge is coming back from D3cold and bus PM was not skipped, so that
the mandatory post-power-on link training delays are observed before
any downstream device is accessed.
The last patch in this series makes the hibernate poweroff_noirq path
put bridges with downstream devices into D3 (including D3cold),
mirroring what the S3 suspend path already does. For that to be safe,
the hibernate image restore path has to bring those bridges back up the
same way suspend resume does; otherwise a downstream device may be
accessed before its parent bridge's link is retrained.
Add the same bridge power up actions to pci_pm_restore_noirq(), gated on
the same conditions as the resume path (bus PM not skipped and the
previous state being D3cold), so that hibernate restore and suspend
resume behave identically for bridges.
Tested-by: Eric Naim <dnaim@cachyos.org>
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
drivers/pci/pci-driver.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index e3d352c7b31d4..d15b3eb55ec53 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -1297,10 +1297,15 @@ static int pci_pm_restore_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;
+ pci_power_t prev_state = pci_dev->current_state;
+ bool skip_bus_pm = pci_dev->skip_bus_pm;
pci_pm_default_resume_early(pci_dev);
pci_fixup_device(pci_fixup_resume_early, pci_dev);
+ if (!skip_bus_pm && prev_state == PCI_D3cold)
+ pci_pm_bridge_power_up_actions(pci_dev);
+
if (pci_has_legacy_pm_support(pci_dev))
return 0;
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 3/3] PCI: Put PCIe bridges with downstream devices into D3 at hibernate
2026-08-31 20:55 [PATCH v3 0/3] Unfiy PCI suspend and hibernate paths Mario Limonciello
2026-08-31 20:55 ` [PATCH v3 1/3] PCI/PM: Split out code from pci_pm_suspend_noirq() into helper Mario Limonciello
2026-08-31 20:55 ` [PATCH v3 2/3] PCI/PM: Run bridge power up actions as part of restore phase Mario Limonciello
@ 2026-08-31 20:55 ` Mario Limonciello
2 siblings, 0 replies; 7+ messages in thread
From: Mario Limonciello @ 2026-08-31 20:55 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.
The restore side is handled by the preceding patch, which runs the
bridge power up actions when a bridge comes back from D3cold.
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 d15b3eb55ec53..a6fb731378d9c 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
@@ -860,7 +860,7 @@ static void pci_pm_suspend_noirq_common(struct pci_dev *pci_dev, bool *skip_bus_
*skip_bus_pm = true;
}
}
-#endif /* CONFIG_SUSPEND */
+#endif /* CONFIG_SUSPEND || CONFIG_HIBERNATE_CALLBACKS */
#ifdef CONFIG_SUSPEND
static void pcie_pme_root_status_cleanup(struct pci_dev *pci_dev)
@@ -1215,6 +1215,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);
@@ -1257,6 +1259,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 = false;
if (dev_pm_skip_suspend(dev))
return 0;
@@ -1278,8 +1281,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);
+ pci_pm_suspend_noirq_common(pci_dev, &skip_bus_pm);
+ if (skip_bus_pm)
+ goto Fixup;
/*
* The reason for doing this here is the same as for the analogous code
@@ -1288,6 +1292,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] 7+ messages in thread
* Re: [PATCH v3 1/3] PCI/PM: Split out code from pci_pm_suspend_noirq() into helper
2026-08-31 20:55 ` [PATCH v3 1/3] PCI/PM: Split out code from pci_pm_suspend_noirq() into helper Mario Limonciello
@ 2026-09-04 13:03 ` Rafael J. Wysocki (Intel)
0 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-09-04 13:03 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 Mon, Aug 31, 2026 at 10:56 PM 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>
> ---
> drivers/pci/pci-driver.c | 75 +++++++++++++++++++++++++---------------
> 1 file changed, 47 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index e16aa59dd7ac8..e3d352c7b31d4 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -818,6 +818,50 @@ 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
> + * @skip_bus_pm: pointer to a boolean indicating whether to skip bus PM
> + *
> + * Save the device state and decide whether bus-level power management should
> + * be skipped, reporting the decision back to the caller via @skip_bus_pm.
> + */
> +static void pci_pm_suspend_noirq_common(struct pci_dev *pci_dev, bool *skip_bus_pm)
Change it to bool.
> +{
> + 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");
> + *skip_bus_pm = true;
And return true here.
> + }
And false here.
Then you won't need the second argument.
> +}
> +#endif /* CONFIG_SUSPEND */
> +
> #ifdef CONFIG_SUSPEND
> static void pcie_pme_root_status_cleanup(struct pci_dev *pci_dev)
> {
> @@ -907,6 +951,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 = false;
>
> if (dev_pm_skip_suspend(dev))
> return 0;
> @@ -937,36 +982,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);
> - }
> -
> - pci_dbg(pci_dev, "PCI PM: Suspend power state: %s\n",
> - pci_power_name(pci_dev->current_state));
> + pci_pm_suspend_noirq_common(pci_dev, &skip_bus_pm);
>
> - 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] 7+ messages in thread
* Re: [PATCH v3 2/3] PCI/PM: Run bridge power up actions as part of restore phase
2026-08-31 20:55 ` [PATCH v3 2/3] PCI/PM: Run bridge power up actions as part of restore phase Mario Limonciello
@ 2026-09-04 13:09 ` Rafael J. Wysocki (Intel)
2026-09-04 13:11 ` Rafael J. Wysocki (Intel)
0 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-09-04 13:09 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 Mon, Aug 31, 2026 at 10:56 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> From: "Mario Limonciello (AMD)" <superm1@kernel.org>
>
> pci_pm_resume_noirq() calls pci_pm_bridge_power_up_actions() when a
> bridge is coming back from D3cold and bus PM was not skipped, so that
> the mandatory post-power-on link training delays are observed before
> any downstream device is accessed.
>
> The last patch in this series makes the hibernate poweroff_noirq path
> put bridges with downstream devices into D3 (including D3cold),
> mirroring what the S3 suspend path already does. For that to be safe,
> the hibernate image restore path has to bring those bridges back up the
> same way suspend resume does; otherwise a downstream device may be
> accessed before its parent bridge's link is retrained.
Note though that pci_pm_restore_noirq() runs after the boot kernel has
initialized the PCI bus, so all of the links should be retrained by
that time.
It then carries out a "freeze" transition before passing control to
the image kernel, so "restore" doesn't really get devices in
"pristine" states.
Moreover, BIOS code runs before "poweroff_noirq" and the boot kernel
and it also messes up with devices in general.
Is there a specific issue that is avoided by this change?
> Add the same bridge power up actions to pci_pm_restore_noirq(), gated on
> the same conditions as the resume path (bus PM not skipped and the
> previous state being D3cold), so that hibernate restore and suspend
> resume behave identically for bridges.
>
> Tested-by: Eric Naim <dnaim@cachyos.org>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> drivers/pci/pci-driver.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index e3d352c7b31d4..d15b3eb55ec53 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -1297,10 +1297,15 @@ static int pci_pm_restore_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;
> + pci_power_t prev_state = pci_dev->current_state;
> + bool skip_bus_pm = pci_dev->skip_bus_pm;
>
> pci_pm_default_resume_early(pci_dev);
> pci_fixup_device(pci_fixup_resume_early, pci_dev);
>
> + if (!skip_bus_pm && prev_state == PCI_D3cold)
> + pci_pm_bridge_power_up_actions(pci_dev);
> +
> if (pci_has_legacy_pm_support(pci_dev))
> return 0;
>
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/3] PCI/PM: Run bridge power up actions as part of restore phase
2026-09-04 13:09 ` Rafael J. Wysocki (Intel)
@ 2026-09-04 13:11 ` Rafael J. Wysocki (Intel)
0 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-09-04 13:11 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 Fri, Sep 4, 2026 at 3:09 PM Rafael J. Wysocki (Intel)
<rafael@kernel.org> wrote:
>
> On Mon, Aug 31, 2026 at 10:56 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
> >
> > From: "Mario Limonciello (AMD)" <superm1@kernel.org>
> >
> > pci_pm_resume_noirq() calls pci_pm_bridge_power_up_actions() when a
> > bridge is coming back from D3cold and bus PM was not skipped, so that
> > the mandatory post-power-on link training delays are observed before
> > any downstream device is accessed.
> >
> > The last patch in this series makes the hibernate poweroff_noirq path
> > put bridges with downstream devices into D3 (including D3cold),
> > mirroring what the S3 suspend path already does. For that to be safe,
> > the hibernate image restore path has to bring those bridges back up the
> > same way suspend resume does; otherwise a downstream device may be
> > accessed before its parent bridge's link is retrained.
>
> Note though that pci_pm_restore_noirq() runs after the boot kernel has
> initialized the PCI bus, so all of the links should be retrained by
> that time.
>
> It then carries out a "freeze" transition before passing control to
s/It then/The boot kernel/
> the image kernel, so "restore" doesn't really get devices in
> "pristine" states.
>
> Moreover, BIOS code runs before "poweroff_noirq" and the boot kernel
s/before/between/
> and it also messes up with devices in general.
Sent too early, sorry about that, but the question below is still relevant.
> Is there a specific issue that is avoided by this change?
>
> > Add the same bridge power up actions to pci_pm_restore_noirq(), gated on
> > the same conditions as the resume path (bus PM not skipped and the
> > previous state being D3cold), so that hibernate restore and suspend
> > resume behave identically for bridges.
> >
> > Tested-by: Eric Naim <dnaim@cachyos.org>
> > Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> > ---
> > drivers/pci/pci-driver.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> > index e3d352c7b31d4..d15b3eb55ec53 100644
> > --- a/drivers/pci/pci-driver.c
> > +++ b/drivers/pci/pci-driver.c
> > @@ -1297,10 +1297,15 @@ static int pci_pm_restore_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;
> > + pci_power_t prev_state = pci_dev->current_state;
> > + bool skip_bus_pm = pci_dev->skip_bus_pm;
> >
> > pci_pm_default_resume_early(pci_dev);
> > pci_fixup_device(pci_fixup_resume_early, pci_dev);
> >
> > + if (!skip_bus_pm && prev_state == PCI_D3cold)
> > + pci_pm_bridge_power_up_actions(pci_dev);
> > +
> > if (pci_has_legacy_pm_support(pci_dev))
> > return 0;
> >
> > --
> > 2.43.0
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-09-04 13:12 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-31 20:55 [PATCH v3 0/3] Unfiy PCI suspend and hibernate paths Mario Limonciello
2026-08-31 20:55 ` [PATCH v3 1/3] PCI/PM: Split out code from pci_pm_suspend_noirq() into helper Mario Limonciello
2026-09-04 13:03 ` Rafael J. Wysocki (Intel)
2026-08-31 20:55 ` [PATCH v3 2/3] PCI/PM: Run bridge power up actions as part of restore phase Mario Limonciello
2026-09-04 13:09 ` Rafael J. Wysocki (Intel)
2026-09-04 13:11 ` Rafael J. Wysocki (Intel)
2026-08-31 20:55 ` [PATCH v3 3/3] PCI: Put PCIe bridges with downstream devices into D3 at hibernate Mario Limonciello
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox