Re: [PATCH v4] 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:
- [High] drm/xe: unconditional GT C6 polling breaks runtime PM on platforms without C6
--

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

drm/xe: Poll GT for C6 before D3

This commit ensures the GT is in the C6 state before transitioning to D3
during runtime suspend. It polls the GT idle status and forces a resume
if the C6 check fails, preventing the device from being stuck in an
intermediate state.

> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> --- 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);
> +		if (ret) {
> +			drm_err(&xe->drm, "Resume failed after suspend was canceled");
> +			return ret;
> +		}
> +		return err;
> +	}
> +

[ ... ]
> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
[ ... ]
> @@ -1030,6 +1030,27 @@ void xe_pm_d3cold_allowed_toggle(struct xe_device *xe)
>  	mutex_unlock(&xe->d3cold.lock);
>  }
>  
> +/**
> + * xe_pm_check_device_idle() - Check if all GTs are idle.
> + * @xe: xe device instance
> + *
> + * One of the ways to do this is to check for GT C6 state. This check
> + * will fail if some component is holding a forcewake, for example.
> + *
> + * Return: 0 on success, -EAGAIN on failure
> + */
> +int xe_pm_check_device_idle(struct xe_device *xe)
> +{
> +	struct xe_gt *gt;
> +	u8 id;
> +
> +	for_each_gt(gt, xe, id)
> +		if (xe_gt_idle_wait_for_c6(gt, 200))
> +			return -EAGAIN;

[Severity: High]
Does this unconditional polling for C6 break runtime PM on platforms
that don't support GT C6 (like PVC)?

If a platform disables or does not support C6, xe_gt_idle_wait_for_c6()
will unconditionally time out after 200ms. Since this function is called 
by xe_pci_runtime_suspend() during the idle transition, the device will 
repeatedly abort the suspend sequence.

Because the runtime autosuspend timer usually fires every second, would
this waste 200ms out of every second busy-waiting for a hardware state 
that can never be reached, leading to a CPU hog and preventing the device
from ever suspending?

> +
> +	return 0;
> +}
> +

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