egl-helpers: Support newer MESA versions

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

Posted by Frediano Ziglio 7 years, 2 months ago

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

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

Posted by Hans de Goede 7 years, 1 month ago

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

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

Posted by Hans de Goede 7 years, 1 month ago

<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

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

Posted by Frediano Ziglio 7 years, 1 month ago

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

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

Posted by Hans de Goede 7 years, 1 month ago

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