[PATCH 1/2] drm/i915/xe3lpd: Disable HDCP Line Rekeying for Xe3

[Kandpal, Suraj]

> -----Original Message-----
> From: Roper, Matthew D <matthew.d.roper at intel.com>
> Sent: Monday, November 4, 2024 11:25 PM
> To: Kandpal, Suraj <suraj.kandpal at intel.com>
> Cc: intel-xe at lists.freedesktop.org; intel-gfx at lists.freedesktop.org
> Subject: Re: [PATCH 1/2] drm/i915/xe3lpd: Disable HDCP Line Rekeying for Xe3
> 
> On Mon, Nov 04, 2024 at 02:01:42PM +0530, Suraj Kandpal wrote:
> > We need to disable HDCP Line Rekeying for Xe3 when we are using an
> > HDMI encoder. Also remove the Wa comment tag as this follows the bspec
> > and does not implement the wa.
> >
> > 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.
> > v5: Add the bspec link and remove the Wa comment tag
> > v6: Rebase over new changes
> >
> > Bspec: 69964
> > Signed-off-by: Suraj Kandpal <suraj.kandpal at intel.com>
> 
> I think the code changes are fine now (although you seem to have dropped
> the general ladder re-order that was done in v4; was that intentional or a
> mistake?), but the subject/commit message seem misleading since the
> change this patch is making isn't changing if/when rekeying is disabled, all it's
> doing is changing the bit used.  So a commit message more along the lines of
> 
>         drm/i915/xe3lpd: Update HDCP rekying bit
> 
>         The TRANS_DDI_FUNC_CTL bit used to enable/disable HDCP rekeying
>         has moved from bit 12 (Xe2) to bit 15 (Xe3); update the RMW
>         toggle accordingly.
> 
>         Also drop the misleading workaround comment tag on this function
>         since disabling of HDCP rekeying is something that happens on
>         all platforms, not just those impacted by that workaround.
> 
> If you decide to reinstate the ladder re-order that you had on an earlier
> version of this patch, you could add another sentence like
> 
>         While we're here, also re-order the if/else ladder to use
>         standard "newest platform first" order.
> 

Sure will reladder the checks and update the commit message accordingly must have missed it
due to the rebase.

Regards,
Suraj Kandpal

> Anyway, with some kind of commit message cleanup,
> 
>         Reviewed-by: Matt Roper <matthew.d.roper at intel.com>
> 
> 
> Matt
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_hdcp.c | 7 +++++--
> >  drivers/gpu/drm/i915/i915_reg.h           | 1 +
> >  2 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > index f6d42ec6949e..8bca532d1176 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > @@ -31,7 +31,6 @@
> >  #define KEY_LOAD_TRIES	5
> >  #define HDCP2_LC_RETRY_CNT			3
> >
> > -/* WA: 16022217614 */
> >  static void
> >  intel_hdcp_disable_hdcp_line_rekeying(struct intel_encoder *encoder,
> >  				      struct intel_hdcp *hdcp)
> > @@ -43,7 +42,11 @@ intel_hdcp_disable_hdcp_line_rekeying(struct
> intel_encoder *encoder,
> >  		return;
> >
> >  	if (DISPLAY_VER(display) >= 14) {
> > -		if (IS_DISPLAY_VERx100_STEP(display, 1400, STEP_D0,
> STEP_FOREVER))
> > +		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, 1400, STEP_D0,
> > +STEP_FOREVER))
> >  			intel_de_rmw(display, MTL_CHICKEN_TRANS(hdcp-
> >cpu_transcoder),
> >  				     0, HDCP_LINE_REKEY_DISABLE);
> >  		else if (IS_DISPLAY_VERx100_STEP(display, 1401, STEP_B0,
> > STEP_FOREVER) || diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index 22be4a731d27..c160e087972a
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -3819,6 +3819,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.34.1
> >
> 
> --
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation