Re:Re:[RFC PATCH v2 1/3] drm/bridge: cadence: cdns-mhdp8546-core: Remove legacy support for connector initialisation in bridge

["Andy Yan" <andyshrk@163.com>]

Hello Jayesh,

At 2025-05-21 16:15:33, "Andy Yan" <andyshrk@163.com> wrote:
>
>
>Hello Javesh,
>
>At 2025-05-21 15:32:35, "Jayesh Choudhary" <j-choudhary@ti.com> wrote:
>>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.
>>
>>Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
>>---
>> .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 186 +++---------------
>> 1 file changed, 25 insertions(+), 161 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
>>index b431e7efd1f0..66bd916c2fe9 100644
>>--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
>>+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
>>@@ -1444,56 +1444,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 +1497,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 +1513,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);
>>@@ -2158,6 +2002,25 @@ static const struct drm_edid *cdns_mhdp_bridge_edid_read(struct drm_bridge *brid
>> 	return cdns_mhdp_edid_read(mhdp, connector);
>> }
>> 
>>+static enum drm_mode_status
>>+cdns_mhdp_bridge_mode_valid(struct drm_bridge *bridge,
>>+			    const struct drm_display_info *info,
>>+			    const struct drm_display_mode *mode)
>>+{
>>+	struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge);
>>+
>>+	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 const struct drm_bridge_funcs cdns_mhdp_bridge_funcs = {
>> 	.atomic_enable = cdns_mhdp_atomic_enable,
>> 	.atomic_disable = cdns_mhdp_atomic_disable,
>>@@ -2172,6 +2035,7 @@ static const struct drm_bridge_funcs cdns_mhdp_bridge_funcs = {
>> 	.edid_read = cdns_mhdp_bridge_edid_read,
>> 	.hpd_enable = cdns_mhdp_bridge_hpd_enable,
>> 	.hpd_disable = cdns_mhdp_bridge_hpd_disable,
>>+	.mode_valid = cdns_mhdp_bridge_mode_valid,
>
>Do we need the detect hook for bridge after cdns_mhdp_connector_detect removed ?

Sorry, I didn't notice before. It's already here.


Reviewed-by: Andy Yan <andyshrk@163.com>

>
>
>
>> };
>> 
>> static bool cdns_mhdp_detect_hpd(struct cdns_mhdp_device *mhdp, bool *hpd_pulse)
>>-- 
>>2.34.1