| Message ID | 20250410153106.4146265-4-sakari.ailus@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Update last busy timestamp in Runtime PM autosuspend callbacks | expand |
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); > } > > /**
Hi Laurent, Thanks for the review. On Thu, Apr 10, 2025 at 11:17:11PM +0300, Laurent Pinchart wrote: > 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/ Ack. I'll address this in v2, but wait for other comments still awhile. > > 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); > > } > > > > /**
On Fri, Apr 11, 2025 at 06:27:06AM +0000, Sakari Ailus wrote: > > s/ and decrement/, decrement/ > > Ack. I'll address this in v2, but wait for other comments still awhile. The paragraph now looks like: * Update the last access time of @dev, decrement 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(). I.e. runtime PM usage counter of @dev is decremented, not the usage counter of the last access time.
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 + * 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); } /**
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(-)