Hello Jyri, Tomi, Michael,
On Thu, 17 Jul 2025 09:49:44 +0200
"Michael Walle" <mwa...@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?
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
