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

[Raag Jadav <raag.jadav@intel.com>]

On Wed, Dec 17, 2025 at 09:38:34AM -0800, Matt Roper wrote:
> On Wed, Dec 17, 2025 at 12:25:32PM -0500, Rodrigo Vivi wrote:
> > On Wed, Dec 17, 2025 at 06:49:09PM +0530, Raag Jadav wrote:
> > > We've been historically ignoring GT resume failure. Since the function
> > > can return error, handle it properly.
> > 
> > I probably had a reason for it, but since I didn't document and
> > cannot remember it, let's go forward and make the clean flow.
> > 
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > > 
> > > Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/xe_pm.c | 14 ++++++++++----
> > >  1 file changed, 10 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> > > index 4390ba69610d..a8b50091d62e 100644
> > > --- a/drivers/gpu/drm/xe/xe_pm.c
> > > +++ b/drivers/gpu/drm/xe/xe_pm.c
> > > @@ -260,8 +260,11 @@ 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)
> > > +			goto err;
> 
> When we propagate these errors upward, what's the end result / where
> does it eventually get handled?  If the device is still [partially]
> usable after an error, wouldn't it be better to not bail out of the loop
> immediately, but rather at least try to resume the other GTs, the
> display, etc. before returning the error at the end to indicate
> something failed?  Then you might still have a partially functioning
> device and have a better chance of at least having your screen turn back
> on to show the relevant error messages?

I had a similar question when I came across xe_device_probe(), but as
Lucas mentioned[1] that the expectation here is pretty much "all or
nothing". Again, not my call but I think we should be consistent.

[1] https://lore.kernel.org/intel-xe/lliho4ci6gi5spxxelttgqntbh7rxr4utg4dgfevlrdy54phrh@2k4mjuofaqye/

Raag

> > > +	}
> > >  
> > >  	xe_display_pm_resume(xe);
> > >  
> > > @@ -656,8 +659,11 @@ 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)
> > > +			goto out;
> > > +	}
> > >  
> > >  	xe_display_pm_runtime_resume(xe);
> > >  
> > > -- 
> > > 2.43.0
> > > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation