[PATCH v1] PM: runtime: Update last_busy before calling rpm_suspend() in rpm_idle()

[PATCH v1] PM: runtime: Update last_busy before calling rpm_suspend() in rpm_idle()

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

When rpm_idle() is about to call rpm_suspend(), there are no reasons
preventing the device from being suspended, but if the device uses an
autosuspend delay, it needs to be taken into account.

For this purpose, rpm_idle() adds RPM_AUTO to the flags passed to
rpm_suspend(), but doing that alone is insufficient because the point
in time from which the autosuspend delay is counted may be too early.

For example, if the device has children (or consumers) and the last
of them goes into runtime suspend, the parent may now become eligible
for suspending, but it may not be worthwhile for it to suspend until
a certain time later.

Quoting Alan Stern [1]:

 Let's say the last remaining active child goes into runtime suspend at
 time T.  Let's also say that the parent expects none of the children
 will need to resume until time T + D.  Is D sufficiently large to make
 it worthwhile for the parent to suspend and then resume?  If not, the
 parent should remain active.  If yes, the parent should autosuspend --
 and the length of time the parent should wait before autosuspending is
 on the order of the value of D (let's call it D*) that corresponds to
 this crossover point.  Thus the parent's autosuspend delay should be
 set to D*, or something close to it, in order to achieve both
 objectives.

Moreover, if the parent's autosuspend delay is set to D*, it should be
counted from time T at which the last child has suspended, but that does
not happen because the parent's last_busy timestamp is not updated at
time T.  That leads to undesirable parent suspends occurring between
child suspend and resume [2].

To address this issue, make rpm_idle() update the last_busy timestamp
of the device when it is about to call rpm_suspend() which most often
is right after the device's last child (or consumer) has gone into
runtime suspend.

Updating the last_busy timestamp when rpm_idle() returns an error
without calling rpm_suspend() is not useful because the device is then
going to remain active for an unknown amount of time and its autosuspend
delay needs to take effect later.

Link: https://lore.kernel.org/linux-pm/fc43eb13-816b-4cd0-9711-848fdab605ef@rowland.harvard.edu/ [1]
Link: https://lore.kernel.org/linux-pm/CAJZ5v0hKpjMsGQm8Cpa89x5wkNxqkggDNbp+eQ0S-E52DETWjQ@mail.gmail.com/ [2]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/power/runtime.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -560,7 +560,11 @@ static int rpm_idle(struct device *dev,
 
  out:
 	trace_rpm_return_int(dev, _THIS_IP_, retval);
-	return retval ? retval : rpm_suspend(dev, rpmflags | RPM_AUTO);
+	if (retval)
+		return retval;
+
+	pm_runtime_mark_last_busy(dev);
+	return rpm_suspend(dev, rpmflags | RPM_AUTO);
 }
 
 /**

Re: [PATCH v1] PM: runtime: Update last_busy before calling rpm_suspend() in rpm_idle()

[Brian Norris]

On Fri, Nov 13, 2026 at 02:27:23PM +0100, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> When rpm_idle() is about to call rpm_suspend(), there are no reasons
> preventing the device from being suspended, but if the device uses an
> autosuspend delay, it needs to be taken into account.
> 
> For this purpose, rpm_idle() adds RPM_AUTO to the flags passed to
> rpm_suspend(), but doing that alone is insufficient because the point
> in time from which the autosuspend delay is counted may be too early.
> 
> For example, if the device has children (or consumers) and the last
> of them goes into runtime suspend, the parent may now become eligible
> for suspending, but it may not be worthwhile for it to suspend until
> a certain time later.
> 
> Quoting Alan Stern [1]:
> 
>  Let's say the last remaining active child goes into runtime suspend at
>  time T.  Let's also say that the parent expects none of the children
>  will need to resume until time T + D.  Is D sufficiently large to make
>  it worthwhile for the parent to suspend and then resume?  If not, the
>  parent should remain active.  If yes, the parent should autosuspend --
>  and the length of time the parent should wait before autosuspending is
>  on the order of the value of D (let's call it D*) that corresponds to
>  this crossover point.  Thus the parent's autosuspend delay should be
>  set to D*, or something close to it, in order to achieve both
>  objectives.
> 
> Moreover, if the parent's autosuspend delay is set to D*, it should be
> counted from time T at which the last child has suspended, but that does
> not happen because the parent's last_busy timestamp is not updated at
> time T.  That leads to undesirable parent suspends occurring between
> child suspend and resume [2].
> 
> To address this issue, make rpm_idle() update the last_busy timestamp
> of the device when it is about to call rpm_suspend() which most often
> is right after the device's last child (or consumer) has gone into
> runtime suspend.

Is it worthwhile to note (or at least hint at) how many other things
change along with this? e.g., how pm_runtime_allow() (and therefore
auto > control/power), all non-autosuspend pm_runtime_put() variants,
etc, also extend the autosuspend timer? I think we agreed these are
acceptable effects, and likely even desireable, but I'd say these are
fairly major changes.

In any case, the change looks good to me:

Reviewed-by: Brian Norris <briannorris@chromium.org>

I meant to make time to test this out a bit more, but ran out; maybe
next week.

Thanks,
Brian

> Updating the last_busy timestamp when rpm_idle() returns an error
> without calling rpm_suspend() is not useful because the device is then
> going to remain active for an unknown amount of time and its autosuspend
> delay needs to take effect later.
> 
> Link: https://lore.kernel.org/linux-pm/fc43eb13-816b-4cd0-9711-848fdab605ef@rowland.harvard.edu/ [1]
> Link: https://lore.kernel.org/linux-pm/CAJZ5v0hKpjMsGQm8Cpa89x5wkNxqkggDNbp+eQ0S-E52DETWjQ@mail.gmail.com/ [2]
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/base/power/runtime.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -560,7 +560,11 @@ static int rpm_idle(struct device *dev,
>  
>   out:
>  	trace_rpm_return_int(dev, _THIS_IP_, retval);
> -	return retval ? retval : rpm_suspend(dev, rpmflags | RPM_AUTO);
> +	if (retval)
> +		return retval;
> +
> +	pm_runtime_mark_last_busy(dev);
> +	return rpm_suspend(dev, rpmflags | RPM_AUTO);
>  }
>  
>  /**
> 
> 
>