[PATCH v2 1/3] Revert "drm/display: bridge_connector: get/put the stored bridges"

Luca Ceresoli posted 3 patches 3 months, 3 weeks ago
[PATCH v2 1/3] Revert "drm/display: bridge_connector: get/put the stored bridges"
Posted by Luca Ceresoli 3 months, 3 weeks ago
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 <m.szyprowski@samsung.com>
Closes: https://lore.kernel.org/all/336fbfdd-c424-490e-b5d1-8ee84043dc80@samsung.com/
Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Closes: https://lore.kernel.org/r/CA+G9fYuKHp3QgPKjgFY3TfkDdh5Vf=Ae5pCW+eU41Bu=D7th2g@mail.gmail.com
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changes in v2: none
---
 drivers/gpu/drm/display/drm_bridge_connector.c | 114 ++++++++-----------------
 1 file changed, 36 insertions(+), 78 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 7b18be3ff9a32b362468351835bdab43c3f524f1..a5bdd6c1064399ece6b19560f145b877c9e0680e 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);

-- 
2.51.0
Re: [PATCH v2 1/3] Revert "drm/display: bridge_connector: get/put the stored bridges"
Posted by Louis Chauvet 3 months, 1 week ago

Le 17/10/2025 à 18:15, Luca Ceresoli a écrit :
> 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 <m.szyprowski@samsung.com>
> Closes: https://lore.kernel.org/all/336fbfdd-c424-490e-b5d1-8ee84043dc80@samsung.com/
> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> Closes: https://lore.kernel.org/r/CA+G9fYuKHp3QgPKjgFY3TfkDdh5Vf=Ae5pCW+eU41Bu=D7th2g@mail.gmail.com
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>

> ---
> 
> Changes in v2: none
> ---
>   drivers/gpu/drm/display/drm_bridge_connector.c | 114 ++++++++-----------------
>   1 file changed, 36 insertions(+), 78 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> index 7b18be3ff9a32b362468351835bdab43c3f524f1..a5bdd6c1064399ece6b19560f145b877c9e0680e 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);
> 

-- 
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 1/3] Revert "drm/display: bridge_connector: get/put the stored bridges"
Posted by Mark Brown 3 months, 1 week ago
On Fri, Oct 17, 2025 at 06:15:04PM +0200, Luca Ceresoli wrote:
> 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.

Is there any news on getting this series merged - the currently broken
code in -next is causing boot issues on several affected platforms (eg,
Rock5B) which is disrupting other testing?  If the other patches are
somehow causing issues could we perhaps get the revert in to fix the
boot issue while those issues are resolved?
Re: [PATCH v2 1/3] Revert "drm/display: bridge_connector: get/put the stored bridges"
Posted by Luca Ceresoli 3 months, 1 week ago
Hello Mark,

On Thu Oct 30, 2025 at 2:11 PM CET, Mark Brown wrote:
> On Fri, Oct 17, 2025 at 06:15:04PM +0200, Luca Ceresoli wrote:
>> 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.
>
> Is there any news on getting this series merged - the currently broken
> code in -next is causing boot issues on several affected platforms (eg,
> Rock5B) which is disrupting other testing?  If the other patches are
> somehow causing issues could we perhaps get the revert in to fix the
> boot issue while those issues are resolved?

Thanks for pinging, I must agree the regression is out since quite a few
weeks. Despite having four Tested-by, this series was lacking Reviewed- and
Tested-by entirely.

Now Louis reviewed the whole series (thanks!), so my understanding of the
drm-misc policy is that I can apply the series, which I plan to do in a few
days to let anybody else comment.

Best regards,
Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com