[PATCH mesa v4] wayland: Add support for eglSwapInterval
Neil Roberts
neil at linux.intel.com
Fri Oct 25 07:50:17 PDT 2013
Ok, here is version 4 of the patch taking into account the discussion
with Jason Ekstrand. The assumption is that if we have enough buffer
slots then we should always get a release event immediately after one
of the attaches. That means we can just rely on sending a sync request
after the commit in order to get a buffer release and we don't need to
bother with the request to disable the queuing mechanism.
The previous version of the patch would block in a loop calling
wl_dispatch_queue if it couldn't find a buffer. This is only a
sensible option if we know that the compositor isn't queueing the
release events. If not this loop would just block indefinitely. If the
theory about getting release events is correct then we should never
actually hit this loop so it probably doesn't really matter what it
does. However, I didn't like the idea of having a loop there that
would just block forever so I changed it to poll the compositor with a
sync request every 10ms in order to force it to flush the queue. It
prints a warning if this case is hit so that we will know there is a
problem.
I made the change to make it use 4 buffer slots in this patch and
tested that it does use exactly all 4 of them when the application is
fullscreen. This does work and it doesn't hit the polling path. I
guess we could change to be five in order to cope with the subsurface
case but I'm a bit reluctant to do that because it seems like quite a
corner case and maybe it's better to just let it hit the warning path
in that case.
In the previous versions of the patch it would only do a sync request
if the swap interval is zero. In this version I've changed it so that
it always installs it. This is necessary because if an application is
doing swap interval 1 but isn't installing a frame callback it would
end up rendering and calling get_back_bo before we've handled any data
from the compositor and it would use a redundant third buffer.
Regards,
- Neil
------- >8 --------------- (use git am --scissors to automatically chop here)
The Wayland EGL platform now respects the eglSwapInterval value. The value is
clamped to either 0 or 1 because it is difficult (and probably not useful) to
sync to more than 1 redraw.
The main change is that if the swap interval is 0 then Mesa won't install a
frame callback so that eglSwapBuffers can be executed as often as necessary.
However it now always does a sync request after the swap buffers and blocks
until this is complete in get_back_bo. The compositor is likely to send a
release event while processing the new buffer attach and this makes sure we
will receive that before deciding whether to allocate a new buffer. This is
done even if the application is using swap interval 1 because otherwise if the
application is not installing its own frame callback it may end up calling
get_back_bo before we've handled any data from the compositor and it would end
up using a redundant extra buffer.
If there are no buffers available then instead of returning with an error,
get_back_bo will now poll the compositor by repeatedly sending sync requests
every 10ms. This is a last resort and in theory this shouldn't happen because
there should be no reason for the compositor to hold on to more than three
buffers. That means whenever we attach the fourth buffer we should always get
an immediate release event which should come in with the notification for the
first sync request that we are throttled to.
When the compositor is directly scanning out from the application's buffer it
may end up holding on to three buffers. These are the one that is is currently
scanning out from, one that has been given to DRM as the next buffer to flip
to, and one that has been attached and will be given to DRM as soon as the
previous flip completes. When we attach a fourth buffer to the compositor it
should replace that third buffer so we should get a release event immediately
after that. This patch therefore also changes the number of buffer slots to 4
so that we can accomodate that situation.
If DRM eventually gets a way to cancel a pending page flip then the compositors
can be changed to only need to hold on to two buffers and this value can be
put back to 3.
This also moves the vblank configuration defines from platform_x11.c to the
common egl_dri2.h header so they can be shared by both platforms.
---
src/egl/drivers/dri2/egl_dri2.h | 9 +-
src/egl/drivers/dri2/platform_wayland.c | 204 +++++++++++++++++++++++++++++---
src/egl/drivers/dri2/platform_x11.c | 6 -
3 files changed, 193 insertions(+), 26 deletions(-)
diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index 7a2e098..7de5916 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -174,6 +174,7 @@ struct dri2_egl_surface
int dx;
int dy;
struct wl_callback *frame_callback;
+ struct wl_callback *sync_callback;
int format;
#endif
@@ -194,7 +195,7 @@ struct dri2_egl_surface
#endif
int locked;
int age;
- } color_buffers[3], *back, *current;
+ } color_buffers[4], *back, *current;
#endif
#ifdef HAVE_ANDROID_PLATFORM
@@ -220,6 +221,12 @@ struct dri2_egl_image
__DRIimage *dri_image;
};
+/* From xmlpool/options.h, user exposed so should be stable */
+#define DRI_CONF_VBLANK_NEVER 0
+#define DRI_CONF_VBLANK_DEF_INTERVAL_0 1
+#define DRI_CONF_VBLANK_DEF_INTERVAL_1 2
+#define DRI_CONF_VBLANK_ALWAYS_SYNC 3
+
/* standard typecasts */
_EGL_DRIVER_STANDARD_TYPECASTS(dri2_egl)
_EGL_DRIVER_TYPECAST(dri2_egl_image, _EGLImage, obj)
diff --git a/src/egl/drivers/dri2/platform_wayland.c b/src/egl/drivers/dri2/platform_wayland.c
index bb8d113..1bf96d7 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -34,6 +34,7 @@
#include <unistd.h>
#include <fcntl.h>
#include <xf86drm.h>
+#include <poll.h>
#include "egl_dri2.h"
@@ -183,8 +184,16 @@ dri2_create_window_surface(_EGLDriver *drv, _EGLDisplay *disp,
_EGLConfig *conf, EGLNativeWindowType window,
const EGLint *attrib_list)
{
- return dri2_create_surface(drv, disp, EGL_WINDOW_BIT, conf,
+ struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
+ _EGLSurface *surf;
+
+ surf = dri2_create_surface(drv, disp, EGL_WINDOW_BIT, conf,
window, attrib_list);
+
+ if (surf != NULL)
+ drv->API.SwapInterval(drv, disp, surf, dri2_dpy->default_swap_interval);
+
+ return surf;
}
/**
@@ -219,6 +228,8 @@ dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *surf)
if (dri2_surf->frame_callback)
wl_callback_destroy(dri2_surf->frame_callback);
+ if (dri2_surf->sync_callback)
+ wl_callback_destroy(dri2_surf->sync_callback);
if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
dri2_surf->wl_win->private = NULL;
@@ -257,6 +268,52 @@ dri2_release_buffers(struct dri2_egl_surface *dri2_surf)
}
static int
+poll_compositor(struct dri2_egl_display *dri2_dpy)
+{
+ static GLboolean seen_wait_warning = GL_FALSE;
+ struct pollfd pollfd;
+
+ /* This path is a last resort and it implies that something has gone wrong
+ * so we'll warn about it */
+ if (!seen_wait_warning) {
+ _eglLog(_EGL_WARNING, "waiting for the compositor to release a buffer");
+ seen_wait_warning = GL_TRUE;
+ }
+
+ /* The plan here is that we'll wait for up to 10ms for some data from the
+ * compositor. If so we'll just return so that it will check if there is
+ * now a buffer available. Otherwise we'll wait for up to 10ms for some new
+ * data before issuing a roundtrip. The roundtrip is necessary because the
+ * compositor may be queuing release events and we need to cause it to
+ * flush the queue */
+
+ if (wl_display_prepare_read_queue(dri2_dpy->wl_dpy,
+ dri2_dpy->wl_queue) != 0)
+ return wl_display_dispatch_queue_pending(dri2_dpy->wl_dpy,
+ dri2_dpy->wl_queue);
+
+ pollfd.fd = wl_display_get_fd(dri2_dpy->wl_dpy);
+ pollfd.events = POLLIN;
+ pollfd.revents = 0;
+
+ if (poll(&pollfd, 1, 10) < 0) {
+ wl_display_cancel_read(dri2_dpy->wl_dpy);
+ return -1;
+ }
+
+ if (pollfd.revents) {
+ if (wl_display_read_events(dri2_dpy->wl_dpy) < 0)
+ return -1;
+
+ return wl_display_dispatch_queue_pending(dri2_dpy->wl_dpy,
+ dri2_dpy->wl_queue);
+ } else {
+ wl_display_cancel_read(dri2_dpy->wl_dpy);
+ return roundtrip(dri2_dpy);
+ }
+}
+
+static int
get_back_bo(struct dri2_egl_surface *dri2_surf, __DRIbuffer *buffer)
{
struct dri2_egl_display *dri2_dpy =
@@ -264,24 +321,46 @@ get_back_bo(struct dri2_egl_surface *dri2_surf, __DRIbuffer *buffer)
__DRIimage *image;
int i, name, pitch;
- /* There might be a buffer release already queued that wasn't processed */
- wl_display_dispatch_queue_pending(dri2_dpy->wl_dpy, dri2_dpy->wl_queue);
+ if (dri2_surf->sync_callback == NULL) {
+ /* There might be a buffer release already queued that wasn't processed */
+ wl_display_dispatch_queue_pending(dri2_dpy->wl_dpy, dri2_dpy->wl_queue);
+ } else {
+ /* We always want to throttle to a sync event after the commit so that
+ * we can be sure the compositor has had a chance to handle it and send
+ * us a release event before we look for a free buffer */
+ do {
+ if (wl_display_dispatch_queue(dri2_dpy->wl_dpy,
+ dri2_dpy->wl_queue) == -1)
+ return EGL_FALSE;
+ } while (dri2_surf->sync_callback != NULL);
+ }
if (dri2_surf->back == NULL) {
- for (i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
- /* Get an unlocked buffer, preferrably one with a dri_buffer already
- * allocated. */
- if (dri2_surf->color_buffers[i].locked)
- continue;
- if (dri2_surf->back == NULL)
- dri2_surf->back = &dri2_surf->color_buffers[i];
- else if (dri2_surf->back->dri_image == NULL)
- dri2_surf->back = &dri2_surf->color_buffers[i];
+ while (1) {
+ for (i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
+ /* Get an unlocked buffer, preferrably one with a dri_buffer
+ * already allocated. */
+ if (dri2_surf->color_buffers[i].locked)
+ continue;
+ if (dri2_surf->back == NULL)
+ dri2_surf->back = &dri2_surf->color_buffers[i];
+ else if (dri2_surf->back->dri_image == NULL)
+ dri2_surf->back = &dri2_surf->color_buffers[i];
+ }
+
+ if (dri2_surf->back != NULL)
+ break;
+
+ /* If we make it here then here then all of the buffers are locked.
+ * In theory this shouldn't happen because the compositor shouldn't
+ * be holding on to so many buffers. As a last resort we will poll
+ * the compositor at regular intervals in order to ensure we
+ * eventually get a buffer */
+
+ poll_compositor(dri2_dpy);
}
}
- if (dri2_surf->back == NULL)
- return -1;
if (dri2_surf->back->dri_image == NULL) {
dri2_surf->back->dri_image =
dri2_dpy->image->createImage(dri2_dpy->dri_screen,
@@ -455,6 +534,21 @@ static const struct wl_callback_listener frame_listener = {
};
static void
+wayland_sync_callback(void *data,
+ struct wl_callback *callback,
+ uint32_t time)
+{
+ struct dri2_egl_surface *dri2_surf = data;
+
+ dri2_surf->sync_callback = NULL;
+ wl_callback_destroy(callback);
+}
+
+static const struct wl_callback_listener throttle_listener = {
+ wayland_sync_callback
+};
+
+static void
create_wl_buffer(struct dri2_egl_surface *dri2_surf)
{
struct dri2_egl_display *dri2_dpy =
@@ -514,11 +608,14 @@ dri2_swap_buffers_with_damage(_EGLDriver *drv,
if (ret < 0)
return EGL_FALSE;
- dri2_surf->frame_callback = wl_surface_frame(dri2_surf->wl_win->surface);
- wl_callback_add_listener(dri2_surf->frame_callback,
- &frame_listener, dri2_surf);
- wl_proxy_set_queue((struct wl_proxy *) dri2_surf->frame_callback,
- dri2_dpy->wl_queue);
+ if (draw->SwapInterval > 0) {
+ dri2_surf->frame_callback =
+ wl_surface_frame(dri2_surf->wl_win->surface);
+ wl_callback_add_listener(dri2_surf->frame_callback,
+ &frame_listener, dri2_surf);
+ wl_proxy_set_queue((struct wl_proxy *) dri2_surf->frame_callback,
+ dri2_dpy->wl_queue);
+ }
for (i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++)
if (dri2_surf->color_buffers[i].age > 0)
@@ -562,6 +659,16 @@ dri2_swap_buffers_with_damage(_EGLDriver *drv,
wl_surface_commit(dri2_surf->wl_win->surface);
+ /* We always want to throttle to a sync callback even if we are also
+ * waiting for a frame callback. The sync callback is checked in
+ * get_back_bo so that we always give a chance for the compositor to handle
+ * the commit and send a release event before checking for a free buffer */
+ dri2_surf->sync_callback = wl_display_sync(dri2_dpy->wl_dpy);
+ wl_callback_add_listener(dri2_surf->sync_callback,
+ &throttle_listener, dri2_surf);
+ wl_proxy_set_queue((struct wl_proxy *) dri2_surf->sync_callback,
+ dri2_dpy->wl_queue);
+
(*dri2_dpy->flush->flush)(dri2_surf->dri_drawable);
(*dri2_dpy->flush->invalidate)(dri2_surf->dri_drawable);
@@ -808,6 +915,60 @@ static const struct wl_registry_listener registry_listener = {
registry_handle_global_remove
};
+static EGLBoolean
+dri2_swap_interval(_EGLDriver *drv,
+ _EGLDisplay *disp,
+ _EGLSurface *surf,
+ EGLint interval)
+{
+ if (interval > surf->Config->MaxSwapInterval)
+ interval = surf->Config->MaxSwapInterval;
+ else if (interval < surf->Config->MinSwapInterval)
+ interval = surf->Config->MinSwapInterval;
+
+ surf->SwapInterval = interval;
+
+ return EGL_TRUE;
+}
+
+static void
+dri2_setup_swap_interval(struct dri2_egl_display *dri2_dpy)
+{
+ GLint vblank_mode = DRI_CONF_VBLANK_DEF_INTERVAL_1;
+
+ /* We can't use values greater than 1 on Wayland because we are using the
+ * frame callback to synchronise the frame and the only way we be sure to
+ * get a frame callback is to attach a new buffer. Therefore we can't just
+ * sit drawing nothing to wait until the next ‘n’ frame callbacks */
+
+ if (dri2_dpy->config)
+ dri2_dpy->config->configQueryi(dri2_dpy->dri_screen,
+ "vblank_mode", &vblank_mode);
+ switch (vblank_mode) {
+ case DRI_CONF_VBLANK_NEVER:
+ dri2_dpy->min_swap_interval = 0;
+ dri2_dpy->max_swap_interval = 0;
+ dri2_dpy->default_swap_interval = 0;
+ break;
+ case DRI_CONF_VBLANK_ALWAYS_SYNC:
+ dri2_dpy->min_swap_interval = 1;
+ dri2_dpy->max_swap_interval = 1;
+ dri2_dpy->default_swap_interval = 1;
+ break;
+ case DRI_CONF_VBLANK_DEF_INTERVAL_0:
+ dri2_dpy->min_swap_interval = 0;
+ dri2_dpy->max_swap_interval = 1;
+ dri2_dpy->default_swap_interval = 0;
+ break;
+ default:
+ case DRI_CONF_VBLANK_DEF_INTERVAL_1:
+ dri2_dpy->min_swap_interval = 0;
+ dri2_dpy->max_swap_interval = 1;
+ dri2_dpy->default_swap_interval = 1;
+ break;
+ }
+}
+
EGLBoolean
dri2_initialize_wayland(_EGLDriver *drv, _EGLDisplay *disp)
{
@@ -824,6 +985,7 @@ dri2_initialize_wayland(_EGLDriver *drv, _EGLDisplay *disp)
drv->API.DestroySurface = dri2_destroy_surface;
drv->API.SwapBuffers = dri2_swap_buffers;
drv->API.SwapBuffersWithDamageEXT = dri2_swap_buffers_with_damage;
+ drv->API.SwapInterval = dri2_swap_interval;
drv->API.Terminate = dri2_terminate;
drv->API.QueryBufferAge = dri2_query_buffer_age;
drv->API.CreateWaylandBufferFromImageWL =
@@ -883,9 +1045,13 @@ dri2_initialize_wayland(_EGLDriver *drv, _EGLDisplay *disp)
dri2_dpy->extensions[2] = &use_invalidate.base;
dri2_dpy->extensions[3] = NULL;
+ dri2_dpy->swap_available = EGL_TRUE;
+
if (!dri2_create_screen(disp))
goto cleanup_driver;
+ dri2_setup_swap_interval(dri2_dpy);
+
/* The server shouldn't advertise WL_DRM_CAPABILITY_PRIME if the driver
* doesn't have createImageFromFds, since we're using the same driver on
* both sides. We don't want crash if that happens anyway, so fall back to
diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/platform_x11.c
index a518db1..cc9a049 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -39,12 +39,6 @@
#include "egl_dri2.h"
-/* From xmlpool/options.h, user exposed so should be stable */
-#define DRI_CONF_VBLANK_NEVER 0
-#define DRI_CONF_VBLANK_DEF_INTERVAL_0 1
-#define DRI_CONF_VBLANK_DEF_INTERVAL_1 2
-#define DRI_CONF_VBLANK_ALWAYS_SYNC 3
-
static void
swrastCreateDrawable(struct dri2_egl_display * dri2_dpy,
struct dri2_egl_surface * dri2_surf,
--
1.8.3.1
More information about the wayland-devel
mailing list