Re: [PATCH 05/37] drm/display: bridge-connector: split code creating the connector to a subfunction

[Luca Ceresoli]

Hi Maxime,

On Thu Jul 16, 2026 at 3:22 PM CEST, Maxime Ripard wrote:
> On Thu, Jul 16, 2026 at 10:37:24AM +0200, Luca Ceresoli wrote:
>> >> > Now if the bridges start doing it themselves we should go back to
>> >> > those encoder drivers and ditch all the drm_bridge_connector from
>> >> > there?
>> >> >
>> >> > I must be missing something. Can you elaborate on this?
>> >>
>> >> drm_bridge_connectors bring together a (complete) bridge chain and a
>> >> connector. If you don't have either anymore, then we shouldn't keep it
>> >> around.
>> >>
>> >> What I was suggesting before was only a suggestion. I guess we could
>> >> also make the encoder own the hotplug handling code and create the
>> >> drm_bridge_connector when the chain is complete, and remove it when it's
>> >> no longer the case.
>> >
>> > That's an interesting option. We don't have to keep drm_bridge_connector
>> > in its current form, but I don't think we should go back to individual
>> > bridge driver creating connectors, especially now that we have bridge
>> > chains where the connector ops are implemented collectively by multiple
>> > bridges.
>>
>> I definitely agree we don't want to add burden back on the encoder.
>
> I don't think Laurent mentioned the encoder anywhere.

Ah, indeed, sorry! However, I think both the bridges and the encoder
drivers should equally have the minimum burden on them.

Right now the recommended practice is:

 - bridges do not create connectors (thanks to DRM_BRIDGE_ATTACH_NO_CONNECTOR)
 - encoders just call drm_bridge_connector_init(), which does all the
   common operations to populate a suitable drm_connector

So all common operations involved in connector creation and bridge chain
analysis are implemented in common code, not per-bridge or
per-encoder. That's good.

>> >> We can discuss alternatives too. But either way, we shouldn't have it
>> >> stick around.
>>
>> Bottom line, I roughly see three ideas mentioned:
>>
>>  1. (this series) extend the drm_bridge_connector to create the
>>     drm_connector based on bridge hotplug events [+rename it]
>>  2. - keep the drm_bridge_connector (mostly) as is
>>     - let each encoder driver add/remove it based on bridge hotplug events
>>     => more burden on encoder drivers -> no
>
> Can you motivate that with *any* reason? Because I really feel like it's
> the best solution going forward.

My understanding of your idea (maybe a bit overstressed just to ensure it's
clear) is that:

 - the drm_bridge_connector should stay (almost) unmodified
 - there should be no new "manager" component (not sure this is actually
   your opinion, can you comment on this specifically?)
 - every encoder driver would have to:
   - register to receive hotplug events
   - when receiving one such event, find out whether the hardware is
     complete or not (by calling drm_bridge_connector_pipeline_is_complete()
     or so)
   - create/destroy a drm_bridge_connector based on hotplug events

Is this somewhat close to what you have in mind?


To me the best solution to add hotplug support is that encoder drivers
replace the single drm_bridge_connector_init() call with a single call to
something new (let's call it a hotplug manager), which takes care of all
the common aspects: registering to receive bridge hotplug events, finding
out whether the hardware pipeline is complete or not, and add/remove the
drm_connector based on that.

In other words, the changes on encoder drivers would be similar to patch
37. In a nutshell:

  -	connector = drm_bridge_connector_init(lcdif->drm, encoder);
  +	drm_hotplug_manager = drm_hotplug_manager_init(lcdif->drm, encoder);

All the hotplug logic would be in common code, and any maintenance and
future improvements to it would stay in a single place, benefitting all
encoders at once.

What do you think about this?

Luca

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