2010/5/18 Jakob Bornecrantz <wallbra...@gmail.com>:
> 2010/5/17 Kristian Høgsberg <k...@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.
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.
> 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@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
--
o...@lunarg.com
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
