Hello Jonas,

On 2026-05-19 17:18 +0200, Jonas Karlman wrote:
> Hello Luca,
>
> On 5/19/2026 2:06 PM, Luca Ceresoli wrote:
> > On Mon, 18 May 2026 18:01:40 +0000, Jonas Karlman <[email protected]> wrote:
> >
> > Hello Jonas,
> >
> >>
> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
> >> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >> index b7bfc0e9a6b2..9d795c550f8a 100644
> >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >> @@ -2600,10 +2609,14 @@ static int dw_hdmi_connector_create(struct dw_hdmi 
> >> *hdmi)
> >>
> >>    drm_connector_helper_add(connector, &dw_hdmi_connector_helper_funcs);
> >>
> >> -  drm_connector_init_with_ddc(hdmi->bridge.dev, connector,
> >> -                              &dw_hdmi_connector_funcs,
> >> -                              DRM_MODE_CONNECTOR_HDMIA,
> >> -                              hdmi->ddc);
> >> +  ret = drm_connector_init_with_ddc(hdmi->bridge.dev, connector,
> >> +                                    &dw_hdmi_connector_funcs,
> >> +                                    DRM_MODE_CONNECTOR_HDMIA,
> >> +                                    hdmi->ddc);
> >> +  if (ret)
> >> +          return ret;
> >> +
> >> +  drm_bridge_get(&hdmi->bridge);
> >
> > I'm not fully following the code paths, but both the report and the fix
> > make sense to me. Only I'd move the drm_bridge_get() before
> > drm_connector_init_with_ddc(), to avoid a short window where no reference
> > is held and the bridge might be destroyed before drm_bridge_get() is
> > called. I'm not sure this can happen, but it's better to write the code in
> > a way that clearly makes it impossible.
>
> dw_hdmi_connector_create() is only called from dw_hdmi_bridge_attach()
> so the bridge should already have a ref for the lifetime of this call.

Ah, that's true. So the patch is correct.

Reviewed-by: Luca Ceresoli <[email protected]>

In case you send a new iteration, please add this extra explanation to the
commit message, similar to the above paragraph.

> I explicitly chose the placement after drm_connector_init_with_ddc()
> to ensure ref count is correctly balanced without having to add a
> drm_bridge_put() call in any error path. I.e. connector destroy() is
> only called when drm_connector_init_with_ddc() succeeds.
>
> This code/call is also planned to be removed in a future series,

In order to remove the !DRM_BRIDGE_ATTACH_NO_CONNECTOR case? That would be
welcome!

Luca

Reply via email to