Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/30] Scope-based forcewake and runtime PM
@ 2025-11-10 23:20 Matt Roper
  2025-11-10 23:20 ` [PATCH v2 01/30] drm/xe/forcewake: Improve kerneldoc Matt Roper
                   ` (34 more replies)
  0 siblings, 35 replies; 74+ messages in thread
From: Matt Roper @ 2025-11-10 23:20 UTC (permalink / raw)
  To: intel-xe; +Cc: matthew.d.roper, Michal Wajdeczko, Lucas De Marchi

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).


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>

Matt Roper (30):
  drm/xe/forcewake: Improve kerneldoc
  drm/xe/eustall: Store forcewake reference in stream structure
  drm/xe/oa: Store forcewake reference in stream structure
  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: Create scoped cleanup class for 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      |  25 +--
 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                    |   3 +-
 drivers/gpu/drm/xe/xe_debugfs.c               |  16 +-
 drivers/gpu/drm/xe/xe_devcoredump.c           |  26 ++-
 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            |  83 +++++-----
 drivers/gpu/drm/xe/xe_eu_stall.c              |   8 +-
 drivers/gpu/drm/xe/xe_force_wake.h            |  28 ++++
 drivers/gpu/drm/xe/xe_force_wake_types.h      |  26 ++-
 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                    | 151 ++++++------------
 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               |  32 ++--
 drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c   |  12 +-
 drivers/gpu/drm/xe/xe_gt_throttle.c           |   3 +-
 drivers/gpu/drm/xe/xe_guc.c                   |  13 +-
 drivers/gpu/drm/xe/xe_guc_debugfs.c           |   3 +-
 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 |   6 +-
 drivers/gpu/drm/xe/xe_hwmon.c                 |  16 +-
 drivers/gpu/drm/xe/xe_mocs.c                  |  18 +--
 drivers/gpu/drm/xe/xe_oa.c                    |   9 +-
 drivers/gpu/drm/xe/xe_oa_types.h              |   3 +
 drivers/gpu/drm/xe/xe_pat.c                   |  36 ++---
 drivers/gpu/drm/xe/xe_pci_sriov.c             |   3 +-
 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        |   6 +-
 drivers/gpu/drm/xe/xe_sriov_vf_ccs.c          |   5 +-
 drivers/gpu/drm/xe/xe_tile_debugfs.c          |   3 +-
 drivers/gpu/drm/xe/xe_tile_sriov_pf_debugfs.c |   3 +-
 drivers/gpu/drm/xe/xe_vram.c                  |   7 +-
 50 files changed, 387 insertions(+), 604 deletions(-)

-- 
2.51.1


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

end of thread, other threads:[~2025-11-13 23:28 UTC | newest]

Thread overview: 74+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-10 23:20 [PATCH v2 00/30] Scope-based forcewake and runtime PM Matt Roper
2025-11-10 23:20 ` [PATCH v2 01/30] drm/xe/forcewake: Improve kerneldoc Matt Roper
2025-11-12 14:04   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 02/30] drm/xe/eustall: Store forcewake reference in stream structure Matt Roper
2025-11-12 15:36   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 03/30] drm/xe/oa: " Matt Roper
2025-11-12 16:11   ` Gustavo Sousa
2025-11-13 17:10   ` Dixit, Ashutosh
2025-11-10 23:20 ` [PATCH v2 04/30] drm/xe/forcewake: Add scope-based cleanup for forcewake Matt Roper
2025-11-12 20:00   ` Gustavo Sousa
2025-11-12 21:01     ` Matt Roper
2025-11-12 21:16     ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 05/30] drm/xe/pm: Add scope-based cleanup helper for runtime PM Matt Roper
2025-11-12 19:53   ` Michal Wajdeczko
2025-11-12 21:48     ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 06/30] drm/xe/gt: Use scope-based cleanup Matt Roper
2025-11-13 12:26   ` Gustavo Sousa
2025-11-13 22:58     ` Matt Roper
2025-11-10 23:20 ` [PATCH v2 07/30] drm/xe/gt_idle: " Matt Roper
2025-11-13 12:39   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 08/30] drm/xe/guc: " Matt Roper
2025-11-13 12:46   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 09/30] drm/xe/guc_pc: " Matt Roper
2025-11-13 13:00   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 10/30] drm/xe/mocs: " Matt Roper
2025-11-13 13:30   ` Gustavo Sousa
2025-11-13 23:28     ` Matt Roper
2025-11-10 23:20 ` [PATCH v2 11/30] drm/xe/pat: Use scope-based forcewake Matt Roper
2025-11-13 13:37   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 12/30] drm/xe/pxp: Use scope-based cleanup Matt Roper
2025-11-13 13:40   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 13/30] drm/xe/gsc: " Matt Roper
2025-11-13 13:46   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 14/30] drm/xe/device: " Matt Roper
2025-11-13 14:04   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 15/30] drm/xe/devcoredump: " Matt Roper
2025-11-13 14:14   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 16/30] drm/xe/display: Use scoped-cleanup Matt Roper
2025-11-13 14:25   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 17/30] drm/xe: Create scoped cleanup class for force_wake_get_any_engine() Matt Roper
2025-11-13 17:39   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 18/30] drm/xe/drm_client: Use scope-based cleanup Matt Roper
2025-11-10 23:20 ` [PATCH v2 19/30] drm/xe/gt_debugfs: " Matt Roper
2025-11-13 17:45   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 20/30] drm/xe/huc: Use scope-based forcewake Matt Roper
2025-11-13 17:46   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 21/30] drm/xe/query: " Matt Roper
2025-11-13 17:50   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 22/30] drm/xe/reg_sr: " Matt Roper
2025-11-13 17:51   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 23/30] drm/xe/vram: " Matt Roper
2025-11-10 23:57   ` [PATCH v2.1 " Matt Roper
2025-11-13 17:52     ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 24/30] drm/xe/bo: Use scope-based runtime PM Matt Roper
2025-11-13 17:54   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 25/30] drm/xe/ggtt: Use scope-based runtime pm Matt Roper
2025-11-13 17:55   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 26/30] drm/xe/hwmon: Use scope-based runtime PM Matt Roper
2025-11-13 18:01   ` Gustavo Sousa
2025-11-13 18:05     ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 27/30] drm/xe/sriov: " Matt Roper
2025-11-13 18:09   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 28/30] drm/xe/tests: " Matt Roper
2025-11-13 18:15   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 29/30] drm/xe/sysfs: Use scope-based runtime power management Matt Roper
2025-11-13 18:25   ` Gustavo Sousa
2025-11-10 23:20 ` [PATCH v2 30/30] drm/xe/debugfs: Use scope-based runtime PM Matt Roper
2025-11-13 18:30   ` Gustavo Sousa
2025-11-11  0:20 ` ✓ CI.KUnit: success for Scope-based forcewake and runtime PM (rev3) Patchwork
2025-11-11  0:57 ` ✓ Xe.CI.BAT: " Patchwork
2025-11-11 10:50 ` ✗ Xe.CI.Full: failure " Patchwork
2025-11-11 10:57 ` [PATCH v2 00/30] Scope-based forcewake and runtime PM Jani Nikula
2025-11-12 16:01   ` Matt Roper
2025-11-13 22:11 ` Matt Roper

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