> -----Original Message----- > From: Roper, Matthew D <matthew.d.roper@xxxxxxxxx> > Sent: Monday, November 4, 2024 11:25 PM > To: Kandpal, Suraj <suraj.kandpal@xxxxxxxxx> > Cc: intel-xe@xxxxxxxxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > 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@xxxxxxxxx> > > 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@xxxxxxxxx> > > > 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