[Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Anshuman Gupta]

Content Protection property should be updated as per the kernel
internal state. Let's say if Content protection is disabled
by userspace, CP property should be set to UNDESIRED so that
reauthentication will not happen until userspace request it again,
but when kernel disables the HDCP due to any DDI disabling sequences
like modeset/DPMS operation, kernel should set the property to
DESIRED, so that when opportunity arises, kernel will start the
HDCP authentication on its own.

Somewhere in the line, state machine to set content protection to
DESIRED from kernel was broken and IGT coverage was missing for it.
This patch fixes it.
IGT patch to catch further regression on this features is being
worked upon.

v2:
 - Incorporated the necessary locking. (Ram)
 - Set content protection property to CP_DESIRED only when
   user has not asked explicitly to set CP_UNDESIRED.

v3:
 - Reset the is_hdcp_undesired flag to false. (Ram)
 - Rephrasing commit log and small comment for is_hdcp_desired
   flag. (Ram)

CC: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_types.h |  6 ++++++
 drivers/gpu/drm/i915/display/intel_hdcp.c          | 13 ++++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 630a94892b7b..401a9a7689fb 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -345,6 +345,12 @@ struct intel_hdcp {
 	struct delayed_work check_work;
 	struct work_struct prop_work;
 
+	/*
+	 * Flag to differentiate that HDCP is being disabled originated from
+	 * userspace or triggered from kernel DDI disable sequence.
+	 */
+	bool is_hdcp_undesired;
+
 	/* HDCP1.4 Encryption status */
 	bool hdcp_encrypted;
 
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 0fdbd39f6641..7f631ebd8395 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -2002,11 +2002,18 @@ int intel_hdcp_disable(struct intel_connector *connector)
 	mutex_lock(&hdcp->mutex);
 
 	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
-		hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
 		if (hdcp->hdcp2_encrypted)
 			ret = _intel_hdcp2_disable(connector);
 		else if (hdcp->hdcp_encrypted)
 			ret = _intel_hdcp_disable(connector);
+
+		if (hdcp->is_hdcp_undesired) {
+			hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
+			hdcp->is_hdcp_undesired = false;
+		} else {
+			hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
+			schedule_work(&hdcp->prop_work);
+		}
 	}
 
 	mutex_unlock(&hdcp->mutex);
@@ -2044,6 +2051,7 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
 {
 	u64 old_cp = old_state->content_protection;
 	u64 new_cp = new_state->content_protection;
+	struct intel_connector *intel_conn = to_intel_connector(connector);
 	struct drm_crtc_state *crtc_state;
 
 	if (!new_state->crtc) {
@@ -2069,6 +2077,9 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
 			return;
 	}
 
+	if (new_cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
+		intel_conn->hdcp.is_hdcp_undesired  =  true;
+
 	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
 						   new_state->crtc);
 	crtc_state->mode_changed = true;
-- 
2.24.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Ramalingam C]

On 2020-01-20 at 11:19:54 +0530, Anshuman Gupta wrote:
> Content Protection property should be updated as per the kernel
> internal state. Let's say if Content protection is disabled
> by userspace, CP property should be set to UNDESIRED so that
> reauthentication will not happen until userspace request it again,
> but when kernel disables the HDCP due to any DDI disabling sequences
> like modeset/DPMS operation, kernel should set the property to
> DESIRED, so that when opportunity arises, kernel will start the
> HDCP authentication on its own.
> 
> Somewhere in the line, state machine to set content protection to
> DESIRED from kernel was broken and IGT coverage was missing for it.
> This patch fixes it.
> IGT patch to catch further regression on this features is being
> worked upon.
> 
> v2:
>  - Incorporated the necessary locking. (Ram)
>  - Set content protection property to CP_DESIRED only when
>    user has not asked explicitly to set CP_UNDESIRED.
> 
> v3:
>  - Reset the is_hdcp_undesired flag to false. (Ram)
>  - Rephrasing commit log and small comment for is_hdcp_desired
>    flag. (Ram)
> 
> CC: Ramalingam C <ramalingam.c@intel.com>
> Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_types.h |  6 ++++++
>  drivers/gpu/drm/i915/display/intel_hdcp.c          | 13 ++++++++++++-
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 630a94892b7b..401a9a7689fb 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -345,6 +345,12 @@ struct intel_hdcp {
>  	struct delayed_work check_work;
>  	struct work_struct prop_work;
>  
> +	/*
> +	 * Flag to differentiate that HDCP is being disabled originated from
> +	 * userspace or triggered from kernel DDI disable sequence.
> +	 */
> +	bool is_hdcp_undesired;
Jani and Daniel,

This flag is added as we need to know the origin of the HDCP disable
(userspace or kernel modeset/DPMS off) at DDI disable sequence. We
couldn't do that as new_conn state is not available there to retrieve
the corresponding content protection state.

Hence we do that at atomic check itself and pass the info through this flag,
which will be referred at hdcp_disable.

If you think we could do it better please suggest the preferred
alternate method. Else I request your ack for merging this.

Thanks,
-Ram
> +
>  	/* HDCP1.4 Encryption status */
>  	bool hdcp_encrypted;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 0fdbd39f6641..7f631ebd8395 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -2002,11 +2002,18 @@ int intel_hdcp_disable(struct intel_connector *connector)
>  	mutex_lock(&hdcp->mutex);
>  
>  	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> -		hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
>  		if (hdcp->hdcp2_encrypted)
>  			ret = _intel_hdcp2_disable(connector);
>  		else if (hdcp->hdcp_encrypted)
>  			ret = _intel_hdcp_disable(connector);
> +
> +		if (hdcp->is_hdcp_undesired) {
> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
> +			hdcp->is_hdcp_undesired = false;
> +		} else {
> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
> +			schedule_work(&hdcp->prop_work);
> +		}
>  	}
>  
>  	mutex_unlock(&hdcp->mutex);
> @@ -2044,6 +2051,7 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>  {
>  	u64 old_cp = old_state->content_protection;
>  	u64 new_cp = new_state->content_protection;
> +	struct intel_connector *intel_conn = to_intel_connector(connector);
>  	struct drm_crtc_state *crtc_state;
>  
>  	if (!new_state->crtc) {
> @@ -2069,6 +2077,9 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>  			return;
>  	}
>  
> +	if (new_cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
> +		intel_conn->hdcp.is_hdcp_undesired  =  true;
> +
>  	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
>  						   new_state->crtc);
>  	crtc_state->mode_changed = true;
> -- 
> 2.24.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Jani Nikula]

On Mon, 20 Jan 2020, Ramalingam C <ramalingam.c@intel.com> wrote:
> On 2020-01-20 at 11:19:54 +0530, Anshuman Gupta wrote:
>> Content Protection property should be updated as per the kernel
>> internal state. Let's say if Content protection is disabled
>> by userspace, CP property should be set to UNDESIRED so that
>> reauthentication will not happen until userspace request it again,
>> but when kernel disables the HDCP due to any DDI disabling sequences
>> like modeset/DPMS operation, kernel should set the property to
>> DESIRED, so that when opportunity arises, kernel will start the
>> HDCP authentication on its own.
>> 
>> Somewhere in the line, state machine to set content protection to
>> DESIRED from kernel was broken and IGT coverage was missing for it.
>> This patch fixes it.
>> IGT patch to catch further regression on this features is being
>> worked upon.
>> 
>> v2:
>>  - Incorporated the necessary locking. (Ram)
>>  - Set content protection property to CP_DESIRED only when
>>    user has not asked explicitly to set CP_UNDESIRED.
>> 
>> v3:
>>  - Reset the is_hdcp_undesired flag to false. (Ram)
>>  - Rephrasing commit log and small comment for is_hdcp_desired
>>    flag. (Ram)
>> 
>> CC: Ramalingam C <ramalingam.c@intel.com>
>> Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
>> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_display_types.h |  6 ++++++
>>  drivers/gpu/drm/i915/display/intel_hdcp.c          | 13 ++++++++++++-
>>  2 files changed, 18 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index 630a94892b7b..401a9a7689fb 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -345,6 +345,12 @@ struct intel_hdcp {
>>  	struct delayed_work check_work;
>>  	struct work_struct prop_work;
>>  
>> +	/*
>> +	 * Flag to differentiate that HDCP is being disabled originated from
>> +	 * userspace or triggered from kernel DDI disable sequence.
>> +	 */
>> +	bool is_hdcp_undesired;
> Jani and Daniel,
>
> This flag is added as we need to know the origin of the HDCP disable
> (userspace or kernel modeset/DPMS off) at DDI disable sequence. We
> couldn't do that as new_conn state is not available there to retrieve
> the corresponding content protection state.
>
> Hence we do that at atomic check itself and pass the info through this flag,
> which will be referred at hdcp_disable.
>
> If you think we could do it better please suggest the preferred
> alternate method. Else I request your ack for merging this.

I don't know hdcp code all that well, but it seems to me at the root of
the problem is the duplication of the content protection state
(drm_connector_state->content_protection) into the connector
(intel_hdcp->value), and them going out of sync.

If you relied on the connector state alone, you wouldn't have to worry
about changing the intel_hdcp->value member at disable or anywhere;
disable looks at old state and disables based on that. No history/future
information needed. Isn't that roughly what everything else does, why is
hdcp special?

BR,
Jani.


>
> Thanks,
> -Ram
>> +
>>  	/* HDCP1.4 Encryption status */
>>  	bool hdcp_encrypted;
>>  
>> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
>> index 0fdbd39f6641..7f631ebd8395 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
>> @@ -2002,11 +2002,18 @@ int intel_hdcp_disable(struct intel_connector *connector)
>>  	mutex_lock(&hdcp->mutex);
>>  
>>  	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
>> -		hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
>>  		if (hdcp->hdcp2_encrypted)
>>  			ret = _intel_hdcp2_disable(connector);
>>  		else if (hdcp->hdcp_encrypted)
>>  			ret = _intel_hdcp_disable(connector);
>> +
>> +		if (hdcp->is_hdcp_undesired) {
>> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
>> +			hdcp->is_hdcp_undesired = false;
>> +		} else {
>> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
>> +			schedule_work(&hdcp->prop_work);
>> +		}
>>  	}
>>  
>>  	mutex_unlock(&hdcp->mutex);
>> @@ -2044,6 +2051,7 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>>  {
>>  	u64 old_cp = old_state->content_protection;
>>  	u64 new_cp = new_state->content_protection;
>> +	struct intel_connector *intel_conn = to_intel_connector(connector);
>>  	struct drm_crtc_state *crtc_state;
>>  
>>  	if (!new_state->crtc) {
>> @@ -2069,6 +2077,9 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>>  			return;
>>  	}
>>  
>> +	if (new_cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
>> +		intel_conn->hdcp.is_hdcp_undesired  =  true;
>> +
>>  	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
>>  						   new_state->crtc);
>>  	crtc_state->mode_changed = true;
>> -- 
>> 2.24.0
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Ramalingam C]

On 2020-01-20 at 12:29:36 +0200, Jani Nikula wrote:
> On Mon, 20 Jan 2020, Ramalingam C <ramalingam.c@intel.com> wrote:
> > On 2020-01-20 at 11:19:54 +0530, Anshuman Gupta wrote:
> >> Content Protection property should be updated as per the kernel
> >> internal state. Let's say if Content protection is disabled
> >> by userspace, CP property should be set to UNDESIRED so that
> >> reauthentication will not happen until userspace request it again,
> >> but when kernel disables the HDCP due to any DDI disabling sequences
> >> like modeset/DPMS operation, kernel should set the property to
> >> DESIRED, so that when opportunity arises, kernel will start the
> >> HDCP authentication on its own.
> >> 
> >> Somewhere in the line, state machine to set content protection to
> >> DESIRED from kernel was broken and IGT coverage was missing for it.
> >> This patch fixes it.
> >> IGT patch to catch further regression on this features is being
> >> worked upon.
> >> 
> >> v2:
> >>  - Incorporated the necessary locking. (Ram)
> >>  - Set content protection property to CP_DESIRED only when
> >>    user has not asked explicitly to set CP_UNDESIRED.
> >> 
> >> v3:
> >>  - Reset the is_hdcp_undesired flag to false. (Ram)
> >>  - Rephrasing commit log and small comment for is_hdcp_desired
> >>    flag. (Ram)
> >> 
> >> CC: Ramalingam C <ramalingam.c@intel.com>
> >> Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
> >> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/display/intel_display_types.h |  6 ++++++
> >>  drivers/gpu/drm/i915/display/intel_hdcp.c          | 13 ++++++++++++-
> >>  2 files changed, 18 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> >> index 630a94892b7b..401a9a7689fb 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> >> @@ -345,6 +345,12 @@ struct intel_hdcp {
> >>  	struct delayed_work check_work;
> >>  	struct work_struct prop_work;
> >>  
> >> +	/*
> >> +	 * Flag to differentiate that HDCP is being disabled originated from
> >> +	 * userspace or triggered from kernel DDI disable sequence.
> >> +	 */
> >> +	bool is_hdcp_undesired;
> > Jani and Daniel,
> >
> > This flag is added as we need to know the origin of the HDCP disable
> > (userspace or kernel modeset/DPMS off) at DDI disable sequence. We
> > couldn't do that as new_conn state is not available there to retrieve
> > the corresponding content protection state.
> >
> > Hence we do that at atomic check itself and pass the info through this flag,
> > which will be referred at hdcp_disable.
> >
> > If you think we could do it better please suggest the preferred
> > alternate method. Else I request your ack for merging this.
> 
> I don't know hdcp code all that well, but it seems to me at the root of
> the problem is the duplication of the content protection state
> (drm_connector_state->content_protection) into the connector
> (intel_hdcp->value), and them going out of sync.
> 
> If you relied on the connector state alone, you wouldn't have to worry
> about changing the intel_hdcp->value member at disable or anywhere;
> disable looks at old state and disables based on that. No history/future
> information needed. Isn't that roughly what everything else does, why is
> hdcp special?
As per uAPI designed for Chrome(first user of HDCP), after the userspace
request for HDCP by setting DESIRED to "content protection" only userspace can DISABLED it.

Incase when HDCP is ENABLED and kernel ended up in disabled the DDI (hot
unplug or DPMS ops), it supposed to disable HDCP encryption and leave the
"Content protection" at DESIRED. So that when the DDI is re enabled,
HDCP authentication will be attempted automatically, as userspace is
still interested in HDCP encryption.

So to set the state of "content protection" as  ENABLED->DESIRED, we
need to know the HDCP disable in DDI disable is not because of userspace request
(derived based on new connector state existance and its content protection state).

Hence we need this change.

-Ram
> 
> BR,
> Jani.
> 
> 
> >
> > Thanks,
> > -Ram
> >> +
> >>  	/* HDCP1.4 Encryption status */
> >>  	bool hdcp_encrypted;
> >>  
> >> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> >> index 0fdbd39f6641..7f631ebd8395 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> >> @@ -2002,11 +2002,18 @@ int intel_hdcp_disable(struct intel_connector *connector)
> >>  	mutex_lock(&hdcp->mutex);
> >>  
> >>  	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> >> -		hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
> >>  		if (hdcp->hdcp2_encrypted)
> >>  			ret = _intel_hdcp2_disable(connector);
> >>  		else if (hdcp->hdcp_encrypted)
> >>  			ret = _intel_hdcp_disable(connector);
> >> +
> >> +		if (hdcp->is_hdcp_undesired) {
> >> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
> >> +			hdcp->is_hdcp_undesired = false;
> >> +		} else {
> >> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
> >> +			schedule_work(&hdcp->prop_work);
> >> +		}
> >>  	}
> >>  
> >>  	mutex_unlock(&hdcp->mutex);
> >> @@ -2044,6 +2051,7 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
> >>  {
> >>  	u64 old_cp = old_state->content_protection;
> >>  	u64 new_cp = new_state->content_protection;
> >> +	struct intel_connector *intel_conn = to_intel_connector(connector);
> >>  	struct drm_crtc_state *crtc_state;
> >>  
> >>  	if (!new_state->crtc) {
> >> @@ -2069,6 +2077,9 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
> >>  			return;
> >>  	}
> >>  
> >> +	if (new_cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
> >> +		intel_conn->hdcp.is_hdcp_undesired  =  true;
> >> +
> >>  	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> >>  						   new_state->crtc);
> >>  	crtc_state->mode_changed = true;
> >> -- 
> >> 2.24.0
> >> 
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Jani Nikula]

On Mon, 20 Jan 2020, Ramalingam C <ramalingam.c@intel.com> wrote:
> On 2020-01-20 at 12:29:36 +0200, Jani Nikula wrote:
>> On Mon, 20 Jan 2020, Ramalingam C <ramalingam.c@intel.com> wrote:
>> > On 2020-01-20 at 11:19:54 +0530, Anshuman Gupta wrote:
>> >> Content Protection property should be updated as per the kernel
>> >> internal state. Let's say if Content protection is disabled
>> >> by userspace, CP property should be set to UNDESIRED so that
>> >> reauthentication will not happen until userspace request it again,
>> >> but when kernel disables the HDCP due to any DDI disabling sequences
>> >> like modeset/DPMS operation, kernel should set the property to
>> >> DESIRED, so that when opportunity arises, kernel will start the
>> >> HDCP authentication on its own.
>> >> 
>> >> Somewhere in the line, state machine to set content protection to
>> >> DESIRED from kernel was broken and IGT coverage was missing for it.
>> >> This patch fixes it.
>> >> IGT patch to catch further regression on this features is being
>> >> worked upon.
>> >> 
>> >> v2:
>> >>  - Incorporated the necessary locking. (Ram)
>> >>  - Set content protection property to CP_DESIRED only when
>> >>    user has not asked explicitly to set CP_UNDESIRED.
>> >> 
>> >> v3:
>> >>  - Reset the is_hdcp_undesired flag to false. (Ram)
>> >>  - Rephrasing commit log and small comment for is_hdcp_desired
>> >>    flag. (Ram)
>> >> 
>> >> CC: Ramalingam C <ramalingam.c@intel.com>
>> >> Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
>> >> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
>> >> ---
>> >>  drivers/gpu/drm/i915/display/intel_display_types.h |  6 ++++++
>> >>  drivers/gpu/drm/i915/display/intel_hdcp.c          | 13 ++++++++++++-
>> >>  2 files changed, 18 insertions(+), 1 deletion(-)
>> >> 
>> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
>> >> index 630a94892b7b..401a9a7689fb 100644
>> >> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> >> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> >> @@ -345,6 +345,12 @@ struct intel_hdcp {
>> >>  	struct delayed_work check_work;
>> >>  	struct work_struct prop_work;
>> >>  
>> >> +	/*
>> >> +	 * Flag to differentiate that HDCP is being disabled originated from
>> >> +	 * userspace or triggered from kernel DDI disable sequence.
>> >> +	 */
>> >> +	bool is_hdcp_undesired;
>> > Jani and Daniel,
>> >
>> > This flag is added as we need to know the origin of the HDCP disable
>> > (userspace or kernel modeset/DPMS off) at DDI disable sequence. We
>> > couldn't do that as new_conn state is not available there to retrieve
>> > the corresponding content protection state.
>> >
>> > Hence we do that at atomic check itself and pass the info through this flag,
>> > which will be referred at hdcp_disable.
>> >
>> > If you think we could do it better please suggest the preferred
>> > alternate method. Else I request your ack for merging this.
>> 
>> I don't know hdcp code all that well, but it seems to me at the root of
>> the problem is the duplication of the content protection state
>> (drm_connector_state->content_protection) into the connector
>> (intel_hdcp->value), and them going out of sync.
>> 
>> If you relied on the connector state alone, you wouldn't have to worry
>> about changing the intel_hdcp->value member at disable or anywhere;
>> disable looks at old state and disables based on that. No history/future
>> information needed. Isn't that roughly what everything else does, why is
>> hdcp special?
> As per uAPI designed for Chrome(first user of HDCP), after the userspace
> request for HDCP by setting DESIRED to "content protection" only userspace can DISABLED it.
>
> Incase when HDCP is ENABLED and kernel ended up in disabled the DDI (hot
> unplug or DPMS ops), it supposed to disable HDCP encryption and leave the
> "Content protection" at DESIRED. So that when the DDI is re enabled,
> HDCP authentication will be attempted automatically, as userspace is
> still interested in HDCP encryption.
>
> So to set the state of "content protection" as  ENABLED->DESIRED, we
> need to know the HDCP disable in DDI disable is not because of userspace request
> (derived based on new connector state existance and its content protection state).
>
> Hence we need this change.

Okay, why do you need to track desired/undesired in intel_hdcp then?
Just track enabled/disabled, and do everything else based on connector
state?

BR,
Jani.


>
> -Ram
>> 
>> BR,
>> Jani.
>> 
>> 
>> >
>> > Thanks,
>> > -Ram
>> >> +
>> >>  	/* HDCP1.4 Encryption status */
>> >>  	bool hdcp_encrypted;
>> >>  
>> >> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
>> >> index 0fdbd39f6641..7f631ebd8395 100644
>> >> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
>> >> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
>> >> @@ -2002,11 +2002,18 @@ int intel_hdcp_disable(struct intel_connector *connector)
>> >>  	mutex_lock(&hdcp->mutex);
>> >>  
>> >>  	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
>> >> -		hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
>> >>  		if (hdcp->hdcp2_encrypted)
>> >>  			ret = _intel_hdcp2_disable(connector);
>> >>  		else if (hdcp->hdcp_encrypted)
>> >>  			ret = _intel_hdcp_disable(connector);
>> >> +
>> >> +		if (hdcp->is_hdcp_undesired) {
>> >> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
>> >> +			hdcp->is_hdcp_undesired = false;
>> >> +		} else {
>> >> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
>> >> +			schedule_work(&hdcp->prop_work);
>> >> +		}
>> >>  	}
>> >>  
>> >>  	mutex_unlock(&hdcp->mutex);
>> >> @@ -2044,6 +2051,7 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>> >>  {
>> >>  	u64 old_cp = old_state->content_protection;
>> >>  	u64 new_cp = new_state->content_protection;
>> >> +	struct intel_connector *intel_conn = to_intel_connector(connector);
>> >>  	struct drm_crtc_state *crtc_state;
>> >>  
>> >>  	if (!new_state->crtc) {
>> >> @@ -2069,6 +2077,9 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>> >>  			return;
>> >>  	}
>> >>  
>> >> +	if (new_cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
>> >> +		intel_conn->hdcp.is_hdcp_undesired  =  true;
>> >> +
>> >>  	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
>> >>  						   new_state->crtc);
>> >>  	crtc_state->mode_changed = true;
>> >> -- 
>> >> 2.24.0
>> >> 
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Ramalingam C]

On 2020-01-20 at 13:24:05 +0200, Jani Nikula wrote:
> On Mon, 20 Jan 2020, Ramalingam C <ramalingam.c@intel.com> wrote:
> > On 2020-01-20 at 12:29:36 +0200, Jani Nikula wrote:
> >> On Mon, 20 Jan 2020, Ramalingam C <ramalingam.c@intel.com> wrote:
> >> > On 2020-01-20 at 11:19:54 +0530, Anshuman Gupta wrote:
> >> >> Content Protection property should be updated as per the kernel
> >> >> internal state. Let's say if Content protection is disabled
> >> >> by userspace, CP property should be set to UNDESIRED so that
> >> >> reauthentication will not happen until userspace request it again,
> >> >> but when kernel disables the HDCP due to any DDI disabling sequences
> >> >> like modeset/DPMS operation, kernel should set the property to
> >> >> DESIRED, so that when opportunity arises, kernel will start the
> >> >> HDCP authentication on its own.
> >> >> 
> >> >> Somewhere in the line, state machine to set content protection to
> >> >> DESIRED from kernel was broken and IGT coverage was missing for it.
> >> >> This patch fixes it.
> >> >> IGT patch to catch further regression on this features is being
> >> >> worked upon.
> >> >> 
> >> >> v2:
> >> >>  - Incorporated the necessary locking. (Ram)
> >> >>  - Set content protection property to CP_DESIRED only when
> >> >>    user has not asked explicitly to set CP_UNDESIRED.
> >> >> 
> >> >> v3:
> >> >>  - Reset the is_hdcp_undesired flag to false. (Ram)
> >> >>  - Rephrasing commit log and small comment for is_hdcp_desired
> >> >>    flag. (Ram)
> >> >> 
> >> >> CC: Ramalingam C <ramalingam.c@intel.com>
> >> >> Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
> >> >> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> >> >> ---
> >> >>  drivers/gpu/drm/i915/display/intel_display_types.h |  6 ++++++
> >> >>  drivers/gpu/drm/i915/display/intel_hdcp.c          | 13 ++++++++++++-
> >> >>  2 files changed, 18 insertions(+), 1 deletion(-)
> >> >> 
> >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> >> >> index 630a94892b7b..401a9a7689fb 100644
> >> >> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> >> >> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> >> >> @@ -345,6 +345,12 @@ struct intel_hdcp {
> >> >>  	struct delayed_work check_work;
> >> >>  	struct work_struct prop_work;
> >> >>  
> >> >> +	/*
> >> >> +	 * Flag to differentiate that HDCP is being disabled originated from
> >> >> +	 * userspace or triggered from kernel DDI disable sequence.
> >> >> +	 */
> >> >> +	bool is_hdcp_undesired;
> >> > Jani and Daniel,
> >> >
> >> > This flag is added as we need to know the origin of the HDCP disable
> >> > (userspace or kernel modeset/DPMS off) at DDI disable sequence. We
> >> > couldn't do that as new_conn state is not available there to retrieve
> >> > the corresponding content protection state.
> >> >
> >> > Hence we do that at atomic check itself and pass the info through this flag,
> >> > which will be referred at hdcp_disable.
> >> >
> >> > If you think we could do it better please suggest the preferred
> >> > alternate method. Else I request your ack for merging this.
> >> 
> >> I don't know hdcp code all that well, but it seems to me at the root of
> >> the problem is the duplication of the content protection state
> >> (drm_connector_state->content_protection) into the connector
> >> (intel_hdcp->value), and them going out of sync.
> >> 
> >> If you relied on the connector state alone, you wouldn't have to worry
> >> about changing the intel_hdcp->value member at disable or anywhere;
> >> disable looks at old state and disables based on that. No history/future
> >> information needed. Isn't that roughly what everything else does, why is
> >> hdcp special?
> > As per uAPI designed for Chrome(first user of HDCP), after the userspace
> > request for HDCP by setting DESIRED to "content protection" only userspace can DISABLED it.
> >
> > Incase when HDCP is ENABLED and kernel ended up in disabled the DDI (hot
> > unplug or DPMS ops), it supposed to disable HDCP encryption and leave the
> > "Content protection" at DESIRED. So that when the DDI is re enabled,
> > HDCP authentication will be attempted automatically, as userspace is
> > still interested in HDCP encryption.
> >
> > So to set the state of "content protection" as  ENABLED->DESIRED, we
> > need to know the HDCP disable in DDI disable is not because of userspace request
> > (derived based on new connector state existance and its content protection state).
> >
> > Hence we need this change.
> 
> Okay, why do you need to track desired/undesired in intel_hdcp then?
> Just track enabled/disabled, and do everything else based on connector
> state?
Jani,

hdcp->value is used to track the internal transistions during the link
failure and re-establishing it. When ever kernel want to update the
"content protection" we take the required locks and update the property
state along with uevent.

-Ram
> 
> BR,
> Jani.
> 
> 
> >
> > -Ram
> >> 
> >> BR,
> >> Jani.
> >> 
> >> 
> >> >
> >> > Thanks,
> >> > -Ram
> >> >> +
> >> >>  	/* HDCP1.4 Encryption status */
> >> >>  	bool hdcp_encrypted;
> >> >>  
> >> >> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> >> >> index 0fdbd39f6641..7f631ebd8395 100644
> >> >> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> >> >> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> >> >> @@ -2002,11 +2002,18 @@ int intel_hdcp_disable(struct intel_connector *connector)
> >> >>  	mutex_lock(&hdcp->mutex);
> >> >>  
> >> >>  	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> >> >> -		hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
> >> >>  		if (hdcp->hdcp2_encrypted)
> >> >>  			ret = _intel_hdcp2_disable(connector);
> >> >>  		else if (hdcp->hdcp_encrypted)
> >> >>  			ret = _intel_hdcp_disable(connector);
> >> >> +
> >> >> +		if (hdcp->is_hdcp_undesired) {
> >> >> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
> >> >> +			hdcp->is_hdcp_undesired = false;
> >> >> +		} else {
> >> >> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
> >> >> +			schedule_work(&hdcp->prop_work);
> >> >> +		}
> >> >>  	}
> >> >>  
> >> >>  	mutex_unlock(&hdcp->mutex);
> >> >> @@ -2044,6 +2051,7 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
> >> >>  {
> >> >>  	u64 old_cp = old_state->content_protection;
> >> >>  	u64 new_cp = new_state->content_protection;
> >> >> +	struct intel_connector *intel_conn = to_intel_connector(connector);
> >> >>  	struct drm_crtc_state *crtc_state;
> >> >>  
> >> >>  	if (!new_state->crtc) {
> >> >> @@ -2069,6 +2077,9 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
> >> >>  			return;
> >> >>  	}
> >> >>  
> >> >> +	if (new_cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
> >> >> +		intel_conn->hdcp.is_hdcp_undesired  =  true;
> >> >> +
> >> >>  	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> >> >>  						   new_state->crtc);
> >> >>  	crtc_state->mode_changed = true;
> >> >> -- 
> >> >> 2.24.0
> >> >> 
> >> 
> >> -- 
> >> Jani Nikula, Intel Open Source Graphics Center
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Jani Nikula]

On Mon, 20 Jan 2020, Ramalingam C <ramalingam.c@intel.com> wrote:
> hdcp->value is used to track the internal transistions during the link
> failure and re-establishing it. When ever kernel want to update the
> "content protection" we take the required locks and update the property
> state along with uevent.

My point is this: How many states does your state machine need?

Considering the tri-state content_protection and tri-state hdcp->value,
you have 9 possible states and 362880 transitions. Add the new flag from
this patch, and you have 18 possible states and 6e15 transitions.

Obviously you don't need or use that many states or transitions, but you
need the code to limit the states and the transitions. You need the
review to ensure any changes take into account all the possible states
and transitions.

I've already noted one possible scenario in the proposed patch where
stuff goes out of sync, and I don't know what's really going to happen.

---

So maybe I don't understand what the hdcp code does, but then maybe you
shouldn't ask me to have a look at it... ;) I'm trying to point out why
I think it's maybe difficult to understand, and why I think adding
another flag might make it more difficult to maintain.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Ramalingam C]

On 2020-01-20 at 15:02:46 +0200, Jani Nikula wrote:
> On Mon, 20 Jan 2020, Ramalingam C <ramalingam.c@intel.com> wrote:
> > hdcp->value is used to track the internal transistions during the link
> > failure and re-establishing it. When ever kernel want to update the
> > "content protection" we take the required locks and update the property
> > state along with uevent.
> 
> My point is this: How many states does your state machine need?
> 
> Considering the tri-state content_protection and tri-state hdcp->value,
> you have 9 possible states and 362880 transitions. Add the new flag from
> this patch, and you have 18 possible states and 6e15 transitions.
> 
> Obviously you don't need or use that many states or transitions, but you
> need the code to limit the states and the transitions. You need the
> review to ensure any changes take into account all the possible states
> and transitions.
Yes. We dont use all those combination. Agreed that we need to review the
changes which impacts the updatation of hdcp state of internal state machine or
property state.

> 
> I've already noted one possible scenario in the proposed patch where
> stuff goes out of sync, and I don't know what's really going to happen.
> 
> ---
> 
> So maybe I don't understand what the hdcp code does, but then maybe you
> shouldn't ask me to have a look at it... ;) I'm trying to point out why
> I think it's maybe difficult to understand, and why I think adding
> another flag might make it more difficult to maintain.

Even we tried alternate solutions like retrieving the new drm_connector
state at DDI disable. We dropped it as that wont be received well. Any
suggestion on alternate approach to get the new connector state at
DDI disable call.

-Ram
> 
> BR,
> Jani.
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/hdcp: Update CP as per the kernel internal state

[Patchwork]

== Series Details ==

Series: drm/i915/hdcp: Update CP as per the kernel internal state
URL   : https://patchwork.freedesktop.org/series/72251/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7775 -> Patchwork_16169
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/index.html

Known issues
------------

  Here are the changes found in Patchwork_16169 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-cfl-guc:         [PASS][1] -> [INCOMPLETE][2] ([i915#505] / [i915#671])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/fi-cfl-guc/igt@i915_module_load@reload-with-fault-injection.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/fi-cfl-guc/igt@i915_module_load@reload-with-fault-injection.html

  * igt@i915_selftest@live_execlists:
    - fi-kbl-soraka:      [PASS][3] -> [DMESG-FAIL][4] ([i915#656])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/fi-kbl-soraka/igt@i915_selftest@live_execlists.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/fi-kbl-soraka/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_mman:
    - fi-bxt-dsi:         [PASS][5] -> [DMESG-WARN][6] ([i915#889]) +23 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/fi-bxt-dsi/igt@i915_selftest@live_mman.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/fi-bxt-dsi/igt@i915_selftest@live_mman.html

  * igt@i915_selftest@live_reset:
    - fi-bxt-dsi:         [PASS][7] -> [DMESG-FAIL][8] ([i915#889]) +7 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/fi-bxt-dsi/igt@i915_selftest@live_reset.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/fi-bxt-dsi/igt@i915_selftest@live_reset.html

  
#### Possible fixes ####

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-skl-6700k2:      [INCOMPLETE][9] ([i915#671]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/fi-skl-6700k2/igt@i915_module_load@reload-with-fault-injection.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/fi-skl-6700k2/igt@i915_module_load@reload-with-fault-injection.html

  
#### Warnings ####

  * igt@i915_pm_rpm@basic-rte:
    - fi-kbl-guc:         [FAIL][11] ([i915#579]) -> [SKIP][12] ([fdo#109271])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/fi-kbl-guc/igt@i915_pm_rpm@basic-rte.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/fi-kbl-guc/igt@i915_pm_rpm@basic-rte.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#505]: https://gitlab.freedesktop.org/drm/intel/issues/505
  [i915#579]: https://gitlab.freedesktop.org/drm/intel/issues/579
  [i915#656]: https://gitlab.freedesktop.org/drm/intel/issues/656
  [i915#671]: https://gitlab.freedesktop.org/drm/intel/issues/671
  [i915#889]: https://gitlab.freedesktop.org/drm/intel/issues/889


Participating hosts (44 -> 39)
------------------------------

  Additional (3): fi-byt-j1900 fi-ivb-3770 fi-elk-e7500 
  Missing    (8): fi-ilk-m540 fi-hsw-4200u fi-hsw-peppy fi-byt-squawks fi-bsw-cyan fi-bwr-2160 fi-gdg-551 fi-bsw-kefka 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7775 -> Patchwork_16169

  CI-20190529: 20190529
  CI_DRM_7775: 18da685e257ffc7a48c249243d5e5c1aa59c5a9c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5372: 0d00a27fbbd4d4a77d24499ea9811e07e65eb0ac @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16169: 75a4e5d0623b67698e557957d39995ee3e48e3d0 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

75a4e5d0623b drm/i915/hdcp: Update CP as per the kernel internal state

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Jani Nikula]

On Mon, 20 Jan 2020, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
> Content Protection property should be updated as per the kernel
> internal state. Let's say if Content protection is disabled
> by userspace, CP property should be set to UNDESIRED so that
> reauthentication will not happen until userspace request it again,
> but when kernel disables the HDCP due to any DDI disabling sequences
> like modeset/DPMS operation, kernel should set the property to
> DESIRED, so that when opportunity arises, kernel will start the
> HDCP authentication on its own.
>
> Somewhere in the line, state machine to set content protection to
> DESIRED from kernel was broken and IGT coverage was missing for it.
> This patch fixes it.
> IGT patch to catch further regression on this features is being
> worked upon.
>
> v2:
>  - Incorporated the necessary locking. (Ram)
>  - Set content protection property to CP_DESIRED only when
>    user has not asked explicitly to set CP_UNDESIRED.
>
> v3:
>  - Reset the is_hdcp_undesired flag to false. (Ram)
>  - Rephrasing commit log and small comment for is_hdcp_desired
>    flag. (Ram)
>
> CC: Ramalingam C <ramalingam.c@intel.com>
> Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_types.h |  6 ++++++
>  drivers/gpu/drm/i915/display/intel_hdcp.c          | 13 ++++++++++++-
>  2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 630a94892b7b..401a9a7689fb 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -345,6 +345,12 @@ struct intel_hdcp {
>  	struct delayed_work check_work;
>  	struct work_struct prop_work;
>  
> +	/*
> +	 * Flag to differentiate that HDCP is being disabled originated from
> +	 * userspace or triggered from kernel DDI disable sequence.
> +	 */
> +	bool is_hdcp_undesired;
> +

First, the comment and the name of the member don't align. The member
name does not explain what it is, and you absolutely need to look at the
comment for it to make sense in code.

>  	/* HDCP1.4 Encryption status */
>  	bool hdcp_encrypted;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 0fdbd39f6641..7f631ebd8395 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -2002,11 +2002,18 @@ int intel_hdcp_disable(struct intel_connector *connector)
>  	mutex_lock(&hdcp->mutex);
>  
>  	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> -		hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
>  		if (hdcp->hdcp2_encrypted)
>  			ret = _intel_hdcp2_disable(connector);
>  		else if (hdcp->hdcp_encrypted)
>  			ret = _intel_hdcp_disable(connector);
> +
> +		if (hdcp->is_hdcp_undesired) {
> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
> +			hdcp->is_hdcp_undesired = false;
> +		} else {
> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
> +			schedule_work(&hdcp->prop_work);
> +		}

Second, all the places you call intel_hdcp_disable() from have access to
the connector state, so IIUC you could deduce the state from there and
pass it to intel_hdcp_disable() without having to add another
flag/sub-state such as ->is_hdcp_undesired.

Third, reading the code, I am not sure I fully appreciate why there is a
duplicated hdcp->value, instead of using the content_protection member
from connector state to begin with. Between those two and the one added
in this patch, looks like there's a lot of state that could go out of
sync.

>  	}
>  
>  	mutex_unlock(&hdcp->mutex);
> @@ -2044,6 +2051,7 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>  {
>  	u64 old_cp = old_state->content_protection;
>  	u64 new_cp = new_state->content_protection;
> +	struct intel_connector *intel_conn = to_intel_connector(connector);
>  	struct drm_crtc_state *crtc_state;
>  
>  	if (!new_state->crtc) {
> @@ -2069,6 +2077,9 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>  			return;
>  	}
>  
> +	if (new_cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
> +		intel_conn->hdcp.is_hdcp_undesired  =  true;
> +

Finally, this underlines the above points. If atomic check fails later
for some unrelated reason, you've still ended up changing the
connector. You'll tell userspace things failed, but mysteriously things
will change anyway. It is almost never a good idea to add more state
variables if you can figure the same thing from the single point of
truth.

BR,
Jani.


>  	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
>  						   new_state->crtc);
>  	crtc_state->mode_changed = true;

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Ramalingam C]

On 2020-01-20 at 11:19:54 +0530, Anshuman Gupta wrote:
> Content Protection property should be updated as per the kernel
> internal state. Let's say if Content protection is disabled
> by userspace, CP property should be set to UNDESIRED so that
> reauthentication will not happen until userspace request it again,
> but when kernel disables the HDCP due to any DDI disabling sequences
> like modeset/DPMS operation, kernel should set the property to
> DESIRED, so that when opportunity arises, kernel will start the
> HDCP authentication on its own.
> 
> Somewhere in the line, state machine to set content protection to
> DESIRED from kernel was broken and IGT coverage was missing for it.
> This patch fixes it.
> IGT patch to catch further regression on this features is being
> worked upon.
> 
> v2:
>  - Incorporated the necessary locking. (Ram)
>  - Set content protection property to CP_DESIRED only when
>    user has not asked explicitly to set CP_UNDESIRED.
> 
> v3:
>  - Reset the is_hdcp_undesired flag to false. (Ram)
>  - Rephrasing commit log and small comment for is_hdcp_desired
>    flag. (Ram)
> 
> CC: Ramalingam C <ramalingam.c@intel.com>
> Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_types.h |  6 ++++++
>  drivers/gpu/drm/i915/display/intel_hdcp.c          | 13 ++++++++++++-
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 630a94892b7b..401a9a7689fb 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -345,6 +345,12 @@ struct intel_hdcp {
>  	struct delayed_work check_work;
>  	struct work_struct prop_work;
>  
> +	/*
> +	 * Flag to differentiate that HDCP is being disabled originated from
> +	 * userspace or triggered from kernel DDI disable sequence.
> +	 */
> +	bool is_hdcp_undesired;
> +
>  	/* HDCP1.4 Encryption status */
>  	bool hdcp_encrypted;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 0fdbd39f6641..7f631ebd8395 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -2002,11 +2002,18 @@ int intel_hdcp_disable(struct intel_connector *connector)
>  	mutex_lock(&hdcp->mutex);
>  
>  	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> -		hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
>  		if (hdcp->hdcp2_encrypted)
>  			ret = _intel_hdcp2_disable(connector);
>  		else if (hdcp->hdcp_encrypted)
>  			ret = _intel_hdcp_disable(connector);
> +
> +		if (hdcp->is_hdcp_undesired) {
> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
> +			hdcp->is_hdcp_undesired = false;
> +		} else {
> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
> +			schedule_work(&hdcp->prop_work);
> +		}
>  	}
>  
>  	mutex_unlock(&hdcp->mutex);
> @@ -2044,6 +2051,7 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>  {
>  	u64 old_cp = old_state->content_protection;
>  	u64 new_cp = new_state->content_protection;
> +	struct intel_connector *intel_conn = to_intel_connector(connector);
>  	struct drm_crtc_state *crtc_state;
>  
>  	if (!new_state->crtc) {
> @@ -2069,6 +2077,9 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>  			return;
>  	}
>  
> +	if (new_cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
> +		intel_conn->hdcp.is_hdcp_undesired  =  true;
This flag is reset at commit only. What if the atomic check failed?
Usually atomic check wont fail for HDCP state change. Possible if it is submitted with other request.
So we need to set true and false both here.

-Ram
> +
>  	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
>  						   new_state->crtc);
>  	crtc_state->mode_changed = true;
> -- 
> 2.24.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Jani Nikula]

On Tue, 21 Jan 2020, Ramalingam C <ramalingam.c@intel.com> wrote:
> On 2020-01-20 at 11:19:54 +0530, Anshuman Gupta wrote:
>> Content Protection property should be updated as per the kernel
>> internal state. Let's say if Content protection is disabled
>> by userspace, CP property should be set to UNDESIRED so that
>> reauthentication will not happen until userspace request it again,
>> but when kernel disables the HDCP due to any DDI disabling sequences
>> like modeset/DPMS operation, kernel should set the property to
>> DESIRED, so that when opportunity arises, kernel will start the
>> HDCP authentication on its own.
>> 
>> Somewhere in the line, state machine to set content protection to
>> DESIRED from kernel was broken and IGT coverage was missing for it.
>> This patch fixes it.
>> IGT patch to catch further regression on this features is being
>> worked upon.
>> 
>> v2:
>>  - Incorporated the necessary locking. (Ram)
>>  - Set content protection property to CP_DESIRED only when
>>    user has not asked explicitly to set CP_UNDESIRED.
>> 
>> v3:
>>  - Reset the is_hdcp_undesired flag to false. (Ram)
>>  - Rephrasing commit log and small comment for is_hdcp_desired
>>    flag. (Ram)
>> 
>> CC: Ramalingam C <ramalingam.c@intel.com>
>> Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
>> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_display_types.h |  6 ++++++
>>  drivers/gpu/drm/i915/display/intel_hdcp.c          | 13 ++++++++++++-
>>  2 files changed, 18 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index 630a94892b7b..401a9a7689fb 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -345,6 +345,12 @@ struct intel_hdcp {
>>  	struct delayed_work check_work;
>>  	struct work_struct prop_work;
>>  
>> +	/*
>> +	 * Flag to differentiate that HDCP is being disabled originated from
>> +	 * userspace or triggered from kernel DDI disable sequence.
>> +	 */
>> +	bool is_hdcp_undesired;
>> +
>>  	/* HDCP1.4 Encryption status */
>>  	bool hdcp_encrypted;
>>  
>> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
>> index 0fdbd39f6641..7f631ebd8395 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
>> @@ -2002,11 +2002,18 @@ int intel_hdcp_disable(struct intel_connector *connector)
>>  	mutex_lock(&hdcp->mutex);
>>  
>>  	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
>> -		hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
>>  		if (hdcp->hdcp2_encrypted)
>>  			ret = _intel_hdcp2_disable(connector);
>>  		else if (hdcp->hdcp_encrypted)
>>  			ret = _intel_hdcp_disable(connector);
>> +
>> +		if (hdcp->is_hdcp_undesired) {
>> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
>> +			hdcp->is_hdcp_undesired = false;
>> +		} else {
>> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
>> +			schedule_work(&hdcp->prop_work);
>> +		}
>>  	}
>>  
>>  	mutex_unlock(&hdcp->mutex);
>> @@ -2044,6 +2051,7 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>>  {
>>  	u64 old_cp = old_state->content_protection;
>>  	u64 new_cp = new_state->content_protection;
>> +	struct intel_connector *intel_conn = to_intel_connector(connector);
>>  	struct drm_crtc_state *crtc_state;
>>  
>>  	if (!new_state->crtc) {
>> @@ -2069,6 +2077,9 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>>  			return;
>>  	}
>>  
>> +	if (new_cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
>> +		intel_conn->hdcp.is_hdcp_undesired  =  true;
> This flag is reset at commit only. What if the atomic check failed?
> Usually atomic check wont fail for HDCP state change. Possible if it is submitted with other request.
> So we need to set true and false both here.

As I explained in my other mail, you don't have the information
available at this point about possible later atomic check failures. I
think it's wrong to change the connector at check phase. You'd need to
be able to revert the change back to what it was... and that's exactly
the reason why we have old and new states, so we can just throw away the
new state if the check fails.

BR,
Jani.


>
> -Ram
>> +
>>  	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
>>  						   new_state->crtc);
>>  	crtc_state->mode_changed = true;
>> -- 
>> 2.24.0
>> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Ramalingam C]

On 2020-01-21 at 14:15:57 +0200, Jani Nikula wrote:
> On Tue, 21 Jan 2020, Ramalingam C <ramalingam.c@intel.com> wrote:
> > On 2020-01-20 at 11:19:54 +0530, Anshuman Gupta wrote:
> >> Content Protection property should be updated as per the kernel
> >> internal state. Let's say if Content protection is disabled
> >> by userspace, CP property should be set to UNDESIRED so that
> >> reauthentication will not happen until userspace request it again,
> >> but when kernel disables the HDCP due to any DDI disabling sequences
> >> like modeset/DPMS operation, kernel should set the property to
> >> DESIRED, so that when opportunity arises, kernel will start the
> >> HDCP authentication on its own.
> >> 
> >> Somewhere in the line, state machine to set content protection to
> >> DESIRED from kernel was broken and IGT coverage was missing for it.
> >> This patch fixes it.
> >> IGT patch to catch further regression on this features is being
> >> worked upon.
> >> 
> >> v2:
> >>  - Incorporated the necessary locking. (Ram)
> >>  - Set content protection property to CP_DESIRED only when
> >>    user has not asked explicitly to set CP_UNDESIRED.
> >> 
> >> v3:
> >>  - Reset the is_hdcp_undesired flag to false. (Ram)
> >>  - Rephrasing commit log and small comment for is_hdcp_desired
> >>    flag. (Ram)
> >> 
> >> CC: Ramalingam C <ramalingam.c@intel.com>
> >> Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
> >> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/display/intel_display_types.h |  6 ++++++
> >>  drivers/gpu/drm/i915/display/intel_hdcp.c          | 13 ++++++++++++-
> >>  2 files changed, 18 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> >> index 630a94892b7b..401a9a7689fb 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> >> @@ -345,6 +345,12 @@ struct intel_hdcp {
> >>  	struct delayed_work check_work;
> >>  	struct work_struct prop_work;
> >>  
> >> +	/*
> >> +	 * Flag to differentiate that HDCP is being disabled originated from
> >> +	 * userspace or triggered from kernel DDI disable sequence.
> >> +	 */
> >> +	bool is_hdcp_undesired;
> >> +
> >>  	/* HDCP1.4 Encryption status */
> >>  	bool hdcp_encrypted;
> >>  
> >> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> >> index 0fdbd39f6641..7f631ebd8395 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> >> @@ -2002,11 +2002,18 @@ int intel_hdcp_disable(struct intel_connector *connector)
> >>  	mutex_lock(&hdcp->mutex);
> >>  
> >>  	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> >> -		hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
> >>  		if (hdcp->hdcp2_encrypted)
> >>  			ret = _intel_hdcp2_disable(connector);
> >>  		else if (hdcp->hdcp_encrypted)
> >>  			ret = _intel_hdcp_disable(connector);
> >> +
> >> +		if (hdcp->is_hdcp_undesired) {
> >> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
> >> +			hdcp->is_hdcp_undesired = false;
> >> +		} else {
> >> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
> >> +			schedule_work(&hdcp->prop_work);
> >> +		}
> >>  	}
> >>  
> >>  	mutex_unlock(&hdcp->mutex);
> >> @@ -2044,6 +2051,7 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
> >>  {
> >>  	u64 old_cp = old_state->content_protection;
> >>  	u64 new_cp = new_state->content_protection;
> >> +	struct intel_connector *intel_conn = to_intel_connector(connector);
> >>  	struct drm_crtc_state *crtc_state;
> >>  
> >>  	if (!new_state->crtc) {
> >> @@ -2069,6 +2077,9 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
> >>  			return;
> >>  	}
> >>  
> >> +	if (new_cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
> >> +		intel_conn->hdcp.is_hdcp_undesired  =  true;
> > This flag is reset at commit only. What if the atomic check failed?
> > Usually atomic check wont fail for HDCP state change. Possible if it is submitted with other request.
> > So we need to set true and false both here.
> 
> As I explained in my other mail, you don't have the information
> available at this point about possible later atomic check failures. I
> think it's wrong to change the connector at check phase. You'd need to
> be able to revert the change back to what it was... and that's exactly
> the reason why we have old and new states, so we can just throw away the
> new state if the check fails.
I completely understand that part. And this flag set at atomic_check
will not change the connector state, untill hdcp_disable called at
atomic commit.

I am completely fine if this method is not acceptable. But uAPI documented is
already broken (ENABLED "content protection" will remain DESIRED/ENABLED until userspace
set it to UNDESIRED). So I am just trying to find a solution to it.

Requirement is as follows:
ddi_disable()->hdcp_disable() gets called for two scenarios 
1. userspace setting the "content protection" to UNDESIRED hence
atomic_check will mark it as modeset hence hdcp will be disabled.
2. userspace is not changing the "content protection", but other display
operations are leading to the ddi_disable hence hdcp is getting disabled

We need to differentiate these two cause for hdcp disable at
hdcp_disable so that "content protection" can be set to "DESIRED" from
"ENABLED" at second case. else HDCP would have been disabled but
property will be left at "ENABLED".

In first scenario we dont need to change the "content protection" as the
userspace itself set it to "UNDESIRED". kernel just need to disable the
authentication.

So yes, this broken uAPI needs to be attended, not sure if anyone is
affected by this already. Please throw some light on the possible
direction from this point.

-Ram

> 
> BR,
> Jani.
> 
> 
> >
> > -Ram
> >> +
> >>  	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> >>  						   new_state->crtc);
> >>  	crtc_state->mode_changed = true;
> >> -- 
> >> 2.24.0
> >> 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Jani Nikula]

On Wed, 22 Jan 2020, Ramalingam C <ramalingam.c@intel.com> wrote:
> On 2020-01-21 at 14:15:57 +0200, Jani Nikula wrote:
>> On Tue, 21 Jan 2020, Ramalingam C <ramalingam.c@intel.com> wrote:
>> > On 2020-01-20 at 11:19:54 +0530, Anshuman Gupta wrote:
>> >> Content Protection property should be updated as per the kernel
>> >> internal state. Let's say if Content protection is disabled
>> >> by userspace, CP property should be set to UNDESIRED so that
>> >> reauthentication will not happen until userspace request it again,
>> >> but when kernel disables the HDCP due to any DDI disabling sequences
>> >> like modeset/DPMS operation, kernel should set the property to
>> >> DESIRED, so that when opportunity arises, kernel will start the
>> >> HDCP authentication on its own.
>> >> 
>> >> Somewhere in the line, state machine to set content protection to
>> >> DESIRED from kernel was broken and IGT coverage was missing for it.
>> >> This patch fixes it.
>> >> IGT patch to catch further regression on this features is being
>> >> worked upon.
>> >> 
>> >> v2:
>> >>  - Incorporated the necessary locking. (Ram)
>> >>  - Set content protection property to CP_DESIRED only when
>> >>    user has not asked explicitly to set CP_UNDESIRED.
>> >> 
>> >> v3:
>> >>  - Reset the is_hdcp_undesired flag to false. (Ram)
>> >>  - Rephrasing commit log and small comment for is_hdcp_desired
>> >>    flag. (Ram)
>> >> 
>> >> CC: Ramalingam C <ramalingam.c@intel.com>
>> >> Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
>> >> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
>> >> ---
>> >>  drivers/gpu/drm/i915/display/intel_display_types.h |  6 ++++++
>> >>  drivers/gpu/drm/i915/display/intel_hdcp.c          | 13 ++++++++++++-
>> >>  2 files changed, 18 insertions(+), 1 deletion(-)
>> >> 
>> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
>> >> index 630a94892b7b..401a9a7689fb 100644
>> >> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> >> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> >> @@ -345,6 +345,12 @@ struct intel_hdcp {
>> >>  	struct delayed_work check_work;
>> >>  	struct work_struct prop_work;
>> >>  
>> >> +	/*
>> >> +	 * Flag to differentiate that HDCP is being disabled originated from
>> >> +	 * userspace or triggered from kernel DDI disable sequence.
>> >> +	 */
>> >> +	bool is_hdcp_undesired;
>> >> +
>> >>  	/* HDCP1.4 Encryption status */
>> >>  	bool hdcp_encrypted;
>> >>  
>> >> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
>> >> index 0fdbd39f6641..7f631ebd8395 100644
>> >> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
>> >> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
>> >> @@ -2002,11 +2002,18 @@ int intel_hdcp_disable(struct intel_connector *connector)
>> >>  	mutex_lock(&hdcp->mutex);
>> >>  
>> >>  	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
>> >> -		hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
>> >>  		if (hdcp->hdcp2_encrypted)
>> >>  			ret = _intel_hdcp2_disable(connector);
>> >>  		else if (hdcp->hdcp_encrypted)
>> >>  			ret = _intel_hdcp_disable(connector);
>> >> +
>> >> +		if (hdcp->is_hdcp_undesired) {
>> >> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
>> >> +			hdcp->is_hdcp_undesired = false;
>> >> +		} else {
>> >> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
>> >> +			schedule_work(&hdcp->prop_work);
>> >> +		}
>> >>  	}
>> >>  
>> >>  	mutex_unlock(&hdcp->mutex);
>> >> @@ -2044,6 +2051,7 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>> >>  {
>> >>  	u64 old_cp = old_state->content_protection;
>> >>  	u64 new_cp = new_state->content_protection;
>> >> +	struct intel_connector *intel_conn = to_intel_connector(connector);
>> >>  	struct drm_crtc_state *crtc_state;
>> >>  
>> >>  	if (!new_state->crtc) {
>> >> @@ -2069,6 +2077,9 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>> >>  			return;
>> >>  	}
>> >>  
>> >> +	if (new_cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
>> >> +		intel_conn->hdcp.is_hdcp_undesired  =  true;
>> > This flag is reset at commit only. What if the atomic check failed?
>> > Usually atomic check wont fail for HDCP state change. Possible if it is submitted with other request.
>> > So we need to set true and false both here.
>> 
>> As I explained in my other mail, you don't have the information
>> available at this point about possible later atomic check failures. I
>> think it's wrong to change the connector at check phase. You'd need to
>> be able to revert the change back to what it was... and that's exactly
>> the reason why we have old and new states, so we can just throw away the
>> new state if the check fails.
> I completely understand that part. And this flag set at atomic_check
> will not change the connector state, untill hdcp_disable called at
> atomic commit.
>
> I am completely fine if this method is not acceptable. But uAPI documented is
> already broken (ENABLED "content protection" will remain DESIRED/ENABLED until userspace
> set it to UNDESIRED). So I am just trying to find a solution to it.
>
> Requirement is as follows:
> ddi_disable()->hdcp_disable() gets called for two scenarios 
> 1. userspace setting the "content protection" to UNDESIRED hence
> atomic_check will mark it as modeset hence hdcp will be disabled.
> 2. userspace is not changing the "content protection", but other display
> operations are leading to the ddi_disable hence hdcp is getting disabled
>
> We need to differentiate these two cause for hdcp disable at
> hdcp_disable so that "content protection" can be set to "DESIRED" from
> "ENABLED" at second case. else HDCP would have been disabled but
> property will be left at "ENABLED".
>
> In first scenario we dont need to change the "content protection" as the
> userspace itself set it to "UNDESIRED". kernel just need to disable the
> authentication.
>
> So yes, this broken uAPI needs to be attended, not sure if anyone is
> affected by this already. Please throw some light on the possible
> direction from this point.

I still think a large part of your problems are due to duplicating the
content_protection state to hdcp->value.

You *are* setting new state content_protection to DESIRED at
intel_hdcp_atomic_check() if it was ENABLED. This should work for both
of your scenarios; I think it's only broken because you lose sync with
hdcp->value.

intel_hdcp_disable() should look at old state and disable if hdcp is
enabled.

intel_hdcp_enable() should look at new state and enable if hdcp is
desired.


BR,
Jani.



>
> -Ram
>
>> 
>> BR,
>> Jani.
>> 
>> 
>> >
>> > -Ram
>> >> +
>> >>  	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
>> >>  						   new_state->crtc);
>> >>  	crtc_state->mode_changed = true;
>> >> -- 
>> >> 2.24.0
>> >> 
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Anshuman Gupta]

On 2020-01-22 at 16:56:06 +0200, Jani Nikula wrote:
> On Wed, 22 Jan 2020, Ramalingam C <ramalingam.c@intel.com> wrote:
> > On 2020-01-21 at 14:15:57 +0200, Jani Nikula wrote:
> >> On Tue, 21 Jan 2020, Ramalingam C <ramalingam.c@intel.com> wrote:
> >> > On 2020-01-20 at 11:19:54 +0530, Anshuman Gupta wrote:
> >> >> Content Protection property should be updated as per the kernel
> >> >> internal state. Let's say if Content protection is disabled
> >> >> by userspace, CP property should be set to UNDESIRED so that
> >> >> reauthentication will not happen until userspace request it again,
> >> >> but when kernel disables the HDCP due to any DDI disabling sequences
> >> >> like modeset/DPMS operation, kernel should set the property to
> >> >> DESIRED, so that when opportunity arises, kernel will start the
> >> >> HDCP authentication on its own.
> >> >> 
> >> >> Somewhere in the line, state machine to set content protection to
> >> >> DESIRED from kernel was broken and IGT coverage was missing for it.
> >> >> This patch fixes it.
> >> >> IGT patch to catch further regression on this features is being
> >> >> worked upon.
> >> >> 
> >> >> v2:
> >> >>  - Incorporated the necessary locking. (Ram)
> >> >>  - Set content protection property to CP_DESIRED only when
> >> >>    user has not asked explicitly to set CP_UNDESIRED.
> >> >> 
> >> >> v3:
> >> >>  - Reset the is_hdcp_undesired flag to false. (Ram)
> >> >>  - Rephrasing commit log and small comment for is_hdcp_desired
> >> >>    flag. (Ram)
> >> >> 
> >> >> CC: Ramalingam C <ramalingam.c@intel.com>
> >> >> Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
> >> >> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> >> >> ---
> >> >>  drivers/gpu/drm/i915/display/intel_display_types.h |  6 ++++++
> >> >>  drivers/gpu/drm/i915/display/intel_hdcp.c          | 13 ++++++++++++-
> >> >>  2 files changed, 18 insertions(+), 1 deletion(-)
> >> >> 
> >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> >> >> index 630a94892b7b..401a9a7689fb 100644
> >> >> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> >> >> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> >> >> @@ -345,6 +345,12 @@ struct intel_hdcp {
> >> >>  	struct delayed_work check_work;
> >> >>  	struct work_struct prop_work;
> >> >>  
> >> >> +	/*
> >> >> +	 * Flag to differentiate that HDCP is being disabled originated from
> >> >> +	 * userspace or triggered from kernel DDI disable sequence.
> >> >> +	 */
> >> >> +	bool is_hdcp_undesired;
> >> >> +
> >> >>  	/* HDCP1.4 Encryption status */
> >> >>  	bool hdcp_encrypted;
> >> >>  
> >> >> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> >> >> index 0fdbd39f6641..7f631ebd8395 100644
> >> >> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> >> >> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> >> >> @@ -2002,11 +2002,18 @@ int intel_hdcp_disable(struct intel_connector *connector)
> >> >>  	mutex_lock(&hdcp->mutex);
> >> >>  
> >> >>  	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) {
> >> >> -		hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
> >> >>  		if (hdcp->hdcp2_encrypted)
> >> >>  			ret = _intel_hdcp2_disable(connector);
> >> >>  		else if (hdcp->hdcp_encrypted)
> >> >>  			ret = _intel_hdcp_disable(connector);
> >> >> +
> >> >> +		if (hdcp->is_hdcp_undesired) {
> >> >> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED;
> >> >> +			hdcp->is_hdcp_undesired = false;
> >> >> +		} else {
> >> >> +			hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;
> >> >> +			schedule_work(&hdcp->prop_work);
> >> >> +		}
> >> >>  	}
> >> >>  
> >> >>  	mutex_unlock(&hdcp->mutex);
> >> >> @@ -2044,6 +2051,7 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
> >> >>  {
> >> >>  	u64 old_cp = old_state->content_protection;
> >> >>  	u64 new_cp = new_state->content_protection;
> >> >> +	struct intel_connector *intel_conn = to_intel_connector(connector);
> >> >>  	struct drm_crtc_state *crtc_state;
> >> >>  
> >> >>  	if (!new_state->crtc) {
> >> >> @@ -2069,6 +2077,9 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
> >> >>  			return;
> >> >>  	}
> >> >>  
> >> >> +	if (new_cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
> >> >> +		intel_conn->hdcp.is_hdcp_undesired  =  true;
> >> > This flag is reset at commit only. What if the atomic check failed?
> >> > Usually atomic check wont fail for HDCP state change. Possible if it is submitted with other request.
> >> > So we need to set true and false both here.
> >> 
> >> As I explained in my other mail, you don't have the information
> >> available at this point about possible later atomic check failures. I
> >> think it's wrong to change the connector at check phase. You'd need to
> >> be able to revert the change back to what it was... and that's exactly
> >> the reason why we have old and new states, so we can just throw away the
> >> new state if the check fails.
> > I completely understand that part. And this flag set at atomic_check
> > will not change the connector state, untill hdcp_disable called at
> > atomic commit.
> >
> > I am completely fine if this method is not acceptable. But uAPI documented is
> > already broken (ENABLED "content protection" will remain DESIRED/ENABLED until userspace
> > set it to UNDESIRED). So I am just trying to find a solution to it.
> >
> > Requirement is as follows:
> > ddi_disable()->hdcp_disable() gets called for two scenarios 
> > 1. userspace setting the "content protection" to UNDESIRED hence
> > atomic_check will mark it as modeset hence hdcp will be disabled.
> > 2. userspace is not changing the "content protection", but other display
> > operations are leading to the ddi_disable hence hdcp is getting disabled
> >
> > We need to differentiate these two cause for hdcp disable at
> > hdcp_disable so that "content protection" can be set to "DESIRED" from
> > "ENABLED" at second case. else HDCP would have been disabled but
> > property will be left at "ENABLED".
> >
> > In first scenario we dont need to change the "content protection" as the
> > userspace itself set it to "UNDESIRED". kernel just need to disable the
> > authentication.
> >
> > So yes, this broken uAPI needs to be attended, not sure if anyone is
> > affected by this already. Please throw some light on the possible
> > direction from this point.
> 
> I still think a large part of your problems are due to duplicating the
> content_protection state to hdcp->value.
> 
> You *are* setting new state content_protection to DESIRED at
> intel_hdcp_atomic_check() if it was ENABLED. This should work for both
IMHO we are setting new state to DESRIED if it was ENABLED for disabled connectors.
Earlier i tried similar way in order to fix this issue 

if (needs_modeset(new_crtc_state)) /* check this after fastset check has been done in atomic check */
          for_each_oldnew_connector_in_state(&state->base, connector, old_conn_state,
                                             new_conn_state, i) {
          	if (old_conn_state->content_protection == DRM_MODE_CONTENT_PROTECTION_ENABLED
               	    &&  new_conn_state->content_protection != DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
                	new_conn_state->content_protection = DRM_MODE_CONTENT_PROTECTION_DESIRED;
                }

but the above solution was missing the uevent part, so i discarded it.
As per uapi doc we still need to send the uevent to notify 
if there is change in state from ENABLED -> DESIRED.
drivers/gpu/drm/drm_connector.c +1011 (uapi)
> of your scenarios; I think it's only broken because you lose sync with
> hdcp->value.
I think with above solution fixes broken uapi except uevent part.
I will send a new patch with above solution with FIXME tag for uevent. 
Thanks,
Anshuman Gupta.
> 
> intel_hdcp_disable() should look at old state and disable if hdcp is
> enabled.
> 
> intel_hdcp_enable() should look at new state and enable if hdcp is
> desired.
> 
> 
> BR,
> Jani.
> 
> 
> 
> >
> > -Ram
> >
> >> 
> >> BR,
> >> Jani.
> >> 
> >> 
> >> >
> >> > -Ram
> >> >> +
> >> >>  	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> >> >>  						   new_state->crtc);
> >> >>  	crtc_state->mode_changed = true;
> >> >> -- 
> >> >> 2.24.0
> >> >> 
> >> > _______________________________________________
> >> > Intel-gfx mailing list
> >> > Intel-gfx@lists.freedesktop.org
> >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> 
> >> -- 
> >> Jani Nikula, Intel Open Source Graphics Center
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/hdcp: Update CP as per the kernel internal state

[Patchwork]

== Series Details ==

Series: drm/i915/hdcp: Update CP as per the kernel internal state
URL   : https://patchwork.freedesktop.org/series/72251/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7775_full -> Patchwork_16169_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_16169_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_busy@busy-vcs1:
    - shard-iclb:         [PASS][1] -> [SKIP][2] ([fdo#112080]) +15 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb2/igt@gem_busy@busy-vcs1.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb6/igt@gem_busy@busy-vcs1.html

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-kbl:          [PASS][3] -> [DMESG-WARN][4] ([i915#180]) +7 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-kbl4/igt@gem_ctx_isolation@rcs0-s3.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-kbl2/igt@gem_ctx_isolation@rcs0-s3.html

  * igt@gem_ctx_persistence@vcs1-mixed-process:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([fdo#109276] / [fdo#112080]) +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb2/igt@gem_ctx_persistence@vcs1-mixed-process.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb8/igt@gem_ctx_persistence@vcs1-mixed-process.html

  * igt@gem_ctx_persistence@vecs0-mixed-process:
    - shard-iclb:         [PASS][7] -> [FAIL][8] ([i915#679])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb5/igt@gem_ctx_persistence@vecs0-mixed-process.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb3/igt@gem_ctx_persistence@vecs0-mixed-process.html
    - shard-glk:          [PASS][9] -> [FAIL][10] ([i915#679])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-glk4/igt@gem_ctx_persistence@vecs0-mixed-process.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-glk9/igt@gem_ctx_persistence@vecs0-mixed-process.html

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [PASS][11] -> [SKIP][12] ([fdo#110841])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb6/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb1/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_balancer@hang:
    - shard-iclb:         [PASS][13] -> [INCOMPLETE][14] ([i915#140])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb8/igt@gem_exec_balancer@hang.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb4/igt@gem_exec_balancer@hang.html

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [PASS][15] -> [SKIP][16] ([fdo#110854])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb2/igt@gem_exec_balancer@smoke.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb8/igt@gem_exec_balancer@smoke.html

  * igt@gem_exec_create@madvise:
    - shard-tglb:         [PASS][17] -> [INCOMPLETE][18] ([i915#472]) +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-tglb7/igt@gem_exec_create@madvise.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-tglb6/igt@gem_exec_create@madvise.html

  * igt@gem_exec_gttfill@basic:
    - shard-tglb:         [PASS][19] -> [INCOMPLETE][20] ([fdo#111593] / [i915#472])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-tglb1/igt@gem_exec_gttfill@basic.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-tglb5/igt@gem_exec_gttfill@basic.html

  * igt@gem_exec_schedule@pi-distinct-iova-bsd:
    - shard-iclb:         [PASS][21] -> [SKIP][22] ([i915#677])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb8/igt@gem_exec_schedule@pi-distinct-iova-bsd.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb4/igt@gem_exec_schedule@pi-distinct-iova-bsd.html

  * igt@gem_exec_schedule@preempt-contexts-bsd2:
    - shard-iclb:         [PASS][23] -> [SKIP][24] ([fdo#109276]) +20 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb1/igt@gem_exec_schedule@preempt-contexts-bsd2.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb5/igt@gem_exec_schedule@preempt-contexts-bsd2.html

  * igt@gem_exec_schedule@preemptive-hang-bsd:
    - shard-iclb:         [PASS][25] -> [SKIP][26] ([fdo#112146]) +4 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb6/igt@gem_exec_schedule@preemptive-hang-bsd.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb1/igt@gem_exec_schedule@preemptive-hang-bsd.html

  * igt@gem_persistent_relocs@forked-faulting-reloc-thrashing:
    - shard-kbl:          [PASS][27] -> [INCOMPLETE][28] ([fdo#103665] / [i915#530])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-kbl2/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-kbl7/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html

  * igt@gem_pipe_control_store_loop@reused-buffer:
    - shard-hsw:          [PASS][29] -> [FAIL][30] ([i915#874])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-hsw4/igt@gem_pipe_control_store_loop@reused-buffer.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-hsw7/igt@gem_pipe_control_store_loop@reused-buffer.html

  * igt@gem_ppgtt@flink-and-close-vma-leak:
    - shard-glk:          [PASS][31] -> [FAIL][32] ([i915#644])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-glk2/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-glk3/igt@gem_ppgtt@flink-and-close-vma-leak.html

  * igt@gem_tiled_blits@interruptible:
    - shard-hsw:          [PASS][33] -> [FAIL][34] ([i915#818])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-hsw6/igt@gem_tiled_blits@interruptible.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-hsw5/igt@gem_tiled_blits@interruptible.html

  * igt@i915_pm_dc@dc5-dpms:
    - shard-iclb:         [PASS][35] -> [FAIL][36] ([i915#447])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb5/igt@i915_pm_dc@dc5-dpms.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb3/igt@i915_pm_dc@dc5-dpms.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-iclb:         [PASS][37] -> [FAIL][38] ([i915#454])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb3/igt@i915_pm_dc@dc6-psr.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb4/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_pm_rpm@system-suspend-execbuf:
    - shard-iclb:         [PASS][39] -> [DMESG-WARN][40] ([fdo#111764])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb1/igt@i915_pm_rpm@system-suspend-execbuf.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb2/igt@i915_pm_rpm@system-suspend-execbuf.html

  * igt@i915_selftest@mock_requests:
    - shard-skl:          [PASS][41] -> [INCOMPLETE][42] ([i915#198])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-skl5/igt@i915_selftest@mock_requests.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-skl7/igt@i915_selftest@mock_requests.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [PASS][43] -> [DMESG-WARN][44] ([i915#180]) +3 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-apl3/igt@i915_suspend@fence-restore-tiled2untiled.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-apl4/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_color@pipe-b-ctm-0-5:
    - shard-skl:          [PASS][45] -> [DMESG-WARN][46] ([i915#109])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-skl1/igt@kms_color@pipe-b-ctm-0-5.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-skl6/igt@kms_color@pipe-b-ctm-0-5.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [PASS][47] -> [FAIL][48] ([i915#79])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-skl7/igt@kms_flip@flip-vs-expired-vblank.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-skl6/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-skl:          [PASS][49] -> [INCOMPLETE][50] ([i915#221])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-skl7/igt@kms_flip@flip-vs-suspend.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-skl6/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_flip@plain-flip-fb-recreate:
    - shard-skl:          [PASS][51] -> [FAIL][52] ([i915#34])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-skl4/igt@kms_flip@plain-flip-fb-recreate.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-skl8/igt@kms_flip@plain-flip-fb-recreate.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-blt:
    - shard-tglb:         [PASS][53] -> [FAIL][54] ([i915#49])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-tglb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-blt.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-tglb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-blt.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [PASS][55] -> [FAIL][56] ([fdo#108145])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-skl3/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-skl4/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [PASS][57] -> [FAIL][58] ([fdo#108145] / [i915#265])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-skl6/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-skl3/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_psr@psr2_no_drrs:
    - shard-iclb:         [PASS][59] -> [SKIP][60] ([fdo#109441]) +1 similar issue
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb2/igt@kms_psr@psr2_no_drrs.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb8/igt@kms_psr@psr2_no_drrs.html

  * igt@prime_mmap_coherency@ioctl-errors:
    - shard-hsw:          [PASS][61] -> [INCOMPLETE][62] ([i915#61])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-hsw1/igt@prime_mmap_coherency@ioctl-errors.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-hsw4/igt@prime_mmap_coherency@ioctl-errors.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-apl:          [DMESG-WARN][63] ([i915#180]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-apl6/igt@gem_ctx_isolation@rcs0-s3.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-apl7/igt@gem_ctx_isolation@rcs0-s3.html

  * igt@gem_ctx_isolation@vcs1-dirty-create:
    - shard-iclb:         [SKIP][65] ([fdo#109276] / [fdo#112080]) -> [PASS][66] +3 similar issues
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb6/igt@gem_ctx_isolation@vcs1-dirty-create.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb1/igt@gem_ctx_isolation@vcs1-dirty-create.html

  * igt@gem_ctx_shared@q-smoketest-bsd1:
    - shard-tglb:         [INCOMPLETE][67] ([fdo#111735] / [i915#472]) -> [PASS][68] +1 similar issue
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-tglb6/igt@gem_ctx_shared@q-smoketest-bsd1.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-tglb7/igt@gem_ctx_shared@q-smoketest-bsd1.html

  * igt@gem_eio@in-flight-contexts-10ms:
    - shard-snb:          [DMESG-WARN][69] -> [PASS][70]
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-snb2/igt@gem_eio@in-flight-contexts-10ms.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-snb2/igt@gem_eio@in-flight-contexts-10ms.html

  * igt@gem_eio@kms:
    - shard-snb:          [INCOMPLETE][71] ([i915#82]) -> [PASS][72]
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-snb2/igt@gem_eio@kms.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-snb2/igt@gem_eio@kms.html

  * igt@gem_exec_await@wide-all:
    - shard-tglb:         [INCOMPLETE][73] ([fdo#111736] / [i915#472]) -> [PASS][74]
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-tglb3/igt@gem_exec_await@wide-all.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-tglb5/igt@gem_exec_await@wide-all.html

  * igt@gem_exec_parallel@vcs1-fds:
    - shard-iclb:         [SKIP][75] ([fdo#112080]) -> [PASS][76] +8 similar issues
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb8/igt@gem_exec_parallel@vcs1-fds.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb4/igt@gem_exec_parallel@vcs1-fds.html

  * igt@gem_exec_schedule@in-order-bsd:
    - shard-iclb:         [SKIP][77] ([fdo#112146]) -> [PASS][78] +7 similar issues
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb4/igt@gem_exec_schedule@in-order-bsd.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb7/igt@gem_exec_schedule@in-order-bsd.html

  * igt@gem_exec_schedule@pi-common-bsd:
    - shard-iclb:         [SKIP][79] ([i915#677]) -> [PASS][80] +2 similar issues
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb2/igt@gem_exec_schedule@pi-common-bsd.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb5/igt@gem_exec_schedule@pi-common-bsd.html

  * igt@gem_exec_schedule@preempt-queue-contexts-vebox:
    - shard-tglb:         [INCOMPLETE][81] ([fdo#111677] / [i915#472]) -> [PASS][82]
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-tglb3/igt@gem_exec_schedule@preempt-queue-contexts-vebox.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-tglb4/igt@gem_exec_schedule@preempt-queue-contexts-vebox.html

  * igt@gem_exec_schedule@smoketest-all:
    - shard-tglb:         [INCOMPLETE][83] ([i915#463] / [i915#472]) -> [PASS][84]
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-tglb4/igt@gem_exec_schedule@smoketest-all.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-tglb8/igt@gem_exec_schedule@smoketest-all.html

  * igt@gem_persistent_relocs@forked-faulting-reloc-thrash-inactive:
    - shard-glk:          [INCOMPLETE][85] ([i915#530] / [i915#58] / [k.org#198133]) -> [PASS][86]
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-glk3/igt@gem_persistent_relocs@forked-faulting-reloc-thrash-inactive.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-glk1/igt@gem_persistent_relocs@forked-faulting-reloc-thrash-inactive.html

  * igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing:
    - shard-apl:          [INCOMPLETE][87] ([CI#80] / [fdo#103927] / [i915#530]) -> [PASS][88]
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-apl3/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-apl8/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html
    - shard-skl:          [TIMEOUT][89] ([fdo#112271] / [i915#530]) -> [PASS][90]
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-skl10/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-skl5/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html
    - shard-hsw:          [TIMEOUT][91] ([fdo#112271] / [i915#530]) -> [PASS][92]
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-hsw5/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-hsw8/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html
    - shard-kbl:          [TIMEOUT][93] ([fdo#112271] / [i915#530]) -> [PASS][94]
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-kbl3/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-kbl7/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html

  * igt@gem_persistent_relocs@forked-interruptible-thrash-inactive:
    - shard-tglb:         [TIMEOUT][95] ([fdo#112126] / [fdo#112271] / [i915#530]) -> [PASS][96]
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-tglb7/igt@gem_persistent_relocs@forked-interruptible-thrash-inactive.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-tglb8/igt@gem_persistent_relocs@forked-interruptible-thrash-inactive.html

  * igt@gem_persistent_relocs@forked-thrashing:
    - shard-iclb:         [TIMEOUT][97] ([fdo#112271] / [i915#530]) -> [PASS][98]
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb5/igt@gem_persistent_relocs@forked-thrashing.html
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb1/igt@gem_persistent_relocs@forked-thrashing.html

  * igt@gem_pipe_control_store_loop@reused-buffer:
    - shard-tglb:         [INCOMPLETE][99] ([i915#472] / [i915#707] / [i915#796]) -> [PASS][100]
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-tglb3/igt@gem_pipe_control_store_loop@reused-buffer.html
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-tglb1/igt@gem_pipe_control_store_loop@reused-buffer.html

  * igt@gem_ppgtt@flink-and-close-vma-leak:
    - shard-apl:          [FAIL][101] ([i915#644]) -> [PASS][102]
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-apl2/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-apl1/igt@gem_ppgtt@flink-and-close-vma-leak.html

  * igt@gem_sync@basic-store-all:
    - shard-tglb:         [INCOMPLETE][103] ([i915#472]) -> [PASS][104]
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-tglb8/igt@gem_sync@basic-store-all.html
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-tglb8/igt@gem_sync@basic-store-all.html

  * igt@gen9_exec_parse@allowed-all:
    - shard-glk:          [DMESG-WARN][105] ([i915#716]) -> [PASS][106]
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-glk1/igt@gen9_exec_parse@allowed-all.html
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-glk1/igt@gen9_exec_parse@allowed-all.html
    - shard-kbl:          [DMESG-WARN][107] ([i915#716]) -> [PASS][108]
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-kbl6/igt@gen9_exec_parse@allowed-all.html
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-kbl3/igt@gen9_exec_parse@allowed-all.html

  * igt@i915_pm_rps@waitboost:
    - shard-iclb:         [FAIL][109] ([i915#413]) -> [PASS][110]
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb1/igt@i915_pm_rps@waitboost.html
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb8/igt@i915_pm_rps@waitboost.html

  * igt@i915_selftest@live_blt:
    - shard-hsw:          [DMESG-FAIL][111] ([i915#553] / [i915#725]) -> [PASS][112]
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-hsw6/igt@i915_selftest@live_blt.html
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-hsw4/igt@i915_selftest@live_blt.html

  * igt@i915_selftest@live_gt_timelines:
    - shard-tglb:         [INCOMPLETE][113] ([i915#455] / [i915#472]) -> [PASS][114]
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-tglb6/igt@i915_selftest@live_gt_timelines.html
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-tglb3/igt@i915_selftest@live_gt_timelines.html

  * igt@i915_selftest@mock_requests:
    - shard-apl:          [INCOMPLETE][115] ([fdo#103927]) -> [PASS][116]
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-apl1/igt@i915_selftest@mock_requests.html
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-apl8/igt@i915_selftest@mock_requests.html
    - shard-kbl:          [INCOMPLETE][117] ([fdo#103665]) -> [PASS][118]
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-kbl7/igt@i915_selftest@mock_requests.html
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-kbl6/igt@i915_selftest@mock_requests.html

  * igt@i915_suspend@forcewake:
    - shard-skl:          [INCOMPLETE][119] ([i915#69]) -> [PASS][120]
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-skl8/igt@i915_suspend@forcewake.html
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-skl9/igt@i915_suspend@forcewake.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-kbl:          [DMESG-WARN][121] ([i915#180]) -> [PASS][122] +8 similar issues
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-kbl7/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-kbl6/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-gtt:
    - shard-tglb:         [SKIP][123] ([i915#668]) -> [PASS][124] +3 similar issues
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-gtt.html
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-tglb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-gtt.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt:
    - shard-tglb:         [FAIL][125] ([i915#49]) -> [PASS][126] +6 similar issues
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-tglb4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html

  * igt@kms_psr@no_drrs:
    - shard-iclb:         [FAIL][127] ([i915#173]) -> [PASS][128]
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb1/igt@kms_psr@no_drrs.html
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb6/igt@kms_psr@no_drrs.html

  * igt@kms_setmode@basic:
    - shard-apl:          [FAIL][129] ([i915#31]) -> [PASS][130]
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-apl2/igt@kms_setmode@basic.html
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-apl2/igt@kms_setmode@basic.html

  * igt@prime_vgem@fence-wait-bsd2:
    - shard-iclb:         [SKIP][131] ([fdo#109276]) -> [PASS][132] +20 similar issues
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-iclb6/igt@prime_vgem@fence-wait-bsd2.html
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-iclb1/igt@prime_vgem@fence-wait-bsd2.html

  
#### Warnings ####

  * igt@gem_tiled_blits@normal:
    - shard-hsw:          [FAIL][133] ([i915#818]) -> [FAIL][134] ([i915#694])
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-hsw5/igt@gem_tiled_blits@normal.html
   [134]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-hsw8/igt@gem_tiled_blits@normal.html

  * igt@runner@aborted:
    - shard-kbl:          ([FAIL][135], [FAIL][136], [FAIL][137], [FAIL][138]) ([fdo#103665] / [i915#716] / [i915#873] / [i915#997]) -> ([FAIL][139], [FAIL][140]) ([i915#997])
   [135]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-kbl2/igt@runner@aborted.html
   [136]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-kbl7/igt@runner@aborted.html
   [137]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-kbl6/igt@runner@aborted.html
   [138]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-kbl4/igt@runner@aborted.html
   [139]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-kbl6/igt@runner@aborted.html
   [140]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-kbl7/igt@runner@aborted.html
    - shard-apl:          ([FAIL][141], [FAIL][142], [FAIL][143]) ([i915#873] / [i915#997]) -> ([FAIL][144], [FAIL][145]) ([i915#997])
   [141]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-apl1/igt@runner@aborted.html
   [142]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-apl3/igt@runner@aborted.html
   [143]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-apl1/igt@runner@aborted.html
   [144]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-apl8/igt@runner@aborted.html
   [145]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-apl1/igt@runner@aborted.html
    - shard-glk:          [FAIL][146] ([k.org#202321]) -> ([FAIL][147], [FAIL][148]) ([i915#997] / [k.org#202321])
   [146]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-glk1/igt@runner@aborted.html
   [147]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-glk3/igt@runner@aborted.html
   [148]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-glk7/igt@runner@aborted.html
    - shard-skl:          ([FAIL][149], [FAIL][150]) ([i915#69] / [i915#997]) -> ([FAIL][151], [FAIL][152]) ([i915#69] / [i915#873] / [i915#997])
   [149]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-skl8/igt@runner@aborted.html
   [150]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7775/shard-skl7/igt@runner@aborted.html
   [151]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-skl7/igt@runner@aborted.html
   [152]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/shard-skl7/igt@runner@aborted.html

  
  [CI#80]: https://gitlab.freedesktop.org/gfx-ci/i915-infra/issues/80
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
  [fdo#111593]: https://bugs.freedesktop.org/show_bug.cgi?id=111593
  [fdo#111677]: https://bugs.freedesktop.org/show_bug.cgi?id=111677
  [fdo#111735]: https://bugs.freedesktop.org/show_bug.cgi?id=111735
  [fdo#111736]: https://bugs.freedesktop.org/show_bug.cgi?id=111736
  [fdo#111764]: https://bugs.freedesktop.org/show_bug.cgi?id=111764
  [fdo#112080]: https://bugs.freedesktop.org/show_bug.cgi?id=112080
  [fdo#112126]: https://bugs.freedesktop.org/show_bug.cgi?id=112126
  [fdo#112146]: https://bugs.freedesktop.org/show_bug.cgi?id=112146
  [fdo#112271]: https://bugs.freedesktop.org/show_bug.cgi?id=112271
  [i915#109]: https://gitlab.freedesktop.org/drm/intel/issues/109
  [i915#140]: https://gitlab.freedesktop.org/drm/intel/issues/140
  [i915#173]: https://gitlab.freedesktop.org/drm/intel/issues/173
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
  [i915#221]: https://gitlab.freedesktop.org/drm/intel/issues/221
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#34]: https://gitlab.freedesktop.org/drm/intel/issues/34
  [i915#413]: https://gitlab.freedesktop.org/drm/intel/issues/413
  [i915#447]: https://gitlab.freedesktop.org/drm/intel/issues/447
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#455]: https://gitlab.freedesktop.org/drm/intel/issues/455
  [i915#463]: https://gitlab.freedesktop.org/drm/intel/issues/463
  [i915#472]: https://gitlab.freedesktop.org/drm/intel/issues/472
  [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
  [i915#530]: https://gitlab.freedesktop.org/drm/intel/issues/530
  [i915#553]: https://gitlab.freedesktop.org/drm/intel/issues/553
  [i915#58]: https://gitlab.freedesktop.org/drm/intel/issues/58
  [i915#61]: https://gitlab.freedesktop.org/drm/intel/issues/61
  [i915#644]: https://gitlab.freedesktop.org/drm/intel/issues/644
  [i915#668]: https://gitlab.freedesktop.org/drm/intel/issues/668
  [i915#677]: https://gitlab.freedesktop.org/drm/intel/issues/677
  [i915#679]: https://gitlab.freedesktop.org/drm/intel/issues/679
  [i915#69]: https://gitlab.freedesktop.org/drm/intel/issues/69
  [i915#694]: https://gitlab.freedesktop.org/drm/intel/issues/694
  [i915#707]: https://gitlab.freedesktop.org/drm/intel/issues/707
  [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
  [i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#796]: https://gitlab.freedesktop.org/drm/intel/issues/796
  [i915#818]: https://gitlab.freedesktop.org/drm/intel/issues/818
  [i915#82]: https://gitlab

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16169/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Anshuman Gupta]

Content Protection property should be updated as per the kernel
internal state. Let's say if Content protection is disabled
by userspace, CP property should be set to UNDESIRED so that
reauthentication will not happen until userspace request it again,
but when kernel disables the HDCP due to any DDI disabling sequences
like modeset/DPMS operation, kernel should set the property to
DESIRED, so that when opportunity arises, kernel will start the
HDCP authentication on its own.

Somewhere in the line, state machine to set content protection to
DESIRED from kernel was broken and IGT coverage was missing for it.
This patch fixes it.

v2:
- Fixing hdcp CP state in connector atomic check function
  intel_hdcp_atomic_check(). [Maarten]
  This will require to check hdcp->value in intel_hdcp_update_pipe()
  in order to avoid enabling hdcp, if it was already enabled.

v3:
- Rebased.

Cc: Ramalingam C <ramalingam.c@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
Link: https://patchwork.freedesktop.org/patch/350962/?series=72664&rev=2 #v1
Link: https://patchwork.freedesktop.org/patch/359396/?series=72251&rev=3 #v2
---
 drivers/gpu/drm/i915/display/intel_hdcp.c | 27 +++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 815b054bb167..0d410652e194 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -2086,6 +2086,7 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state,
 		(conn_state->hdcp_content_type != hdcp->content_type &&
 		 conn_state->content_protection !=
 		 DRM_MODE_CONTENT_PROTECTION_UNDESIRED);
+	bool desired_and_not_enabled = false;
 
 	/*
 	 * During the HDCP encryption session if Type change is requested,
@@ -2108,8 +2109,15 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state,
 	}
 
 	if (conn_state->content_protection ==
-	    DRM_MODE_CONTENT_PROTECTION_DESIRED ||
-	    content_protection_type_changed)
+	    DRM_MODE_CONTENT_PROTECTION_DESIRED) {
+		mutex_lock(&hdcp->mutex);
+		/* Avoid enabling hdcp, if it already ENABLED */
+		desired_and_not_enabled =
+			hdcp->value != DRM_MODE_CONTENT_PROTECTION_ENABLED;
+		mutex_unlock(&hdcp->mutex);
+	}
+
+	if (desired_and_not_enabled || content_protection_type_changed)
 		intel_hdcp_enable(connector,
 				  crtc_state->cpu_transcoder,
 				  (u8)conn_state->hdcp_content_type);
@@ -2158,6 +2166,19 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
 		return;
 	}
 
+	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
+						   new_state->crtc);
+	/*
+	 * Fix the HDCP uapi content protection state in case of modeset.
+	 * FIXME: As per HDCP content protection property uapi doc, an uevent()
+	 * need to be sent if there is transition from ENABLED->DESIRED.
+	 */
+	if (drm_atomic_crtc_needs_modeset(crtc_state) &&
+	    (old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
+	    new_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
+		new_state->content_protection =
+			DRM_MODE_CONTENT_PROTECTION_DESIRED;
+
 	/*
 	 * Nothing to do if the state didn't change, or HDCP was activated since
 	 * the last commit. And also no change in hdcp content type.
@@ -2170,8 +2191,6 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
 			return;
 	}
 
-	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
-						   new_state->crtc);
 	crtc_state->mode_changed = true;
 }
 
-- 
2.26.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Jani Nikula]

Uma, is the R-b still valid? It's been a while.

BR,
Jani.


On Tue, 30 Jun 2020, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
> Content Protection property should be updated as per the kernel
> internal state. Let's say if Content protection is disabled
> by userspace, CP property should be set to UNDESIRED so that
> reauthentication will not happen until userspace request it again,
> but when kernel disables the HDCP due to any DDI disabling sequences
> like modeset/DPMS operation, kernel should set the property to
> DESIRED, so that when opportunity arises, kernel will start the
> HDCP authentication on its own.
>
> Somewhere in the line, state machine to set content protection to
> DESIRED from kernel was broken and IGT coverage was missing for it.
> This patch fixes it.
>
> v2:
> - Fixing hdcp CP state in connector atomic check function
>   intel_hdcp_atomic_check(). [Maarten]
>   This will require to check hdcp->value in intel_hdcp_update_pipe()
>   in order to avoid enabling hdcp, if it was already enabled.
>
> v3:
> - Rebased.
>
> Cc: Ramalingam C <ramalingam.c@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> Link: https://patchwork.freedesktop.org/patch/350962/?series=72664&rev=2 #v1
> Link: https://patchwork.freedesktop.org/patch/359396/?series=72251&rev=3 #v2
> ---
>  drivers/gpu/drm/i915/display/intel_hdcp.c | 27 +++++++++++++++++++----
>  1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 815b054bb167..0d410652e194 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -2086,6 +2086,7 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state,
>  		(conn_state->hdcp_content_type != hdcp->content_type &&
>  		 conn_state->content_protection !=
>  		 DRM_MODE_CONTENT_PROTECTION_UNDESIRED);
> +	bool desired_and_not_enabled = false;
>  
>  	/*
>  	 * During the HDCP encryption session if Type change is requested,
> @@ -2108,8 +2109,15 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state,
>  	}
>  
>  	if (conn_state->content_protection ==
> -	    DRM_MODE_CONTENT_PROTECTION_DESIRED ||
> -	    content_protection_type_changed)
> +	    DRM_MODE_CONTENT_PROTECTION_DESIRED) {
> +		mutex_lock(&hdcp->mutex);
> +		/* Avoid enabling hdcp, if it already ENABLED */
> +		desired_and_not_enabled =
> +			hdcp->value != DRM_MODE_CONTENT_PROTECTION_ENABLED;
> +		mutex_unlock(&hdcp->mutex);
> +	}
> +
> +	if (desired_and_not_enabled || content_protection_type_changed)
>  		intel_hdcp_enable(connector,
>  				  crtc_state->cpu_transcoder,
>  				  (u8)conn_state->hdcp_content_type);
> @@ -2158,6 +2166,19 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>  		return;
>  	}
>  
> +	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> +						   new_state->crtc);
> +	/*
> +	 * Fix the HDCP uapi content protection state in case of modeset.
> +	 * FIXME: As per HDCP content protection property uapi doc, an uevent()
> +	 * need to be sent if there is transition from ENABLED->DESIRED.
> +	 */
> +	if (drm_atomic_crtc_needs_modeset(crtc_state) &&
> +	    (old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
> +	    new_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
> +		new_state->content_protection =
> +			DRM_MODE_CONTENT_PROTECTION_DESIRED;
> +
>  	/*
>  	 * Nothing to do if the state didn't change, or HDCP was activated since
>  	 * the last commit. And also no change in hdcp content type.
> @@ -2170,8 +2191,6 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>  			return;
>  	}
>  
> -	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> -						   new_state->crtc);
>  	crtc_state->mode_changed = true;
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Shankar, Uma]

> -----Original Message-----
> From: Jani Nikula <jani.nikula@intel.com>
> Sent: Tuesday, June 30, 2020 3:30 PM
> To: Gupta, Anshuman <anshuman.gupta@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Shankar, Uma <uma.shankar@intel.com>
> Subject: Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel
> internal state
> 
> 
> Uma, is the R-b still valid? It's been a while.

Yeah Jani, the changes look good. Will need a rebase and fresh CI results though.

Regards,
Uma Shankar

> BR,
> Jani.
> 
> 
> On Tue, 30 Jun 2020, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
> > Content Protection property should be updated as per the kernel
> > internal state. Let's say if Content protection is disabled by
> > userspace, CP property should be set to UNDESIRED so that
> > reauthentication will not happen until userspace request it again, but
> > when kernel disables the HDCP due to any DDI disabling sequences like
> > modeset/DPMS operation, kernel should set the property to DESIRED, so
> > that when opportunity arises, kernel will start the HDCP
> > authentication on its own.
> >
> > Somewhere in the line, state machine to set content protection to
> > DESIRED from kernel was broken and IGT coverage was missing for it.
> > This patch fixes it.
> >
> > v2:
> > - Fixing hdcp CP state in connector atomic check function
> >   intel_hdcp_atomic_check(). [Maarten]
> >   This will require to check hdcp->value in intel_hdcp_update_pipe()
> >   in order to avoid enabling hdcp, if it was already enabled.
> >
> > v3:
> > - Rebased.
> >
> > Cc: Ramalingam C <ramalingam.c@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > Link:
> > https://patchwork.freedesktop.org/patch/350962/?series=72664&rev=2 #v1
> > Link:
> > https://patchwork.freedesktop.org/patch/359396/?series=72251&rev=3 #v2
> > ---
> >  drivers/gpu/drm/i915/display/intel_hdcp.c | 27
> > +++++++++++++++++++----
> >  1 file changed, 23 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > index 815b054bb167..0d410652e194 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > @@ -2086,6 +2086,7 @@ void intel_hdcp_update_pipe(struct
> intel_atomic_state *state,
> >  		(conn_state->hdcp_content_type != hdcp->content_type &&
> >  		 conn_state->content_protection !=
> >  		 DRM_MODE_CONTENT_PROTECTION_UNDESIRED);
> > +	bool desired_and_not_enabled = false;
> >
> >  	/*
> >  	 * During the HDCP encryption session if Type change is requested,
> > @@ -2108,8 +2109,15 @@ void intel_hdcp_update_pipe(struct
> intel_atomic_state *state,
> >  	}
> >
> >  	if (conn_state->content_protection ==
> > -	    DRM_MODE_CONTENT_PROTECTION_DESIRED ||
> > -	    content_protection_type_changed)
> > +	    DRM_MODE_CONTENT_PROTECTION_DESIRED) {
> > +		mutex_lock(&hdcp->mutex);
> > +		/* Avoid enabling hdcp, if it already ENABLED */
> > +		desired_and_not_enabled =
> > +			hdcp->value !=
> DRM_MODE_CONTENT_PROTECTION_ENABLED;
> > +		mutex_unlock(&hdcp->mutex);
> > +	}
> > +
> > +	if (desired_and_not_enabled || content_protection_type_changed)
> >  		intel_hdcp_enable(connector,
> >  				  crtc_state->cpu_transcoder,
> >  				  (u8)conn_state->hdcp_content_type);
> > @@ -2158,6 +2166,19 @@ void intel_hdcp_atomic_check(struct
> drm_connector *connector,
> >  		return;
> >  	}
> >
> > +	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> > +						   new_state->crtc);
> > +	/*
> > +	 * Fix the HDCP uapi content protection state in case of modeset.
> > +	 * FIXME: As per HDCP content protection property uapi doc, an uevent()
> > +	 * need to be sent if there is transition from ENABLED->DESIRED.
> > +	 */
> > +	if (drm_atomic_crtc_needs_modeset(crtc_state) &&
> > +	    (old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
> > +	    new_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
> > +		new_state->content_protection =
> > +			DRM_MODE_CONTENT_PROTECTION_DESIRED;
> > +
> >  	/*
> >  	 * Nothing to do if the state didn't change, or HDCP was activated since
> >  	 * the last commit. And also no change in hdcp content type.
> > @@ -2170,8 +2191,6 @@ void intel_hdcp_atomic_check(struct drm_connector
> *connector,
> >  			return;
> >  	}
> >
> > -	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> > -						   new_state->crtc);
> >  	crtc_state->mode_changed = true;
> >  }
> 
> --
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Shankar, Uma]

> > -----Original Message-----
> > From: Jani Nikula <jani.nikula@intel.com>
> > Sent: Tuesday, June 30, 2020 3:30 PM
> > To: Gupta, Anshuman <anshuman.gupta@intel.com>; intel-
> > gfx@lists.freedesktop.org
> > Cc: Shankar, Uma <uma.shankar@intel.com>
> > Subject: Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per
> > the kernel internal state
> >
> >
> > Uma, is the R-b still valid? It's been a while.
> 
> Yeah Jani, the changes look good. Will need a rebase and fresh CI results though.

Seems the CI results are already out and we are clean.

> Regards,
> Uma Shankar
> 
> > BR,
> > Jani.
> >
> >
> > On Tue, 30 Jun 2020, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
> > > Content Protection property should be updated as per the kernel
> > > internal state. Let's say if Content protection is disabled by
> > > userspace, CP property should be set to UNDESIRED so that
> > > reauthentication will not happen until userspace request it again,
> > > but when kernel disables the HDCP due to any DDI disabling sequences
> > > like modeset/DPMS operation, kernel should set the property to
> > > DESIRED, so that when opportunity arises, kernel will start the HDCP
> > > authentication on its own.
> > >
> > > Somewhere in the line, state machine to set content protection to
> > > DESIRED from kernel was broken and IGT coverage was missing for it.
> > > This patch fixes it.
> > >
> > > v2:
> > > - Fixing hdcp CP state in connector atomic check function
> > >   intel_hdcp_atomic_check(). [Maarten]
> > >   This will require to check hdcp->value in intel_hdcp_update_pipe()
> > >   in order to avoid enabling hdcp, if it was already enabled.
> > >
> > > v3:
> > > - Rebased.
> > >
> > > Cc: Ramalingam C <ramalingam.c@intel.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > Link:
> > > https://patchwork.freedesktop.org/patch/350962/?series=72664&rev=2
> > > #v1
> > > Link:
> > > https://patchwork.freedesktop.org/patch/359396/?series=72251&rev=3
> > > #v2
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_hdcp.c | 27
> > > +++++++++++++++++++----
> > >  1 file changed, 23 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > index 815b054bb167..0d410652e194 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > @@ -2086,6 +2086,7 @@ void intel_hdcp_update_pipe(struct
> > intel_atomic_state *state,
> > >  		(conn_state->hdcp_content_type != hdcp->content_type &&
> > >  		 conn_state->content_protection !=
> > >  		 DRM_MODE_CONTENT_PROTECTION_UNDESIRED);
> > > +	bool desired_and_not_enabled = false;
> > >
> > >  	/*
> > >  	 * During the HDCP encryption session if Type change is requested,
> > > @@ -2108,8 +2109,15 @@ void intel_hdcp_update_pipe(struct
> > intel_atomic_state *state,
> > >  	}
> > >
> > >  	if (conn_state->content_protection ==
> > > -	    DRM_MODE_CONTENT_PROTECTION_DESIRED ||
> > > -	    content_protection_type_changed)
> > > +	    DRM_MODE_CONTENT_PROTECTION_DESIRED) {
> > > +		mutex_lock(&hdcp->mutex);
> > > +		/* Avoid enabling hdcp, if it already ENABLED */
> > > +		desired_and_not_enabled =
> > > +			hdcp->value !=
> > DRM_MODE_CONTENT_PROTECTION_ENABLED;
> > > +		mutex_unlock(&hdcp->mutex);
> > > +	}
> > > +
> > > +	if (desired_and_not_enabled || content_protection_type_changed)
> > >  		intel_hdcp_enable(connector,
> > >  				  crtc_state->cpu_transcoder,
> > >  				  (u8)conn_state->hdcp_content_type);
> > > @@ -2158,6 +2166,19 @@ void intel_hdcp_atomic_check(struct
> > drm_connector *connector,
> > >  		return;
> > >  	}
> > >
> > > +	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> > > +						   new_state->crtc);
> > > +	/*
> > > +	 * Fix the HDCP uapi content protection state in case of modeset.
> > > +	 * FIXME: As per HDCP content protection property uapi doc, an uevent()
> > > +	 * need to be sent if there is transition from ENABLED->DESIRED.
> > > +	 */
> > > +	if (drm_atomic_crtc_needs_modeset(crtc_state) &&
> > > +	    (old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
> > > +	    new_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
> > > +		new_state->content_protection =
> > > +			DRM_MODE_CONTENT_PROTECTION_DESIRED;
> > > +
> > >  	/*
> > >  	 * Nothing to do if the state didn't change, or HDCP was activated since
> > >  	 * the last commit. And also no change in hdcp content type.
> > > @@ -2170,8 +2191,6 @@ void intel_hdcp_atomic_check(struct
> > > drm_connector
> > *connector,
> > >  			return;
> > >  	}
> > >
> > > -	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> > > -						   new_state->crtc);
> > >  	crtc_state->mode_changed = true;
> > >  }
> >
> > --
> > Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Anshuman Gupta]

On 2020-07-01 at 13:31:18 +0530, Shankar, Uma wrote:
> > > -----Original Message-----
> > > From: Jani Nikula <jani.nikula@intel.com>
> > > Sent: Tuesday, June 30, 2020 3:30 PM
> > > To: Gupta, Anshuman <anshuman.gupta@intel.com>; intel-
> > > gfx@lists.freedesktop.org
> > > Cc: Shankar, Uma <uma.shankar@intel.com>
> > > Subject: Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per
> > > the kernel internal state
> > >
> > >
> > > Uma, is the R-b still valid? It's been a while.
> > 
> > Yeah Jani, the changes look good. Will need a rebase and fresh CI results though.
> 
> Seems the CI results are already out and we are clean.
Hi Ram ,
CI results are clean for this rebase patch,
Could you please help with merging,
I belive your RB's are valid either-way wrt to your comment.
Thanks,
Anshuman Gupta.

> 
> > Regards,
> > Uma Shankar
> > 
> > > BR,
> > > Jani.
> > >
> > >
> > > On Tue, 30 Jun 2020, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
> > > > Content Protection property should be updated as per the kernel
> > > > internal state. Let's say if Content protection is disabled by
> > > > userspace, CP property should be set to UNDESIRED so that
> > > > reauthentication will not happen until userspace request it again,
> > > > but when kernel disables the HDCP due to any DDI disabling sequences
> > > > like modeset/DPMS operation, kernel should set the property to
> > > > DESIRED, so that when opportunity arises, kernel will start the HDCP
> > > > authentication on its own.
> > > >
> > > > Somewhere in the line, state machine to set content protection to
> > > > DESIRED from kernel was broken and IGT coverage was missing for it.
> > > > This patch fixes it.
> > > >
> > > > v2:
> > > > - Fixing hdcp CP state in connector atomic check function
> > > >   intel_hdcp_atomic_check(). [Maarten]
> > > >   This will require to check hdcp->value in intel_hdcp_update_pipe()
> > > >   in order to avoid enabling hdcp, if it was already enabled.
> > > >
> > > > v3:
> > > > - Rebased.
> > > >
> > > > Cc: Ramalingam C <ramalingam.c@intel.com>
> > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> > > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > > Link:
> > > > https://patchwork.freedesktop.org/patch/350962/?series=72664&rev=2
> > > > #v1
> > > > Link:
> > > > https://patchwork.freedesktop.org/patch/359396/?series=72251&rev=3
> > > > #v2
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_hdcp.c | 27
> > > > +++++++++++++++++++----
> > > >  1 file changed, 23 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > > b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > > index 815b054bb167..0d410652e194 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > > @@ -2086,6 +2086,7 @@ void intel_hdcp_update_pipe(struct
> > > intel_atomic_state *state,
> > > >  		(conn_state->hdcp_content_type != hdcp->content_type &&
> > > >  		 conn_state->content_protection !=
> > > >  		 DRM_MODE_CONTENT_PROTECTION_UNDESIRED);
> > > > +	bool desired_and_not_enabled = false;
> > > >
> > > >  	/*
> > > >  	 * During the HDCP encryption session if Type change is requested,
> > > > @@ -2108,8 +2109,15 @@ void intel_hdcp_update_pipe(struct
> > > intel_atomic_state *state,
> > > >  	}
> > > >
> > > >  	if (conn_state->content_protection ==
> > > > -	    DRM_MODE_CONTENT_PROTECTION_DESIRED ||
> > > > -	    content_protection_type_changed)
> > > > +	    DRM_MODE_CONTENT_PROTECTION_DESIRED) {
> > > > +		mutex_lock(&hdcp->mutex);
> > > > +		/* Avoid enabling hdcp, if it already ENABLED */
> > > > +		desired_and_not_enabled =
> > > > +			hdcp->value !=
> > > DRM_MODE_CONTENT_PROTECTION_ENABLED;
> > > > +		mutex_unlock(&hdcp->mutex);
> > > > +	}
> > > > +
> > > > +	if (desired_and_not_enabled || content_protection_type_changed)
> > > >  		intel_hdcp_enable(connector,
> > > >  				  crtc_state->cpu_transcoder,
> > > >  				  (u8)conn_state->hdcp_content_type);
> > > > @@ -2158,6 +2166,19 @@ void intel_hdcp_atomic_check(struct
> > > drm_connector *connector,
> > > >  		return;
> > > >  	}
> > > >
> > > > +	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> > > > +						   new_state->crtc);
> > > > +	/*
> > > > +	 * Fix the HDCP uapi content protection state in case of modeset.
> > > > +	 * FIXME: As per HDCP content protection property uapi doc, an uevent()
> > > > +	 * need to be sent if there is transition from ENABLED->DESIRED.
> > > > +	 */
> > > > +	if (drm_atomic_crtc_needs_modeset(crtc_state) &&
> > > > +	    (old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
> > > > +	    new_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
> > > > +		new_state->content_protection =
> > > > +			DRM_MODE_CONTENT_PROTECTION_DESIRED;
> > > > +
> > > >  	/*
> > > >  	 * Nothing to do if the state didn't change, or HDCP was activated since
> > > >  	 * the last commit. And also no change in hdcp content type.
> > > > @@ -2170,8 +2191,6 @@ void intel_hdcp_atomic_check(struct
> > > > drm_connector
> > > *connector,
> > > >  			return;
> > > >  	}
> > > >
> > > > -	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> > > > -						   new_state->crtc);
> > > >  	crtc_state->mode_changed = true;
> > > >  }
> > >
> > > --
> > > Jani Nikula, Intel Open Source Graphics Center
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Ramalingam C]

On 2020-07-08 at 13:55:16 +0530, Anshuman Gupta wrote:
> On 2020-07-01 at 13:31:18 +0530, Shankar, Uma wrote:
> > > > -----Original Message-----
> > > > From: Jani Nikula <jani.nikula@intel.com>
> > > > Sent: Tuesday, June 30, 2020 3:30 PM
> > > > To: Gupta, Anshuman <anshuman.gupta@intel.com>; intel-
> > > > gfx@lists.freedesktop.org
> > > > Cc: Shankar, Uma <uma.shankar@intel.com>
> > > > Subject: Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per
> > > > the kernel internal state
> > > >
> > > >
> > > > Uma, is the R-b still valid? It's been a while.
> > > 
> > > Yeah Jani, the changes look good. Will need a rebase and fresh CI results though.
> > 
> > Seems the CI results are already out and we are clean.
> Hi Ram ,
> CI results are clean for this rebase patch,
> Could you please help with merging,
Pushed into dinq. Thanks for the change.

-Ram
> I belive your RB's are valid either-way wrt to your comment.
> Thanks,
> Anshuman Gupta.
> 
> > 
> > > Regards,
> > > Uma Shankar
> > > 
> > > > BR,
> > > > Jani.
> > > >
> > > >
> > > > On Tue, 30 Jun 2020, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
> > > > > Content Protection property should be updated as per the kernel
> > > > > internal state. Let's say if Content protection is disabled by
> > > > > userspace, CP property should be set to UNDESIRED so that
> > > > > reauthentication will not happen until userspace request it again,
> > > > > but when kernel disables the HDCP due to any DDI disabling sequences
> > > > > like modeset/DPMS operation, kernel should set the property to
> > > > > DESIRED, so that when opportunity arises, kernel will start the HDCP
> > > > > authentication on its own.
> > > > >
> > > > > Somewhere in the line, state machine to set content protection to
> > > > > DESIRED from kernel was broken and IGT coverage was missing for it.
> > > > > This patch fixes it.
> > > > >
> > > > > v2:
> > > > > - Fixing hdcp CP state in connector atomic check function
> > > > >   intel_hdcp_atomic_check(). [Maarten]
> > > > >   This will require to check hdcp->value in intel_hdcp_update_pipe()
> > > > >   in order to avoid enabling hdcp, if it was already enabled.
> > > > >
> > > > > v3:
> > > > > - Rebased.
> > > > >
> > > > > Cc: Ramalingam C <ramalingam.c@intel.com>
> > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > > Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> > > > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > > > Link:
> > > > > https://patchwork.freedesktop.org/patch/350962/?series=72664&rev=2
> > > > > #v1
> > > > > Link:
> > > > > https://patchwork.freedesktop.org/patch/359396/?series=72251&rev=3
> > > > > #v2
> > > > > ---
> > > > >  drivers/gpu/drm/i915/display/intel_hdcp.c | 27
> > > > > +++++++++++++++++++----
> > > > >  1 file changed, 23 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > > > b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > > > index 815b054bb167..0d410652e194 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > > > @@ -2086,6 +2086,7 @@ void intel_hdcp_update_pipe(struct
> > > > intel_atomic_state *state,
> > > > >  		(conn_state->hdcp_content_type != hdcp->content_type &&
> > > > >  		 conn_state->content_protection !=
> > > > >  		 DRM_MODE_CONTENT_PROTECTION_UNDESIRED);
> > > > > +	bool desired_and_not_enabled = false;
> > > > >
> > > > >  	/*
> > > > >  	 * During the HDCP encryption session if Type change is requested,
> > > > > @@ -2108,8 +2109,15 @@ void intel_hdcp_update_pipe(struct
> > > > intel_atomic_state *state,
> > > > >  	}
> > > > >
> > > > >  	if (conn_state->content_protection ==
> > > > > -	    DRM_MODE_CONTENT_PROTECTION_DESIRED ||
> > > > > -	    content_protection_type_changed)
> > > > > +	    DRM_MODE_CONTENT_PROTECTION_DESIRED) {
> > > > > +		mutex_lock(&hdcp->mutex);
> > > > > +		/* Avoid enabling hdcp, if it already ENABLED */
> > > > > +		desired_and_not_enabled =
> > > > > +			hdcp->value !=
> > > > DRM_MODE_CONTENT_PROTECTION_ENABLED;
> > > > > +		mutex_unlock(&hdcp->mutex);
> > > > > +	}
> > > > > +
> > > > > +	if (desired_and_not_enabled || content_protection_type_changed)
> > > > >  		intel_hdcp_enable(connector,
> > > > >  				  crtc_state->cpu_transcoder,
> > > > >  				  (u8)conn_state->hdcp_content_type);
> > > > > @@ -2158,6 +2166,19 @@ void intel_hdcp_atomic_check(struct
> > > > drm_connector *connector,
> > > > >  		return;
> > > > >  	}
> > > > >
> > > > > +	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> > > > > +						   new_state->crtc);
> > > > > +	/*
> > > > > +	 * Fix the HDCP uapi content protection state in case of modeset.
> > > > > +	 * FIXME: As per HDCP content protection property uapi doc, an uevent()
> > > > > +	 * need to be sent if there is transition from ENABLED->DESIRED.
> > > > > +	 */
> > > > > +	if (drm_atomic_crtc_needs_modeset(crtc_state) &&
> > > > > +	    (old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
> > > > > +	    new_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
> > > > > +		new_state->content_protection =
> > > > > +			DRM_MODE_CONTENT_PROTECTION_DESIRED;
> > > > > +
> > > > >  	/*
> > > > >  	 * Nothing to do if the state didn't change, or HDCP was activated since
> > > > >  	 * the last commit. And also no change in hdcp content type.
> > > > > @@ -2170,8 +2191,6 @@ void intel_hdcp_atomic_check(struct
> > > > > drm_connector
> > > > *connector,
> > > > >  			return;
> > > > >  	}
> > > > >
> > > > > -	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> > > > > -						   new_state->crtc);
> > > > >  	crtc_state->mode_changed = true;
> > > > >  }
> > > >
> > > > --
> > > > Jani Nikula, Intel Open Source Graphics Center
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Jani Nikula]

Sean, Ram, good to go?

BR,
Jani.


On Tue, 30 Jun 2020, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
> Content Protection property should be updated as per the kernel
> internal state. Let's say if Content protection is disabled
> by userspace, CP property should be set to UNDESIRED so that
> reauthentication will not happen until userspace request it again,
> but when kernel disables the HDCP due to any DDI disabling sequences
> like modeset/DPMS operation, kernel should set the property to
> DESIRED, so that when opportunity arises, kernel will start the
> HDCP authentication on its own.
>
> Somewhere in the line, state machine to set content protection to
> DESIRED from kernel was broken and IGT coverage was missing for it.
> This patch fixes it.
>
> v2:
> - Fixing hdcp CP state in connector atomic check function
>   intel_hdcp_atomic_check(). [Maarten]
>   This will require to check hdcp->value in intel_hdcp_update_pipe()
>   in order to avoid enabling hdcp, if it was already enabled.
>
> v3:
> - Rebased.
>
> Cc: Ramalingam C <ramalingam.c@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> Link: https://patchwork.freedesktop.org/patch/350962/?series=72664&rev=2 #v1
> Link: https://patchwork.freedesktop.org/patch/359396/?series=72251&rev=3 #v2
> ---
>  drivers/gpu/drm/i915/display/intel_hdcp.c | 27 +++++++++++++++++++----
>  1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 815b054bb167..0d410652e194 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -2086,6 +2086,7 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state,
>  		(conn_state->hdcp_content_type != hdcp->content_type &&
>  		 conn_state->content_protection !=
>  		 DRM_MODE_CONTENT_PROTECTION_UNDESIRED);
> +	bool desired_and_not_enabled = false;
>  
>  	/*
>  	 * During the HDCP encryption session if Type change is requested,
> @@ -2108,8 +2109,15 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state,
>  	}
>  
>  	if (conn_state->content_protection ==
> -	    DRM_MODE_CONTENT_PROTECTION_DESIRED ||
> -	    content_protection_type_changed)
> +	    DRM_MODE_CONTENT_PROTECTION_DESIRED) {
> +		mutex_lock(&hdcp->mutex);
> +		/* Avoid enabling hdcp, if it already ENABLED */
> +		desired_and_not_enabled =
> +			hdcp->value != DRM_MODE_CONTENT_PROTECTION_ENABLED;
> +		mutex_unlock(&hdcp->mutex);
> +	}
> +
> +	if (desired_and_not_enabled || content_protection_type_changed)
>  		intel_hdcp_enable(connector,
>  				  crtc_state->cpu_transcoder,
>  				  (u8)conn_state->hdcp_content_type);
> @@ -2158,6 +2166,19 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>  		return;
>  	}
>  
> +	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> +						   new_state->crtc);
> +	/*
> +	 * Fix the HDCP uapi content protection state in case of modeset.
> +	 * FIXME: As per HDCP content protection property uapi doc, an uevent()
> +	 * need to be sent if there is transition from ENABLED->DESIRED.
> +	 */
> +	if (drm_atomic_crtc_needs_modeset(crtc_state) &&
> +	    (old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
> +	    new_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
> +		new_state->content_protection =
> +			DRM_MODE_CONTENT_PROTECTION_DESIRED;
> +
>  	/*
>  	 * Nothing to do if the state didn't change, or HDCP was activated since
>  	 * the last commit. And also no change in hdcp content type.
> @@ -2170,8 +2191,6 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>  			return;
>  	}
>  
> -	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> -						   new_state->crtc);
>  	crtc_state->mode_changed = true;
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/hdcp: Update CP as per the kernel internal state

[Ramalingam C]

On 2020-06-30 at 13:50:48 +0530, Anshuman Gupta wrote:
> Content Protection property should be updated as per the kernel
> internal state. Let's say if Content protection is disabled
> by userspace, CP property should be set to UNDESIRED so that
> reauthentication will not happen until userspace request it again,
> but when kernel disables the HDCP due to any DDI disabling sequences
> like modeset/DPMS operation, kernel should set the property to
> DESIRED, so that when opportunity arises, kernel will start the
> HDCP authentication on its own.
> 
> Somewhere in the line, state machine to set content protection to
> DESIRED from kernel was broken and IGT coverage was missing for it.
> This patch fixes it.
> 
> v2:
> - Fixing hdcp CP state in connector atomic check function
>   intel_hdcp_atomic_check(). [Maarten]
>   This will require to check hdcp->value in intel_hdcp_update_pipe()
>   in order to avoid enabling hdcp, if it was already enabled.
> 
> v3:
> - Rebased.
> 
> Cc: Ramalingam C <ramalingam.c@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> Link: https://patchwork.freedesktop.org/patch/350962/?series=72664&rev=2 #v1
> Link: https://patchwork.freedesktop.org/patch/359396/?series=72251&rev=3 #v2
> ---
>  drivers/gpu/drm/i915/display/intel_hdcp.c | 27 +++++++++++++++++++----
>  1 file changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 815b054bb167..0d410652e194 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -2086,6 +2086,7 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state,
>  		(conn_state->hdcp_content_type != hdcp->content_type &&
>  		 conn_state->content_protection !=
>  		 DRM_MODE_CONTENT_PROTECTION_UNDESIRED);
> +	bool desired_and_not_enabled = false;
>  
>  	/*
>  	 * During the HDCP encryption session if Type change is requested,
> @@ -2108,8 +2109,15 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state,
>  	}
>  
>  	if (conn_state->content_protection ==
> -	    DRM_MODE_CONTENT_PROTECTION_DESIRED ||
> -	    content_protection_type_changed)
> +	    DRM_MODE_CONTENT_PROTECTION_DESIRED) {
> +		mutex_lock(&hdcp->mutex);
> +		/* Avoid enabling hdcp, if it already ENABLED */
> +		desired_and_not_enabled =
> +			hdcp->value != DRM_MODE_CONTENT_PROTECTION_ENABLED;

It will be good to move before the hdcp->value modification as below,
if (content_protection_type_changed)
	hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED;

But this will impact only when content_protection_type_changed is true.
But when content_protection_type_changed is true we dont care the state of
desired_and_not_enabled as we use them in logical OR.

Either way. looks good to me.

Reviewed-by: Ramalingam C <ramalingam.c@intel.com>

> +		mutex_unlock(&hdcp->mutex);
> +	}
> +
> +	if (desired_and_not_enabled || content_protection_type_changed)
>  		intel_hdcp_enable(connector,
>  				  crtc_state->cpu_transcoder,
>  				  (u8)conn_state->hdcp_content_type);
> @@ -2158,6 +2166,19 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>  		return;
>  	}
>  
> +	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> +						   new_state->crtc);
> +	/*
> +	 * Fix the HDCP uapi content protection state in case of modeset.
> +	 * FIXME: As per HDCP content protection property uapi doc, an uevent()
> +	 * need to be sent if there is transition from ENABLED->DESIRED.
> +	 */
> +	if (drm_atomic_crtc_needs_modeset(crtc_state) &&
Based on the IGT test result I infer needs_modeset status wont change from No->yes
after this point.

-ram
> +	    (old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
> +	    new_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
> +		new_state->content_protection =
> +			DRM_MODE_CONTENT_PROTECTION_DESIRED;
> +
>  	/*
>  	 * Nothing to do if the state didn't change, or HDCP was activated since
>  	 * the last commit. And also no change in hdcp content type.
> @@ -2170,8 +2191,6 @@ void intel_hdcp_atomic_check(struct drm_connector *connector,
>  			return;
>  	}
>  
> -	crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> -						   new_state->crtc);
>  	crtc_state->mode_changed = true;
>  }
>  
> -- 
> 2.26.2
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx