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

[Hans de Goede]

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 <address@hidden>
> > > ---
> > > 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