[Mesa-dev] [PATCH] egl: Introduce alternative native display types

Jakob Bornecrantz wallbraker at gmail.com
Tue May 18 06:53:45 PDT 2010


2010/5/17 Kristian Høgsberg <krh at bitplanet.net>:
> The EGL native platform API is determined at compile time and resolves
> to Win32, X11 or Symbian at this point.   This means that if we want to
> support XCB or a native DRM implementation, they have to be their platforms
> and result in different libEGL.so's with identical ABI but different
> entrypoint semantics.  From a distro point of view, and really, any point
> of view, this is a mess, since the different libraries can't easily co-exist
> without fiddling with linker paths.  And if you get it wrong, an application
> requiring the X11 platform libEGL.so will happily link against any other
> libEGL.so and segfault when eglGetDisplay() doesn't do what it expects.
>
> We can get around this by overloading the X11 native entrypoints instead.
> The X Display struct has as its first member a pointer to XExtData.  The
> pointer will always have bits 0 and 1 set to 0, which we can use to
> distinguish a struct that's not an X display.  This lets us pass in a
> custom struct that can provide initialization data for other platforms
> in the same libEGL.so.
>
> The convention we're establishing is that the first field of such a struct
> must be a pointer, so as to overlap with the layout of the X display
> struct.  We can then enummerate the different display types using odd
> numbers cast to a pointer (ensuring bit 0 is set).  This patch introduces
> two new types of displays: EGL_DISPLAY_TYPE_DRM_MESA which lets us
> initialize EGL directly on a DRM file descriptor or device filename and
> EGL_DISPLAY_TYPE_XCB_MESA which lets us initialize EGL on X using an
> xcb_connection_t instead of a classic Xlib Display.

This sounds good to me, there are just some minor nitpicks that would
be nice to be resolved.

1. Could you write some docu about EGLDisplayTypeDRMMESA, like that
the display should return the FD after init in the struct. Also what
happens if (drm->device_name == null && drm->fd < 0) looking at the
code it would fail to load is this what we want?

2. A helper function _eglNativeDisplayIsX(EGLNativeDisplay) or
something like that currently the drivers aren't that future proof if
we add new displaytypes. But we can add that later.

3. See inlined comment. Can also wait.

Other then that you have my Reviewed-by.

Cheers Jakob.

> ---
>  include/EGL/eglplatform.h       |   17 ++++++++++++++
>  src/egl/drivers/dri2/egl_dri2.c |   45 ++++++++++++++++++++++++++++++--------
>  2 files changed, 52 insertions(+), 10 deletions(-)
>
> diff --git a/include/EGL/eglplatform.h b/include/EGL/eglplatform.h
> index c625088..f9d0eb3 100644
> --- a/include/EGL/eglplatform.h
> +++ b/include/EGL/eglplatform.h
> @@ -88,6 +88,23 @@ typedef Display *EGLNativeDisplayType;
>  typedef Pixmap   EGLNativePixmapType;
>  typedef Window   EGLNativeWindowType;
>
> +typedef struct EGLDisplayTypeMESA EGLDisplayTypeMESA;
> +
> +#define EGL_DISPLAY_TYPE_DRM_MESA ((EGLDisplayTypeMESA *) 1)
> +
> +typedef struct EGLDisplayTypeDRMMESA {
> +   const EGLDisplayTypeMESA *type;
> +   const char *device;
> +   int fd;
> +} EGLDisplayTypeDRMMESA;
> +
> +#define EGL_DISPLAY_TYPE_XCB_MESA ((EGLDisplayTypeMESA *) 3)
> +
> +typedef struct EGLDisplayTypeXCBMESA {
> +   const EGLDisplayTypeMESA *type;
> +   struct xcb_connection_t *conn;
> +} EGLDisplayTypeXCBMESA;
> +
>  #else
>  #error "Platform not recognized"
>  #endif
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index a7030f0..39c90b3 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -634,6 +634,10 @@ dri2_add_configs_for_visuals(struct dri2_egl_display *dri2_dpy,
>    return EGL_TRUE;
>  }
>
> +struct generic_display {
> +   void *pointer;
> +};
> +
>  /**
>  * Called via eglInitialize(), GLX_drv->API.Initialize().
>  */
> @@ -645,25 +649,42 @@ dri2_initialize(_EGLDriver *drv, _EGLDisplay *disp,
>    struct dri2_egl_display *dri2_dpy;
>    char path[PATH_MAX], *search_paths, *p, *next, *end;
>    unsigned int api_mask;
> +   struct generic_display *generic;
> +   EGLDisplayTypeDRMMESA *drm_display;
> +   EGLDisplayTypeXCBMESA *xcb_display;

Couldn't generic just be replaced with:
EGLDisplayTypeMESA **type = (EGLDisplayTypeMESA **)disp->NativeDisplay;
if (type &&  *type == EGL_DISPLAY_TYPE_DRM_MESA) {
    /* stuffzor */
}

>
>    dri2_dpy = malloc(sizeof *dri2_dpy);
>    if (!dri2_dpy)
>       return _eglError(EGL_BAD_ALLOC, "eglInitialize");
>
> +   memset(dri2_dpy, 0, sizeof *dri2_dpy);
>    disp->DriverData = (void *) dri2_dpy;
> +   generic = (struct generic_display *) disp->NativeDisplay;
> +
>    if (disp->NativeDisplay == NULL) {
>       dri2_dpy->conn = xcb_connect(0, 0);
>       if (!dri2_dpy->conn) {
>         _eglLog(_EGL_WARNING, "DRI2: xcb_connect failed");
>         goto cleanup_dpy;
>       }
> -   } else {
> +   } else if (generic->pointer == EGL_DISPLAY_TYPE_DRM_MESA) {
> +      drm_display = (EGLDisplayTypeDRMMESA *) disp->NativeDisplay;
> +      if (drm_display->device) {
> +        dri2_dpy->device_name = strdup(drm_display->device);
> +      } else {
> +        dri2_dpy->fd = drm_display->fd;
> +      }
> +      dri2_dpy->driver_name = strdup("i965");
> +   } else if (generic->pointer == EGL_DISPLAY_TYPE_XCB_MESA) {
> +      xcb_display = (EGLDisplayTypeXCBMESA *) disp->NativeDisplay;
> +      dri2_dpy->conn = xcb_display->conn;
> +   } else if (((long) generic->pointer & 1) == 0) {
>       dri2_dpy->conn = XGetXCBConnection(disp->NativeDisplay);
> +   } else {
> +      _eglLog(_EGL_WARNING,
> +             "DRI2: unknown display type: %d", (long) generic->pointer);
>    }
>
> -   if (dri2_dpy->conn == NULL)
> -      goto cleanup_conn;
> -
>    if (dri2_dpy->conn) {
>       if (!dri2_connect(dri2_dpy))
>         goto cleanup_conn;
> @@ -709,12 +730,13 @@ dri2_initialize(_EGLDriver *drv, _EGLDisplay *disp,
>    if (!dri2_bind_extensions(dri2_dpy, dri2_driver_extensions, extensions))
>       goto cleanup_driver;
>
> -   dri2_dpy->fd = open(dri2_dpy->device_name, O_RDWR);
> -   if (dri2_dpy->fd == -1) {
> -      _eglLog(_EGL_WARNING,
> -             "DRI2: could not open %s (%s)", dri2_dpy->device_name,
> -              strerror(errno));
> -      goto cleanup_driver;
> +   if (dri2_dpy->device_name != NULL) {
> +      dri2_dpy->fd = open(dri2_dpy->device_name, O_RDWR);
> +      if (dri2_dpy->fd == -1) {
> +        _eglLog(_EGL_WARNING, "DRI2: could not open %s (%s)",
> +                dri2_dpy->device_name, strerror(errno));
> +        goto cleanup_driver;
> +      }
>    }
>
>    if (dri2_dpy->conn) {
> @@ -787,6 +809,9 @@ dri2_initialize(_EGLDriver *drv, _EGLDisplay *disp,
>    *major = 1;
>    *minor = 4;
>
> +   if (generic->pointer == EGL_DISPLAY_TYPE_DRM_MESA)
> +      drm_display->fd = dri2_dpy->fd;
> +
>    return EGL_TRUE;
>
>  cleanup_configs:
> --
> 1.7.0.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>


More information about the mesa-dev mailing list