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

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

On Mon, Nov 25, 2024 at 07:11:39AM -0500, Rodrigo Vivi wrote:
> On Tue, Nov 19, 2024 at 09:26:42PM +0100, Michal Wajdeczko wrote:
> > On 19.11.2024 20:59, Lucas De Marchi wrote:
> > > 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.
> > 
> > and this 'easier' is not just that as it also means that code is much
> > more robust as it's the compiler job to take care of all the cleanup,
> > which could be tricky when doing early exits from different paths, or
> > due to late updates to the code done by someone else
> > 
> > >> 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.
> > 
> > there was a little more in the cover letter:
> > 
> > ""
> > This should allow us to use:
> > 
> > 	CLASS(xe_fw, var)(fw, XE_FORCEWAKE_ALL);
> > or
> > 	guard(xe_fw)(fw, XE_FW_GT);
> > or
> > 	scoped_guard(xe_fw, fw, XE_FW_GT)
> > 		foo();
> > 
> > without any concern of leaking the force-wake references.
> > ""
> > 
> > do you want/need more?
> 
> this block here also just say 'allow to use' but fail to describe
> why we should use. But the upper block started by Raav seems a good
> why. Just make sure that that is in the commit message and
> 
> Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

with

Reviewed-by: Raag Jadav <raag.jadav@intel.com>