[PATCH] drm/i915/hdcp: Handle HDCP Line Rekeying for HDCP 1.4

[Kandpal, Suraj]

> -----Original Message-----
> From: Nikula, Jani <jani.nikula at intel.com>
> Sent: Wednesday, November 6, 2024 9:59 PM
> To: Kandpal, Suraj <suraj.kandpal at intel.com>; intel-xe at lists.freedesktop.org;
> intel-gfx at lists.freedesktop.org
> Cc: Roper, Matthew D <matthew.d.roper at intel.com>; Kandpal, Suraj
> <suraj.kandpal at intel.com>
> Subject: Re: [PATCH] drm/i915/hdcp: Handle HDCP Line Rekeying for HDCP 1.4
> 
> On Wed, 06 Nov 2024, Suraj Kandpal <suraj.kandpal at intel.com> wrote:
> > TRANS_DDI_FUNC_CTL asks us to disable hdcp line rekeying when not in
> > hdcp 2.2 and we are not using an hdmi transcoder and it need to be
> > enabled when we are using an HDMI transcoder to enable HDCP 1.4.
> > We use intel_de_rmw cycles to update TRANS_DDI_FUNC_CTL register so
> we
> > cannot depend on the value being 0 by default everytime which calls
> > for seprate handling of HDCP 1.4 case.
> >
> > --v2
> > -Use the exising function and modify it based on a bool rather than
> > have a different function [Matt]
> >
> > --v3
> > -No need for kzalloc [Jani]
> 
> The code still needs to make sense! You can't just remove allocation and use
> whatever stack garbage rekey_reg happens to point at. Look at all the other
> code that uses i915_reg_t. You can't possibly assume I would've needed to
> say "it shouldn't be a pointer at all"?
> 

Sure Jani ill fix that, I thought I had changed it from a pointer to a normal variable,
Seems to have missed it.

Regards,
Suraj Kandpal

> BR,
> Jani.
> 
> >
> > Bspec: 69964, 50493, 50054
> > Signed-off-by: Suraj Kandpal <suraj.kandpal at intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_hdcp.c | 36
> > +++++++++++++----------
> >  1 file changed, 21 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > index 4e937fbba4d2..6ad5efc9711c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > @@ -32,27 +32,31 @@
> >  #define HDCP2_LC_RETRY_CNT			3
> >
> >  static void
> > -intel_hdcp_disable_hdcp_line_rekeying(struct intel_encoder *encoder,
> > -				      struct intel_hdcp *hdcp)
> > +intel_hdcp_adjust_hdcp_line_rekeying(struct intel_encoder *encoder,
> > +				     struct intel_hdcp *hdcp,
> > +				     bool enable)
> >  {
> >  	struct intel_display *display = to_intel_display(encoder);
> > +	i915_reg_t *rekey_reg;
> > +	u32 rekey_bit;
> >
> >  	/* Here we assume HDMI is in TMDS mode of operation */
> >  	if (encoder->type != INTEL_OUTPUT_HDMI)
> >  		return;
> >
> > -	if (DISPLAY_VER(display) >= 30)
> > -		intel_de_rmw(display,
> > -			     TRANS_DDI_FUNC_CTL(display, hdcp-
> >cpu_transcoder),
> > -			     0, XE3_TRANS_DDI_HDCP_LINE_REKEY_DISABLE);
> > -	else if (IS_DISPLAY_VERx100_STEP(display, 1401, STEP_B0,
> STEP_FOREVER) ||
> > -		 IS_DISPLAY_VERx100_STEP(display, 2000, STEP_B0,
> STEP_FOREVER))
> > -		intel_de_rmw(display,
> > -			     TRANS_DDI_FUNC_CTL(display, hdcp-
> >cpu_transcoder),
> > -			     0, TRANS_DDI_HDCP_LINE_REKEY_DISABLE);
> > -	else if (IS_DISPLAY_VERx100_STEP(display, 1400, STEP_D0,
> STEP_FOREVER))
> > -		intel_de_rmw(display, MTL_CHICKEN_TRANS(hdcp-
> >cpu_transcoder),
> > -			     0, HDCP_LINE_REKEY_DISABLE);
> > +	if (DISPLAY_VER(display) >= 30) {
> > +		*rekey_reg = TRANS_DDI_FUNC_CTL(display, hdcp-
> >cpu_transcoder);
> > +		rekey_bit = XE3_TRANS_DDI_HDCP_LINE_REKEY_DISABLE;
> > +	} else if (IS_DISPLAY_VERx100_STEP(display, 1401, STEP_B0,
> STEP_FOREVER) ||
> > +		   IS_DISPLAY_VERx100_STEP(display, 2000, STEP_B0,
> STEP_FOREVER)) {
> > +		*rekey_reg = TRANS_DDI_FUNC_CTL(display, hdcp-
> >cpu_transcoder);
> > +		rekey_bit = TRANS_DDI_HDCP_LINE_REKEY_DISABLE;
> > +	} else if (IS_DISPLAY_VERx100_STEP(display, 1400, STEP_D0,
> STEP_FOREVER)) {
> > +		*rekey_reg = MTL_CHICKEN_TRANS(hdcp->cpu_transcoder);
> > +		rekey_bit = HDCP_LINE_REKEY_DISABLE;
> > +	}
> > +
> > +	intel_de_rmw(display, *rekey_reg, rekey_bit, enable ? 0 :
> > +rekey_bit);
> >  }
> >
> >  static int intel_conn_to_vcpi(struct intel_atomic_state *state, @@
> > -1049,6 +1053,8 @@ static int intel_hdcp1_enable(struct intel_connector
> *connector)
> >  		return ret;
> >  	}
> >
> > +	intel_hdcp_adjust_hdcp_line_rekeying(connector->encoder, hdcp,
> > +true);
> > +
> >  	/* Incase of authentication failures, HDCP spec expects reauth. */
> >  	for (i = 0; i < tries; i++) {
> >  		ret = intel_hdcp_auth(connector);
> > @@ -2062,7 +2068,7 @@ static int _intel_hdcp2_enable(struct
> intel_atomic_state *state,
> >  		    connector->base.base.id, connector->base.name,
> >  		    hdcp->content_type);
> >
> > -	intel_hdcp_disable_hdcp_line_rekeying(connector->encoder, hdcp);
> > +	intel_hdcp_adjust_hdcp_line_rekeying(connector->encoder, hdcp,
> > +false);
> >
> >  	ret = hdcp2_authenticate_and_encrypt(state, connector);
> >  	if (ret) {
> 
> --
> Jani Nikula, Intel