Re: [PATCH 04/10] drm/i915: Wait for pipe to start before sampling vblank timestamps on gen2

Daniel Vetter
2015-12-16T10:25:58Z

On Mon, Dec 14, 2015 at 06:23:43PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We use the vblank timestamps to generate the vblank frame counter value
> on gen2. That means we need the pipe scanout position to be accurate
> when we call drm_crtc_vblank_on(), otherwise the frame counter
> guesstimate may jump when the pipe actually start.
>
> What I observed on my 85x is that the DSL initially reads 0, and when
> the pipe actually starts DSL jumps to vblank_start. On gen2 DSL==0 means
> actually vtotal-1 (see update_scanline_offset()), so if we initially
> get vtotal-1, and then very quickly vblank_start (or thereabouts), the
> scanout position will appear to jump backwards by approximately one
> vblank length. Which means the frame counter guesstimate will also
> jump backwards. That's no good, so let's make sure the pipe has
> started before we call drm_crtc_vblank_on().
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Can't confirm your hw findings, but makes sense. And not the first bug
we've fixed with a "let's just wait for a bit" in the vblank area.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/intel_display.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7dd7200d3ba9..78845d1cfd2e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2152,6 +2152,17 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
>
> I915_WRITE(reg, val | PIPECONF_ENABLE);
> 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()
> + */
> + 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));
> }
>
> /**
> --
> 2.4.10
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation