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

[Lin, Shuicheng]

On Tues, July 8, 2025 12:12 AM 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 at intel.com>
> > Cc: John Harrison <John.C.Harrison at Intel.com>
> > Signed-off-by: Shuicheng Lin <shuicheng.lin at 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.

Totally agree with you. Actually, I am not sure the Fixes tag should be added or not.
Let me remove the Fixes tag, and loop display expert to help review the change also.
Thanks for the detailed information.

Shuicheng

> 
> 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 at intel.com>
> 
> But please also get an ack from someone on the display side to make sure this
> was not intentional.
> 
> 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);