From: Jayesh Choudhary <[email protected]>

In case if we get errors in cdns_mhdp_link_up() or cdns_mhdp_reg_read()
in atomic_enable, we will go to cdns_mhdp_modeset_retry_fn() and will hit
NULL pointer while trying to access the mutex. We need the connector to
be set before that. Unlike in legacy cases with flag
!DRM_BRIDGE_ATTACH_NO_CONNECTOR, we do not have connector initialised
in bridge_attach(), so add the mhdp->connector_ptr in device structure
to handle both cases with DRM_BRIDGE_ATTACH_NO_CONNECTOR and
!DRM_BRIDGE_ATTACH_NO_CONNECTOR, set it in atomic_enable() earlier to
avoid possible NULL pointer dereference in recovery paths like
modeset_retry_fn() with the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag set.

Fixes: c932ced6b585 ("drm/tidss: Update encoder/bridge chain connect model")
Signed-off-by: Jayesh Choudhary <[email protected]>
Signed-off-by: Harikrishna Shenoy <[email protected]>
---
 .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 29 ++++++++++---------
 .../drm/bridge/cadence/cdns-mhdp8546-core.h   |  1 +
 .../drm/bridge/cadence/cdns-mhdp8546-hdcp.c   | 26 ++++++++++++-----
 3 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c 
b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index 38726ae1bf150..f3076e9cdabbe 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -740,7 +740,7 @@ static void cdns_mhdp_fw_cb(const struct firmware *fw, void 
*context)
        bridge_attached = mhdp->bridge_attached;
        spin_unlock(&mhdp->start_lock);
        if (bridge_attached) {
-               if (mhdp->connector.dev)
+               if (mhdp->connector_ptr && mhdp->connector_ptr->dev)
                        drm_kms_helper_hotplug_event(mhdp->bridge.dev);
                else
                        drm_bridge_hpd_notify(&mhdp->bridge, 
cdns_mhdp_detect(mhdp));
@@ -1636,6 +1636,7 @@ static int cdns_mhdp_connector_init(struct 
cdns_mhdp_device *mhdp)
                return ret;
        }
 
+       mhdp->connector_ptr = conn;
        drm_connector_helper_add(conn, &cdns_mhdp_conn_helper_funcs);
 
        ret = drm_display_info_set_bus_formats(&conn->display_info,
@@ -1915,17 +1916,25 @@ static void cdns_mhdp_atomic_enable(struct drm_bridge 
*bridge,
        struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge);
        struct cdns_mhdp_bridge_state *mhdp_state;
        struct drm_crtc_state *crtc_state;
-       struct drm_connector *connector;
        struct drm_connector_state *conn_state;
        struct drm_bridge_state *new_state;
        const struct drm_display_mode *mode;
        u32 resp;
-       int ret;
+       int ret = 0;
 
        dev_dbg(mhdp->dev, "bridge enable\n");
 
        mutex_lock(&mhdp->link_mutex);
 
+       mhdp->connector_ptr = drm_atomic_get_new_connector_for_encoder(state,
+                                                                      
bridge->encoder);
+       if (WARN_ON(!mhdp->connector_ptr))
+               goto out;
+
+       conn_state = drm_atomic_get_new_connector_state(state, 
mhdp->connector_ptr);
+       if (WARN_ON(!conn_state))
+               goto out;
+
        if (mhdp->plugged && !mhdp->link_up) {
                ret = cdns_mhdp_link_up(mhdp);
                if (ret < 0)
@@ -1945,15 +1954,6 @@ static void cdns_mhdp_atomic_enable(struct drm_bridge 
*bridge,
        cdns_mhdp_reg_write(mhdp, CDNS_DPTX_CAR,
                            resp | CDNS_VIF_CLK_EN | CDNS_VIF_CLK_RSTN);
 
-       connector = drm_atomic_get_new_connector_for_encoder(state,
-                                                            bridge->encoder);
-       if (WARN_ON(!connector))
-               goto out;
-
-       conn_state = drm_atomic_get_new_connector_state(state, connector);
-       if (WARN_ON(!conn_state))
-               goto out;
-
        if (mhdp->hdcp_supported &&
            mhdp->hw_state == MHDP_HW_READY &&
            conn_state->content_protection ==
@@ -2030,6 +2030,7 @@ static void cdns_mhdp_atomic_disable(struct drm_bridge 
*bridge,
        if (mhdp->info && mhdp->info->ops && mhdp->info->ops->disable)
                mhdp->info->ops->disable(mhdp);
 
+       mhdp->connector_ptr = NULL;
        mutex_unlock(&mhdp->link_mutex);
 }
 
@@ -2296,7 +2297,7 @@ static void cdns_mhdp_modeset_retry_fn(struct work_struct 
*work)
 
        mhdp = container_of(work, typeof(*mhdp), modeset_retry_work);
 
-       conn = &mhdp->connector;
+       conn = mhdp->connector_ptr;
 
        /* Grab the locks before changing connector property */
        mutex_lock(&conn->dev->mode_config.mutex);
@@ -2373,7 +2374,7 @@ static void cdns_mhdp_hpd_work(struct work_struct *work)
        int ret;
 
        ret = cdns_mhdp_update_link_status(mhdp);
-       if (mhdp->connector.dev) {
+       if (mhdp->connector_ptr && mhdp->connector_ptr->dev) {
                if (ret < 0)
                        schedule_work(&mhdp->modeset_retry_work);
                else
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h 
b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
index bad2fc0c73066..a76775c768956 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
@@ -376,6 +376,7 @@ struct cdns_mhdp_device {
        struct mutex link_mutex;
 
        struct drm_connector connector;
+       struct drm_connector *connector_ptr;
        struct drm_bridge bridge;
 
        struct cdns_mhdp_link link;
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c 
b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c
index 42248f179b69d..5ac2fad2f0078 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c
@@ -393,8 +393,10 @@ static int _cdns_mhdp_hdcp_disable(struct cdns_mhdp_device 
*mhdp)
 {
        int ret;
 
-       dev_dbg(mhdp->dev, "[%s:%d] HDCP is being disabled...\n",
-               mhdp->connector.name, mhdp->connector.base.id);
+       if (mhdp->connector_ptr) {
+               dev_dbg(mhdp->dev, "[%s:%d] HDCP is being disabled...\n",
+                       mhdp->connector_ptr->name, 
mhdp->connector_ptr->base.id);
+       }
 
        ret = cdns_mhdp_hdcp_set_config(mhdp, 0, false);
 
@@ -443,9 +445,11 @@ static int cdns_mhdp_hdcp_check_link(struct 
cdns_mhdp_device *mhdp)
        if (!ret && hdcp_port_status & HDCP_PORT_STS_AUTH)
                goto out;
 
-       dev_err(mhdp->dev,
-               "[%s:%d] HDCP link failed, retrying authentication\n",
-               mhdp->connector.name, mhdp->connector.base.id);
+       if (mhdp->connector_ptr) {
+               dev_err(mhdp->dev,
+                       "[%s:%d] HDCP link failed, retrying authentication\n",
+                       mhdp->connector_ptr->name, 
mhdp->connector_ptr->base.id);
+       }
 
        ret = _cdns_mhdp_hdcp_disable(mhdp);
        if (ret) {
@@ -487,13 +491,19 @@ static void cdns_mhdp_hdcp_prop_work(struct work_struct 
*work)
        struct cdns_mhdp_device *mhdp = container_of(hdcp,
                                                     struct cdns_mhdp_device,
                                                     hdcp);
-       struct drm_device *dev = mhdp->connector.dev;
+       struct drm_device *dev = NULL;
        struct drm_connector_state *state;
 
+       if (mhdp->connector_ptr)
+               dev = mhdp->connector_ptr->dev;
+
+       if (!dev)
+               return;
+
        drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
        mutex_lock(&mhdp->hdcp.mutex);
-       if (mhdp->hdcp.value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
-               state = mhdp->connector.state;
+       if (mhdp->connector_ptr && mhdp->hdcp.value != 
DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
+               state = mhdp->connector_ptr->state;
                state->content_protection = mhdp->hdcp.value;
        }
        mutex_unlock(&mhdp->hdcp.mutex);
-- 
2.34.1

Reply via email to