| Message ID | 20221130231709.4870-1-matthew.s.atwood@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915/mtl: Initial display workarounds | expand |
On 30/11/2022 23:17, Matt Atwood wrote: > From: Jouni Högander <jouni.hogander@intel.com> > > This patch introduces initial workarounds for mtl platform > > Bspec: 66624 > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> > --- > drivers/gpu/drm/i915/display/intel_fbc.c | 4 +++- > drivers/gpu/drm/i915/display/intel_hdmi.c | 3 ++- > drivers/gpu/drm/i915/display/intel_psr.c | 27 ++++++++++++++++------- > drivers/gpu/drm/i915/i915_drv.h | 4 ++++ > 4 files changed, 28 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c > index b5ee5ea0d010..8f269553d826 100644 > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > @@ -1095,7 +1095,9 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state, > } > > /* Wa_14016291713 */ > - if (IS_DISPLAY_VER(i915, 12, 13) && crtc_state->has_psr) { > + if ((IS_DISPLAY_VER(i915, 12, 13) || > + IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && > + crtc_state->has_psr) { > plane_state->no_fbc_reason = "PSR1 enabled (Wa_14016291713)"; > return 0; > } > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c > index e82f8a07e2b0..efa2da080f62 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c > @@ -537,7 +537,8 @@ void hsw_write_infoframe(struct intel_encoder *encoder, > 0); > > /* Wa_14013475917 */ > - if (DISPLAY_VER(dev_priv) == 13 && crtc_state->has_psr && > + if ((DISPLAY_VER(dev_priv) == 13 || > + IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) && crtc_state->has_psr && > type == DP_SDP_VSC) > return; > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > index 5b678916e6db..7982157fb1ff 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -797,7 +797,7 @@ static bool psr2_granularity_check(struct intel_dp *intel_dp, > return intel_dp->psr.su_y_granularity == 4; > > /* > - * adl_p and display 14+ platforms has 1 line granularity. > + * adl_p and mtl platforms has 1 line granularity. > * For other platforms with SW tracking we can adjust the y coordinates > * to match sink requirement if multiple of 4. > */ > @@ -1170,11 +1170,14 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, > PSR2_ADD_VERTICAL_LINE_COUNT); > > /* > - * Wa_16014451276:adlp > + * Wa_16014451276:adlp,mtl[a0,b0] > * All supported adlp panels have 1-based X granularity, this may > * cause issues if non-supported panels are used. > */ > - if (IS_ALDERLAKE_P(dev_priv)) > + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) > + intel_de_rmw(dev_priv, MTL_CHICKEN_TRANS(cpu_transcoder), 0, > + ADLP_1_BASED_X_GRANULARITY); > + else if (IS_ALDERLAKE_P(dev_priv)) > intel_de_rmw(dev_priv, CHICKEN_TRANS(cpu_transcoder), 0, > ADLP_1_BASED_X_GRANULARITY); > > @@ -1185,8 +1188,12 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, > TRANS_SET_CONTEXT_LATENCY_MASK, > TRANS_SET_CONTEXT_LATENCY_VALUE(1)); > > - /* Wa_16012604467:adlp */ > - if (IS_ALDERLAKE_P(dev_priv)) > + /* Wa_16012604467:adlp,mtl[a0,b0] */ > + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) > + intel_de_rmw(dev_priv, > + MTL_CLKGATE_DIS_TRANS(cpu_transcoder), 0, > + MTL_CLKGATE_DIS_TRANS_DMASC_GATING_DIS); > + else if (IS_ALDERLAKE_P(dev_priv)) > intel_de_rmw(dev_priv, CLKGATE_DIS_MISC, 0, > CLKGATE_DIS_MISC_DMASC_GATING_DIS); > > @@ -1362,8 +1369,12 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp) > TRANS_SET_CONTEXT_LATENCY(intel_dp->psr.transcoder), > TRANS_SET_CONTEXT_LATENCY_MASK, 0); > > - /* Wa_16012604467:adlp */ > - if (IS_ALDERLAKE_P(dev_priv)) > + /* Wa_16012604467:adlp,mtl[a0,b0] */ > + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) > + intel_de_rmw(dev_priv, > + MTL_CLKGATE_DIS_TRANS(intel_dp->psr.transcoder), > + MTL_CLKGATE_DIS_TRANS_DMASC_GATING_DIS, 0); > + else if (IS_ALDERLAKE_P(dev_priv)) > intel_de_rmw(dev_priv, CLKGATE_DIS_MISC, > CLKGATE_DIS_MISC_DMASC_GATING_DIS, 0); > > @@ -1625,7 +1636,7 @@ static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state, > > if (full_update) { > /* > - * Not applying Wa_14014971508:adlp as we do not support the > + * Not applying Wa_14014971508:adlp,mtl as we do not support the > * feature that requires this workaround. > */ > val |= man_trk_ctl_single_full_frame_bit_get(dev_priv); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index a8a5bd426e78..ecb027626a21 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -727,6 +727,10 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, > (IS_SUBPLATFORM(__i915, INTEL_METEORLAKE, INTEL_SUBPLATFORM_##variant) && \ > IS_GRAPHICS_STEP(__i915, since, until)) > > +#define IS_MTL_DISPLAY_STEP(__i915, since, until) \ > + (DISPLAY_VER(__i915) == 14 && \ > + IS_DISPLAY_STEP(__i915, since, until)) You might want IS_METEORLAKE somewhere in here if macro is called IS_MTL_.. otherwise it may surprise in the future. Unless it is guaranteed MTL will be the only with display_ver == 14. Take care of checkpatch warnings as well please. Regards, Tvrtko > + > /* > * DG2 hardware steppings are a bit unusual. The hardware design was forked to > * create three variants (G10, G11, and G12) which each have distinct
On Thu, Dec 01, 2022 at 12:51:33PM +0000, Tvrtko Ursulin wrote: > > On 30/11/2022 23:17, Matt Atwood wrote: > > From: Jouni Högander <jouni.hogander@intel.com> > > > > This patch introduces initial workarounds for mtl platform > > > > Bspec: 66624 > > > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_fbc.c | 4 +++- > > drivers/gpu/drm/i915/display/intel_hdmi.c | 3 ++- > > drivers/gpu/drm/i915/display/intel_psr.c | 27 ++++++++++++++++------- > > drivers/gpu/drm/i915/i915_drv.h | 4 ++++ > > 4 files changed, 28 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c > > index b5ee5ea0d010..8f269553d826 100644 > > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > > @@ -1095,7 +1095,9 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state, > > } > > /* Wa_14016291713 */ > > - if (IS_DISPLAY_VER(i915, 12, 13) && crtc_state->has_psr) { > > + if ((IS_DISPLAY_VER(i915, 12, 13) || > > + IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && > > + crtc_state->has_psr) { > > plane_state->no_fbc_reason = "PSR1 enabled (Wa_14016291713)"; > > return 0; > > } > > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c > > index e82f8a07e2b0..efa2da080f62 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c > > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c > > @@ -537,7 +537,8 @@ void hsw_write_infoframe(struct intel_encoder *encoder, > > 0); > > /* Wa_14013475917 */ > > - if (DISPLAY_VER(dev_priv) == 13 && crtc_state->has_psr && > > + if ((DISPLAY_VER(dev_priv) == 13 || > > + IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) && crtc_state->has_psr && > > type == DP_SDP_VSC) > > return; > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > > index 5b678916e6db..7982157fb1ff 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > @@ -797,7 +797,7 @@ static bool psr2_granularity_check(struct intel_dp *intel_dp, > > return intel_dp->psr.su_y_granularity == 4; > > /* > > - * adl_p and display 14+ platforms has 1 line granularity. > > + * adl_p and mtl platforms has 1 line granularity. > > * For other platforms with SW tracking we can adjust the y coordinates > > * to match sink requirement if multiple of 4. > > */ > > @@ -1170,11 +1170,14 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, > > PSR2_ADD_VERTICAL_LINE_COUNT); > > /* > > - * Wa_16014451276:adlp > > + * Wa_16014451276:adlp,mtl[a0,b0] > > * All supported adlp panels have 1-based X granularity, this may > > * cause issues if non-supported panels are used. > > */ > > - if (IS_ALDERLAKE_P(dev_priv)) > > + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) > > + intel_de_rmw(dev_priv, MTL_CHICKEN_TRANS(cpu_transcoder), 0, > > + ADLP_1_BASED_X_GRANULARITY); > > + else if (IS_ALDERLAKE_P(dev_priv)) > > intel_de_rmw(dev_priv, CHICKEN_TRANS(cpu_transcoder), 0, > > ADLP_1_BASED_X_GRANULARITY); > > @@ -1185,8 +1188,12 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, > > TRANS_SET_CONTEXT_LATENCY_MASK, > > TRANS_SET_CONTEXT_LATENCY_VALUE(1)); > > - /* Wa_16012604467:adlp */ > > - if (IS_ALDERLAKE_P(dev_priv)) > > + /* Wa_16012604467:adlp,mtl[a0,b0] */ > > + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) > > + intel_de_rmw(dev_priv, > > + MTL_CLKGATE_DIS_TRANS(cpu_transcoder), 0, > > + MTL_CLKGATE_DIS_TRANS_DMASC_GATING_DIS); > > + else if (IS_ALDERLAKE_P(dev_priv)) > > intel_de_rmw(dev_priv, CLKGATE_DIS_MISC, 0, > > CLKGATE_DIS_MISC_DMASC_GATING_DIS); > > @@ -1362,8 +1369,12 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp) > > TRANS_SET_CONTEXT_LATENCY(intel_dp->psr.transcoder), > > TRANS_SET_CONTEXT_LATENCY_MASK, 0); > > - /* Wa_16012604467:adlp */ > > - if (IS_ALDERLAKE_P(dev_priv)) > > + /* Wa_16012604467:adlp,mtl[a0,b0] */ > > + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) > > + intel_de_rmw(dev_priv, > > + MTL_CLKGATE_DIS_TRANS(intel_dp->psr.transcoder), > > + MTL_CLKGATE_DIS_TRANS_DMASC_GATING_DIS, 0); > > + else if (IS_ALDERLAKE_P(dev_priv)) > > intel_de_rmw(dev_priv, CLKGATE_DIS_MISC, > > CLKGATE_DIS_MISC_DMASC_GATING_DIS, 0); > > @@ -1625,7 +1636,7 @@ static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state, > > if (full_update) { > > /* > > - * Not applying Wa_14014971508:adlp as we do not support the > > + * Not applying Wa_14014971508:adlp,mtl as we do not support the > > * feature that requires this workaround. > > */ > > val |= man_trk_ctl_single_full_frame_bit_get(dev_priv); > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index a8a5bd426e78..ecb027626a21 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -727,6 +727,10 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, > > (IS_SUBPLATFORM(__i915, INTEL_METEORLAKE, INTEL_SUBPLATFORM_##variant) && \ > > IS_GRAPHICS_STEP(__i915, since, until)) > > +#define IS_MTL_DISPLAY_STEP(__i915, since, until) \ > > + (DISPLAY_VER(__i915) == 14 && \ > > + IS_DISPLAY_STEP(__i915, since, until)) > > You might want IS_METEORLAKE somewhere in here if macro is called IS_MTL_.. > otherwise it may surprise in the future. Unless it is guaranteed MTL will be > the only with display_ver == 14. ack > > Take care of checkpatch warnings as well please. Checkpatch failures are all "macro argument reuse '__i915' - possible side-effects?" this appears to be the standard for these macros (the passing of __i915 to this macro to be fed into IS_[DISPLAY|GRAPHICS]_step functions.). Was there something more I'm missing, or were you wanting something more done to the overall format of the file? > > Regards, > > Tvrtko > > > + > > /* > > * DG2 hardware steppings are a bit unusual. The hardware design was forked to > > * create three variants (G10, G11, and G12) which each have distinct
On Wed, Nov 30, 2022 at 03:17:08PM -0800, Matt Atwood wrote: > From: Jouni Högander <jouni.hogander@intel.com> > > This patch introduces initial workarounds for mtl platform > > Bspec: 66624 > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> > --- > drivers/gpu/drm/i915/display/intel_fbc.c | 4 +++- > drivers/gpu/drm/i915/display/intel_hdmi.c | 3 ++- > drivers/gpu/drm/i915/display/intel_psr.c | 27 ++++++++++++++++------- > drivers/gpu/drm/i915/i915_drv.h | 4 ++++ > 4 files changed, 28 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c > index b5ee5ea0d010..8f269553d826 100644 > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > @@ -1095,7 +1095,9 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state, > } > > /* Wa_14016291713 */ > - if (IS_DISPLAY_VER(i915, 12, 13) && crtc_state->has_psr) { > + if ((IS_DISPLAY_VER(i915, 12, 13) || > + IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && > + crtc_state->has_psr) { > plane_state->no_fbc_reason = "PSR1 enabled (Wa_14016291713)"; > return 0; > } > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c > index e82f8a07e2b0..efa2da080f62 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c > @@ -537,7 +537,8 @@ void hsw_write_infoframe(struct intel_encoder *encoder, > 0); > > /* Wa_14013475917 */ > - if (DISPLAY_VER(dev_priv) == 13 && crtc_state->has_psr && > + if ((DISPLAY_VER(dev_priv) == 13 || > + IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) && crtc_state->has_psr && > type == DP_SDP_VSC) > return; > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > index 5b678916e6db..7982157fb1ff 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -797,7 +797,7 @@ static bool psr2_granularity_check(struct intel_dp *intel_dp, > return intel_dp->psr.su_y_granularity == 4; > > /* > - * adl_p and display 14+ platforms has 1 line granularity. > + * adl_p and mtl platforms has 1 line granularity. > * For other platforms with SW tracking we can adjust the y coordinates > * to match sink requirement if multiple of 4. > */ > @@ -1170,11 +1170,14 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, > PSR2_ADD_VERTICAL_LINE_COUNT); > > /* > - * Wa_16014451276:adlp > + * Wa_16014451276:adlp,mtl[a0,b0] These days we don't really add steppings in these comments; at best they're just reiterating information that can be easily seen from the code below, at worst they get out of sync and cause confusion. I'd drop the "[a0,b0]" part (which also isn't accurate anyway if you're using range notation...it would be "[a0..b0)" in that case). Honestly even the list of platform names on workarounds is of questionable value and I'm thinking about writing a patch that just drops all of them throughout i915, leaving just the workaround lineage number. Maybe I'd keep the platform names in the few cases where we have multiple workaround numbers (with different sets of platforms) all requesting the same programming of a register... > * All supported adlp panels have 1-based X granularity, this may > * cause issues if non-supported panels are used. > */ > - if (IS_ALDERLAKE_P(dev_priv)) > + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) > + intel_de_rmw(dev_priv, MTL_CHICKEN_TRANS(cpu_transcoder), 0, > + ADLP_1_BASED_X_GRANULARITY); > + else if (IS_ALDERLAKE_P(dev_priv)) > intel_de_rmw(dev_priv, CHICKEN_TRANS(cpu_transcoder), 0, > ADLP_1_BASED_X_GRANULARITY); > > @@ -1185,8 +1188,12 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, > TRANS_SET_CONTEXT_LATENCY_MASK, > TRANS_SET_CONTEXT_LATENCY_VALUE(1)); > > - /* Wa_16012604467:adlp */ > - if (IS_ALDERLAKE_P(dev_priv)) > + /* Wa_16012604467:adlp,mtl[a0,b0] */ > + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) > + intel_de_rmw(dev_priv, > + MTL_CLKGATE_DIS_TRANS(cpu_transcoder), 0, > + MTL_CLKGATE_DIS_TRANS_DMASC_GATING_DIS); > + else if (IS_ALDERLAKE_P(dev_priv)) > intel_de_rmw(dev_priv, CLKGATE_DIS_MISC, 0, > CLKGATE_DIS_MISC_DMASC_GATING_DIS); > > @@ -1362,8 +1369,12 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp) > TRANS_SET_CONTEXT_LATENCY(intel_dp->psr.transcoder), > TRANS_SET_CONTEXT_LATENCY_MASK, 0); > > - /* Wa_16012604467:adlp */ > - if (IS_ALDERLAKE_P(dev_priv)) > + /* Wa_16012604467:adlp,mtl[a0,b0] */ > + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) > + intel_de_rmw(dev_priv, > + MTL_CLKGATE_DIS_TRANS(intel_dp->psr.transcoder), > + MTL_CLKGATE_DIS_TRANS_DMASC_GATING_DIS, 0); > + else if (IS_ALDERLAKE_P(dev_priv)) > intel_de_rmw(dev_priv, CLKGATE_DIS_MISC, > CLKGATE_DIS_MISC_DMASC_GATING_DIS, 0); > > @@ -1625,7 +1636,7 @@ static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state, > > if (full_update) { > /* > - * Not applying Wa_14014971508:adlp as we do not support the > + * Not applying Wa_14014971508:adlp,mtl as we do not support the > * feature that requires this workaround. > */ > val |= man_trk_ctl_single_full_frame_bit_get(dev_priv); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index a8a5bd426e78..ecb027626a21 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -727,6 +727,10 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, > (IS_SUBPLATFORM(__i915, INTEL_METEORLAKE, INTEL_SUBPLATFORM_##variant) && \ > IS_GRAPHICS_STEP(__i915, since, until)) > > +#define IS_MTL_DISPLAY_STEP(__i915, since, until) \ > + (DISPLAY_VER(__i915) == 14 && \ As Tvrtko noted, this could come back to bite us in the future if another platform shows up with 14.10, 14.50, etc. MTL has exactly version 14.0, so best to make this DISPLAY_VER_FULL(__i915) == IP_VER(14, 0) so that it won't automatically capture future platforms by accident. Matt > + IS_DISPLAY_STEP(__i915, since, until)) > + > /* > * DG2 hardware steppings are a bit unusual. The hardware design was forked to > * create three variants (G10, G11, and G12) which each have distinct > -- > 2.38.1 >
On Thu, Dec 01, 2022 at 03:01:05PM -0800, Matt Roper wrote: >On Wed, Nov 30, 2022 at 03:17:08PM -0800, Matt Atwood wrote: >> From: Jouni Högander <jouni.hogander@intel.com> >> >> This patch introduces initial workarounds for mtl platform >> >> Bspec: 66624 >> >> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> >> Signed-off-by: Jouni Högander <jouni.hogander@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_fbc.c | 4 +++- >> drivers/gpu/drm/i915/display/intel_hdmi.c | 3 ++- >> drivers/gpu/drm/i915/display/intel_psr.c | 27 ++++++++++++++++------- >> drivers/gpu/drm/i915/i915_drv.h | 4 ++++ >> 4 files changed, 28 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c >> index b5ee5ea0d010..8f269553d826 100644 >> --- a/drivers/gpu/drm/i915/display/intel_fbc.c >> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c >> @@ -1095,7 +1095,9 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state, >> } >> >> /* Wa_14016291713 */ >> - if (IS_DISPLAY_VER(i915, 12, 13) && crtc_state->has_psr) { >> + if ((IS_DISPLAY_VER(i915, 12, 13) || >> + IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && >> + crtc_state->has_psr) { >> plane_state->no_fbc_reason = "PSR1 enabled (Wa_14016291713)"; >> return 0; >> } >> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c >> index e82f8a07e2b0..efa2da080f62 100644 >> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c >> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c >> @@ -537,7 +537,8 @@ void hsw_write_infoframe(struct intel_encoder *encoder, >> 0); >> >> /* Wa_14013475917 */ >> - if (DISPLAY_VER(dev_priv) == 13 && crtc_state->has_psr && >> + if ((DISPLAY_VER(dev_priv) == 13 || >> + IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) && crtc_state->has_psr && >> type == DP_SDP_VSC) >> return; >> >> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c >> index 5b678916e6db..7982157fb1ff 100644 >> --- a/drivers/gpu/drm/i915/display/intel_psr.c >> +++ b/drivers/gpu/drm/i915/display/intel_psr.c >> @@ -797,7 +797,7 @@ static bool psr2_granularity_check(struct intel_dp *intel_dp, >> return intel_dp->psr.su_y_granularity == 4; >> >> /* >> - * adl_p and display 14+ platforms has 1 line granularity. >> + * adl_p and mtl platforms has 1 line granularity. >> * For other platforms with SW tracking we can adjust the y coordinates >> * to match sink requirement if multiple of 4. >> */ >> @@ -1170,11 +1170,14 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, >> PSR2_ADD_VERTICAL_LINE_COUNT); >> >> /* >> - * Wa_16014451276:adlp >> + * Wa_16014451276:adlp,mtl[a0,b0] > >These days we don't really add steppings in these comments; at best >they're just reiterating information that can be easily seen from the >code below, at worst they get out of sync and cause confusion. I'd drop >the "[a0,b0]" part (which also isn't accurate anyway if you're using >range notation...it would be "[a0..b0)" in that case). > >Honestly even the list of platform names on workarounds is of >questionable value and I'm thinking about writing a patch that just >drops all of them throughout i915, leaving just the workaround lineage >number. Maybe I'd keep the platform names in the few cases where we >have multiple workaround numbers (with different sets of platforms) all >requesting the same programming of a register... I'd be happy to ack such patch :) > >> * All supported adlp panels have 1-based X granularity, this may >> * cause issues if non-supported panels are used. >> */ >> - if (IS_ALDERLAKE_P(dev_priv)) >> + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) >> + intel_de_rmw(dev_priv, MTL_CHICKEN_TRANS(cpu_transcoder), 0, >> + ADLP_1_BASED_X_GRANULARITY); >> + else if (IS_ALDERLAKE_P(dev_priv)) >> intel_de_rmw(dev_priv, CHICKEN_TRANS(cpu_transcoder), 0, >> ADLP_1_BASED_X_GRANULARITY); >> >> @@ -1185,8 +1188,12 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, >> TRANS_SET_CONTEXT_LATENCY_MASK, >> TRANS_SET_CONTEXT_LATENCY_VALUE(1)); >> >> - /* Wa_16012604467:adlp */ >> - if (IS_ALDERLAKE_P(dev_priv)) >> + /* Wa_16012604467:adlp,mtl[a0,b0] */ >> + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) >> + intel_de_rmw(dev_priv, >> + MTL_CLKGATE_DIS_TRANS(cpu_transcoder), 0, >> + MTL_CLKGATE_DIS_TRANS_DMASC_GATING_DIS); >> + else if (IS_ALDERLAKE_P(dev_priv)) >> intel_de_rmw(dev_priv, CLKGATE_DIS_MISC, 0, >> CLKGATE_DIS_MISC_DMASC_GATING_DIS); >> >> @@ -1362,8 +1369,12 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp) >> TRANS_SET_CONTEXT_LATENCY(intel_dp->psr.transcoder), >> TRANS_SET_CONTEXT_LATENCY_MASK, 0); >> >> - /* Wa_16012604467:adlp */ >> - if (IS_ALDERLAKE_P(dev_priv)) >> + /* Wa_16012604467:adlp,mtl[a0,b0] */ >> + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) >> + intel_de_rmw(dev_priv, >> + MTL_CLKGATE_DIS_TRANS(intel_dp->psr.transcoder), >> + MTL_CLKGATE_DIS_TRANS_DMASC_GATING_DIS, 0); >> + else if (IS_ALDERLAKE_P(dev_priv)) >> intel_de_rmw(dev_priv, CLKGATE_DIS_MISC, >> CLKGATE_DIS_MISC_DMASC_GATING_DIS, 0); >> >> @@ -1625,7 +1636,7 @@ static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state, >> >> if (full_update) { >> /* >> - * Not applying Wa_14014971508:adlp as we do not support the >> + * Not applying Wa_14014971508:adlp,mtl as we do not support the >> * feature that requires this workaround. >> */ >> val |= man_trk_ctl_single_full_frame_bit_get(dev_priv); >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index a8a5bd426e78..ecb027626a21 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -727,6 +727,10 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, >> (IS_SUBPLATFORM(__i915, INTEL_METEORLAKE, INTEL_SUBPLATFORM_##variant) && \ >> IS_GRAPHICS_STEP(__i915, since, until)) >> >> +#define IS_MTL_DISPLAY_STEP(__i915, since, until) \ >> + (DISPLAY_VER(__i915) == 14 && \ > >As Tvrtko noted, this could come back to bite us in the future if >another platform shows up with 14.10, 14.50, etc. MTL has exactly >version 14.0, so best to make this > > DISPLAY_VER_FULL(__i915) == IP_VER(14, 0) > >so that it won't automatically capture future platforms by accident. I think it's better to do a platform check as the other platforms are doing. See DG2 for example: #define IS_DG2_DISPLAY_STEP(__i915, since, until) \ (IS_DG2(__i915) && \ IS_DISPLAY_STEP(__i915, since, until)) Lucas De Marchi
On Thu, Dec 01, 2022 at 03:27:25PM -0800, Lucas De Marchi wrote: > On Thu, Dec 01, 2022 at 03:01:05PM -0800, Matt Roper wrote: > > On Wed, Nov 30, 2022 at 03:17:08PM -0800, Matt Atwood wrote: > > > From: Jouni Högander <jouni.hogander@intel.com> > > > > > > This patch introduces initial workarounds for mtl platform > > > > > > Bspec: 66624 > > > > > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> > > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_fbc.c | 4 +++- > > > drivers/gpu/drm/i915/display/intel_hdmi.c | 3 ++- > > > drivers/gpu/drm/i915/display/intel_psr.c | 27 ++++++++++++++++------- > > > drivers/gpu/drm/i915/i915_drv.h | 4 ++++ > > > 4 files changed, 28 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c > > > index b5ee5ea0d010..8f269553d826 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > > > @@ -1095,7 +1095,9 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state, > > > } > > > > > > /* Wa_14016291713 */ > > > - if (IS_DISPLAY_VER(i915, 12, 13) && crtc_state->has_psr) { > > > + if ((IS_DISPLAY_VER(i915, 12, 13) || > > > + IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && > > > + crtc_state->has_psr) { > > > plane_state->no_fbc_reason = "PSR1 enabled (Wa_14016291713)"; > > > return 0; > > > } > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c > > > index e82f8a07e2b0..efa2da080f62 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c > > > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c > > > @@ -537,7 +537,8 @@ void hsw_write_infoframe(struct intel_encoder *encoder, > > > 0); > > > > > > /* Wa_14013475917 */ > > > - if (DISPLAY_VER(dev_priv) == 13 && crtc_state->has_psr && > > > + if ((DISPLAY_VER(dev_priv) == 13 || > > > + IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) && crtc_state->has_psr && > > > type == DP_SDP_VSC) > > > return; > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > > > index 5b678916e6db..7982157fb1ff 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > @@ -797,7 +797,7 @@ static bool psr2_granularity_check(struct intel_dp *intel_dp, > > > return intel_dp->psr.su_y_granularity == 4; > > > > > > /* > > > - * adl_p and display 14+ platforms has 1 line granularity. > > > + * adl_p and mtl platforms has 1 line granularity. > > > * For other platforms with SW tracking we can adjust the y coordinates > > > * to match sink requirement if multiple of 4. > > > */ > > > @@ -1170,11 +1170,14 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, > > > PSR2_ADD_VERTICAL_LINE_COUNT); > > > > > > /* > > > - * Wa_16014451276:adlp > > > + * Wa_16014451276:adlp,mtl[a0,b0] > > > > These days we don't really add steppings in these comments; at best > > they're just reiterating information that can be easily seen from the > > code below, at worst they get out of sync and cause confusion. I'd drop > > the "[a0,b0]" part (which also isn't accurate anyway if you're using > > range notation...it would be "[a0..b0)" in that case). > > > > Honestly even the list of platform names on workarounds is of > > questionable value and I'm thinking about writing a patch that just > > drops all of them throughout i915, leaving just the workaround lineage > > number. Maybe I'd keep the platform names in the few cases where we > > have multiple workaround numbers (with different sets of platforms) all > > requesting the same programming of a register... > > I'd be happy to ack such patch :) > > > > > > * All supported adlp panels have 1-based X granularity, this may > > > * cause issues if non-supported panels are used. > > > */ > > > - if (IS_ALDERLAKE_P(dev_priv)) > > > + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) > > > + intel_de_rmw(dev_priv, MTL_CHICKEN_TRANS(cpu_transcoder), 0, > > > + ADLP_1_BASED_X_GRANULARITY); > > > + else if (IS_ALDERLAKE_P(dev_priv)) > > > intel_de_rmw(dev_priv, CHICKEN_TRANS(cpu_transcoder), 0, > > > ADLP_1_BASED_X_GRANULARITY); > > > > > > @@ -1185,8 +1188,12 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, > > > TRANS_SET_CONTEXT_LATENCY_MASK, > > > TRANS_SET_CONTEXT_LATENCY_VALUE(1)); > > > > > > - /* Wa_16012604467:adlp */ > > > - if (IS_ALDERLAKE_P(dev_priv)) > > > + /* Wa_16012604467:adlp,mtl[a0,b0] */ > > > + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) > > > + intel_de_rmw(dev_priv, > > > + MTL_CLKGATE_DIS_TRANS(cpu_transcoder), 0, > > > + MTL_CLKGATE_DIS_TRANS_DMASC_GATING_DIS); > > > + else if (IS_ALDERLAKE_P(dev_priv)) > > > intel_de_rmw(dev_priv, CLKGATE_DIS_MISC, 0, > > > CLKGATE_DIS_MISC_DMASC_GATING_DIS); > > > > > > @@ -1362,8 +1369,12 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp) > > > TRANS_SET_CONTEXT_LATENCY(intel_dp->psr.transcoder), > > > TRANS_SET_CONTEXT_LATENCY_MASK, 0); > > > > > > - /* Wa_16012604467:adlp */ > > > - if (IS_ALDERLAKE_P(dev_priv)) > > > + /* Wa_16012604467:adlp,mtl[a0,b0] */ > > > + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) > > > + intel_de_rmw(dev_priv, > > > + MTL_CLKGATE_DIS_TRANS(intel_dp->psr.transcoder), > > > + MTL_CLKGATE_DIS_TRANS_DMASC_GATING_DIS, 0); > > > + else if (IS_ALDERLAKE_P(dev_priv)) > > > intel_de_rmw(dev_priv, CLKGATE_DIS_MISC, > > > CLKGATE_DIS_MISC_DMASC_GATING_DIS, 0); > > > > > > @@ -1625,7 +1636,7 @@ static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state, > > > > > > if (full_update) { > > > /* > > > - * Not applying Wa_14014971508:adlp as we do not support the > > > + * Not applying Wa_14014971508:adlp,mtl as we do not support the > > > * feature that requires this workaround. > > > */ > > > val |= man_trk_ctl_single_full_frame_bit_get(dev_priv); > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > > index a8a5bd426e78..ecb027626a21 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -727,6 +727,10 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, > > > (IS_SUBPLATFORM(__i915, INTEL_METEORLAKE, INTEL_SUBPLATFORM_##variant) && \ > > > IS_GRAPHICS_STEP(__i915, since, until)) > > > > > > +#define IS_MTL_DISPLAY_STEP(__i915, since, until) \ > > > + (DISPLAY_VER(__i915) == 14 && \ > > > > As Tvrtko noted, this could come back to bite us in the future if > > another platform shows up with 14.10, 14.50, etc. MTL has exactly > > version 14.0, so best to make this > > > > DISPLAY_VER_FULL(__i915) == IP_VER(14, 0) > > > > so that it won't automatically capture future platforms by accident. > > I think it's better to do a platform check as the other platforms are > doing. See DG2 for example: > > #define IS_DG2_DISPLAY_STEP(__i915, since, until) \ > (IS_DG2(__i915) && \ > IS_DISPLAY_STEP(__i915, since, until)) I guess that's fine in the short term, but in the long term that's the kind of thing we need to move away from. We're really not supposed to be using platform checks (which are derived from PCI ID) anymore going forward. At some point, even things like IS_MTL_DISPLAY_STEP() will get replaced with something more along the lines of IS_GMDID_DISPLAY_STEP(12, 70, STEP_A0, STEP_C0) because the expectation is that none of this is actually tied to the platform anymore, just to the IP versions of the various chiplets (which can be mixed-and-matched and in theory could be re-used on other platforms). But today MTL is the first/only hardware we have using this design, so it doesn't matter too much; we don't have to clean this all up immediately. Matt > > Lucas De Marchi
On Thu, Dec 01, 2022 at 03:44:07PM -0800, Matt Roper wrote: >On Thu, Dec 01, 2022 at 03:27:25PM -0800, Lucas De Marchi wrote: >> On Thu, Dec 01, 2022 at 03:01:05PM -0800, Matt Roper wrote: >> > On Wed, Nov 30, 2022 at 03:17:08PM -0800, Matt Atwood wrote: >> > > From: Jouni Högander <jouni.hogander@intel.com> >> > > >> > > This patch introduces initial workarounds for mtl platform >> > > >> > > Bspec: 66624 >> > > >> > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com> >> > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> >> > > --- >> > > drivers/gpu/drm/i915/display/intel_fbc.c | 4 +++- >> > > drivers/gpu/drm/i915/display/intel_hdmi.c | 3 ++- >> > > drivers/gpu/drm/i915/display/intel_psr.c | 27 ++++++++++++++++------- >> > > drivers/gpu/drm/i915/i915_drv.h | 4 ++++ >> > > 4 files changed, 28 insertions(+), 10 deletions(-) >> > > >> > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c >> > > index b5ee5ea0d010..8f269553d826 100644 >> > > --- a/drivers/gpu/drm/i915/display/intel_fbc.c >> > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c >> > > @@ -1095,7 +1095,9 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state, >> > > } >> > > >> > > /* Wa_14016291713 */ >> > > - if (IS_DISPLAY_VER(i915, 12, 13) && crtc_state->has_psr) { >> > > + if ((IS_DISPLAY_VER(i915, 12, 13) || >> > > + IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && >> > > + crtc_state->has_psr) { >> > > plane_state->no_fbc_reason = "PSR1 enabled (Wa_14016291713)"; >> > > return 0; >> > > } >> > > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c >> > > index e82f8a07e2b0..efa2da080f62 100644 >> > > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c >> > > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c >> > > @@ -537,7 +537,8 @@ void hsw_write_infoframe(struct intel_encoder *encoder, >> > > 0); >> > > >> > > /* Wa_14013475917 */ >> > > - if (DISPLAY_VER(dev_priv) == 13 && crtc_state->has_psr && >> > > + if ((DISPLAY_VER(dev_priv) == 13 || >> > > + IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) && crtc_state->has_psr && >> > > type == DP_SDP_VSC) >> > > return; >> > > >> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c >> > > index 5b678916e6db..7982157fb1ff 100644 >> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c >> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c >> > > @@ -797,7 +797,7 @@ static bool psr2_granularity_check(struct intel_dp *intel_dp, >> > > return intel_dp->psr.su_y_granularity == 4; >> > > >> > > /* >> > > - * adl_p and display 14+ platforms has 1 line granularity. >> > > + * adl_p and mtl platforms has 1 line granularity. >> > > * For other platforms with SW tracking we can adjust the y coordinates >> > > * to match sink requirement if multiple of 4. >> > > */ >> > > @@ -1170,11 +1170,14 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, >> > > PSR2_ADD_VERTICAL_LINE_COUNT); >> > > >> > > /* >> > > - * Wa_16014451276:adlp >> > > + * Wa_16014451276:adlp,mtl[a0,b0] >> > >> > These days we don't really add steppings in these comments; at best >> > they're just reiterating information that can be easily seen from the >> > code below, at worst they get out of sync and cause confusion. I'd drop >> > the "[a0,b0]" part (which also isn't accurate anyway if you're using >> > range notation...it would be "[a0..b0)" in that case). >> > >> > Honestly even the list of platform names on workarounds is of >> > questionable value and I'm thinking about writing a patch that just >> > drops all of them throughout i915, leaving just the workaround lineage >> > number. Maybe I'd keep the platform names in the few cases where we >> > have multiple workaround numbers (with different sets of platforms) all >> > requesting the same programming of a register... >> >> I'd be happy to ack such patch :) >> >> > >> > > * All supported adlp panels have 1-based X granularity, this may >> > > * cause issues if non-supported panels are used. >> > > */ >> > > - if (IS_ALDERLAKE_P(dev_priv)) >> > > + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) >> > > + intel_de_rmw(dev_priv, MTL_CHICKEN_TRANS(cpu_transcoder), 0, >> > > + ADLP_1_BASED_X_GRANULARITY); >> > > + else if (IS_ALDERLAKE_P(dev_priv)) >> > > intel_de_rmw(dev_priv, CHICKEN_TRANS(cpu_transcoder), 0, >> > > ADLP_1_BASED_X_GRANULARITY); >> > > >> > > @@ -1185,8 +1188,12 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, >> > > TRANS_SET_CONTEXT_LATENCY_MASK, >> > > TRANS_SET_CONTEXT_LATENCY_VALUE(1)); >> > > >> > > - /* Wa_16012604467:adlp */ >> > > - if (IS_ALDERLAKE_P(dev_priv)) >> > > + /* Wa_16012604467:adlp,mtl[a0,b0] */ >> > > + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) >> > > + intel_de_rmw(dev_priv, >> > > + MTL_CLKGATE_DIS_TRANS(cpu_transcoder), 0, >> > > + MTL_CLKGATE_DIS_TRANS_DMASC_GATING_DIS); >> > > + else if (IS_ALDERLAKE_P(dev_priv)) >> > > intel_de_rmw(dev_priv, CLKGATE_DIS_MISC, 0, >> > > CLKGATE_DIS_MISC_DMASC_GATING_DIS); >> > > >> > > @@ -1362,8 +1369,12 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp) >> > > TRANS_SET_CONTEXT_LATENCY(intel_dp->psr.transcoder), >> > > TRANS_SET_CONTEXT_LATENCY_MASK, 0); >> > > >> > > - /* Wa_16012604467:adlp */ >> > > - if (IS_ALDERLAKE_P(dev_priv)) >> > > + /* Wa_16012604467:adlp,mtl[a0,b0] */ >> > > + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) >> > > + intel_de_rmw(dev_priv, >> > > + MTL_CLKGATE_DIS_TRANS(intel_dp->psr.transcoder), >> > > + MTL_CLKGATE_DIS_TRANS_DMASC_GATING_DIS, 0); >> > > + else if (IS_ALDERLAKE_P(dev_priv)) >> > > intel_de_rmw(dev_priv, CLKGATE_DIS_MISC, >> > > CLKGATE_DIS_MISC_DMASC_GATING_DIS, 0); >> > > >> > > @@ -1625,7 +1636,7 @@ static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state, >> > > >> > > if (full_update) { >> > > /* >> > > - * Not applying Wa_14014971508:adlp as we do not support the >> > > + * Not applying Wa_14014971508:adlp,mtl as we do not support the >> > > * feature that requires this workaround. >> > > */ >> > > val |= man_trk_ctl_single_full_frame_bit_get(dev_priv); >> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> > > index a8a5bd426e78..ecb027626a21 100644 >> > > --- a/drivers/gpu/drm/i915/i915_drv.h >> > > +++ b/drivers/gpu/drm/i915/i915_drv.h >> > > @@ -727,6 +727,10 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, >> > > (IS_SUBPLATFORM(__i915, INTEL_METEORLAKE, INTEL_SUBPLATFORM_##variant) && \ >> > > IS_GRAPHICS_STEP(__i915, since, until)) >> > > >> > > +#define IS_MTL_DISPLAY_STEP(__i915, since, until) \ >> > > + (DISPLAY_VER(__i915) == 14 && \ >> > >> > As Tvrtko noted, this could come back to bite us in the future if >> > another platform shows up with 14.10, 14.50, etc. MTL has exactly >> > version 14.0, so best to make this >> > >> > DISPLAY_VER_FULL(__i915) == IP_VER(14, 0) >> > >> > so that it won't automatically capture future platforms by accident. >> >> I think it's better to do a platform check as the other platforms are >> doing. See DG2 for example: >> >> #define IS_DG2_DISPLAY_STEP(__i915, since, until) \ >> (IS_DG2(__i915) && \ >> IS_DISPLAY_STEP(__i915, since, until)) > >I guess that's fine in the short term, but in the long term that's the >kind of thing we need to move away from. We're really not supposed to >be using platform checks (which are derived from PCI ID) anymore going >forward. At some point, even things like IS_MTL_DISPLAY_STEP() will get >replaced with something more along the lines of > > IS_GMDID_DISPLAY_STEP(12, 70, STEP_A0, STEP_C0) I think we should only check for the IP version if we eliminate IS_MTL_ from the name. Or... when the macro is renamed/reimplemented, it may be a good time to change. Lucas De Marchi >because the expectation is that none of this is actually tied to the >platform anymore, just to the IP versions of the various chiplets (which >can be mixed-and-matched and in theory could be re-used on other >platforms). But today MTL is the first/only hardware we have using this >design, so it doesn't matter too much; we don't have to clean this all >up immediately. > > >Matt > >> >> Lucas De Marchi > >-- >Matt Roper >Graphics Software Engineer >VTT-OSGC Platform Enablement >Intel Corporation
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index b5ee5ea0d010..8f269553d826 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -1095,7 +1095,9 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state, } /* Wa_14016291713 */ - if (IS_DISPLAY_VER(i915, 12, 13) && crtc_state->has_psr) { + if ((IS_DISPLAY_VER(i915, 12, 13) || + IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && + crtc_state->has_psr) { plane_state->no_fbc_reason = "PSR1 enabled (Wa_14016291713)"; return 0; } diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index e82f8a07e2b0..efa2da080f62 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -537,7 +537,8 @@ void hsw_write_infoframe(struct intel_encoder *encoder, 0); /* Wa_14013475917 */ - if (DISPLAY_VER(dev_priv) == 13 && crtc_state->has_psr && + if ((DISPLAY_VER(dev_priv) == 13 || + IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) && crtc_state->has_psr && type == DP_SDP_VSC) return; diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 5b678916e6db..7982157fb1ff 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -797,7 +797,7 @@ static bool psr2_granularity_check(struct intel_dp *intel_dp, return intel_dp->psr.su_y_granularity == 4; /* - * adl_p and display 14+ platforms has 1 line granularity. + * adl_p and mtl platforms has 1 line granularity. * For other platforms with SW tracking we can adjust the y coordinates * to match sink requirement if multiple of 4. */ @@ -1170,11 +1170,14 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, PSR2_ADD_VERTICAL_LINE_COUNT); /* - * Wa_16014451276:adlp + * Wa_16014451276:adlp,mtl[a0,b0] * All supported adlp panels have 1-based X granularity, this may * cause issues if non-supported panels are used. */ - if (IS_ALDERLAKE_P(dev_priv)) + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) + intel_de_rmw(dev_priv, MTL_CHICKEN_TRANS(cpu_transcoder), 0, + ADLP_1_BASED_X_GRANULARITY); + else if (IS_ALDERLAKE_P(dev_priv)) intel_de_rmw(dev_priv, CHICKEN_TRANS(cpu_transcoder), 0, ADLP_1_BASED_X_GRANULARITY); @@ -1185,8 +1188,12 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, TRANS_SET_CONTEXT_LATENCY_MASK, TRANS_SET_CONTEXT_LATENCY_VALUE(1)); - /* Wa_16012604467:adlp */ - if (IS_ALDERLAKE_P(dev_priv)) + /* Wa_16012604467:adlp,mtl[a0,b0] */ + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) + intel_de_rmw(dev_priv, + MTL_CLKGATE_DIS_TRANS(cpu_transcoder), 0, + MTL_CLKGATE_DIS_TRANS_DMASC_GATING_DIS); + else if (IS_ALDERLAKE_P(dev_priv)) intel_de_rmw(dev_priv, CLKGATE_DIS_MISC, 0, CLKGATE_DIS_MISC_DMASC_GATING_DIS); @@ -1362,8 +1369,12 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp) TRANS_SET_CONTEXT_LATENCY(intel_dp->psr.transcoder), TRANS_SET_CONTEXT_LATENCY_MASK, 0); - /* Wa_16012604467:adlp */ - if (IS_ALDERLAKE_P(dev_priv)) + /* Wa_16012604467:adlp,mtl[a0,b0] */ + if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) + intel_de_rmw(dev_priv, + MTL_CLKGATE_DIS_TRANS(intel_dp->psr.transcoder), + MTL_CLKGATE_DIS_TRANS_DMASC_GATING_DIS, 0); + else if (IS_ALDERLAKE_P(dev_priv)) intel_de_rmw(dev_priv, CLKGATE_DIS_MISC, CLKGATE_DIS_MISC_DMASC_GATING_DIS, 0); @@ -1625,7 +1636,7 @@ static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state, if (full_update) { /* - * Not applying Wa_14014971508:adlp as we do not support the + * Not applying Wa_14014971508:adlp,mtl as we do not support the * feature that requires this workaround. */ val |= man_trk_ctl_single_full_frame_bit_get(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a8a5bd426e78..ecb027626a21 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -727,6 +727,10 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, (IS_SUBPLATFORM(__i915, INTEL_METEORLAKE, INTEL_SUBPLATFORM_##variant) && \ IS_GRAPHICS_STEP(__i915, since, until)) +#define IS_MTL_DISPLAY_STEP(__i915, since, until) \ + (DISPLAY_VER(__i915) == 14 && \ + IS_DISPLAY_STEP(__i915, since, until)) + /* * DG2 hardware steppings are a bit unusual. The hardware design was forked to * create three variants (G10, G11, and G12) which each have distinct