[v3] drm/ingenic: fix bridge allocation

[PATCH v3] drm/ingenic: fix bridge allocation

Posted by H. Nikolaus Schaller 1 month ago

Bridge allocation API has changed and ingenic/drm was broken
leading to

[   54.997593] dw-hdmi-ingenic 10180000.hdmi: Detected HDMI \X controller v1.31a with HDCP (DWC HDMI 3D TX PHY)
[   55.491338] dw-hdmi-ingenic 10180000.hdmi: registered DesignWare HDMI I2C bus driver
[   55.899132] [drm] DRM bridge corrupted or not allocated by devm_drm_bridge_alloc()
[   55.904136] ------------[ cut here ]------------
[   55.908753] WARNING: lib/refcount.c:25 at drm_bridge_get+0x58/0x6c [drm], CPU#0: kworker/u4:2/36
[   55.917538] refcount_t: addition on 0; use-after-free.
...
[   56.354928] [<c04898b8>] drm_bridge_attach+0x80/0x208 [drm]
...

Fixes: 9347f2fbb0183b0 ("drm/bridge: add warning for bridges using neither devm_drm_bridge_alloc() nor drm_bridge_add()")
Tested-by: Waldemar Brodkorb <wbx@openadk.org> (on CI20 with HDMI)
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
Cc: Waldemar Brodkorb <wbx@openadk.org>
Cc: stable@vger.kernel.org
---

Notes:
    v3: fixed a malformed diff
    
    v2: removed ib->bridge->ops = DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_DETECT as suggested by Sashiko-reviews
        https://sashiko.dev/#/patchset/400ba2fe0d4f76484e929d2efaa32f67a940163a.1787477392.git.hns@goldelico.com?part=1

 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index 42c86f195c66b3..8d7979a7859332 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -122,7 +122,7 @@ struct ingenic_drm {
 
 struct ingenic_drm_bridge {
 	struct drm_encoder encoder;
-	struct drm_bridge bridge, *next_bridge;
+	struct drm_bridge *bridge, *next_bridge;
 
 	struct drm_bus_cfg bus_cfg;
 };
@@ -802,7 +802,7 @@ static int ingenic_drm_bridge_attach(struct drm_bridge *bridge,
 	struct ingenic_drm_bridge *ib = to_ingenic_drm_bridge(encoder);
 
 	return drm_bridge_attach(encoder, ib->next_bridge,
-				 &ib->bridge, flags);
+				 bridge, flags);
 }
 
 static int ingenic_drm_bridge_atomic_check(struct drm_bridge *bridge,
@@ -1314,10 +1314,25 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 
 		drm_encoder_helper_add(encoder, &ingenic_drm_encoder_helper_funcs);
 
-		ib->bridge.funcs = &ingenic_drm_bridge_funcs;
+		ib->bridge = devm_drm_bridge_alloc(priv->dev, struct drm_bridge,
+						   base, &ingenic_drm_bridge_funcs);
+		if (IS_ERR(ib->bridge)) {
+			ret = PTR_ERR(ib->bridge);
+			goto err_drvdata;
+		}
+
+		ib->bridge->of_node = priv->dev->of_node;
 		ib->next_bridge = bridge;
 
-		ret = drm_bridge_attach(encoder, &ib->bridge, NULL,
+		ib->bridge->interlace_allowed = true;
+
+		ret = devm_drm_bridge_add(priv->dev, ib->bridge);
+		if (ret) {
+			dev_err(dev, "Failed to register DRM bridge: %d\n", ret);
+			goto err_drvdata;
+		}
+
+		ret = drm_bridge_attach(encoder, ib->bridge, NULL,
 					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
 		if (ret) {
 			dev_err(dev, "Unable to attach bridge\n");
-- 
2.55.0

Re: [PATCH v3] drm/ingenic: fix bridge allocation

Posted by Paul Cercueil 1 month ago

Hi Nikolaus,

Le dimanche 23 août 2026 à 16:12 +0200, H. Nikolaus Schaller a écrit :
> Bridge allocation API has changed and ingenic/drm was broken
> leading to
> 
> [   54.997593] dw-hdmi-ingenic 10180000.hdmi: Detected HDMI \X
> controller v1.31a with HDCP (DWC HDMI 3D TX PHY)
> [   55.491338] dw-hdmi-ingenic 10180000.hdmi: registered DesignWare
> HDMI I2C bus driver
> [   55.899132] [drm] DRM bridge corrupted or not allocated by
> devm_drm_bridge_alloc()
> [   55.904136] ------------[ cut here ]------------
> [   55.908753] WARNING: lib/refcount.c:25 at drm_bridge_get+0x58/0x6c
> [drm], CPU#0: kworker/u4:2/36
> [   55.917538] refcount_t: addition on 0; use-after-free.
> ...
> [   56.354928] [<c04898b8>] drm_bridge_attach+0x80/0x208 [drm]
> ...
> 
> Fixes: 9347f2fbb0183b0 ("drm/bridge: add warning for bridges using
> neither devm_drm_bridge_alloc() nor drm_bridge_add()")
> Tested-by: Waldemar Brodkorb <wbx@openadk.org> (on CI20 with HDMI)
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> Cc: Waldemar Brodkorb <wbx@openadk.org>
> Cc: stable@vger.kernel.org
> ---
> 
> Notes:
>     v3: fixed a malformed diff
>     
>     v2: removed ib->bridge->ops = DRM_BRIDGE_OP_EDID |
> DRM_BRIDGE_OP_DETECT as suggested by Sashiko-reviews
>        
> https://sashiko.dev/#/patchset/400ba2fe0d4f76484e929d2efaa32f67a940163a.1787477392.git.hns@goldelico.com?part=1
> 
>  drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 23 +++++++++++++++++++--
> --
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> index 42c86f195c66b3..8d7979a7859332 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> @@ -122,7 +122,7 @@ struct ingenic_drm {
>  
>  struct ingenic_drm_bridge {
>  	struct drm_encoder encoder;
> -	struct drm_bridge bridge, *next_bridge;
> +	struct drm_bridge *bridge, *next_bridge;
>  
>  	struct drm_bus_cfg bus_cfg;
>  };
> @@ -802,7 +802,7 @@ static int ingenic_drm_bridge_attach(struct
> drm_bridge *bridge,
>  	struct ingenic_drm_bridge *ib =
> to_ingenic_drm_bridge(encoder);
>  
>  	return drm_bridge_attach(encoder, ib->next_bridge,
> -				 &ib->bridge, flags);
> +				 bridge, flags);
>  }
>  
>  static int ingenic_drm_bridge_atomic_check(struct drm_bridge
> *bridge,
> @@ -1314,10 +1314,25 @@ static int ingenic_drm_bind(struct device
> *dev, bool has_components)
>  
>  		drm_encoder_helper_add(encoder,
> &ingenic_drm_encoder_helper_funcs);
>  
> -		ib->bridge.funcs = &ingenic_drm_bridge_funcs;
> +		ib->bridge = devm_drm_bridge_alloc(priv->dev, struct
> drm_bridge,
> +						   base,
> &ingenic_drm_bridge_funcs);
> +		if (IS_ERR(ib->bridge)) {
> +			ret = PTR_ERR(ib->bridge);
> +			goto err_drvdata;
> +		}
> +
> +		ib->bridge->of_node = priv->dev->of_node;
>  		ib->next_bridge = bridge;
>  
> -		ret = drm_bridge_attach(encoder, &ib->bridge, NULL,
> +		ib->bridge->interlace_allowed = true;

That one line feels like it doesn't belong here, but in its own patch.

Cheers,
-Paul

> +
> +		ret = devm_drm_bridge_add(priv->dev, ib->bridge);
> +		if (ret) {
> +			dev_err(dev, "Failed to register DRM bridge:
> %d\n", ret);
> +			goto err_drvdata;
> +		}
> +
> +		ret = drm_bridge_attach(encoder, ib->bridge, NULL,
>  					DRM_BRIDGE_ATTACH_NO_CONNECT
> OR);
>  		if (ret) {
>  			dev_err(dev, "Unable to attach bridge\n");

Re: [PATCH v3] drm/ingenic: fix bridge allocation

Posted by H. Nikolaus Schaller 1 month ago

Hi Paul,

> Am 24.08.2026 um 11:56 schrieb Paul Cercueil <paul@crapouillou.net>:
> 
> Hi Nikolaus,
> 
> Le dimanche 23 août 2026 à 16:12 +0200, H. Nikolaus Schaller a écrit :
>> Bridge allocation API has changed and ingenic/drm was broken
>> leading to
>> 
>> [   54.997593] dw-hdmi-ingenic 10180000.hdmi: Detected HDMI \X
>> controller v1.31a with HDCP (DWC HDMI 3D TX PHY)
>> [   55.491338] dw-hdmi-ingenic 10180000.hdmi: registered DesignWare
>> HDMI I2C bus driver
>> [   55.899132] [drm] DRM bridge corrupted or not allocated by
>> devm_drm_bridge_alloc()
>> [   55.904136] ------------[ cut here ]------------
>> [   55.908753] WARNING: lib/refcount.c:25 at drm_bridge_get+0x58/0x6c
>> [drm], CPU#0: kworker/u4:2/36
>> [   55.917538] refcount_t: addition on 0; use-after-free.
>> ...
>> [   56.354928] [<c04898b8>] drm_bridge_attach+0x80/0x208 [drm]
>> ...
>> 
>> Fixes: 9347f2fbb0183b0 ("drm/bridge: add warning for bridges using
>> neither devm_drm_bridge_alloc() nor drm_bridge_add()")
>> Tested-by: Waldemar Brodkorb <wbx@openadk.org> (on CI20 with HDMI)
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> Cc: Waldemar Brodkorb <wbx@openadk.org>
>> Cc: stable@vger.kernel.org
>> ---
>> 
>> Notes:
>>     v3: fixed a malformed diff
>>     
>>     v2: removed ib->bridge->ops = DRM_BRIDGE_OP_EDID |
>> DRM_BRIDGE_OP_DETECT as suggested by Sashiko-reviews
>>        
>> https://sashiko.dev/#/patchset/400ba2fe0d4f76484e929d2efaa32f67a940163a.1787477392.git.hns@goldelico.com?part=1
>> 
>>  drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 23 +++++++++++++++++++--
>> --
>>  1 file changed, 19 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>> index 42c86f195c66b3..8d7979a7859332 100644
>> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>> @@ -122,7 +122,7 @@ struct ingenic_drm {
>>  
>>  
>> - ret = drm_bridge_attach(encoder, &ib->bridge, NULL,
>> + ib->bridge->interlace_allowed = true;
> 
> That one line feels like it doesn't belong here, but in its own patch.

You are right, it is not a fix but adds a feature.

I'll send a v4 asap.

> 
> Cheers,
> -Paul

BR,
Nikolaus