Re: [PATCH] drm/i915: Fix pre-CTG vblank counter

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Sep 26, 2013 at 07:32:58PM +0200, Mario Kleiner wrote:
> On 25.09.13 22:55, Daniel Vetter wrote:
> >On Wed, Sep 25, 2013 at 07:55:26PM +0300, ville.syrjala@linux.intel.com wrote:
> >>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >>The old style frame counter increments at the start of active video.
> >>However for i915_get_vblank_counter() we want a counter that increments
> >>at the start of vblank.
> >>
> >>Fortunately the low frame counter register also contains the pixel
> >>counter for the current frame. We can can compare that against the
> >>vblank start pixel count to determine if we need to increment the
> >>frame counter by 1 to get the correct answer.
> >>
> >>Also reorganize the function pointer assignments in intel_irq_init() a
> >>bit to avoid confusing people.
> >>
> >>Cc: Mario Kleiner <mario.kleiner@tuebingen.mpg.de>
> >>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>---
> >>
> >>Just another small vblank related gem I forgot to polish up and send
> >>out until Imre started asking me questions about the vblank counter
> >>functions.
> >
> >Hm, I've thought the magic fixup code does take care of that for us? But I
> >agree that we should do this explicitly ...
> >
> >This could explain some of the strange vblank timestamp off failures QA
> >has reported (if there's too much delay and the fixup doesn't fire any
> >more), care to attach this patch to the relevant bug reports? Searching
> >for ts jitter + pre-gen5 should be good enough.
> >-Daniel
> >
> 
> The vblank off code has one known race in that if
> dev->driver->get_vblank_counter does not increment its counter at
> start of vblank, it can have off-by-one vblank. There's a comment in
> that function about that, e.g.,
> 
> http://lxr.free-electrons.com/source/drivers/gpu/drm/drm_irq.c#L102
> 
> The magic code there only fixes races between the vblank off code
> and the vblank irq handler. Normally that race would not really
> affect applications as long as the vblank_off_delay is as large (5
> secs) as it is now. But in QA testing, if you exercise that code not
> the way a normal app would do it, i'd expect you to see that error
> about 4% of the time.
> 
> The only fix is to fixup the vblank counter in the kms driver, so
> this patch looks like a perfect solution to that. You can add my
> reviewed-by if you wish.

r-b added and patch merged. I think we need to repoke qa to test stuff
since our kms_flip testcase has been a bit broken the last few days (and
they've failed to correctly untangle the mess a bit).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch