On Wed, May 14, 2014 at 06:49:10AM -0700, Jamey Sharp wrote: > 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.
It's a FAIL currently. > 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. With divisor=0 swaps, the driver is allowed to perform immediate completion of swaps if target_msc == current_msc. So it is possible for the driver to receive a swap request and reply that it completed it that msc just prior to an interrupt occuring. > 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." Right. But using the divisor=0 logic above we don't have queued frames if a client never requests a swap in the future. As soon as that happens, we end up with a steady state of swaps and fail to break out of the rut never to return to the expected fast behaviour. > > > 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. Except when there is not an outstanding swap because the spec allows the driver to complete the swap immediately for the target_msc == current_msc with divisor == 0. > 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. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
