On Tue, Sep 01, 2026 at 04:30:40PM +0200, Luca Ceresoli wrote: > On Tue Sep 1, 2026 at 2:40 PM CEST, Luca Ceresoli wrote: > > [...] > > >>> >> > And we'd essentially move drm_panel_bridge into drm_panel.c, and make it > >>> >> > private. > >>> >> > >>> >> Yes in theory, but the panel_bridge code uses other parts of the > >>> >> drm_kms_helper module: drm_atomic_helper and drm_probe_helper, maybe more, > >>> >> so we'd have to move them into the drm module too. > >>> > > >>> > Ah, right. What would happen if we were doing it the other way around > >>> > then? Move drm_panel out of the main drm module? > >>> > >>> Into the drm_kms_helper module? > >>> > >>> I had a look and did some experiments and I found at least one user in the drm > >>> module calling a drm_panel API, and guess who: > >>> > >>> drm_of_find_panel_or_bridge() (in drm_of.c, drm module) > >>> -> calls of_drm_find_panel (in drm_panel.c, drm_kms_helper module) > >>> > >>> Based on our discussion after patch 3, I'm not sure > >>> drm_of_find_panel_or_bridge() will disappear soon. If it doesn't, I guess > >>> we can try to move drm_of_find_panel_or_bridge() into bridge/panel.c which > >>> is in the drm_kms_helper module (and from drm_of.h info > >>> drm_bridge.h?). That however might trigger build failures for drivers which > >>> currently don't select DRM_KMS_HELPER and which would have to select it. > >>> > >>> I'll give it a try, and if I see major drawbacks I will get back to moving > >>> all the drm_kms_helper code into the main drm module. > >> > >> No, I meant into its own module. > > > > Looks like a good idea indeed, making the design more modular and > > dependencies cleaner. > > > >> Do we have any dependency from the main > >> drm module into drm_panel? > > > > As far as I can see there is only the one mentioned above: > > > > drm_of_find_panel_or_bridge() (in drm_of.c, drm module) > > -> calls of_drm_find_panel (in drm_panel.c, would-be the new drm_panel module) > > > > And while drm_of_find_panel_or_bridge() is possibly going to disappear at > > the end of my series, moving drm_panel.c to its own module would make the > > series non-build-bisectable, which would be very annoying. > > > > However there seem to be no calls from the drm module to > > drm_of_find_panel_or_bridge(), so we could move it out as well. What about > > a series doing, in this order: > > > > - move drm_of_find_panel_or_bridge() from the drm module to the > > drm_kms_helper module > > - move drm_panel.c to a new drm_panel module > > - Main change: embed a drm_bridge into every panel > > - convert drivers to stop using the panel_bridge, hopefully removing all > > calls to drm_of_find_panel_or_bridge() > > - remove drm_of_find_panel_or_bridge() > > Ah, no, that won't work. There would be a circular module dependency loop > later on when we embed a drm_bridge into evern drm_panel: indeed at that > point drm_panel will use the atomic and probe helpers to implement the > embedded drm_bridge, resulting in: > > * the panel_bridge code in bridge/panel.c [drm_kms_helper module] > already depends on the drm_panel.c code (it manipulates a drm_panel, OK) > * additionally, the drm_panel.c code, in order to create a drm_bridge, > will depend on the helpers in drm_atomic_helper and drm_probe_helper > code [drm_kms_helper module] > > The loop is only between kernel modules (.ko), not in actual code. So I > think this revised plan should work (the 2nd bullet is key): > > - move drm_of_find_panel_or_bridge() from the drm module to > bridge/panel.c [currently drm_kms_helper module] > - move bridge/panel.o to a new drm_panel_bridge module (NEW) > - move drm_panel.c to a new drm_panel module Looks good on principle, but iirc the starting point of that discussion was to move bridge panel into the new panel module, so I guess we could: 1) Move drm_of_find_panel_or_bridge() to drm_panel.c. Both are still in the drm module at this point, so it should be ok. 2) Create a new panel module, with a dependency on bridge 3) Move the bridge/panel.c code into the new panel module to create the bridge at the same time we create the panel. Would that work? Maxime