Re: [PATCH RFC v2 05/19] drm/panel: embed a drm_bridge into every drm_panel

[Luca Ceresoli]

Hello Jani,

On Tue Sep 8, 2026 at 6:18 PM CEST, Jani Nikula wrote:
> On Thu, 03 Sep 2026, Luca Ceresoli <luca.ceresoli@xxxxxxxxxxx> wrote:
>> Adding a drm_panel does currently not add a panel_bridge wrapping
>> it. Usually the panel_bridge creation happens later, when some other driver
>> (e.g. the previous bridge or the encoder) calls *_of_get_bridge() and the
>> following element in the pipeline is a panel.
>>
>> This has some drawbacks:
>>
>>  * the bridge API is currently the best practice to access various
>>    components of the pipeline, especially with complex cards where bridges
>>    can be combined in different ways on different hardware
>>  * the panel_bridge is not created in the context of the driver of the
>>    underlying physical device (the panel driver), but of some other driver
>>  * that other driver is not aware of whether the returned drm_bridge
>>    pointer is a panel_bridge created on the fly, a pre-existing
>>    panel_bridge or a non-panel bridge
>>  * removal of a panel_bridge requires calling drm_panel_bridge_remove(),
>>    but that other driver doesn't know whether this is needed because it
>>    doesn't know whether it has created a panel_bridge or not
>>
>> Other drivers call [a variant of] drm_panel_bridge_add(), which also has
>> some of the above drawbacks.
>>
>> So far the current approach was working mostly because devm and drmm ensure
>> the panel bridge would be dealloacted at some later point. However with the
>> upcoming implementation of bridge hotplug and dynamic bridge lifetime this
>> will get more complicated.
>>
>> Switch to the new approach: embed a drm_bridge inside every drm_panel,
>> which behaves just like the current drm_panel_bridge.
>
> What does this mean for drivers like i915 that use drm_panel *only* for
> handling panel followers? We don't need the bridge for anything. It'll
> just be excess midlayer baggage.

i915 does not call drm_bridge_attach() nor drm_bridge_add(), right? So the
newly embedded bridge would not even be observable, it would not even be in
the global bridge_list. The only effect would be some extra allocated
memory.

> I'm also concerned about the embedded struct drm_connector being added,
> since that can't and will not be a drm_connector that we'll use.

The drm_connector is added only if drm_bridge_attach() is called (with the
DRM_BRIDGE_ATTACH_NO_CONNECTOR flag not set). As above, i915 does not call
drm_bridge_attach() at all, so no drm_connector is added.

> All of
> our drm_connector are embedded in intel_connector, and all of our
> codebase expects this, and we init them ourselves. Having additional
> drm_connector (not embedded in intel_connector) added by library code
> *will* oops in our driver.

It would be great if you could test this series (even without reviewing if
you can't). Based on my analysis above there should be no regression, and
having that confirmed runtime would be helpful.

And if you can review, this is the one patch that matters. The following
only affect bridge-based drivers. The previous ones are about moving code
around across modules and will be different in v4.

> I haven't had the time for an in-depth look, but it feels like this
> assumes a certain driver model, instead of providing building blocks for
> drivers to use.

For drivers not using drm_bridge, this patch should be just implementation
details changes and some more allocated memory.

Luca

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