[PATCH v4 3/7] drm/i915/xe3lpd: Disable HDCP Line Rekeying for Xe3

[Matt Roper]

On Thu, Oct 24, 2024 at 02:52:14AM +0000, Kandpal, Suraj wrote:
> 
> 
> > -----Original Message-----
> > From: Roper, Matthew D <matthew.d.roper at intel.com>
> > Sent: Wednesday, October 23, 2024 11:22 PM
> > To: Atwood, Matthew S <matthew.s.atwood at intel.com>
> > Cc: intel-xe at lists.freedesktop.org; intel-gfx at lists.freedesktop.org; Kandpal,
> > Suraj <suraj.kandpal at intel.com>
> > Subject: Re: [PATCH v4 3/7] drm/i915/xe3lpd: Disable HDCP Line Rekeying for
> > Xe3
> > 
> > On Fri, Oct 18, 2024 at 01:03:07PM -0700, Matt Atwood wrote:
> > > From: Suraj Kandpal <suraj.kandpal at intel.com>
> > >
> > > We need to disable HDCP Line Rekeying for Xe3 when we are using an
> > > HDMI encoder.
> > 
> > This is still missing the "why" for this change.  Is there a bspec reference that
> > gives the details?  From the description of the bit itself, it sounds like the
> > setting here (for both Xe3 and earlier Xe2) should be based on the HDCP
> > version rather than the platform/stepping.
> > 
> > As mentioned previously, this entire function is labeled as "/* WA:
> > 16022217614 */"  If we're now using this function for something other than
> > that specific workaround, then we need to fix/move that comment.
> > 
> > 
> 
> Bspec: 68933

I think you pasted the wrong number here?  This is a generic page that
just has links to four transcoder DDI registers and nothing else.  It
doesn't have anything to do with HDCP rekeying.

Maybe you meant 69964 (which is one of the four links from the page
above) that gives the register definition of TRANS_DDI_FUNC_CTL?  But
the info there implies that we're not really handling this properly
since it says that we need to enable/disable rekeying based on the HDCP
version.  We're disabling for HDCP 2.0 and above here (correct), but I
don't see where we're handling the enabling for HDCP 1.4 and earlier?
Unless I'm overlooking something, it seems like the driver always
updates TRANS_DDI_FUNC_CTL with a rmw cycle rather than building a new
value from scratch, so we can't really rely on the bit being 0 by
default for the cases where rekeying should be enabled.


Matt

> 
> 
> > Matt
> > 
> > >
> > > v2: add additional definition instead of function, commit message typo
> > > fix and update.
> > > v3: restore lost conditional from v2.
> > > v4: subject line and subject message updated, fix the if ladder order,
> > > fix the bit definition order.
> > >
> > > Signed-off-by: Suraj Kandpal <suraj.kandpal at intel.com>
> > > Signed-off-by: Matt Atwood <matthew.s.atwood at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_hdcp.c | 10 +++++++---
> > >  drivers/gpu/drm/i915/i915_reg.h           |  1 +
> > >  2 files changed, 8 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > index ed6aa87403e2..70dfc9d4d6ac 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > @@ -43,14 +43,18 @@ intel_hdcp_disable_hdcp_line_rekeying(struct
> > intel_encoder *encoder,
> > >  		return;
> > >
> > >  	if (DISPLAY_VER(display) >= 14) {
> > > -		if (IS_DISPLAY_VER_STEP(display, IP_VER(14, 0), STEP_D0,
> > STEP_FOREVER))
> > > -			intel_de_rmw(display, MTL_CHICKEN_TRANS(hdcp-
> > >cpu_transcoder),
> > > -				     0, HDCP_LINE_REKEY_DISABLE);
> > > +		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_VER_STEP(display, IP_VER(14, 1), STEP_B0,
> > STEP_FOREVER) ||
> > >  			 IS_DISPLAY_VER_STEP(display, IP_VER(20, 0),
> > 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_VER_STEP(display, IP_VER(14, 0), STEP_D0,
> > STEP_FOREVER))
> > > +			intel_de_rmw(display, MTL_CHICKEN_TRANS(hdcp-
> > >cpu_transcoder),
> > > +				     0, HDCP_LINE_REKEY_DISABLE);
> > >  	}
> > >  }
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h index 89e4381f8baa..8d758947f301
> > > 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -3817,6 +3817,7 @@ enum skl_power_gate {
> > >  #define  TRANS_DDI_PVSYNC		(1 << 17)
> > >  #define  TRANS_DDI_PHSYNC		(1 << 16)
> > >  #define  TRANS_DDI_PORT_SYNC_ENABLE	REG_BIT(15)
> > > +#define  XE3_TRANS_DDI_HDCP_LINE_REKEY_DISABLE	REG_BIT(15)
> > >  #define  TRANS_DDI_EDP_INPUT_MASK	(7 << 12)
> > >  #define  TRANS_DDI_EDP_INPUT_A_ON	(0 << 12)
> > >  #define  TRANS_DDI_EDP_INPUT_A_ONOFF	(4 << 12)
> > > --
> > > 2.45.0
> > >
> > 
> > --
> > Matt Roper
> > Graphics Software Engineer
> > Linux GPU Platform Enablement
> > Intel Corporation

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation