Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/27] Scope-based forcewake and runtime PM
@ 2025-11-14 21:43 Matt Roper
  2025-11-14 21:43 ` [PATCH v3 01/27] drm/xe/forcewake: Add scope-based cleanup for forcewake Matt Roper
                   ` (30 more replies)
  0 siblings, 31 replies; 39+ messages in thread
From: Matt Roper @ 2025-11-14 21:43 UTC (permalink / raw)
  To: intel-xe
  Cc: matthew.d.roper, Michal Wajdeczko, Lucas De Marchi, Gustavo Sousa

Forcewake and runtime PM both follow reference-counted get/put models;
when used in functions that can encounter errors and return early, it's
easy for developers to make mistakes and fail to drop a reference on all
of the error paths.  Cleanup of these reference counts is often
addressed by goto-based error handling which is somewhat ugly and
subject to its own set of mistakes once we accumulate too many error
labels in a function.

Scope-based cleanup ([1][2]) has been gaining increasing popularity in
the Linux kernel for cleaning up various kinds of resources in a more
automated way when code has lots of error paths and early exits.  Let's
add scope-based cleanup for both forcewake and runtime PM, based on the
mechanisms provided in include/linux/cleanup.h.  Scope-based cleanup
allows cleanup destructors to be executed automatically when the current
scope is exited by any means (end of block, return, break, etc.).

For xe_runtime_pm_{get,put} pairs that were grabbed and released within
a single function or block, the preferred replacement is now just

        guard(xe_pm_runtime_noresume)(xe);

which will take care of releasing the runtime PM reference
automatically.  scoped_guard() can be used instead if the reference
should only be held over part of the block.  There are also guard
variants added for xe_pm_runtime_noresume and xe_pm_runtime_ioctl that
allow replacement of those alternate functions as well.

Unlike runtime PM, where all reference tracking is done within the
object parameter itself, forcewake is currently a model where get
operations return a cookie that needs to be passed back to put
operations.  That necessitates a slightly different type of cleanup
helper (CLASS instead of guard), although the underlying mechanisms are
the same.  For forcewake that is grabbed and released within a single
function or block, the preferred form is now:

        CLASS(xe_force_wake, fw_ref)(gt_to_fw(gt), XE_FW_GT);

which, like the runtime PM equivalent, will cause the forcewake
reference to be dropped automatically.  If forcewake needs to be held
over only a subset of the current block,

        xe_with_force_wake(fw_ref, gt_to_fw(gt), XE_FW_GT) { ... }

can be used in the same way scoped_guard() is used for runtime PM.

The first few patches in this series make some general cleanups and
restructuring of the existing force wake code.  Then the new guards and
classes for runtime PM and forcewake are defined.  Finally, most of the
existing runtime PM and forcewake usage in the driver is converted to
the scope-based form in the remainder of the series.  Some of the
conversions eliminate goto-based cleanup models and/or significantly
simplify the code.  Other conversions don't significantly simplify the
code (aside from a slight reduction in line count), but are still useful
for consistency across our codebase.

An advantage of doing the conversion everywhere possible, not just the
places where it noticeably simplifies the code, is that it helps
highlight the remaining get/put usage as special cases where wake
references follow more complicated lifetimes (e.g., obtained in one
function and released in a different one, often tied to some other type
of resource or operation).  With fewer direct get/put calls overall, its
easier to identify the ones that remain as special cases and make sure
they truly are paired up properly.

There's other areas where scope-based cleanup could potentially be
applied in the future (e.g., mutex locks, bo locking, etc.), but this
series does not try to address those, even in places where those
resources are also part of the same error handling cleanup paths as
forcewake and runtime PM.  We can potentially think about converting
other types of resources to scope-based cleanup down the road if it
winds up working well here for forcewake and PM.

v2:
 - Add a proper success condition to the xe_pm_runtime_ioctl class so
   that conditional guards properly distinguish success (requiring
   cleanup) from errors (no cleanup).  (CI)
 - Don't bother changing the signature of xe_force_wake_get() before
   adding the forcewake class.  Simply create a separate constructor
   that wraps xe_force_wake_get().  (Michal)
 - Split ACQUIRE_ERR assignments from the condition checks.  Even though
   this takes an extra line of code and deviates from most of the other
   uses in the kernel, it's easier to read (and avoids checkpatch
   warnings).

v3:
 - Wrap xe_with_force_wake's 'done' marker in __UNIQUE_ID. (Gustavo)
 - Add a general xe_force_wake_release_only class which eliminates the
   need for the one-off xe_force_wake_any_engine class later in the
   series.  (Gustavo)
 - Drop scope-based usage from gt_reset_worker() since goto-based flows
   are still used there.  (Gustavo)
 - Add kerneldoc to existing _get functions explaining that scope-based
   options should be preferred when possible.  (Gustavo)
 - Eliminate several 'ret' local variables that become unnecessary after
   the scope-based conversion.  (Gustavo)

References:
 [1] https://www.kernel.org/doc/html/next/core-api/cleanup.html
 [2] https://lwn.net/Articles/934679/


Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Gustavo Sousa <gustavo.sousa@intel.com>

Matt Roper (27):
  drm/xe/forcewake: Add scope-based cleanup for forcewake
  drm/xe/pm: Add scope-based cleanup helper for runtime PM
  drm/xe/gt: Use scope-based cleanup
  drm/xe/gt_idle: Use scope-based cleanup
  drm/xe/guc: Use scope-based cleanup
  drm/xe/guc_pc: Use scope-based cleanup
  drm/xe/mocs: Use scope-based cleanup
  drm/xe/pat: Use scope-based forcewake
  drm/xe/pxp: Use scope-based cleanup
  drm/xe/gsc: Use scope-based cleanup
  drm/xe/device: Use scope-based cleanup
  drm/xe/devcoredump: Use scope-based cleanup
  drm/xe/display: Use scoped-cleanup
  drm/xe: Return forcewake reference type from
    force_wake_get_any_engine()
  drm/xe/drm_client: Use scope-based cleanup
  drm/xe/gt_debugfs: Use scope-based cleanup
  drm/xe/huc: Use scope-based forcewake
  drm/xe/query: Use scope-based forcewake
  drm/xe/reg_sr: Use scope-based forcewake
  drm/xe/vram: Use scope-based forcewake
  drm/xe/bo: Use scope-based runtime PM
  drm/xe/ggtt: Use scope-based runtime pm
  drm/xe/hwmon: Use scope-based runtime PM
  drm/xe/sriov: Use scope-based runtime PM
  drm/xe/tests: Use scope-based runtime PM
  drm/xe/sysfs: Use scope-based runtime power management
  drm/xe/debugfs: Use scope-based runtime PM

 drivers/gpu/drm/xe/display/xe_fb_pin.c        |  23 ++--
 drivers/gpu/drm/xe/display/xe_hdcp_gsc.c      |  31 ++---
 drivers/gpu/drm/xe/tests/xe_bo.c              |  10 +-
 drivers/gpu/drm/xe/tests/xe_dma_buf.c         |   3 +-
 drivers/gpu/drm/xe/tests/xe_migrate.c         |  10 +-
 drivers/gpu/drm/xe/tests/xe_mocs.c            |  27 ++--
 drivers/gpu/drm/xe/xe_bo.c                    |   8 +-
 drivers/gpu/drm/xe/xe_debugfs.c               |  16 +--
 drivers/gpu/drm/xe/xe_devcoredump.c           |  30 ++--
 drivers/gpu/drm/xe/xe_device.c                |  33 ++---
 drivers/gpu/drm/xe/xe_device_sysfs.c          |  33 ++---
 drivers/gpu/drm/xe/xe_drm_client.c            |  69 +++++-----
 drivers/gpu/drm/xe/xe_force_wake.c            |   7 +
 drivers/gpu/drm/xe/xe_force_wake.h            |  40 ++++++
 drivers/gpu/drm/xe/xe_ggtt.c                  |   3 +-
 drivers/gpu/drm/xe/xe_gsc.c                   |  21 +--
 drivers/gpu/drm/xe/xe_gsc_debugfs.c           |   3 +-
 drivers/gpu/drm/xe/xe_gsc_proxy.c             |  17 +--
 drivers/gpu/drm/xe/xe_gt.c                    | 130 ++++++------------
 drivers/gpu/drm/xe/xe_gt_debugfs.c            |  29 ++--
 drivers/gpu/drm/xe/xe_gt_freq.c               |  27 ++--
 drivers/gpu/drm/xe/xe_gt_idle.c               |  41 ++----
 drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c   |  12 +-
 drivers/gpu/drm/xe/xe_gt_throttle.c           |   9 +-
 drivers/gpu/drm/xe/xe_guc.c                   |  13 +-
 drivers/gpu/drm/xe/xe_guc_debugfs.c           |   8 +-
 drivers/gpu/drm/xe/xe_guc_log.c               |  10 +-
 drivers/gpu/drm/xe/xe_guc_pc.c                |  62 +++------
 drivers/gpu/drm/xe/xe_guc_submit.c            |  11 +-
 drivers/gpu/drm/xe/xe_guc_tlb_inval.c         |   4 +-
 drivers/gpu/drm/xe/xe_huc.c                   |   7 +-
 drivers/gpu/drm/xe/xe_huc_debugfs.c           |   3 +-
 drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c |  16 +--
 drivers/gpu/drm/xe/xe_hwmon.c                 |  52 ++-----
 drivers/gpu/drm/xe/xe_mocs.c                  |  18 +--
 drivers/gpu/drm/xe/xe_pat.c                   |  36 ++---
 drivers/gpu/drm/xe/xe_pci_sriov.c             |  10 +-
 drivers/gpu/drm/xe/xe_pm.c                    |  21 +++
 drivers/gpu/drm/xe/xe_pm.h                    |  17 +++
 drivers/gpu/drm/xe/xe_pxp.c                   |  55 +++-----
 drivers/gpu/drm/xe/xe_query.c                 |  16 +--
 drivers/gpu/drm/xe/xe_reg_sr.c                |  17 +--
 drivers/gpu/drm/xe/xe_sriov_pf_debugfs.c      |   6 +-
 drivers/gpu/drm/xe/xe_sriov_pf_sysfs.c        |  16 +--
 drivers/gpu/drm/xe/xe_sriov_vf_ccs.c          |   5 +-
 drivers/gpu/drm/xe/xe_tile_debugfs.c          |   8 +-
 drivers/gpu/drm/xe/xe_tile_sriov_pf_debugfs.c |   3 +-
 drivers/gpu/drm/xe/xe_vram.c                  |   6 +-
 48 files changed, 399 insertions(+), 653 deletions(-)

-- 
2.51.1


^ permalink raw reply	[flat|nested] 39+ messages in thread

end of thread, other threads:[~2025-11-17 22:29 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-14 21:43 [PATCH v3 00/27] Scope-based forcewake and runtime PM Matt Roper
2025-11-14 21:43 ` [PATCH v3 01/27] drm/xe/forcewake: Add scope-based cleanup for forcewake Matt Roper
2025-11-17 22:03   ` 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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox