| Message ID | 20241015231124.23982-4-matthew.s.atwood@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add xe3lpd edp enabling | expand |
On Tue, Oct 15, 2024 at 04:11:20PM -0700, Matt Atwood wrote: > From: Suraj Kandpal <suraj.kandpal@intel.com> > > hblank restriction now includes all of xe3. > > v2: add additional definition instead of function, commit message typo > fix and update. > v3: restore lost conditional from v2. > > 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 | 4 ++++ > drivers/gpu/drm/i915/i915_reg.h | 1 + > 2 files changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c > index ed6aa87403e2..a99b41f258e4 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > @@ -51,6 +51,10 @@ intel_hdcp_disable_hdcp_line_rekeying(struct intel_encoder *encoder, > intel_de_rmw(display, > TRANS_DDI_FUNC_CTL(display, hdcp->cpu_transcoder), > 0, TRANS_DDI_HDCP_LINE_REKEY_DISABLE); > + else if (DISPLAY_VER(display) >=30) We might want to take this opportunity to update the if/else ladder with standard "newest platform first" rather than continuing to stack these things in the wrong order. Also, you're missing a space after the comparison here. But more fundamentally I still think there needs to be more justification in the commit message for this. Right now we're editing a function that's labelled "/* WA: 16022217614 */" which implies we shouldn't be doing real work here on platforms that the workaround doesn't apply to (as is the case for Xe3). So we probably need to move that comment down to the specific conditions for the older platforms that are impacted (and maybe re-write it in standard Wa_16022217614 notation to make it more greppable), and also add a bspec reference to the commit message that reviewers can check to confirm that we really should be disabling rekeying on platforms not impacted by the workaround. > + intel_de_rmw(display, > + TRANS_DDI_FUNC_CTL(display, hdcp->cpu_transcoder), > + 0, XE3_TRANS_DDI_HDCP_LINE_REKEY_DISABLE); > } > } > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index d30459f8d1cb..fc30e0056b07 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -3833,6 +3833,7 @@ enum skl_power_gate { > #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 XE3_TRANS_DDI_HDCP_LINE_REKEY_DISABLE REG_BIT(15) This line isn't sorted properly. Matt > #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 >
> -----Original Message----- > From: Roper, Matthew D <matthew.d.roper@intel.com> > Sent: Wednesday, October 16, 2024 8:58 PM > 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 v3 3/7] drm/i915/xe3lpd: Include hblank restriction for > xe3lpd > > On Tue, Oct 15, 2024 at 04:11:20PM -0700, Matt Atwood wrote: > > From: Suraj Kandpal <suraj.kandpal@intel.com> > > > > hblank restriction now includes all of xe3. > > > > v2: add additional definition instead of function, commit message typo > > fix and update. > > v3: restore lost conditional from v2. > > > > 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 | 4 ++++ > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > 2 files changed, 5 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > > b/drivers/gpu/drm/i915/display/intel_hdcp.c > > index ed6aa87403e2..a99b41f258e4 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > > @@ -51,6 +51,10 @@ intel_hdcp_disable_hdcp_line_rekeying(struct > intel_encoder *encoder, > > intel_de_rmw(display, > > TRANS_DDI_FUNC_CTL(display, hdcp- > >cpu_transcoder), > > 0, > TRANS_DDI_HDCP_LINE_REKEY_DISABLE); > > + else if (DISPLAY_VER(display) >=30) > > We might want to take this opportunity to update the if/else ladder with > standard "newest platform first" rather than continuing to stack these things > in the wrong order. Also, you're missing a space after the comparison here. Okay got it will get the if ladder fixed here > > But more fundamentally I still think there needs to be more justification in > the commit message for this. Right now we're editing a function that's > labelled "/* WA: 16022217614 */" which implies we shouldn't be doing real > work here on platforms that the workaround doesn't apply to (as is the case > for Xe3). So we probably need to move that comment down to the specific > conditions for the older platforms that are impacted (and maybe re-write it > in standard Wa_16022217614 notation to make it more greppable), and also > add a bspec reference to the commit message that reviewers can check to > confirm that we really should be disabling rekeying on platforms not > impacted by the workaround. You are right this the commit message and subject here should have been "Disable HDCP Line Rekeying for Xe3" And "We need to disable HDCP Line Rekeying for Xe3 when we are using an HDMI encoder" as this is what the bspec expects of us and really not the WA (which was applied only for a few gen for the early stepping versions) we are really not restricting the hblank here since this pruning modes based on if HDCP is enabled at this point wont be possible. As HDCP get enabled after modes are already pruned so when modes are being pruned HDCP will not be disabled. Regards, Suraj Kandpal > > > + intel_de_rmw(display, > > + TRANS_DDI_FUNC_CTL(display, hdcp- > >cpu_transcoder), > > + 0, > XE3_TRANS_DDI_HDCP_LINE_REKEY_DISABLE); > > } > > } > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h index d30459f8d1cb..fc30e0056b07 > > 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -3833,6 +3833,7 @@ enum skl_power_gate { > > #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 XE3_TRANS_DDI_HDCP_LINE_REKEY_DISABLE REG_BIT(15) > > This line isn't sorted properly. > > > Matt > > > #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
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index ed6aa87403e2..a99b41f258e4 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -51,6 +51,10 @@ intel_hdcp_disable_hdcp_line_rekeying(struct intel_encoder *encoder, intel_de_rmw(display, TRANS_DDI_FUNC_CTL(display, hdcp->cpu_transcoder), 0, TRANS_DDI_HDCP_LINE_REKEY_DISABLE); + else 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); } } diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index d30459f8d1cb..fc30e0056b07 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3833,6 +3833,7 @@ enum skl_power_gate { #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 XE3_TRANS_DDI_HDCP_LINE_REKEY_DISABLE REG_BIT(15) #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)