On 24/08/16 06:35 AM, Eric Anholt wrote: > Michel Dänzer <mic...@daenzer.net> writes: >> On 20/08/16 04:42 AM, Eric Anholt wrote: >>> Michel Dänzer <mic...@daenzer.net> writes: >>> >>>> From: Michel Dänzer <michel.daen...@amd.com> >>>> >>>> Always use 3 buffers when flipping. With only 2 buffers, we have to wait >>>> for a flip to complete (which takes non-0 time even with asynchronous >>>> flips) before we can start working on the next frame. We were previously >>>> only using 2 buffers for flipping if the X server supports asynchronous >>>> flips, even when we're not using asynchronous flips. This could result >>>> in bad performance (the referenced bug report is an extreme case, where >>>> the inter-frame stalls were preventing the GPU from reaching its maximum >>>> clocks). >>>> >>>> I couldn't measure any performance boost using 4 buffers with flipping. >>>> Performance actually seemed to go down slightly, but that might have >>>> been just noise. >>>> >>>> Without flipping, a single back buffer is enough for swap interval 0, >>>> but we need to use 2 back buffers when the swap interval is non-0, >>>> otherwise we have to wait for the swap interval to pass before we can >>>> start working on the next frame. This condition was previously reversed. >>>> >>>> Cc: "12.0 11.2" <mesa-sta...@lists.freedesktop.org> >>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97260 >>>> Signed-off-by: Michel Dänzer <michel.daen...@amd.com> >>> >>> Reviewed-by: Eric Anholt <e...@anholt.net> >> >> Thanks. >> >> Note that four piglit EGL tests fail with this change, but AFAICT all of >> those tests are broken. I submitted fixes for three of them: >> >> https://patchwork.freedesktop.org/patch/106809/ >> https://patchwork.freedesktop.org/patch/106807/ >> https://patchwork.freedesktop.org/patch/106808/ >> >> The last one is egl_chromium_sync_control. It calls eglSwapBuffers twice >> in a row and expects that the SBC is higher than before immediately >> after that. I.e. it expects that there is only one back buffer, so that >> at least the second eglSwapBuffers has to block until the first swap >> finishes. Which is no longer true with this DRI3 change. Not sure what >> to do about this one. > > From a quick glance, change the test to wait on the SBC before querying > the new one, maybe?
I'm not sure how to do that. How about the attached patch? If it looks good, somebody please push it for me. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
From 851a77647eb58e56bf909ac4c49217d6fa43c663 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michel=20D=C3=A4nzer?= <michel.daen...@amd.com>
Date: Thu, 25 Aug 2016 18:05:53 +0900
Subject: [PATCH] EGL_CHROMIUM_sync_control: Wait for up to 2 seconds for SBC
to increase
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Instead of expecting it to increase immediately after two eglSwapBuffers
calls.
Mesa's DRI3 platform can use more than one back buffer, in which case it
can queue up multiple buffer swaps before any of them completes.
We wait for up to 2 seconds so that we can pass with the Present
extension fake CRTC, which ticks at 1Hz.
Signed-off-by: Michel Dänzer <michel.daen...@amd.com>
---
.../egl_chromium_sync_control.c | 32 +++++++++++-----------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/tests/egl/spec/egl_chromium_sync_control/egl_chromium_sync_control.c b/tests/egl/spec/egl_chromium_sync_control/egl_chromium_sync_control.c
index 2df2a52..6f2aee8 100644
--- a/tests/egl/spec/egl_chromium_sync_control/egl_chromium_sync_control.c
+++ b/tests/egl/spec/egl_chromium_sync_control/egl_chromium_sync_control.c
@@ -161,10 +161,8 @@ test_eglGetSyncValuesCHROMIUM_ust_test(struct egl_state *state)
*
* Calling glFinish() won't force the swap buffers to happen; it will only flush
* the command to the compositor. By default, the buffer swap for each drawable
- * object is rate limited to happen no more than 60 times a second.
- * SwapBuffers() will block in order to enforce this. Therefore, by the time
- * our second SwapBuffer() call returns, we are guaranteed that SBC will have
- * incremented at least once, possibly twice, but not more than twice.
+ * object is rate limited to happen no more than 60 times a second. Therefore,
+ * we have to wait for the SBC to increase, but it should happen eventually.
*
* When buffer swaps are synchronized to the vertical retrace (which is the
* case when the surface's swap interval is non-zero), then the MSC increments
@@ -180,6 +178,7 @@ test_eglGetSyncValuesCHROMIUM_msc_and_sbc_test(struct egl_state *state)
EGLuint64KHR msc2 = canary;
EGLuint64KHR sbc2 = canary;
EGLint min_swap_interval;
+ int i;
if (!eglGetConfigAttrib(state->egl_dpy, state->cfg,
EGL_MIN_SWAP_INTERVAL, &min_swap_interval)) {
@@ -188,7 +187,7 @@ test_eglGetSyncValuesCHROMIUM_msc_and_sbc_test(struct egl_state *state)
}
if (!peglGetSyncValuesCHROMIUM(state->egl_dpy, state->surf,
&ust, &msc, &sbc)) {
- piglit_loge("Unexpected failure on first sbc fetch");
+ piglit_loge("Unexpected failure on sbc fetch #1");
return PIGLIT_FAIL;
}
@@ -196,25 +195,26 @@ test_eglGetSyncValuesCHROMIUM_msc_and_sbc_test(struct egl_state *state)
glClear(GL_COLOR_BUFFER_BIT);
eglSwapBuffers(state->egl_dpy, state->surf);
- glClearColor(1.0, 0.0, 0.0, 1.0);
- glClear(GL_COLOR_BUFFER_BIT);
- eglSwapBuffers(state->egl_dpy, state->surf);
+ for (i = 0, sbc2 = sbc; i < 2000 && sbc2 == sbc; i++) {
+ if (!peglGetSyncValuesCHROMIUM(state->egl_dpy, state->surf,
+ &ust2, &msc2, &sbc2)) {
+ piglit_loge("Unexpected failure on sbc fetch #%d", i + 2);
+ return PIGLIT_FAIL;
+ }
- if (!peglGetSyncValuesCHROMIUM(state->egl_dpy, state->surf,
- &ust2, &msc2, &sbc2)) {
- piglit_loge("Unexpected failure on second sbc fetch");
- return PIGLIT_FAIL;
+ usleep(1000);
}
+
if (sbc2 == sbc) {
- piglit_loge("SBC did not change after second SwapBuffers: %lu", sbc);
+ piglit_loge("SBC did not change after %dms: %lu", i, sbc);
return PIGLIT_FAIL;
}
if (min_swap_interval > 0 && msc == msc2) {
- piglit_loge("MSC did not change after second SwapBuffers: %lu", msc);
+ piglit_loge("MSC did not change after SwapBuffers: %lu", msc);
return PIGLIT_FAIL;
}
- if (sbc2 > (sbc + 2)) {
- piglit_loge("SBC increased by more than two after second SwapBuffers: %lu (before) %lu (after)", sbc, sbc2);
+ if (sbc2 > (sbc + 1)) {
+ piglit_loge("SBC increased by more than 1 after SwapBuffers: %lu (before) %lu (after)", sbc, sbc2);
return PIGLIT_FAIL;
}
return PIGLIT_PASS;
--
2.9.3
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
