[PATCH 34/34] drm/bridge: panel: convert to devm_drm_bridge_alloc() API

[Maxime Ripard]

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.ceresoli at bootlin.com>
> ---
> 
> To: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> To: Maxime Ripard <mripard at kernel.org>
> To: Thomas Zimmermann <tzimmermann at suse.de>
> To: David Airlie <airlied at gmail.com>
> To: Simona Vetter <simona at ffwll.ch>
> To: Andrzej Hajda <andrzej.hajda at intel.com>
> To: Neil Armstrong <neil.armstrong at linaro.org>
> To: Robert Foss <rfoss at kernel.org>
> To: Laurent Pinchart <Laurent.pinchart at ideasonboard.com>
> To: Jonas Karlman <jonas at kwiboo.se>
> To: Jernej Skrabec <jernej.skrabec at gmail.com>
> To: Jagan Teki <jagan at amarulasolutions.com>
> To: Shawn Guo <shawnguo at kernel.org>
> To: Sascha Hauer <s.hauer at pengutronix.de>
> To: Pengutronix Kernel Team <kernel at pengutronix.de>
> To: Fabio Estevam <festevam at gmail.com>
> To: Douglas Anderson <dianders at chromium.org>
> To: Chun-Kuang Hu <chunkuang.hu at kernel.org>
> To: Krzysztof Kozlowski <krzk at kernel.org>
> To: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> Cc: Anusha Srivatsa <asrivats at redhat.com>
> Cc: Paul Kocialkowski <paulk at sys-base.io>
> Cc: Dmitry Baryshkov <lumag at kernel.org>
> Cc: Hervé Codina <herve.codina at bootlin.com>
> Cc: Hui Pu <Hui.Pu at gehealthcare.com>
> Cc: Thomas Petazzoni <thomas.petazzoni at bootlin.com>
> Cc: dri-devel at lists.freedesktop.org
> Cc: asahi at lists.linux.dev
> Cc: linux-kernel at vger.kernel.org
> Cc: chrome-platform at lists.linux.dev
> Cc: imx at lists.linux.dev
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-mediatek at lists.infradead.org
> Cc: linux-amlogic at lists.infradead.org
> Cc: linux-renesas-soc at vger.kernel.org
> Cc: platform-driver-x86 at vger.kernel.org
> Cc: linux-samsung-soc at vger.kernel.org
> Cc: linux-arm-msm at vger.kernel.org
> Cc: freedreno at lists.freedesktop.org
> Cc: linux-stm32 at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/freedreno/attachments/20250408/2405be8f/attachment.sig>