Hello Tomi,

On 27/05/25 13:08, Tomi Valkeinen wrote:
Hi,

On 21/05/2025 10:32, Jayesh Choudhary 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.


Why is the series RFC? Does it not work? Is there something here you're
not comfortable with?

These changes work without any issue.

I was a little doubtful about the second patch so kept it as RFC.


Signed-off-by: Jayesh Choudhary <j-choudh...@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,
  };
static bool cdns_mhdp_detect_hpd(struct cdns_mhdp_device *mhdp, bool *hpd_pulse)

Why do you need to add bridge mode_valid() when removing the legacy
non-DRM_BRIDGE_ATTACH_NO_CONNECTOR code?

Okay. Then I will add the bridge mode_valid as a separate patch.

Warm Regards,
Jayesh


  Tomi

Reply via email to