On Wed, 2020-10-21 at 22:24 +0800, Lee Shawn C wrote:
> Driver should refer to commit 'b2fc2252ce41 ("drm/i915/psr:
> Always wait for idle state when disabling PSR")' to wait for
> idle state when turn PSR off. But it did not follow
> previous method. Driver just call intel_psr_exit() in
> intel_psr_invalidate() and psr_force_hw_tracking_exit().
> Then leave the function right away.
>
> After PSR disabled, we found some user space applications
> would enabled PSR again immediately. That caused particular
> TCON to get into incorrect state machine and can't recognize
> video data from source properly.
How? I don't see how this is possible this change is only adding delay between userspace calls.
Take a look at intel_psr_work(), PSR will only be enabled again when idle.
>
> Add this change to wait PSR idle state in intel_psr_invalidate()
> and psr_force_hw_tracking_exit(). This symptom is not able
> to replicate anymore.
>
> Fixes: b2fc2252ce41 (drm/i915/psr: Always wait for idle state
> when disabling PSR).
>
> Cc: Manasi Navare <manasi.d.navare@xxxxxxxxx>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: José Roberto de Souza <jose.souza@xxxxxxxxx>
> Cc: Cooper Chiou <cooper.chiou@xxxxxxxxx>
> Cc: Khaled Almahallawy <khaled.almahallawy@xxxxxxxxx>
> Signed-off-by: Lee Shawn C <shawn.c.lee@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_psr.c | 43 ++++++++++++++----------
> 1 file changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index a591a475f148..83b642a5567e 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1036,6 +1036,25 @@ void intel_psr_enable(struct intel_dp *intel_dp,
> mutex_unlock(&dev_priv->psr.lock);
> }
>
>
>
>
> +static void intel_psr_wait_idle(struct drm_i915_private *dev_priv)
> +{
> + i915_reg_t psr_status;
> + u32 psr_status_mask;
> +
> + if (dev_priv->psr.psr2_enabled) {
> + psr_status = EDP_PSR2_STATUS(dev_priv->psr.transcoder);
> + psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
> + } else {
> + psr_status = EDP_PSR_STATUS(dev_priv->psr.transcoder);
> + psr_status_mask = EDP_PSR_STATUS_STATE_MASK;
> + }
> +
> + /* Wait till PSR is idle */
> + if (intel_de_wait_for_clear(dev_priv, psr_status,
> + psr_status_mask, 2000))
> + drm_err(&dev_priv->drm, "Timed out waiting PSR idle state\n");
> +}
> +
> static void intel_psr_exit(struct drm_i915_private *dev_priv)
> {
> u32 val;
> @@ -1076,8 +1095,6 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv)
> static void intel_psr_disable_locked(struct intel_dp *intel_dp)
> {
> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> - i915_reg_t psr_status;
> - u32 psr_status_mask;
>
>
>
>
> lockdep_assert_held(&dev_priv->psr.lock);
>
>
>
>
> @@ -1088,19 +1105,7 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp)
> dev_priv->psr.psr2_enabled ? "2" : "1");
>
>
>
>
> intel_psr_exit(dev_priv);
> -
> - if (dev_priv->psr.psr2_enabled) {
> - psr_status = EDP_PSR2_STATUS(dev_priv->psr.transcoder);
> - psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
> - } else {
> - psr_status = EDP_PSR_STATUS(dev_priv->psr.transcoder);
> - psr_status_mask = EDP_PSR_STATUS_STATE_MASK;
> - }
> -
> - /* Wait till PSR is idle */
> - if (intel_de_wait_for_clear(dev_priv, psr_status,
> - psr_status_mask, 2000))
> - drm_err(&dev_priv->drm, "Timed out waiting PSR idle state\n");
> + intel_psr_wait_idle(dev_priv);
>
>
>
>
> /* WA 1408330847 */
> if (dev_priv->psr.psr2_sel_fetch_enabled &&
> @@ -1158,12 +1163,14 @@ static void psr_force_hw_tracking_exit(struct drm_i915_private *dev_priv)
> * pipe.
> */
> intel_de_write(dev_priv, CURSURFLIVE(dev_priv->psr.pipe), 0);
> - else
> + else {
> /*
> * A write to CURSURFLIVE do not cause HW tracking to exit PSR
> * on older gens so doing the manual exit instead.
> */
> intel_psr_exit(dev_priv);
> + intel_psr_wait_idle(dev_priv);
> + }
> }
>
>
>
>
> void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
> @@ -1593,8 +1600,10 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv,
> frontbuffer_bits &= INTEL_FRONTBUFFER_ALL_MASK(dev_priv->psr.pipe);
> dev_priv->psr.busy_frontbuffer_bits |= frontbuffer_bits;
>
>
>
>
> - if (frontbuffer_bits)
> + if (frontbuffer_bits) {
> intel_psr_exit(dev_priv);
> + intel_psr_wait_idle(dev_priv);
> + }
>
>
>
>
> mutex_unlock(&dev_priv->psr.lock);
> }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
