Re: [PATCH] drm/ingenic: Allocate and register the internal bridge separately

[H. Nikolaus Schaller]

Hi,

> Am 05.11.2026 um 21:35 schrieb Florian Fainelli <f.fainelli@xxxxxxxxx>:
> 
> The LCD controller's internal bridge is embedded in an object allocated
> with drmm_encoder_alloc(). This initializes the encoder but leaves the
> bridge's reference count at zero. Since drm_bridge_attach() takes a
> reference, probing the CI20 display produces addition-on-zero and
> underflow warnings and fails to attach the HDMI bridge chain.
> 
> Allocate the internal bridge with devm_drm_bridge_alloc() and register
> it with devm_drm_bridge_add() before attaching it. Keep the encoder in a
> separate DRM-managed allocation: encoder cleanup can drop the final
> bridge reference before it has finished accessing the encoder. Find
> the internal bridge through the encoder's bridge chain during mode
> setting, and use the bridge argument directly in bridge callbacks.
> 
> Fixes: 94d50c1a2ca3 ("drm/bridge: get/put the bridge reference in drm_bridge_attach/detach()")
> Signed-off-by: Florian Fainelli <f.fainelli@xxxxxxxxx>
> ---
> drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 31 +++++++++++++++--------
> 1 file changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> index 625ec76cf04d..520912f9ed0d 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> @@ -121,16 +121,15 @@ struct ingenic_drm {
> };
> 
> struct ingenic_drm_bridge {
> - struct drm_encoder encoder;
> struct drm_bridge bridge, *next_bridge;
> 
> struct drm_bus_cfg bus_cfg;
> };
> 
> static inline struct ingenic_drm_bridge *
> -to_ingenic_drm_bridge(struct drm_encoder *encoder)
> +to_ingenic_drm_bridge(struct drm_bridge *bridge)
> {
> - return container_of(encoder, struct ingenic_drm_bridge, encoder);
> + return container_of(bridge, struct ingenic_drm_bridge, bridge);
> }
> 
> static inline struct ingenic_drm_private_state *
> @@ -738,7 +737,9 @@ static void ingenic_drm_encoder_atomic_mode_set(struct drm_encoder *encoder,
> {
> struct ingenic_drm *priv = drm_device_get_priv(encoder->dev);
> struct drm_display_mode *mode = &crtc_state->adjusted_mode;
> - struct ingenic_drm_bridge *bridge = to_ingenic_drm_bridge(encoder);
> + struct drm_bridge *first_bridge __free(drm_bridge_put) =
> + drm_bridge_chain_get_first_bridge(encoder);
> + struct ingenic_drm_bridge *bridge = to_ingenic_drm_bridge(first_bridge);
> unsigned int cfg, rgbcfg = 0;
> 
> priv->panel_is_sharp = bridge->bus_cfg.flags & DRM_BUS_FLAG_SHARP_SIGNALS;
> @@ -799,7 +800,7 @@ static int ingenic_drm_bridge_attach(struct drm_bridge *bridge,
>     struct drm_encoder *encoder,
>     enum drm_bridge_attach_flags flags)
> {
> - struct ingenic_drm_bridge *ib = to_ingenic_drm_bridge(encoder);
> + struct ingenic_drm_bridge *ib = to_ingenic_drm_bridge(bridge);
> 
> return drm_bridge_attach(encoder, ib->next_bridge,
> &ib->bridge, flags);
> @@ -811,7 +812,7 @@ static int ingenic_drm_bridge_atomic_check(struct drm_bridge *bridge,
>   struct drm_connector_state *conn_state)
> {
> struct drm_display_mode *mode = &crtc_state->adjusted_mode;
> - struct ingenic_drm_bridge *ib = to_ingenic_drm_bridge(bridge->encoder);
> + struct ingenic_drm_bridge *ib = to_ingenic_drm_bridge(bridge);
> 
> ib->bus_cfg = bridge_state->output_bus_cfg;
> 
> @@ -1303,22 +1304,32 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
> drm_panel_put(panel);
> }
> 
> - ib = drmm_encoder_alloc(drm, struct ingenic_drm_bridge, encoder,
> - NULL, DRM_MODE_ENCODER_DPI, NULL);
> + ib = devm_drm_bridge_alloc(dev, struct ingenic_drm_bridge, bridge,
> +   &ingenic_drm_bridge_funcs);
> if (IS_ERR(ib)) {
> ret = PTR_ERR(ib);
> + dev_err(dev, "Failed to allocate bridge: %d\n", ret);
> + goto err_drvdata;
> + }
> +
> + encoder = drmm_plain_encoder_alloc(drm, NULL,
> +   DRM_MODE_ENCODER_DPI, NULL);
> + if (IS_ERR(encoder)) {
> + ret = PTR_ERR(encoder);
> dev_err(dev, "Failed to init encoder: %d\n", ret);
> goto err_drvdata;
> }
> 
> - encoder = &ib->encoder;
> encoder->possible_crtcs = drm_crtc_mask(&priv->crtc);
> 
> drm_encoder_helper_add(encoder, &ingenic_drm_encoder_helper_funcs);
> 
> - ib->bridge.funcs = &ingenic_drm_bridge_funcs;
> ib->next_bridge = bridge;
> 
> + ret = devm_drm_bridge_add(dev, &ib->bridge);
> + if (ret)
> + goto err_drvdata;
> +
> ret = drm_bridge_attach(encoder, &ib->bridge, NULL,
> DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> if (ret) {
> -- 
> 2.53.0

Works for me on CI20 with HDMI display.

Tested-by: H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> # CI20 V2a