From: Luca Ceresoli Date: Fri, 17 Oct 2025 16:15:04 +0000 (+0200) Subject: Revert "drm/display: bridge_connector: get/put the stored bridges" X-Git-Tag: v6.19-rc1~157^2~16^2~38 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b4027536933f813e51cc53be0b7542012f09aa38;p=thirdparty%2Fkernel%2Flinux.git Revert "drm/display: bridge_connector: get/put the stored bridges" This reverts commit 2be300f9a0b6f6b0ae2a90be97e558ec0535be54. The commit being reverted moved all the bridge_connector->bridge_* assignments to just before the final successful return in order to handle the bridge refcounting in a clean way. This introduced a bug, because a bit before the successful return drmm_connector_hdmi_cec_register() is called, which calls funcs->init() which is drm_bridge_connector_hdmi_cec_init() which needs bridge_connector->bridge_hdmi_cec to be set. The reported bug may be fixed in a relatively simple way, but other similar patterns are potentially present, so just revert the offending commit. A different approach will be implemented. Fixes: 2be300f9a0b6 ("drm/display: bridge_connector: get/put the stored bridges") Reported-by: Marek Szyprowski Closes: https://lore.kernel.org/all/336fbfdd-c424-490e-b5d1-8ee84043dc80@samsung.com/ Reported-by: Naresh Kamboju Closes: https://lore.kernel.org/r/CA+G9fYuKHp3QgPKjgFY3TfkDdh5Vf=Ae5pCW+eU41Bu=D7th2g@mail.gmail.com Reviewed-by: Louis Chauvet Reviewed-by: Dmitry Baryshkov Tested-by: Dmitry Baryshkov # db410c Tested-by: Geert Uytterhoeven Tested-by: Nicolas Frattaroli Tested-by: Tommaso Merciai Tested-by: Marek Szyprowski Link: https://patch.msgid.link/20251017-drm-bridge-alloc-getput-bridge-connector-fix-hdmi_cec-v2-1-667abf6d47c0@bootlin.com Signed-off-by: Luca Ceresoli --- diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c index 7b18be3ff9a32..a5bdd6c106439 100644 --- a/drivers/gpu/drm/display/drm_bridge_connector.c +++ b/drivers/gpu/drm/display/drm_bridge_connector.c @@ -618,20 +618,6 @@ static const struct drm_connector_hdmi_cec_funcs drm_bridge_connector_hdmi_cec_f * Bridge Connector Initialisation */ -static void drm_bridge_connector_put_bridges(struct drm_device *dev, void *data) -{ - struct drm_bridge_connector *bridge_connector = (struct drm_bridge_connector *)data; - - drm_bridge_put(bridge_connector->bridge_edid); - drm_bridge_put(bridge_connector->bridge_hpd); - drm_bridge_put(bridge_connector->bridge_detect); - drm_bridge_put(bridge_connector->bridge_modes); - drm_bridge_put(bridge_connector->bridge_hdmi); - drm_bridge_put(bridge_connector->bridge_hdmi_audio); - drm_bridge_put(bridge_connector->bridge_dp_audio); - drm_bridge_put(bridge_connector->bridge_hdmi_cec); -} - /** * drm_bridge_connector_init - Initialise a connector for a chain of bridges * @drm: the DRM device @@ -652,15 +638,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, struct drm_bridge_connector *bridge_connector; struct drm_connector *connector; struct i2c_adapter *ddc = NULL; - struct drm_bridge *panel_bridge __free(drm_bridge_put) = NULL; - struct drm_bridge *bridge_edid __free(drm_bridge_put) = NULL; - struct drm_bridge *bridge_hpd __free(drm_bridge_put) = NULL; - struct drm_bridge *bridge_detect __free(drm_bridge_put) = NULL; - struct drm_bridge *bridge_modes __free(drm_bridge_put) = NULL; - struct drm_bridge *bridge_hdmi __free(drm_bridge_put) = NULL; - struct drm_bridge *bridge_hdmi_audio __free(drm_bridge_put) = NULL; - struct drm_bridge *bridge_dp_audio __free(drm_bridge_put) = NULL; - struct drm_bridge *bridge_hdmi_cec __free(drm_bridge_put) = NULL; + struct drm_bridge *panel_bridge = NULL; unsigned int supported_formats = BIT(HDMI_COLORSPACE_RGB); unsigned int max_bpc = 8; bool support_hdcp = false; @@ -671,10 +649,6 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, if (!bridge_connector) return ERR_PTR(-ENOMEM); - ret = drmm_add_action(drm, drm_bridge_connector_put_bridges, bridge_connector); - if (ret) - return ERR_PTR(ret); - bridge_connector->encoder = encoder; /* @@ -698,30 +672,22 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, if (!bridge->ycbcr_420_allowed) connector->ycbcr_420_allowed = false; - if (bridge->ops & DRM_BRIDGE_OP_EDID) { - drm_bridge_put(bridge_edid); - bridge_edid = drm_bridge_get(bridge); - } - if (bridge->ops & DRM_BRIDGE_OP_HPD) { - drm_bridge_put(bridge_hpd); - bridge_hpd = drm_bridge_get(bridge); - } - if (bridge->ops & DRM_BRIDGE_OP_DETECT) { - drm_bridge_put(bridge_detect); - bridge_detect = drm_bridge_get(bridge); - } - if (bridge->ops & DRM_BRIDGE_OP_MODES) { - drm_bridge_put(bridge_modes); - bridge_modes = drm_bridge_get(bridge); - } + if (bridge->ops & DRM_BRIDGE_OP_EDID) + bridge_connector->bridge_edid = bridge; + if (bridge->ops & DRM_BRIDGE_OP_HPD) + bridge_connector->bridge_hpd = bridge; + if (bridge->ops & DRM_BRIDGE_OP_DETECT) + bridge_connector->bridge_detect = bridge; + if (bridge->ops & DRM_BRIDGE_OP_MODES) + bridge_connector->bridge_modes = bridge; if (bridge->ops & DRM_BRIDGE_OP_HDMI) { - if (bridge_hdmi) + if (bridge_connector->bridge_hdmi) return ERR_PTR(-EBUSY); if (!bridge->funcs->hdmi_write_infoframe || !bridge->funcs->hdmi_clear_infoframe) return ERR_PTR(-EINVAL); - bridge_hdmi = drm_bridge_get(bridge); + bridge_connector->bridge_hdmi = bridge; if (bridge->supported_formats) supported_formats = bridge->supported_formats; @@ -730,10 +696,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, } if (bridge->ops & DRM_BRIDGE_OP_HDMI_AUDIO) { - if (bridge_hdmi_audio) + if (bridge_connector->bridge_hdmi_audio) return ERR_PTR(-EBUSY); - if (bridge_dp_audio) + if (bridge_connector->bridge_dp_audio) return ERR_PTR(-EBUSY); if (!bridge->hdmi_audio_max_i2s_playback_channels && @@ -744,14 +710,14 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, !bridge->funcs->hdmi_audio_shutdown) return ERR_PTR(-EINVAL); - bridge_hdmi_audio = drm_bridge_get(bridge); + bridge_connector->bridge_hdmi_audio = bridge; } if (bridge->ops & DRM_BRIDGE_OP_DP_AUDIO) { - if (bridge_dp_audio) + if (bridge_connector->bridge_dp_audio) return ERR_PTR(-EBUSY); - if (bridge_hdmi_audio) + if (bridge_connector->bridge_hdmi_audio) return ERR_PTR(-EBUSY); if (!bridge->hdmi_audio_max_i2s_playback_channels && @@ -762,7 +728,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, !bridge->funcs->dp_audio_shutdown) return ERR_PTR(-EINVAL); - bridge_dp_audio = drm_bridge_get(bridge); + bridge_connector->bridge_dp_audio = bridge; } if (bridge->ops & DRM_BRIDGE_OP_HDMI_CEC_NOTIFIER) { @@ -773,10 +739,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, } if (bridge->ops & DRM_BRIDGE_OP_HDMI_CEC_ADAPTER) { - if (bridge_hdmi_cec) + if (bridge_connector->bridge_hdmi_cec) return ERR_PTR(-EBUSY); - bridge_hdmi_cec = drm_bridge_get(bridge); + bridge_connector->bridge_hdmi_cec = bridge; if (!bridge->funcs->hdmi_cec_enable || !bridge->funcs->hdmi_cec_log_addr || @@ -796,7 +762,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, ddc = bridge->ddc; if (drm_bridge_is_panel(bridge)) - panel_bridge = drm_bridge_get(bridge); + panel_bridge = bridge; if (bridge->support_hdcp) support_hdcp = true; @@ -805,13 +771,13 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, if (connector_type == DRM_MODE_CONNECTOR_Unknown) return ERR_PTR(-EINVAL); - if (bridge_hdmi) { + if (bridge_connector->bridge_hdmi) { if (!connector->ycbcr_420_allowed) supported_formats &= ~BIT(HDMI_COLORSPACE_YUV420); ret = drmm_connector_hdmi_init(drm, connector, - bridge_hdmi->vendor, - bridge_hdmi->product, + bridge_connector->bridge_hdmi->vendor, + bridge_connector->bridge_hdmi->product, &drm_bridge_connector_funcs, &drm_bridge_connector_hdmi_funcs, connector_type, ddc, @@ -827,14 +793,15 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, return ERR_PTR(ret); } - if (bridge_hdmi_audio || bridge_dp_audio) { + if (bridge_connector->bridge_hdmi_audio || + bridge_connector->bridge_dp_audio) { struct device *dev; struct drm_bridge *bridge; - if (bridge_hdmi_audio) - bridge = bridge_hdmi_audio; + if (bridge_connector->bridge_hdmi_audio) + bridge = bridge_connector->bridge_hdmi_audio; else - bridge = bridge_dp_audio; + bridge = bridge_connector->bridge_dp_audio; dev = bridge->hdmi_audio_dev; @@ -848,9 +815,9 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, return ERR_PTR(ret); } - if (bridge_hdmi_cec && - bridge_hdmi_cec->ops & DRM_BRIDGE_OP_HDMI_CEC_NOTIFIER) { - struct drm_bridge *bridge = bridge_hdmi_cec; + if (bridge_connector->bridge_hdmi_cec && + bridge_connector->bridge_hdmi_cec->ops & DRM_BRIDGE_OP_HDMI_CEC_NOTIFIER) { + struct drm_bridge *bridge = bridge_connector->bridge_hdmi_cec; ret = drmm_connector_hdmi_cec_notifier_register(connector, NULL, @@ -859,9 +826,9 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, return ERR_PTR(ret); } - if (bridge_hdmi_cec && - bridge_hdmi_cec->ops & DRM_BRIDGE_OP_HDMI_CEC_ADAPTER) { - struct drm_bridge *bridge = bridge_hdmi_cec; + if (bridge_connector->bridge_hdmi_cec && + bridge_connector->bridge_hdmi_cec->ops & DRM_BRIDGE_OP_HDMI_CEC_ADAPTER) { + struct drm_bridge *bridge = bridge_connector->bridge_hdmi_cec; ret = drmm_connector_hdmi_cec_register(connector, &drm_bridge_connector_hdmi_cec_funcs, @@ -874,9 +841,9 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs); - if (bridge_hpd) + if (bridge_connector->bridge_hpd) connector->polled = DRM_CONNECTOR_POLL_HPD; - else if (bridge_detect) + else if (bridge_connector->bridge_detect) connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT; @@ -887,15 +854,6 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, IS_ENABLED(CONFIG_DRM_DISPLAY_HDCP_HELPER)) drm_connector_attach_content_protection_property(connector, true); - bridge_connector->bridge_edid = drm_bridge_get(bridge_edid); - bridge_connector->bridge_hpd = drm_bridge_get(bridge_hpd); - bridge_connector->bridge_detect = drm_bridge_get(bridge_detect); - bridge_connector->bridge_modes = drm_bridge_get(bridge_modes); - bridge_connector->bridge_hdmi = drm_bridge_get(bridge_hdmi); - bridge_connector->bridge_hdmi_audio = drm_bridge_get(bridge_hdmi_audio); - bridge_connector->bridge_dp_audio = drm_bridge_get(bridge_dp_audio); - bridge_connector->bridge_hdmi_cec = drm_bridge_get(bridge_hdmi_cec); - return connector; } EXPORT_SYMBOL_GPL(drm_bridge_connector_init);