Instead of finding bugs where we may or may not release force_wake, I've decided to be inspired by the spinlock guards, and use the same ones to do xe_force_wake handling. Examples are added as documentation in xe_force_wake.c Signed-off-by: Maarten Lankhorst <dev@xxxxxxxxxxxx> --- drivers/gpu/drm/xe/xe_force_wake.c | 51 ++++++++++++++++++++++++++++++ drivers/gpu/drm/xe/xe_force_wake.h | 15 +++++++++ 2 files changed, 66 insertions(+) diff --git a/drivers/gpu/drm/xe/xe_force_wake.c b/drivers/gpu/drm/xe/xe_force_wake.c index 4f6784e5abf88..805c19f6de9e7 100644 --- a/drivers/gpu/drm/xe/xe_force_wake.c +++ b/drivers/gpu/drm/xe/xe_force_wake.c @@ -16,6 +16,57 @@ #define XE_FORCE_WAKE_ACK_TIMEOUT_MS 50 +/** + * DOC: Force wake handling + * + * Traditionally, the force wake handling has been done using the error prone + * set of calls: + * + * int func(struct xe_force_wake *fw) + * { + * unsigned int fw_ref = xe_force_wake_get(fw, XE_FORCEWAKE_ALL); + * if (!fw_ref) + * return -ETIMEDOUT; + * + * err = do_something(); + * + * xe_force_wake_put(fw, fw_ref); + * return err; + * } + * + * A new, failure-safe approach is by using the scoped helpers, + * which changes the function to this: + * + * int func(struct xe_force_wake *fw) + * { + * scoped_cond_guard(xe_force_wake_get, return -ETIMEDOUT, fw, XE_FORCEWAKE_ALL) { + * return do_something(); + * } + * } + * + * For completeness, the following options also work: + * void func(struct xe_force_wake *fw) + * { + * scoped_guard(xe_force_wake_get, fw, XE_FORCEWAKE_ALL) { + * do_something_only_if_fw_acquired(); + * } + * } + * + * You can use xe_force_wake instead of force_wake_get, if the code + * must run but errors acquiring ignored: + * void func(struct xe_force_wake *fw) + * { + * scoped_guard(xe_force_wake, fw, XE_FORCEWAKE_ALL) { + * always_do_something_maybe_fw(); + * } + * + * do_something_no_fw(); + * + * guard(xe_force_wake)(fw, XE_FORCEWAKE_ALL); + * always_do_something_maybe_fw(); + * } + */ + static const char *str_wake_sleep(bool wake) { return wake ? "wake" : "sleep"; diff --git a/drivers/gpu/drm/xe/xe_force_wake.h b/drivers/gpu/drm/xe/xe_force_wake.h index 0e3e84bfa51c3..0fb1baae0a3a3 100644 --- a/drivers/gpu/drm/xe/xe_force_wake.h +++ b/drivers/gpu/drm/xe/xe_force_wake.h @@ -9,6 +9,8 @@ #include "xe_assert.h" #include "xe_force_wake_types.h" +#include <linux/cleanup.h> + struct xe_gt; void xe_force_wake_init_gt(struct xe_gt *gt, @@ -61,4 +63,17 @@ xe_force_wake_ref_has_domain(unsigned int fw_ref, enum xe_force_wake_domains dom return fw_ref & domain; } +DEFINE_LOCK_GUARD_1(xe_force_wake, struct xe_force_wake, + _T->fw_ref = xe_force_wake_get(_T->lock, domain), + xe_force_wake_put(_T->lock, _T->fw_ref), + unsigned int fw_ref, enum xe_force_wake_domains domain); + +DEFINE_LOCK_GUARD_1_COND(xe_force_wake, _get, + _T->fw_ref = xe_force_wake_get_all(_T->lock, domain), + enum xe_force_wake_domains domain); + +/* Only useful for guard xe_force_wake, guard xe_force_wake_get gets all or nothing */ +#define xe_force_wake_scope_has_domain(domain) \ + (xe_force_wake_ref_has_domain(scope.fw_ref, domain)) + #endif -- 2.47.1