Message-ID: <CAPDyKFr8LpkR+DK2suNzGaBzuQ+gohYML4TC-FS+oCUWr6TTzA@mail.gmail.com>
Date: Thu, 26 Jun 2025 12:26:34 +0200
From: Ulf Hansson <ulf.hansson@...aro.org>
To: "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc: Linux PM <linux-pm@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>, 
	Linux ACPI <linux-acpi@...r.kernel.org>, Linux PCI <linux-pci@...r.kernel.org>, 
	Mika Westerberg <mika.westerberg@...ux.intel.com>
Subject: Re: [PATCH v1 3/9] PM: Make pm_runtime_force_resume() work with DPM_FLAG_SMART_SUSPEND

On Wed, 25 Jun 2025 at 21:25, Rafael J. Wysocki <rjw@...ysocki.net> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> Curently, drivers using pm_runtime_force_suspend/resume() cannot set
> DPM_FLAG_SMART_SUSPEND because the devices with that flag set may need
> to be resumed during system-wide resume regardless of whether or not
> they have power.needs_force_resume set.  That can happen due to a
> dependency resolved at the beginning of a system-wide resume transition
> (for instance, a bus type or PM domain has decided to resume a
> subordinate device with DPM_FLAG_SMART_SUSPEND and its parent and
> suppliers also need to be resumed).
>
> To overcome this limitation, modify pm_runtime_force_resume() to check
> the device's power.smart_suspend flag (which is set for devices with
> DPM_FLAG_SMART_SUSPEND set that meet some additional requirements) and
> the device's runtime PM status in addition to power.needs_force_resume.
> Also change it to clear power.smart_suspend to ensure that it will not
> handle the same device twice during one transition.
>
> The underlying observation is that there are two cases in which the
> device needs to be resumed by pm_runtime_force_resume().  One of them
> is when the device has power.needs_force_resume set, which means that
> pm_runtime_force_suspend() has suspended it and decided that it should
> be resumed during the subsequent system resume.  The other one is when
> power.smart_suspend is set and the device's runtume PM status is
> RPM_ACTIVE.
>
> Update kerneldoc comments in accordance with the code changes.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>

Looks good to me!

Reviewed-by: Ulf Hansson <ulf.hansson@...aro.org>

Kind regards
Uffe

> ---
>  drivers/base/power/runtime.c |   38 +++++++++++++++++++++++++++-----------
>  1 file changed, 27 insertions(+), 11 deletions(-)
>
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -1964,10 +1964,6 @@
>   * sure the device is put into low power state and it should only be used during
>   * system-wide PM transitions to sleep states.  It assumes that the analogous
>   * pm_runtime_force_resume() will be used to resume the device.
> - *
> - * Do not use with DPM_FLAG_SMART_SUSPEND as this can lead to an inconsistent
> - * state where this function has called the ->runtime_suspend callback but the
> - * PM core marks the driver as runtime active.
>   */
>  int pm_runtime_force_suspend(struct device *dev)
>  {
> @@ -2014,20 +2010,28 @@
>   * pm_runtime_force_resume - Force a device into resume state if needed.
>   * @dev: Device to resume.
>   *
> - * Prior invoking this function we expect the user to have brought the device
> - * into low power state by a call to pm_runtime_force_suspend(). Here we reverse
> - * those actions and bring the device into full power, if it is expected to be
> - * used on system resume.  In the other case, we defer the resume to be managed
> - * via runtime PM.
> + * This function expects that either pm_runtime_force_suspend() has put the
> + * device into a low-power state prior to calling it, or the device had been
> + * runtime-suspended before the preceding system-wide suspend transition and it
> + * was left in suspend during that transition.
> + *
> + * The actions carried out by pm_runtime_force_suspend(), or by a runtime
> + * suspend in general, are reversed and the device is brought back into full
> + * power if it is expected to be used on system resume, which is the case when
> + * its needs_force_resume flag is set or when its smart_suspend flag is set and
> + * its runtime PM status is "active".
> + *
> + * In other cases, the resume is deferred to be managed via runtime PM.
>   *
> - * Typically this function may be invoked from a system resume callback.
> + * Typically, this function may be invoked from a system resume callback.
>   */
>  int pm_runtime_force_resume(struct device *dev)
>  {
>         int (*callback)(struct device *);
>         int ret = 0;
>
> -       if (!dev->power.needs_force_resume)
> +       if (!dev->power.needs_force_resume && (!dev_pm_smart_suspend(dev) ||
> +           pm_runtime_status_suspended(dev)))
>                 goto out;
>
>         callback = RPM_GET_CALLBACK(dev, runtime_resume);
> @@ -2041,8 +2045,20 @@
>         }
>
>         pm_runtime_mark_last_busy(dev);
> +
>  out:
> +       /*
> +        * The smart_suspend flag can be cleared here because it is not going
> +        * to be necessary until the next system-wide suspend transition that
> +        * will update it again.
> +        */
> +       dev->power.smart_suspend = false;
> +       /*
> +        * Also clear needs_force_resume to make this function skip devices that
> +        * have been seen by it once.
> +        */
>         dev->power.needs_force_resume = false;
> +
>         pm_runtime_enable(dev);
>         return ret;
>  }
>
>
>

Powered by blists - more mailing lists