[PATCH v5 6/6] drm/bridge: cadence: cdns-mhdp8546-core: Handle HDCP state in bridge atomic check

Harikrishna Shenoy posted 6 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH v5 6/6] drm/bridge: cadence: cdns-mhdp8546-core: Handle HDCP state in bridge atomic check
Posted by Harikrishna Shenoy 1 month, 3 weeks ago
Now that we have DBANC framework and legacy connector functions removed,
handle the HDCP disabling in bridge atomic check rather than in connector
atomic check previously.

Signed-off-by: Harikrishna Shenoy <h-shenoy@ti.com>
---
 .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index 4fb1db3e030c..af41b2908a74 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -1960,6 +1960,10 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge,
 {
 	struct cdns_mhdp_device *mhdp = bridge_to_mhdp(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;
+	u64 old_cp, new_cp;
 
 	mutex_lock(&mhdp->link_mutex);
 
@@ -1979,6 +1983,25 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge,
 	if (mhdp->info)
 		bridge_state->input_bus_cfg.flags = *mhdp->info->input_bus_flags;
 
+	if (conn && mhdp->hdcp_supported) {
+		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;
+			crtc_state = drm_atomic_get_new_crtc_state(state, new_state->crtc);
+			crtc_state->mode_changed = true;
+		}
+
+		if (!new_state->crtc) {
+			if (old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED)
+				new_state->content_protection = DRM_MODE_CONTENT_PROTECTION_DESIRED;
+		}
+	}
+
 	mutex_unlock(&mhdp->link_mutex);
 	return 0;
 }
-- 
2.34.1
Re: [PATCH v5 6/6] drm/bridge: cadence: cdns-mhdp8546-core: Handle HDCP state in bridge atomic check
Posted by Tomi Valkeinen 1 month ago
Hi,

On 11/08/2025 10:59, Harikrishna Shenoy wrote:
> Now that we have DBANC framework and legacy connector functions removed,
> handle the HDCP disabling in bridge atomic check rather than in connector
> atomic check previously.

Both this and the patch 4 make me feel a bit confused: In patch 1 a
bunch of code is removed. Then in patches 4 and 6 we add it back. Yes,
we don't add it back the same way, but this raises the question if the
first patch then breaks these two features, until patches 4 and 6 add it
back.

Or is the case that with DRM_BRIDGE_ATTACH_NO_CONNECTOR, which is the
way tidss uses this, none of the code removed in patch 1 was even being
called? And thus, in theory, patches 4 and 6 could even be added before
patch 1?

The patches nor the cover letter really explain well what's going on
here. The "fixes" tags also confuse me. So is the current upstream
driver working fine or not? Are there bugs? It would be good to fix
those bugs first, then do the cleanup of removing the old code. Maybe
that's difficult to do and this patch order makes sense, but it's all
left very unclear to the reviewer.

 Tomi

> 
> Signed-off-by: Harikrishna Shenoy <h-shenoy@ti.com>
> ---
>  .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 23 +++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> index 4fb1db3e030c..af41b2908a74 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> @@ -1960,6 +1960,10 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge,
>  {
>  	struct cdns_mhdp_device *mhdp = bridge_to_mhdp(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;
> +	u64 old_cp, new_cp;
>  
>  	mutex_lock(&mhdp->link_mutex);
>  
> @@ -1979,6 +1983,25 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge,
>  	if (mhdp->info)
>  		bridge_state->input_bus_cfg.flags = *mhdp->info->input_bus_flags;
>  
> +	if (conn && mhdp->hdcp_supported) {
> +		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;
> +			crtc_state = drm_atomic_get_new_crtc_state(state, new_state->crtc);
> +			crtc_state->mode_changed = true;
> +		}
> +
> +		if (!new_state->crtc) {
> +			if (old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED)
> +				new_state->content_protection = DRM_MODE_CONTENT_PROTECTION_DESIRED;
> +		}
> +	}
> +
>  	mutex_unlock(&mhdp->link_mutex);
>  	return 0;
>  }
Re: [PATCH v5 6/6] drm/bridge: cadence: cdns-mhdp8546-core: Handle HDCP state in bridge atomic check
Posted by Harikrishna Shenoy 1 month ago
On 9/1/25 15:48, Tomi Valkeinen wrote:
> Hi,
>
> On 11/08/2025 10:59, Harikrishna Shenoy wrote:
>> Now that we have DBANC framework and legacy connector functions removed,
>> handle the HDCP disabling in bridge atomic check rather than in connector
>> atomic check previously.
> Both this and the patch 4 make me feel a bit confused: In patch 1 a
> bunch of code is removed. Then in patches 4 and 6 we add it back. Yes,
> we don't add it back the same way, but this raises the question if the
> first patch then breaks these two features, until patches 4 and 6 add it
> back.
>
> Or is the case that with DRM_BRIDGE_ATTACH_NO_CONNECTOR, which is the
> way tidss uses this, none of the code removed in patch 1 was even being
> called? And thus, in theory, patches 4 and 6 could even be added before
> patch 1?

Patch 4 and 6 preserve the cases which might miss handling like HDCP 
when removing

bunch of code associated with connector as those are basically rendered 
dead due to

DBANC flag, will recombine these patches (4 and 6) and also be clear 
about the bug fixed


>
> The patches nor the cover letter really explain well what's going on
> here. The "fixes" tags also confuse me. So is the current upstream
> driver working fine or not? Are there bugs? It would be good to fix
> those bugs first, then do the cleanup of removing the old code. Maybe
> that's difficult to do and this patch order makes sense, but it's all
> left very unclear to the reviewer.
>   Tomi
Will respin the series
>> Signed-off-by: Harikrishna Shenoy <h-shenoy@ti.com>
>> ---
>>   .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 23 +++++++++++++++++++
>>   1 file changed, 23 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
>> index 4fb1db3e030c..af41b2908a74 100644
>> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
>> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
>> @@ -1960,6 +1960,10 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge,
>>   {
>>   	struct cdns_mhdp_device *mhdp = bridge_to_mhdp(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;
>> +	u64 old_cp, new_cp;
>>   
>>   	mutex_lock(&mhdp->link_mutex);
>>   
>> @@ -1979,6 +1983,25 @@ static int cdns_mhdp_atomic_check(struct drm_bridge *bridge,
>>   	if (mhdp->info)
>>   		bridge_state->input_bus_cfg.flags = *mhdp->info->input_bus_flags;
>>   
>> +	if (conn && mhdp->hdcp_supported) {
>> +		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;
>> +			crtc_state = drm_atomic_get_new_crtc_state(state, new_state->crtc);
>> +			crtc_state->mode_changed = true;
>> +		}
>> +
>> +		if (!new_state->crtc) {
>> +			if (old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED)
>> +				new_state->content_protection = DRM_MODE_CONTENT_PROTECTION_DESIRED;
>> +		}
>> +	}
>> +
>>   	mutex_unlock(&mhdp->link_mutex);
>>   	return 0;
>>   }