Quoting Tarun Vyas (2018-05-14 21:49:20)
> intel_pipe_update_start also needs to wait for PSR to idle
> out. Need some minor modifications in psr_wait_for_idle in
> order to reuse it.
>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Tarun Vyas <tarun.vyas@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_psr.c | 29 ++++++++++++++++++-----------
> 1 file changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index db27f2faa1de..40aafc0f4513 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -889,11 +889,15 @@ static bool psr_wait_for_idle(struct drm_i915_private *dev_priv)
> i915_reg_t reg;
> u32 mask;
> int err;
> + bool wait = false;
> +
> + mutex_lock(&dev_priv->psr.lock);
>
> intel_dp = dev_priv->psr.enabled;
> if (!intel_dp)
> - return false;
> + goto unlock;
>
> + wait = true;
> if (HAS_DDI(dev_priv)) {
> if (dev_priv->psr.psr2_enabled) {
> reg = EDP_PSR2_STATUS;
> @@ -911,15 +915,18 @@ static bool psr_wait_for_idle(struct drm_i915_private *dev_priv)
> mask = VLV_EDP_PSR_IN_TRANS;
> }
>
> +unlock:
> mutex_unlock(&dev_priv->psr.lock);
>
> - err = intel_wait_for_register(dev_priv, reg, mask, 0, 50);
> - if (err)
> - DRM_ERROR("Timed out waiting for PSR Idle for re-enable\n");
> + if(wait) {
> + err = intel_wait_for_register(dev_priv, reg, mask, 0, 50);
> + if (err) {
> + DRM_ERROR("Timed out waiting for PSR Idle for re-enable\n");
> + wait = false;
> + }
> + }
>
> - /* After the unlocked wait, verify that PSR is still wanted! */
> - mutex_lock(&dev_priv->psr.lock);
> - return err == 0 && dev_priv->psr.enabled;
> + return wait;
> }
>
> static void intel_psr_work(struct work_struct *work)
> @@ -927,7 +934,6 @@ static void intel_psr_work(struct work_struct *work)
> struct drm_i915_private *dev_priv =
> container_of(work, typeof(*dev_priv), psr.work.work);
>
> - mutex_lock(&dev_priv->psr.lock);
>
> /*
> * We have to make sure PSR is ready for re-enable
> @@ -936,14 +942,15 @@ static void intel_psr_work(struct work_struct *work)
> * and be ready for re-enable.
> */
> if (!psr_wait_for_idle(dev_priv))
> - goto unlock;
> + return;
>
> - /*
> + /* After the unlocked wait, verify that PSR is still wanted!
> * The delayed work can race with an invalidate hence we need to
> * recheck. Since psr_flush first clears this and then reschedules we
> * won't ever miss a flush when bailing out here.
> */
> - if (dev_priv->psr.busy_frontbuffer_bits)
> + mutex_lock(&dev_priv->psr.lock);
> + if (dev_priv->psr.enabled && dev_priv->psr.busy_frontbuffer_bits)
> goto unlock;
I'm not sold on the locking dropping here, doing so inside the wait is
bad enough. (And do we need to there anyway?)
Since you need to introduce intel_psr_wait_for_idle() anyway, how about
void intel_psr_wait_for_idle(...)
{
mutex_lock(&i915->psr.lock);
psr_wait_for_idle();
mutex_unlock(&i915->psr.lock);
}
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
