[Intel-gfx] [PATCH 17/19] drm/i915: Clear .last vblank count before drm_vblank_off() when sanitizing crtc state

[Daniel Vetter]

On Wed, Aug 06, 2014 at 02:50:00PM +0300, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> We call drm_vblank_off() during crtc sanitation to make sure the
> software and hardware states agree. However drm_vblank_off() will
> try to update the vblank timestamp and sequence number which lands
> us in some trouble.
> 
> As the pipe is disabled the hardware frame counter query will
> return 0. If the .last doesn't match the code will try to add the
> difference to the user visible sequence number. During driver init
> that's OK as .last == 0, but during resume .last may be anything.
> So we should make sure we don't try to apply the diff here by zeroing
> .last beforehand. Otherwise there maybe be a random jump in the user
> visible sequence number across suspend.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ae5f20d..c00bcd0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12918,8 +12918,11 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
>  	/* restore vblank interrupts to correct state */
>  	if (crtc->active)
>  		drm_vblank_on(dev, crtc->pipe);
> -	else
> +	else {
> +		/* avoid random jumps in seq/ts */
> +		dev->vblank[crtc->pipe].last = 0;

Should we have a drm_vblank_reset for this? I guess other drivers will
have similar issues with "sorry, just lost it all" kind of transitions.

Also this case should only happen when we enter the system resume with the
pipes in dpms off state. In that case we should have sampled something in
drm_vblank_off already and resampling doesn't look like a good idea.

Do we instead need some protection in drm_vblank_off to avoid re-sampling?
-Daniel

>  		drm_vblank_off(dev, crtc->pipe);
> +	}
>  
>  	/* We need to sanitize the plane -> pipe mapping first because this will
>  	 * disable the crtc (and hence change the state) if it is wrong. Note
> -- 
> 1.8.5.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch