> -----Original Message-----
> From: Pottumuttu, Sai Teja <sai.teja.pottumuttu@xxxxxxxxx>
> Sent: Tuesday, October 29, 2024 2:13 PM
> To: Kandpal, Suraj <suraj.kandpal@xxxxxxxxx>; intel-xe@xxxxxxxxxxxxxxxxxxxxx;
> intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH] drm/i915/xe3lpd: Disable HDCP Line Rekeying for Xe3
>
>
> On 29-10-2024 10:56, 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
> >
> > Bspec: 69964
> I don't think we still address the why? part. The register and bit doesn't give
> any explanation about disabling line rekeying on xe3 right?
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/display/intel_hdcp.c | 11 +++++++----
> > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > 2 files changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > index ed6aa87403e2..7a32bfef8d87 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 */
> Probably instead of removing it we can add the lineage of the workaround
> (Wa_16021352814) somewhere inside the function so that we know why are
> we disabling line rekeying for those specific ip versions and steppings.
> > static void
> > intel_hdcp_disable_hdcp_line_rekeying(struct intel_encoder *encoder,
> > struct intel_hdcp *hdcp)
> > @@ -43,14 +42,18 @@ intel_hdcp_disable_hdcp_line_rekeying(struct
> intel_encoder *encoder,
> > return;
> >
> > if (DISPLAY_VER(display) >= 14) {
>
> Not related to this patch but probably we can remove the above if right?
> The internal if else ladder would take care of everything.
Why should we do the extra checks for other display versions that is two checks for display_ver<14 instead of just one check
Which would have happened .
Regards,
Suraj Kandpal
>
> Thank You
> Sai Teja
>
> > - 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 405f409e9761..184420011a88
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -3816,6 +3816,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)
