Re: [PATCH v2 05/30] drm/xe/pm: Add scope-based cleanup helper for runtime PM

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 11/11/2025 12:20 AM, Matt Roper wrote:
> Add a scope-based helpers for runtime PM that may be used to simplify
> cleanup logic and potentially avoid goto-based cleanup.
> 
> For example, using
> 
>         guard(xe_pm_runtime)(xe);
> 

for the record:

last year [1] use of DEFINE_GUARD for our RPM was considered almost as API abuse ;)
but now [2] [3] it is used exactly for such scenarios

[1] https://patchwork.freedesktop.org/patch/599366/?series=134955&rev=1
[2] https://elixir.bootlin.com/linux/v6.18-rc5/source/include/linux/pm_runtime.h#L617
[3] https://elixir.bootlin.com/linux/v6.18-rc5/source/drivers/tty/serial/8250/8250.h#L191

> will get runtime PM and cause a corresponding put to occur automatically
> when the current scope is exited.  'xe_pm_runtime_noresume' can be used
> as a guard replacement for the corresponding 'noresume' variant.
> There's also an xe_pm_runtime_ioctl conditional guard that can be used
> as a replacement for xe_runtime_ioctl():
> 
>         ACQUIRE(xe_pm_runtime_ioctl, pm)(xe);
>         if ((ret = ACQUIRE_ERR(xe_pm_runtime_ioctl, &pm)) < 0)
>                 /* failed */
> 
> In a few rare cases (such as gt_reset_worker()) we need to ensure that
> runtime PM is dropped when the function is exited by any means
> (including error paths), but the function does not need to acquire
> runtime PM because that has already been done earlier by a different
> function.  For these special cases, an 'xe_pm_runtime_release_only'
> guard can be used to handle the release without doing an acquisition.
> 
> These guards will be used in future patches to eliminate some of our
> goto-based cleanup.
> 
> v2:
>  - Specify success condition for xe_pm runtime_ioctl as _RET >= 0 so
>    that positive values will be properly identified as success and
>    trigger destructor cleanup properly.
> 
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_pm.h | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pm.h b/drivers/gpu/drm/xe/xe_pm.h
> index f7f89a18b6fc..6b27039e7b2d 100644
> --- a/drivers/gpu/drm/xe/xe_pm.h
> +++ b/drivers/gpu/drm/xe/xe_pm.h
> @@ -6,6 +6,7 @@
>  #ifndef _XE_PM_H_
>  #define _XE_PM_H_
>  
> +#include <linux/cleanup.h>
>  #include <linux/pm_runtime.h>
>  
>  #define DEFAULT_VRAM_THRESHOLD 300 /* in MB */
> @@ -37,4 +38,20 @@ int xe_pm_block_on_suspend(struct xe_device *xe);
>  void xe_pm_might_block_on_suspend(void);
>  int xe_pm_module_init(void);
>  
> +static inline void __xe_pm_runtime_noop(struct xe_device *xe) {}
> +
> +DEFINE_GUARD(xe_pm_runtime, struct xe_device *,
> +	     xe_pm_runtime_get(_T), xe_pm_runtime_put(_T))
> +DEFINE_GUARD(xe_pm_runtime_noresume, struct xe_device *,
> +	     xe_pm_runtime_get_noresume(_T), xe_pm_runtime_put(_T))
> +DEFINE_GUARD_COND(xe_pm_runtime, _ioctl, xe_pm_runtime_get_ioctl(_T), _RET >= 0)
> +
> +/*
> + * Used when a function needs to release runtime PM in all possible cases
> + * and error paths, but the wakeref was already acquired by a different
> + * function (i.e., get() has already happened so only a put() is needed).
> + */
> +DEFINE_GUARD(xe_pm_runtime_release_only, struct xe_device *,
> +	     __xe_pm_runtime_noop(_T), xe_pm_runtime_put(_T));

maybe instead defining the noop() helper, we can just skip this param:

DEFINE_GUARD(xe_pm_runtime_release_only, struct xe_device *, , xe_pm_runtime_put(_T));

> +
>  #endif

either way,

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>