Re: [PATCH 5/5] drm/i915: Allow vblank interrupts during modeset and eliminate some vblank races

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Feb 28, 2014 at 10:56:20AM +0200, Ville Syrjälä wrote:
> On Fri, Feb 21, 2014 at 09:03:35PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Tell the drm core vblank code to reject drm_vblank_get()s only between
> > drm_vblank_off() and drm_vblank_on() calls, and sprinkle the appropriate
> > drm_vblank_on() calls to the .crtc_enable() hooks. At this time I kept
> > the off calls in their current position, and added the on calls to the
> > end of .crtc_enable(). Later on these will be moved inwards a bit to
> > allow vblank interrupts during plane enable/disable steps.
> > 
> > We can kill of the drm_vblank_{pre,post}_modeset() calls since those are
> > there simply to make drm_vblank_get() fail during a modeset. The way
> > they do it is by grabbing a vblank reference, and after drm_vblank_off()
> > gets called this will results in drm_vblank_get() failing due to the
> > elevated refcount while vblank interrupts are disabled. Unfortunately
> > this means there's no point during modeset where the behaviour can be
> > restored back to the normal state until the vblank refcount drops to 0.
> > There's no gurantee of that happening even after the modeset has
> > completed, so simply dropping the drm_vblank_{pre,post}_modeset() calls
> > is the best option. The new reject mechanism will take care of things
> > in a much more consistent and race free manner.
> > 
> > Testcase: igt/kms_flip/{dpms,modeset}-vs-vblank-race
> 
> QA hit the new tests and filed a bug.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75593

I have another test request since you've just fixed this bug:

Across suspend/resume the vblank state restoring through the
pre/post_modeset hacks isn't just racy but flat-out doesn't work. Keith
Packard stumbled over this when working on his present extension. So I
think since you've (likely) also fixed this, we also should have a
testcase for it.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch