| Message ID | 20230317110437.2780483-4-jouni.hogander@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | High refresh rate PSR fixes | expand |
On Fri, Mar 17, 2023 at 01:04:37PM +0200, Jouni Högander wrote: > Implement Wa_1136 for SKL/BXT/ICL. > > Bspec: 21664 > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> > --- > drivers/gpu/drm/i915/display/intel_psr.c | 15 +++++++++++++++ > drivers/gpu/drm/i915/display/skl_watermark.c | 5 ----- > 2 files changed, 15 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > index a385cb8dbf13..e6bd46441392 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -1049,6 +1049,13 @@ void intel_psr_compute_config(struct intel_dp *intel_dp, > return; > } > > + /* Wa_1136 */ The syntax we've used for the old w/as is different. > + if (DISPLAY_VER(dev_priv) < 12 && crtc_state->wm_level_disabled) { This won't have been calculated yet. As for the platform check. I think the one hsd we still have left indicates that icl already got some kind of full fix. So probably that should at least be safe. And I do think the KBL+ should also work fine. But we could do that as followups: 1. do this 2. switch to the chicken bit approach for icl 3. switch to the chicken bit approach for kbl+ Then of any issue later come up that point to a problem with the chicken bits we could more easily revert to full psr disable. > + drm_dbg_kms(&dev_priv->drm, > + "PSR condition failed: WM level disabled and no HW WA available\n"); > + return; > + } > + > crtc_state->has_psr = true; > crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp, crtc_state); > > @@ -1260,6 +1267,10 @@ static void intel_psr_enable_locked(struct intel_dp *intel_dp, > > drm_WARN_ON(&dev_priv->drm, intel_dp->psr.enabled); > > + /* Wa_1136 */ > + if (DISPLAY_VER(dev_priv) < 12 && crtc_state->wm_level_disabled) It's a bit weird to handle this differently than the active_planes case. Though the fact that the pre and post updatre hooks also do things in different ways is also confusing. Seems to me some general cleanup in this area could be worthwile. > + return; > + > intel_dp->psr.psr2_enabled = crtc_state->has_psr2; > intel_dp->psr.busy_frontbuffer_bits = 0; > intel_dp->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe; > @@ -1940,6 +1951,10 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state, > needs_to_disable |= !new_crtc_state->active_planes; > needs_to_disable |= new_crtc_state->has_psr2 != psr->psr2_enabled; > > + /* Wa_1136 */ > + needs_to_disable |= DISPLAY_VER(i915) < 12 && > + new_crtc_state->wm_level_disabled; > + > if (psr->enabled && needs_to_disable) > intel_psr_disable_locked(intel_dp); > > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c > index afb751c024ba..ced61da8b496 100644 > --- a/drivers/gpu/drm/i915/display/skl_watermark.c > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c > @@ -2278,11 +2278,6 @@ static int skl_wm_check_vblank(struct intel_crtc_state *crtc_state) > */ > crtc_state->wm_level_disabled = level < i915->display.wm.num_levels - 1; > > - /* > - * FIXME also related to skl+ w/a 1136 (also unimplemented as of > - * now) perhaps? > - */ > - > for (level++; level < i915->display.wm.num_levels; level++) { > enum plane_id plane_id; > > -- > 2.34.1
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index a385cb8dbf13..e6bd46441392 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -1049,6 +1049,13 @@ void intel_psr_compute_config(struct intel_dp *intel_dp, return; } + /* Wa_1136 */ + if (DISPLAY_VER(dev_priv) < 12 && crtc_state->wm_level_disabled) { + drm_dbg_kms(&dev_priv->drm, + "PSR condition failed: WM level disabled and no HW WA available\n"); + return; + } + crtc_state->has_psr = true; crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp, crtc_state); @@ -1260,6 +1267,10 @@ static void intel_psr_enable_locked(struct intel_dp *intel_dp, drm_WARN_ON(&dev_priv->drm, intel_dp->psr.enabled); + /* Wa_1136 */ + if (DISPLAY_VER(dev_priv) < 12 && crtc_state->wm_level_disabled) + return; + intel_dp->psr.psr2_enabled = crtc_state->has_psr2; intel_dp->psr.busy_frontbuffer_bits = 0; intel_dp->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe; @@ -1940,6 +1951,10 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state, needs_to_disable |= !new_crtc_state->active_planes; needs_to_disable |= new_crtc_state->has_psr2 != psr->psr2_enabled; + /* Wa_1136 */ + needs_to_disable |= DISPLAY_VER(i915) < 12 && + new_crtc_state->wm_level_disabled; + if (psr->enabled && needs_to_disable) intel_psr_disable_locked(intel_dp); diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c index afb751c024ba..ced61da8b496 100644 --- a/drivers/gpu/drm/i915/display/skl_watermark.c +++ b/drivers/gpu/drm/i915/display/skl_watermark.c @@ -2278,11 +2278,6 @@ static int skl_wm_check_vblank(struct intel_crtc_state *crtc_state) */ crtc_state->wm_level_disabled = level < i915->display.wm.num_levels - 1; - /* - * FIXME also related to skl+ w/a 1136 (also unimplemented as of - * now) perhaps? - */ - for (level++; level < i915->display.wm.num_levels; level++) { enum plane_id plane_id;
Implement Wa_1136 for SKL/BXT/ICL. Bspec: 21664 Signed-off-by: Jouni Högander <jouni.hogander@intel.com> --- drivers/gpu/drm/i915/display/intel_psr.c | 15 +++++++++++++++ drivers/gpu/drm/i915/display/skl_watermark.c | 5 ----- 2 files changed, 15 insertions(+), 5 deletions(-)