Re: [PATCH] drm/tidss: encoder: convert to devm_drm_bridge_alloc()

[Luca Ceresoli <luca.ceresoli@bootlin.com>]

Hello Tomi,

On Fri, 18 Jul 2025 14:13:03 +0300
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:

> Hi,
> 
> On 17/07/2025 16:56, Luca Ceresoli wrote:
> > Hello Jyri, Tomi, Michael,
> > 
> > On Thu, 17 Jul 2025 09:49:44 +0200
> > "Michael Walle" <mwalle@kernel.org> wrote:
> >   
> >> Hi,
> >>
> >> thanks for taking a look!
> >>  
> >>> However allocating an encoder using a bridge alloc function (while we
> >>> used to call an encoder allocation function) looks counter-intuitive.
> >>>
> >>> We had discussed on IRC a different idea, adding a wrapper structure
> >>> around the bridge. Quoting your proposal:
> >>>
> >>>   struct tidss_encoder_bridge {
> >>>     struct drm_bridge bridge;
> >>>     struct tidss_encoder *encoder
> >>>   }
> >>>
> >>> and then in the bridge funcs go from drm_bridge to tidss_encoder_brigde
> >>> and use the pointer to get the original private struct.    
> >>
> >> I was doing that until I've realized that meson/meson_encoder_* is
> >> doing it the way this patch does it.  
> > 
> > Which was done by, er, myself. O:-)
> > 
> > To my excuse, meson was using *_encoder_alloc() but rather
> > devm_kzalloc() + drm_simple_encoder_init(), and the change was
> > semi-automated via a coccinelle script, so I didn't fully realize that.
> >   
> >>> That would be cleaner and more intuitive, but use a bit more memory and
> >>> have an additional pointer deref, thus I think we can live with the
> >>> patch you just sent, at least for now.    
> >>
> >> I'm fine with changing it to the wrapper struct. It's your/the
> >> maintainers call :)  
> > 
> > I think the driver maintainers opinion is more relevant, but in lack of
> > one I think we can take the patch as is, given it's already written.
> > 
> > Jyri, Tomi?  
> 
> I think this is fine, even though I do agree the tidss_encoder.c is very
> confusing.
> 
> I'll pick this up. I think drm-misc-next-fixes is the correct branch here.

Looks like to correct branch to me.

Thanks for handling this!

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com