[RFC PATCH] PM: sleep: Allow direct-complete for wakeup devices in suspend-to-idle

[RFC PATCH] PM: sleep: Allow direct-complete for wakeup devices in suspend-to-idle

[Paul Menzel]

Since commit dc351d4c5f4f ("PM / core: Propagate dev->power.wakeup_path
when no callbacks"), device_suspend() clears direct_complete for every
device with wakeup enabled or in the wakeup path of a child.  That commit
addressed a device without PM callbacks, where no ->prepare() callback has
looked at the device, but the check applies to all devices.

For PCI devices, ->prepare() already takes wakeup into account:
pci_pm_prepare() only returns 1 if pci_dev_need_resume() is false, which
compares device_may_wakeup() with the ACPI wakeup configuration the device
got for runtime PM (acpi_pci_need_resume()) and computes the target state
with pci_target_state(dev, device_may_wakeup(dev)), and it then adjusts
PME with pci_dev_adjust_pme().  For a device with wakeup enabled, this
decision is discarded right afterwards.  As PCI bridges that can do D3
(pci_acpi_setup()), PCI USB host controllers (usb_hcd_pci_probe()) and
Thunderbolt NHIs (nhi_probe()) enable wakeup by default, direct-complete is
effectively limited to devices with wakeup disabled on x86 laptops.  On a
Dell XPS 13 9310 (Tiger Lake) these runtime-suspended functions are
affected:

    0000:00:07.0 pcieport     D3cold  wakeup=enabled
    0000:00:07.2 pcieport     D3cold  wakeup=enabled
    0000:00:0d.0 xhci_hcd     D3cold  wakeup=enabled
    0000:00:0d.2 thunderbolt  D3cold  wakeup=enabled
    0000:00:0d.3 thunderbolt  D3cold  wakeup=enabled
    0000:00:14.0 xhci_hcd     D3hot   wakeup=enabled

There, every suspend-to-idle resume spends 231 ms of the noirq phase
powering up the empty Thunderbolt root ports 00:07.0 and 00:07.2 and the
NHIs 00:0d.2 and 00:0d.3 from D3cold (ACPI _ON of the TBT0/TBT1 power
resources ~125 ms, root port _PS0 ~108 ms), only to runtime-suspend them
again after the resume.  That is more than a third of the kernel resume
(641 ms with the panel lit, 427 ms with it off, from the end of
machine_suspend to "PM: suspend exit"):

    230.77 ms  noirq bus  pcieport     0000:00:07.2
    227.40 ms  noirq bus  pcieport     0000:00:07.0
    142.53 ms  noirq bus  thunderbolt  0000:00:0d.3
    137.27 ms  noirq bus  thunderbolt  0000:00:0d.2

In suspend-to-idle no firmware is involved in the transition, and a device
left in runtime suspend keeps the wakeup configuration it got for runtime
PM.  So allow direct-complete for wakeup devices there, and propagate
wakeup_path from a direct-completed device to its parent, as
device_suspend() does for devices that go through the callbacks.

Together with three more changes the Thunderbolt functions stay in D3cold,
and the noirq phase takes 33 ms instead of 232 ms with the panel off, and
34 ms instead of 231 ms with the panel lit (medians of 3-4 cycles).

Fixes: dc351d4c5f4f ("PM / core: Propagate dev->power.wakeup_path when no callbacks")
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---

Notes:

Other three changes:

- PCI/portdrv: no DPM_FLAG_NO_DIRECT_COMPLETE for ports on a root bus, as
  the reason for it in commit 52be9464aa7e ("PCI/portdrv: Resume upon exit
  from system suspend if left runtime suspended"), a parent going through
  D3cold, does not apply to them,
- thunderbolt: nhi_complete() does not runtime-resume a direct-completed
  NHI,
- ACPI: power: __acpi_power_on() resumes only dependents whose _PR0
  resources are all on.  Otherwise resuming the xHCI 00:0d.0 during system
  suspend (xhci-pci does not set DPM_FLAG_SMART_SUSPEND) turns on the
  shared D3C resource and runtime-resumes the root ports and NHIs, which
  also need TBT0/TBT1.

This is an RFC, because the runtime wakeup configuration of a device can
differ from the one its driver sets up for system sleep.  For example,
tb_switch_suspend() also arms wake on DisplayPort for runtime suspend, and
if user space disables wakeup for a Thunderbolt router, a direct-completed
NHI still wakes up the system when a device is connected.  Maybe
subsystems should opt in with a flag instead.

An alternative is to keep the check for devices relying on
no_pm_callbacks, where nothing has looked at the wakeup settings, to drop
it for devices whose ->prepare() returned a positive value, and to
propagate wakeup_path from direct-completed devices in both cases.  That
works for all sleep states, but does not help in the example above: the
Thunderbolt domain and host router devices (domain0, 0-0) below the NHI
have no PM callbacks, but wakeup enabled, so they still go through
device_suspend() and clear direct_complete of the NHI with
dpm_clear_superiors_direct_complete().

Tested on the Dell XPS 13 9310 with Linux 7.4-rc1 and the three changes
above (plus a fix that keeps the absent root ports TRP1/TRP3 from being
powered up at boot, without which TBT0/TBT1 never turn off): 17
suspend-to-idle cycles of 6-8 s with rtcwake and sleepgraph, no failures,
111 s in hardware sleep in total (/sys/power/suspend_stats/total_hw_sleep).
Hot-plugging during suspend was not tested.

Tool use (Documentation/process/generated-content.rst): the analysis, the
code change and this changelog were drafted by an LLM coding assistant
(Claude Opus 5.5 via Claude Code), in a session asked to "instrument S0ix
resume [on the Dell XPS 13 9310], and halve the time".  It traced the
resume with sleepgraph, function_graph and kprobes/rpm tracepoints, built
the test kernels and ran the measurements on the laptop.

 drivers/base/power/main.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 88a31e09e4f3f..f36c38d1585b0 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1999,8 +1999,14 @@ static void device_suspend(struct device *dev, pm_message_t state, bool async)
 	if (dev->power.syscore)
 		goto Complete;
 
-	/* Avoid direct_complete to let wakeup_path propagate. */
-	if (device_may_wakeup(dev) || device_wakeup_path(dev))
+	/*
+	 * Avoid direct_complete to let wakeup_path propagate.  In
+	 * suspend-to-idle, though, no firmware is involved and a device left in
+	 * runtime suspend keeps the wakeup configuration it got for runtime PM,
+	 * so allow direct_complete there and propagate wakeup_path below.
+	 */
+	if ((device_may_wakeup(dev) || device_wakeup_path(dev)) &&
+	    pm_suspend_target_state != PM_SUSPEND_TO_IDLE)
 		dev->power.direct_complete = false;
 
 	if (dev->power.direct_complete) {
@@ -2009,6 +2015,10 @@ static void device_suspend(struct device *dev, pm_message_t state, bool async)
 			if (pm_runtime_status_suspended(dev)) {
 				pm_dev_dbg(dev, state, "direct-complete ");
 				dev->power.is_suspended = true;
+				if (device_may_wakeup(dev))
+					dev->power.wakeup_path = true;
+
+				dpm_propagate_wakeup_to_parent(dev);
 				goto Complete;
 			}
 

base-commit: 5fcaa701fd2d61b003dff89bbb9dd29ed6123740
-- 
2.54.0

Re: [RFC PATCH] PM: sleep: Allow direct-complete for wakeup devices in suspend-to-idle

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential
issues to consider:
- [High] PM: sleep: device_suspend() direct_complete in s2idle breaks
  wakeup setup
- [High] PM: sleep: device_suspend() direct_complete ignores late
  wakeup_path
- [High] PM: sleep: direct_complete disables runtime PM during s2idle
  wakeup resume
- [Medium] PM: sleep: direct-completed Thunderbolt NHI resumes in
  nhi_complete()
--

commit 483a192c54e90f429063c4920d07666e5377cd77
Author: Paul Menzel <pmenzel@molgen.mpg.de>

PM: sleep: Allow direct-complete for wakeup devices in suspend-to-idle

This commit allows runtime-suspended wakeup devices and devices on a wakeup
path to use direct-complete during suspend-to-idle and propagates
wakeup_path to their parents.

> For PCI devices, ->prepare() already takes wakeup into account:
> pci_pm_prepare() only returns 1 if pci_dev_need_resume() is false, which
> compares device_may_wakeup() with the ACPI wakeup configuration the device
> got for runtime PM (acpi_pci_need_resume()) and computes the target state
> with pci_target_state(dev, device_may_wakeup(dev)), and it then adjusts
> PME with pci_dev_adjust_pme().  For a device with wakeup enabled, this
> decision is discarded right afterwards.  As PCI bridges that can do D3
> (pci_acpi_setup()), PCI USB host controllers (usb_hcd_pci_probe()) and
> Thunderbolt NHIs (nhi_probe()) enable wakeup by default, direct-complete is
> effectively limited to devices with wakeup disabled on x86 laptops.  On a
> Dell XPS 13 9310 (Tiger Lake) these runtime-suspended functions are
> affected:
>
>     0000:00:07.0 pcieport     D3cold  wakeup=enabled
>     0000:00:07.2 pcieport     D3cold  wakeup=enabled
>     0000:00:0d.0 xhci_hcd     D3cold  wakeup=enabled
>     0000:00:0d.2 thunderbolt  D3cold  wakeup=enabled
>     0000:00:0d.3 thunderbolt  D3cold  wakeup=enabled
>     0000:00:14.0 xhci_hcd     D3hot   wakeup=enabled

[Severity: Medium]
With this change alone, does a direct-completed Thunderbolt NHI avoid being
powered back up to D0 on resume?  Because direct-complete skips system
resume callbacks, the NHI reaches dpm_complete() still runtime-suspended,
where nhi_complete() synchronously resumes it from D3cold anyway:

drivers/thunderbolt/nhi.c:nhi_complete() {
	if (pm_runtime_suspended(dev))
		pm_runtime_resume(dev);
	else
		tb_domain_complete(tb);
}

Also, is the commit message accurate that pcieport and xhci_hcd have their
pci_pm_prepare() decision discarded by the wakeup check in
device_suspend()?  pcie_portdrv_probe() sets DPM_FLAG_NO_DIRECT_COMPLETE,
so device_prepare() never enables direct_complete for pcieport, and
xhci_hcd has child USB root hub devices whose usb_dev_prepare() returns 0,
which clears direct_complete on xhci_hcd via
dpm_clear_superiors_direct_complete().

> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index 88a31e09e4f3f..f36c38d1585b0 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -1999,8 +1999,14 @@ static void device_suspend(struct device *dev, pm_message_t state, bool async)
>  	if (dev->power.syscore)
>  		goto Complete;
>  
> -	/* Avoid direct_complete to let wakeup_path propagate. */
> -	if (device_may_wakeup(dev) || device_wakeup_path(dev))
> +	/*
> +	 * Avoid direct_complete to let wakeup_path propagate.  In
> +	 * suspend-to-idle, though, no firmware is involved and a device left in
> +	 * runtime suspend keeps the wakeup configuration it got for runtime PM,
> +	 * so allow direct_complete there and propagate wakeup_path below.
> +	 */
> +	if ((device_may_wakeup(dev) || device_wakeup_path(dev)) &&
> +	    pm_suspend_target_state != PM_SUSPEND_TO_IDLE)
>  		dev->power.direct_complete = false;
>  
>  	if (dev->power.direct_complete) {

[Severity: High]
Does allowing direct_complete for all wakeup-enabled devices in
PM_SUSPEND_TO_IDLE cause a regression for drivers that only arm wakeup
interrupts or adjust wakeup masks in their system suspend callbacks?

Even in suspend-to-idle, dpm_suspend_noirq() calls suspend_device_irqs(),
which disables any device IRQ not marked with enable_irq_wake().  Several
subsystems return 1 from ->prepare() for runtime-suspended devices in S0
(such as acpi_subsys_prepare() via acpi_dev_needs_resume() and
dwc3_pm_prepare()), while their drivers only call enable_irq_wake() in
->suspend():

drivers/bluetooth/hci_bcm.c:bcm_suspend() {
	...
	if (device_may_wakeup(dev) && bdev->irq > 0) {
		error = enable_irq_wake(bdev->irq);
		if (!error)
			bt_dev_dbg(bdev, "BCM irq: enabled");
	}
}

Skipping ->suspend() via direct_complete leaves those wakeup IRQs unarmed
so suspend_device_irqs() disables them and the device cannot wake the
system from s2idle.

Conversely, drivers such as thunderbolt (tb_switch_suspend() and
usb4_switch_set_wake()) and r8169 (rtl8169_runtime_suspend()) enable wider
wakeup events for runtime suspend than for system sleep (for example,
wake-on-connect, wake-on-disconnect, DisplayPort wake, or WAKE_PHY when
carrier is off) even when wakeup is disabled in sysfs or via ethtool.
Skipping system suspend leaves those runtime-only wakeup events armed
during s2idle.

[Severity: High]
When a direct-completed device wakes the system from suspend-to-idle, does
disabling runtime PM across the direct_complete path drop the wakeup
resume request?

Taking direct_complete in device_suspend() calls pm_runtime_disable(dev),
and device_resume_noirq() and device_resume_early() both skip
pm_runtime_enable(dev) when dev->power.direct_complete is set, leaving
runtime PM disabled until device_resume() runs in dpm_resume().  However,
s2idle wakeup notifications and wakeup IRQs are dispatched before
dpm_resume() (for example, acpi_s2idle_wake() flushes pci_acpi_wake_dev()
during platform_resume_noirq(), and dpm_resume_noirq() runs
resume_device_irqs() and handle_threaded_wake_irq()).  While
dev->power.disable_depth > 0 and the device is RPM_SUSPENDED, rpm_resume()
rejects both pm_request_resume() and pm_runtime_resume() with -EACCES
without queuing RPM_REQ_RESUME, so hardware PME/wakeup status is cleared
while the waking device stays runtime-suspended after resume completes.

> @@ -2009,6 +2015,10 @@ static void device_suspend(struct device *dev, pm_message_t state, bool async)
>  			if (pm_runtime_status_suspended(dev)) {
>  				pm_dev_dbg(dev, state, "direct-complete ");
>  				dev->power.is_suspended = true;
> +				if (device_may_wakeup(dev))
> +					dev->power.wakeup_path = true;
> +
> +				dpm_propagate_wakeup_to_parent(dev);
>  				goto Complete;
>  			}
>  

[Severity: High]
Can propagating wakeup_path from a direct-completed child without clearing
direct_complete on its ancestors leave the parent misconfigured for child
wakeup in suspend-to-idle?

dpm_prepare() runs top-down and clears dev->power.wakeup_path before
invoking ->prepare(), so a parent's ->prepare() always sees
device_wakeup_path(parent) == false.  If a runtime-suspended parent has
device_may_wakeup(parent) == false while its child has wakeup enabled, the
parent's ->prepare() configures the parent assuming wakeup is not needed.
For example, on a PCI parent in D1/D2/D3hot, pci_pm_prepare() calls
pci_dev_adjust_pme(), which disables PME before returning 1:

drivers/pci/pci.c:pci_dev_adjust_pme() {
	if (pm_runtime_suspended(dev) && !device_may_wakeup(dev) &&
	    pci_dev->current_state < PCI_D3cold)
		__pci_pme_active(pci_dev, false);
}

Similarly, in serial_core, wakeup is enabled on the child tty_dev (which
has no_pm_callbacks set), while the parent UART port calls
enable_irq_wake(uport->irq) in uart_suspend_port() during ->suspend().
Because dpm_clear_superiors_direct_complete() is not called and
device_suspend() no longer clears direct_complete when
device_wakeup_path(dev) is set in PM_SUSPEND_TO_IDLE, the parent also
takes direct_complete and child wakeup fails.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114001551.219620-1-pmenzel@molgen.mpg.de?part=1