| Message ID | 20230329094532.221450-3-jouni.hogander@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | High refresh rate PSR fixes | expand |
On Wed, Mar 29, 2023 at 12:45:28PM +0300, Jouni Högander wrote: > Wa_16013835468 is a separate from Wa_14015648006 and needs to be > applied for TGL onwards. Fix this by removing all the references to > Wa_14015648006 and apply Wa_16013835468 according to Bspec. > > Also move workaround into separate function as a preparation for > Wa_14015648006 implementation. > > Bspec: 55378 > > v2: > - keep applying the wa in intel_psr_enable_source > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> > --- > drivers/gpu/drm/i915/display/intel_psr.c | 35 ++++++++++++++++-------- > 1 file changed, 24 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > index 8dbf452d63c2..26ad4365960f 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -1134,6 +1134,28 @@ static u32 wa_16013835468_bit_get(struct intel_dp *intel_dp) > } > } > > +/* > + * Wa_16013835468 > + */ > +static void wm_optimization_wa(struct intel_dp *intel_dp, > + const struct intel_crtc_state *crtc_state) > +{ > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > + bool set_wa_bit = false; > + > + /* Wa_16013835468 */ > + if (DISPLAY_VER(dev_priv) >= 12) Looks like this should actually be == 12 > + set_wa_bit |= crtc_state->hw.adjusted_mode.crtc_vblank_start != > + crtc_state->hw.adjusted_mode.crtc_vdisplay; > + > + if (set_wa_bit) > + intel_de_rmw(dev_priv, GEN8_CHICKEN_DCPR_1, 0, Can you drop that 0 to the next line so the two branches at least looks a bit more alike? Alternatively intel_de_rmw(dev_priv, GEN8_CHICKEN_DCPR_1, wa_16013835468_bit_get(intel_dp), set_wa_bit ? wa_16013835468_bit_get(intel_dp) : 0); or something along those lines. > + wa_16013835468_bit_get(intel_dp)); > + else > + intel_de_rmw(dev_priv, GEN8_CHICKEN_DCPR_1, > + wa_16013835468_bit_get(intel_dp), 0); > +} > + > static void intel_psr_enable_source(struct intel_dp *intel_dp, > const struct intel_crtc_state *crtc_state) > { > @@ -1175,15 +1197,8 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, > > /* > * Wa_16013835468 > - * Wa_14015648006 > */ > - if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0) || > - IS_DISPLAY_VER(dev_priv, 12, 13)) { > - if (crtc_state->hw.adjusted_mode.crtc_vblank_start != > - crtc_state->hw.adjusted_mode.crtc_vdisplay) > - intel_de_rmw(dev_priv, GEN8_CHICKEN_DCPR_1, 0, > - wa_16013835468_bit_get(intel_dp)); > - } > + wm_optimization_wa(intel_dp, crtc_state); > > if (intel_dp->psr.psr2_enabled) { > if (DISPLAY_VER(dev_priv) == 9) > @@ -1359,10 +1374,8 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp) > > /* > * Wa_16013835468 > - * Wa_14015648006 > */ > - if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0) || > - IS_DISPLAY_VER(dev_priv, 12, 13)) > + if (DISPLAY_VER(dev_priv) >= 12) > intel_de_rmw(dev_priv, GEN8_CHICKEN_DCPR_1, > wa_16013835468_bit_get(intel_dp), 0); > > -- > 2.34.1
On Wed, 2023-03-29 at 14:40 +0300, Ville Syrjälä wrote: > On Wed, Mar 29, 2023 at 12:45:28PM +0300, Jouni Högander wrote: > > Wa_16013835468 is a separate from Wa_14015648006 and needs to be > > applied for TGL onwards. Fix this by removing all the references to > > Wa_14015648006 and apply Wa_16013835468 according to Bspec. > > > > Also move workaround into separate function as a preparation for > > Wa_14015648006 implementation. > > > > Bspec: 55378 > > > > v2: > > - keep applying the wa in intel_psr_enable_source > > > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_psr.c | 35 ++++++++++++++++---- > > ---- > > 1 file changed, 24 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > b/drivers/gpu/drm/i915/display/intel_psr.c > > index 8dbf452d63c2..26ad4365960f 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > @@ -1134,6 +1134,28 @@ static u32 wa_16013835468_bit_get(struct > > intel_dp *intel_dp) > > } > > } > > > > +/* > > + * Wa_16013835468 > > + */ > > +static void wm_optimization_wa(struct intel_dp *intel_dp, > > + const struct intel_crtc_state > > *crtc_state) > > +{ > > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > + bool set_wa_bit = false; > > + > > + /* Wa_16013835468 */ > > + if (DISPLAY_VER(dev_priv) >= 12) > > Looks like this should actually be == 12 Ok,I will fix this. It seems I misread the Bspec on this. > > + set_wa_bit |= crtc_state- > > >hw.adjusted_mode.crtc_vblank_start != > > + crtc_state->hw.adjusted_mode.crtc_vdisplay; > > + > > + if (set_wa_bit) > > + intel_de_rmw(dev_priv, GEN8_CHICKEN_DCPR_1, 0, > > Can you drop that 0 to the next line so the two branches > at least looks a bit more alike? I will change it. > > Alternatively > intel_de_rmw(dev_priv, GEN8_CHICKEN_DCPR_1, > wa_16013835468_bit_get(intel_dp), > set_wa_bit ? wa_16013835468_bit_get(intel_dp) : 0); > or something along those lines. > > > + wa_16013835468_bit_get(intel_dp)); > > + else > > + intel_de_rmw(dev_priv, GEN8_CHICKEN_DCPR_1, > > + wa_16013835468_bit_get(intel_dp), 0); > > +} > > + > > static void intel_psr_enable_source(struct intel_dp *intel_dp, > > const struct intel_crtc_state > > *crtc_state) > > { > > @@ -1175,15 +1197,8 @@ static void intel_psr_enable_source(struct > > intel_dp *intel_dp, > > > > /* > > * Wa_16013835468 > > - * Wa_14015648006 > > */ > > - if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0) || > > - IS_DISPLAY_VER(dev_priv, 12, 13)) { > > - if (crtc_state->hw.adjusted_mode.crtc_vblank_start > > != > > - crtc_state->hw.adjusted_mode.crtc_vdisplay) > > - intel_de_rmw(dev_priv, GEN8_CHICKEN_DCPR_1, > > 0, > > - > > wa_16013835468_bit_get(intel_dp)); > > - } > > + wm_optimization_wa(intel_dp, crtc_state); > > > > if (intel_dp->psr.psr2_enabled) { > > if (DISPLAY_VER(dev_priv) == 9) > > @@ -1359,10 +1374,8 @@ static void intel_psr_disable_locked(struct > > intel_dp *intel_dp) > > > > /* > > * Wa_16013835468 > > - * Wa_14015648006 > > */ > > - if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0) || > > - IS_DISPLAY_VER(dev_priv, 12, 13)) > > + if (DISPLAY_VER(dev_priv) >= 12) > > intel_de_rmw(dev_priv, GEN8_CHICKEN_DCPR_1, > > wa_16013835468_bit_get(intel_dp), 0); > > > > -- > > 2.34.1 >
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 8dbf452d63c2..26ad4365960f 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -1134,6 +1134,28 @@ static u32 wa_16013835468_bit_get(struct intel_dp *intel_dp) } } +/* + * Wa_16013835468 + */ +static void wm_optimization_wa(struct intel_dp *intel_dp, + const struct intel_crtc_state *crtc_state) +{ + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); + bool set_wa_bit = false; + + /* Wa_16013835468 */ + if (DISPLAY_VER(dev_priv) >= 12) + set_wa_bit |= crtc_state->hw.adjusted_mode.crtc_vblank_start != + crtc_state->hw.adjusted_mode.crtc_vdisplay; + + if (set_wa_bit) + intel_de_rmw(dev_priv, GEN8_CHICKEN_DCPR_1, 0, + wa_16013835468_bit_get(intel_dp)); + else + intel_de_rmw(dev_priv, GEN8_CHICKEN_DCPR_1, + wa_16013835468_bit_get(intel_dp), 0); +} + static void intel_psr_enable_source(struct intel_dp *intel_dp, const struct intel_crtc_state *crtc_state) { @@ -1175,15 +1197,8 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, /* * Wa_16013835468 - * Wa_14015648006 */ - if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0) || - IS_DISPLAY_VER(dev_priv, 12, 13)) { - if (crtc_state->hw.adjusted_mode.crtc_vblank_start != - crtc_state->hw.adjusted_mode.crtc_vdisplay) - intel_de_rmw(dev_priv, GEN8_CHICKEN_DCPR_1, 0, - wa_16013835468_bit_get(intel_dp)); - } + wm_optimization_wa(intel_dp, crtc_state); if (intel_dp->psr.psr2_enabled) { if (DISPLAY_VER(dev_priv) == 9) @@ -1359,10 +1374,8 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp) /* * Wa_16013835468 - * Wa_14015648006 */ - if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0) || - IS_DISPLAY_VER(dev_priv, 12, 13)) + if (DISPLAY_VER(dev_priv) >= 12) intel_de_rmw(dev_priv, GEN8_CHICKEN_DCPR_1, wa_16013835468_bit_get(intel_dp), 0);
Wa_16013835468 is a separate from Wa_14015648006 and needs to be applied for TGL onwards. Fix this by removing all the references to Wa_14015648006 and apply Wa_16013835468 according to Bspec. Also move workaround into separate function as a preparation for Wa_14015648006 implementation. Bspec: 55378 v2: - keep applying the wa in intel_psr_enable_source Signed-off-by: Jouni Högander <jouni.hogander@intel.com> --- drivers/gpu/drm/i915/display/intel_psr.c | 35 ++++++++++++++++-------- 1 file changed, 24 insertions(+), 11 deletions(-)