[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <49378457-2a75-4db9-9560-5df0c8a9344a@ti.com>
Date: Tue, 2 Sep 2025 14:02:09 +0530
From: Harikrishna Shenoy <h-shenoy@...com>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
CC: <jonas@...boo.se>, <jernej.skrabec@...il.com>,
<maarten.lankhorst@...ux.intel.com>, <tzimmermann@...e.de>,
<airlied@...il.com>, <simona@...ll.ch>, <lyude@...hat.com>,
<luca.ceresoli@...tlin.com>, <viro@...iv.linux.org.uk>,
<andy.yan@...k-chips.com>, <linux@...blig.org>, <javierm@...hat.com>,
<linux-kernel@...r.kernel.org>, <devarsht@...com>,
<j-choudhary@...com>, <u-kumar1@...com>, <s-jain1@...com>,
<andrzej.hajda@...el.com>, <neil.armstrong@...aro.org>,
<rfoss@...nel.org>, <Laurent.pinchart@...asonboard.com>,
<mripard@...nel.org>, <lumag@...nel.org>, <dianders@...omium.org>,
<dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH v5 2/6] drm/bridge: cadence: cdns-mhdp8546*: Change
drm_connector from structure to pointer
On 9/1/25 15:30, Tomi Valkeinen wrote:
> Hi,
>
> On 11/08/2025 10:59, Harikrishna Shenoy wrote:
>> From: Jayesh Choudhary <j-choudhary@...com>
>>
>> After adding DBANC framework, mhdp->connector is not initialised during
>> bridge_attach(). The connector is however required in few driver calls
>> like cdns_mhdp_hdcp_enable() and cdns_mhdp_modeset_retry_fn().
> Does this mean that if you apply only the previous commit, mhdp will
> crash/misbehave as mdhp->connector is not initialized?
>
possible of crash/misbehave of driver due to NULL pointer de-reference
has been discussed
in previous versions of series, will highlight it to make it more clear.
>> Use drm_connector pointer instead of structure, set it in bridge_enable()
>> and clear it in bridge_disable(), and make appropriate changes.
>>
>> Fixes: c932ced6b585 ("drm/tidss: Update encoder/bridge chain connect model")
> This also has a fixes tag, but I don't see any mention of any bug being
> fixed.
>
> For the subjects of the whole series, I think you can just use
> "drm/bridge: cdns-mhdp: ...". That's much shorter.
>
> Tomi
Explanation of bug along with logs are given in previous versions of the
series,
will include them as highlights in cover letter to make more clear.
>> Signed-off-by: Jayesh Choudhary <j-choudhary@...com>
>> ---
>> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 12 ++++++------
>> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h | 2 +-
>> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c | 8 ++++----
>> 3 files changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
>> index 08702ade2903..c2ce3d6e5a88 100644
>> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
>> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
>> @@ -1755,7 +1755,6 @@ 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;
>> @@ -1785,12 +1784,12 @@ 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))
>> + mhdp->connector = drm_atomic_get_new_connector_for_encoder(state,
>> + bridge->encoder);
>> + if (WARN_ON(!mhdp->connector))
>> goto out;
>>
>> - conn_state = drm_atomic_get_new_connector_state(state, connector);
>> + conn_state = drm_atomic_get_new_connector_state(state, mhdp->connector);
>> if (WARN_ON(!conn_state))
>> goto out;
>>
>> @@ -1853,6 +1852,7 @@ static void cdns_mhdp_atomic_disable(struct drm_bridge *bridge,
>> cdns_mhdp_hdcp_disable(mhdp);
>>
>> mhdp->bridge_enabled = false;
>> + mhdp->connector = NULL;
>> cdns_mhdp_reg_read(mhdp, CDNS_DP_FRAMER_GLOBAL_CONFIG, &resp);
>> resp &= ~CDNS_DP_FRAMER_EN;
>> resp |= CDNS_DP_NO_VIDEO_MODE;
>> @@ -2134,7 +2134,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;
>>
>> /* Grab the locks before changing connector property */
>> mutex_lock(&conn->dev->mode_config.mutex);
>> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
>> index bad2fc0c7306..b297db53ba28 100644
>> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
>> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
>> @@ -375,7 +375,7 @@ struct cdns_mhdp_device {
>> */
>> struct mutex link_mutex;
>>
>> - struct drm_connector connector;
>> + struct drm_connector *connector;
>> 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 42248f179b69..59f18c3281ef 100644
>> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c
>> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c
>> @@ -394,7 +394,7 @@ 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);
>> + mhdp->connector->name, mhdp->connector->base.id);
>>
>> ret = cdns_mhdp_hdcp_set_config(mhdp, 0, false);
>>
>> @@ -445,7 +445,7 @@ static int cdns_mhdp_hdcp_check_link(struct cdns_mhdp_device *mhdp)
>>
>> dev_err(mhdp->dev,
>> "[%s:%d] HDCP link failed, retrying authentication\n",
>> - mhdp->connector.name, mhdp->connector.base.id);
>> + mhdp->connector->name, mhdp->connector->base.id);
>>
>> ret = _cdns_mhdp_hdcp_disable(mhdp);
>> if (ret) {
>> @@ -487,13 +487,13 @@ 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 = mhdp->connector->dev;
>> struct drm_connector_state *state;
>>
>> 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;
>> + state = mhdp->connector->state;
>> state->content_protection = mhdp->hdcp.value;
>> }
>> mutex_unlock(&mhdp->hdcp.mutex);
Powered by blists - more mailing lists