| Message ID | 20240527052118.1624216-2-suraj.kandpal@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915/hdcp: Fix IS_METEORLAKE usage for HDCP line rekeying | expand |
> -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Suraj > Kandpal > Sent: Monday, May 27, 2024 10:51 AM > To: intel-gfx@lists.freedesktop.org > Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; Roper, > Matthew D <matthew.d.roper@intel.com>; Kandpal, Suraj > <suraj.kandpal@intel.com> > Subject: [PATCH] drm/i915/hdcp: Fix IS_METEORLAKE usage for HDCP line > rekeying > > Replace IS_METEORLAKE usage with a more appropriate macro. While we are > at it also add the stepping restrictions for other platforms. > > Fixes: 6a3691ca4799 ("drm/i915/hdcp: Disable HDCP Line Rekeying for HDCP2.2 > on HDMI") > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > --- LGTM, Reviewed-by: Dnyaneshwar Bhadane <dnyaneshwar.bhadane@intel.com> > drivers/gpu/drm/i915/display/intel_hdcp.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > b/drivers/gpu/drm/i915/display/intel_hdcp.c > index 02cbbbfd8e25..5767070248bb 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > @@ -42,10 +42,11 @@ intel_hdcp_disable_hdcp_line_rekeying(struct > intel_encoder *encoder, > return; > > if (DISPLAY_VER(dev_priv) >= 14) { > - if (IS_METEORLAKE(dev_priv)) > + if (IS_DISPLAY_IP_STEP(dev_priv, IP_VER(14, 0), STEP_D0, > +STEP_FOREVER)) > intel_de_rmw(dev_priv, MTL_CHICKEN_TRANS(hdcp- > >cpu_transcoder), > 0, HDCP_LINE_REKEY_DISABLE); > - else > + else if (IS_DISPLAY_IP_STEP(dev_priv, IP_VER(14, 1), STEP_B0, > STEP_FOREVER) || > + IS_DISPLAY_IP_STEP(dev_priv, IP_VER(20, 0), STEP_B0, > STEP_FOREVER)) > intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL(hdcp- > >cpu_transcoder), > 0, > TRANS_DDI_HDCP_LINE_REKEY_DISABLE); > } > -- > 2.43.2
I'm not terribly familiar with HDCP, but from reading the spec my
understanding is that in general we need to enable rekeying for HDCP 1.4
and disable it for higher HDCP versions, right? That would be the
expected "normal" programming for all platforms if we didn't have any
workarounds. However the issue here is that we have a workaround
(Wa_16021352814) which affects the early steppings of certain platforms
and instructs us to "Prune timings with hblank below 138" instead of
disabling rekeying.
In that case, I'd expect the logic for this function to be something
like the following pseudocode:
needs_wa_16021352814() {
return IS_DISPLAY_STEP(IP_VER(14, 0), STEP_A0, STEP_D0) ||
IS_DISPLAY_STEP(IP_VER(14, 1), STEP_A0, STEP_B0) ||
IS_DISPLAY_STEP(IP_VER(20, 0), STEP_A0, STEP_B0);
}
intel_hdcp_disable_hdcp_line_rekeying() {
assert(output is HDMI);
assert(HDCP > 1.4);
/*
* Wa_16021352814: Rather than disabling rekeying, we need
* to prune timings with hblank below 138 pixels, which is
* taken care of in xxxxxx().
*/
if (needs_wa_16021352814())
return;
if (DISPLAY_VER_FULL() >= IP_VER(14, 1))
intel_de_rmw(TRANS_DDI_FUNC_CTL, ...);
else
intel_de_rmw(MTL_CHICKEN_TRANS, ...);
}
And the logic to prune modes according to hblank for this workaround
would need to be programmed somewhere as well...has that already
happened?
Matt
On Mon, May 27, 2024 at 10:51:19AM +0530, Suraj Kandpal wrote:
> Replace IS_METEORLAKE usage with a more appropriate macro. While
> we are at it also add the stepping restrictions for other platforms.
>
> Fixes: 6a3691ca4799 ("drm/i915/hdcp: Disable HDCP Line Rekeying for HDCP2.2 on HDMI")
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_hdcp.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 02cbbbfd8e25..5767070248bb 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -42,10 +42,11 @@ intel_hdcp_disable_hdcp_line_rekeying(struct intel_encoder *encoder,
> return;
>
> if (DISPLAY_VER(dev_priv) >= 14) {
> - if (IS_METEORLAKE(dev_priv))
> + if (IS_DISPLAY_IP_STEP(dev_priv, IP_VER(14, 0), STEP_D0, STEP_FOREVER))
> intel_de_rmw(dev_priv, MTL_CHICKEN_TRANS(hdcp->cpu_transcoder),
> 0, HDCP_LINE_REKEY_DISABLE);
> - else
> + else if (IS_DISPLAY_IP_STEP(dev_priv, IP_VER(14, 1), STEP_B0, STEP_FOREVER) ||
> + IS_DISPLAY_IP_STEP(dev_priv, IP_VER(20, 0), STEP_B0, STEP_FOREVER))
> intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL(hdcp->cpu_transcoder),
> 0, TRANS_DDI_HDCP_LINE_REKEY_DISABLE);
> }
> --
> 2.43.2
>
, Chaitanya Kumar > <chaitanya.kumar.borah@intel.com> > Subject: Re: [PATCH] drm/i915/hdcp: Fix IS_METEORLAKE usage for HDCP line > rekeying > > I'm not terribly familiar with HDCP, but from reading the spec my > understanding is that in general we need to enable rekeying for HDCP 1.4 and > disable it for higher HDCP versions, right? That would be the expected > "normal" programming for all platforms if we didn't have any workarounds. > However the issue here is that we have a workaround > (Wa_16021352814) which affects the early steppings of certain platforms and > instructs us to "Prune timings with hblank below 138" instead of disabling > rekeying. > > In that case, I'd expect the logic for this function to be something like the > following pseudocode: > > needs_wa_16021352814() { > return IS_DISPLAY_STEP(IP_VER(14, 0), STEP_A0, STEP_D0) || > IS_DISPLAY_STEP(IP_VER(14, 1), STEP_A0, STEP_B0) || > IS_DISPLAY_STEP(IP_VER(20, 0), STEP_A0, STEP_B0); > } > > intel_hdcp_disable_hdcp_line_rekeying() { > assert(output is HDMI); > assert(HDCP > 1.4); > > /* > * Wa_16021352814: Rather than disabling rekeying, we need > * to prune timings with hblank below 138 pixels, which is > * taken care of in xxxxxx(). > */ > if (needs_wa_16021352814()) > return; > > if (DISPLAY_VER_FULL() >= IP_VER(14, 1)) > intel_de_rmw(TRANS_DDI_FUNC_CTL, ...); > else > intel_de_rmw(MTL_CHICKEN_TRANS, ...); > } > > And the logic to prune modes according to hblank for this workaround would > need to be programmed somewhere as well...has that already happened? > > > Matt > So the disable line rekeying is called from a flow which would only come into picture when HDCP 1.4 is being enabled. Next the WA says to prune modes when HDCP 1.4 is being enabled. Now we prune modes in intel_dp/hdmi_detect which happens way before HDCP is called for enablement. would require a change in code flow. Does doing that make sense keeping in mind that this is a temporary WA and we are to use disable HDCP Line Rekeying on the latest stepping anyways specially the upstream code? Regards, Suraj Kandpal > On Mon, May 27, 2024 at 10:51:19AM +0530, Suraj Kandpal wrote: > > Replace IS_METEORLAKE usage with a more appropriate macro. While we > > are at it also add the stepping restrictions for other platforms. > > > > Fixes: 6a3691ca4799 ("drm/i915/hdcp: Disable HDCP Line Rekeying for > > HDCP2.2 on HDMI") > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_hdcp.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > > b/drivers/gpu/drm/i915/display/intel_hdcp.c > > index 02cbbbfd8e25..5767070248bb 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > > @@ -42,10 +42,11 @@ intel_hdcp_disable_hdcp_line_rekeying(struct > intel_encoder *encoder, > > return; > > > > if (DISPLAY_VER(dev_priv) >= 14) { > > - if (IS_METEORLAKE(dev_priv)) > > + if (IS_DISPLAY_IP_STEP(dev_priv, IP_VER(14, 0), STEP_D0, > > +STEP_FOREVER)) > > intel_de_rmw(dev_priv, MTL_CHICKEN_TRANS(hdcp- > >cpu_transcoder), > > 0, HDCP_LINE_REKEY_DISABLE); > > - else > > + else if (IS_DISPLAY_IP_STEP(dev_priv, IP_VER(14, 1), STEP_B0, > STEP_FOREVER) || > > + IS_DISPLAY_IP_STEP(dev_priv, IP_VER(20, 0), > STEP_B0, > > +STEP_FOREVER)) > > intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL(hdcp- > >cpu_transcoder), > > 0, > TRANS_DDI_HDCP_LINE_REKEY_DISABLE); > > } > > -- > > 2.43.2 > > > > -- > Matt Roper > Graphics Software Engineer > Linux GPU Platform Enablement > Intel Corporation
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index 02cbbbfd8e25..5767070248bb 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -42,10 +42,11 @@ intel_hdcp_disable_hdcp_line_rekeying(struct intel_encoder *encoder, return; if (DISPLAY_VER(dev_priv) >= 14) { - if (IS_METEORLAKE(dev_priv)) + if (IS_DISPLAY_IP_STEP(dev_priv, IP_VER(14, 0), STEP_D0, STEP_FOREVER)) intel_de_rmw(dev_priv, MTL_CHICKEN_TRANS(hdcp->cpu_transcoder), 0, HDCP_LINE_REKEY_DISABLE); - else + else if (IS_DISPLAY_IP_STEP(dev_priv, IP_VER(14, 1), STEP_B0, STEP_FOREVER) || + IS_DISPLAY_IP_STEP(dev_priv, IP_VER(20, 0), STEP_B0, STEP_FOREVER)) intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL(hdcp->cpu_transcoder), 0, TRANS_DDI_HDCP_LINE_REKEY_DISABLE); }
Replace IS_METEORLAKE usage with a more appropriate macro. While we are at it also add the stepping restrictions for other platforms. Fixes: 6a3691ca4799 ("drm/i915/hdcp: Disable HDCP Line Rekeying for HDCP2.2 on HDMI") Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> --- drivers/gpu/drm/i915/display/intel_hdcp.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)