From 376d4b22e4d7dd81cb0c1ea1dfe1db0a0dc4b0e2 Mon Sep 17 00:00:00 2001 From: Vivek Kasireddy Date: Mon, 16 Jun 2025 21:32:26 -0700 Subject: ui/spice: Enable gl=on option for non-local or remote clients MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Newer versions of Spice server should be able to accept dmabuf fds from Qemu for clients that are connected via the network. In other words, when this option is enabled, Qemu would share a dmabuf fd with Spice which would encode and send the data associated with the fd to a client that could be located on a different machine. Cc: Gerd Hoffmann Cc: Marc-André Lureau Cc: Dmitry Osipenko Cc: Frediano Ziglio Cc: Dongwon Kim Cc: Michael Scherle Reviewed-by: Marc-André Lureau Signed-off-by: Vivek Kasireddy Message-Id: <20250617043546.1022779-3-vivek.kasireddy@intel.com> --- ui/spice-display.c | 1 + 1 file changed, 1 insertion(+) (limited to 'ui/spice-display.c') diff --git a/ui/spice-display.c b/ui/spice-display.c index 9c39d2c5c8..0fb72f6d6f 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -31,6 +31,7 @@ #include "standard-headers/drm/drm_fourcc.h" bool spice_opengl; +bool spice_remote_client; int qemu_spice_rect_is_empty(const QXLRect* r) { -- cgit 1.4.1 From 50d135e3779f276eba93c63dff49a940b85e23a5 Mon Sep 17 00:00:00 2001 From: Vivek Kasireddy Date: Mon, 16 Jun 2025 21:32:28 -0700 Subject: ui/spice: Add an option to submit gl_draw requests at fixed rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 (refresh rate) to a fixed value using the GUI refresh timer. Otherwise, the updates or gl_draw requests would be sent as soon as the Guest submits a new frame which is not optimal as it would lead to increased network traffic and wastage of GPU cycles if the frames get dropped. Cc: Gerd Hoffmann Cc: Marc-André Lureau Cc: Dmitry Osipenko Cc: Frediano Ziglio Cc: Dongwon Kim Cc: Michael Scherle Reviewed-by: Marc-André Lureau Signed-off-by: Vivek Kasireddy Message-Id: <20250617043546.1022779-5-vivek.kasireddy@intel.com> --- include/ui/spice-display.h | 1 + qemu-options.hx | 5 ++++ ui/spice-core.c | 12 +++++++++ ui/spice-display.c | 62 ++++++++++++++++++++++++++++++++++++++-------- 4 files changed, 70 insertions(+), 10 deletions(-) (limited to 'ui/spice-display.c') diff --git a/include/ui/spice-display.h b/include/ui/spice-display.h index 6c55f38c8b..9bdde78266 100644 --- a/include/ui/spice-display.h +++ b/include/ui/spice-display.h @@ -152,6 +152,7 @@ struct SimpleSpiceCursor { extern bool spice_opengl; extern bool spice_remote_client; +extern int spice_max_refresh_rate; int qemu_spice_rect_is_empty(const QXLRect* r); void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r); diff --git a/qemu-options.hx b/qemu-options.hx index 8f6a228a89..ac09abfd71 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2282,6 +2282,7 @@ DEF("spice", HAS_ARG, QEMU_OPTION_spice, " [,disable-agent-file-xfer=on|off][,agent-mouse=[on|off]]\n" " [,playback-compression=[on|off]][,seamless-migration=[on|off]]\n" " [,video-codec=\n" + " [,max-refresh-rate=rate\n" " [,gl=[on|off]][,rendernode=]\n" " enable spice\n" " at least one of {port, tls-port} is mandatory\n", @@ -2377,6 +2378,10 @@ SRST would be used as default. And, for the case where gl=off, the default codec to be used is determined by the Spice server. + ``max-refresh-rate=rate`` + Provide the maximum refresh rate (or FPS) at which the encoding + requests should be sent to the Spice server. Default would be 30. + ``gl=[on|off]`` Enable/disable OpenGL context. Default is off. diff --git a/ui/spice-core.c b/ui/spice-core.c index 51bdcef1af..5992f9daec 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -56,6 +56,8 @@ struct SpiceTimer { QEMUTimer *timer; }; +#define DEFAULT_MAX_REFRESH_RATE 30 + static SpiceTimer *timer_add(SpiceTimerFunc func, void *opaque) { SpiceTimer *timer; @@ -491,6 +493,9 @@ static QemuOptsList qemu_spice_opts = { },{ .name = "video-codec", .type = QEMU_OPT_STRING, + },{ + .name = "max-refresh-rate", + .type = QEMU_OPT_NUMBER, },{ .name = "agent-mouse", .type = QEMU_OPT_BOOL, @@ -804,6 +809,13 @@ static void qemu_spice_init(void) spice_server_set_streaming_video(spice_server, SPICE_STREAM_VIDEO_OFF); } + spice_max_refresh_rate = qemu_opt_get_number(opts, "max-refresh-rate", + DEFAULT_MAX_REFRESH_RATE); + if (spice_max_refresh_rate <= 0) { + error_report("max refresh rate/fps is invalid"); + exit(1); + } + spice_server_set_agent_mouse (spice_server, qemu_opt_get_bool(opts, "agent-mouse", 1)); spice_server_set_playback_compression diff --git a/ui/spice-display.c b/ui/spice-display.c index 0fb72f6d6f..e409b6bdb2 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -32,6 +32,7 @@ bool spice_opengl; bool spice_remote_client; +int spice_max_refresh_rate; int qemu_spice_rect_is_empty(const QXLRect* r) { @@ -844,12 +845,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 (spice_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; + /* E.g, to achieve 60 FPS, update_interval needs to be ~16.66 ms */ + dcl->update_interval = 1000 / spice_max_refresh_rate; + } return; } @@ -857,11 +878,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; } } @@ -954,6 +972,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 (spice_remote_client && ssd->gl_updates && ssd->have_scanout) { + ssd->gl_updates = 0; + qemu_spice_gl_block(ssd, false); + } + } spice_server_gl_scanout(&ssd->qxl, NULL, 0, 0, NULL, NULL, 0, DRM_FORMAT_INVALID, DRM_FORMAT_MOD_INVALID, false); qemu_spice_gl_monitor_config(ssd, 0, 0, 0, 0); @@ -1061,7 +1093,6 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl, EGLint fourcc = 0; bool render_cursor = false; bool y_0_top = false; /* FIXME */ - uint64_t cookie; uint32_t width, height, texture; if (!ssd->have_scanout) { @@ -1159,8 +1190,19 @@ 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); + + /* + * In the case of remote clients, the submission of gl_draw request is + * deferred here, so that it can be submitted later (to spice server) + * from spice_gl_refresh() timer callback. This is done to ensure that + * Guest updates are submitted at a steady rate (e.g. 60 FPS) instead + * of submitting them arbitrarily. + */ + if (spice_remote_client) { + ssd->gl_updates++; + } else { + spice_gl_draw(ssd, x, y, w, h); + } } static const DisplayChangeListenerOps display_listener_gl_ops = { -- cgit 1.4.1 From 2103690b1a7d98f88f7c150f48fcd951d3ee8b36 Mon Sep 17 00:00:00 2001 From: Vivek Kasireddy Date: Mon, 16 Jun 2025 21:32:30 -0700 Subject: ui/spice: Create a new texture with linear layout when gl=on is specified MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since most encoders/decoders (invoked by Spice) may not work properly with tiled memory associated with a texture, we need to create another texture that has linear memory layout and use that instead. Note that, there does not seem to be a direct way to indicate to the GL implementation that a texture's backing memory needs to be linear. Instead, we have to do it in a roundabout way where we need to first create a tiled texture and import that as a memory object to create a new texture that has a linear memory layout. Cc: Gerd Hoffmann Cc: Marc-André Lureau Cc: Dmitry Osipenko Cc: Frediano Ziglio Cc: Dongwon Kim Reviewed-by: Marc-André Lureau Co-developed-by: Michael Scherle Signed-off-by: Vivek Kasireddy Reviewed-by: Dmitry Osipenko Message-Id: <20250617043546.1022779-7-vivek.kasireddy@intel.com> --- include/ui/surface.h | 1 + ui/console-gl.c | 6 ++++ ui/spice-display.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+) (limited to 'ui/spice-display.c') diff --git a/include/ui/surface.h b/include/ui/surface.h index f16f7be8be..006b1986bb 100644 --- a/include/ui/surface.h +++ b/include/ui/surface.h @@ -22,6 +22,7 @@ typedef struct DisplaySurface { GLenum glformat; GLenum gltype; GLuint texture; + GLuint mem_obj; #endif qemu_pixman_shareable share_handle; uint32_t share_handle_offset; diff --git a/ui/console-gl.c b/ui/console-gl.c index afb36dba64..403fc36fbd 100644 --- a/ui/console-gl.c +++ b/ui/console-gl.c @@ -184,6 +184,12 @@ void surface_gl_destroy_texture(QemuGLShader *gls, } glDeleteTextures(1, &surface->texture); surface->texture = 0; +#ifdef GL_EXT_memory_object_fd + if (surface->mem_obj) { + glDeleteMemoryObjectsEXT(1, &surface->mem_obj); + surface->mem_obj = 0; + } +#endif } void surface_gl_setup_viewport(QemuGLShader *gls, diff --git a/ui/spice-display.c b/ui/spice-display.c index e409b6bdb2..854a97c198 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -893,6 +893,81 @@ static void spice_gl_update(DisplayChangeListener *dcl, ssd->gl_updates++; } +static bool spice_gl_replace_fd_texture(SimpleSpiceDisplay *ssd, + int *fds, uint64_t *modifier, + int *num_planes) +{ + uint32_t offsets[DMABUF_MAX_PLANES], strides[DMABUF_MAX_PLANES]; + GLuint texture; + GLuint mem_obj; + int fourcc; + bool ret; + + if (!spice_remote_client) { + return true; + } + + if (*modifier == DRM_FORMAT_MOD_LINEAR) { + return true; + } + + if (*num_planes > 1) { + error_report("spice: cannot replace texture with multiple planes"); + return false; + } + + /* + * We really want to ensure that the memory layout of the texture + * is linear; otherwise, the encoder's output may show corruption. + */ + if (!surface_gl_create_texture_from_fd(ssd->ds, fds[0], &texture, + &mem_obj)) { + error_report("spice: cannot create new texture from fd"); + return false; + } + + /* + * A successful return after glImportMemoryFdEXT() means that + * the ownership of fd has been passed to GL. In other words, + * the fd we got above should not be used anymore. + */ + ret = egl_dmabuf_export_texture(texture, + fds, + (EGLint *)offsets, + (EGLint *)strides, + &fourcc, + num_planes, + modifier); + if (!ret) { + glDeleteTextures(1, &texture); +#ifdef GL_EXT_memory_object_fd + glDeleteMemoryObjectsEXT(1, &mem_obj); +#endif + + /* + * Since we couldn't export our newly create texture (or create, + * an fd associated with it) we need to backtrack and try to + * recreate the fd associated with the original texture. + */ + ret = egl_dmabuf_export_texture(ssd->ds->texture, + fds, + (EGLint *)offsets, + (EGLint *)strides, + &fourcc, + num_planes, + modifier); + if (!ret) { + surface_gl_destroy_texture(ssd->gls, ssd->ds); + warn_report("spice: no texture available to display"); + } + } else { + surface_gl_destroy_texture(ssd->gls, ssd->ds); + ssd->ds->texture = texture; + ssd->ds->mem_obj = mem_obj; + } + return ret; +} + static void spice_server_gl_scanout(QXLInstance *qxl, const int *fd, uint32_t width, uint32_t height, @@ -917,6 +992,7 @@ static void spice_gl_switch(DisplayChangeListener *dcl, struct DisplaySurface *new_surface) { SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl); + bool ret; if (ssd->ds) { surface_gl_destroy_texture(ssd->gls, ssd->ds); @@ -939,6 +1015,12 @@ static void spice_gl_switch(DisplayChangeListener *dcl, return; } + ret = spice_gl_replace_fd_texture(ssd, fd, &modifier, &num_planes); + if (!ret) { + surface_gl_destroy_texture(ssd->gls, ssd->ds); + return; + } + trace_qemu_spice_gl_surface(ssd->qxl.id, surface_width(ssd->ds), surface_height(ssd->ds), -- cgit 1.4.1 From f851cd65ebe24cc716a70a2fa68c149e5440f2f4 Mon Sep 17 00:00:00 2001 From: Vivek Kasireddy Date: Mon, 16 Jun 2025 21:32:31 -0700 Subject: ui/spice: Blit the scanout texture if its memory layout is not linear MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In cases where the scanout buffer is provided as a texture (e.g. Virgl) we need to check to see if it has a linear memory layout or not. If it doesn't have a linear layout, then blitting it onto the texture associated with the display surface (which already has a linear layout) seems to ensure that there is no corruption seen regardless of which encoder or decoder is used. Cc: Gerd Hoffmann Cc: Marc-André Lureau Cc: Dmitry Osipenko Cc: Frediano Ziglio Cc: Dongwon Kim Cc: Michael Scherle Reviewed-by: Marc-André Lureau Signed-off-by: Vivek Kasireddy Message-Id: <20250617043546.1022779-8-vivek.kasireddy@intel.com> --- include/ui/spice-display.h | 3 ++ ui/spice-display.c | 81 ++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 78 insertions(+), 6 deletions(-) (limited to 'ui/spice-display.c') diff --git a/include/ui/spice-display.h b/include/ui/spice-display.h index 9bdde78266..690ece7380 100644 --- a/include/ui/spice-display.h +++ b/include/ui/spice-display.h @@ -132,6 +132,9 @@ struct SimpleSpiceDisplay { egl_fb guest_fb; egl_fb blit_fb; egl_fb cursor_fb; + bool backing_y_0_top; + bool blit_scanout_texture; + bool new_scanout_texture; bool have_hot; #endif }; diff --git a/ui/spice-display.c b/ui/spice-display.c index 854a97c198..9ce622cefc 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -1086,7 +1086,7 @@ static void qemu_spice_gl_scanout_texture(DisplayChangeListener *dcl, { SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl); EGLint offset[DMABUF_MAX_PLANES], stride[DMABUF_MAX_PLANES], fourcc = 0; - int fd[DMABUF_MAX_PLANES], num_planes; + int fd[DMABUF_MAX_PLANES], num_planes, i; uint64_t modifier; assert(tex_id); @@ -1098,11 +1098,26 @@ static void qemu_spice_gl_scanout_texture(DisplayChangeListener *dcl, trace_qemu_spice_gl_scanout_texture(ssd->qxl.id, w, h, fourcc); - /* note: spice server will close the fd */ - spice_server_gl_scanout(&ssd->qxl, fd, backing_width, backing_height, - (uint32_t *)offset, (uint32_t *)stride, num_planes, - fourcc, modifier, y_0_top); - qemu_spice_gl_monitor_config(ssd, x, y, w, h); + if (spice_remote_client && modifier != DRM_FORMAT_MOD_LINEAR) { + egl_fb_destroy(&ssd->guest_fb); + egl_fb_setup_for_tex(&ssd->guest_fb, + backing_width, backing_height, + tex_id, false); + ssd->backing_y_0_top = y_0_top; + ssd->blit_scanout_texture = true; + ssd->new_scanout_texture = true; + + for (i = 0; i < num_planes; i++) { + close(fd[i]); + } + } else { + /* note: spice server will close the fd */ + spice_server_gl_scanout(&ssd->qxl, fd, backing_width, backing_height, + (uint32_t *)offset, (uint32_t *)stride, + num_planes, fourcc, modifier, y_0_top); + qemu_spice_gl_monitor_config(ssd, x, y, w, h); + } + ssd->have_surface = false; ssd->have_scanout = true; } @@ -1168,6 +1183,50 @@ static void qemu_spice_gl_release_dmabuf(DisplayChangeListener *dcl, egl_dmabuf_release_texture(dmabuf); } +static bool spice_gl_blit_scanout_texture(SimpleSpiceDisplay *ssd, + egl_fb *scanout_tex_fb) +{ + uint32_t offsets[DMABUF_MAX_PLANES], strides[DMABUF_MAX_PLANES]; + int fds[DMABUF_MAX_PLANES], num_planes, fourcc; + uint64_t modifier; + bool ret; + + egl_fb_destroy(scanout_tex_fb); + egl_fb_setup_for_tex(scanout_tex_fb, + surface_width(ssd->ds), surface_height(ssd->ds), + ssd->ds->texture, false); + egl_fb_blit(scanout_tex_fb, &ssd->guest_fb, false); + glFlush(); + + if (!ssd->new_scanout_texture) { + return true; + } + + ret = egl_dmabuf_export_texture(ssd->ds->texture, + fds, + (EGLint *)offsets, + (EGLint *)strides, + &fourcc, + &num_planes, + &modifier); + if (!ret) { + error_report("spice: failed to get fd for texture"); + return false; + } + + spice_server_gl_scanout(&ssd->qxl, fds, + surface_width(ssd->ds), + surface_height(ssd->ds), + (uint32_t *)offsets, (uint32_t *)strides, + num_planes, fourcc, modifier, + ssd->backing_y_0_top); + qemu_spice_gl_monitor_config(ssd, 0, 0, + surface_width(ssd->ds), + surface_height(ssd->ds)); + ssd->new_scanout_texture = false; + return true; +} + static void qemu_spice_gl_update(DisplayChangeListener *dcl, uint32_t x, uint32_t y, uint32_t w, uint32_t h) { @@ -1175,6 +1234,7 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl, EGLint fourcc = 0; bool render_cursor = false; bool y_0_top = false; /* FIXME */ + bool ret; uint32_t width, height, texture; if (!ssd->have_scanout) { @@ -1269,6 +1329,15 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl, glFlush(); } + if (spice_remote_client && ssd->blit_scanout_texture) { + egl_fb scanout_tex_fb; + + ret = spice_gl_blit_scanout_texture(ssd, &scanout_tex_fb); + if (!ret) { + return; + } + } + trace_qemu_spice_gl_update(ssd->qxl.id, w, h, x, y); qemu_spice_gl_block(ssd, true); glFlush(); -- cgit 1.4.1