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

["Bai, Zongyao" <zongyao.bai@intel.com>]

On 10/13/2026 11:52 AM, Rodrigo Vivi wrote:
> 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?

Hi Vivi,

Thanks for your time. I totally agree with you about the over-engineering.

The series started as a minimal hrtimer-based delayed put. At first I
ignored the races with shutdown, driver unload, device remove, FLR, PCIe
reset, etc. Each review round exposed another race or lifecycle problem,
and fixing them made the patches more and more complex.

Why I did not pick delayed queue earlier:
When you asked about it before, I rejected it because the hold was 100us,
below jiffy granularity. I treated 100us as a target value, but it was
only an early tuning value. That was my mistake.

Switching to delayed_work has a trade-off: the hold becomes jiffy-granular
(one jiffy is 4ms with HZ=250, that is 4-8ms sleep hold in each cycle),
so the effective hold is a few milliseconds instead of 100us.
And since only the opt-in engine-cycles path is affected,
at the cost of slightly more power after a profiler query.

I'll also drop the configfs knob; with jiffy granularity a fixed hold of
a few milliseconds is enough, so a tunable is not needed.
If no more requirements, I'll submit a delay queue version in this week.

Thanks,
Zongyao

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