Re: [PATCH] drm/xe/pm: Restore display pm if there is error after display suspend

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Jul 07, 2025 at 09:11:41AM -0700, Daniele Ceraolo Spurio wrote:
> 
> 
> On 7/6/2025 6:58 PM, Shuicheng Lin wrote:
> > xe_bo_evict_all() is called after xe_display_pm_suspend(). So if there
> > is error with xe_bo_evict_all(), display pm should be restored.
> > 
> > Fixes: 51462211f4a9 ("drm/xe/pxp: add PXP PM support")
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Cc: John Harrison <John.C.Harrison@Intel.com>
> > Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
> 
> If you look at 51462211f4a9, you'll see that before the PXP changes were
> added xe_display_pm_resume was only called from the error path of
> xe_gt_suspend (even though xe_bo_evict_all was already there), so I've kept
> that behavior when I added the PXP changes.
> That pre-PXP behavior is still present in the 6.12 LTS kernel, so if that's
> wrong it needs to be fixed there as well.
> 
> Looking at the history, it looks like the patch that moved the call to
> xe_display_pm_resume to before the evict_all without adjusting the error
> management was this one:
> cb8f81c17531 ("drm/xe/display: Make display suspend/resume work on
> discrete")
> 
> Given how different the code is, I don't know if it is better to send a
> separate patch for that to the stable tree, or to update the fixes tag on
> this one and send the separate patch when it fails to apply.
> 
> If you want to go with the former, this is:
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> 
> But please also get an ack from someone on the display side to make sure
> this was not intentional.

not intentional. definitely a bug.

Instead of removing the fixes tag, I added an extra one. And pushed to
drm-xe-next.

Shuicheng whenever you receive a notification that the patch failed to apply
in some stable branch, please follow up sending a backported version directly
to the stable mailing list.

Thanks for the patch and review.

> 
> Daniele
> 
> > ---
> >   drivers/gpu/drm/xe/xe_pm.c | 3 +--
> >   1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> > index ff749edc005b..bcfda545e74f 100644
> > --- a/drivers/gpu/drm/xe/xe_pm.c
> > +++ b/drivers/gpu/drm/xe/xe_pm.c
> > @@ -134,7 +134,7 @@ int xe_pm_suspend(struct xe_device *xe)
> >   	/* FIXME: Super racey... */
> >   	err = xe_bo_evict_all(xe);
> >   	if (err)
> > -		goto err_pxp;
> > +		goto err_display;
> >   	for_each_gt(gt, xe, id) {
> >   		err = xe_gt_suspend(gt);
> > @@ -151,7 +151,6 @@ int xe_pm_suspend(struct xe_device *xe)
> >   err_display:
> >   	xe_display_pm_resume(xe);
> > -err_pxp:
> >   	xe_pxp_pm_resume(xe->pxp);
> >   err:
> >   	drm_dbg(&xe->drm, "Device suspend failed %d\n", err);
>