Hi Maxime, thanks for the quick feedback!
On Mon Aug 17, 2026 at 10:26 AM CEST, Maxime Ripard wrote: > Hi, > > On Fri, Aug 14, 2026 at 04:05:51PM +0200, Luca Ceresoli wrote: >> By the documentation drm_of_find_panel_or_bridge() returns a "drm_panel or >> drm_bridge", without specifying which one is returned in case both exist. >> >> Definitely it never returns both. If both exist (and @bridge is != NULL), >> the current implementation prioritizes the drm_panel pointer and returns >> that. In most cases (including devm_drm_of_get_bridge() and >> drmm_of_get_bridge()) this is used to implement the following logic >> (simplified): >> >> drm_of_find_panel_or_bridge(..., &panel, &bridge); >> if (panel) >> bridge = [devm_]drm_panel_bridge_add[_typed](panel); >> >> Work is in progress to make every drm_panel automatically create a >> panel_bridge, so a panel_bridge will always be present for every >> drm_panel. This means the above logic would create a panel_bridge that >> already exists. Avoid it by returning the drm_bridge when both are present, >> instead of the drm_panel. >> >> For the case where @bridge == NULL, this commit does not change anything: >> the 'if (bridge)' body is never executed and the drm_panel (if found) is >> always returned. >> >> Signed-off-by: Luca Ceresoli <[email protected]> > > I'm always a bit concerned about that kind of change, because you > basically change some part of the API we don't really know if a driver > relies on. Indeed, this was a pretty wild attempt to start discussion. And in fact Sashiko found 3 drivers which would break: sun4i, fsl-dcu and tegra. I had a look, some could possibly be fixed quite simply, but not all. > That being said, if we do create a panel_bridge for every panel, why do > we still need drm_of_find_panel_or_bridge()? We'll only really need > of_drm_find_bridge(), no? Yes, in the long term we can get rid of drm_of_find_panel_or_bridge(), but not in the short term due to the above mentioned drivers, and others which use it in "creative" ways. The point is about the transition. This patch was an attempt to convert at once all the many drivers which use the most common pattern: | drm_of_find_panel_or_bridge(..., &panel, &bridge); | if (panel) | bridge = drm_panel_bridge_add(); to then fix manually the remaining drivers (patches 6-11). But this approach doesn't fly due to the legacy users of drm_of_find_panel_or_bridge() as mentioned above. So what about this other approach instead: 1. leave drm_of_find_panel_or_bridge() as is (it's already deprecated anyway) 2. keep patch 5 (which "breaks" mayt drivers which would create an additional panel_bridge) 3. perhaps add a warning when an additional bridge is created for the some panel 4. *in the same series*, manually convert *all* drivers to use of_drm_find_bridge() or of_drm_get_bridge_by_endpoint() -- like patches 6-11, but done on all drivers 5. apply the series atomically, because it wouldn't be bisectable between items 2 and 4 Your opinion? Kind regards, Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
