On Wed, May 14, 2014 at 01:31:14PM +0100, Chris Wilson wrote: > On Tue, May 13, 2014 at 09:20:45AM -0700, Jamey Sharp wrote: > > Yeah, we have new tests that haven't quite been merged yet as we're > > addressing Eric's review comments. Our current version, revised > > yesterday, > > is available here: > > > > [2]https://github.com/ThirteenFish/piglit > > > > Make sure you do a full 'piglit-run.py -t OML_sync_control ...' as we run > > the new tests in a variety of configurations, and the full-screen ones > > are > > particularly interesting here. Though IIRC, SNA fails some of the others > > as well. > > Looks like there are quite a few bugs remaining in the test cases.
Thanks for reviewing them, Chris! > glXGetSyncValuesOML returns the current hardware frame counter and time > of last frame update, not the last client swap frame or swap time. You > make an assumption in the timing loop that the calls to SwapBuffers > and GetSyncValues are instantaneous and do not account for a vblank > interrupt that can happen in between updating the hardware counters. We do make that assumption, but if the assumption is violated we only emit a warning. And it would be very surprising if a correctly-functioning driver violated that assumption, because WaitForMsc/Sbc are required to return as soon as possible once the swap completes. So we should have something like 16ms to issue the follow-up GetSyncValues before the values will change. We included that warning because drivers call DRI2SwapComplete with zeroes for UST and MSC, which is clearly always wrong, and this also lets us continue on to test whether they're at least syncing to vblank. > The computation of interframe jitter is wonky, thanks to using > (new_timestamp - -1) on the first pass. Oops, we broke that while addressing Eric's review comments on Monday. We'll fix it. (Theo, if I don't get to it first: both the if and else starting on line 182 need to be guarded with (last_timestamp >= 0). We'll have to deal with the 80-column limit some other way.) > "glXSwapBuffersMscOML: If <divisor> = 0, > the swap will occur when MSC becomes greater than or equal to > <target_msc>." Yes, except: "If there are multiple outstanding swaps for the same window, at most one such swap can be satisfied per increment of MSC." > The minimum expected_msc is therefore the target_msc in many cases and > no time may pass at all during the timing loop if msc-delta=0. Note that timing.c's main() prohibits using WaitForMsc without also setting either msc-delta or divisor, because that combination would fail in the way you describe. However, because of the above-quoted sentence in the spec, we believe that SwapBuffersMsc must behave as if we'd set msc-delta non-zero. In the open-source implementations, swap_interval defaults to 1 and so it does behave this way. If you override swap_interval to 0, you get non-conforming behavior, but I don't believe that's a test bug. > In repeat-swapbuffers.c, you query the current hw counters before the > swap may have completed, That's because that test's purpose is, as the file comment says, "Verifies that glXSwapBuffersMscOML does not increment SBC more often than MSC". In that test we don't want to wait for any swaps to complete. > and runs afoul of the divisor==0 logic above. Since that test only uses SwapBuffersMsc, the same reasoning applies. Again, thanks for looking this over! Did you catch anything else when you ran the new tests? Jamey
signature.asc
Description: Digital signature
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
