Re: [PATCH v5] drm/xe: Poll GT for C6 before D3

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/xe: D3cold runtime suspend always fails due to C6 check after GT PG disable
--

commit 691645f0da262033183939383983163842757c50
Author: Vinay Belgaumkar <vinay.belgaumkar@intel.com>

drm/xe: Poll GT for C6 before D3

This commit adds a check for GT C6 state before the D3 transition in the
runtime suspend flow, aborting and resuming if the check fails.

> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 00b2b5dfb8f6c..488745ce0473f 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
[ ... ]
> @@ -1394,6 +1394,17 @@ static int xe_pci_runtime_suspend(struct device *dev)
>  	if (err)
>  		return err;
>  
> +	err = xe_pm_check_device_idle(xe);
> +	if (err) {
> +		drm_dbg(&xe->drm, "Resuming - Device idle check failed!");
> +		ret = xe_pm_runtime_resume(xe);

[Severity: Medium]
Will this idle check always fail when D3cold is allowed?

Looking at the sequence, xe_pci_runtime_suspend() calls
xe_pm_runtime_suspend() right before this check. When D3cold is allowed,
xe_pm_runtime_suspend() executes xe_gt_suspend():

drivers/gpu/drm/xe/xe_pm.c:xe_pm_runtime_suspend() {
...
	for_each_gt(gt, xe, id) {
		err = xe->d3cold.allowed ? xe_gt_suspend(gt) : xe_gt_runtime_suspend(gt);
...
}

xe_gt_suspend() then prepares the GT for power removal by explicitly disabling
power gating:

drivers/gpu/drm/xe/xe_gt.c:xe_gt_suspend() {
...
	xe_gt_idle_disable_pg(gt);
...
}

Because xe_gt_idle_disable_pg() writes 0 to the POWERGATE_ENABLE register,
the hardware is prevented from entering the C6 power gating state. Won't
the subsequent poll for GT_IDLE_C6 in xe_pm_check_device_idle() always
timeout and return -EAGAIN, causing the device to constantly abort suspend
and immediately resume?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915210113.2915363-1-vinay.belgaumkar@intel.com?part=1