[PATCH 1/2] wayland: Add support for eglSwapInterval

Pekka Paalanen ppaalanen at gmail.com
Fri Sep 13 03:48:03 PDT 2013


On Wed, 11 Sep 2013 22:29:13 +0200
Axel Davy <davy at clipper.ens.fr> wrote:

> I think you should too set the number of back buffers to 4 instead of
> 3.
> 
> It looks like if the compositor wants to use the buffers as
> framebuffers and do a pageflip,
> it uses 2 buffers at a time (one used for the frame displayed, and
> one used for the pending pageFlip request). The third buffer sent is
> not released and waits for the next frame. If there is only 3
> buffers, the client will be without free buffers and blocked in
> eglSwapBuffers. I may be wrong, but I think the number of back
> buffers should be set to 4.

We had a lengthy discussion in IRC about that, and came to the idea
that we might need the maximum to be actually 5, not 4.

The +1 comes from sub-surfaces, and *might* happen when you have a
swapinterval=0 EGL rendering on a sub-surface in a busy-loop in a
thread, the sub-surface is in synchronized mode, parent surface gets
committed only occasionally, and the sub-surface ends up in an hardware
overlay plane. Sheesh...

If we really want to guarantee maximal framerate on the sub-surface
whose display is throttled not only by the compositor, but other parts
of the client too...


Thanks,
pq

> Le 11/09/2013 20:28, 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 it simply
> > doesn't install a frame callback so that the next time
> > eglSwapBuffers is called it won't delay.
> >
> > The second change is that in get_back_bo instead of returning with
> > an error if all three buffers are locked it will now block in a
> > dispatch loop so that it can receive the buffer release events. The
> > assumption is that the compositor is unlikely to lock all three
> > buffers so if we find that all the buffers are locked then we are
> > probably just rendering faster than we are processing the release
> > events. Therefore the release events should be available very early.
> >
> > 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         |   6 ++
> >   src/egl/drivers/dri2/platform_wayland.c | 121
> > ++++++++++++++++++++++++++------
> > src/egl/drivers/dri2/platform_x11.c     |   6 -- 3 files changed,
> > 107 insertions(+), 26 deletions(-)
> >
> > diff --git a/src/egl/drivers/dri2/egl_dri2.h
> > b/src/egl/drivers/dri2/egl_dri2.h index fba5f81..849927b 100644
> > --- a/src/egl/drivers/dri2/egl_dri2.h
> > +++ b/src/egl/drivers/dri2/egl_dri2.h
> > @@ -221,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 ffc5959..83e7aab
> > 100644 --- a/src/egl/drivers/dri2/platform_wayland.c
> > +++ b/src/egl/drivers/dri2/platform_wayland.c
> > @@ -180,8 +180,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;
> >   }
> >   
> >   /**
> > @@ -261,24 +269,36 @@ 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->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];
> > +      /* 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); +
> > +      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)
> > +            break;
> > +
> > +         /* If we make it here then here then all of the buffers
> > are locked.
> > +          * It wouldn't make sense for the compositor to keep all
> > three
> > +          * buffers so it must mean that we are rendering too fast
> > without
> > +          * getting a chance to see the release events. Therefore
> > we can just
> > +          * block for the release event here */
> > +         if (wl_display_dispatch_queue(dri2_dpy->wl_dpy,
> > +                                       dri2_dpy->wl_queue) < 0)
> > +            return -1;
> >         }
> >      }
> >   
> > -   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,
> > @@ -511,11 +531,13 @@ 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)
> > @@ -802,6 +824,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)
> >   {
> > @@ -817,6 +893,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 =
> > @@ -876,9 +953,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 ec76aec..f2cd4fc 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,
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel



More information about the wayland-devel mailing list