Re: [PATCH v8 08/10] drm/xe: Improve wedged state management

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

On Thu, Jun 04, 2026 at 12:22:35PM +0530, Tauro, Riana wrote:
> On 6/3/2026 4:26 PM, Rodrigo Vivi wrote:
> > On Wed, Jun 03, 2026 at 03:47:08PM +0530, Raag Jadav wrote:
> > > Currently, wedged state is serving a single usecase where the device is
> > > declared wedged, but this doesn't allow any wedged state management for
> > > runtime usecases. In preparation of usecases which requires to facilitate
> > > temporary device wedging, convert wedged.flag to wedged.ref which serves
> > > as a driver internal refcount and blocks critical path execution during
> > > device lifetime and while at it, introduce wedged.fini and use it as a
> > > cleanup indicator during driver unwind which operates independent of the
> > > refcount.
> > > 
> > > Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> > > ---
> > >   drivers/gpu/drm/xe/xe_device.c       |  5 +++--
> > >   drivers/gpu/drm/xe/xe_device.h       | 12 +++++++++++-
> > >   drivers/gpu/drm/xe/xe_device_types.h |  6 ++++--
> > >   3 files changed, 18 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> > > index 9e0cbad8adb0..d77f8f054a1a 100644
> > > --- a/drivers/gpu/drm/xe/xe_device.c
> > > +++ b/drivers/gpu/drm/xe/xe_device.c
> > > @@ -917,7 +917,7 @@ static void xe_device_wedged_fini(struct drm_device *drm, void *arg)
> > >   {
> > >   	struct xe_device *xe = arg;
> > > -	if (atomic_read(&xe->wedged.flag))
> > > +	if (atomic_read(&xe->wedged.fini))
> > >   		xe_pm_runtime_put(xe);
> > >   }
> > > @@ -1413,7 +1413,8 @@ void xe_device_declare_wedged(struct xe_device *xe)
> > >   		return;
> > >   	}
> > > -	if (!atomic_xchg(&xe->wedged.flag, 1)) {
> > > +	if (!atomic_xchg(&xe->wedged.fini, 1)) {
> 
> Just curious. Why do we need runtime pm reference held throughout till the
> end of driver unload?
> Even if this is needed why not associate releasing runtime pm wakeref with
> the flr_on_fini flag.

I guess flr_on_fini has multiple setters and it's not always clear how many.

> I was facing a similar issue for uncorrectable series and i was thinking of
> releasing runtime
> pm wakeref based on flr_fini.

Sure but I'm not touching any existing behaviour in this series. Probably
worth exploring as a follow up.

Raag

> > > +		xe_device_wedged_get(xe);
> > >   		xe->needs_flr_on_fini = true;
> > >   		xe_pm_runtime_get_noresume(xe);
> > >   		drm_err(&xe->drm,
> > > diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
> > > index d61fdb362f91..14677729aa24 100644
> > > --- a/drivers/gpu/drm/xe/xe_device.h
> > > +++ b/drivers/gpu/drm/xe/xe_device.h
> > > @@ -194,9 +194,19 @@ bool xe_device_is_l2_flush_optimized(struct xe_device *xe);
> > >   void xe_device_td_flush(struct xe_device *xe);
> > >   void xe_device_l2_flush(struct xe_device *xe);
> > > +static inline void xe_device_wedged_get(struct xe_device *xe)
> > > +{
> > > +	atomic_inc(&xe->wedged.ref);
> > > +}
> > > +
> > > +static inline void xe_device_wedged_put(struct xe_device *xe)
> > > +{
> > > +	atomic_dec(&xe->wedged.ref);
> > > +}
> > > +
> > >   static inline bool xe_device_wedged(struct xe_device *xe)
> > >   {
> > > -	return atomic_read(&xe->wedged.flag);
> > > +	return atomic_read(&xe->wedged.ref);
> > >   }
> > >   void xe_device_set_wedged_method(struct xe_device *xe, unsigned long method);
> > > diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> > > index 32dd2ffbc796..66e673e4e3e7 100644
> > > --- a/drivers/gpu/drm/xe/xe_device_types.h
> > > +++ b/drivers/gpu/drm/xe/xe_device_types.h
> > > @@ -485,8 +485,10 @@ struct xe_device {
> > >   	/** @wedged: Struct to control Wedged States and mode */
> > >   	struct {
> > > -		/** @wedged.flag: Xe device faced a critical error and is now blocked. */
> > > -		atomic_t flag;
> > > +		/** @wedged.fini: Needs cleanup on fini */
> > > +		atomic_t fini;
> > to me it is easier to see this as a 'permanent' flag than
> > a flag to 'clean' on 'fini'.
> > 
> > but up to you...
> > 
> > either way:
> > 
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > > +		/** @wedged.ref: Refcount for wedged device, blocks critical path execution */
> > > +		atomic_t ref;
> > >   		/** @wedged.mode: Mode controlled by kernel parameter and debugfs */
> > >   		enum xe_wedged_mode mode;
> > >   		/** @wedged.method: Recovery method to be sent in the drm device wedged uevent */
> > > -- 
> > > 2.43.0
> > >