Re: [PATCH 02/12] drm: Make the vblank disable timer per-crtc

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, May 21, 2014 at 01:17:49PM +0200, Thierry Reding wrote:
> On Wed, May 14, 2014 at 08:51:04PM +0200, Daniel Vetter wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Currently there's one per-device vblank disable timer, and it gets
> > reset wheneven the vblank refcount for any crtc drops to zero. That
> 
> "whenever"
> 
> > means that one crtc could accidentally be keeping the vblank interrupts
> > for other crtcs enabled even if there are no users for them. Make the
> > disable timer per-crtc to avoid this issue.
> 
> Very pedantically: s/crtc/CRTC/ and maybe even s/vblank/VBLANK/. Feel
> free to ignore those, though. =)
> 
> Also, and I may have asked before, why do we even need this timer? Why
> not simply disable interrupts when the last vblank reference goes away?

Without intricate knowledge of where exactly the vblank interrupt fires
wrt the hw frame counter the enabling/disabling of the vblank machinery as
implemented in drm_irq.c is racy. Which means we shouldn't do it all the
time.

In i915 we are now solid enough with vblank handling in general and also
well-covered in tests that we'll attempt to kill the disabling timer as
the next step. Since keeping vblanks going when we don't need them if you
have a hw vblank counter seriously hampers deep sleep states residency.

But given how bug-riddled our vblank code was I want to move slowly. And
we need to keep the hack for all those drivers which haven't properly been
audited and tested (i.e. everyone else).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch