Re: [PATCH v2 2/2] 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] System suspend cancellation fails to abort the suspend process, overwrites the error code, and incorrectly uses a runtime PM resume function for recovery, leaving the device in an inconsistent state.
--

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

drm/xe: Poll GT for C6 before D3

This commit adds a check to ensure GTs are in C6 before transitioning
to D3 during suspend. If the GTs are not in C6, it attempts to resume
the device and abort the suspend.

> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 278cfd390364c..04f0884f21c1b 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -1342,6 +1342,15 @@ static int xe_pci_suspend(struct device *dev)
>  	 */
>  	d3cold_toggle(pdev, D3COLD_ENABLE);
>  
> +	err = xe_pm_check_runtime_idle(xe);
> +	if (err) {
> +		err = xe_pm_runtime_resume(xe);
> +		if (err) {
> +			drm_err(&xe->drm, "Resume failed after suspend was canceled");
> +			return err;
> +		}
> +	}

[Severity: High]
If xe_pm_check_runtime_idle() returns an error to abort the suspend, this
block assigns the result of xe_pm_runtime_resume() to the same 'err'
variable. 

If the resume succeeds and returns 0, the function will continue executing
and return 0. Doesn't this overwrite the abort error code and allow the
system suspend to proceed instead of successfully cancelling it?

Additionally, xe_pci_suspend() is a system PM suspend path. Calling
xe_pm_runtime_resume() here invokes runtime PM semantics which skip certain
initializations based on the xe->d3cold.allowed flag. 

Since system PM paths require full hardware reinitialization unconditionally,
does this unwinding path need to use xe_pm_resume() or an equivalent system
PM recovery function to ensure the device is left in a consistent state?

> +
>  	pci_save_state(pdev);
>  	pci_disable_device(pdev);
>  	pci_set_power_state(pdev, PCI_D3cold);

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