Re: [PATCH v6 2/7] drm/xe/pm: introduce PM PME support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Failed runtime suspend leaves PCI PME wake enabled because the software flag is prematurely cleared before the hardware cleanup check.
--

commit 31cf117b704a2fc5858594d67a6c704b16fd2e7b
Author: Vinod Govindapillai <vinod.govindapillai@intel.com>

drm/xe/pm: introduce PM PME support

This commit introduces PME support for capable devices to avoid relying on
HPD polling. It assesses PME capabilities during PCI probe and enables them
for specific contexts during the PM runtime suspend call.

> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 32e10e1a26eff..ef5fefdd09e4d 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
[ ... ]
> @@ -1380,8 +1386,14 @@ static int xe_pci_runtime_suspend(struct device *dev)
>  	xe_assert(xe, !pci_num_vf(pdev));
>  
>  	err = xe_pm_runtime_suspend(xe);
> -	if (err)
> +	if (err) {
> +		if (xe_pm_pme_enabled(xe)) {

[Severity: Medium]
Since xe_pm_runtime_suspend() already clears the software flag in its 
failure path, will this check always evaluate to false?

If the flag is cleared early by the callee, it appears 
pci_enable_wake(pdev, PCI_D3hot, false) is skipped. Could this leave the 
PCI device with PME wake enabled while in D0 after a failed runtime suspend, 
potentially causing spurious PME interrupts?

> +			pci_enable_wake(pdev, PCI_D3hot, false);
> +			xe_pm_update_pme_enabled(xe, false);
> +		}
> +
>  		return err;
> +	}
>  
>  	pci_save_state(pdev);
[ ... ]
> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> index a5289a9df8d25..6453adbc3410d 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
[ ... ]
> @@ -650,6 +663,7 @@ int xe_pm_runtime_suspend(struct xe_device *xe)
>  	return 0;
>  
>  out_resume:
> +	xe_pm_update_pme_enabled(xe, false);

[Severity: Medium]
Does clearing the PME enabled flag here inadvertently defeat the hardware 
cleanup logic in the xe_pci_runtime_suspend() caller?

>  	xe_display_pm_runtime_resume(xe);
>  	xe_pxp_pm_resume(xe->pxp);
>  out:
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914194737.272679-1-vinod.govindapillai@intel.com?part=2