Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] drm/bridge: dw-hdmi: missing error check for drm_connector_init_with_ddc - [Medium] drm/bridge: dw-hdmi: max_bpc state is lost during drm_mode_config_reset() --
commit 00a6167b722bff3c39b80a7bcee2827035fd652c Author: Maxime Ripard <[email protected]> Subject: drm/bridge: synopsys: dw-hdmi: Convert to atomic_create_state The dw-hdmi driver allocates an initial connector state during creation. This commit converts the allocation from drm_atomic_helper_connector_reset() to drm_atomic_helper_connector_create_state() and adds proper error handling. > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index 917cf764c9c5d..5f11707c67b5c 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -2605,7 +2605,15 @@ static int dw_hdmi_connector_create(struct dw_hdmi > *hdmi) > * drm_connector_attach_max_bpc_property() requires the > * connector to have a state. > */ > - drm_atomic_helper_connector_reset(connector); > + if (!connector->state) { > + struct drm_connector_state *conn_state; > + > + conn_state = > drm_atomic_helper_connector_create_state(connector); > + if (IS_ERR(conn_state)) > + return PTR_ERR(conn_state); > + > + connector->state = conn_state; > + } [Severity: High] Does this code handle failures from drm_connector_init_with_ddc() correctly? Looking at dw_hdmi_connector_create(), drm_connector_init_with_ddc() is called just before this block, but its return value is ignored: drm_connector_init_with_ddc(hdmi->bridge.dev, connector, &dw_hdmi_connector_funcs, DRM_MODE_CONNECTOR_HDMIA, hdmi->ddc); If this initialization fails, connector->base.properties remains uninitialized (NULL). When execution continues to drm_connector_attach_max_bpc_property(), it will attempt to attach the property and dereference connector->base.properties, resulting in a NULL pointer dereference. > > drm_connector_attach_max_bpc_property(connector, 8, 16); [Severity: Medium] Will this max_bpc initialization be lost during a drm_mode_config_reset()? The driver assigns .atomic_create_state = drm_atomic_helper_connector_create_state in dw_hdmi_connector_funcs without a custom hook to restore max_bpc. If a host DRM driver calls drm_mode_config_reset(), the core fallback in drm_mode_config_connector_reset_with_create_state() will destroy this initially allocated state and replace it with a newly allocated one: if (connector->state) { connector->funcs->atomic_destroy_state(connector, connector->state); connector->state = NULL; } return drm_mode_config_connector_create_state(connector); This wipes out the max_bpc property attached here, resetting it to 0. This could cause the driver to incorrectly fall back to 8-bit color depth later in dw_hdmi_bridge_mode_set(). > -- Sashiko AI review · https://sashiko.dev/#/patchset/20261014-drm-no-more-connector-reset-v1-0-586c9c026...@kernel.org?part=57
