Re: [PATCH v2] drm/xe/pm: Handle GT resume failure

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

On Fri, 2026-01-09 at 14:43 +0100, Raag Jadav wrote:
> On Fri, Jan 09, 2026 at 04:37:39PM +0530, Nilawar, Badal wrote:
> > On 09-01-2026 11:28, Raag Jadav wrote:
> > > On Fri, Jan 09, 2026 at 08:14:05AM +0530, Nilawar, Badal wrote:
> > > > On 20-12-2025 13:06, Raag Jadav wrote:
> > > > > We've been historically ignoring GT resume failure. Since the
> > > > > function
> > > > > can return error, handle it properly.
> > > > > 
> > > > > v2: Bring up display before bailing (Matt Roper, Rodrigo)
> > > > > 
> > > > > Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> > > > > ---
> > > > >    drivers/gpu/drm/xe/xe_pm.c | 26 ++++++++++++++++++++++----
> > > > >    1 file changed, 22 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/xe/xe_pm.c
> > > > > b/drivers/gpu/drm/xe/xe_pm.c
> > > > > index 4390ba69610d..559cf5490ac0 100644
> > > > > --- a/drivers/gpu/drm/xe/xe_pm.c
> > > > > +++ b/drivers/gpu/drm/xe/xe_pm.c
> > > > > @@ -260,10 +260,19 @@ int xe_pm_resume(struct xe_device *xe)
> > > > >    	xe_irq_resume(xe);
> > > > > -	for_each_gt(gt, xe, id)
> > > > > -		xe_gt_resume(gt);
> > > > > +	for_each_gt(gt, xe, id) {
> > > > > +		err = xe_gt_resume(gt);
> > > > > +		if (err)
> > > > > +			break;
> > > > GT and SAMedia are different entities (even if both are treated
> > > > as GTs in
> > > > software), should we not continue attempting to resume the
> > > > remaining GT even
> > > > if resuming first one fails.
> > > My limited understanding is that GUI needs render engine, which
> > > the user
> > > won't be getting back either way.
> > 
> > Ok, but how about multi-tile platform like PVC which doesn't have
> > Render
> > engine and Media?
> 
> I'm assuming we don't need display to be able to debug them?
> 
> Regardless, a semi-working hardware would cause even more problems
> than
> solve IMHO but I'll leave to the experts to decide.

What I liked in this Raag's version is that this is the most generic
best-effort attempt we can get to still bring some display for some
debug or information instead of only a blank screen or only keep
moving as if nothing had happened.

A more complete version would be checks per platform and per type of
GT etc. We can still attempt to try this as a follow up.

For server platforms such as PVC it is a bogus discussion. System
Suspend is not a valid case there. Period.

For some discrete like BMG where render is there in the main GT,
but display is fused off, we can simply return the error.

For the case where main GT failed, we still can try to bring display
up and get some output like Raag's attempt.

For the case where media GT failed we could proceed with the resume,
but kind of a partial wedge of the media gt...

> 
> Raag
> 
> > > > > +	}
> > > > > +	/*
> > > > > +	 * Try to bring up display before bailing from GT
> > > > > resume failure,
> > > > > +	 * so we don't leave the user clueless with a blank
> > > > > screen.
> > > > > +	 */
> > > > >    	xe_display_pm_resume(xe);
> > > > > +	if (err)
> > > > > +		goto err;
> > > > >    	err = xe_bo_restore_late(xe);
> > > > >    	if (err)
> > > > > @@ -656,10 +665,19 @@ int xe_pm_runtime_resume(struct
> > > > > xe_device *xe)
> > > > >    	xe_irq_resume(xe);
> > > > > -	for_each_gt(gt, xe, id)
> > > > > -		xe->d3cold.allowed ? xe_gt_resume(gt) :
> > > > > xe_gt_runtime_resume(gt);
> > > > > +	for_each_gt(gt, xe, id) {
> > > > > +		err = xe->d3cold.allowed ? xe_gt_resume(gt)
> > > > > : xe_gt_runtime_resume(gt);
> > > > > +		if (err)
> > > > > +			break;
> > > > > +	}
> > > > > +	/*
> > > > > +	 * Try to bring up display before bailing from GT
> > > > > resume failure,
> > > > > +	 * so we don't leave the user clueless with a blank
> > > > > screen.
> > > > > +	 */
> > > > >    	xe_display_pm_runtime_resume(xe);
> > > > > +	if (err)
> > > > > +		goto out;
> > > > >    	if (xe->d3cold.allowed) {
> > > > >    		err = xe_bo_restore_late(xe);