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

[Luca Ceresoli]

Hi Maxime,

On Fri Jun 26, 2026 at 12:09 PM CEST, Maxime Ripard wrote:
> On Wed, Jun 24, 2026 at 05:47:10PM +0200, Luca Ceresoli wrote:
>> On Wed Jun 24, 2026 at 1:41 PM CEST, Maxime Ripard wrote:
>> > Hi,x
>> >
>> > On Fri, Jun 12, 2026 at 02:56:24PM +0200, Luca Ceresoli wrote:
>> >> On Mon Jun 8, 2026 at 1:40 PM CEST, Maxime Ripard wrote:
>> >> > On Tue, May 19, 2026 at 12:37:22PM +0200, Luca Ceresoli wrote:
>> >> >> In preparation to introduce bridge hotplug, split out from
>> >> >> drm_bridge_connector_init() the code adding the drm_connector into a
>> >> >> dedicated function. This will be needed to be able to add (and re-add) the
>> >> >> connector from different code paths.
>> >> >
>> >> > Same story here, explaining what you need later on that calls for that
>> >> > change would be nice.
>> >>
>> >> Here's a more verbose version:
>> >>
>> >>     Currently drm_bridge_connector_init() does two things:
>> >>
>> >>      * allocate and initialize the drm_bridge_connector
>> >>        (which embeds a drm_connector)
>> >>      * initialize and register the embedded drm_connector
>> >>
>> >>     For bridge hotplug we need to separate these two actions:
>> >>
>> >>      * the drm_connector needs to be added and removed at any time based on
>> >>        hotplug events
>> >>      * the drm_bridge_connector is designated to create and remove the
>> >>        drm_connector, so it must be persistent for the card lifetime
>> >>
>> >>     As the lifetimes of drm_bridge_connector and drm_connector become
>> >>     different, we need to create them in different moments.
>> >>
>> >>     In preparation to support that, split out from
>> >>     drm_bridge_connector_init() the code adding the drm_connector into a
>> >>     dedicated function. No functional changes, just moving code around for
>> >>     now. A future commit will make the drm_connector be created based on
>> >>     hotplug events.
>> >>
>> >> Looks good?
>> >
>> > The message itself, yes, thanks.
>> >
>> > However, I have questions now :)
>> >
>> > Do we really expect drm_bridge_connector to stick around when a bridge
>> > gets unplugged? If so, how does it cope with having, say, an HDMI
>> > connector, and then swapping out the hotplugged part for an LVDS one?
>> > Does the HDMI connector sticks around indefinitely?
>>
>> In your example, the HDMI drm_connector would be unregistered and put on
>> hotunplug. Its allocation will stick around until the last put but that's
>> quite irrelevant. Then, on plugging the LVDS addon, a new LVDS
>> drm_connector will be created and registered.
>>
>> > *Especially* if we're using overlays for this, I'd expect everything
>> >  after the first hotplugged bridge to be destroyed, no?
>>
>> As said, it would be unregistered immediately but might be freed later on
>> if still refcounted.
>>
>> This is visible in patches 36+15, the path to follow is:
>>
>>  drm_bridge_connector_handle_event(event = DRM_BRIDGE_DETACHED) [patch 36]
>>  -> drm_bridge_connector_dynconn_release()                      [patch 15]
>>
>> Does this solve your concern?
>
> Not really, I'm talking about drm_bridge_connector. The fact that
> bridges are destroyed make sense to me. The fact that
> drm_bridge_connector sticks around doesn't. It's supposed to be a
> connector for bridges. If you don't have bridges because they got
> destroyed, and connector, drm_bridge_connector doesn't have a reason to
> exist anymore, unless it's drm_bridge_hotplug in a trench coat :)

It is not a hotplug-bridge in a trench coat, no :) The code is clear about
this.

I'd say with this series a "drm_bridge_connector" is just becoming
something more (perhaps something else too). Somewhat as "a drm_bridge is
either a bridge or something else". :)


But let's leave names aside for a moment. If just looking at the current
code, the drm_bridge_connector is "a handler, owned by the card/encoder and
having the same lifetime, which takes care of drm_connector
creation/destruction at card probe/removal".

What we need now is just the same plug " and on hotplug events" appended.

So in both cases there needs to be "a handler persitent with the card".

Do we agree so far?


Now, the reason I chose to extend the drm_bridge_connector to achieve the
above is what I tried to motivate in the cover letter:

> The drm_bridge_connector is nowadays the recommended way to implement DRM
> connectors when a chain of bridges is used. It takes care of adding the
> drm_connector when the pipeline is composed by an arbitrarily long chain of
> bridges, which it scans to properly implement the drm_connector
> operations.
>
> As such the drm_bridge_connector looked like the ideal component to
> implement DRM bridge hotplug.
>
> This series augments the drm_bridge_connector code to be able to create and
> destroy the drm_connector reacting on hot(un)plug events.

Before taking that approach I considered some options:

 1. Create a new component, which is a "a handler, owned by the
    card/encoder and having the same lifetime, which takes care of
    drm_connector creation/destruction at card probe/removal and on hotplug
    events", and wait for drivers to migrate from the drm_bridge_connector
    to this new thing

 2. Create a new component, which is a "handler, owned by the card/encoder
    and having the same lifetime, which reacts to hotplug events by
    creating/destroying a drm_bridge_connector (slightly modified to be
    non-drmm), which in turn takes care of drm_connector
    creation/destruction", and wait for drivers to migrate to this new
    thing

 3. Extend the dm_bridge_connector to:
    - behave as before if using the current API
    - behave as before + hotplug if using a new API
    (the migration in most cases is as simple as patch 37)

All 3 options require changes in card drivers to use a new API (and a new
object type for cases 1 and 2).

To me options 1 and 2 involve more redundant code and/or more complexity.

Your thoughts?

Luca

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