Re: [PATCH 3/7] pm: runtime: Mark last busy stamp in pm_runtime_put_autosuspend()

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Sakari,

Thank you for the patch.

On Thu, Apr 10, 2025 at 06:31:02PM +0300, Sakari Ailus wrote:
> Set device's last busy timestamp to current time in
> pm_runtime_put_autosuspend(). Callers wishing not to do that will need to
> use __pm_runtime_put_autosuspend().
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  Documentation/power/runtime_pm.rst | 23 ++++++++++-------------
>  include/linux/pm_runtime.h         | 12 +++++++-----
>  2 files changed, 17 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/power/runtime_pm.rst b/Documentation/power/runtime_pm.rst
> index 63344bea8393..e7bbdc66d64c 100644
> --- a/Documentation/power/runtime_pm.rst
> +++ b/Documentation/power/runtime_pm.rst
> @@ -411,8 +411,9 @@ drivers/base/power/runtime.c and include/linux/pm_runtime.h:
>        pm_request_idle(dev) and return its result
>  
>    `int pm_runtime_put_autosuspend(struct device *dev);`
> -    - does the same as __pm_runtime_put_autosuspend() for now, but in the
> -      future, will also call pm_runtime_mark_last_busy() as well, DO NOT USE!
> +    - set the power.last_busy field to the current time and decrement the
> +      device's usage counter; if the result is 0 then run
> +      pm_request_autosuspend(dev) and return its result
>  
>    `int __pm_runtime_put_autosuspend(struct device *dev);`
>      - decrement the device's usage counter; if the result is 0 then run
> @@ -870,11 +871,9 @@ device is automatically suspended (the subsystem or driver still has to call
>  the appropriate PM routines); rather it means that runtime suspends will
>  automatically be delayed until the desired period of inactivity has elapsed.
>  
> -Inactivity is determined based on the power.last_busy field.  Drivers should
> -call pm_runtime_mark_last_busy() to update this field after carrying out I/O,
> -typically just before calling __pm_runtime_put_autosuspend().  The desired
> -length of the inactivity period is a matter of policy.  Subsystems can set this
> -length initially by calling pm_runtime_set_autosuspend_delay(), but after device
> +Inactivity is determined based on the power.last_busy field. The desired length
> +of the inactivity period is a matter of policy.  Subsystems can set this length
> +initially by calling pm_runtime_set_autosuspend_delay(), but after device
>  registration the length should be controlled by user space, using the
>  /sys/devices/.../power/autosuspend_delay_ms attribute.
>  
> @@ -885,7 +884,7 @@ instead of the non-autosuspend counterparts::
>  
>  	Instead of: pm_runtime_suspend    use: pm_runtime_autosuspend;
>  	Instead of: pm_schedule_suspend   use: pm_request_autosuspend;
> -	Instead of: pm_runtime_put        use: __pm_runtime_put_autosuspend;
> +	Instead of: pm_runtime_put        use: pm_runtime_put_autosuspend;
>  	Instead of: pm_runtime_put_sync   use: pm_runtime_put_sync_autosuspend.
>  
>  Drivers may also continue to use the non-autosuspend helper functions; they
> @@ -922,12 +921,10 @@ Here is a schematic pseudo-code example::
>  	foo_io_completion(struct foo_priv *foo, void *req)
>  	{
>  		lock(&foo->private_lock);
> -		if (--foo->num_pending_requests == 0) {
> -			pm_runtime_mark_last_busy(&foo->dev);
> -			__pm_runtime_put_autosuspend(&foo->dev);
> -		} else {
> +		if (--foo->num_pending_requests == 0)
> +			pm_runtime_put_autosuspend(&foo->dev);
> +		else
>  			foo_process_next_request(foo);
> -		}
>  		unlock(&foo->private_lock);
>  		/* Send req result back to the user ... */
>  	}
> diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
> index 3e31cbebc527..0ade3f75d903 100644
> --- a/include/linux/pm_runtime.h
> +++ b/include/linux/pm_runtime.h
> @@ -562,11 +562,13 @@ static inline int __pm_runtime_put_autosuspend(struct device *dev)
>  }
>  
>  /**
> - * pm_runtime_put_autosuspend - Drop device usage counter and queue autosuspend if 0.
> + * pm_runtime_put_autosuspend - Update the last access time of a device, drop
> + * its usage counter and queue autosuspend if the usage counter becomes 0.
>   * @dev: Target device.
>   *
> - * Decrement the runtime PM usage counter of @dev and if it turns out to be
> - * equal to 0, queue up a work item for @dev like in pm_request_autosuspend().
> + * Update the last access time of @dev and decrement its runtime PM usage

s/ and decrement/, decrement/

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> + * counter and if it turns out to be equal to 0, queue up a work item for @dev
> + * like in pm_request_autosuspend().
>   *
>   * Return:
>   * * 0: Success.
> @@ -581,8 +583,8 @@ static inline int __pm_runtime_put_autosuspend(struct device *dev)
>   */
>  static inline int pm_runtime_put_autosuspend(struct device *dev)
>  {
> -	return __pm_runtime_suspend(dev,
> -	    RPM_GET_PUT | RPM_ASYNC | RPM_AUTO);
> +	pm_runtime_mark_last_busy(dev);
> +	return __pm_runtime_put_autosuspend(dev);
>  }
>  
>  /**

-- 
Regards,

Laurent Pinchart