[PATCH xserver 1/3] xwayland: Add hook to check wl interface for glamor

[Lyude Paul]

On Wed, 2018-05-30 at 11:30 +0200, Olivier Fourdan wrote:
> Add an optional egl_backend callback to check that the required Wayland
> interfaces are available.
> 
> Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
> ---
>  hw/xwayland/xwayland-glamor.c | 9 +++++++++
>  hw/xwayland/xwayland.c        | 4 ++++
>  hw/xwayland/xwayland.h        | 7 +++++++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/hw/xwayland/xwayland-glamor.c b/hw/xwayland/xwayland-glamor.c
> index 6ae274c08..aeb0b27b9 100644
> --- a/hw/xwayland/xwayland-glamor.c
> +++ b/hw/xwayland/xwayland-glamor.c
> @@ -162,6 +162,15 @@ xwl_glamor_init_wl_registry(struct xwl_screen
> *xwl_screen,
>                                                   interface, id, version);
>  }
>  
> +Bool
> +xwl_glamor_has_wl_interface(struct xwl_screen *xwl_screen)
> +{
> +    if (xwl_screen->egl_backend.has_wl_interface)
> +        return xwl_screen->egl_backend.has_wl_interface(xwl_screen);
> +
> +    return TRUE;
> +}
> +
>  struct wl_buffer *
>  xwl_glamor_pixmap_get_wl_buffer(PixmapPtr pixmap,
>                                  unsigned short width,
> diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
> index 4dd8f3810..d831291ca 100644
> --- a/hw/xwayland/xwayland.c
> +++ b/hw/xwayland/xwayland.c
> @@ -1091,6 +1091,10 @@ xwl_screen_init(ScreenPtr pScreen, int argc, char
> **argv)
>          return FALSE;
>  
>  #ifdef XWL_HAS_GLAMOR
> +    if (xwl_screen->glamor && !xwl_glamor_has_wl_interface(xwl_screen)) {
> +        ErrorF("Missing Wayland interfaces required for glamor, falling
> back to sw\n");
> +        xwl_screen->glamor = 0;
> +    }
NAK. I'm starting to see the problems with this approach I originally hit now.

So: there's some unexpected catches when it comes to EGL client extensions. On
a system using glvnd with the nvidia driver, the EGL client extension list
will have /both/ the client extensions for the nvidia EGL driver and the
client extensions for the Mesa driver. An example from my test machine:

EGL client extensions string:
    EGL_EXT_platform_base EGL_EXT_device_base # ← nvidia!
    EGL_KHR_client_get_all_proc_addresses EGL_EXT_client_extensions
    EGL_KHR_debug EGL_EXT_platform_x11 EGL_EXT_platform_device
    EGL_EXT_platform_wayland EGL_MESA_platform_gbm # ← mesa!
    EGL_MESA_platform_surfaceless

This is fine, but what it means is that we're going to have to take the
presence of either of these extensions as a sign that their respective EGL
backends -might- be there. e.g. we can't try to make a choice on avoiding
trying EGLStreams simply because GBM might be there, which is currently what
this patch does:

 * During init, xwl_glamor_should_use_gbm() tells us we should prefer GBM over
   EGLStreams
 * We select the gbm egl backend's hook for checking for the wl interfaces
 * In xwl_screen_init(), we check for wl_drm and don't see it
 * xwl_glamor_has_wl_interface() returns FALSE, which makes us disable glamor
   and fall back to software rendering

I think the approach we're going to have to take instead is to use the EGL
extensions to determine which EGL backends we should try probing the wl
interfaces for. If -eglstream gets specified, we only try probing for EGL
streams, etc. etc.

Afterwards, if we have gbm we try probing for it's wl interfaces. If that
works, select gbm as the egl backend. If that doesn't work and we have
eglstreams, try probing for it's wl interfaces. If that works, use eglstreams
as the EGL backend. And finally, if all else fails default to sw.

iirc, this is basically what I did very early on when working on adding
eglstreams.
>      if (xwl_screen->glamor && !xwl_glamor_init(xwl_screen)) {
>          ErrorF("Failed to initialize glamor, falling back to sw\n");
>          xwl_screen->glamor = 0;
> diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
> index ff746114c..62cb40f08 100644
> --- a/hw/xwayland/xwayland.h
> +++ b/hw/xwayland/xwayland.h
> @@ -120,6 +120,12 @@ struct xwl_screen {
>                                   const char *name, uint32_t id,
>                                   uint32_t version);
>  
> +        /* Check that the required Wayland interfaces are available.
> +         * This callback is optional.
> +         */
> +        Bool (*has_wl_interface)(struct xwl_screen *xwl_screen);
> +
> +
>          /* Called before glamor has been initialized. Backends should setup
> a
>           * valid, glamor compatible EGL context in this hook.
>           */
> @@ -429,6 +435,7 @@ void xwl_glamor_init_wl_registry(struct xwl_screen
> *xwl_screen,
>                                   struct wl_registry *registry,
>                                   uint32_t id, const char *interface,
>                                   uint32_t version);
> +Bool xwl_glamor_has_wl_interface(struct xwl_screen *xwl_screen);
>  void xwl_glamor_post_damage(struct xwl_window *xwl_window,
>                              PixmapPtr pixmap, RegionPtr region);
>  Bool xwl_glamor_allow_commits(struct xwl_window *xwl_window);
-- 
Cheers,
	Lyude Paul