Re: [PATCH v1 3/7] ui/spice: Submit the gl_draw requests at 60 FPS for remote clients

[Marc-André Lureau]

Hi

On Sat, Jan 20, 2024 at 4:54 AM Vivek Kasireddy
<vivek.kasireddy@intel.com> wrote:
>
> In the specific case where the display layer (virtio-gpu) is using
> dmabuf, and if remote clients are enabled (-spice gl=on,port=xxxx),
> it makes sense to limit the maximum (streaming) rate to 60 FPS
> using the GUI timer. This matches the behavior of GTK UI where the
> display updates are submitted at 60 FPS (assuming the underlying
> mode is WxY@60).

One of the issues with this approach is that the DMABUF isn't owned by
the consumer. By delaying the usage of it, we risk having
damaged/invalid updates.

It would be great if we could have a mechanism for double/triple
buffering with virtio-gpu, as far as I know this is not possible yet.

I wonder if setting dpy_set_ui_info() with the fixed refresh_rate is
enough for the guest to have a fixed FPS. It will only work with gfx
hw that support ui_info() though.



>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Frediano Ziglio <freddy77@gmail.com>
> Cc: Dongwon Kim <dongwon.kim@intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>  ui/spice-display.c | 38 ++++++++++++++++++++++++++++----------
>  1 file changed, 28 insertions(+), 10 deletions(-)
>
> diff --git a/ui/spice-display.c b/ui/spice-display.c
> index 384b8508d4..90c04623ec 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -841,12 +841,31 @@ static void qemu_spice_gl_block_timer(void *opaque)
>      warn_report("spice: no gl-draw-done within one second");
>  }
>
> +static void spice_gl_draw(SimpleSpiceDisplay *ssd,
> +                           uint32_t x, uint32_t y, uint32_t w, uint32_t h)
> +{
> +    uint64_t cookie;
> +
> +    cookie = (uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_GL_DRAW_DONE, 0);
> +    spice_qxl_gl_draw_async(&ssd->qxl, x, y, w, h, cookie);
> +}
> +
>  static void spice_gl_refresh(DisplayChangeListener *dcl)
>  {
>      SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
> -    uint64_t cookie;
> +    QemuDmaBuf *dmabuf = ssd->guest_dmabuf;
>
> -    if (!ssd->ds || qemu_console_is_gl_blocked(ssd->dcl.con)) {
> +    if (!ssd->ds) {
> +        return;
> +    }
> +
> +    if (qemu_console_is_gl_blocked(ssd->dcl.con)) {
> +        if (remote_client && ssd->gl_updates && dmabuf) {
> +            spice_gl_draw(ssd, 0, 0, dmabuf->width, dmabuf->height);
> +            ssd->gl_updates = 0;
> +            /* To stream at 60 FPS, the (GUI) timer delay needs to be ~17 ms 
> */
> +            dcl->update_interval = 1000 / (2 * GUI_REFRESH_INTERVAL_DEFAULT) 
> + 1;
> +        }
>          return;
>      }
>
> @@ -854,11 +873,8 @@ static void spice_gl_refresh(DisplayChangeListener *dcl)
>      if (ssd->gl_updates && ssd->have_surface) {
>          qemu_spice_gl_block(ssd, true);
>          glFlush();
> -        cookie = (uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_GL_DRAW_DONE, 0);
> -        spice_qxl_gl_draw_async(&ssd->qxl, 0, 0,
> -                                surface_width(ssd->ds),
> -                                surface_height(ssd->ds),
> -                                cookie);
> +        spice_gl_draw(ssd, 0, 0,
> +                      surface_width(ssd->ds), surface_height(ssd->ds));
>          ssd->gl_updates = 0;
>      }
>  }
> @@ -1025,7 +1041,6 @@ static void qemu_spice_gl_update(DisplayChangeListener 
> *dcl,
>      EGLint stride = 0, fourcc = 0;
>      bool render_cursor = false;
>      bool y_0_top = false; /* FIXME */
> -    uint64_t cookie;
>      int fd;
>
>      if (!ssd->have_scanout) {
> @@ -1098,8 +1113,11 @@ static void qemu_spice_gl_update(DisplayChangeListener 
> *dcl,
>      trace_qemu_spice_gl_update(ssd->qxl.id, w, h, x, y);
>      qemu_spice_gl_block(ssd, true);
>      glFlush();
> -    cookie = (uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_GL_DRAW_DONE, 0);
> -    spice_qxl_gl_draw_async(&ssd->qxl, x, y, w, h, cookie);
> +    if (remote_client) {
> +        ssd->gl_updates++;
> +    } else {
> +        spice_gl_draw(ssd, x, y, w, h);
> +    }
>  }
>
>  static const DisplayChangeListenerOps display_listener_gl_ops = {
> --
> 2.39.2
>
>


--
Marc-André Lureau