Hi, On Mon, Apr 07, 2025 at 05:27:39PM +0200, Luca Ceresoli wrote: > This is the new API for allocating DRM bridges. > > The devm lifetime management of this driver is peculiar. The underlying > device for the panel_bridge is the panel, and the devm lifetime is tied the > panel device (panel->dev). However the panel_bridge allocation is not > performed by the panel driver, but rather by a separate entity (typically > the previous bridge in the encoder chain). > > Thus when that separate entoty is destroyed, the panel_bridge is not > removed automatically by devm, so it is rather done explicitly by calling > drm_panel_bridge_remove(). This is the function that does devm_kfree() the > panel_bridge in current code, so update it as well to put the bridge > reference instead. > > Signed-off-by: Luca Ceresoli <[email protected]> > --- > > To: Maarten Lankhorst <[email protected]> > To: Maxime Ripard <[email protected]> > To: Thomas Zimmermann <[email protected]> > To: David Airlie <[email protected]> > To: Simona Vetter <[email protected]> > To: Andrzej Hajda <[email protected]> > To: Neil Armstrong <[email protected]> > To: Robert Foss <[email protected]> > To: Laurent Pinchart <[email protected]> > To: Jonas Karlman <[email protected]> > To: Jernej Skrabec <[email protected]> > To: Jagan Teki <[email protected]> > To: Shawn Guo <[email protected]> > To: Sascha Hauer <[email protected]> > To: Pengutronix Kernel Team <[email protected]> > To: Fabio Estevam <[email protected]> > To: Douglas Anderson <[email protected]> > To: Chun-Kuang Hu <[email protected]> > To: Krzysztof Kozlowski <[email protected]> > To: Dmitry Baryshkov <[email protected]> > Cc: Anusha Srivatsa <[email protected]> > Cc: Paul Kocialkowski <[email protected]> > Cc: Dmitry Baryshkov <[email protected]> > Cc: Hervé Codina <[email protected]> > Cc: Hui Pu <[email protected]> > Cc: Thomas Petazzoni <[email protected]> > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > --- > drivers/gpu/drm/bridge/panel.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c > index > 79b009ab9396048eac57ad47631a902e949d77c6..ddd1e91970d09b93aa64f50cd9155939a12a2c6f > 100644 > --- a/drivers/gpu/drm/bridge/panel.c > +++ b/drivers/gpu/drm/bridge/panel.c > @@ -287,15 +287,14 @@ struct drm_bridge *drm_panel_bridge_add_typed(struct > drm_panel *panel, > if (!panel) > return ERR_PTR(-EINVAL); > > - panel_bridge = devm_kzalloc(panel->dev, sizeof(*panel_bridge), > - GFP_KERNEL); > - if (!panel_bridge) > - return ERR_PTR(-ENOMEM); > + panel_bridge = devm_drm_bridge_alloc(panel->dev, struct panel_bridge, > bridge, > + &panel_bridge_bridge_funcs); > + if (IS_ERR(panel_bridge)) > + return (void *)panel_bridge; > > panel_bridge->connector_type = connector_type; > panel_bridge->panel = panel; > > - panel_bridge->bridge.funcs = &panel_bridge_bridge_funcs; > panel_bridge->bridge.of_node = panel->dev->of_node; > panel_bridge->bridge.ops = DRM_BRIDGE_OP_MODES; > panel_bridge->bridge.type = connector_type; > @@ -327,7 +326,7 @@ void drm_panel_bridge_remove(struct drm_bridge *bridge) > panel_bridge = drm_bridge_to_panel_bridge(bridge); > > drm_bridge_remove(bridge); > - devm_kfree(panel_bridge->panel->dev, bridge); > + devm_drm_put_bridge(panel_bridge->panel->dev, bridge); > } > EXPORT_SYMBOL(drm_panel_bridge_remove);
I'm fine with it on principle, but as a temporary measure. Now that we have the panel allocation function in place, we can just allocate a bridge for each panel and don't need drm_panel_bridge_add_* at all. As I was saying before, it doesn't need to happen right now, or before the rest of your work for hotplug goes in. But this needs to be tackled at some point. Maxime
signature.asc
Description: PGP signature
