Hi Vivek
On Tue, Apr 29, 2025 at 10:19 AM Vivek Kasireddy
<vivek.kasire...@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).
I guess it would make sense to make it configurable, for any UI/remote protocol.
For some UI, refresh rate is set via dpy_set_ui_info(). Unfortunately,
none of the vnc, spice or dbus protocols provide the refresh rate.
I wonder if it would make sense to set it on the GPU.. Perhaps a
"max-refresh-rate" device property?
>
> Cc: Gerd Hoffmann <kra...@redhat.com>
> Cc: Marc-André Lureau <marcandre.lur...@redhat.com>
> Cc: Dmitry Osipenko <dmitry.osipe...@collabora.com>
> Cc: Frediano Ziglio <fredd...@gmail.com>
> Cc: Dongwon Kim <dongwon....@intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasire...@intel.com>
> ---
> ui/spice-display.c | 53 +++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 43 insertions(+), 10 deletions(-)
>
> diff --git a/ui/spice-display.c b/ui/spice-display.c
> index bf4caf0d1b..2c4daa0707 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -842,12 +842,32 @@ 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;
>
> - 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 && ssd->have_scanout) {
> + glFlush();
> + spice_gl_draw(ssd, 0, 0,
> + surface_width(ssd->ds), surface_height(ssd->ds));
> + 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;
That expression doesn't make much sense to me.
"update_interval" is in ms. GUI_REFRESH_INTERVAL_DEFAULT is 30ms.
(iow, it's not 30fps)
If you need 60fps, just add a new constant/macro value instead?
> + }
> return;
> }
>
> @@ -855,11 +875,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;
> }
> }
> @@ -926,6 +943,20 @@ static void
> qemu_spice_gl_scanout_disable(DisplayChangeListener *dcl)
> SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
>
> trace_qemu_spice_gl_scanout_disable(ssd->qxl.id);
> +
> + /*
> + * We need to check for the case of "lost" updates, where a gl_draw
> + * was not submitted because the timer did not get a chance to run.
> + * One case where this happens is when the Guest VM is getting
> + * rebooted. If the console is blocked in this situation, we need
> + * to unblock it. Otherwise, newer updates would not take effect.
> + */
> + if (qemu_console_is_gl_blocked(ssd->dcl.con)) {
> + if (remote_client && ssd->gl_updates && ssd->have_scanout) {
> + ssd->gl_updates = 0;
> + qemu_spice_gl_block(ssd, false);
> + }
> + }
> spice_qxl_gl_scanout(&ssd->qxl, -1, 0, 0, 0, 0, false);
> qemu_spice_gl_monitor_config(ssd, 0, 0, 0, 0);
> ssd->have_surface = false;
> @@ -1029,7 +1060,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;
> uint32_t width, height, texture;
>
> @@ -1107,8 +1137,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.49.0
>
>
--
Marc-André Lureau
