Re: [PATCH RFC v2 05/19] drm/panel: embed a drm_bridge into every drm_panel

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

Hi Maxime,

On Tue Sep 8, 2026 at 5:21 PM CEST, Maxime Ripard wrote:

[...]

>> > So there's a lot to unravel, and I wished you had split it, but I can't
>> > find a good way to split it either.
>>
>> As I see it, this could be maybe split in 2:
>>
>>  - copy all needed symbols from bridge/panel.c (or move or whatever, see
>>    discussion below) without using them
>>  - change _alloc, _get, _put etc to actually embed the bridge and start
>>    using the copied/moved symbols
>>
>> Would you prefer that?
>>
>> Note the second bullet, which is the core change, is pretty small, I think
>> a <100 lines diff, but I don't think it can be split further.
>
> Yeah, that's why I'm not sure it's going to make a big difference
> anyway. Sometimes it's just not great and there's nothing to do about
> it, really :/

Indeed.

I think however I'll do the split (two bullets above), as I think it would
help clarifying the actual functional changes introduced by this patch. I
hope the smaller diff will help Jani understand the impact on no-bridge
drivers like i915.

>> >> +int drm_bridge_set_connector_orientation(const struct drm_bridge *bridge,
>> >> +					 struct drm_connector *connector)
>> >> +{
>> >> +	struct drm_panel *panel = drm_bridge_to_panel(bridge);
>> >> +
>> >> +	return drm_connector_set_orientation_from_panel(connector, panel);
>> >> +}
>> >> +EXPORT_SYMBOL(drm_bridge_set_connector_orientation);
>> >
>> > I don't think we should create new ones. Just move the code from
>> > bridge/panel here and remove it there. The only thing left will be the
>> > panel_bridge_add() variants that become almost trivial now, and you can
>> > cleanup the drivers in later patches.
>>
>> What do you mean by "move" exactly? Really move all relevant functions from
>> bridge/panel.c into drm_panel.c, so evey user driver will depend on the new
>> drm_panel module instead of the old one for these symbols?
>
> Yes. Basically, I don't want to end up in a situation where we have
> competing implementations of the same function and we have to mess
> around with names to accomodate one or the other.

Makes sense, sure. I'll target that for v4.

>> > This also allows to get rid of all the symbol renaming, which isn't
>> > great in itself, but also the existing names were good so it's hard to
>> > come with better ones.
>>
>> I think the names in bridge/panel.c make sense there, but not in
>> drm_panel.c. Here's my interpretation (using atomic_enable as an example):
>>
>>  - panel_bridge_atomic_enable
>>    ^^^^^^^^^^^^ <- this is about the panel_bridge
>>
>>  - drm_panel_bridge_atomic_enable
>>    ^^^^^^^^^        <- this is drm_panel stuff
>>              ^^^^^^ <- and specifically about the bridge embedded in drm_panel
>>
>> Does it make sense to you too in this perspective?
>
> It's an internal symbol anyway, right? So it's really doesn't matter
> that much to me.

OK.

Anyway, renaming does not make sense based on the above (moving the
panel_bridge code into drm_panel.c).

Luca

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