| Message ID | cf298d66-cc10-0cf5-b5d1-b36c20fb05f7@redhat.com |
|---|---|
| State | New |
| Headers | show |
> > <resend with some more people added to the Cc> > > Hi, > > On 20-02-17 10:50, Frediano Ziglio wrote: > > According to > > https://www.khronos.org/registry/EGL/extensions/MESA/EGL_MESA_platform_gbm.txt > > if MESA_platform_gbm is supported display should be initialized > > from a GBM handle using eglGetPlatformDisplayEXT. > > > > Signed-off-by: Frediano Ziglio <fziglio@redhat.com> > > --- > > This should fix > > http://www.spinics.net/linux/fedora/libvir/msg142837.html > > > > Tested on Fedora rawhide. > > --- > > ui/egl-helpers.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c > > index cd24568..964c5a5 100644 > > --- a/ui/egl-helpers.c > > +++ b/ui/egl-helpers.c > > @@ -219,7 +219,11 @@ int qemu_egl_init_dpy(EGLNativeDisplayType dpy, bool > > gles, bool debug) > > } > > > > egl_dbg("eglGetDisplay (dpy %p) ...\n", dpy); > > +#ifdef EGL_MESA_platform_gbm > > + qemu_egl_display = eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_MESA, > > dpy, NULL); > > +#else > > qemu_egl_display = eglGetDisplay(dpy); > > +#endif > > if (qemu_egl_display == EGL_NO_DISPLAY) { > > error_report("egl: eglGetDisplay failed"); > > return -1; > > > > That fix is incomplete, you need some magic to work properly on older libGL > versions. > > Attached is a (compile tested only) proper patch. I do not have a qemu git > clone > handy atm, so this is not a git format-patch patch, it is against the Fedora > Rawhide > srpm. I've a test build with these patches here: > > https://fedorapeople.org/~jwrdegoede/qemu-glvnd/ > > I was planning on doing a git clone qemu and send a proper patch after I got > some > testing feedback. Feel free to use this as a base for a v2 of your patch. > > Regards, > > Hans > Wouldn't be easier to call the "old" eglGetDisplay if eglGetPlatformDisplayEXT returns EGL_NO_DISPLAY ? Kind of egl_dbg("eglGetDisplay (dpy %p) ...\n", dpy); - qemu_egl_display = eglGetDisplay(dpy); +#ifdef EGL_MESA_platform_gbm + qemu_egl_display = eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_MESA, dpy, NULL); +#else + qemu_egl_display = EGL_NO_DISPLAY; +#endif + if (qemu_egl_display == EGL_NO_DISPLAY) + qemu_egl_display = eglGetDisplay(dpy); if (qemu_egl_display == EGL_NO_DISPLAY) { error_report("egl: eglGetDisplay failed"); return -1; Your patch should not even compile on older system which does not define EGL_PLATFORM_GBM_MESA. I tested my patch and works on both rawhide and FC25 (with very recent Mesa version) Frediano
Hi, On 13-03-17 10:39, Frediano Ziglio wrote: >> >> <resend with some more people added to the Cc> >> >> Hi, >> >> On 20-02-17 10:50, Frediano Ziglio wrote: >>> According to >>> https://www.khronos.org/registry/EGL/extensions/MESA/EGL_MESA_platform_gbm.txt >>> if MESA_platform_gbm is supported display should be initialized >>> from a GBM handle using eglGetPlatformDisplayEXT. >>> >>> Signed-off-by: Frediano Ziglio <fziglio@redhat.com> >>> --- >>> This should fix >>> http://www.spinics.net/linux/fedora/libvir/msg142837.html >>> >>> Tested on Fedora rawhide. >>> --- >>> ui/egl-helpers.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c >>> index cd24568..964c5a5 100644 >>> --- a/ui/egl-helpers.c >>> +++ b/ui/egl-helpers.c >>> @@ -219,7 +219,11 @@ int qemu_egl_init_dpy(EGLNativeDisplayType dpy, bool >>> gles, bool debug) >>> } >>> >>> egl_dbg("eglGetDisplay (dpy %p) ...\n", dpy); >>> +#ifdef EGL_MESA_platform_gbm >>> + qemu_egl_display = eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_MESA, >>> dpy, NULL); >>> +#else >>> qemu_egl_display = eglGetDisplay(dpy); >>> +#endif >>> if (qemu_egl_display == EGL_NO_DISPLAY) { >>> error_report("egl: eglGetDisplay failed"); >>> return -1; >>> >> >> That fix is incomplete, you need some magic to work properly on older libGL >> versions. >> >> Attached is a (compile tested only) proper patch. I do not have a qemu git >> clone >> handy atm, so this is not a git format-patch patch, it is against the Fedora >> Rawhide >> srpm. I've a test build with these patches here: >> >> https://fedorapeople.org/~jwrdegoede/qemu-glvnd/ >> >> I was planning on doing a git clone qemu and send a proper patch after I got >> some >> testing feedback. Feel free to use this as a base for a v2 of your patch. >> >> Regards, >> >> Hans >> > > Wouldn't be easier to call the "old" eglGetDisplay if eglGetPlatformDisplayEXT > returns EGL_NO_DISPLAY ? Kind of With libEGL you (unfortunately) cannot assume that the lib your building against is the lib you will run against. > > egl_dbg("eglGetDisplay (dpy %p) ...\n", dpy); > - qemu_egl_display = eglGetDisplay(dpy); > +#ifdef EGL_MESA_platform_gbm > + qemu_egl_display = eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_MESA, dpy, NULL); So this may very well lead to a runtime unresolved symbol error. IOW you really need to do the special magic dance I'm afraid. > +#else > + qemu_egl_display = EGL_NO_DISPLAY; > +#endif > + if (qemu_egl_display == EGL_NO_DISPLAY) > + qemu_egl_display = eglGetDisplay(dpy); > if (qemu_egl_display == EGL_NO_DISPLAY) { > error_report("egl: eglGetDisplay failed"); > return -1; > > Your patch should not even compile on older system which does not define > EGL_PLATFORM_GBM_MESA. True, so I guess my patch needs an #ifdef around the new qemu_egl_get_display() function and then on the actual call site #ifdef #else #endif like your patch, but instead of directly calling eglGetPlatformDisplayEXT call qemu_egl_get_display. > I tested my patch and works on both rawhide and FC25 (with very recent Mesa version) mesa is not the problem, it is say running qemu on ARM and using some binary driver libEGL.so which is the problem, without the dance that will result in a runtime dynamic linker error and qemu aborting. Regards, Hans
diff -up qemu-2.8.0/ui/egl-helpers.c~ qemu-2.8.0/ui/egl-helpers.c --- qemu-2.8.0/ui/egl-helpers.c~ 2016-12-20 21:16:54.000000000 +0100 +++ qemu-2.8.0/ui/egl-helpers.c 2017-03-13 08:56:32.402845087 +0100 @@ -172,6 +172,50 @@ EGLSurface qemu_egl_init_surface_x11(EGL /* ---------------------------------------------------------------------- */ +/* + * Taken from glamor_egl.h from the Xorg xserver, which is MIT licensed + * + * Create an EGLDisplay from a native display type. This is a little quirky + * for a few reasons. + * + * 1: GetPlatformDisplayEXT and GetPlatformDisplay are the API you want to + * use, but have different function signatures in the third argument; this + * happens not to matter for us, at the moment, but it means epoxy won't alias + * them together. + * + * 2: epoxy 1.3 and earlier don't understand EGL client extensions, which + * means you can't call "eglGetPlatformDisplayEXT" directly, as the resolver + * will crash. + * + * 3: You can't tell whether you have EGL 1.5 at this point, because + * eglQueryString(EGL_VERSION) is a property of the display, which we don't + * have yet. So you have to query for extensions no matter what. Fortunately + * epoxy_has_egl_extension _does_ let you query for client extensions, so + * we don't have to write our own extension string parsing. + * + * 4. There is no EGL_KHR_platform_base to complement the EXT one, thus one + * needs to know EGL 1.5 is supported in order to use the eglGetPlatformDisplay + * function pointer. + * We can workaround this (circular dependency) by probing for the EGL 1.5 + * platform extensions (EGL_KHR_platform_gbm and friends) yet it doesn't seem + * like mesa will be able to adverise these (even though it can do EGL 1.5). + */ +static EGLDisplay qemu_egl_get_display(EGLint type, void *native) +{ + EGLDisplay dpy = NULL; + + /* In practise any EGL 1.5 implementation would support the EXT extension */ + if (epoxy_has_egl_extension(NULL, "EGL_EXT_platform_base")) { + PFNEGLGETPLATFORMDISPLAYEXTPROC getPlatformDisplayEXT = + (void *) eglGetProcAddress("eglGetPlatformDisplayEXT"); + if (getPlatformDisplayEXT) + return getPlatformDisplayEXT(type, native, NULL); + } + + /* Welp, everything is awful. */ + return eglGetDisplay(native); +} + int qemu_egl_init_dpy(EGLNativeDisplayType dpy, bool gles, bool debug) { static const EGLint conf_att_gl[] = { @@ -202,8 +246,8 @@ int qemu_egl_init_dpy(EGLNativeDisplayTy setenv("LIBGL_DEBUG", "verbose", true); } - egl_dbg("eglGetDisplay (dpy %p) ...\n", dpy); - qemu_egl_display = eglGetDisplay(dpy); + egl_dbg("qemu_egl_get_display (dpy %p) ...\n", dpy); + qemu_egl_display = qemu_egl_get_display(EGL_PLATFORM_GBM_MESA, dpy); if (qemu_egl_display == EGL_NO_DISPLAY) { error_report("egl: eglGetDisplay failed"); return -1;