[Mesa-dev] [PATCH] egl: Introduce alternative native display types
Kristian Høgsberg
krh at bitplanet.net
Tue May 18 10:41:40 PDT 2010
On Tue, May 18, 2010 at 12:41 PM, Chia-I Wu <olvaffe at gmail.com> wrote:
> 2010/5/18 Jakob Bornecrantz <wallbraker at gmail.com>:
>> 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?
> I think the EGL prefix should be dropped. We are defining a new
> platform here: a platform whose display can be an xlib display, xcb
> connection or drm. It should not have the "EGL" prefix.
That is a good point - so we'll call it MesaDisplayTypeDRM and
MESA_DISPLAY_TYPE_DRM and similar for XCB.
> Actually, I think this platform should have its own header file.
> eglplatform.h will include the header file and typedef the native
> types defined there to EGL types.
You mean that the native types for the other platforms are defined in
their respective header files and so should these new types? I can
see that point of view, but from a more pragmatic point of view, I
don't think it's worth the trouble. Also, the types are specific to
the mesa implementation, they are not data types from another display
system or platform, and the Khronos implementors guide suggests that
the vendor/implementor can modify eglplatform.h.
Kristian
>> 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
>>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
>
>
> --
> olv at LunarG.com
>
More information about the mesa-dev
mailing list