[Intel-gfx] Intel-gfx Digest, Vol 57, Issue 7

[Mario Kleiner]

On 02.10.12 20:08, Daniel Vetter wrote:
> On Tue, Oct 2, 2012 at 7:45 PM, Mario Kleiner
> <mario.kleiner at tuebingen.mpg.de> wrote:
>>
>> I'm fine with removing the hack and fixing this properly, especially if you
>> say that it didn't work realiably in some cases. But i hope this means that
>> timestamping sanity tests via flip_test are a part of your regular QA
>> testing before you release a new kms driver, so bugs get caught pre-release?
>
> Running i-g-t tests is part of our nightly testrun. If you add in the
> machines Chris&me have we pretty much cover every platform ever
> shipped by Intel ;-)
>

Cool, that makes me happy :)

>> Otherwise many scientists would get pretty nervous when using Intel gpus in
>> the future.
>
> ... so I hope your scientists won't get nervous ;-) I still have a few
> things I need to add to the flip test though:

I'll have a closer look at flip_test in the near future and see if i 
have some ideas. Psychtoolbox has many built-in tests for timestamp 
consistency. I'll have a look how it compares to flip_test and if some 
of the simple tests could be transplanted. Many of the more complex 
offline tests are unfortunately in Octave scripting language and require 
lots of human intervention, interpretation and special test equipment, 
so not really suitable for quick QA testing.

> - interactions with wait_vblank (both the blocking version and the
> event version)
> - checking whether the timestamps are accurately space (since we can
> at least read out the line counter, we should be far accurater that 1%
> there).

My testing on a Mini Atom Netbook with Intel GMA-950 in 2010 confirmed 
that. The OML_sync_control timestamps were off by less than 0.2 msecs 
wrt. photo-diode measurements on a fast CRT monitor and jitter in the 
low microsecond range < 1 scanline duration. Basically perfect within 
measurement accuracy of the external equipment.

Here's a poster from 2010, for a vision scientist audience, if you are 
interested in the procedure i used and some results:

https://github.com/kleinerm/Psychtoolbox-3/blob/master/Psychtoolbox/PsychDocumentation/ECVP2010Poster_VisualTimingPrecision.pdf

> - if you have any ideas for further test, their' highly welcome. One
> thing we unfortunately can't really test is whether the timestamp is
> really when the hw starts scanning out the first line of the new
> frame. But extremely low jitter and whether we pick the right frame
> (or accidentally overcorrect by an entire frame like this code
> sometimes managed to do) should able be covered.
>

See above: I have the equipment and methods to test that, but 
unfortunately difficulty getting my hands on Intel gpus in our lab, but 
at least during testing at the end of 2010, when i could borrow a few, 
the kms timestamping was basically perfect.

> Eventually I want to also get rid of the delayed vblank disabling and
> make the frame counter readout race-free (we probably need to move a
> bit of drm core logic into the driver for that). Again, I'll only
> merge patches once I'm happy with the evilness of the tests ...
>

Happy to review that once you get at it. I spent a lot of time staring 
at that code after Matthew Garret proposed some patches for early vblank 
disabling a while ago. If i'm not mistaken, the vblank disable/enable 
code in the core should be almost race-free. If you could guarantee that 
the driver->get_vblank_count() function returns a count "as if" the hw 
counter increments always at a fixed point in the scanout, identical for 
all kms drivers, e.g., defined to increment always at the 1st or last 
scanline of vblank, it should be race free. The enable/disable code in 
the core makes the implicit assumption that the hw counter increments 
either at beginning or end of vblank, can't remember atm. which one it was.

Maybe there are better ways on intel hardware, but my sketchy idea was 
to do something like read scanline, read hw vblank counter, read 
scanline, do some comparisons and find out if either +1 needs to be 
added to the returned counter value to fudge it, or if the query needs 
to be repeated once in very rare cases. It worked at least on paper. I 
should really stop making notes only on scrap paper ;-) -- maybe i can 
find it somewhere in my paper pile.

>> Btw. if you remove the hack in the followup patch, you can also remove the
>> do_gettimeofday(&tnow).
>
> Indeed, thanks for spotting this. I'll resend v2 of this patch.
>

Fwiw, you have my

Reviewed-by: mario.kleiner at tuebingen.mpg.de

thanks,
-mario