[We should probably have moved this conversation to the piglit list when it shifted to discussing these new OML_sync_control tests.]
On Wed, May 14, 2014 at 04:12:32PM +0100, Chris Wilson wrote: > 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. Hmm, right. I've rewritten those checks to just verify that alternating between calls to WaitForSbc and GetSyncValues always have monotonically increasing values for both UST and MSC. That's clearly required by the spec, and still catches the driver bugs I was concerned about. Regarding the behavior of divisor==0: I had to think very hard about what you were saying, what the spec says, what the X.Org implementation does, whether SwapInterval affects any of the above, etc. (In case anyone wondered, I did not enjoy that.) Theo and I have concluded that you're right, regardless of what SwapInterval is set to. (It was confusing since a non-zero SwapInterval would in some cases make divisor==0 sync to vblank almost as if you'd set divisor==swap_interval, but not in all cases.) As a result, we've changed timing.c to never use divisor==0, because we want to validate that the driver syncs to vblank when required, and as you say, the spec only requires that if divisor>0. Instead, when testing in -msc-delta mode we set divisor=1, to specify that if the swap misses our target_msc, it should wait until the next vblank. We're also retracting the repeat-swapbuffers test. There may still be utility in a test like it but it isn't testing anything we actually care about and we don't want to think about it further. You can review/test these changes and others, currently in separate patches, at https://github.com/ThirteenFish/piglit. We'll rebase them into the series when we send it out for review again. > > > 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. Now fixed in the above git repo. Thanks, Jamey
signature.asc
Description: Digital signature
_______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel