Re: [PATCH v2 7/8] drm/i915: Reauthenticate HDCP on failure

[Sean Paul <seanpaul@chromium.org>]

On Sat, Feb 03, 2018 at 01:41:35AM +0530, Ramalingam C wrote:
> Incase of HDCP authentication failure, HDCP spec expects
> reauthentication. Hence this patch adds the reauthentications
> to be compliance with spec.
> 
> v2:
>   do-while to for loop for simplicity. [Seanpaul].
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hdcp.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index 4b2821feb4b4..4a3db5de229e 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -572,7 +572,7 @@ static int _intel_hdcp_disable(struct intel_connector *connector)
>  static int _intel_hdcp_enable(struct intel_connector *connector)
>  {
>  	struct drm_i915_private *dev_priv = connector->base.dev->dev_private;
> -	int i, ret;
> +	int i, ret, tries = 3;
>  
>  	DRM_DEBUG_KMS("[%s:%d] HDCP is being enabled...\n",
>  		      connector->base.name, connector->base.base.id);
> @@ -593,17 +593,26 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
>  		return ret;
>  	}
>  
> -	ret = intel_hdcp_auth(conn_to_dig_port(connector),
> -			      connector->hdcp_shim);
> -	if (ret) {
> -		DRM_ERROR("Failed to authenticate HDCP (%d)\n", ret);
> +	/* Incase of authentication failures, HDCP spec expects reauth. */
> +	for (i = 0; i < tries; i++) {
> +		if (i)
> +			DRM_DEBUG_KMS("[%s:%d] HDCP Reauth...\n",
> +				      connector->base.name,
> +				      connector->base.base.id);
> +
> +		ret = intel_hdcp_auth(conn_to_dig_port(connector),
> +				      connector->hdcp_shim);
> +		if (!ret)
> +			return 0;
> +
> +		DRM_ERROR("[%s:%d] Failed to authenticate HDCP (%d)\n",
> +			  connector->base.name, connector->base.base.id, ret);

This wasn't quite what I had laid out. This code should be outside the loop
(it only runs if the number of tries is exceeded). Also, I intentionally did not
check i for the debug message since it's a debug message and we can assume it's
free. So that can go back below the if (!ret) return statement without the
conditional.

Further, let's try to be consistent with where we print the connector name/id.
Since you are now printing it at the start of the function, it's not needed in
any of these statements. I had also changed the wording of the retry message, so
it would be nice if you brought that forth (minus the connector info, ofc)

Sean

>  
>  		/* Ensuring HDCP encryption and signalling are stopped. */
>  		_intel_hdcp_disable(connector);
> -		return ret;
>  	}
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static void intel_hdcp_check_work(struct work_struct *work)
> -- 
> 2.7.4
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx