RE: [PATCH 8/8] drm: writeback: rename drm_writeback_connector_init_with_encoder()

[Kandpal, Suraj]

> -----Original Message-----
> From: Kandpal, Suraj
> Sent: Friday, August 8, 2025 10:35 AM
> To: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>; Dmitry Baryshkov
> <dmitry.baryshkov@xxxxxxxxxxxxxxxx>; Jani Nikula
> <jani.nikula@xxxxxxxxxxxxxxx>; Harry Wentland <harry.wentland@xxxxxxx>;
> Leo Li <sunpeng.li@xxxxxxx>; Rodrigo Siqueira <siqueira@xxxxxxxxxx>; Alex
> Deucher <alexander.deucher@xxxxxxx>; Christian König
> <christian.koenig@xxxxxxx>; David Airlie <airlied@xxxxxxxxx>; Simona Vetter
> <simona@xxxxxxxx>; Liviu Dudau <liviu.dudau@xxxxxxx>; Maarten Lankhorst
> <maarten.lankhorst@xxxxxxxxxxxxxxx>; Maxime Ripard <mripard@xxxxxxxxxx>;
> Thomas Zimmermann <tzimmermann@xxxxxxx>; Rob Clark
> <robin.clark@xxxxxxxxxxxxxxxx>; Dmitry Baryshkov <lumag@xxxxxxxxxx>;
> Abhinav Kumar <abhinav.kumar@xxxxxxxxx>; Jessica Zhang
> <jessica.zhang@xxxxxxxxxxxxxxxx>; Sean Paul <sean@xxxxxxxxxx>; Marijn
> Suijten <marijn.suijten@xxxxxxxxxxxxxx>; Laurent Pinchart
> <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>; Tomi Valkeinen
> <tomi.valkeinen+renesas@xxxxxxxxxxxxxxxx>; Kieran Bingham
> <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>; Geert Uytterhoeven
> <geert+renesas@xxxxxxxxx>; Magnus Damm <magnus.damm@xxxxxxxxx>;
> Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>; Maíra Canal
> <mcanal@xxxxxxxxxx>; Raspberry Pi Kernel Maintenance <kernel-
> list@xxxxxxxxxxxxxxx>
> Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; linux-arm-msm@xxxxxxxxxxxxxxx;
> freedreno@xxxxxxxxxxxxxxxxxxxxx; linux-renesas-soc@xxxxxxxxxxxxxxx
> Subject: RE: [PATCH 8/8] drm: writeback: rename
> drm_writeback_connector_init_with_encoder()
> 
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
> 
> LGTM,
> Reviewed-by: Suraj Kandpal <suraj.kandpal@xxxxxxxxx>

One thing I noticed was after the Rename both drm_writeback_connector_init
And drmm_writeback_connector_init have identical comments both allowing custom encoders
To be used now is that what we were aiming for with the only difference being the 
ret = drmm_add_action_or_reset(dev, drm_writeback_connector_cleanup,
                                       wb_connector);
call ?

Regards,
Suraj Kandpal

> 
> > > ---
> > >   drivers/gpu/drm/drm_writeback.c | 14 +++++++-------
> > >   include/drm/drm_writeback.h     | 10 +++++-----
> > >   2 files changed, 12 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_writeback.c
> > > b/drivers/gpu/drm/drm_writeback.c index
> > >
> >
> 1a01df91b2c5868e158d489b782f4c57c61a272c..ec2575c4c21b7449707b05953
> > 22e
> > > 2202a0cf9865 100644
> > > --- a/drivers/gpu/drm/drm_writeback.c
> > > +++ b/drivers/gpu/drm/drm_writeback.c
> > > @@ -235,7 +235,7 @@ static int __drm_writeback_connector_init(struct
> > drm_device *dev,
> > >   }
> > >
> > >   /**
> > > - * drm_writeback_connector_init_with_encoder - Initialize a
> > > writeback connector with
> > > + * drm_writeback_connector_init - Initialize a writeback connector
> > > + with
> > >    * a custom encoder
> >
> > If I understood correctly your series you want to reduce the usage of
> > non- drmm writeback, so maybe we can add a comment to direct poeple to
> > drmm variant to avoid new usage of this API?
> >
> > With or without this:
> >
> > Reviewed-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>
> >
> > >    *
> > >    * @dev: DRM device
> > > @@ -263,11 +263,11 @@ static int
> > > __drm_writeback_connector_init(struct
> > drm_device *dev,
> > >    *
> > >    * Returns: 0 on success, or a negative error code
> > >    */
> > > -int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
> > > -					      struct drm_writeback_connector
> > *wb_connector,
> > > -					      struct drm_encoder *enc,
> > > -					      const struct drm_connector_funcs
> > *con_funcs,
> > > -					      const u32 *formats, int n_formats)
> > > +int drm_writeback_connector_init(struct drm_device *dev,
> > > +				 struct drm_writeback_connector
> > *wb_connector,
> > > +				 const struct drm_connector_funcs
> > *con_funcs,
> > > +				 struct drm_encoder *enc,
> > > +				 const u32 *formats, int n_formats)
> > >   {
> > >   	struct drm_connector *connector = &wb_connector->base;
> > >   	int ret;
> > > @@ -284,7 +284,7 @@ int
> > > drm_writeback_connector_init_with_encoder(struct drm_device *dev,
> > >
> > >   	return ret;
> > >   }
> > > -EXPORT_SYMBOL(drm_writeback_connector_init_with_encoder);
> > > +EXPORT_SYMBOL(drm_writeback_connector_init);
> > >
> > >   /**
> > >    * drm_writeback_connector_cleanup - Cleanup the writeback
> > > connector diff --git a/include/drm/drm_writeback.h
> > > b/include/drm/drm_writeback.h index
> > >
> >
> 879ca103320cc225ffb3687419088361315535fc..958466a05e604b387722610fc1
> > 1f
> > > 9e841316d21b 100644
> > > --- a/include/drm/drm_writeback.h
> > > +++ b/include/drm/drm_writeback.h
> > > @@ -137,11 +137,11 @@ drm_connector_to_writeback(struct
> > drm_connector *connector)
> > >   	return container_of(connector, struct drm_writeback_connector, base);
> > >   }
> > >
> > > -int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
> > > -				struct drm_writeback_connector
> > *wb_connector,
> > > -				struct drm_encoder *enc,
> > > -				const struct drm_connector_funcs
> > *con_funcs, const u32 *formats,
> > > -				int n_formats);
> > > +int drm_writeback_connector_init(struct drm_device *dev,
> > > +				 struct drm_writeback_connector
> > *wb_connector,
> > > +				 const struct drm_connector_funcs
> > *con_funcs,
> > > +				 struct drm_encoder *enc,
> > > +				 const u32 *formats, int n_formats);
> > >
> > >   int drmm_writeback_connector_init(struct drm_device *dev,
> > >   				  struct drm_writeback_connector
> > *wb_connector,
> > >
> >
> > --
> > Louis Chauvet, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com