Re: [PATCH 2/4] drm/xe/pm: plumb PME capability to runtime suspend handler

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing rollback of GT suspend operations on error paths in PM suspend callbacks leaves previously suspended GTs in a powered-down state.
--

commit beb79b15731f1c3df49e1a0d1e8cf997804da5af
Author: Vinod Govindapillai <vinod.govindapillai@intel.com>
Subject: drm/xe/pm: plumb PME capability to runtime suspend handler

If the device is PME capable, we don't need to start the HPD polling
during the runtime suspend. This patch plumbs the pme_capable flag
through the runtime suspend stack.

> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> index 1488c4dfcc055..c3a53c5f8b330 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
[ ... ]
> @@ -639,9 +640,9 @@ int xe_pm_runtime_suspend(struct xe_device *xe)
>  			goto out_resume;
                        ^^^^^^^^^^^^^^^

[Severity: High]
This isn't a bug introduced by this patch, but modifying this function
highlights incomplete error handling on this path.

If suspending a GT fails (e.g., due to a forcewake timeout), the out_resume
error path skips resuming the GTs that successfully suspended prior to the
failure:

out_resume:
	xe_display_pm_runtime_resume(xe);
	xe_pxp_pm_resume(xe->pxp);
out:
	xe_rpm_lockmap_release(xe);
	xe_pm_write_callback_task(xe, NULL);
	return err;

Since the PM core considers the device to be active when the suspend callback
returns an error, won't subsequent accesses to the successfully suspended GTs
cause hardware hangs or kernel oops?

Also, a similar issue appears in the system suspend path in xe_pm_suspend():

	for_each_gt(gt, xe, id) {
		err = xe_gt_suspend(gt);
		if (err)
			goto err_display;
	}

Could we ensure previously suspended GTs are properly resumed in these error
paths?

>  	}
>  
> -	xe_irq_suspend(xe, false);
> +	xe_irq_suspend(xe, pme_capable);
>  
> -	xe_display_pm_runtime_suspend_late(xe);
> +	xe_display_pm_runtime_suspend_late(xe, pme_capable);
>  
>  	xe_i2c_pm_suspend(xe);

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