> Am 06.11.2026 um 13:21 schrieb Paul Cercueil <[email protected]>:
>
> Nikolaus:
>
> Le vendredi 06 novembre 2026 à 11:59 +0100, H. Nikolaus Schaller a écrit :
>> Hi,
>>
>>> Am 05.11.2026 um 21:35 schrieb Florian Fainelli <[email protected]>:
>>>
>>> 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 <[email protected]>
>>> ---
>>> 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 <[email protected]> # CI20 V2a
>
> This patches basically overrides your "[PATCH v6] drm/ingenic: fix bridge
> allocation", correct?
Indeed.
Well, I am not sure which is the right one. Both work for me. So you can choose
(e.g. if struct ingenic_drm_bridge should embed an struct drm_encoder encoder
or not.
Best regards,
Nikolaus
