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. 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. > - 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. 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. 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. Maxime
signature.asc
Description: PGP signature
