Re: [Qemu-devel] [PATCH] egl-helpers: Support newer MESA versions

[Hans de Goede]

Hi,

On 13-03-17 10:39, Frediano Ziglio wrote:




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 
---
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

Re: [Qemu-devel] [PATCH] egl-helpers: Support newer MESA versions

[Frediano Ziglio]

> 
> 
> 
> 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 
> > ---
> > 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

Re: [Qemu-devel] [PATCH] egl-helpers: Support newer MESA versions

[Hans de Goede]

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 
---
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



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.0 +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;

Re: [Qemu-devel] [PATCH] egl-helpers: Support newer MESA versions

[Hans de Goede]

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 
---
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


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.0 +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;