From: Matt Roper <matthew.d.roper@intel.com>
To: intel-xe@lists.freedesktop.org
Cc: matthew.d.roper@intel.com,
Michal Wajdeczko <michal.wajdeczko@intel.com>,
Gustavo Sousa <gustavo.sousa@intel.com>
Subject: [PATCH v3 01/27] drm/xe/forcewake: Add scope-based cleanup for forcewake
Date: Fri, 14 Nov 2025 13:43:37 -0800 [thread overview]
Message-ID: <20251114214335.2388972-30-matthew.d.roper@intel.com> (raw)
In-Reply-To: <20251114214335.2388972-29-matthew.d.roper@intel.com>
Since forcewake uses a reference counting get/put model, there are many
places where we need to be careful to drop the forcewake reference when
bailing out of a function early on an error path. Add scope-based
cleanup options that can be used in place of explicit get/put to help
prevent mistakes in this area.
Examples:
CLASS(xe_force_wake, fw_ref)(gt_to_fw(gt), XE_FW_GT);
Obtain forcewake on the XE_FW_GT domain and hold it until the
end of the current block. The wakeref will be dropped
automatically when the current scope is exited by any means
(return, break, reaching the end of the block, etc.).
xe_with_force_wake(fw_ref, gt_to_fw(ss->gt), XE_FORCEWAKE_ALL) {
...
}
Hold all forcewake domains for the following block. As with the
CLASS usage, forcewake will be dropped automatically when the
block is exited by any means.
Use of these cleanup helpers should allow us to remove some ugly
goto-based error handling and help avoid mistakes in functions with lots
of early error exits.
An 'xe_force_wake_release_only' class is also added for cases where a
forcewake reference is passed in from another function and the current
function is responsible for releasing it in every flow and error path.
v2:
- Create a separate constructor that just wraps xe_force_wake_get for
use in the class. This eliminates the need to update the signature
of xe_force_wake_get(). (Michal)
v3:
- Wrap xe_with_force_wake's 'done' marker in __UNIQUE_ID. (Gustavo)
- Add a note to xe_force_wake_get()'s kerneldoc explaining that
scope-based cleanup is preferred when possible. (Gustavo)
- Add an xe_force_wake_release_only class. (Gustavo)
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Gustavo Sousa <gustavo.sousa@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/xe/xe_force_wake.c | 7 ++++++
drivers/gpu/drm/xe/xe_force_wake.h | 40 ++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_force_wake.c b/drivers/gpu/drm/xe/xe_force_wake.c
index c59a9b330697..76e054f314ee 100644
--- a/drivers/gpu/drm/xe/xe_force_wake.c
+++ b/drivers/gpu/drm/xe/xe_force_wake.c
@@ -166,6 +166,13 @@ static int domain_sleep_wait(struct xe_gt *gt,
* xe_force_wake_ref_has_domain() function. Caller must call
* xe_force_wake_put() function to decrease incremented refcounts.
*
+ * When possible, scope-based forcewake (through CLASS(xe_force_wake, ...) or
+ * xe_with_force_wake()) should be used instead of direct calls to this
+ * function. Direct usage of get/put should only be used when the function
+ * has goto-based flows that can interfere with scope-based cleanup, or when
+ * the lifetime of the forcewake reference does not match a specific scope
+ * (e.g., forcewake obtained in one function and released in a different one).
+ *
* Return: opaque reference to woken domains or zero if none of requested
* domains were awake.
*/
diff --git a/drivers/gpu/drm/xe/xe_force_wake.h b/drivers/gpu/drm/xe/xe_force_wake.h
index 0e3e84bfa51c..ffc4e103fe31 100644
--- a/drivers/gpu/drm/xe/xe_force_wake.h
+++ b/drivers/gpu/drm/xe/xe_force_wake.h
@@ -61,4 +61,44 @@ xe_force_wake_ref_has_domain(unsigned int fw_ref, enum xe_force_wake_domains dom
return fw_ref & domain;
}
+struct xe_force_wake_ref {
+ struct xe_force_wake *fw;
+ unsigned int domains;
+};
+
+static struct xe_force_wake_ref
+xe_force_wake_constructor(struct xe_force_wake *fw, unsigned int domains)
+{
+ struct xe_force_wake_ref fw_ref = { .fw = fw };
+
+ fw_ref.domains = xe_force_wake_get(fw, domains);
+
+ return fw_ref;
+}
+
+DEFINE_CLASS(xe_force_wake, struct xe_force_wake_ref,
+ xe_force_wake_put(_T.fw, _T.domains),
+ xe_force_wake_constructor(fw, domains),
+ struct xe_force_wake *fw, unsigned int domains);
+
+/*
+ * Scoped helper for the forcewake class, using the same trick as scoped_guard()
+ * to bind the lifetime to the next statement/block.
+ */
+#define __xe_with_force_wake(ref, fw, domains, done) \
+ for (CLASS(xe_force_wake, ref)(fw, domains), *(done) = NULL; \
+ !(done); (done) = (void *)1)
+
+#define xe_with_force_wake(ref, fw, domains) \
+ __xe_with_force_wake(ref, fw, domains, __UNIQUE_ID(done))
+
+/*
+ * Used when xe_force_wake_constructor() has already been called by another
+ * function and the current function is responsible for releasing the forcewake
+ * reference in all possible cases and error paths.
+ */
+DEFINE_CLASS(xe_force_wake_release_only, struct xe_force_wake_ref,
+ xe_force_wake_put(_T.fw, _T.domains), fw_ref,
+ struct xe_force_wake_ref fw_ref);
+
#endif
--
2.51.1
next prev parent reply other threads:[~2025-11-14 21:43 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-14 21:43 [PATCH v3 00/27] Scope-based forcewake and runtime PM Matt Roper
2025-11-14 21:43 ` Matt Roper [this message]
2025-11-17 22:03 ` [PATCH v3 01/27] drm/xe/forcewake: Add scope-based cleanup for forcewake Gustavo Sousa
2025-11-17 22:17 ` Gustavo Sousa
2025-11-14 21:43 ` [PATCH v3 02/27] drm/xe/pm: Add scope-based cleanup helper for runtime PM Matt Roper
2025-11-17 22:04 ` Gustavo Sousa
2025-11-14 21:43 ` [PATCH v3 03/27] drm/xe/gt: Use scope-based cleanup Matt Roper
2025-11-14 21:43 ` [PATCH v3 04/27] drm/xe/gt_idle: " Matt Roper
2025-11-14 21:43 ` [PATCH v3 05/27] drm/xe/guc: " Matt Roper
2025-11-14 21:43 ` [PATCH v3 06/27] drm/xe/guc_pc: " Matt Roper
2025-11-14 21:43 ` [PATCH v3 07/27] drm/xe/mocs: " Matt Roper
2025-11-14 21:43 ` [PATCH v3 08/27] drm/xe/pat: Use scope-based forcewake Matt Roper
2025-11-14 21:43 ` [PATCH v3 09/27] drm/xe/pxp: Use scope-based cleanup Matt Roper
2025-11-14 21:43 ` [PATCH v3 10/27] drm/xe/gsc: " Matt Roper
2025-11-14 21:43 ` [PATCH v3 11/27] drm/xe/device: " Matt Roper
2025-11-14 21:43 ` [PATCH v3 12/27] drm/xe/devcoredump: " Matt Roper
2025-11-17 22:09 ` Gustavo Sousa
2025-11-14 21:43 ` [PATCH v3 13/27] drm/xe/display: Use scoped-cleanup Matt Roper
2025-11-17 22:11 ` Gustavo Sousa
2025-11-14 21:43 ` [PATCH v3 14/27] drm/xe: Return forcewake reference type from force_wake_get_any_engine() Matt Roper
2025-11-17 22:19 ` Gustavo Sousa
2025-11-14 21:43 ` [PATCH v3 15/27] drm/xe/drm_client: Use scope-based cleanup Matt Roper
2025-11-17 22:28 ` Gustavo Sousa
2025-11-14 21:43 ` [PATCH v3 16/27] drm/xe/gt_debugfs: " Matt Roper
2025-11-14 21:43 ` [PATCH v3 17/27] drm/xe/huc: Use scope-based forcewake Matt Roper
2025-11-14 21:43 ` [PATCH v3 18/27] drm/xe/query: " Matt Roper
2025-11-14 21:43 ` [PATCH v3 19/27] drm/xe/reg_sr: " Matt Roper
2025-11-14 21:43 ` [PATCH v3 20/27] drm/xe/vram: " Matt Roper
2025-11-14 21:43 ` [PATCH v3 21/27] drm/xe/bo: Use scope-based runtime PM Matt Roper
2025-11-14 21:43 ` [PATCH v3 22/27] drm/xe/ggtt: Use scope-based runtime pm Matt Roper
2025-11-14 21:43 ` [PATCH v3 23/27] drm/xe/hwmon: Use scope-based runtime PM Matt Roper
2025-11-14 21:44 ` [PATCH v3 24/27] drm/xe/sriov: " Matt Roper
2025-11-14 21:44 ` [PATCH v3 25/27] drm/xe/tests: " Matt Roper
2025-11-14 21:44 ` [PATCH v3 26/27] drm/xe/sysfs: Use scope-based runtime power management Matt Roper
2025-11-14 21:44 ` [PATCH v3 27/27] drm/xe/debugfs: Use scope-based runtime PM Matt Roper
2025-11-14 23:22 ` ✗ CI.checkpatch: warning for Scope-based forcewake and runtime PM (rev4) Patchwork
2025-11-14 23:23 ` ✓ CI.KUnit: success " Patchwork
2025-11-15 0:14 ` ✓ Xe.CI.BAT: " Patchwork
2025-11-15 11:18 ` ✗ Xe.CI.Full: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251114214335.2388972-30-matthew.d.roper@intel.com \
--to=matthew.d.roper@intel.com \
--cc=gustavo.sousa@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=michal.wajdeczko@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox