[PATCH RESEND v9 5/6] cadence: cdns-mhdp8546*: Change drm_connector from structure to pointer

Harikrishna Shenoy posted 6 patches 1 week, 4 days ago
[PATCH RESEND v9 5/6] cadence: cdns-mhdp8546*: Change drm_connector from structure to pointer
Posted by Harikrishna Shenoy 1 week, 4 days ago
After enabling DRM_BRIDGE_ATTACH_NO_CONNECTOR flag, 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().Now that we have dropped the legacy code
which became redundant with introduction of DRM_BRIDGE_ATTACH_NO_CONNECTOR
usecase in driver,we can cleanly switch to drm_connector pointer
instead of structure.

Set it in bridge_enable() and clear it in bridge_disable(),
and make appropriate changes.

This allows us to dynamically set the reference in bridge_enable()
when the connector becomes available and clear it in bridge_disable().
This change is necessary to properly integrate with the
DRM_BRIDGE_ATTACH_NO_CONNECTOR flag set while maintaining all
connector-dependent functionality in the driver.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
Signed-off-by: Harikrishna Shenoy <h-shenoy@ti.com>
---
 .../gpu/drm/bridge/cadence/cdns-mhdp8546-core.c  | 14 +++++++-------
 .../gpu/drm/bridge/cadence/cdns-mhdp8546-core.h  |  3 +--
 .../gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c  | 16 ++++++++--------
 3 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index 816d5d87b45fe..002b4be3de674 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -1765,12 +1765,12 @@ static void cdns_mhdp_atomic_enable(struct drm_bridge *bridge,
 
 	mutex_lock(&mhdp->link_mutex);
 
-	mhdp->connector_ptr = drm_atomic_get_new_connector_for_encoder(state,
-								       bridge->encoder);
-	if (WARN_ON(!mhdp->connector_ptr))
+	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, mhdp->connector_ptr);
+	conn_state = drm_atomic_get_new_connector_state(state, mhdp->connector);
 	if (WARN_ON(!conn_state))
 		goto out;
 
@@ -1869,7 +1869,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;
+	mhdp->connector = NULL;
 	mutex_unlock(&mhdp->link_mutex);
 }
 
@@ -1964,7 +1964,7 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge,
 	const struct drm_display_mode *mode = &crtc_state->adjusted_mode;
 	struct drm_connector_state *old_state, *new_state;
 	struct drm_atomic_state *state = crtc_state->state;
-	struct drm_connector *conn = mhdp->connector_ptr;
+	struct drm_connector *conn = mhdp->connector;
 	u64 old_cp, new_cp;
 
 	mutex_lock(&mhdp->link_mutex);
@@ -2179,7 +2179,7 @@ static void cdns_mhdp_modeset_retry_fn(struct work_struct *work)
 
 	mhdp = container_of(work, typeof(*mhdp), modeset_retry_work);
 
-	conn = mhdp->connector_ptr;
+	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 a76775c768956..b297db53ba283 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
@@ -375,8 +375,7 @@ struct cdns_mhdp_device {
 	 */
 	struct mutex link_mutex;
 
-	struct drm_connector connector;
-	struct drm_connector *connector_ptr;
+	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 5ac2fad2f0078..1d433ad3fe878 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c
@@ -393,9 +393,9 @@ static int _cdns_mhdp_hdcp_disable(struct cdns_mhdp_device *mhdp)
 {
 	int ret;
 
-	if (mhdp->connector_ptr) {
+	if (mhdp->connector) {
 		dev_dbg(mhdp->dev, "[%s:%d] HDCP is being disabled...\n",
-			mhdp->connector_ptr->name, mhdp->connector_ptr->base.id);
+			mhdp->connector->name, mhdp->connector->base.id);
 	}
 
 	ret = cdns_mhdp_hdcp_set_config(mhdp, 0, false);
@@ -445,10 +445,10 @@ static int cdns_mhdp_hdcp_check_link(struct cdns_mhdp_device *mhdp)
 	if (!ret && hdcp_port_status & HDCP_PORT_STS_AUTH)
 		goto out;
 
-	if (mhdp->connector_ptr) {
+	if (mhdp->connector) {
 		dev_err(mhdp->dev,
 			"[%s:%d] HDCP link failed, retrying authentication\n",
-			mhdp->connector_ptr->name, mhdp->connector_ptr->base.id);
+			mhdp->connector->name, mhdp->connector->base.id);
 	}
 
 	ret = _cdns_mhdp_hdcp_disable(mhdp);
@@ -494,16 +494,16 @@ static void cdns_mhdp_hdcp_prop_work(struct work_struct *work)
 	struct drm_device *dev = NULL;
 	struct drm_connector_state *state;
 
-	if (mhdp->connector_ptr)
-		dev = mhdp->connector_ptr->dev;
+	if (mhdp->connector)
+		dev = mhdp->connector->dev;
 
 	if (!dev)
 		return;
 
 	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
 	mutex_lock(&mhdp->hdcp.mutex);
-	if (mhdp->connector_ptr && mhdp->hdcp.value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
-		state = mhdp->connector_ptr->state;
+	if (mhdp->connector && mhdp->hdcp.value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
+		state = mhdp->connector->state;
 		state->content_protection = mhdp->hdcp.value;
 	}
 	mutex_unlock(&mhdp->hdcp.mutex);
-- 
2.34.1
Re: [PATCH RESEND v9 5/6] cadence: cdns-mhdp8546*: Change drm_connector from structure to pointer
Posted by Tomi Valkeinen 1 week, 3 days ago
Hi,

On 20/11/2025 14:14, Harikrishna Shenoy wrote:
> After enabling DRM_BRIDGE_ATTACH_NO_CONNECTOR flag, 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().Now that we have dropped the legacy code
> which became redundant with introduction of DRM_BRIDGE_ATTACH_NO_CONNECTOR
> usecase in driver,we can cleanly switch to drm_connector pointer
> instead of structure.
> 
> Set it in bridge_enable() and clear it in bridge_disable(),
> and make appropriate changes.
> 
> This allows us to dynamically set the reference in bridge_enable()
> when the connector becomes available and clear it in bridge_disable().
> This change is necessary to properly integrate with the
> DRM_BRIDGE_ATTACH_NO_CONNECTOR flag set while maintaining all
> connector-dependent functionality in the driver.

Now the code looks fine, but you didn't update the description, which
now looks to be quite wrong for this.

 Tomi

> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
> Signed-off-by: Harikrishna Shenoy <h-shenoy@ti.com>
> ---
>  .../gpu/drm/bridge/cadence/cdns-mhdp8546-core.c  | 14 +++++++-------
>  .../gpu/drm/bridge/cadence/cdns-mhdp8546-core.h  |  3 +--
>  .../gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c  | 16 ++++++++--------
>  3 files changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> index 816d5d87b45fe..002b4be3de674 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> @@ -1765,12 +1765,12 @@ static void cdns_mhdp_atomic_enable(struct drm_bridge *bridge,
>  
>  	mutex_lock(&mhdp->link_mutex);
>  
> -	mhdp->connector_ptr = drm_atomic_get_new_connector_for_encoder(state,
> -								       bridge->encoder);
> -	if (WARN_ON(!mhdp->connector_ptr))
> +	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, mhdp->connector_ptr);
> +	conn_state = drm_atomic_get_new_connector_state(state, mhdp->connector);
>  	if (WARN_ON(!conn_state))
>  		goto out;
>  
> @@ -1869,7 +1869,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;
> +	mhdp->connector = NULL;
>  	mutex_unlock(&mhdp->link_mutex);
>  }
>  
> @@ -1964,7 +1964,7 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge,
>  	const struct drm_display_mode *mode = &crtc_state->adjusted_mode;
>  	struct drm_connector_state *old_state, *new_state;
>  	struct drm_atomic_state *state = crtc_state->state;
> -	struct drm_connector *conn = mhdp->connector_ptr;
> +	struct drm_connector *conn = mhdp->connector;
>  	u64 old_cp, new_cp;
>  
>  	mutex_lock(&mhdp->link_mutex);
> @@ -2179,7 +2179,7 @@ static void cdns_mhdp_modeset_retry_fn(struct work_struct *work)
>  
>  	mhdp = container_of(work, typeof(*mhdp), modeset_retry_work);
>  
> -	conn = mhdp->connector_ptr;
> +	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 a76775c768956..b297db53ba283 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.h
> @@ -375,8 +375,7 @@ struct cdns_mhdp_device {
>  	 */
>  	struct mutex link_mutex;
>  
> -	struct drm_connector connector;
> -	struct drm_connector *connector_ptr;
> +	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 5ac2fad2f0078..1d433ad3fe878 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c
> @@ -393,9 +393,9 @@ static int _cdns_mhdp_hdcp_disable(struct cdns_mhdp_device *mhdp)
>  {
>  	int ret;
>  
> -	if (mhdp->connector_ptr) {
> +	if (mhdp->connector) {
>  		dev_dbg(mhdp->dev, "[%s:%d] HDCP is being disabled...\n",
> -			mhdp->connector_ptr->name, mhdp->connector_ptr->base.id);
> +			mhdp->connector->name, mhdp->connector->base.id);
>  	}
>  
>  	ret = cdns_mhdp_hdcp_set_config(mhdp, 0, false);
> @@ -445,10 +445,10 @@ static int cdns_mhdp_hdcp_check_link(struct cdns_mhdp_device *mhdp)
>  	if (!ret && hdcp_port_status & HDCP_PORT_STS_AUTH)
>  		goto out;
>  
> -	if (mhdp->connector_ptr) {
> +	if (mhdp->connector) {
>  		dev_err(mhdp->dev,
>  			"[%s:%d] HDCP link failed, retrying authentication\n",
> -			mhdp->connector_ptr->name, mhdp->connector_ptr->base.id);
> +			mhdp->connector->name, mhdp->connector->base.id);
>  	}
>  
>  	ret = _cdns_mhdp_hdcp_disable(mhdp);
> @@ -494,16 +494,16 @@ static void cdns_mhdp_hdcp_prop_work(struct work_struct *work)
>  	struct drm_device *dev = NULL;
>  	struct drm_connector_state *state;
>  
> -	if (mhdp->connector_ptr)
> -		dev = mhdp->connector_ptr->dev;
> +	if (mhdp->connector)
> +		dev = mhdp->connector->dev;
>  
>  	if (!dev)
>  		return;
>  
>  	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>  	mutex_lock(&mhdp->hdcp.mutex);
> -	if (mhdp->connector_ptr && mhdp->hdcp.value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> -		state = mhdp->connector_ptr->state;
> +	if (mhdp->connector && mhdp->hdcp.value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> +		state = mhdp->connector->state;
>  		state->content_protection = mhdp->hdcp.value;
>  	}
>  	mutex_unlock(&mhdp->hdcp.mutex);
Re: [PATCH RESEND v9 5/6] cadence: cdns-mhdp8546*: Change drm_connector from structure to pointer
Posted by Harikrishna shenoy 1 week, 3 days ago

On 21/11/25 17:32, Tomi Valkeinen wrote:
> Now the code looks fine, but you didn't update the description, which
> now looks to be quite wrong for this.

Hi Tomi,

mhdp->connector is still a structure before this commit and changed to 
pointer in this commit, we had introduced a extra connector pointer and 
variable for fix and kind of got renamed to 'connector' from 
'connector_ptr' and added checks in HDCP , will tweak the commit message 
describing these changes.

Thanks.