Message ID | 20241008223741.82790-7-matthew.s.atwood@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Add xe3lpd edp enabling | expand |
On Tue, Oct 08, 2024 at 03:37:37PM -0700, Matt Atwood wrote: > From: Suraj Kandpal <suraj.kandpal@intel.com> > > DISPLAY_VER() >= 30 has the HDCP_LINE_REKEY bit redefined from > bit 12 to bit 14. Create a macro to choose the correct bit based Typo? The actual bit moved to 15, not 14. > on DISPLAY_VER(). > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > --- > drivers/gpu/drm/i915/display/intel_hdcp.c | 5 +++-- > drivers/gpu/drm/i915/i915_reg.h | 2 +- > 2 files changed, 4 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..e9b0414590ce 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > @@ -47,10 +47,11 @@ intel_hdcp_disable_hdcp_line_rekeying(struct intel_encoder *encoder, > intel_de_rmw(display, MTL_CHICKEN_TRANS(hdcp->cpu_transcoder), > 0, 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)) > + IS_DISPLAY_VER_STEP(display, IP_VER(20, 0), STEP_B0, STEP_FOREVER) || > + DISPLAY_VER(display) >= 30) Is this correct? The programming here is to account for Wa_16021352814 which applies to MTL, LNL, and BMG (but gives different direction for different steppings of each platform). The workaround does not apply to PTL, so is there something else in the spec indicating that we need to disable the rekeying? Also, the commit message doesn't say anything about this change, only about the disable bit switching spots in the register. Matt > intel_de_rmw(display, > TRANS_DDI_FUNC_CTL(display, hdcp->cpu_transcoder), > - 0, TRANS_DDI_HDCP_LINE_REKEY_DISABLE); > + 0, TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display)); > } > } > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index d30459f8d1cb..da65500cd0c8 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -3832,7 +3832,7 @@ enum skl_power_gate { > #define TRANS_DDI_EDP_INPUT_B_ONOFF (5 << 12) > #define TRANS_DDI_EDP_INPUT_C_ONOFF (6 << 12) > #define TRANS_DDI_EDP_INPUT_D_ONOFF (7 << 12) > -#define TRANS_DDI_HDCP_LINE_REKEY_DISABLE REG_BIT(12) > +#define TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display) (DISPLAY_VER(display) >= 30 ? REG_BIT(15) : REG_BIT(12)) > #define TRANS_DDI_MST_TRANSPORT_SELECT_MASK REG_GENMASK(11, 10) > #define TRANS_DDI_MST_TRANSPORT_SELECT(trans) \ > REG_FIELD_PREP(TRANS_DDI_MST_TRANSPORT_SELECT_MASK, trans) > -- > 2.45.0 >
On Tue, 08 Oct 2024, Matt Atwood <matthew.s.atwood@intel.com> wrote: > From: Suraj Kandpal <suraj.kandpal@intel.com> > > DISPLAY_VER() >= 30 has the HDCP_LINE_REKEY bit redefined from > bit 12 to bit 14. Create a macro to choose the correct bit based > on DISPLAY_VER(). > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > --- > drivers/gpu/drm/i915/display/intel_hdcp.c | 5 +++-- > drivers/gpu/drm/i915/i915_reg.h | 2 +- > 2 files changed, 4 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..e9b0414590ce 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > @@ -47,10 +47,11 @@ intel_hdcp_disable_hdcp_line_rekeying(struct intel_encoder *encoder, > intel_de_rmw(display, MTL_CHICKEN_TRANS(hdcp->cpu_transcoder), > 0, 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)) > + IS_DISPLAY_VER_STEP(display, IP_VER(20, 0), STEP_B0, STEP_FOREVER) || > + DISPLAY_VER(display) >= 30) > intel_de_rmw(display, > TRANS_DDI_FUNC_CTL(display, hdcp->cpu_transcoder), > - 0, TRANS_DDI_HDCP_LINE_REKEY_DISABLE); > + 0, TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display)); > } > } > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index d30459f8d1cb..da65500cd0c8 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -3832,7 +3832,7 @@ enum skl_power_gate { > #define TRANS_DDI_EDP_INPUT_B_ONOFF (5 << 12) > #define TRANS_DDI_EDP_INPUT_C_ONOFF (6 << 12) > #define TRANS_DDI_EDP_INPUT_D_ONOFF (7 << 12) > -#define TRANS_DDI_HDCP_LINE_REKEY_DISABLE REG_BIT(12) > +#define TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display) (DISPLAY_VER(display) >= 30 ? REG_BIT(15) : REG_BIT(12)) Do we really want to extend this style to individual bits? BR, Jani. > #define TRANS_DDI_MST_TRANSPORT_SELECT_MASK REG_GENMASK(11, 10) > #define TRANS_DDI_MST_TRANSPORT_SELECT(trans) \ > REG_FIELD_PREP(TRANS_DDI_MST_TRANSPORT_SELECT_MASK, trans)
> -----Original Message----- > From: Roper, Matthew D <matthew.d.roper@intel.com> > Sent: Wednesday, October 9, 2024 5:07 AM > To: Atwood, Matthew S <matthew.s.atwood@intel.com> > Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Kandpal, > Suraj <suraj.kandpal@intel.com> > Subject: Re: [PATCH 06/10] drm/i915/xe3lpd: Add macro to choose > HDCP_LINE_REKEY bit > > On Tue, Oct 08, 2024 at 03:37:37PM -0700, Matt Atwood wrote: > > From: Suraj Kandpal <suraj.kandpal@intel.com> > > > > DISPLAY_VER() >= 30 has the HDCP_LINE_REKEY bit redefined from bit 12 > > to bit 14. Create a macro to choose the correct bit based > > Typo? The actual bit moved to 15, not 14. Yes that is a typo it should have been bit 15 > > > on DISPLAY_VER(). > > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_hdcp.c | 5 +++-- > > drivers/gpu/drm/i915/i915_reg.h | 2 +- > > 2 files changed, 4 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..e9b0414590ce 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > > @@ -47,10 +47,11 @@ intel_hdcp_disable_hdcp_line_rekeying(struct > intel_encoder *encoder, > > intel_de_rmw(display, MTL_CHICKEN_TRANS(hdcp- > >cpu_transcoder), > > 0, 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)) > > + IS_DISPLAY_VER_STEP(display, IP_VER(20, 0), > STEP_B0, STEP_FOREVER) || > > + DISPLAY_VER(display) >= 30) > > Is this correct? The programming here is to account for Wa_16021352814 > which applies to MTL, LNL, and BMG (but gives different direction for > different steppings of each platform). The workaround does not apply to > PTL, so is there something else in the spec indicating that we need to > disable the rekeying? > > Also, the commit message doesn't say anything about this change, only > about the disable bit switching spots in the register. Right that is the hblank restriction that comes for the following platoforms MTL >= D0 BMG >=B0 LNL >= B0 And for all platforms of PTL So the line rekeying needs to be done for the following and below that the hblank restriction needs to be done So this code just ends up adding code to do line rekeying for the above platforms. We can add the part that we need to add ptl to the platforms for which line rekeying needs to be done Regards, Suraj Kandpal > > > Matt > > > intel_de_rmw(display, > > TRANS_DDI_FUNC_CTL(display, hdcp- > >cpu_transcoder), > > - 0, > TRANS_DDI_HDCP_LINE_REKEY_DISABLE); > > + 0, > TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display)); > > } > > } > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h index d30459f8d1cb..da65500cd0c8 > > 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -3832,7 +3832,7 @@ enum skl_power_gate { > > #define TRANS_DDI_EDP_INPUT_B_ONOFF (5 << 12) > > #define TRANS_DDI_EDP_INPUT_C_ONOFF (6 << 12) > > #define TRANS_DDI_EDP_INPUT_D_ONOFF (7 << 12) > > -#define TRANS_DDI_HDCP_LINE_REKEY_DISABLE REG_BIT(12) > > +#define TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display) > (DISPLAY_VER(display) >= 30 ? REG_BIT(15) : REG_BIT(12)) > > #define TRANS_DDI_MST_TRANSPORT_SELECT_MASK > REG_GENMASK(11, 10) > > #define TRANS_DDI_MST_TRANSPORT_SELECT(trans) \ > > REG_FIELD_PREP(TRANS_DDI_MST_TRANSPORT_SELECT_MASK, > trans) > > -- > > 2.45.0 > > > > -- > Matt Roper > Graphics Software Engineer > Linux GPU Platform Enablement > Intel Corporation
> -----Original Message----- > From: Jani Nikula <jani.nikula@linux.intel.com> > Sent: Wednesday, October 9, 2024 1:09 PM > To: Atwood, Matthew S <matthew.s.atwood@intel.com>; intel- > xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > Cc: Kandpal, Suraj <suraj.kandpal@intel.com>; Atwood, Matthew S > <matthew.s.atwood@intel.com> > Subject: Re: [PATCH 06/10] drm/i915/xe3lpd: Add macro to choose > HDCP_LINE_REKEY bit > > On Tue, 08 Oct 2024, Matt Atwood <matthew.s.atwood@intel.com> wrote: > > From: Suraj Kandpal <suraj.kandpal@intel.com> > > > > DISPLAY_VER() >= 30 has the HDCP_LINE_REKEY bit redefined from bit 12 > > to bit 14. Create a macro to choose the correct bit based on > > DISPLAY_VER(). > > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_hdcp.c | 5 +++-- > > drivers/gpu/drm/i915/i915_reg.h | 2 +- > > 2 files changed, 4 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..e9b0414590ce 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > > @@ -47,10 +47,11 @@ intel_hdcp_disable_hdcp_line_rekeying(struct > intel_encoder *encoder, > > intel_de_rmw(display, MTL_CHICKEN_TRANS(hdcp- > >cpu_transcoder), > > 0, 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)) > > + IS_DISPLAY_VER_STEP(display, IP_VER(20, 0), > STEP_B0, STEP_FOREVER) || > > + DISPLAY_VER(display) >= 30) > > intel_de_rmw(display, > > TRANS_DDI_FUNC_CTL(display, hdcp- > >cpu_transcoder), > > - 0, > TRANS_DDI_HDCP_LINE_REKEY_DISABLE); > > + 0, > TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display)); > > } > > } > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h index d30459f8d1cb..da65500cd0c8 > > 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -3832,7 +3832,7 @@ enum skl_power_gate { > > #define TRANS_DDI_EDP_INPUT_B_ONOFF (5 << 12) > > #define TRANS_DDI_EDP_INPUT_C_ONOFF (6 << 12) > > #define TRANS_DDI_EDP_INPUT_D_ONOFF (7 << 12) > > -#define TRANS_DDI_HDCP_LINE_REKEY_DISABLE REG_BIT(12) > > +#define TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display) > (DISPLAY_VER(display) >= 30 ? REG_BIT(15) : REG_BIT(12)) > > Do we really want to extend this style to individual bits? I just thought this might be cleaner should we use TRANS_DDI_HDCP_LINE_REKEY_DISABLE And XE3_TRANS_DDI_HDCP_LINE_REKEY_DISABLE instead then? Regards, Suraj Kandpal > > BR, > Jani. > > > #define TRANS_DDI_MST_TRANSPORT_SELECT_MASK > REG_GENMASK(11, 10) > > #define TRANS_DDI_MST_TRANSPORT_SELECT(trans) \ > > REG_FIELD_PREP(TRANS_DDI_MST_TRANSPORT_SELECT_MASK, > trans) > > -- > Jani Nikula, Intel
On Thu, 10 Oct 2024, "Kandpal, Suraj" <suraj.kandpal@intel.com> wrote: >> -----Original Message----- >> From: Jani Nikula <jani.nikula@linux.intel.com> >> Sent: Wednesday, October 9, 2024 1:09 PM >> To: Atwood, Matthew S <matthew.s.atwood@intel.com>; intel- >> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org >> Cc: Kandpal, Suraj <suraj.kandpal@intel.com>; Atwood, Matthew S >> <matthew.s.atwood@intel.com> >> Subject: Re: [PATCH 06/10] drm/i915/xe3lpd: Add macro to choose >> HDCP_LINE_REKEY bit >> >> On Tue, 08 Oct 2024, Matt Atwood <matthew.s.atwood@intel.com> wrote: >> > From: Suraj Kandpal <suraj.kandpal@intel.com> >> > >> > DISPLAY_VER() >= 30 has the HDCP_LINE_REKEY bit redefined from bit 12 >> > to bit 14. Create a macro to choose the correct bit based on >> > DISPLAY_VER(). >> > >> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> >> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> >> > --- >> > drivers/gpu/drm/i915/display/intel_hdcp.c | 5 +++-- >> > drivers/gpu/drm/i915/i915_reg.h | 2 +- >> > 2 files changed, 4 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..e9b0414590ce 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c >> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c >> > @@ -47,10 +47,11 @@ intel_hdcp_disable_hdcp_line_rekeying(struct >> intel_encoder *encoder, >> > intel_de_rmw(display, MTL_CHICKEN_TRANS(hdcp- >> >cpu_transcoder), >> > 0, 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)) >> > + IS_DISPLAY_VER_STEP(display, IP_VER(20, 0), >> STEP_B0, STEP_FOREVER) || >> > + DISPLAY_VER(display) >= 30) >> > intel_de_rmw(display, >> > TRANS_DDI_FUNC_CTL(display, hdcp- >> >cpu_transcoder), >> > - 0, >> TRANS_DDI_HDCP_LINE_REKEY_DISABLE); >> > + 0, >> TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display)); >> > } >> > } >> > >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h >> > b/drivers/gpu/drm/i915/i915_reg.h index d30459f8d1cb..da65500cd0c8 >> > 100644 >> > --- a/drivers/gpu/drm/i915/i915_reg.h >> > +++ b/drivers/gpu/drm/i915/i915_reg.h >> > @@ -3832,7 +3832,7 @@ enum skl_power_gate { >> > #define TRANS_DDI_EDP_INPUT_B_ONOFF (5 << 12) >> > #define TRANS_DDI_EDP_INPUT_C_ONOFF (6 << 12) >> > #define TRANS_DDI_EDP_INPUT_D_ONOFF (7 << 12) >> > -#define TRANS_DDI_HDCP_LINE_REKEY_DISABLE REG_BIT(12) >> > +#define TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display) >> (DISPLAY_VER(display) >= 30 ? REG_BIT(15) : REG_BIT(12)) >> >> Do we really want to extend this style to individual bits? > > I just thought this might be cleaner should we use > TRANS_DDI_HDCP_LINE_REKEY_DISABLE > And > XE3_TRANS_DDI_HDCP_LINE_REKEY_DISABLE instead then? I think so yes. If it becomes too ugly in code, at the very least define the bits separately instead of inline in the ternary operator. BR, Jani. > > Regards, > Suraj Kandpal > >> >> BR, >> Jani. >> >> > #define TRANS_DDI_MST_TRANSPORT_SELECT_MASK >> REG_GENMASK(11, 10) >> > #define TRANS_DDI_MST_TRANSPORT_SELECT(trans) \ >> > REG_FIELD_PREP(TRANS_DDI_MST_TRANSPORT_SELECT_MASK, >> trans) >> >> -- >> Jani Nikula, Intel
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index ed6aa87403e2..e9b0414590ce 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -47,10 +47,11 @@ intel_hdcp_disable_hdcp_line_rekeying(struct intel_encoder *encoder, intel_de_rmw(display, MTL_CHICKEN_TRANS(hdcp->cpu_transcoder), 0, 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)) + IS_DISPLAY_VER_STEP(display, IP_VER(20, 0), STEP_B0, STEP_FOREVER) || + DISPLAY_VER(display) >= 30) intel_de_rmw(display, TRANS_DDI_FUNC_CTL(display, hdcp->cpu_transcoder), - 0, TRANS_DDI_HDCP_LINE_REKEY_DISABLE); + 0, TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display)); } } diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index d30459f8d1cb..da65500cd0c8 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3832,7 +3832,7 @@ enum skl_power_gate { #define TRANS_DDI_EDP_INPUT_B_ONOFF (5 << 12) #define TRANS_DDI_EDP_INPUT_C_ONOFF (6 << 12) #define TRANS_DDI_EDP_INPUT_D_ONOFF (7 << 12) -#define TRANS_DDI_HDCP_LINE_REKEY_DISABLE REG_BIT(12) +#define TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display) (DISPLAY_VER(display) >= 30 ? REG_BIT(15) : REG_BIT(12)) #define TRANS_DDI_MST_TRANSPORT_SELECT_MASK REG_GENMASK(11, 10) #define TRANS_DDI_MST_TRANSPORT_SELECT(trans) \ REG_FIELD_PREP(TRANS_DDI_MST_TRANSPORT_SELECT_MASK, trans)