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 <luca.ceres...@bootlin.com> > --- > > To: Maarten Lankhorst <maarten.lankho...@linux.intel.com> > To: Maxime Ripard <mrip...@kernel.org> > To: Thomas Zimmermann <tzimmerm...@suse.de> > To: David Airlie <airl...@gmail.com> > To: Simona Vetter <sim...@ffwll.ch> > To: Andrzej Hajda <andrzej.ha...@intel.com> > To: Neil Armstrong <neil.armstr...@linaro.org> > To: Robert Foss <rf...@kernel.org> > To: Laurent Pinchart <laurent.pinch...@ideasonboard.com> > To: Jonas Karlman <jo...@kwiboo.se> > To: Jernej Skrabec <jernej.skra...@gmail.com> > To: Jagan Teki <ja...@amarulasolutions.com> > To: Shawn Guo <shawn...@kernel.org> > To: Sascha Hauer <s.ha...@pengutronix.de> > To: Pengutronix Kernel Team <ker...@pengutronix.de> > To: Fabio Estevam <feste...@gmail.com> > To: Douglas Anderson <diand...@chromium.org> > To: Chun-Kuang Hu <chunkuang...@kernel.org> > To: Krzysztof Kozlowski <k...@kernel.org> > To: Dmitry Baryshkov <dmitry.barysh...@linaro.org> > Cc: Anusha Srivatsa <asriv...@redhat.com> > Cc: Paul Kocialkowski <pa...@sys-base.io> > Cc: Dmitry Baryshkov <lu...@kernel.org> > Cc: Hervé Codina <herve.cod...@bootlin.com> > Cc: Hui Pu <hui...@gehealthcare.com> > Cc: Thomas Petazzoni <thomas.petazz...@bootlin.com> > Cc: dri-devel@lists.freedesktop.org > Cc: as...@lists.linux.dev > Cc: linux-ker...@vger.kernel.org > Cc: chrome-platf...@lists.linux.dev > Cc: i...@lists.linux.dev > Cc: linux-arm-ker...@lists.infradead.org > Cc: linux-media...@lists.infradead.org > Cc: linux-amlo...@lists.infradead.org > Cc: linux-renesas-...@vger.kernel.org > Cc: platform-driver-...@vger.kernel.org > Cc: linux-samsung-...@vger.kernel.org > Cc: linux-arm-...@vger.kernel.org > Cc: freedr...@lists.freedesktop.org > Cc: linux-st...@st-md-mailman.stormreply.com > --- > 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
