Re: [PATCH v2 2/3] drm/xe/forcewake: flush delayed release at power and teardown boundaries

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] drm/xe/forcewake: delayed domains stranded if runtime suspend
  aborts
--

commit f7553aeb9060950e90243740fda0d4e4ec003892
Author: Zongyao Bai <zongyao.bai@intel.com>

drm/xe/forcewake: flush delayed release at power and teardown boundaries

This commit adds xe_force_wake_flush() and xe_force_wake_stop() to cancel
delayed forcewake releases and put delayed domains to sleep across suspend,
shutdown, and device removal paths.

> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> index 5ed96807fe342..a0a238f1a4515 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c

[ ... ]

> @@ -1104,7 +1106,7 @@ int xe_gt_runtime_suspend(struct xe_gt *gt)
>  {
>  	xe_gt_dbg(gt, "runtime suspending\n");
>  
> -	CLASS(xe_force_wake, fw_ref)(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> +	CLASS(xe_force_wake_flush, fw_ref)(gt_to_fw(gt), XE_FORCEWAKE_ALL);
>  	if (!xe_force_wake_ref_has_domain(fw_ref.domains, XE_FORCEWAKE_ALL)) {
>  		xe_gt_err(gt, "runtime suspend failed (%pe)\n", ERR_PTR(-ETIMEDOUT));
>  		return -ETIMEDOUT;

[Severity: Medium]
Can an aborted runtime suspend leave fw->delayed_domains awake with no
worker pending?

Once rpm_suspend() sets the device status to RPM_SUSPENDING,
xe_pm_runtime_get_if_active() returns false, so
xe_force_wake_delayed_release_work() exits early without sleeping
fw->delayed_domains or requeuing itself, relying on the suspend flush here
and in xe_gt_suspend().

However, xe_pm_runtime_suspend() can fail before reaching this flush on all
GTs:

drivers/gpu/drm/xe/xe_pm.c:xe_pm_runtime_suspend() {
	...
	if (xe->d3cold.allowed) {
		err = xe_bo_evict_all(xe);
		if (err)
			goto out_resume;
	}

	for_each_gt(gt, xe, id) {
		err = xe->d3cold.allowed ? xe_gt_suspend(gt) : xe_gt_runtime_suspend(gt);
		if (err)
			goto out_resume;
	}
	...
}

If xe_bo_evict_all() fails, or an earlier GT fails in the loop, rpm_suspend()
restores the device status to RPM_ACTIVE without flushing the remaining GTs.
Any domain in fw->delayed_domains whose worker ran during the RPM_SUSPENDING
window then stays awake with ref == 0 and delayed_release_work inactive,
violating the DELAYED HOLD invariant documented in struct xe_force_wake and
preventing GT C6 power saving until a later forcewake access or suspend.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110220542.432595-1-zongyao.bai@intel.com?part=2