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 <ofour...@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 <ly...@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 _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel