> -----Original Message----- > From: Roper, Matthew D <matthew.d.roper@xxxxxxxxx> > Sent: Wednesday, October 30, 2024 5:33 AM > To: Kandpal, Suraj <suraj.kandpal@xxxxxxxxx> > Cc: Nautiyal, Ankit K <ankit.k.nautiyal@xxxxxxxxx>; Atwood, Matthew S > <matthew.s.atwood@xxxxxxxxx>; intel-xe@xxxxxxxxxxxxxxxxxxxxx; intel- > gfx@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH v4 3/7] drm/i915/xe3lpd: Disable HDCP Line Rekeying for > Xe3 > > On Sun, Oct 27, 2024 at 09:11:07PM -0700, Kandpal, Suraj wrote: > > > > > > > -----Original Message----- > > > From: Roper, Matthew D <matthew.d.roper@xxxxxxxxx> > > > Sent: Saturday, October 26, 2024 12:28 AM > > > To: Kandpal, Suraj <suraj.kandpal@xxxxxxxxx> > > > Cc: Atwood, Matthew S <matthew.s.atwood@xxxxxxxxx>; intel- > > > xe@xxxxxxxxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > Subject: Re: [PATCH v4 3/7] drm/i915/xe3lpd: Disable HDCP Line > > > Rekeying for > > > Xe3 > > > > > > On Thu, Oct 24, 2024 at 02:52:14AM +0000, Kandpal, Suraj wrote: > > > > > > > > > > > > > -----Original Message----- > > > > > From: Roper, Matthew D <matthew.d.roper@xxxxxxxxx> > > > > > Sent: Wednesday, October 23, 2024 11:22 PM > > > > > To: Atwood, Matthew S <matthew.s.atwood@xxxxxxxxx> > > > > > Cc: intel-xe@xxxxxxxxxxxxxxxxxxxxx; > > > > > intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Kandpal, Suraj > > > > > <suraj.kandpal@xxxxxxxxx> > > > > > 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@xxxxxxxxx> > > > > > > > > > > > > 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. > > > > From what I can see TRANS_DDI_FUNC_CTL is written via > > intel_ddi_enable_transcoder_func() > > Which fills in the values to be written by > > intel_ddi_transcoder_func_reg_val_get where the line rekey bit ends up > being 0 by default which make me believe that separate handling for HDCP > 1.4 case may not be required. > I think I get your point ill float a separate patch that takes care of enabling HDCP line rekeying for HDCP 1.4 In the meantime can you review this patch which is just a modification due to the register bit change I have Sent a new patch in this link https://patchwork.freedesktop.org/series/140625/ > Can HDCP only be enabled/disabled during a full modeset or can it be > changed on a non-modeset pipe update? If it only changes on full modesets, > then I agree that the initialization in > intel_ddi_enable_transcoder_func() probably covers it (although I'm not sure > why we've been using RMW cycles everywhere in that case rather than just > setting the proper value during the initial register write). > > It sounds like nothing has truly changed logic-wise here in Xe3 compared to > MTL or Xe2; the only real change is that the register bit has switched to a new > location again. Yes that's true Regards, Suraj Kandpal > > > Matt > > > > > Regards, > > Suraj Kandpal > > > > > > > > > > > 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@xxxxxxxxx> > > > > > > Signed-off-by: Matt Atwood <matthew.s.atwood@xxxxxxxxx> > > > > > > --- > > > > > > 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 > > -- > Matt Roper > Graphics Software Engineer > Linux GPU Platform Enablement > Intel Corporation
