Hi Dmitry,
On Fri, 7 Feb 2025 21:43:26 +0200
Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> 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
https://bootlin.com
