Re: [Intel-gfx] [PATCH] drm: Aggressively disable vblanks

From: Andrew Lutomirski <luto@mit.edu>
To: Keith Packard <keithp@keithp.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm: Aggressively disable vblanks
Date: Mon, 20 Dec 2010 22:34:12 -0500	[thread overview]
Message-ID: <AANLkTik-1zni1DdS6i+1ARoenx6p=9jQAAiA6t5uGg_K@mail.gmail.com> (raw)
In-Reply-To: <yunfwtrrepv.fsf@aiko.keithp.com>

On Mon, Dec 20, 2010 at 10:23 PM, Keith Packard <keithp@keithp.com> wrote:
> On Mon, 20 Dec 2010 14:00:54 -0500, Andy Lutomirski <luto@MIT.EDU> wrote:
>
>> Enabling and disabling the vblank interrupt (on devices that
>> support it) is cheap.  So disable it quickly after each
>> interrupt.
>
> So, the concern (and reason for the original design) was that you might
> lose count of vblank interrupts as vblanks are counted differently while
> off than while on. If vblank interrupts get enabled near the interrupt
> time, is it possible that you'll end up off by one somehow?

So the race is:

1. Vblank happens.
2. vblank_get runs, reads hardware counter, and enables the interrupt
(and maybe not quite in that order)
3. Interrupt fires and increments the counter.  Now the counter is one too high.

What if we read the hardware counter from the IRQ the first time that
it fires after being enabled?  That way, if the hardware and software
counters match (mod 2^23 or whatever the magic number is), we don't
increment the counter.

>
> Leaving them enabled for 'a while' was supposed to reduce the impact of
> this potential error.
>

Fair enough,

But five seconds is a *long* time, and anything short enough that the
interrupt actually gets turned off in normal use risks the same race.

--Andy