Re: [PATCH 1/8] drm/i915: Handle failure from 2nd stage HDCP auth

[Ramalingam C <ramalingam.c@xxxxxxxxx>]

On Friday 02 February 2018 07:39 PM, Sean Paul wrote:
> On Fri, Feb 02, 2018 at 04:15:13PM +0530, Ramalingam C wrote:
> > We enable the HDCP encryption as a part of first stage authentication.
> > So when second stage authentication fails, we need to disable the HDCP
> > encryption and signalling.
> >
> > This patch handles above requirement.
> >
> > For reusing the _intel_hdcp_disable from generic authentication flow,
> > this patch retain the reference to connector till the generic hdcp flow.
> > This will be handy to provide the connector details in HDCP enable
> > debug info.
> >
> > Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx>
> > ---
> >   drivers/gpu/drm/i915/intel_hdcp.c | 41 +++++++++++++++++++++++----------------
> >   1 file changed, 24 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> > index b97184eccd9c..0021511ae4d7 100644
> > --- a/drivers/gpu/drm/i915/intel_hdcp.c
> > +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> > @@ -16,6 +16,8 @@
> >   
> >   #define KEY_LOAD_TRIES	5
> >   
> > +static int _intel_hdcp_disable(struct intel_connector *connector);
> > +
> >   static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
> >   				    const struct intel_hdcp_shim *shim)
> >   {
> > @@ -138,17 +140,24 @@ bool intel_hdcp_is_ksv_valid(u8 *ksv)
> >   	return true;
> >   }
> >   
> > +static
> > +struct intel_digital_port *conn_to_dig_port(struct intel_connector *connector)
> > +{
> > +	return enc_to_dig_port(&connector->encoder->base);
> > +}
> > +
> >   /* Implements Part 2 of the HDCP authorization procedure */
> >   static
> > -int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
> > -			       const struct intel_hdcp_shim *shim)
> > +int intel_hdcp_auth_downstream(struct intel_connector *connector)
> >   {
> >   	struct drm_i915_private *dev_priv;
> >   	u32 vprime, sha_text, sha_leftovers, rep_ctl;
> >   	u8 bstatus[2], num_downstream, *ksv_fifo;
> >   	int ret, i, j, sha_idx;
> > +	const struct intel_hdcp_shim *shim = connector->hdcp_shim;
> > +	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> >   
> > -	dev_priv = intel_dig_port->base.base.dev->dev_private;
> > +	dev_priv = to_i915(connector->base.dev);
> >   
> >   	ret = intel_hdcp_poll_ksv_fifo(intel_dig_port, shim);
> >   	if (ret) {
> > @@ -385,8 +394,7 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,
> >   }
> >   
> >   /* Implements Part 1 of the HDCP authorization procedure */
> > -static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
> > -			   const struct intel_hdcp_shim *shim)
> > +static int intel_hdcp_auth(struct intel_connector *connector)
> >   {
> >   	struct drm_i915_private *dev_priv;
> >   	enum port port;
> > @@ -405,9 +413,10 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
> >   		u8 shim[DRM_HDCP_RI_LEN];
> >   	} ri;
> >   	bool repeater_present;
> > +	const struct intel_hdcp_shim *shim = connector->hdcp_shim;
> > +	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> >   
> > -	dev_priv = intel_dig_port->base.base.dev->dev_private;
> > -
> > +	dev_priv = to_i915(connector->base.dev);
> >   	port = intel_dig_port->base.port;
> >   
> >   	/* Initialize An with 2 random values and acquire it */
> > @@ -498,19 +507,18 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
> >   	 * on those as well.
> >   	 */
> >   
> > -	if (repeater_present)
> > -		return intel_hdcp_auth_downstream(intel_dig_port, shim);
> > +	if (repeater_present) {
> > +		ret = intel_hdcp_auth_downstream(connector);
> > +		if (ret) {
> > +			_intel_hdcp_disable(connector);
> If you just call this from _intel_hdcp_enable when intel_hdcp_auth() fails, you
> can avoid all of this code, it'd just be one line.
Yes. Thought about that option too. Actually I would prefer having the 
reference of intel_connector untill generic hdcp auth implementation.
we can pass the intel digital port for encoder specific shims.

This will help me with
    providing the connector details for state transitions as in next 
patches
    next steps like SRM passing in for revocation and downstream 
topology gathering (perhaps will start with availability of 
complementing user space)

At this point if we dont want this code, I will disable hdcp from enable 
functions.

-Ram
> > +			return ret;
> > +		}
> > +	}
> >   
> >   	DRM_DEBUG_KMS("HDCP is enabled (no repeater present)\n");
> >   	return 0;
> >   }
> >   
> > -static
> > -struct intel_digital_port *conn_to_dig_port(struct intel_connector *connector)
> > -{
> > -	return enc_to_dig_port(&intel_attached_encoder(&connector->base)->base);
> > -}
> > -
> >   static int _intel_hdcp_disable(struct intel_connector *connector)
> >   {
> >   	struct drm_i915_private *dev_priv = connector->base.dev->dev_private;
> > @@ -558,8 +566,7 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
> >   		return ret;
> >   	}
> >   
> > -	ret = intel_hdcp_auth(conn_to_dig_port(connector),
> > -			      connector->hdcp_shim);
> > +	ret = intel_hdcp_auth(connector);
> >   	if (ret) {
> >   		DRM_ERROR("Failed to authenticate HDCP (%d)\n", ret);
> >   		return ret;
> > --
> > 2.7.4

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