Re: [RFC 0/3] FW guard class

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

On Mon, Jun 17, 2024 at 05:24:24PM +0000, Matthew Brost wrote:
> On Mon, Jun 17, 2024 at 04:34:27PM +0200, Michal Wajdeczko wrote:
> > There is support for 'classes' with constructor and destructor
> > semantics that can be used for any scope-based resource management,
> > like device force-wake management.
> > 
> > Add necessary definitions explicitly, since existing macros from
> > linux/cleanup.h can't deal with our specific requirements yet.
> > 
> > This should allow us to use:
> > 
> > 	scoped_guard(xe_fw, fw, XE_FW_GT)
> > 		foo();
> > or
> > 	CLASS(xe_fw, var)(fw, XE_FW_GT);
> > 
> > without any concern of leaking the force-wake references.
> > 
> > Note: this is preliminary code as right now it's unclear how to
> > correctly handle errors from the force-wake functions.
> > 
> 
> I'm personally don't like this at all. IMO it obfuscate the code with
> little real benefit. This is just an opinion though, others opinions may
> differ from mine.

Well, on the positive side, it is not adding a driver only thing like
i915's with_runtime_pm() macro.

But I'm also not sure if I like the overall idea anyway:

- I don't like adding C++isms in a pure C code. Specially something not
so standard and common that will decrease the ramp-up time for newcomers.
- It looks like and extra overhead on the object creation destruction.
- It looks not flexible for handling different cases... like forcewake for
instance where we might want to ignore the ack timeout in some cases.

> 
> Matt
> 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > 
> > Michal Wajdeczko (3):
> >   drm/xe: Introduce force-wake guard class
> >   drm/xe: Use new FW guard in xe_mocs.c
> >   drm/xe: Use new FW guard in xe_pat.c
> > 
> >  drivers/gpu/drm/xe/xe_force_wake.h       | 48 +++++++++++++++++++
> >  drivers/gpu/drm/xe/xe_force_wake_types.h | 12 +++++
> >  drivers/gpu/drm/xe/xe_mocs.c             | 12 +----
> >  drivers/gpu/drm/xe/xe_pat.c              | 60 ++++++++----------------
> >  4 files changed, 82 insertions(+), 50 deletions(-)
> > 
> > -- 
> > 2.43.0
> >