Re: [PATCH 1/2] drm: Fix off-by-one in vblank hardware counter wraparound handling

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, May 26, 2015 at 01:48:11PM +0200, Daniel Vetter wrote:
> On Tue, May 26, 2015 at 05:53:38PM +0900, Michel Dänzer wrote:
> > From: Michel Dänzer <michel.daenzer@amd.com>
> > 
> > dev->max_vblank_count contains the largest value that can be represented
> > by the hardware counter. When the hardware counter wraps around, we have
> > to add that value + 1 to get the same value as if the hardware counter
> > didn't wrap around.
> > 
> > Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> 
> Well there's two users of this really, one wants the max (this one here)
> and the other a mask. And all the drivers use it as a mask. Maybe rename
> it to vblank_counter_mask or similar while at it to prevent further
> confusion?

Well, really it's just 'value % (max + 1)'. Just happens to work with &
since it's POT-1.

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/drm_irq.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > index c8a3447..f9634da 100644
> > --- a/drivers/gpu/drm/drm_irq.c
> > +++ b/drivers/gpu/drm/drm_irq.c
> > @@ -100,7 +100,7 @@ static void drm_update_vblank_count(struct drm_device *dev, int crtc)
> >  	/*
> >  	 * Interrupts were disabled prior to this call, so deal with counter
> >  	 * wrap if needed.
> > -	 * NOTE!  It's possible we lost a full dev->max_vblank_count events
> > +	 * NOTE!  It's possible we lost a full dev->max_vblank_count + 1 events
> >  	 * here if the register is small or we had vblank interrupts off for
> >  	 * a long time.
> >  	 *
> > @@ -117,7 +117,7 @@ static void drm_update_vblank_count(struct drm_device *dev, int crtc)
> >  	/* Deal with counter wrap */
> >  	diff = cur_vblank - vblank->last;
> >  	if (cur_vblank < vblank->last) {
> > -		diff += dev->max_vblank_count;
> > +		diff += dev->max_vblank_count + 1;
> >  
> >  		DRM_DEBUG("last_vblank[%d]=0x%x, cur_vblank=0x%x => diff=0x%x\n",
> >  			  crtc, vblank->last, cur_vblank, diff);
> > -- 
> > 2.1.4
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel