Re: [Intel-gfx] [PATCH v5 03/10] drm/hdcp: Update property value on content type and user changes

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Apr 11, 2022 at 08:47:32PM +0000, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> This patch updates the connector's property value in 2 cases which were
> previously missed:
> 
> 1- Content type changes. The value should revert back to DESIRED from
>    ENABLED in case the driver must re-authenticate the link due to the
>    new content type.
> 
> 2- Userspace sets value to DESIRED while ENABLED. In this case, the
>    value should be reset immediately to ENABLED since the link is
>    actively being encrypted.
> 
> To accommodate these changes, I've split up the conditionals to make
> things a bit more clear (as much as one can with this mess of state).
> 
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> Reviewed-by: Abhinav Kumar <abhinavk@codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Link: https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-4-sean@poorly.run #v1
> Link: https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-4-sean@poorly.run #v2
> Link: https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-4-sean@poorly.run #v3
> Link: https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-4-sean@poorly.run #v4
> 
> Changes in v2:
> -None
> Changes in v3:
> -Fixed indentation issue identified by 0-day
> Changes in v4:
> -None
> Changes in v5:
> -None
> ---
>  drivers/gpu/drm/drm_hdcp.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
> index dd8fa91c51d6..8c851d40cd45 100644
> --- a/drivers/gpu/drm/drm_hdcp.c
> +++ b/drivers/gpu/drm/drm_hdcp.c
> @@ -487,21 +487,29 @@ bool drm_hdcp_atomic_check(struct drm_connector *connector,
>  		return true;
>  
>  	/*
> -	 * Nothing to do if content type is unchanged and one of:
> -	 *  - state didn't change
> +	 * Content type changes require an HDCP disable/enable cycle.
> +	 */
> +	if (new_conn_state->hdcp_content_type != old_conn_state->hdcp_content_type) {

shouldn't we add some && ( old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED)) {
here?

> +		new_conn_state->content_protection =
> +			DRM_MODE_CONTENT_PROTECTION_DESIRED;
> +		return true;
> +	}
> +
> +	/*
> +	 * Ignore meaningless state changes:
>  	 *  - HDCP was activated since the last commit
> -	 *  - attempting to set to desired while already enabled
> +	 *  - Attempting to set to desired while already enabled
>  	 */
> -	if (old_hdcp == new_hdcp ||
> -	    (old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
> +	if ((old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
>  	     new_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED) ||
>  	    (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
>  	     new_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED)) {
> -		if (old_conn_state->hdcp_content_type ==
> -				new_conn_state->hdcp_content_type)
> -			return false;
> +		new_conn_state->content_protection =
> +			DRM_MODE_CONTENT_PROTECTION_ENABLED;
> +		return false;
>  	}
>  
> -	return true;
> +	/* Finally, if state changes, we need action */
> +	return old_hdcp != new_hdcp;
>  }
>  EXPORT_SYMBOL(drm_hdcp_atomic_check);
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
>