[PATCH xserver 5/5] xwayland: refactor egl_backends for wayland registry
Lyude Paul
lyude at redhat.com
Mon Jun 4 21:17:45 UTC 2018
On Fri, 2018-06-01 at 16:31 +0200, Olivier Fourdan wrote:
> To be able to check for availability of the Wayland interfaces required
> to run a given EGL backend (either GBM or EGL streams for now), we need
> to have each backend structures and vfuncs in place before we enter the
> Wayland registry dance.
>
> That basically means that we should init all backends at first, connect
> to the Wayland compositor and query the available interfaces and then
> decide which backend is available and should be used (or none if either
> the Wayland interfaces or the EGL extensions are not available).
>
> For this purpose, hold an egl_backend struct for each backend we are to
> consider prior to connect to the Wayland display so that, when we get to
> query the Wayland interfaces, everything is in place for each backend to
> handle the various Wayland interfaces.
>
> Eventually, when we need to chose which EGL backend to use for glamor,
> the available Wayland interfaces and EGL extensions available are all
> known to Xwayland.
>
> Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
> ---
> hw/xwayland/xwayland-glamor-eglstream.c | 27 ++++++----
> hw/xwayland/xwayland-glamor-gbm.c | 38 ++++++++++----
> hw/xwayland/xwayland-glamor.c | 69 ++++++++++++++++++-------
> hw/xwayland/xwayland-present.c | 7 +--
> hw/xwayland/xwayland.c | 10 ++--
> hw/xwayland/xwayland.h | 15 ++++--
> 6 files changed, 118 insertions(+), 48 deletions(-)
>
> diff --git a/hw/xwayland/xwayland-glamor-eglstream.c b/hw/xwayland/xwayland-
> glamor-eglstream.c
> index 85b562c31..ee03c4a86 100644
> --- a/hw/xwayland/xwayland-glamor-eglstream.c
> +++ b/hw/xwayland/xwayland-glamor-eglstream.c
> @@ -638,11 +638,11 @@ const struct wl_eglstream_display_listener
> eglstream_display_listener = {
> .swapinterval_override =
> xwl_eglstream_display_handle_swapinterval_override,
> };
>
> -static void
> +static Bool
> xwl_glamor_eglstream_init_wl_registry(struct xwl_screen *xwl_screen,
> struct wl_registry *wl_registry,
> - const char *name,
> - uint32_t id, uint32_t version)
> + uint32_t id, const char *name,
> + uint32_t version)
> {
> struct xwl_eglstream_private *xwl_eglstream =
> xwl_eglstream_get(xwl_screen);
> @@ -654,10 +654,15 @@ xwl_glamor_eglstream_init_wl_registry(struct
> xwl_screen *xwl_screen,
> wl_eglstream_display_add_listener(xwl_eglstream->display,
> &eglstream_display_listener,
> xwl_screen);
> + return TRUE;
> } else if (strcmp(name, "wl_eglstream_controller") == 0) {
> xwl_eglstream->controller = wl_registry_bind(
> wl_registry, id, &wl_eglstream_controller_interface, version);
> + return TRUE;
> }
> +
> + /* no match */
> + return FALSE;
> }
>
> static Bool
> @@ -896,6 +901,7 @@ xwl_glamor_init_eglstream(struct xwl_screen *xwl_screen)
> struct xwl_eglstream_private *xwl_eglstream;
> EGLDeviceEXT egl_device;
>
> + xwl_screen->eglstreams_backend.is_available = FALSE;
> egl_device = xwl_eglstream_get_device(xwl_screen);
> if (egl_device == EGL_NO_DEVICE_EXT)
> return FALSE;
> @@ -915,13 +921,14 @@ xwl_glamor_init_eglstream(struct xwl_screen
> *xwl_screen)
> xwl_eglstream->egl_device = egl_device;
> xorg_list_init(&xwl_eglstream->pending_streams);
>
> - xwl_screen->egl_backend.init_egl = xwl_glamor_eglstream_init_egl;
> - xwl_screen->egl_backend.init_wl_registry =
> xwl_glamor_eglstream_init_wl_registry;
> - xwl_screen->egl_backend.has_wl_interfaces =
> xwl_glamor_eglstream_has_wl_interfaces;
> - xwl_screen->egl_backend.init_screen = xwl_glamor_eglstream_init_screen;
> - xwl_screen->egl_backend.get_wl_buffer_for_pixmap =
> xwl_glamor_eglstream_get_wl_buffer_for_pixmap;
> - xwl_screen->egl_backend.post_damage = xwl_glamor_eglstream_post_damage;
> - xwl_screen->egl_backend.allow_commits =
> xwl_glamor_eglstream_allow_commits;
> + xwl_screen->eglstreams_backend.init_egl =
> xwl_glamor_eglstream_init_egl;
> + xwl_screen->eglstreams_backend.init_wl_registry =
> xwl_glamor_eglstream_init_wl_registry;
> + xwl_screen->eglstreams_backend.has_wl_interfaces =
> xwl_glamor_eglstream_has_wl_interfaces;
> + xwl_screen->eglstreams_backend.init_screen =
> xwl_glamor_eglstream_init_screen;
> + xwl_screen->eglstreams_backend.get_wl_buffer_for_pixmap =
> xwl_glamor_eglstream_get_wl_buffer_for_pixmap;
> + xwl_screen->eglstreams_backend.post_damage =
> xwl_glamor_eglstream_post_damage;
> + xwl_screen->eglstreams_backend.allow_commits =
> xwl_glamor_eglstream_allow_commits;
> + xwl_screen->eglstreams_backend.is_available = TRUE;
>
> ErrorF("glamor: Using nvidia's eglstream interface, direct rendering
> impossible.\n");
> ErrorF("glamor: Performance may be affected. Ask your vendor to support
> GBM!\n");
> diff --git a/hw/xwayland/xwayland-glamor-gbm.c b/hw/xwayland/xwayland-
> glamor-gbm.c
> index f34c45b9a..3566af0bb 100644
> --- a/hw/xwayland/xwayland-glamor-gbm.c
> +++ b/hw/xwayland/xwayland-glamor-gbm.c
> @@ -734,16 +734,28 @@ xwl_screen_set_dmabuf_interface(struct xwl_screen
> *xwl_screen,
> return TRUE;
> }
>
> -static void
> +static Bool
> xwl_glamor_gbm_init_wl_registry(struct xwl_screen *xwl_screen,
> struct wl_registry *wl_registry,
> - const char *name,
> - uint32_t id, uint32_t version)
> + uint32_t id, const char *name,
> + uint32_t version)
> {
> - if (strcmp(name, "wl_drm") == 0)
> + if (strcmp(name, "wl_drm") == 0) {
> xwl_screen_set_drm_interface(xwl_screen, id, version);
> - else if (strcmp(name, "zwp_linux_dmabuf_v1") == 0)
> + return TRUE;
> + } else if (strcmp(name, "zwp_linux_dmabuf_v1") == 0) {
> xwl_screen_set_dmabuf_interface(xwl_screen, id, version);
> + return TRUE;
> + }
> +
> + /* no match */
> + return FALSE;
> +}
> +
> +static Bool
> +xwl_glamor_gbm_has_egl_extension(void)
> +{
> + return epoxy_has_egl_extension(NULL, "EGL_MESA_platform_gbm");
> }
>
> static Bool
> @@ -879,6 +891,11 @@ xwl_glamor_init_gbm(struct xwl_screen *xwl_screen)
> {
> struct xwl_gbm_private *xwl_gbm;
>
> + xwl_screen->gbm_backend.is_available = FALSE;
> +
> + if (!xwl_glamor_gbm_has_egl_extension())
> + return FALSE;
> +
> if (!dixRegisterPrivateKey(&xwl_gbm_private_key, PRIVATE_SCREEN, 0))
> return FALSE;
>
> @@ -891,11 +908,12 @@ xwl_glamor_init_gbm(struct xwl_screen *xwl_screen)
> dixSetPrivate(&xwl_screen->screen->devPrivates, &xwl_gbm_private_key,
> xwl_gbm);
>
> - xwl_screen->egl_backend.init_wl_registry =
> xwl_glamor_gbm_init_wl_registry;
> - xwl_screen->egl_backend.has_wl_interfaces =
> xwl_glamor_gbm_has_wl_interfaces;
> - xwl_screen->egl_backend.init_egl = xwl_glamor_gbm_init_egl;
> - xwl_screen->egl_backend.init_screen = xwl_glamor_gbm_init_screen;
> - xwl_screen->egl_backend.get_wl_buffer_for_pixmap =
> xwl_glamor_gbm_get_wl_buffer_for_pixmap;
> + xwl_screen->gbm_backend.init_wl_registry =
> xwl_glamor_gbm_init_wl_registry;
> + xwl_screen->gbm_backend.has_wl_interfaces =
> xwl_glamor_gbm_has_wl_interfaces;
> + xwl_screen->gbm_backend.init_egl = xwl_glamor_gbm_init_egl;
> + xwl_screen->gbm_backend.init_screen = xwl_glamor_gbm_init_screen;
> + xwl_screen->gbm_backend.get_wl_buffer_for_pixmap =
> xwl_glamor_gbm_get_wl_buffer_for_pixmap;
> + xwl_screen->gbm_backend.is_available = TRUE;
>
> return TRUE;
> }
> diff --git a/hw/xwayland/xwayland-glamor.c b/hw/xwayland/xwayland-glamor.c
> index eb8ebf032..9d15a0351 100644
> --- a/hw/xwayland/xwayland-glamor.c
> +++ b/hw/xwayland/xwayland-glamor.c
> @@ -71,9 +71,24 @@ xwl_glamor_init_wl_registry(struct xwl_screen
> *xwl_screen,
> uint32_t id, const char *interface,
> uint32_t version)
> {
> - if (xwl_screen->egl_backend.init_wl_registry)
> - xwl_screen->egl_backend.init_wl_registry(xwl_screen, registry,
> - interface, id, version);
> +#ifdef GLAMOR_HAS_GBM
> + if (xwl_screen->gbm_backend.is_available &&
> + xwl_screen->gbm_backend.init_wl_registry &&
> + xwl_screen->gbm_backend.init_wl_registry(xwl_screen,
> + registry,
> + id,
> + interface,
> + version)); /* no-op */
> +#endif
> +#ifdef XWL_HAS_EGLSTREAM
> + else if (xwl_screen->eglstreams_backend.is_available &&
> + xwl_screen->eglstreams_backend.init_wl_registry &&
> + xwl_screen->eglstreams_backend.init_wl_registry(xwl_screen,
> + registry,
> + id,
> + interface,
> + version)); /*
> no-op */
> +#endif
> }
>
> Bool
> @@ -95,8 +110,8 @@ xwl_glamor_pixmap_get_wl_buffer(PixmapPtr pixmap,
> {
> struct xwl_screen *xwl_screen = xwl_screen_get(pixmap-
> >drawable.pScreen);
>
> - if (xwl_screen->egl_backend.get_wl_buffer_for_pixmap)
> - return xwl_screen->egl_backend.get_wl_buffer_for_pixmap(pixmap,
> + if (xwl_screen->egl_backend->get_wl_buffer_for_pixmap)
> + return xwl_screen->egl_backend->get_wl_buffer_for_pixmap(pixmap,
> width,
> height,
> created);
> @@ -110,8 +125,8 @@ xwl_glamor_post_damage(struct xwl_window *xwl_window,
> {
> struct xwl_screen *xwl_screen = xwl_window->xwl_screen;
>
> - if (xwl_screen->egl_backend.post_damage)
> - xwl_screen->egl_backend.post_damage(xwl_window, pixmap, region);
> + if (xwl_screen->egl_backend->post_damage)
> + xwl_screen->egl_backend->post_damage(xwl_window, pixmap, region);
> }
>
> Bool
> @@ -119,8 +134,8 @@ xwl_glamor_allow_commits(struct xwl_window *xwl_window)
> {
> struct xwl_screen *xwl_screen = xwl_window->xwl_screen;
>
> - if (xwl_screen->egl_backend.allow_commits)
> - return xwl_screen->egl_backend.allow_commits(xwl_window);
> + if (xwl_screen->egl_backend && xwl_screen->egl_backend->allow_commits)
> + return xwl_screen->egl_backend->allow_commits(xwl_window);
> else
> return TRUE;
> }
> @@ -165,17 +180,35 @@ glamor_egl_fd_name_from_pixmap(ScreenPtr screen,
> void
> xwl_glamor_init_backends(struct xwl_screen *xwl_screen, Bool
> use_eglstreams)
> {
> +#ifdef GLAMOR_HAS_GBM
> + /* Init GBM even if "-eglstream" is requested, as EGL streams may fail
> */
> + xwl_glamor_init_gbm(xwl_screen);
> +#endif
> #ifdef XWL_HAS_EGLSTREAM
> + xwl_glamor_init_eglstream(xwl_screen);
> if (use_eglstreams) {
> - if (!xwl_glamor_init_eglstream(xwl_screen)) {
> - ErrorF("xwayland glamor: failed to setup eglstream backend\n");
> - use_eglstreams = FALSE;
> - }
> + /* Force GBM backend off */
> + xwl_screen->gbm_backend.is_available = FALSE;
> + if (!xwl_screen->eglstreams_backend.is_available)
> + ErrorF("xwayland glamor: EGLstreams requested but not
> available\n");
> }
> #endif
> - if (!use_eglstreams && !xwl_glamor_init_gbm(xwl_screen)) {
> - ErrorF("xwayland glamor: failed to setup GBM backend, falling back
> to sw accel\n");
> - xwl_screen->glamor = 0;
> +}
> +
> +void
> +xwl_glamor_select_backend(struct xwl_screen *xwl_screen, Bool
> use_eglstreams)
> +{
> + if (xwl_screen->egl_backend == NULL && xwl_screen-
> >gbm_backend.is_available) {
> + if (xwl_glamor_has_wl_interfaces(xwl_screen, &xwl_screen-
> >gbm_backend))
> + xwl_screen->egl_backend = &xwl_screen->gbm_backend;
> + else
> + ErrorF("Missing Wayland requirements for glamor GBM
> backend\n");
> + }
> + if (xwl_screen->egl_backend == NULL && xwl_screen-
> >eglstreams_backend.is_available) {
> + if (xwl_glamor_has_wl_interfaces(xwl_screen, &xwl_screen-
> >eglstreams_backend))
> + xwl_screen->egl_backend = &xwl_screen->eglstreams_backend;
> + else
> + ErrorF("Missing Wayland requirements for glamor EGL streams
> backend\n");
> }
> }
>
> @@ -191,7 +224,7 @@ xwl_glamor_init(struct xwl_screen *xwl_screen)
> return FALSE;
> }
>
> - if (!xwl_screen->egl_backend.init_egl(xwl_screen)) {
> + if (!xwl_screen->egl_backend->init_egl(xwl_screen)) {
> ErrorF("EGL setup failed, disabling glamor\n");
> return FALSE;
> }
> @@ -201,7 +234,7 @@ xwl_glamor_init(struct xwl_screen *xwl_screen)
> return FALSE;
> }
>
> - if (!xwl_screen->egl_backend.init_screen(xwl_screen)) {
> + if (!xwl_screen->egl_backend->init_screen(xwl_screen)) {
> ErrorF("EGL backend init_screen() failed, disabling glamor\n");
> return FALSE;
> }
> diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c
> index b5ae80dcf..b36c59b24 100644
> --- a/hw/xwayland/xwayland-present.c
> +++ b/hw/xwayland/xwayland-present.c
> @@ -544,14 +544,15 @@ static present_wnmd_info_rec xwl_present_info = {
> Bool
> xwl_present_init(ScreenPtr screen)
> {
> +#ifdef XWL_HAS_EGLSTREAM
> struct xwl_screen *xwl_screen = xwl_screen_get(screen);
>
> /*
> - * doesn't work with the streams backend. we don't have an explicit
> - * boolean for that, but we do know gbm doesn't fill in this hook...
> + * doesn't work with the streams backend.
> */
> - if (xwl_screen->egl_backend.post_damage != NULL)
> + if (xwl_screen->egl_backend == &xwl_screen->eglstreams_backend)
> return FALSE;
> +#endif
Shouldn't this be in it's own patch?
Additionally this does slightly break the warning I had added for initializing
the EGLStreams backend to discourage people from using it:
glamor: Using nvidia's eglstream interface, direct rendering impossible.
glamor: Performance may be affected. Ask your vendor to support GBM!
glamor: 'wl_drm' not supported
Missing Wayland requirements for glamor GBM backend
With those two things fixed, the whole series is:
Reviewed-by: Lyude Paul <lyude at redhat.com>
>
> if (!dixRegisterPrivateKey(&xwl_present_window_private_key,
> PRIVATE_WINDOW, 0))
> return FALSE;
> diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
> index 0584b53e6..2f31688fa 100644
> --- a/hw/xwayland/xwayland.c
> +++ b/hw/xwayland/xwayland.c
> @@ -1079,9 +1079,13 @@ xwl_screen_init(ScreenPtr pScreen, int argc, char
> **argv)
> return FALSE;
>
> #ifdef XWL_HAS_GLAMOR
> - if (xwl_screen->glamor && !xwl_glamor_init(xwl_screen)) {
> - ErrorF("Failed to initialize glamor, falling back to sw\n");
> - xwl_screen->glamor = 0;
> + if (xwl_screen->glamor) {
> + xwl_glamor_select_backend(xwl_screen, use_eglstreams);
> +
> + if (xwl_screen->egl_backend == NULL ||
> !xwl_glamor_init(xwl_screen)) {
> + ErrorF("Failed to initialize glamor, falling back to sw\n");
> + xwl_screen->glamor = 0;
> + }
> }
>
> if (xwl_screen->glamor && xwl_screen->rootless)
> diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
> index 40918e87b..e4c6455a5 100644
> --- a/hw/xwayland/xwayland.h
> +++ b/hw/xwayland/xwayland.h
> @@ -60,13 +60,16 @@ struct xwl_window;
> struct xwl_screen;
>
> struct xwl_egl_backend {
> + /* Set by the backend if available */
> + Bool is_available;
> +
> /* Called once for each interface in the global registry. Backends
> * should use this to bind to any wayland interfaces they need. This
> * callback is optional.
> */
> - void (*init_wl_registry)(struct xwl_screen *xwl_screen,
> + Bool (*init_wl_registry)(struct xwl_screen *xwl_screen,
> struct wl_registry *wl_registry,
> - const char *name, uint32_t id,
> + uint32_t id, const char *name,
> uint32_t version);
>
> /* Check that the required Wayland interfaces are available. This
> @@ -164,8 +167,10 @@ struct xwl_screen {
> struct xwl_format *formats;
> void *egl_display, *egl_context;
>
> - /* the current backend for creating pixmaps on wayland */
> - struct xwl_egl_backend egl_backend;
> + struct xwl_egl_backend gbm_backend;
> + struct xwl_egl_backend eglstreams_backend;
> + /* pointer to the current backend for creating pixmaps on wayland */
> + struct xwl_egl_backend *egl_backend;
>
> struct glamor_context *glamor_ctx;
>
> @@ -425,6 +430,8 @@ struct wl_buffer *xwl_shm_pixmap_get_wl_buffer(PixmapPtr
> pixmap);
> #ifdef XWL_HAS_GLAMOR
> void xwl_glamor_init_backends(struct xwl_screen *xwl_screen,
> Bool use_eglstreams);
> +void xwl_glamor_select_backend(struct xwl_screen *xwl_screen,
> + Bool use_eglstreams);
> Bool xwl_glamor_init(struct xwl_screen *xwl_screen);
>
> Bool xwl_screen_set_drm_interface(struct xwl_screen *xwl_screen,
--
Cheers,
Lyude Paul
More information about the xorg-devel
mailing list