Hi Maxime, On Mon Jul 20, 2026 at 4:28 PM CEST, Maxime Ripard wrote: > On Fri, Jul 17, 2026 at 11:41:33AM +0200, Luca Ceresoli wrote: >> 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. > > I agree, but another way to phrase it is: bridges aren't aware of how > the chain is setup, the encoder ties it all together. > >> >> >> 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 > > Yes, and bridges should ideally remain as lightly affected as possible.
I fully agree. > We have probably around 100 bridge drivers at the moment, having some > kind of opt-in to enable hotplug would mean that we can't expect hotplug > to work on a new platform, which should be a last resort. A few changes to each bridge wanting to support hotplug will unavoidably be needed. The .get_next_bridge callback we mentioned in the discussion for patch 30 at least. I'm keeping any other changes, if any, to a minimum. >> - 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? > > Yes. To make things a bit more precise, we need two things: the encoder > to put the chain together, and "something" (that you used to call > manager) to react to hotplug events and handle the bridge > detach/destruction, connector creation/destruction, etc and should stick > around when we enable hotplug. > > What I'm suggesting is that, since the encoder already owns and creates > the chain in the first place, and is there forever, it's only natural > for the encoder to be that "something", and we don't necessarily mean > creating a new entity or piece of code. A bunch of helpers and hooks a > probably going to be enough. That's the idea I had reached too, yes. Except the "bunch of helpers and hooks" could be perhaps as small as one single helper function or little more. > This is where the opt-in part should be, and I'd like, if possible, for > hotplug-enabled encoders to work with any bridge. > >> 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? > > From a high level point-of-view, I think we mostly agree. Good. > We can argue > on the name, and if we should merge it with something else > (drm_encoder_init, drm_bridge_attach, something else?) but that's the > path forward I think. OK, let's see what I can come up with in v2. Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
