Re: [PATCH v3 02/27] drm/xe/pm: Add scope-based cleanup helper for runtime PM

[Gustavo Sousa <gustavo.sousa@intel.com>]

Quoting Matt Roper (2025-11-14 18:43:38-03:00)
>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);
>
>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.
>
>v3:
> - Add comments to the kerneldoc for the existing 'get' functions
>   indicating that scope-based handling should be preferred where
>   possible.  (Gustavo)
>
>Cc: Gustavo Sousa <gustavo.sousa@intel.com>
>Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

>---
> drivers/gpu/drm/xe/xe_pm.c | 21 +++++++++++++++++++++
> drivers/gpu/drm/xe/xe_pm.h | 17 +++++++++++++++++
> 2 files changed, 38 insertions(+)
>
>diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
>index 0e573fb11bbb..ad92a142dc7e 100644
>--- a/drivers/gpu/drm/xe/xe_pm.c
>+++ b/drivers/gpu/drm/xe/xe_pm.c
>@@ -722,6 +722,13 @@ static void xe_pm_runtime_lockdep_prime(void)
> /**
>  * xe_pm_runtime_get - Get a runtime_pm reference and resume synchronously
>  * @xe: xe device instance
>+ *
>+ * When possible, scope-based runtime PM (through guard(xe_pm_runtime)) is
>+ * be preferred over direct usage of this function.  Manual get/put handling
>+ * should only be used when the function contains goto-based logic which
>+ * can break scope-based handling, or when the lifetime of the runtime PM
>+ * reference does not match a specific scope (e.g., runtime PM obtained in one
>+ * function and released in a different one).
>  */
> void xe_pm_runtime_get(struct xe_device *xe)
> {
>@@ -754,6 +761,13 @@ void xe_pm_runtime_put(struct xe_device *xe)
>  * xe_pm_runtime_get_ioctl - Get a runtime_pm reference before ioctl
>  * @xe: xe device instance
>  *
>+ * When possible, scope-based runtime PM (through
>+ * ACQUIRE(xe_pm_runtime_ioctl, ...)) is be preferred over direct usage of this
>+ * function.  Manual get/put handling should only be used when the function
>+ * contains goto-based logic which can break scope-based handling, or when the
>+ * lifetime of the runtime PM reference does not match a specific scope (e.g.,
>+ * runtime PM obtained in one function and released in a different one).
>+ *
>  * Returns: Any number greater than or equal to 0 for success, negative error
>  * code otherwise.
>  */
>@@ -823,6 +837,13 @@ static bool xe_pm_suspending_or_resuming(struct xe_device *xe)
>  * It will warn if not protected.
>  * The reference should be put back after this function regardless, since it
>  * will always bump the usage counter, regardless.
>+ *
>+ * When possible, scope-based runtime PM (through guard(xe_pm_runtime_noresume))
>+ * is be preferred over direct usage of this function.  Manual get/put handling
>+ * should only be used when the function contains goto-based logic which can
>+ * break scope-based handling, or when the lifetime of the runtime PM reference
>+ * does not match a specific scope (e.g., runtime PM obtained in one function
>+ * and released in a different one).
>  */
> void xe_pm_runtime_get_noresume(struct xe_device *xe)
> {
>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));
>+
> #endif
>-- 
>2.51.1
>