diff mbox series

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

Message ID 20250407-drm-bridge-convert-to-alloc-api-v1-34-42113ff8d9c0@bootlin.com (mailing list archive)
State New
Headers show
Series drm: convert all bridges to devm_drm_bridge_alloc() | expand

Commit Message

Luca Ceresoli April 7, 2025, 3:27 p.m. UTC 
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@bootlin.com>
---

To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: Maxime Ripard <mripard@kernel.org>
To: Thomas Zimmermann <tzimmermann@suse.de>
To: David Airlie <airlied@gmail.com>
To: Simona Vetter <simona@ffwll.ch>
To: Andrzej Hajda <andrzej.hajda@intel.com>
To: Neil Armstrong <neil.armstrong@linaro.org>
To: Robert Foss <rfoss@kernel.org>
To: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
To: Jonas Karlman <jonas@kwiboo.se>
To: Jernej Skrabec <jernej.skrabec@gmail.com>
To: Jagan Teki <jagan@amarulasolutions.com>
To: Shawn Guo <shawnguo@kernel.org>
To: Sascha Hauer <s.hauer@pengutronix.de>
To: Pengutronix Kernel Team <kernel@pengutronix.de>
To: Fabio Estevam <festevam@gmail.com>
To: Douglas Anderson <dianders@chromium.org>
To: Chun-Kuang Hu <chunkuang.hu@kernel.org>
To: Krzysztof Kozlowski <krzk@kernel.org>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Anusha Srivatsa <asrivats@redhat.com>
Cc: Paul Kocialkowski <paulk@sys-base.io>
Cc: Dmitry Baryshkov <lumag@kernel.org>
Cc: Hervé Codina <herve.codina@bootlin.com>
Cc: Hui Pu <Hui.Pu@gehealthcare.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: dri-devel@lists.freedesktop.org
Cc: asahi@lists.linux.dev
Cc: linux-kernel@vger.kernel.org
Cc: chrome-platform@lists.linux.dev
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-mediatek@lists.infradead.org
Cc: linux-amlogic@lists.infradead.org
Cc: linux-renesas-soc@vger.kernel.org
Cc: platform-driver-x86@vger.kernel.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Cc: linux-stm32@st-md-mailman.stormreply.com
---
 drivers/gpu/drm/bridge/panel.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Comments

Maxime Ripard April 8, 2025, 3:51 p.m. UTC | #1 
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@bootlin.com>
> ---
> 
> To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> To: Maxime Ripard <mripard@kernel.org>
> To: Thomas Zimmermann <tzimmermann@suse.de>
> To: David Airlie <airlied@gmail.com>
> To: Simona Vetter <simona@ffwll.ch>
> To: Andrzej Hajda <andrzej.hajda@intel.com>
> To: Neil Armstrong <neil.armstrong@linaro.org>
> To: Robert Foss <rfoss@kernel.org>
> To: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> To: Jonas Karlman <jonas@kwiboo.se>
> To: Jernej Skrabec <jernej.skrabec@gmail.com>
> To: Jagan Teki <jagan@amarulasolutions.com>
> To: Shawn Guo <shawnguo@kernel.org>
> To: Sascha Hauer <s.hauer@pengutronix.de>
> To: Pengutronix Kernel Team <kernel@pengutronix.de>
> To: Fabio Estevam <festevam@gmail.com>
> To: Douglas Anderson <dianders@chromium.org>
> To: Chun-Kuang Hu <chunkuang.hu@kernel.org>
> To: Krzysztof Kozlowski <krzk@kernel.org>
> To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Cc: Anusha Srivatsa <asrivats@redhat.com>
> Cc: Paul Kocialkowski <paulk@sys-base.io>
> Cc: Dmitry Baryshkov <lumag@kernel.org>
> Cc: Hervé Codina <herve.codina@bootlin.com>
> Cc: Hui Pu <Hui.Pu@gehealthcare.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: asahi@lists.linux.dev
> Cc: linux-kernel@vger.kernel.org
> Cc: chrome-platform@lists.linux.dev
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-mediatek@lists.infradead.org
> Cc: linux-amlogic@lists.infradead.org
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: platform-driver-x86@vger.kernel.org
> Cc: linux-samsung-soc@vger.kernel.org
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> Cc: linux-stm32@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
diff mbox series

Patch

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);