[PATCH mesa v5 2/2] wayland: Add support for eglSwapInterval
Axel Davy
davy at clipper.ens.fr
Fri Nov 15 06:15:09 PST 2013
I'm not sure waiting 10ms when all the buffers aren't released is the
best solution.
In fact, I experimented a eglSwapInterval(0) behaviour with a simple
patch to enable it:
having 4 back buffers and using a roundtrip after commiting (that is: a
bit similar to what you do, but without the poll)
And with glmark2, for all the benchmarks (10 seconds by benchmark) I had
4-5 egl error messages
because there was no buffer free in get_back_bo (the benchmarks continued).
That makes me think that the poll case might happen more often than you
think.
I suggest to have a lighter solution: for example poll a first time with
1ms instead of 10ms,
and then if no buffers are free, try again with a poll of 10ms.
Why not doing directly another roundtrip?
Axel Davy
Le 15/11/2013 14:50, Neil Roberts a écrit :
> 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.
> Instead it will do a sync request after the swap buffers. It will block for
> sync complete event in get_back_bo instead of the frame callback. 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.
>
> 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 | 10 +-
> src/egl/drivers/dri2/platform_wayland.c | 199 +++++++++++++++++++++++++++-----
> src/egl/drivers/dri2/platform_x11.c | 6 -
> 3 files changed, 179 insertions(+), 36 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
> index 8c303d9..d29dd98 100644
> --- a/src/egl/drivers/dri2/egl_dri2.h
> +++ b/src/egl/drivers/dri2/egl_dri2.h
> @@ -174,7 +174,7 @@ struct dri2_egl_surface
> struct wl_egl_window *wl_win;
> int dx;
> int dy;
> - struct wl_callback *frame_callback;
> + struct wl_callback *throttle_callback;
> int format;
> #endif
>
> @@ -194,7 +194,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 +220,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 5ce8981..fca35db 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;
> }
>
> /**
> @@ -217,8 +226,8 @@ dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *surf)
> dri2_dpy->dri2->releaseBuffer(dri2_dpy->dri_screen,
> dri2_surf->dri_buffers[i]);
>
> - if (dri2_surf->frame_callback)
> - wl_callback_destroy(dri2_surf->frame_callback);
> + if (dri2_surf->throttle_callback)
> + wl_callback_destroy(dri2_surf->throttle_callback);
>
> if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
> dri2_surf->wl_win->private = NULL;
> @@ -257,41 +266,99 @@ 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)
> {
> struct dri2_egl_display *dri2_dpy =
> dri2_egl_display(dri2_surf->base.Resource.Display);
> int i;
>
> - if (dri2_surf->frame_callback == NULL) {
> + if (dri2_surf->throttle_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 throttle to the frame callback here so that we can be sure to have
> - * received any release events before trying to decide whether to
> - * allocate a new buffer */
> + /* We always want to throttle to some event (either a frame callback or
> + * a sync request) 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->frame_callback != NULL);
> + } while (dri2_surf->throttle_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,
> @@ -497,16 +564,18 @@ static const __DRIimageLoaderExtension image_loader_extension = {
> };
>
> static void
> -wayland_frame_callback(void *data, struct wl_callback *callback, uint32_t time)
> +wayland_throttle_callback(void *data,
> + struct wl_callback *callback,
> + uint32_t time)
> {
> struct dri2_egl_surface *dri2_surf = data;
>
> - dri2_surf->frame_callback = NULL;
> + dri2_surf->throttle_callback = NULL;
> wl_callback_destroy(callback);
> }
>
> -static const struct wl_callback_listener frame_listener = {
> - wayland_frame_callback
> +static const struct wl_callback_listener throttle_listener = {
> + wayland_throttle_callback
> };
>
> static void
> @@ -581,11 +650,14 @@ dri2_swap_buffers_with_damage(_EGLDriver *drv,
> 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->throttle_callback =
> + wl_surface_frame(dri2_surf->wl_win->surface);
> + wl_callback_add_listener(dri2_surf->throttle_callback,
> + &throttle_listener, dri2_surf);
> + wl_proxy_set_queue((struct wl_proxy *) dri2_surf->throttle_callback,
> + dri2_dpy->wl_queue);
> + }
>
> dri2_surf->back->age = 1;
> dri2_surf->current = dri2_surf->back;
> @@ -618,6 +690,18 @@ dri2_swap_buffers_with_damage(_EGLDriver *drv,
>
> wl_surface_commit(dri2_surf->wl_win->surface);
>
> + /* If we're not waiting for a frame callback then we'll at least throttle
> + * to a sync callback 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 */
> + if (dri2_surf->throttle_callback == NULL) {
> + dri2_surf->throttle_callback = wl_display_sync(dri2_dpy->wl_dpy);
> + wl_callback_add_listener(dri2_surf->throttle_callback,
> + &throttle_listener, dri2_surf);
> + wl_proxy_set_queue((struct wl_proxy *) dri2_surf->throttle_callback,
> + dri2_dpy->wl_queue);
> + }
> +
> (*dri2_dpy->flush->flush)(dri2_surf->dri_drawable);
> (*dri2_dpy->flush->invalidate)(dri2_surf->dri_drawable);
>
> @@ -787,6 +871,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)
> {
> @@ -803,6 +941,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;
>
> @@ -861,9 +1000,13 @@ dri2_initialize_wayland(_EGLDriver *drv, _EGLDisplay *disp)
> dri2_dpy->extensions[3] = &use_invalidate.base;
> dri2_dpy->extensions[4] = 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 c56a413..413518e 100644
> --- a/src/egl/drivers/dri2/platform_x11.c
> +++ b/src/egl/drivers/dri2/platform_x11.c
> @@ -41,12 +41,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,
More information about the wayland-devel
mailing list