Thread (54 messages) 54 messages, 4 authors, 2025-12-16

Re: [PATCH 06/26] drm/bridge: add devm_drm_of_find_bridge

From: "Luca Ceresoli" <luca.ceresoli@bootlin.com>
Date: 2025-12-15 14:11:45
Also in: imx, linux-amlogic, linux-doc, linux-mediatek, linux-renesas-soc, linux-samsung-soc, lkml

Hi Maxime,

On Mon Dec 15, 2025 at 11:35 AM CET, Maxime Ripard wrote:
[...]
quoted
quoted
Additionally, as a matter of fact there are currently drivers storing
bridge pointers. The next_bridge is the most common case. Code using
drm_bridge_connector_init() for example can store up to eight of them, but
individual drivers are the hardest to hunt for.

I can see these (potential) tools to handle this (not mutually exclusive):

 1. remove drm_bridge pointers pointing to other bridges
 2. check whether a bridge (say B) still exists before any dereference
    to B->another_bridge: that's drm_bridge_enter/exit()
 3. let owners of bridge pointers be notified when a bridge is unplugged,
    so they can actively put their reference and clear their pointer

For item 1, I think the drm_of_bridge_attach() idea quoted above would
work, at least for the simple cases where bridge drivers use the
next_bridge only for attach. A next_bridge pointer in struct drm_bridge is
not even needed in that case, the pointer would be computed from OF when
needed and not stored. I can do an experiment and send a first series, do
you think it would be useful?
I had a look and, while the implementation should be simple, only a few
drivers could benefit right now. The majority fall into one of these
categories:

 * drivers using drm_of_find_panel_or_bridge() or *_of_get_bridge()
   (maybe 60-80% of all drivers, those will have to wait for the panel
   improvements)
 * drivers using the next_bridge pointer for more than just attach
 * drivers doing more complicated stuff

I think your "put next_bridge in __drm_bridge_free" idea would fit well the
2nd category and perhaps also the 1st one. For the 3rd category we'd need
something different, e.g. a per-driver .destroy callback.
Yep, that's fine. We should optimize for the common case, with an escape
hatch. That's exactly what we are talking about here.
Not sure why, but it's taking a while before I grasp your ideas about this
series and meld them with mine. I hopefully got a clear POV now, so based
on it my plan is to rework this series to:

 * keep drm_of_find_bridge() but renamed to of_drm_get_bridge(), and keep
   patches 1-5 (with the changes suggested by you and Louis, nothing big
   and all already sent in v2)
 * not add devm_drm_of_find_bridge()
 * add next_bridge pointer to struct drm_bridge and call
   drm_bridge_put(bridge->next_bridge) in __drm_bridge_free, document it
 * convert patches 7-26 to use bridge->next_bridge where applicable,
   or to do something different when needed
 * maybe remove part of patches 7-26 just to reduce spam and rework effort
   in case of further iterations, to send them separately once the approach
   is accepted

Does it look OK?

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help