On Tue, 2019-03-05 at 22:47 -0800, José Roberto de Souza wrote:
> If PSR is active when pipe CRC is enabled the CRC calculations will
> be inhibit by the transition to low power states that PSR brings.
The MMIO write to enable CRCs should bring the hardware out from PSR,
but I can imagine some initial CRCs are going to be corrupt or
unavailable.
> So lets for a PSR exit and as soon as pipe CRC is enabled it will
There is a missing word.
> block PSR activation avoid CRC timeouts when running IGT tests.
This surely has fdo bugs, please include them in the commit message.
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> ---
> 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 d3e3996551c6..5d66e7313c75 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;
> @@ -851,6 +852,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
> @@ -875,8 +890,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)
> {
PSR2 is enabled, then user requests CRCs, the mode_changed commit
switches to PSR1. The above condition isn't true in that case.
Also, since the CRC workaround is done before enabling pipe CRC, isn't
there a possibility that the idle frame counter times out and activates
PSR1 before CRC is 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);
The patch fixes a PSR1 issue, why isn't there any reference to PSR1
anywhere?
> +
> goto unlock;
> + }
>
> if (psr->enabled)
> intel_psr_disable_locked(intel_dp);
> @@ -1146,18 +1166,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);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
