Re: [PATCH 04/11] drm/panel: merge the drm_kms_helper module into the drm module

["Luca Ceresoli" <luca.ceresoli@bootlin.com>]

Hi Maxime,

On Wed Sep 2, 2026 at 2:36 PM CEST, Maxime Ripard wrote:
> 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,

That was the starting point, but working on it I realized it was very
tricky. So I tried instead copying the relevant bridge/panel.c code into
drm_panel.c, but with more suitable function names, and the result is way
cleaner.

This changed the steps, but not the end result: remove
drm_of_find_panel_or_bridge() and bridge/panel.c entirely.

> 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?

Maybe, but as above, my plan has changed a bit.

My opinion is that for this tricky task we should focus more on the end
result (embed a bridge into every panel, remove
drm_of_find_panel_or_bridge() and bridge/panel.c) than on finding the most
optimal path to get there. So I'm aiming at a path that is
build-bisectable, clean and understandable.

What do you think about this principle?

I'm sending a RFC v2 soon, so we can discuss on a specific proposal.

Luca

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