[PATCH 1/2] drm/i915: Fix vblank timestamp/frame counter jumps on gen2

* [PATCH 1/2] drm/i915: Fix vblank timestamp/frame counter jumps on gen2
@ 2017-11-13 15:32 Ville Syrjala
  2017-11-13 15:32 ` [PATCH 2/2] drm/i915: Wait for pipe to start on i830 as well Ville Syrjala
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Ville Syrjala @ 2017-11-13 15:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable, Daniel Vetter

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Previously I was under the impression that the scanline counter
reads 0 when the pipe is off. Turns out that's not correct, and
instead the scanline counter simply stops when the pipe stops, and
it retains it's last value until the pipe starts up again, at which
point the scanline counter jumps to vblank start.

These jumps can cause the timestamp to jump backwards by one frame.
Since we use the timestamps to guesstimage also the frame counter
value on gen2, that would cause the frame counter to also jump
backwards, which leads to a massice difference from the previous value.
The end result is that flips/vblank events don't appear to complete as
they're stuck waiting for the frame counter to catch up to that massive
difference.

Fix the problem properly by actually making sure the scanline counter
has started to move before we assume that it's safe to enable vblank
processing.

Cc: stable@vger.kernel.org
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Fixes: b7792d8b54cc ("drm/i915: Wait for pipe to start before sampling vblank timestamps on gen2")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0ebf3f283b87..810b1147a0ac 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -998,7 +998,8 @@ enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv,
 	return crtc->config->cpu_transcoder;
 }
 
-static bool pipe_dsl_stopped(struct drm_i915_private *dev_priv, enum pipe pipe)
+static bool pipe_dsl_stopped(struct drm_i915_private *dev_priv,
+			     enum pipe pipe, bool stopped)
 {
 	i915_reg_t reg = PIPEDSL(pipe);
 	u32 line1, line2;
@@ -1013,7 +1014,7 @@ static bool pipe_dsl_stopped(struct drm_i915_private *dev_priv, enum pipe pipe)
 	msleep(5);
 	line2 = I915_READ(reg) & line_mask;
 
-	return line1 == line2;
+	return (line1 == line2) == stopped;
 }
 
 /*
@@ -1048,11 +1049,21 @@ static void intel_wait_for_pipe_off(struct intel_crtc *crtc)
 			WARN(1, "pipe_off wait timed out\n");
 	} else {
 		/* Wait for the display line to settle */
-		if (wait_for(pipe_dsl_stopped(dev_priv, pipe), 100))
+		if (wait_for(pipe_dsl_stopped(dev_priv, pipe, true), 100))
 			WARN(1, "pipe_off wait timed out\n");
 	}
 }
 
+static void intel_wait_for_pipe_on(struct intel_crtc *crtc)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
+
+	/* Wait for the display line to start moving */
+	if (wait_for(pipe_dsl_stopped(dev_priv, pipe, false), 100))
+		WARN(1, "pipe_on wait timed out\n");
+}
+
 /* Only for pre-ILK configs */
 void assert_pll(struct drm_i915_private *dev_priv,
 		enum pipe pipe, bool state)
@@ -1935,15 +1946,14 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
 	POSTING_READ(reg);
 
 	/*
-	 * Until the pipe starts DSL will read as 0, which would cause
-	 * an apparent vblank timestamp jump, which messes up also the
-	 * frame count when it's derived from the timestamps. So let's
-	 * wait for the pipe to start properly before we call
-	 * drm_crtc_vblank_on()
+	 * Until the pipe starts DSL can give a bogus value, which cause
+	 * an apparent vblank timestamp jump when the DSL resets to its
+	 * proper value, which messes up also the frame count when it's
+	 * derived from the timestamps. So let's wait for the pipe to
+	 * start properly before we call drm_crtc_vblank_on()
 	 */
-	if (dev->max_vblank_count == 0 &&
-	    wait_for(intel_get_crtc_scanline(crtc) != crtc->scanline_offset, 50))
-		DRM_ERROR("pipe %c didn't start\n", pipe_name(pipe));
+	if (dev->max_vblank_count == 0)
+		intel_wait_for_pipe_on(crtc);
 }
 
 /**
@@ -14707,7 +14717,7 @@ void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
 	I915_WRITE(PIPECONF(pipe), 0);
 	POSTING_READ(PIPECONF(pipe));
 
-	if (wait_for(pipe_dsl_stopped(dev_priv, pipe), 100))
+	if (wait_for(pipe_dsl_stopped(dev_priv, pipe, true), 100))
 		DRM_ERROR("pipe %c off wait timed out\n", pipe_name(pipe));
 
 	I915_WRITE(DPLL(pipe), DPLL_VGA_MODE_DIS);
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 8+ messages in thread