| Message ID | 20190308000050.6226-8-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v6,1/9] drm/i915/psr: Remove PSR2 FIXME | expand |
On Thu, 2019-03-07 at 16:00 -0800, José Roberto de Souza wrote: > If PSR1 is active when pipe CRC is enabled the CRC calculations will > be inhibit by the transition to low power states that PSR1 brings. > So lets force a PSR1 exit and as soon as pipe CRC is enabled it will > block PSR1 activation and avoid CRC timeouts when running IGT tests. > > There is a little window between the call to force exit PSR and the > write to pipe CRC registers that needs to happen within the minimum > of 6 idles frames otherwise PSR1 will be active again causing the CRC > timeouts but anyways this will at least reduce the occurrence of CRC > timeouts. > > This can possibily fix issues present right now but I did not found > any open, I mostly got this issue from previous CI runs of this > series, bellow some exambles: > > * igt@kms_color@pipe-b-ctm-0-75: > - shard-apl: PASS -> FAIL +9 > > * igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-legacy: > - shard-apl: PASS -> DMESG-FAIL +17 > > * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-pgflip-blt: > - shard-kbl: PASS -> DMESG-FAIL +12 > > * igt@kms_pipe_crc_basic@read-crc-pipe-c: > - shard-kbl: PASS -> FAIL +7 > > v6: s/PSR/PSR1 (Dhinakaran) I forgot to add DK's rv-b https://patchwork.freedesktop.org/patch/290564/?series=57628&rev=1#comment_547027 > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/intel_psr.c | 36 ++++++++++++++++++++-------- > ---- > 1 file changed, 23 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > b/drivers/gpu/drm/i915/intel_psr.c > index 9847f6b0cd9a..053dbba6abde 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -452,6 +452,7 @@ static void hsw_activate_psr1(struct intel_dp > *intel_dp) > * frames, we'll go with 9 frames for now > */ > idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency > + 1); > + > val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT; > > val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT; > @@ -853,6 +854,20 @@ void intel_psr_disable(struct intel_dp > *intel_dp, > cancel_work_sync(&dev_priv->psr.work); > } > > +static void psr_force_hw_tracking_exit(struct drm_i915_private > *dev_priv) > +{ > + /* > + * Display WA #0884: all > + * This documented WA for bxt can be safely applied > + * broadly so we can force HW tracking to exit PSR > + * instead of disabling and re-enabling. > + * Workaround tells us to write 0 to CUR_SURFLIVE_A, > + * but it makes more sense write to the current active > + * pipe. > + */ > + I915_WRITE(CURSURFLIVE(dev_priv->psr.pipe), 0); > +} > + > /** > * intel_psr_update - Update PSR state > * @intel_dp: Intel DP > @@ -877,8 +892,13 @@ void intel_psr_update(struct intel_dp *intel_dp, > enable = crtc_state->has_psr && psr_global_enabled(psr->debug); > psr2_enable = intel_psr2_enabled(dev_priv, crtc_state); > > - if (enable == psr->enabled && psr2_enable == psr->psr2_enabled) > + if (enable == psr->enabled && psr2_enable == psr->psr2_enabled) > { > + /* Force a PSR exit when enabling CRC to avoid CRC > timeouts */ > + if (crtc_state->crc_enabled && psr->enabled) > + psr_force_hw_tracking_exit(dev_priv); > + > goto unlock; > + } > > if (psr->enabled) > intel_psr_disable_locked(intel_dp); > @@ -1148,18 +1168,8 @@ void intel_psr_flush(struct drm_i915_private > *dev_priv, > dev_priv->psr.busy_frontbuffer_bits &= ~frontbuffer_bits; > > /* By definition flush = invalidate + flush */ > - if (frontbuffer_bits) { > - /* > - * Display WA #0884: all > - * This documented WA for bxt can be safely applied > - * broadly so we can force HW tracking to exit PSR > - * instead of disabling and re-enabling. > - * Workaround tells us to write 0 to CUR_SURFLIVE_A, > - * but it makes more sense write to the current active > - * pipe. > - */ > - I915_WRITE(CURSURFLIVE(dev_priv->psr.pipe), 0); > - } > + if (frontbuffer_bits) > + psr_force_hw_tracking_exit(dev_priv); > > if (!dev_priv->psr.active && !dev_priv- > >psr.busy_frontbuffer_bits) > schedule_work(&dev_priv->psr.work);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 9847f6b0cd9a..053dbba6abde 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -452,6 +452,7 @@ static void hsw_activate_psr1(struct intel_dp *intel_dp) * frames, we'll go with 9 frames for now */ idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency + 1); + val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT; val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT; @@ -853,6 +854,20 @@ void intel_psr_disable(struct intel_dp *intel_dp, cancel_work_sync(&dev_priv->psr.work); } +static void psr_force_hw_tracking_exit(struct drm_i915_private *dev_priv) +{ + /* + * Display WA #0884: all + * This documented WA for bxt can be safely applied + * broadly so we can force HW tracking to exit PSR + * instead of disabling and re-enabling. + * Workaround tells us to write 0 to CUR_SURFLIVE_A, + * but it makes more sense write to the current active + * pipe. + */ + I915_WRITE(CURSURFLIVE(dev_priv->psr.pipe), 0); +} + /** * intel_psr_update - Update PSR state * @intel_dp: Intel DP @@ -877,8 +892,13 @@ void intel_psr_update(struct intel_dp *intel_dp, enable = crtc_state->has_psr && psr_global_enabled(psr->debug); psr2_enable = intel_psr2_enabled(dev_priv, crtc_state); - if (enable == psr->enabled && psr2_enable == psr->psr2_enabled) + if (enable == psr->enabled && psr2_enable == psr->psr2_enabled) { + /* Force a PSR exit when enabling CRC to avoid CRC timeouts */ + if (crtc_state->crc_enabled && psr->enabled) + psr_force_hw_tracking_exit(dev_priv); + goto unlock; + } if (psr->enabled) intel_psr_disable_locked(intel_dp); @@ -1148,18 +1168,8 @@ void intel_psr_flush(struct drm_i915_private *dev_priv, dev_priv->psr.busy_frontbuffer_bits &= ~frontbuffer_bits; /* By definition flush = invalidate + flush */ - if (frontbuffer_bits) { - /* - * Display WA #0884: all - * This documented WA for bxt can be safely applied - * broadly so we can force HW tracking to exit PSR - * instead of disabling and re-enabling. - * Workaround tells us to write 0 to CUR_SURFLIVE_A, - * but it makes more sense write to the current active - * pipe. - */ - I915_WRITE(CURSURFLIVE(dev_priv->psr.pipe), 0); - } + if (frontbuffer_bits) + psr_force_hw_tracking_exit(dev_priv); if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits) schedule_work(&dev_priv->psr.work);
If PSR1 is active when pipe CRC is enabled the CRC calculations will be inhibit by the transition to low power states that PSR1 brings. So lets force a PSR1 exit and as soon as pipe CRC is enabled it will block PSR1 activation and avoid CRC timeouts when running IGT tests. There is a little window between the call to force exit PSR and the write to pipe CRC registers that needs to happen within the minimum of 6 idles frames otherwise PSR1 will be active again causing the CRC timeouts but anyways this will at least reduce the occurrence of CRC timeouts. This can possibily fix issues present right now but I did not found any open, I mostly got this issue from previous CI runs of this series, bellow some exambles: * igt@kms_color@pipe-b-ctm-0-75: - shard-apl: PASS -> FAIL +9 * igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-legacy: - shard-apl: PASS -> DMESG-FAIL +17 * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-pgflip-blt: - shard-kbl: PASS -> DMESG-FAIL +12 * igt@kms_pipe_crc_basic@read-crc-pipe-c: - shard-kbl: PASS -> FAIL +7 v6: s/PSR/PSR1 (Dhinakaran) Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/intel_psr.c | 36 ++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 13 deletions(-)