[PATCH v4 3/7] drm/i915/xe3lpd: Disable HDCP Line Rekeying for Xe3
Kandpal, Suraj
suraj.kandpal at intel.com
Mon Nov 4 03:44:59 UTC 2024
> -----Original Message-----
> From: Roper, Matthew D <matthew.d.roper at intel.com>
> Sent: Wednesday, October 30, 2024 5:33 AM
> To: Kandpal, Suraj <suraj.kandpal at intel.com>
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal at intel.com>; Atwood, Matthew S
> <matthew.s.atwood at intel.com>; intel-xe at lists.freedesktop.org; intel-
> gfx at lists.freedesktop.org
> 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 at intel.com>
> > > Sent: Saturday, October 26, 2024 12:28 AM
> > > To: Kandpal, Suraj <suraj.kandpal at intel.com>
> > > Cc: Atwood, Matthew S <matthew.s.atwood at intel.com>; intel-
> > > xe at lists.freedesktop.org; intel-gfx at lists.freedesktop.org
> > > 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 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.
> >
> > 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 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
>
> --
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation
More information about the Intel-gfx
mailing list