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

[Lucas De Marchi]

On Tue, Nov 19, 2024 at 09:21:17PM +0200, Raag Jadav wrote:
>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.

I'm ok with using this pattern as I already expressed before.
Particularly seeing other subsystems adopting it and being used well in
the core kernel as well.

Main ask here is to provide proper justification in the commit message.

Lucas De Marchi

>
>Raag
>
>> >
>> > Define xe_fw class that can be used for force-wake management.
>> >
>> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> > Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>> > Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray at 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
>> >