[Intel-gfx] [PATCH 1/4] drm/i915: Delay first PSR activation.

[R, Durgadoss]

Hi Rodrigo,

>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces at lists.freedesktop.org] On Behalf Of Rodrigo Vivi
>Sent: Thursday, November 12, 2015 1:07 AM
>To: intel-gfx at lists.freedesktop.org
>Cc: Vivi, Rodrigo
>Subject: [Intel-gfx] [PATCH 1/4] drm/i915: Delay first PSR activation.
>
>When debuging the frozen screen caused by HW tracking with low
>power state I noticed that if we keep moving the mouse non stop
>you will miss the screen updates for a while. At least
>until we stop moving the mouse for a small time and move again.
>
>The actual enabling should happen immediately after
>Display Port enabling sequence finished with links trained and
>everything enabled. However we face many issues when enabling PSR
>right after a modeset.
>
>On VLV/CHV we face blank screens on this scenario and on HSW+
>we face a recoverable frozen screen, at least until next
>exit-activate sequence.
>
>Another workaround for the same issue here would be to increase
>re-enable idle time from 100 to 500 as we did for VLV/CHV.
>However this patch workaround this issue in a better
>way since it doesn't reduce PSR residency and also
>allow us to reduce the delay time between re-enables at least
>on VLV/CHV.
>
>This is also important to make the sysfs toggle working properly.
>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>---
> drivers/gpu/drm/i915/intel_psr.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
>index 213581c..6b24c24 100644
>--- a/drivers/gpu/drm/i915/intel_psr.c
>+++ b/drivers/gpu/drm/i915/intel_psr.c
>@@ -427,6 +427,19 @@ void intel_psr_enable(struct intel_dp *intel_dp)
> 		vlv_psr_enable_source(intel_dp);
> 	}
>
>+	/*
>+	 * FIXME: Activation should happen immediately since this function
>+	 * is just called after pipe is fully trained and enabled.
>+	 * However on every platform we face issues when first activation
>+	 * follows a modeset so quickly.
>+	 *     - On VLV/CHV we get bank screen on first activation
>+	 *     - On HSW/BDW we get a recoverable frozen screen until next
>+	 *       exit-activate sequence.
>+	 */
>+	if (INTEL_INFO(dev)->gen < 9)
>+		schedule_delayed_work(&dev_priv->psr.work,
>+				      msecs_to_jiffies(intel_dp->panel_power_cycle_delay * 5));
>+
> 	dev_priv->psr.enabled = intel_dp;
> unlock:
> 	mutex_unlock(&dev_priv->psr.lock);
>@@ -735,8 +748,9 @@ void intel_psr_flush(struct drm_device *dev,
> 	}
>
> 	if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
>-		schedule_delayed_work(&dev_priv->psr.work,
>-				      msecs_to_jiffies(delay_ms));
>+		if (!work_busy(&dev_priv->psr.work.work))
>+			schedule_delayed_work(&dev_priv->psr.work,
>+					      msecs_to_jiffies(delay_ms));

Agree with the theory of the patch as such.. But, Is there any specific reason for
the !work_busy() check here ?

I believe when the later work runs, it will anyway bail out in _activate
function, if it sees PSR_ENABLE bit set already. So, is this check just to
prevent scheduling one more work item when there is one pending
already ? (or it helps in something else also ?)

Thanks,
Durga

> 	mutex_unlock(&dev_priv->psr.lock);
> }
>
>--
>2.4.3
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx at lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/intel-gfx