Re: [PATCH 14/19] drm: Don't update vblank timestamp when the counter didn't change

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Aug 06, 2014 at 02:56:14PM +0200, Daniel Vetter wrote:
> On Wed, Aug 06, 2014 at 02:49:57PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > If we already have a timestamp for the current vblank counter, don't
> > update it with a new timestmap. Small errors can creep in between two
> > timestamp queries for the same vblank count, which could be confusing to
> > userspace when it queries the timestamp for the same vblank sequence
> > number twice.
> > 
> > This problem gets exposed when the vblank disable timer is not used
> > (or is set to expire quickly) and thus we can get multiple vblank
> > disable<->enable transition during the same frame which would all
> > attempt to update the timestamp with the latest estimate.
> > 
> > Testcase: igt/kms_flip/flip-vs-expired-vblank
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

On second consideration this seems to just paper over drivers that enable
vblanks a few too many times. Or a bug in the vblank_get handling in
drm_irq.c. If it's just that I think we should just tighten up the checks
to catch that and drop this patch here.
-Daniel

> > ---
> >  drivers/gpu/drm/drm_irq.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > index af33df1..0523f5b 100644
> > --- a/drivers/gpu/drm/drm_irq.c
> > +++ b/drivers/gpu/drm/drm_irq.c
> > @@ -106,6 +106,9 @@ static void drm_update_vblank_count(struct drm_device *dev, int crtc)
> >  	DRM_DEBUG("enabling vblank interrupts on crtc %d, missed %d\n",
> >  		  crtc, diff);
> >  
> > +	if (diff == 0)
> > +		return;
> > +
> >  	/* Reinitialize corresponding vblank timestamp if high-precision query
> >  	 * available. Skip this step if query unsupported or failed. Will
> >  	 * reinitialize delayed at next vblank interrupt in that case.
> > -- 
> > 1.8.5.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@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

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