linux-kernel - Re: [PATCH] drm/tidss: encoder: convert to devm_drm_bridge

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250717155659.000eb000@booty>
Date: Thu, 17 Jul 2025 15:56:59 +0200
From: Luca Ceresoli <luca.ceresoli@...tlin.com>
To: "Michael Walle" <mwalle@...nel.org>, "Jyri Sarha" <jyri.sarha@....fi>,
 "Tomi Valkeinen" <tomi.valkeinen@...asonboard.com>
Cc: "Maarten Lankhorst" <maarten.lankhorst@...ux.intel.com>, "Maxime Ripard"
 <mripard@...nel.org>, "Thomas Zimmermann" <tzimmermann@...e.de>, "David
 Airlie" <airlied@...il.com>, "Simona Vetter" <simona@...ll.ch>,
 <dri-devel@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/tidss: encoder: convert to devm_drm_bridge_alloc()

Hello Jyri, Tomi, Michael,

On Thu, 17 Jul 2025 09:49:44 +0200
"Michael Walle" <mwalle@...nel.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