[Mesa-dev] [PATCH 2/3] egl/x11: store xcb_screen_t *screen instead of int screen

Eric Engestrom eric.engestrom at imgtec.com
Thu Nov 17 13:59:46 UTC 2016


On Friday, 2016-11-11 16:31:16 +0000, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov at collabora.com>
> 
> Just fetch and store it once, rather than doing the
> xcb_setup_roots_iterator + get_xcb_screen dance five times.
> 
> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> ---
> These two patches are an example of the duplication we have within each 
> loader (be that egl, gbm or glx)_and how we can fold it up :-)
> 
>  src/egl/drivers/dri2/egl_dri2.h          |  2 +-
>  src/egl/drivers/dri2/platform_x11.c      | 51 ++++++++++----------------------
>  src/egl/drivers/dri2/platform_x11_dri3.c | 32 ++------------------
>  3 files changed, 19 insertions(+), 66 deletions(-)
> 
> diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
> index 0020a5b..fa94cbe 100644
> --- a/src/egl/drivers/dri2/egl_dri2.h
> +++ b/src/egl/drivers/dri2/egl_dri2.h
> @@ -199,7 +199,7 @@ struct dri2_egl_display
>  
>  #ifdef HAVE_X11_PLATFORM
>     xcb_connection_t         *conn;
> -   int                      screen;
> +   xcb_screen_t             *screen;
>     int                      swap_available;
>  #ifdef HAVE_DRI3
>     struct loader_dri3_extensions loader_dri3_ext;
> diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/platform_x11.c
> index e152868..f4bcf4a 100644
> --- a/src/egl/drivers/dri2/platform_x11.c
> +++ b/src/egl/drivers/dri2/platform_x11.c
> @@ -206,10 +206,8 @@ dri2_x11_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type,
>     struct dri2_egl_surface *dri2_surf;
>     xcb_get_geometry_cookie_t cookie;
>     xcb_get_geometry_reply_t *reply;
> -   xcb_screen_iterator_t s;
>     xcb_generic_error_t *error;
>     xcb_drawable_t drawable;
> -   xcb_screen_t *screen;
>     const __DRIconfig *config;
>  
>     STATIC_ASSERT(sizeof(uintptr_t) == sizeof(native_surface));
> @@ -228,16 +226,9 @@ dri2_x11_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type,
>  
>     dri2_surf->region = XCB_NONE;
>     if (type == EGL_PBUFFER_BIT) {
> -      s = xcb_setup_roots_iterator(xcb_get_setup(dri2_dpy->conn));
> -      screen = get_xcb_screen(s, dri2_dpy->screen);
> -      if (!screen) {
> -         _eglError(EGL_BAD_ALLOC, "failed to get xcb screen");
> -         goto cleanup_surf;
> -      }
> -
>        dri2_surf->drawable = xcb_generate_id(dri2_dpy->conn);
>        xcb_create_pixmap(dri2_dpy->conn, conf->BufferSize,
> -                       dri2_surf->drawable, screen->root,
> +                       dri2_surf->drawable, dri2_dpy->screen->root,
>  			dri2_surf->base.Width, dri2_surf->base.Height);
>     } else {
>        if (!drawable) {
> @@ -544,20 +535,10 @@ dri2_x11_do_authenticate(struct dri2_egl_display *dri2_dpy, uint32_t id)
>  {
>     xcb_dri2_authenticate_reply_t *authenticate;
>     xcb_dri2_authenticate_cookie_t authenticate_cookie;
> -   xcb_screen_iterator_t s;
> -   xcb_screen_t *screen;
>     int ret = 0;
>  
> -   s = xcb_setup_roots_iterator(xcb_get_setup(dri2_dpy->conn));
> -
> -   screen = get_xcb_screen(s, dri2_dpy->screen);
> -   if (!screen) {
> -      _eglLog(_EGL_WARNING, "DRI2: failed to get xcb screen");
> -      return -1;
> -   }
> -
>     authenticate_cookie =
> -      xcb_dri2_authenticate_unchecked(dri2_dpy->conn, screen->root, id);
> +      xcb_dri2_authenticate_unchecked(dri2_dpy->conn, dri2_dpy->screen->root, id);
>     authenticate =
>        xcb_dri2_authenticate_reply(dri2_dpy->conn, authenticate_cookie, NULL);
>  
> @@ -598,8 +579,6 @@ dri2_x11_connect(struct dri2_egl_display *dri2_dpy)
>     xcb_dri2_connect_reply_t *connect;
>     xcb_dri2_connect_cookie_t connect_cookie;
>     xcb_generic_error_t *error;
> -   xcb_screen_iterator_t s;
> -   xcb_screen_t *screen;
>     char *driver_name, *loader_driver_name, *device_name;
>     const xcb_query_extension_reply_t *extension;
>  
> @@ -622,13 +601,7 @@ dri2_x11_connect(struct dri2_egl_display *dri2_dpy)
>  					       XCB_DRI2_MAJOR_VERSION,
>  					       XCB_DRI2_MINOR_VERSION);
>  
> -   s = xcb_setup_roots_iterator(xcb_get_setup(dri2_dpy->conn));
> -   screen = get_xcb_screen(s, dri2_dpy->screen);
> -   if (!screen) {
> -      _eglLog(_EGL_WARNING, "DRI2: failed to get xcb screen");
> -      return EGL_FALSE;
> -   }
> -   connect_cookie = xcb_dri2_connect_unchecked(dri2_dpy->conn, screen->root,
> +   connect_cookie = xcb_dri2_connect_unchecked(dri2_dpy->conn, dri2_dpy->screen->root,
>                                     XCB_DRI2_DRIVER_TYPE_DRI);
>  
>     xfixes_query =
> @@ -720,7 +693,6 @@ static EGLBoolean
>  dri2_x11_add_configs_for_visuals(struct dri2_egl_display *dri2_dpy,
>                                   _EGLDisplay *disp, bool supports_preserved)
>  {
> -   xcb_screen_iterator_t s;
>     xcb_depth_iterator_t d;
>     xcb_visualtype_t *visuals;
>     int i, j, count;
> @@ -732,8 +704,7 @@ dri2_x11_add_configs_for_visuals(struct dri2_egl_display *dri2_dpy,
>  	   EGL_NONE
>     };
>  
> -   s = xcb_setup_roots_iterator(xcb_get_setup(dri2_dpy->conn));
> -   d = xcb_screen_allowed_depths_iterator(get_xcb_screen(s, dri2_dpy->screen));
> +   d = xcb_screen_allowed_depths_iterator(dri2_dpy->screen);
>     count = 0;
>  
>     surface_type =
> @@ -1183,20 +1154,30 @@ static EGLBoolean
>  dri2_get_xcb_connection(_EGLDriver *drv, _EGLDisplay *disp,
>                          struct dri2_egl_display *dri2_dpy)
>  {
> +   xcb_screen_iterator_t s;
> +   int screen = 0;
> +
>     disp->DriverData = (void *) dri2_dpy;
>     if (disp->PlatformDisplay == NULL) {
> -      dri2_dpy->conn = xcb_connect(0, &dri2_dpy->screen);
> +      dri2_dpy->conn = xcb_connect(NULL, &screen);
>        dri2_dpy->own_device = true;
>     } else {
>        Display *dpy = disp->PlatformDisplay;
>  
>        dri2_dpy->conn = XGetXCBConnection(dpy);
> -      dri2_dpy->screen = DefaultScreen(dpy);
> +      screen = DefaultScreen(dpy);
>     }
>  
>     if (!dri2_dpy->conn || xcb_connection_has_error(dri2_dpy->conn))
>        return _eglError(EGL_BAD_ALLOC, "xcb_connect failed");
>  
> +   s = xcb_setup_roots_iterator(xcb_get_setup(dri2_dpy->conn));
> +   dri2_dpy->screen = get_xcb_screen(s, screen);
> +   if (!dri2_dpy->screen) {
> +      _eglError(EGL_BAD_ALLOC, "failed to get xcb screen");
> +      return EGL_FALSE;

Shouldn't we disconnect xcb here? We're past the point handling its
failure, so it should be connected now.

With that fixed, the series is
Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>

> +   }
> +
>     return EGL_TRUE;
>  }
>  
> diff --git a/src/egl/drivers/dri2/platform_x11_dri3.c b/src/egl/drivers/dri2/platform_x11_dri3.c
> index 475ec05..5542925 100644
> --- a/src/egl/drivers/dri2/platform_x11_dri3.c
> +++ b/src/egl/drivers/dri2/platform_x11_dri3.c
> @@ -160,16 +160,6 @@ dri3_set_swap_interval(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *surf,
>     return EGL_TRUE;
>  }
>  
> -static xcb_screen_t *
> -get_xcb_screen(xcb_screen_iterator_t iter, int screen)
> -{
> -    for (; iter.rem; --screen, xcb_screen_next(&iter))
> -        if (screen == 0)
> -            return iter.data;
> -
> -    return NULL;
> -}
> -
>  static _EGLSurface *
>  dri3_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type,
>                      _EGLConfig *conf, void *native_surface,
> @@ -180,8 +170,6 @@ dri3_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type,
>     struct dri3_egl_surface *dri3_surf;
>     const __DRIconfig *dri_config;
>     xcb_drawable_t drawable;
> -   xcb_screen_iterator_t s;
> -   xcb_screen_t *screen;
>  
>     STATIC_ASSERT(sizeof(uintptr_t) == sizeof(native_surface));
>     drawable = (uintptr_t) native_surface;
> @@ -198,16 +186,9 @@ dri3_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type,
>        goto cleanup_surf;
>  
>     if (type == EGL_PBUFFER_BIT) {
> -      s = xcb_setup_roots_iterator(xcb_get_setup(dri2_dpy->conn));
> -      screen = get_xcb_screen(s, dri2_dpy->screen);
> -      if (!screen) {
> -         _eglError(EGL_BAD_NATIVE_WINDOW, "dri3_create_surface");
> -         goto cleanup_surf;
> -      }
> -
>        drawable = xcb_generate_id(dri2_dpy->conn);
>        xcb_create_pixmap(dri2_dpy->conn, conf->BufferSize,
> -                        drawable, screen->root,
> +                        drawable, dri2_dpy->screen->root,
>                          dri3_surf->base.Width, dri3_surf->base.Height);
>     }
>  
> @@ -473,8 +454,6 @@ dri3_x11_connect(struct dri2_egl_display *dri2_dpy)
>     xcb_present_query_version_reply_t *present_query;
>     xcb_present_query_version_cookie_t present_query_cookie;
>     xcb_generic_error_t *error;
> -   xcb_screen_iterator_t s;
> -   xcb_screen_t *screen;
>     const xcb_query_extension_reply_t *extension;
>  
>     xcb_prefetch_extension_data (dri2_dpy->conn, &xcb_dri3_id);
> @@ -517,14 +496,7 @@ dri3_x11_connect(struct dri2_egl_display *dri2_dpy)
>     }
>     free(present_query);
>  
> -   s = xcb_setup_roots_iterator(xcb_get_setup(dri2_dpy->conn));
> -   screen = get_xcb_screen(s, dri2_dpy->screen);
> -   if (!screen) {
> -      _eglError(EGL_BAD_NATIVE_WINDOW, "dri3_x11_connect");
> -      return EGL_FALSE;
> -   }
> -
> -   dri2_dpy->fd = loader_dri3_open(dri2_dpy->conn, screen->root, 0);
> +   dri2_dpy->fd = loader_dri3_open(dri2_dpy->conn, dri2_dpy->screen->root, 0);
>     if (dri2_dpy->fd < 0) {
>        int conn_error = xcb_connection_has_error(dri2_dpy->conn);
>        _eglLog(_EGL_WARNING, "DRI3: Screen seems not DRI3 capable");
> -- 
> 2.10.2
> 


More information about the mesa-dev mailing list