Re: [PATCH v2 1/2] drm/xe: Introduce force-wake guard class

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

On Tue, Nov 19, 2024 at 01:47:28PM -0500, Rodrigo Vivi wrote:
> On Mon, Nov 18, 2024 at 07:45:10PM +0100, Michal Wajdeczko wrote:
> > There is support for 'classes' with constructor and destructor
> > semantics that can be used for any scope-based resource management,
> > like our domain force-wake management.
> 
> There is, okay...
> It can be, okay...
> 
> But why do we want?
> What are the advantages?

Makes error cases easier to handle + less code.
We significantly cleaned up intel pinctrl and gpio drivers a few months ago
and almost halved the footprint in some cases.

Raag

> > 
> > Define xe_fw class that can be used for force-wake management.
> > 
> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> > ---
> > v2: aligned to new force-wake usage model (Michal)
> > ---
> >  drivers/gpu/drm/xe/xe_force_wake.h       | 12 ++++++++++++
> >  drivers/gpu/drm/xe/xe_force_wake_types.h | 10 ++++++++++
> >  2 files changed, 22 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_force_wake.h b/drivers/gpu/drm/xe/xe_force_wake.h
> > index 0e3e84bfa51c..e46e44f7eb70 100644
> > --- a/drivers/gpu/drm/xe/xe_force_wake.h
> > +++ b/drivers/gpu/drm/xe/xe_force_wake.h
> > @@ -6,6 +6,8 @@
> >  #ifndef _XE_FORCE_WAKE_H_
> >  #define _XE_FORCE_WAKE_H_
> >  
> > +#include <linux/cleanup.h>
> > +
> >  #include "xe_assert.h"
> >  #include "xe_force_wake_types.h"
> >  
> > @@ -61,4 +63,14 @@ xe_force_wake_ref_has_domain(unsigned int fw_ref, enum xe_force_wake_domains dom
> >  	return fw_ref & domain;
> >  }
> >  
> > +DEFINE_CLASS(xe_fw, struct xe_force_wake_guard,
> > +	     xe_force_wake_put(_T.fw, _T.ref),
> > +	     ({ (struct xe_force_wake_guard){ fw, xe_force_wake_get(fw, domains) }; }),
> > +	     struct xe_force_wake *fw, enum xe_force_wake_domains domains);
> > +
> > +static inline void *class_xe_fw_lock_ptr(class_xe_fw_t *_T)
> > +{
> > +	return _T->ref ? _T : NULL;
> > +}
> > +
> >  #endif
> > diff --git a/drivers/gpu/drm/xe/xe_force_wake_types.h b/drivers/gpu/drm/xe/xe_force_wake_types.h
> > index 899fbbcb3ea9..15456a24f707 100644
> > --- a/drivers/gpu/drm/xe/xe_force_wake_types.h
> > +++ b/drivers/gpu/drm/xe/xe_force_wake_types.h
> > @@ -85,4 +85,14 @@ struct xe_force_wake {
> >  	struct xe_force_wake_domain domains[XE_FW_DOMAIN_ID_COUNT];
> >  };
> >  
> > +/**
> > + * struct xe_force_wake_guard - Xe force wake guard
> > + */
> > +struct xe_force_wake_guard {
> > +	/** @fw: force wake manager */
> > +	struct xe_force_wake *fw;
> > +	/** @ref: opaque reference to woken domains */
> > +	unsigned int ref;
> > +};
> > +
> >  #endif
> > -- 
> > 2.43.0
> >