[PATCH] drm: reduce default drm vblank off delay to 50ms

[Mario Kleiner]

On 02.11.12 19:37, Jesse Barnes wrote:
> On Fri, 02 Nov 2012 06:56:57 +0100
> Mario Kleiner <mario.kleiner at tuebingen.mpg.de> wrote:
>> Jesse, thanks for cc'ing me, much appreciated :)
>>
>> Psychtoolbox should be fine with a 50 msecs vblank off delay. I think i
>> tested with values somewhere between 50 - 100 msecs at the time the drm
>> patches were made. The way i schedule swaps for a specific target time
>> 'twhen' is to:
>>
>> 1. glXGetSyncValuesOML(msc,ust) as a baseline vblank count and time.
>> 2. Some basic math to calculate targetMSC based on 'twhen' and step 1.
>> 3. glXSwapBuffersMscOML(targetMSC);
>>
>> So it should tolerate wrong vblank counts due to races, as long as
>> vblank doesn't get disabled between 1. and 3. A default vblank off delay
>> lower than maybe 1 refresh duration would make me nervous, given that
>> most kms drivers are not race-free and it is possible for a userspace
>> app or the x-server to get stalled for a couple of msecs.
>>
>> E.g., radeon wouldn't allow this race-free, because the drm code assumes
>> that the hw vblank counter increments at leading edge of vblank, whereas
>> radeon hw does it at the traling edge, iirc. nouveau doesn't have any hw
>> vblank counter support, so it is "race free" until somebody fixes the
>> driver ;-) - ie., it would work with apps that use steps 1/2/3 above,
>> but broken for any other app.
>
> Right, ok that clarifies things for me.  I knew you had some kind of
> race between reading the hw value and scheduling things.  Is the above
> in a comment somewhere in the vblank code?
>

There's some explanation in drm_irq.c - vblank_disable_and_save().

Fixing it would mean to make sure that the driver->get_vblank_counter() 
function always returns hw counts "as if" the hardware increments at 
leading edge. If the hw does that, great. If not, one can use the 
drivers scanout position query to find out where the scanout was while 
reading the hw vblank counter register. Then, based on knowledge when 
the gpu actually increments, one can either return the read hw value as 
is, or hw + 1 or whatever to fudge the "increment at leading edge" 
behaviour.

>> What kind of race in your code do you mean? What does your commit
>> message "...now that we've dealt with the hw races in Mario's updated
>> code..." refer to?
>
> I was vaguely referring to all the fixes from a couple of years ago,
> plus a foggy recollection of the above (which I thought might have been
> magically fixed somehow).
>

Those fixed races between the irq vblank handler and the vblank on/off 
code. The race above between the on/off code and hw counter remains.

-mario