Re: [RFC 1/2] drm: add devm release action

[Aravind Iddamsetty <aravind.iddamsetty@xxxxxxxxxxxxxxx>]

On 21/03/24 12:17, Jani Nikula wrote:

> On Thu, 21 Mar 2024, Aravind Iddamsetty <aravind.iddamsetty@xxxxxxxxxxxxxxx> wrote:
> > On 21/03/24 02:23, Rodrigo Vivi wrote:
> > > On Wed, Mar 20, 2024 at 04:14:25PM +0530, Aravind Iddamsetty wrote:
> > > > In scenarios where drm_dev_put is directly called by driver we want to
> > > > release devm_drm_dev_init_release action associated with struct
> > > > drm_device.
> > > >
> > > > Cc: Thomas Hellstr_m <thomas.hellstrom@xxxxxxxxxxxxxxx>
> > > >
> > > > Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@xxxxxxxxxxxxxxx>
> > > > ---
> > > >  drivers/gpu/drm/drm_drv.c | 6 ++++++
> > > >  include/drm/drm_drv.h     | 4 ++++
> > > >  2 files changed, 10 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > > > index 243cacb3575c..b23c5081812b 100644
> > > > --- a/drivers/gpu/drm/drm_drv.c
> > > > +++ b/drivers/gpu/drm/drm_drv.c
> > > > @@ -714,6 +714,12 @@ static int devm_drm_dev_init(struct device *parent,
> > > >  					devm_drm_dev_init_release, dev);
> > > >  }
> > > >  
> > > > +void __devm_drm_dev_release_action(struct drm_device *dev)
> > > > +{
> > > > +	devm_release_action(dev->dev, devm_drm_dev_init_release, dev);
> > > > +}
> > > > +EXPORT_SYMBOL(__devm_drm_dev_release_action);
> > > > +
> > > >  void *__devm_drm_dev_alloc(struct device *parent,
> > > >  			   const struct drm_driver *driver,
> > > >  			   size_t size, size_t offset)
> > > > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> > > > index 8878260d7529..a1d12cb72088 100644
> > > > --- a/include/drm/drm_drv.h
> > > > +++ b/include/drm/drm_drv.h
> > > > @@ -444,6 +444,8 @@ struct drm_driver {
> > > >  	const struct file_operations *fops;
> > > >  };
> > > >  
> > > > +void __devm_drm_dev_release_action(struct drm_device *dev);
> > > > +
> > > >  void *__devm_drm_dev_alloc(struct device *parent,
> > > >  			   const struct drm_driver *driver,
> > > >  			   size_t size, size_t offset);
> > > > @@ -477,6 +479,8 @@ void *__devm_drm_dev_alloc(struct device *parent,
> > > >  #define devm_drm_dev_alloc(parent, driver, type, member) \
> > > >  	((type *) __devm_drm_dev_alloc(parent, driver, sizeof(type), \
> > > >  				       offsetof(type, member)))
> > > > +#define devm_drm_release_action(drm_dev) \
> > > > +	__devm_drm_dev_release_action(drm_dev)
> > > >           
> > > why not simply renaming the original function?
> > >         
> > didn't get you which function are you referring to?
> >       
> Why add a function with double-underscores, and then wrap it in a macro
> without the underscores?
>
> devm_drm_dev_alloc() is that way only because it needs sizeof and
> offsetof on the arguments, not to set a general example.
>

Oh ok got it Rodrigo was referring to _devm_drm_dev_release_action.

    right, I took the reference of devm_drm_dev_alloc, thanks for the explanation
    will change it.
    Regards,
    Aravind.

> BR,
> Jani.
>
> > Thanks,
> > Aravind.
> > > >  
> > > >  struct drm_device *drm_dev_alloc(const struct drm_driver *driver,
> > > >  				 struct device *parent);
> > > > -- 
> > > > 2.25.1
> > > >
> > > >           
> > >         
> >       
>