Re: [PATCH v6 08/26] drm/bridge: panel: add a panel_bridge to every panel

Luca Ceresoli
2025-02-10T17:12:00Z

Hi Dmitry,

On Fri, 7 Feb 2025 21:43:26 +0200
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote:

> > > > @@ -1018,6 +1067,11 @@ struct drm_bridge *devm_drm_panel_bridge_add_typed(struct device *dev,
> > > > {
> > > > struct drm_bridge **ptr, *bridge;
> > > >
> > > > + if (panel->bridge) {
> > > > + DRM_DEBUG("panel %s: returning existing bridge=%p", dev_name(dev), panel->bridge);
> > > > + return panel->bridge;
> > > > + }
> > >
> > > Shouldn't the rest of the function also be removed as you do in other
> > > cases?
> >
> > Indeed it should.
> >
> > And even more, I now realize drm_panel_bridge_add_typed() should also
> > become a simple 'return panel->bridge', like its devm and drmm
> > variants, and its code, implementing the actual creation of a panel
> > bridge, move to an internal function. Otherwise this patch is a bug:
> > existing drivers which do call drm_panel_bridge_add_typed() would end
> > up in having two panel_bridges for the same panel.
> >
> > I must say the process of developing this patch together with the
> > hotplug work was "convoluted" to say the least, which probably explains
> > why this got unnoticed so far.
>
> That's why I suggested to post this series separately - it saves you
> from rebasing hotplug work on top.

Yes, that's sure, but not keeping my hotplug patches on top of the
panel_bridge ones makes it much harder for me to test on real hardware,
so each way has pros and cons.

However I might send only the panel_bridge patches at the next
iteration.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering