Re: [PATCH v22 06/18] ui/sdl2: Implement dpy dmabuf functions

["Alex Bennée" <alex.bennee@linaro.org>]

Dmitry Osipenko <dmitry.osipenko@collabora.com> writes:

> On 3/4/26 11:49, Alex Bennée wrote:
>> Dmitry Osipenko <dmitry.osipenko@collabora.com> writes:
>> 
>>> From: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
>>>
>>> If EGL is used, we can rely on dmabuf to import textures without
>>> doing copies.
>>>
>>> To get this working on X11, we use the existing SDL hint:
>>> SDL_HINT_VIDEO_X11_FORCE_EGL (because dmabuf can't be used with GLX).
>>>
>>> Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>>> Tested-by: Alex Bennée <alex.bennee@linaro.org>
>>> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
>>> Reviewed-by: Yiwei Zhang <zzyiwei@gmail.com>
>>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>>> ---
>>>  include/ui/sdl2.h |  7 +++++
>>>  meson.build       |  6 ++---
>>>  ui/sdl2-gl.c      | 66 +++++++++++++++++++++++++++++++++++++++++++++++
>>>  ui/sdl2.c         | 42 ++++++++++++++++++++++++++++++
>>>  4 files changed, 117 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
>>> index dbe6e3d9739b..9daf5ecffae7 100644
>>> --- a/include/ui/sdl2.h
>>> +++ b/include/ui/sdl2.h
>>> @@ -45,6 +45,7 @@ struct sdl2_console {
>>>      bool gui_keysym;
>>>      SDL_GLContext winctx;
>>>      QKbdState *kbd;
>>> +    bool has_dmabuf;
>>>  #ifdef CONFIG_OPENGL
>>>      QemuGLShader *gls;
>>>      egl_fb guest_fb;
>>> @@ -96,5 +97,11 @@ void sdl2_gl_scanout_texture(DisplayChangeListener *dcl,
>>>                               void *d3d_tex2d);
>>>  void sdl2_gl_scanout_flush(DisplayChangeListener *dcl,
>>>                             uint32_t x, uint32_t y, uint32_t w, uint32_t h);
>>> +void sdl2_gl_scanout_dmabuf(DisplayChangeListener *dcl,
>>> +                            QemuDmaBuf *dmabuf);
>>> +void sdl2_gl_release_dmabuf(DisplayChangeListener *dcl,
>>> +                            QemuDmaBuf *dmabuf);
>>> +bool sdl2_gl_has_dmabuf(DisplayChangeListener *dcl);
>>> +void sdl2_gl_console_init(struct sdl2_console *scon);
>>>  
>>>  #endif /* SDL2_H */
>>> diff --git a/meson.build b/meson.build
>>> index 3cd1d8dbc669..0ce84d45e6f3 100644
>>> --- a/meson.build
>>> +++ b/meson.build
>>> @@ -1937,10 +1937,8 @@ if get_option('gtk') \
>>>    endif
>>>  endif
>>>  
>>> -x11 = not_found
>>> -if gtkx11.found()
>>> -  x11 = dependency('x11', method: 'pkg-config', required: gtkx11.found())
>>> -endif
>>> +x11 = dependency('x11', method: 'pkg-config', required: gtkx11.found())
>>> +
>>>  png = not_found
>>>  if get_option('png').allowed() and have_system
>>>     png = dependency('libpng', version: '>=1.6.34', required: get_option('png'),
>>> diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
>>> index fbac3edbc09d..697ba2169c34 100644
>>> --- a/ui/sdl2-gl.c
>>> +++ b/ui/sdl2-gl.c
>>> @@ -26,6 +26,8 @@
>>>   */
>>>  
>>>  #include "qemu/osdep.h"
>>> +#include "qemu/main-loop.h"
>>> +#include "qemu/error-report.h"
>>>  #include "ui/console.h"
>>>  #include "ui/input.h"
>>>  #include "ui/sdl2.h"
>>> @@ -250,3 +252,67 @@ void sdl2_gl_scanout_flush(DisplayChangeListener *dcl,
>>>  
>>>      SDL_GL_SwapWindow(scon->real_window);
>>>  }
>>> +
>>> +void sdl2_gl_scanout_dmabuf(DisplayChangeListener *dcl,
>>> +                            QemuDmaBuf *dmabuf)
>>> +{
>>> +    struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
>>> +    const int *fds;
>>> +
>>> +    assert(scon->opengl);
>>> +    SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
>>> +
>>> +    egl_dmabuf_import_texture(dmabuf);
>>> +    if (!qemu_dmabuf_get_texture(dmabuf)) {
>>> +        fds = qemu_dmabuf_get_fds(dmabuf, NULL);
>>> +        error_report("%s: failed fd=%d", __func__, fds ? fds[0] : -1);
>>> +        return;
>>> +    }
>>> +
>>> +    sdl2_gl_scanout_texture(dcl, qemu_dmabuf_get_texture(dmabuf), false,
>>> +                            qemu_dmabuf_get_width(dmabuf),
>>> +                            qemu_dmabuf_get_height(dmabuf),
>>> +                            0, 0,
>>> +                            qemu_dmabuf_get_width(dmabuf),
>>> +                            qemu_dmabuf_get_height(dmabuf),
>>> +                            NULL);
>>> +
>>> +    if (qemu_dmabuf_get_allow_fences(dmabuf)) {
>>> +        scon->guest_fb.dmabuf = dmabuf;
>>> +    }
>>> +}
>>> +
>>> +void sdl2_gl_release_dmabuf(DisplayChangeListener *dcl,
>>> +                            QemuDmaBuf *dmabuf)
>>> +{
>>> +    egl_dmabuf_release_texture(dmabuf);
>>> +}
>>> +
>>> +bool sdl2_gl_has_dmabuf(DisplayChangeListener *dcl)
>>> +{
>>> +    struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
>>> +
>>> +    return scon->has_dmabuf;
>>> +}
>>> +
>>> +void sdl2_gl_console_init(struct sdl2_console *scon)
>>> +{
>>> +    bool hidden = scon->hidden;
>>> +
>>> +    scon->hidden = true;
>>> +    scon->surface = qemu_create_displaysurface(1, 1);
>>> +    sdl2_window_create(scon);
>>> +
>>> +    /*
>>> +     * QEMU checks whether console supports dma-buf before switching
>>> +     * to the console.  To break this chicken-egg problem we pre-check
>>> +     * dma-buf availability beforehand using a dummy SDL window.
>>> +     */
>>> +    scon->has_dmabuf = qemu_egl_has_dmabuf();
>>> +
>>> +    sdl2_window_destroy(scon);
>>> +    qemu_free_displaysurface(scon->surface);
>>> +
>>> +    scon->surface = NULL;
>>> +    scon->hidden = hidden;
>>> +}
>>> diff --git a/ui/sdl2.c b/ui/sdl2.c
>>> index 032dc14bc398..3bb2676f847b 100644
>>> --- a/ui/sdl2.c
>>> +++ b/ui/sdl2.c
>>> @@ -35,6 +35,10 @@
>>>  #include "qemu/log.h"
>>>  #include "qemu-main.h"
>>>  
>>> +#ifdef CONFIG_X11
>>> +#include <X11/Xlib.h>
>>> +#endif
>>> +
>>>  static int sdl2_num_outputs;
>>>  static struct sdl2_console *sdl2_console;
>>>  
>>> @@ -120,6 +124,9 @@ void sdl2_window_create(struct sdl2_console *scon)
>>>          /* The SDL renderer is only used by sdl2-2D, when OpenGL is disabled */
>>>          scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
>>>      }
>>> +
>>> +    qemu_egl_display = eglGetCurrentDisplay();
>>> +
>> 
>> This fails on MacOS for some reason:
>> 
>>   o libsystem.a.p/ui_sdl2.c.o -c ../ui/sdl2.c
>>   ../ui/sdl2.c:128:5: error: use of undeclared identifier 'qemu_egl_display'
>>     128 |     qemu_egl_display = eglGetCurrentDisplay();
>>         |     ^
>>   ../ui/sdl2.c:128:24: error: call to undeclared function
>> 'eglGetCurrentDisplay'; ISO C99 and later do not support implicit
>> function declarations [-Wimplicit-function-declaration]
>>     128 |     qemu_egl_display = eglGetCurrentDisplay();
>>         |                        ^
>>   2 errors generated.
>> 
>> See: https://gitlab.com/stsquad/qemu/-/jobs/13340079105
>> 
>> Not sure what the include differences are because --enable-sdl builds
>> fine on Linux.
>
> Please try with this change and let me know if you could squash it or I
> need to send a patch:
>
> diff --git a/ui/sdl2.c b/ui/sdl2.c
> index 3bb2676f847b..2603b0c2bdd8 100644
> --- a/ui/sdl2.c
> +++ b/ui/sdl2.c
> @@ -125,7 +125,9 @@ void sdl2_window_create(struct sdl2_console *scon)
>          scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
>      }
>
> +#ifdef CONFIG_OPENGL
>      qemu_egl_display = eglGetCurrentDisplay();
> +#endif
>
>      sdl_update_caption(scon);
>  }

There is one remaining build failure for win64:

  ../ui/sdl2-gl.c: In function 'sdl2_gl_scanout_dmabuf':
  ../ui/sdl2-gl.c:265:5: error: implicit declaration of function 'egl_dmabuf_import_texture'; did you mean 'qemu_dmabuf_set_texture'? [-Wimplicit-function-declaration]
    265 |     egl_dmabuf_import_texture(dmabuf);
        |     ^~~~~~~~~~~~~~~~~~~~~~~~~
        |     qemu_dmabuf_set_texture
  ../ui/sdl2-gl.c:265:5: error: nested extern declaration of 'egl_dmabuf_import_texture' [-Werror=nested-externs]
  ../ui/sdl2-gl.c: In function 'sdl2_gl_release_dmabuf':
  ../ui/sdl2-gl.c:288:5: error: implicit declaration of function 'egl_dmabuf_release_texture'; did you mean 'qemu_dmabuf_set_texture'? [-Wimplicit-function-declaration]
    288 |     egl_dmabuf_release_texture(dmabuf);
        |     ^~~~~~~~~~~~~~~~~~~~~~~~~~
        |     qemu_dmabuf_set_texture
  ../ui/sdl2-gl.c:288:5: error: nested extern declaration of 'egl_dmabuf_release_texture' [-Werror=nested-externs]
  cc1: all warnings being treated as errors

See: https://gitlab.com/stsquad/qemu/-/jobs/13340078806

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro