From: Jayesh Choudhary <j-choudhary@ti.com>

Now that we have DBANC framework, remove the connector initialisation code
as that piece of code is not called if DRM_BRIDGE_ATTACH_NO_CONNECTOR flag
is used. Only TI K3 platforms consume this driver and tidss (their display
controller) has this flag set. So this legacy support can be dropped.

Fixes: c932ced6b585 ("drm/tidss: Update encoder/bridge chain connect model")
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
---
 .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 187 +-----------------
 1 file changed, 10 insertions(+), 177 deletions(-)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index a614d1384f71..08702ade2903 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -739,12 +739,8 @@ static void cdns_mhdp_fw_cb(const struct firmware *fw, void *context)
 	spin_lock(&mhdp->start_lock);
 	bridge_attached = mhdp->bridge_attached;
 	spin_unlock(&mhdp->start_lock);
-	if (bridge_attached) {
-		if (mhdp->connector.dev)
-			drm_kms_helper_hotplug_event(mhdp->bridge.dev);
-		else
-			drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
-	}
+	if (bridge_attached)
+		drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
 }
 
 static int cdns_mhdp_load_firmware(struct cdns_mhdp_device *mhdp)
@@ -1444,56 +1440,6 @@ static const struct drm_edid *cdns_mhdp_edid_read(struct cdns_mhdp_device *mhdp,
 	return drm_edid_read_custom(connector, cdns_mhdp_get_edid_block, mhdp);
 }
 
-static int cdns_mhdp_get_modes(struct drm_connector *connector)
-{
-	struct cdns_mhdp_device *mhdp = connector_to_mhdp(connector);
-	const struct drm_edid *drm_edid;
-	int num_modes;
-
-	if (!mhdp->plugged)
-		return 0;
-
-	drm_edid = cdns_mhdp_edid_read(mhdp, connector);
-
-	drm_edid_connector_update(connector, drm_edid);
-
-	if (!drm_edid) {
-		dev_err(mhdp->dev, "Failed to read EDID\n");
-		return 0;
-	}
-
-	num_modes = drm_edid_connector_add_modes(connector);
-	drm_edid_free(drm_edid);
-
-	/*
-	 * HACK: Warn about unsupported display formats until we deal
-	 *       with them correctly.
-	 */
-	if (connector->display_info.color_formats &&
-	    !(connector->display_info.color_formats &
-	      mhdp->display_fmt.color_format))
-		dev_warn(mhdp->dev,
-			 "%s: No supported color_format found (0x%08x)\n",
-			__func__, connector->display_info.color_formats);
-
-	if (connector->display_info.bpc &&
-	    connector->display_info.bpc < mhdp->display_fmt.bpc)
-		dev_warn(mhdp->dev, "%s: Display bpc only %d < %d\n",
-			 __func__, connector->display_info.bpc,
-			 mhdp->display_fmt.bpc);
-
-	return num_modes;
-}
-
-static int cdns_mhdp_connector_detect(struct drm_connector *conn,
-				      struct drm_modeset_acquire_ctx *ctx,
-				      bool force)
-{
-	struct cdns_mhdp_device *mhdp = connector_to_mhdp(conn);
-
-	return cdns_mhdp_detect(mhdp);
-}
-
 static u32 cdns_mhdp_get_bpp(struct cdns_mhdp_display_fmt *fmt)
 {
 	u32 bpp;
@@ -1547,114 +1493,6 @@ bool cdns_mhdp_bandwidth_ok(struct cdns_mhdp_device *mhdp,
 	return true;
 }
 
-static
-enum drm_mode_status cdns_mhdp_mode_valid(struct drm_connector *conn,
-					  const struct drm_display_mode *mode)
-{
-	struct cdns_mhdp_device *mhdp = connector_to_mhdp(conn);
-
-	mutex_lock(&mhdp->link_mutex);
-
-	if (!cdns_mhdp_bandwidth_ok(mhdp, mode, mhdp->link.num_lanes,
-				    mhdp->link.rate)) {
-		mutex_unlock(&mhdp->link_mutex);
-		return MODE_CLOCK_HIGH;
-	}
-
-	mutex_unlock(&mhdp->link_mutex);
-	return MODE_OK;
-}
-
-static int cdns_mhdp_connector_atomic_check(struct drm_connector *conn,
-					    struct drm_atomic_state *state)
-{
-	struct cdns_mhdp_device *mhdp = connector_to_mhdp(conn);
-	struct drm_connector_state *old_state, *new_state;
-	struct drm_crtc_state *crtc_state;
-	u64 old_cp, new_cp;
-
-	if (!mhdp->hdcp_supported)
-		return 0;
-
-	old_state = drm_atomic_get_old_connector_state(state, conn);
-	new_state = drm_atomic_get_new_connector_state(state, conn);
-	old_cp = old_state->content_protection;
-	new_cp = new_state->content_protection;
-
-	if (old_state->hdcp_content_type != new_state->hdcp_content_type &&
-	    new_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
-		new_state->content_protection = DRM_MODE_CONTENT_PROTECTION_DESIRED;
-		goto mode_changed;
-	}
-
-	if (!new_state->crtc) {
-		if (old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED)
-			new_state->content_protection = DRM_MODE_CONTENT_PROTECTION_DESIRED;
-		return 0;
-	}
-
-	if (old_cp == new_cp ||
-	    (old_cp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
-	     new_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED))
-		return 0;
-
-mode_changed:
-	crtc_state = drm_atomic_get_new_crtc_state(state, new_state->crtc);
-	crtc_state->mode_changed = true;
-
-	return 0;
-}
-
-static const struct drm_connector_helper_funcs cdns_mhdp_conn_helper_funcs = {
-	.detect_ctx = cdns_mhdp_connector_detect,
-	.get_modes = cdns_mhdp_get_modes,
-	.mode_valid = cdns_mhdp_mode_valid,
-	.atomic_check = cdns_mhdp_connector_atomic_check,
-};
-
-static const struct drm_connector_funcs cdns_mhdp_conn_funcs = {
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
-	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-	.reset = drm_atomic_helper_connector_reset,
-	.destroy = drm_connector_cleanup,
-};
-
-static int cdns_mhdp_connector_init(struct cdns_mhdp_device *mhdp)
-{
-	u32 bus_format = MEDIA_BUS_FMT_RGB121212_1X36;
-	struct drm_connector *conn = &mhdp->connector;
-	struct drm_bridge *bridge = &mhdp->bridge;
-	int ret;
-
-	conn->polled = DRM_CONNECTOR_POLL_HPD;
-
-	ret = drm_connector_init(bridge->dev, conn, &cdns_mhdp_conn_funcs,
-				 DRM_MODE_CONNECTOR_DisplayPort);
-	if (ret) {
-		dev_err(mhdp->dev, "Failed to initialize connector with drm\n");
-		return ret;
-	}
-
-	drm_connector_helper_add(conn, &cdns_mhdp_conn_helper_funcs);
-
-	ret = drm_display_info_set_bus_formats(&conn->display_info,
-					       &bus_format, 1);
-	if (ret)
-		return ret;
-
-	ret = drm_connector_attach_encoder(conn, bridge->encoder);
-	if (ret) {
-		dev_err(mhdp->dev, "Failed to attach connector to encoder\n");
-		return ret;
-	}
-
-	if (mhdp->hdcp_supported)
-		ret = drm_connector_attach_content_protection_property(conn, true);
-
-	return ret;
-}
-
 static int cdns_mhdp_attach(struct drm_bridge *bridge,
 			    struct drm_encoder *encoder,
 			    enum drm_bridge_attach_flags flags)
@@ -1671,9 +1509,11 @@ static int cdns_mhdp_attach(struct drm_bridge *bridge,
 		return ret;
 
 	if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
-		ret = cdns_mhdp_connector_init(mhdp);
-		if (ret)
-			goto aux_unregister;
+		ret = -EINVAL;
+		dev_err(mhdp->dev,
+			"Connector initialisation not supported in bridge_attach %d\n",
+			ret);
+		goto aux_unregister;
 	}
 
 	spin_lock(&mhdp->start_lock);
@@ -2368,17 +2208,10 @@ static void cdns_mhdp_hpd_work(struct work_struct *work)
 	struct cdns_mhdp_device *mhdp = container_of(work,
 						     struct cdns_mhdp_device,
 						     hpd_work);
-	int ret;
 
-	ret = cdns_mhdp_update_link_status(mhdp);
-	if (mhdp->connector.dev) {
-		if (ret < 0)
-			schedule_work(&mhdp->modeset_retry_work);
-		else
-			drm_kms_helper_hotplug_event(mhdp->bridge.dev);
-	} else {
-		drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
-	}
+	cdns_mhdp_update_link_status(mhdp);
+
+	drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
 }
 
 static int cdns_mhdp_probe(struct platform_device *pdev)
-- 
2.34.1

Hi,

On 11/08/2025 10:58, Harikrishna Shenoy wrote:
> From: Jayesh Choudhary <j-choudhary@ti.com>
> 
> Now that we have DBANC framework, remove the connector initialisation code
> as that piece of code is not called if DRM_BRIDGE_ATTACH_NO_CONNECTOR flag
> is used. Only TI K3 platforms consume this driver and tidss (their display
> controller) has this flag set. So this legacy support can be dropped.
> 
> Fixes: c932ced6b585 ("drm/tidss: Update encoder/bridge chain connect model")

You have a fixes tag here. What bug does this fix?

> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
> ---
>  .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 187 +-----------------
>  1 file changed, 10 insertions(+), 177 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> index a614d1384f71..08702ade2903 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> @@ -739,12 +739,8 @@ static void cdns_mhdp_fw_cb(const struct firmware *fw, void *context)
>  	spin_lock(&mhdp->start_lock);
>  	bridge_attached = mhdp->bridge_attached;
>  	spin_unlock(&mhdp->start_lock);
> -	if (bridge_attached) {
> -		if (mhdp->connector.dev)
> -			drm_kms_helper_hotplug_event(mhdp->bridge.dev);
> -		else
> -			drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
> -	}
> +	if (bridge_attached)
> +		drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
>  }
>  
>  static int cdns_mhdp_load_firmware(struct cdns_mhdp_device *mhdp)
> @@ -1444,56 +1440,6 @@ static const struct drm_edid *cdns_mhdp_edid_read(struct cdns_mhdp_device *mhdp,
>  	return drm_edid_read_custom(connector, cdns_mhdp_get_edid_block, mhdp);
>  }
>  
> -static int cdns_mhdp_get_modes(struct drm_connector *connector)
> -{
> -	struct cdns_mhdp_device *mhdp = connector_to_mhdp(connector);
> -	const struct drm_edid *drm_edid;
> -	int num_modes;
> -
> -	if (!mhdp->plugged)
> -		return 0;
> -
> -	drm_edid = cdns_mhdp_edid_read(mhdp, connector);
> -
> -	drm_edid_connector_update(connector, drm_edid);
> -
> -	if (!drm_edid) {
> -		dev_err(mhdp->dev, "Failed to read EDID\n");
> -		return 0;
> -	}
> -
> -	num_modes = drm_edid_connector_add_modes(connector);
> -	drm_edid_free(drm_edid);
> -
> -	/*
> -	 * HACK: Warn about unsupported display formats until we deal
> -	 *       with them correctly.
> -	 */
> -	if (connector->display_info.color_formats &&
> -	    !(connector->display_info.color_formats &
> -	      mhdp->display_fmt.color_format))
> -		dev_warn(mhdp->dev,
> -			 "%s: No supported color_format found (0x%08x)\n",
> -			__func__, connector->display_info.color_formats);
> -
> -	if (connector->display_info.bpc &&
> -	    connector->display_info.bpc < mhdp->display_fmt.bpc)
> -		dev_warn(mhdp->dev, "%s: Display bpc only %d < %d\n",
> -			 __func__, connector->display_info.bpc,
> -			 mhdp->display_fmt.bpc);
> -
> -	return num_modes;
> -}
> -
> -static int cdns_mhdp_connector_detect(struct drm_connector *conn,
> -				      struct drm_modeset_acquire_ctx *ctx,
> -				      bool force)
> -{
> -	struct cdns_mhdp_device *mhdp = connector_to_mhdp(conn);
> -
> -	return cdns_mhdp_detect(mhdp);
> -}
> -
>  static u32 cdns_mhdp_get_bpp(struct cdns_mhdp_display_fmt *fmt)
>  {
>  	u32 bpp;
> @@ -1547,114 +1493,6 @@ bool cdns_mhdp_bandwidth_ok(struct cdns_mhdp_device *mhdp,
>  	return true;
>  }
>  
> -static
> -enum drm_mode_status cdns_mhdp_mode_valid(struct drm_connector *conn,
> -					  const struct drm_display_mode *mode)
> -{
> -	struct cdns_mhdp_device *mhdp = connector_to_mhdp(conn);
> -
> -	mutex_lock(&mhdp->link_mutex);
> -
> -	if (!cdns_mhdp_bandwidth_ok(mhdp, mode, mhdp->link.num_lanes,
> -				    mhdp->link.rate)) {
> -		mutex_unlock(&mhdp->link_mutex);
> -		return MODE_CLOCK_HIGH;
> -	}
> -
> -	mutex_unlock(&mhdp->link_mutex);
> -	return MODE_OK;
> -}
> -
> -static int cdns_mhdp_connector_atomic_check(struct drm_connector *conn,
> -					    struct drm_atomic_state *state)
> -{
> -	struct cdns_mhdp_device *mhdp = connector_to_mhdp(conn);
> -	struct drm_connector_state *old_state, *new_state;
> -	struct drm_crtc_state *crtc_state;
> -	u64 old_cp, new_cp;
> -
> -	if (!mhdp->hdcp_supported)
> -		return 0;
> -
> -	old_state = drm_atomic_get_old_connector_state(state, conn);
> -	new_state = drm_atomic_get_new_connector_state(state, conn);
> -	old_cp = old_state->content_protection;
> -	new_cp = new_state->content_protection;
> -
> -	if (old_state->hdcp_content_type != new_state->hdcp_content_type &&
> -	    new_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> -		new_state->content_protection = DRM_MODE_CONTENT_PROTECTION_DESIRED;
> -		goto mode_changed;
> -	}
> -
> -	if (!new_state->crtc) {
> -		if (old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED)
> -			new_state->content_protection = DRM_MODE_CONTENT_PROTECTION_DESIRED;
> -		return 0;
> -	}
> -
> -	if (old_cp == new_cp ||
> -	    (old_cp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
> -	     new_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED))
> -		return 0;
> -
> -mode_changed:
> -	crtc_state = drm_atomic_get_new_crtc_state(state, new_state->crtc);
> -	crtc_state->mode_changed = true;
> -
> -	return 0;
> -}
> -
> -static const struct drm_connector_helper_funcs cdns_mhdp_conn_helper_funcs = {
> -	.detect_ctx = cdns_mhdp_connector_detect,
> -	.get_modes = cdns_mhdp_get_modes,
> -	.mode_valid = cdns_mhdp_mode_valid,
> -	.atomic_check = cdns_mhdp_connector_atomic_check,
> -};
> -
> -static const struct drm_connector_funcs cdns_mhdp_conn_funcs = {
> -	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> -	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> -	.reset = drm_atomic_helper_connector_reset,
> -	.destroy = drm_connector_cleanup,
> -};
> -
> -static int cdns_mhdp_connector_init(struct cdns_mhdp_device *mhdp)
> -{
> -	u32 bus_format = MEDIA_BUS_FMT_RGB121212_1X36;
> -	struct drm_connector *conn = &mhdp->connector;
> -	struct drm_bridge *bridge = &mhdp->bridge;
> -	int ret;
> -
> -	conn->polled = DRM_CONNECTOR_POLL_HPD;
> -
> -	ret = drm_connector_init(bridge->dev, conn, &cdns_mhdp_conn_funcs,
> -				 DRM_MODE_CONNECTOR_DisplayPort);
> -	if (ret) {
> -		dev_err(mhdp->dev, "Failed to initialize connector with drm\n");
> -		return ret;
> -	}
> -
> -	drm_connector_helper_add(conn, &cdns_mhdp_conn_helper_funcs);
> -
> -	ret = drm_display_info_set_bus_formats(&conn->display_info,
> -					       &bus_format, 1);
> -	if (ret)
> -		return ret;
> -
> -	ret = drm_connector_attach_encoder(conn, bridge->encoder);
> -	if (ret) {
> -		dev_err(mhdp->dev, "Failed to attach connector to encoder\n");
> -		return ret;
> -	}
> -
> -	if (mhdp->hdcp_supported)
> -		ret = drm_connector_attach_content_protection_property(conn, true);
> -
> -	return ret;
> -}
> -
>  static int cdns_mhdp_attach(struct drm_bridge *bridge,
>  			    struct drm_encoder *encoder,
>  			    enum drm_bridge_attach_flags flags)
> @@ -1671,9 +1509,11 @@ static int cdns_mhdp_attach(struct drm_bridge *bridge,
>  		return ret;
>  
>  	if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
> -		ret = cdns_mhdp_connector_init(mhdp);
> -		if (ret)
> -			goto aux_unregister;
> +		ret = -EINVAL;
> +		dev_err(mhdp->dev,
> +			"Connector initialisation not supported in bridge_attach %d\n",
> +			ret);
> +		goto aux_unregister;
>  	}
>  
>  	spin_lock(&mhdp->start_lock);
> @@ -2368,17 +2208,10 @@ static void cdns_mhdp_hpd_work(struct work_struct *work)
>  	struct cdns_mhdp_device *mhdp = container_of(work,
>  						     struct cdns_mhdp_device,
>  						     hpd_work);
> -	int ret;
>  
> -	ret = cdns_mhdp_update_link_status(mhdp);
> -	if (mhdp->connector.dev) {
> -		if (ret < 0)
> -			schedule_work(&mhdp->modeset_retry_work);
> -		else
> -			drm_kms_helper_hotplug_event(mhdp->bridge.dev);
> -	} else {
> -		drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
> -	}
> +	cdns_mhdp_update_link_status(mhdp);

We don't check the return value anymore... This function is void, so we
can't propagate the error further. We could change
cdns_mhdp_update_link_status to return void, but maybe it's better to
catch the error here, and print an error.

 Tomi

> +
> +	drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
>  }
>  
>  static int cdns_mhdp_probe(struct platform_device *pdev)

On 9/1/25 15:22, Tomi Valkeinen wrote:
> Hi,
>
> On 11/08/2025 10:58, Harikrishna Shenoy wrote:
>> From: Jayesh Choudhary <j-choudhary@ti.com>
>>
>> Now that we have DBANC framework, remove the connector initialisation code
>> as that piece of code is not called if DRM_BRIDGE_ATTACH_NO_CONNECTOR flag
>> is used. Only TI K3 platforms consume this driver and tidss (their display
>> controller) has this flag set. So this legacy support can be dropped.
>>
>> Fixes: c932ced6b585 ("drm/tidss: Update encoder/bridge chain connect model")
> You have a fixes tag here. What bug does this fix?

https://lore.kernel.org/all/05948e1c-fa08-4aca-b705-b2e3a228f758@ti.com/

will include explanation in cover-letter.

>> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
>> ---
>>   .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 187 +-----------------
>>   1 file changed, 10 insertions(+), 177 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
>> index a614d1384f71..08702ade2903 100644
>> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
>> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
>> @@ -739,12 +739,8 @@ static void cdns_mhdp_fw_cb(const struct firmware *fw, void *context)
>>   	spin_lock(&mhdp->start_lock);
>>   	bridge_attached = mhdp->bridge_attached;
>>   	spin_unlock(&mhdp->start_lock);
>> -	if (bridge_attached) {
>> -		if (mhdp->connector.dev)
>> -			drm_kms_helper_hotplug_event(mhdp->bridge.dev);
>> -		else
>> -			drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
>> -	}
>> +	if (bridge_attached)
>> +		drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
>>   }
>>   
>>   static int cdns_mhdp_load_firmware(struct cdns_mhdp_device *mhdp)
>> @@ -1444,56 +1440,6 @@ static const struct drm_edid *cdns_mhdp_edid_read(struct cdns_mhdp_device *mhdp,
>>   	return drm_edid_read_custom(connector, cdns_mhdp_get_edid_block, mhdp);
>>   }
>>   
>> -static int cdns_mhdp_get_modes(struct drm_connector *connector)
>> -{
>> -	struct cdns_mhdp_device *mhdp = connector_to_mhdp(connector);
>> -	const struct drm_edid *drm_edid;
>> -	int num_modes;
>> -
>> -	if (!mhdp->plugged)
>> -		return 0;
>> -
>> -	drm_edid = cdns_mhdp_edid_read(mhdp, connector);
>> -
>> -	drm_edid_connector_update(connector, drm_edid);
>> -
>> -	if (!drm_edid) {
>> -		dev_err(mhdp->dev, "Failed to read EDID\n");
>> -		return 0;
>> -	}
>> -
>> -	num_modes = drm_edid_connector_add_modes(connector);
>> -	drm_edid_free(drm_edid);
>> -
>> -	/*
>> -	 * HACK: Warn about unsupported display formats until we deal
>> -	 *       with them correctly.
>> -	 */
>> -	if (connector->display_info.color_formats &&
>> -	    !(connector->display_info.color_formats &
>> -	      mhdp->display_fmt.color_format))
>> -		dev_warn(mhdp->dev,
>> -			 "%s: No supported color_format found (0x%08x)\n",
>> -			__func__, connector->display_info.color_formats);
>> -
>> -	if (connector->display_info.bpc &&
>> -	    connector->display_info.bpc < mhdp->display_fmt.bpc)
>> -		dev_warn(mhdp->dev, "%s: Display bpc only %d < %d\n",
>> -			 __func__, connector->display_info.bpc,
>> -			 mhdp->display_fmt.bpc);
>> -
>> -	return num_modes;
>> -}
>> -
>> -static int cdns_mhdp_connector_detect(struct drm_connector *conn,
>> -				      struct drm_modeset_acquire_ctx *ctx,
>> -				      bool force)
>> -{
>> -	struct cdns_mhdp_device *mhdp = connector_to_mhdp(conn);
>> -
>> -	return cdns_mhdp_detect(mhdp);
>> -}
>> -
>>   static u32 cdns_mhdp_get_bpp(struct cdns_mhdp_display_fmt *fmt)
>>   {
>>   	u32 bpp;
>> @@ -1547,114 +1493,6 @@ bool cdns_mhdp_bandwidth_ok(struct cdns_mhdp_device *mhdp,
>>   	return true;
>>   }
>>   
>> -static
>> -enum drm_mode_status cdns_mhdp_mode_valid(struct drm_connector *conn,
>> -					  const struct drm_display_mode *mode)
>> -{
>> -	struct cdns_mhdp_device *mhdp = connector_to_mhdp(conn);
>> -
>> -	mutex_lock(&mhdp->link_mutex);
>> -
>> -	if (!cdns_mhdp_bandwidth_ok(mhdp, mode, mhdp->link.num_lanes,
>> -				    mhdp->link.rate)) {
>> -		mutex_unlock(&mhdp->link_mutex);
>> -		return MODE_CLOCK_HIGH;
>> -	}
>> -
>> -	mutex_unlock(&mhdp->link_mutex);
>> -	return MODE_OK;
>> -}
>> -
>> -static int cdns_mhdp_connector_atomic_check(struct drm_connector *conn,
>> -					    struct drm_atomic_state *state)
>> -{
>> -	struct cdns_mhdp_device *mhdp = connector_to_mhdp(conn);
>> -	struct drm_connector_state *old_state, *new_state;
>> -	struct drm_crtc_state *crtc_state;
>> -	u64 old_cp, new_cp;
>> -
>> -	if (!mhdp->hdcp_supported)
>> -		return 0;
>> -
>> -	old_state = drm_atomic_get_old_connector_state(state, conn);
>> -	new_state = drm_atomic_get_new_connector_state(state, conn);
>> -	old_cp = old_state->content_protection;
>> -	new_cp = new_state->content_protection;
>> -
>> -	if (old_state->hdcp_content_type != new_state->hdcp_content_type &&
>> -	    new_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
>> -		new_state->content_protection = DRM_MODE_CONTENT_PROTECTION_DESIRED;
>> -		goto mode_changed;
>> -	}
>> -
>> -	if (!new_state->crtc) {
>> -		if (old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED)
>> -			new_state->content_protection = DRM_MODE_CONTENT_PROTECTION_DESIRED;
>> -		return 0;
>> -	}
>> -
>> -	if (old_cp == new_cp ||
>> -	    (old_cp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
>> -	     new_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED))
>> -		return 0;
>> -
>> -mode_changed:
>> -	crtc_state = drm_atomic_get_new_crtc_state(state, new_state->crtc);
>> -	crtc_state->mode_changed = true;
>> -
>> -	return 0;
>> -}
>> -
>> -static const struct drm_connector_helper_funcs cdns_mhdp_conn_helper_funcs = {
>> -	.detect_ctx = cdns_mhdp_connector_detect,
>> -	.get_modes = cdns_mhdp_get_modes,
>> -	.mode_valid = cdns_mhdp_mode_valid,
>> -	.atomic_check = cdns_mhdp_connector_atomic_check,
>> -};
>> -
>> -static const struct drm_connector_funcs cdns_mhdp_conn_funcs = {
>> -	.fill_modes = drm_helper_probe_single_connector_modes,
>> -	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>> -	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> -	.reset = drm_atomic_helper_connector_reset,
>> -	.destroy = drm_connector_cleanup,
>> -};
>> -
>> -static int cdns_mhdp_connector_init(struct cdns_mhdp_device *mhdp)
>> -{
>> -	u32 bus_format = MEDIA_BUS_FMT_RGB121212_1X36;
>> -	struct drm_connector *conn = &mhdp->connector;
>> -	struct drm_bridge *bridge = &mhdp->bridge;
>> -	int ret;
>> -
>> -	conn->polled = DRM_CONNECTOR_POLL_HPD;
>> -
>> -	ret = drm_connector_init(bridge->dev, conn, &cdns_mhdp_conn_funcs,
>> -				 DRM_MODE_CONNECTOR_DisplayPort);
>> -	if (ret) {
>> -		dev_err(mhdp->dev, "Failed to initialize connector with drm\n");
>> -		return ret;
>> -	}
>> -
>> -	drm_connector_helper_add(conn, &cdns_mhdp_conn_helper_funcs);
>> -
>> -	ret = drm_display_info_set_bus_formats(&conn->display_info,
>> -					       &bus_format, 1);
>> -	if (ret)
>> -		return ret;
>> -
>> -	ret = drm_connector_attach_encoder(conn, bridge->encoder);
>> -	if (ret) {
>> -		dev_err(mhdp->dev, "Failed to attach connector to encoder\n");
>> -		return ret;
>> -	}
>> -
>> -	if (mhdp->hdcp_supported)
>> -		ret = drm_connector_attach_content_protection_property(conn, true);
>> -
>> -	return ret;
>> -}
>> -
>>   static int cdns_mhdp_attach(struct drm_bridge *bridge,
>>   			    struct drm_encoder *encoder,
>>   			    enum drm_bridge_attach_flags flags)
>> @@ -1671,9 +1509,11 @@ static int cdns_mhdp_attach(struct drm_bridge *bridge,
>>   		return ret;
>>   
>>   	if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
>> -		ret = cdns_mhdp_connector_init(mhdp);
>> -		if (ret)
>> -			goto aux_unregister;
>> +		ret = -EINVAL;
>> +		dev_err(mhdp->dev,
>> +			"Connector initialisation not supported in bridge_attach %d\n",
>> +			ret);
>> +		goto aux_unregister;
>>   	}
>>   
>>   	spin_lock(&mhdp->start_lock);
>> @@ -2368,17 +2208,10 @@ static void cdns_mhdp_hpd_work(struct work_struct *work)
>>   	struct cdns_mhdp_device *mhdp = container_of(work,
>>   						     struct cdns_mhdp_device,
>>   						     hpd_work);
>> -	int ret;
>>   
>> -	ret = cdns_mhdp_update_link_status(mhdp);
>> -	if (mhdp->connector.dev) {
>> -		if (ret < 0)
>> -			schedule_work(&mhdp->modeset_retry_work);
>> -		else
>> -			drm_kms_helper_hotplug_event(mhdp->bridge.dev);
>> -	} else {
>> -		drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
>> -	}
>> +	cdns_mhdp_update_link_status(mhdp);
> We don't check the return value anymore... This function is void, so we
> can't propagate the error further. We could change
> cdns_mhdp_update_link_status to return void, but maybe it's better to
> catch the error here, and print an error.
>
>   Tomi
>
will print a warn about return value in hpd_work function.
>> +
>> +	drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp));
>>   }
>>   
>>   static int cdns_mhdp_probe(struct platform_device *pdev)