Re: [PATCH v10 0/4] drm/xe/forcewake: add delayed release and lifecycle handling

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Wed, Sep 30, 2026 at 03:08:16PM -0700, Zongyao Bai wrote:
> From: "Bai, Zongyao" <zongyao.bai@intel.com>
> 
> Engine-cycle queries used by profiling tools such as VTune and PTI can
> arrive in bursts. Releasing forcewake immediately after every query makes
> the next query issue another wake request and wait for its ACK.
> 
> Add an opt-in delayed-release path with a bind-time configurable hold
> interval, defaulting to 100 us. When the final overlapping forcewake
> reference is dropped after a delayed-release request, keep the domain awake
> for that interval. A get for the same domain before expiry cancels the
> pending release and reuses the awake domain without another wake request or
> ACK wait.
> 
> Only engine-cycle queries request delayed release. Existing callers
> continue to use the immediate-release API. Delayed-release state is per
> domain, so a put that drops the final overlapping reference honors any
> recorded delayed-release request.
> 
> Protect engine-cycle MMIO access with drm_dev_enter()/drm_dev_exit() so
> drm_dev_unplug() waits for admitted queries to drop their forcewake
> references before teardown. For system and runtime suspend, synchronously
> drain delayed state while keeping delayed release available after resume.
> 
> For shutdown, removal, post-registration probe unwind, and devres teardown,
> permanently disable new delayed releases and settle pending state with
> xe_force_wake_stop(). Register the device-wide stop action after the
> Driver-FLR action so reverse devres ordering settles delayed forcewake
> state before Driver-FLR while MMIO is still valid. Keep the per-GT cleanup
> as an idempotent fallback for earlier probe failures.
> 
> For PCI error recovery, use xe_force_wake_stop_nosettle() before
> pci_disable_device() on reset and on paths returning
> PCI_ERS_RESULT_DISCONNECT. It disables delayed release, cancels timers, and
> discards delayed-release bookkeeping without accessing MMIO.

I'm having a hard time reviewing this series. The patches seems to mix
multiple stuff into one. And the solution looks a lot over-engineering.

why not using a simple histeresis timed put with a delayed queue?

> 
> v10:
>  - Please ignore v6~v9 in this series:
>    https://patchwork.freedesktop.org/series/167697
>    Those revisions used incorrect Root msg-id(s) by mistake.
>    Created v10 to clean up the threading mess in previous revisions.
>    No functional changes relative to v6.
>    Rebase onto drm-xe-next at fa8b3b6c4a4b.
> 
> v6:
>  - Add a permanent delayed-release disable state and clear stale recorded
>    requests on the immediate-release path.
>  - Keep flush for system/runtime suspend and use terminal stop for
>    shutdown, remove, probe unwind, and devres teardown.
>  - Order the device-wide stop before Driver-FLR while retaining per-GT
>    cleanup for earlier probe failures.
>  - Add a no-MMIO stop for PCI reset and disconnect handling.
>  - Use the configfs group lock when reading the hold interval.
>  - Document the per-domain drain and discard state transitions.
> 
> v5:
>  - Split the original patch into four patches: unplug synchronization,
>    delayed-release infrastructure, power-boundary flushing, and opt-in
>    enablement/configuration.
>  - Preserve a newer hold interval when an older timer callback is running,
>    and settle an outstanding sleep ACK before waking a domain again.
>    (Sashiko, Matt)
>  - Synchronously cancel delayed-release timers during forcewake cleanup.
>    (Sashiko, Matt)
>  - Flush delayed releases during shutdown, device removal, and
>    post-registration probe unwind in addition to system/runtime suspend.
>    (Matt)
> 
> Zongyao Bai (4):
>   drm/xe/forcewake: synchronize engine-cycle access with unplug
>   drm/xe/forcewake: add delayed-release state machine
>   drm/xe/forcewake: quiesce delayed release across PM and teardown
>   drm/xe/forcewake: enable configurable delayed forcewake release
> 
>  drivers/gpu/drm/xe/xe_configfs.c         |  78 ++++++
>  drivers/gpu/drm/xe/xe_configfs.h         |   5 +
>  drivers/gpu/drm/xe/xe_defaults.h         |   1 +
>  drivers/gpu/drm/xe/xe_device.c           |  33 ++-
>  drivers/gpu/drm/xe/xe_device.h           |   1 +
>  drivers/gpu/drm/xe/xe_device_types.h     |   3 +
>  drivers/gpu/drm/xe/xe_force_wake.c       | 337 +++++++++++++++++++++--
>  drivers/gpu/drm/xe/xe_force_wake.h       |  48 +++-
>  drivers/gpu/drm/xe/xe_force_wake_types.h | 123 +++++++++
>  drivers/gpu/drm/xe/xe_gt.c               |  18 +-
>  drivers/gpu/drm/xe/xe_pci_error.c        |  23 +-
>  drivers/gpu/drm/xe/xe_query.c            |  81 ++++--
>  12 files changed, 696 insertions(+), 55 deletions(-)
> 
> 
> base-commit: fa8b3b6c4a4b297bf95935065b63f8852cfc5481
> -- 
> 2.43.0