From: Qiang Yu <yuq825@gmail.com>
Signed-off-by: Qiang Yu <yuq825@gmail.com>
---
meson.build | 2 +-
ui/spice-display.c | 65 +++++++++++++++++++++++-----------------------
2 files changed, 34 insertions(+), 33 deletions(-)
diff --git a/meson.build b/meson.build
index 9d9c11731f..b87704a83b 100644
--- a/meson.build
+++ b/meson.build
@@ -1329,7 +1329,7 @@ if get_option('spice') \
.require(pixman.found(),
error_message: 'cannot enable SPICE if pixman is not available') \
.allowed()
- spice = dependency('spice-server', version: '>=0.14.0',
+ spice = dependency('spice-server', version: '>=0.14.3',
required: get_option('spice'),
method: 'pkg-config')
endif
diff --git a/ui/spice-display.c b/ui/spice-display.c
index b7016ece6a..46b6d5dfc9 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -28,6 +28,8 @@
#include "ui/spice-display.h"
+#include "standard-headers/drm/drm_fourcc.h"
+
bool spice_opengl;
int qemu_spice_rect_is_empty(const QXLRect* r)
@@ -884,16 +886,11 @@ static void spice_gl_switch(DisplayChangeListener *dcl,
if (ssd->ds) {
uint32_t offset[DMABUF_MAX_PLANES], stride[DMABUF_MAX_PLANES];
int fd[DMABUF_MAX_PLANES], num_planes, fourcc;
+ uint64_t modifier;
surface_gl_create_texture(ssd->gls, ssd->ds);
if (!egl_dmabuf_export_texture(ssd->ds->texture, fd, (EGLint *)offset,
- (EGLint *)stride, &fourcc, &num_planes, NULL)) {
- surface_gl_destroy_texture(ssd->gls, ssd->ds);
- return;
- }
-
- if (num_planes > 1) {
- fprintf(stderr, "%s: does not support multi-plane texture\n", __func__);
+ (EGLint *)stride, &fourcc, &num_planes, &modifier)) {
surface_gl_destroy_texture(ssd->gls, ssd->ds);
return;
}
@@ -904,10 +901,11 @@ static void spice_gl_switch(DisplayChangeListener *dcl,
fourcc);
/* note: spice server will close the fd */
- spice_qxl_gl_scanout(&ssd->qxl, fd[0],
- surface_width(ssd->ds),
- surface_height(ssd->ds),
- stride[0], fourcc, false);
+ spice_qxl_gl_scanout2(&ssd->qxl, fd,
+ surface_width(ssd->ds),
+ surface_height(ssd->ds),
+ offset, stride, num_planes,
+ fourcc, modifier, false);
ssd->have_surface = true;
ssd->have_scanout = false;
@@ -930,7 +928,8 @@ 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);
- spice_qxl_gl_scanout(&ssd->qxl, -1, 0, 0, 0, 0, false);
+ spice_qxl_gl_scanout2(&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);
ssd->have_surface = false;
ssd->have_scanout = false;
@@ -948,22 +947,21 @@ 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;
+ uint64_t modifier;
assert(tex_id);
if (!egl_dmabuf_export_texture(tex_id, fd, offset, stride, &fourcc,
- &num_planes, NULL)) {
+ &num_planes, &modifier)) {
fprintf(stderr, "%s: failed to export dmabuf for texture\n", __func__);
return;
}
- if (num_planes > 1) {
- fprintf(stderr, "%s: does not support multi-plane dmabuf\n", __func__);
- return;
- }
+
trace_qemu_spice_gl_scanout_texture(ssd->qxl.id, w, h, fourcc);
/* note: spice server will close the fd */
- spice_qxl_gl_scanout(&ssd->qxl, fd[0], backing_width, backing_height,
- stride[0], fourcc, y_0_top);
+ spice_qxl_gl_scanout2(&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;
@@ -1034,11 +1032,10 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
uint32_t x, uint32_t y, uint32_t w, uint32_t h)
{
SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
- EGLint stride = 0, fourcc = 0;
+ EGLint fourcc = 0;
bool render_cursor = false;
bool y_0_top = false; /* FIXME */
uint64_t cookie;
- int fd;
uint32_t width, height, texture;
if (!ssd->have_scanout) {
@@ -1075,6 +1072,7 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
ssd->blit_fb.height != height) {
int fds[DMABUF_MAX_PLANES], num_planes;
uint32_t offsets[DMABUF_MAX_PLANES], strides[DMABUF_MAX_PLANES];
+ uint64_t modifier;
trace_qemu_spice_gl_render_dmabuf(ssd->qxl.id, width,
height);
@@ -1083,27 +1081,30 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
width, height);
if (!egl_dmabuf_export_texture(ssd->blit_fb.texture, fds,
(EGLint *)offsets, (EGLint *)strides,
- &fourcc, &num_planes, NULL)) {
+ &fourcc, &num_planes, &modifier)) {
fprintf(stderr, "%s: failed to export dmabuf for texture\n", __func__);
return;
}
- if (num_planes > 1) {
- fprintf(stderr, "%s: does not support multi-plane dmabuf\n", __func__);
- return;
- }
- spice_qxl_gl_scanout(&ssd->qxl, fds[0], width, height,
- strides[0], fourcc, false);
+
+ spice_qxl_gl_scanout2(&ssd->qxl, fds, width, height, offsets, strides,
+ num_planes, fourcc, modifier, false);
}
} else {
- stride = qemu_dmabuf_get_stride(dmabuf)[0];
+ int fds[DMABUF_MAX_PLANES];
+
fourcc = qemu_dmabuf_get_fourcc(dmabuf);
y_0_top = qemu_dmabuf_get_y0_top(dmabuf);
- qemu_dmabuf_dup_fd(dmabuf, &fd);
+ qemu_dmabuf_dup_fd(dmabuf, fds);
trace_qemu_spice_gl_forward_dmabuf(ssd->qxl.id, width, height);
/* note: spice server will close the fd, so hand over a dup */
- spice_qxl_gl_scanout(&ssd->qxl, fd, width, height,
- stride, fourcc, y_0_top);
+ spice_qxl_gl_scanout2(&ssd->qxl, fds, width, height,
+ qemu_dmabuf_get_offset(dmabuf),
+ qemu_dmabuf_get_stride(dmabuf),
+ qemu_dmabuf_get_num_planes(dmabuf),
+ fourcc,
+ qemu_dmabuf_get_modifier(dmabuf),
+ y_0_top);
}
qemu_spice_gl_monitor_config(ssd, 0, 0, width, height);
ssd->guest_dmabuf_refresh = false;
--
2.43.0Hi
On Mon, Mar 24, 2025 at 12:20 PM <yuq825@gmail.com> wrote:
>
> From: Qiang Yu <yuq825@gmail.com>
>
> Signed-off-by: Qiang Yu <yuq825@gmail.com>
> ---
> meson.build | 2 +-
> ui/spice-display.c | 65 +++++++++++++++++++++++-----------------------
> 2 files changed, 34 insertions(+), 33 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index 9d9c11731f..b87704a83b 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1329,7 +1329,7 @@ if get_option('spice') \
> .require(pixman.found(),
> error_message: 'cannot enable SPICE if pixman is not available') \
> .allowed()
> - spice = dependency('spice-server', version: '>=0.14.0',
> + spice = dependency('spice-server', version: '>=0.14.3',
I am okay with bumping dependency requirements, but the nicer
alternative would be to allow the current version and check the
existence of the new API/function instead.
> required: get_option('spice'),
> method: 'pkg-config')
> endif
> diff --git a/ui/spice-display.c b/ui/spice-display.c
> index b7016ece6a..46b6d5dfc9 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -28,6 +28,8 @@
>
> #include "ui/spice-display.h"
>
> +#include "standard-headers/drm/drm_fourcc.h"
> +
> bool spice_opengl;
>
> int qemu_spice_rect_is_empty(const QXLRect* r)
> @@ -884,16 +886,11 @@ static void spice_gl_switch(DisplayChangeListener *dcl,
> if (ssd->ds) {
> uint32_t offset[DMABUF_MAX_PLANES], stride[DMABUF_MAX_PLANES];
> int fd[DMABUF_MAX_PLANES], num_planes, fourcc;
> + uint64_t modifier;
>
> surface_gl_create_texture(ssd->gls, ssd->ds);
> if (!egl_dmabuf_export_texture(ssd->ds->texture, fd, (EGLint *)offset,
> - (EGLint *)stride, &fourcc, &num_planes, NULL)) {
> - surface_gl_destroy_texture(ssd->gls, ssd->ds);
> - return;
> - }
> -
> - if (num_planes > 1) {
> - fprintf(stderr, "%s: does not support multi-plane texture\n", __func__);
> + (EGLint *)stride, &fourcc, &num_planes, &modifier)) {
> surface_gl_destroy_texture(ssd->gls, ssd->ds);
> return;
> }
> @@ -904,10 +901,11 @@ static void spice_gl_switch(DisplayChangeListener *dcl,
> fourcc);
>
> /* note: spice server will close the fd */
> - spice_qxl_gl_scanout(&ssd->qxl, fd[0],
> - surface_width(ssd->ds),
> - surface_height(ssd->ds),
> - stride[0], fourcc, false);
> + spice_qxl_gl_scanout2(&ssd->qxl, fd,
> + surface_width(ssd->ds),
> + surface_height(ssd->ds),
> + offset, stride, num_planes,
> + fourcc, modifier, false);
> ssd->have_surface = true;
> ssd->have_scanout = false;
>
> @@ -930,7 +928,8 @@ 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);
> - spice_qxl_gl_scanout(&ssd->qxl, -1, 0, 0, 0, 0, false);
> + spice_qxl_gl_scanout2(&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);
> ssd->have_surface = false;
> ssd->have_scanout = false;
> @@ -948,22 +947,21 @@ 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;
> + uint64_t modifier;
>
> assert(tex_id);
> if (!egl_dmabuf_export_texture(tex_id, fd, offset, stride, &fourcc,
> - &num_planes, NULL)) {
> + &num_planes, &modifier)) {
> fprintf(stderr, "%s: failed to export dmabuf for texture\n", __func__);
> return;
> }
> - if (num_planes > 1) {
> - fprintf(stderr, "%s: does not support multi-plane dmabuf\n", __func__);
> - return;
> - }
> +
> trace_qemu_spice_gl_scanout_texture(ssd->qxl.id, w, h, fourcc);
>
> /* note: spice server will close the fd */
> - spice_qxl_gl_scanout(&ssd->qxl, fd[0], backing_width, backing_height,
> - stride[0], fourcc, y_0_top);
> + spice_qxl_gl_scanout2(&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;
> @@ -1034,11 +1032,10 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
> uint32_t x, uint32_t y, uint32_t w, uint32_t h)
> {
> SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
> - EGLint stride = 0, fourcc = 0;
> + EGLint fourcc = 0;
> bool render_cursor = false;
> bool y_0_top = false; /* FIXME */
> uint64_t cookie;
> - int fd;
> uint32_t width, height, texture;
>
> if (!ssd->have_scanout) {
> @@ -1075,6 +1072,7 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
> ssd->blit_fb.height != height) {
> int fds[DMABUF_MAX_PLANES], num_planes;
> uint32_t offsets[DMABUF_MAX_PLANES], strides[DMABUF_MAX_PLANES];
> + uint64_t modifier;
>
> trace_qemu_spice_gl_render_dmabuf(ssd->qxl.id, width,
> height);
> @@ -1083,27 +1081,30 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
> width, height);
> if (!egl_dmabuf_export_texture(ssd->blit_fb.texture, fds,
> (EGLint *)offsets, (EGLint *)strides,
> - &fourcc, &num_planes, NULL)) {
> + &fourcc, &num_planes, &modifier)) {
> fprintf(stderr, "%s: failed to export dmabuf for texture\n", __func__);
> return;
> }
> - if (num_planes > 1) {
> - fprintf(stderr, "%s: does not support multi-plane dmabuf\n", __func__);
> - return;
> - }
> - spice_qxl_gl_scanout(&ssd->qxl, fds[0], width, height,
> - strides[0], fourcc, false);
> +
> + spice_qxl_gl_scanout2(&ssd->qxl, fds, width, height, offsets, strides,
> + num_planes, fourcc, modifier, false);
> }
> } else {
> - stride = qemu_dmabuf_get_stride(dmabuf)[0];
> + int fds[DMABUF_MAX_PLANES];
> +
> fourcc = qemu_dmabuf_get_fourcc(dmabuf);
> y_0_top = qemu_dmabuf_get_y0_top(dmabuf);
> - qemu_dmabuf_dup_fd(dmabuf, &fd);
> + qemu_dmabuf_dup_fd(dmabuf, fds);
>
> trace_qemu_spice_gl_forward_dmabuf(ssd->qxl.id, width, height);
> /* note: spice server will close the fd, so hand over a dup */
> - spice_qxl_gl_scanout(&ssd->qxl, fd, width, height,
> - stride, fourcc, y_0_top);
> + spice_qxl_gl_scanout2(&ssd->qxl, fds, width, height,
> + qemu_dmabuf_get_offset(dmabuf),
> + qemu_dmabuf_get_stride(dmabuf),
> + qemu_dmabuf_get_num_planes(dmabuf),
> + fourcc,
> + qemu_dmabuf_get_modifier(dmabuf),
> + y_0_top);
> }
> qemu_spice_gl_monitor_config(ssd, 0, 0, width, height);
> ssd->guest_dmabuf_refresh = false;
> --
> 2.43.0
>
>
--
Marc-André Lureau
On Mon, Mar 24, 2025 at 5:30 PM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Mon, Mar 24, 2025 at 12:20 PM <yuq825@gmail.com> wrote:
> >
> > From: Qiang Yu <yuq825@gmail.com>
> >
> > Signed-off-by: Qiang Yu <yuq825@gmail.com>
> > ---
> > meson.build | 2 +-
> > ui/spice-display.c | 65 +++++++++++++++++++++++-----------------------
> > 2 files changed, 34 insertions(+), 33 deletions(-)
> >
> > diff --git a/meson.build b/meson.build
> > index 9d9c11731f..b87704a83b 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -1329,7 +1329,7 @@ if get_option('spice') \
> > .require(pixman.found(),
> > error_message: 'cannot enable SPICE if pixman is not available') \
> > .allowed()
> > - spice = dependency('spice-server', version: '>=0.14.0',
> > + spice = dependency('spice-server', version: '>=0.14.3',
>
> I am okay with bumping dependency requirements, but the nicer
> alternative would be to allow the current version and check the
> existence of the new API/function instead.
>
I'm not familiar with how qemu handle new API dependency, but isn't it much
convenient to just bump lib version instead of a macro all around? And I can't
see similar macro in meson.build
>
> > required: get_option('spice'),
> > method: 'pkg-config')
> > endif
> > diff --git a/ui/spice-display.c b/ui/spice-display.c
> > index b7016ece6a..46b6d5dfc9 100644
> > --- a/ui/spice-display.c
> > +++ b/ui/spice-display.c
> > @@ -28,6 +28,8 @@
> >
> > #include "ui/spice-display.h"
> >
> > +#include "standard-headers/drm/drm_fourcc.h"
> > +
> > bool spice_opengl;
> >
> > int qemu_spice_rect_is_empty(const QXLRect* r)
> > @@ -884,16 +886,11 @@ static void spice_gl_switch(DisplayChangeListener *dcl,
> > if (ssd->ds) {
> > uint32_t offset[DMABUF_MAX_PLANES], stride[DMABUF_MAX_PLANES];
> > int fd[DMABUF_MAX_PLANES], num_planes, fourcc;
> > + uint64_t modifier;
> >
> > surface_gl_create_texture(ssd->gls, ssd->ds);
> > if (!egl_dmabuf_export_texture(ssd->ds->texture, fd, (EGLint *)offset,
> > - (EGLint *)stride, &fourcc, &num_planes, NULL)) {
> > - surface_gl_destroy_texture(ssd->gls, ssd->ds);
> > - return;
> > - }
> > -
> > - if (num_planes > 1) {
> > - fprintf(stderr, "%s: does not support multi-plane texture\n", __func__);
> > + (EGLint *)stride, &fourcc, &num_planes, &modifier)) {
> > surface_gl_destroy_texture(ssd->gls, ssd->ds);
> > return;
> > }
> > @@ -904,10 +901,11 @@ static void spice_gl_switch(DisplayChangeListener *dcl,
> > fourcc);
> >
> > /* note: spice server will close the fd */
> > - spice_qxl_gl_scanout(&ssd->qxl, fd[0],
> > - surface_width(ssd->ds),
> > - surface_height(ssd->ds),
> > - stride[0], fourcc, false);
> > + spice_qxl_gl_scanout2(&ssd->qxl, fd,
> > + surface_width(ssd->ds),
> > + surface_height(ssd->ds),
> > + offset, stride, num_planes,
> > + fourcc, modifier, false);
> > ssd->have_surface = true;
> > ssd->have_scanout = false;
> >
> > @@ -930,7 +928,8 @@ 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);
> > - spice_qxl_gl_scanout(&ssd->qxl, -1, 0, 0, 0, 0, false);
> > + spice_qxl_gl_scanout2(&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);
> > ssd->have_surface = false;
> > ssd->have_scanout = false;
> > @@ -948,22 +947,21 @@ 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;
> > + uint64_t modifier;
> >
> > assert(tex_id);
> > if (!egl_dmabuf_export_texture(tex_id, fd, offset, stride, &fourcc,
> > - &num_planes, NULL)) {
> > + &num_planes, &modifier)) {
> > fprintf(stderr, "%s: failed to export dmabuf for texture\n", __func__);
> > return;
> > }
> > - if (num_planes > 1) {
> > - fprintf(stderr, "%s: does not support multi-plane dmabuf\n", __func__);
> > - return;
> > - }
> > +
> > trace_qemu_spice_gl_scanout_texture(ssd->qxl.id, w, h, fourcc);
> >
> > /* note: spice server will close the fd */
> > - spice_qxl_gl_scanout(&ssd->qxl, fd[0], backing_width, backing_height,
> > - stride[0], fourcc, y_0_top);
> > + spice_qxl_gl_scanout2(&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;
> > @@ -1034,11 +1032,10 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
> > uint32_t x, uint32_t y, uint32_t w, uint32_t h)
> > {
> > SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
> > - EGLint stride = 0, fourcc = 0;
> > + EGLint fourcc = 0;
> > bool render_cursor = false;
> > bool y_0_top = false; /* FIXME */
> > uint64_t cookie;
> > - int fd;
> > uint32_t width, height, texture;
> >
> > if (!ssd->have_scanout) {
> > @@ -1075,6 +1072,7 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
> > ssd->blit_fb.height != height) {
> > int fds[DMABUF_MAX_PLANES], num_planes;
> > uint32_t offsets[DMABUF_MAX_PLANES], strides[DMABUF_MAX_PLANES];
> > + uint64_t modifier;
> >
> > trace_qemu_spice_gl_render_dmabuf(ssd->qxl.id, width,
> > height);
> > @@ -1083,27 +1081,30 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
> > width, height);
> > if (!egl_dmabuf_export_texture(ssd->blit_fb.texture, fds,
> > (EGLint *)offsets, (EGLint *)strides,
> > - &fourcc, &num_planes, NULL)) {
> > + &fourcc, &num_planes, &modifier)) {
> > fprintf(stderr, "%s: failed to export dmabuf for texture\n", __func__);
> > return;
> > }
> > - if (num_planes > 1) {
> > - fprintf(stderr, "%s: does not support multi-plane dmabuf\n", __func__);
> > - return;
> > - }
> > - spice_qxl_gl_scanout(&ssd->qxl, fds[0], width, height,
> > - strides[0], fourcc, false);
> > +
> > + spice_qxl_gl_scanout2(&ssd->qxl, fds, width, height, offsets, strides,
> > + num_planes, fourcc, modifier, false);
> > }
> > } else {
> > - stride = qemu_dmabuf_get_stride(dmabuf)[0];
> > + int fds[DMABUF_MAX_PLANES];
> > +
> > fourcc = qemu_dmabuf_get_fourcc(dmabuf);
> > y_0_top = qemu_dmabuf_get_y0_top(dmabuf);
> > - qemu_dmabuf_dup_fd(dmabuf, &fd);
> > + qemu_dmabuf_dup_fd(dmabuf, fds);
> >
> > trace_qemu_spice_gl_forward_dmabuf(ssd->qxl.id, width, height);
> > /* note: spice server will close the fd, so hand over a dup */
> > - spice_qxl_gl_scanout(&ssd->qxl, fd, width, height,
> > - stride, fourcc, y_0_top);
> > + spice_qxl_gl_scanout2(&ssd->qxl, fds, width, height,
> > + qemu_dmabuf_get_offset(dmabuf),
> > + qemu_dmabuf_get_stride(dmabuf),
> > + qemu_dmabuf_get_num_planes(dmabuf),
> > + fourcc,
> > + qemu_dmabuf_get_modifier(dmabuf),
> > + y_0_top);
> > }
> > qemu_spice_gl_monitor_config(ssd, 0, 0, width, height);
> > ssd->guest_dmabuf_refresh = false;
> > --
> > 2.43.0
> >
> >
>
>
> --
> Marc-André Lureau
On Mon, Mar 24, 2025 at 5:35 PM Qiang Yu <yuq825@gmail.com> wrote:
>
> On Mon, Mar 24, 2025 at 5:30 PM Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> >
> > Hi
> >
> > On Mon, Mar 24, 2025 at 12:20 PM <yuq825@gmail.com> wrote:
> > >
> > > From: Qiang Yu <yuq825@gmail.com>
> > >
> > > Signed-off-by: Qiang Yu <yuq825@gmail.com>
> > > ---
> > > meson.build | 2 +-
> > > ui/spice-display.c | 65 +++++++++++++++++++++++-----------------------
> > > 2 files changed, 34 insertions(+), 33 deletions(-)
> > >
> > > diff --git a/meson.build b/meson.build
> > > index 9d9c11731f..b87704a83b 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -1329,7 +1329,7 @@ if get_option('spice') \
> > > .require(pixman.found(),
> > > error_message: 'cannot enable SPICE if pixman is not available') \
> > > .allowed()
> > > - spice = dependency('spice-server', version: '>=0.14.0',
> > > + spice = dependency('spice-server', version: '>=0.14.3',
> >
> > I am okay with bumping dependency requirements, but the nicer
> > alternative would be to allow the current version and check the
> > existence of the new API/function instead.
> >
> I'm not familiar with how qemu handle new API dependency, but isn't it much
> convenient to just bump lib version instead of a macro all around? And I can't
> see similar macro in meson.build
Yes it is generally simpler to bump requirements, but as Daniel
hinted, we have policies about supporting older environments.
For your series, I think we could simply have:
if spice.found()
config_host_data.set('HAVE_SPICE_QXL_GL_SCANOUT2',
cc.has_function('spice_qxl_gl_scanout2', dependencies: spice))
endif
>
> >
> > > required: get_option('spice'),
> > > method: 'pkg-config')
> > > endif
> > > diff --git a/ui/spice-display.c b/ui/spice-display.c
> > > index b7016ece6a..46b6d5dfc9 100644
> > > --- a/ui/spice-display.c
> > > +++ b/ui/spice-display.c
> > > @@ -28,6 +28,8 @@
> > >
> > > #include "ui/spice-display.h"
> > >
> > > +#include "standard-headers/drm/drm_fourcc.h"
> > > +
> > > bool spice_opengl;
> > >
> > > int qemu_spice_rect_is_empty(const QXLRect* r)
> > > @@ -884,16 +886,11 @@ static void spice_gl_switch(DisplayChangeListener *dcl,
> > > if (ssd->ds) {
> > > uint32_t offset[DMABUF_MAX_PLANES], stride[DMABUF_MAX_PLANES];
> > > int fd[DMABUF_MAX_PLANES], num_planes, fourcc;
> > > + uint64_t modifier;
> > >
> > > surface_gl_create_texture(ssd->gls, ssd->ds);
> > > if (!egl_dmabuf_export_texture(ssd->ds->texture, fd, (EGLint *)offset,
> > > - (EGLint *)stride, &fourcc, &num_planes, NULL)) {
> > > - surface_gl_destroy_texture(ssd->gls, ssd->ds);
> > > - return;
> > > - }
> > > -
> > > - if (num_planes > 1) {
> > > - fprintf(stderr, "%s: does not support multi-plane texture\n", __func__);
> > > + (EGLint *)stride, &fourcc, &num_planes, &modifier)) {
> > > surface_gl_destroy_texture(ssd->gls, ssd->ds);
> > > return;
> > > }
> > > @@ -904,10 +901,11 @@ static void spice_gl_switch(DisplayChangeListener *dcl,
> > > fourcc);
> > >
> > > /* note: spice server will close the fd */
> > > - spice_qxl_gl_scanout(&ssd->qxl, fd[0],
> > > - surface_width(ssd->ds),
> > > - surface_height(ssd->ds),
> > > - stride[0], fourcc, false);
> > > + spice_qxl_gl_scanout2(&ssd->qxl, fd,
> > > + surface_width(ssd->ds),
> > > + surface_height(ssd->ds),
> > > + offset, stride, num_planes,
> > > + fourcc, modifier, false);
> > > ssd->have_surface = true;
> > > ssd->have_scanout = false;
> > >
> > > @@ -930,7 +928,8 @@ 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);
> > > - spice_qxl_gl_scanout(&ssd->qxl, -1, 0, 0, 0, 0, false);
> > > + spice_qxl_gl_scanout2(&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);
> > > ssd->have_surface = false;
> > > ssd->have_scanout = false;
> > > @@ -948,22 +947,21 @@ 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;
> > > + uint64_t modifier;
> > >
> > > assert(tex_id);
> > > if (!egl_dmabuf_export_texture(tex_id, fd, offset, stride, &fourcc,
> > > - &num_planes, NULL)) {
> > > + &num_planes, &modifier)) {
> > > fprintf(stderr, "%s: failed to export dmabuf for texture\n", __func__);
> > > return;
> > > }
> > > - if (num_planes > 1) {
> > > - fprintf(stderr, "%s: does not support multi-plane dmabuf\n", __func__);
> > > - return;
> > > - }
> > > +
> > > trace_qemu_spice_gl_scanout_texture(ssd->qxl.id, w, h, fourcc);
> > >
> > > /* note: spice server will close the fd */
> > > - spice_qxl_gl_scanout(&ssd->qxl, fd[0], backing_width, backing_height,
> > > - stride[0], fourcc, y_0_top);
> > > + spice_qxl_gl_scanout2(&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;
> > > @@ -1034,11 +1032,10 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
> > > uint32_t x, uint32_t y, uint32_t w, uint32_t h)
> > > {
> > > SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
> > > - EGLint stride = 0, fourcc = 0;
> > > + EGLint fourcc = 0;
> > > bool render_cursor = false;
> > > bool y_0_top = false; /* FIXME */
> > > uint64_t cookie;
> > > - int fd;
> > > uint32_t width, height, texture;
> > >
> > > if (!ssd->have_scanout) {
> > > @@ -1075,6 +1072,7 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
> > > ssd->blit_fb.height != height) {
> > > int fds[DMABUF_MAX_PLANES], num_planes;
> > > uint32_t offsets[DMABUF_MAX_PLANES], strides[DMABUF_MAX_PLANES];
> > > + uint64_t modifier;
> > >
> > > trace_qemu_spice_gl_render_dmabuf(ssd->qxl.id, width,
> > > height);
> > > @@ -1083,27 +1081,30 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
> > > width, height);
> > > if (!egl_dmabuf_export_texture(ssd->blit_fb.texture, fds,
> > > (EGLint *)offsets, (EGLint *)strides,
> > > - &fourcc, &num_planes, NULL)) {
> > > + &fourcc, &num_planes, &modifier)) {
> > > fprintf(stderr, "%s: failed to export dmabuf for texture\n", __func__);
> > > return;
> > > }
> > > - if (num_planes > 1) {
> > > - fprintf(stderr, "%s: does not support multi-plane dmabuf\n", __func__);
> > > - return;
> > > - }
> > > - spice_qxl_gl_scanout(&ssd->qxl, fds[0], width, height,
> > > - strides[0], fourcc, false);
> > > +
> > > + spice_qxl_gl_scanout2(&ssd->qxl, fds, width, height, offsets, strides,
> > > + num_planes, fourcc, modifier, false);
> > > }
> > > } else {
> > > - stride = qemu_dmabuf_get_stride(dmabuf)[0];
> > > + int fds[DMABUF_MAX_PLANES];
> > > +
> > > fourcc = qemu_dmabuf_get_fourcc(dmabuf);
> > > y_0_top = qemu_dmabuf_get_y0_top(dmabuf);
> > > - qemu_dmabuf_dup_fd(dmabuf, &fd);
> > > + qemu_dmabuf_dup_fd(dmabuf, fds);
> > >
> > > trace_qemu_spice_gl_forward_dmabuf(ssd->qxl.id, width, height);
> > > /* note: spice server will close the fd, so hand over a dup */
> > > - spice_qxl_gl_scanout(&ssd->qxl, fd, width, height,
> > > - stride, fourcc, y_0_top);
> > > + spice_qxl_gl_scanout2(&ssd->qxl, fds, width, height,
> > > + qemu_dmabuf_get_offset(dmabuf),
> > > + qemu_dmabuf_get_stride(dmabuf),
> > > + qemu_dmabuf_get_num_planes(dmabuf),
> > > + fourcc,
> > > + qemu_dmabuf_get_modifier(dmabuf),
> > > + y_0_top);
> > > }
> > > qemu_spice_gl_monitor_config(ssd, 0, 0, width, height);
> > > ssd->guest_dmabuf_refresh = false;
> > > --
> > > 2.43.0
> > >
> > >
> >
> >
> > --
> > Marc-André Lureau
--
Marc-André Lureau
On Mon, Mar 24, 2025 at 10:02 PM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> On Mon, Mar 24, 2025 at 5:35 PM Qiang Yu <yuq825@gmail.com> wrote:
> >
> > On Mon, Mar 24, 2025 at 5:30 PM Marc-André Lureau
> > <marcandre.lureau@gmail.com> wrote:
> > >
> > > Hi
> > >
> > > On Mon, Mar 24, 2025 at 12:20 PM <yuq825@gmail.com> wrote:
> > > >
> > > > From: Qiang Yu <yuq825@gmail.com>
> > > >
> > > > Signed-off-by: Qiang Yu <yuq825@gmail.com>
> > > > ---
> > > > meson.build | 2 +-
> > > > ui/spice-display.c | 65 +++++++++++++++++++++++-----------------------
> > > > 2 files changed, 34 insertions(+), 33 deletions(-)
> > > >
> > > > diff --git a/meson.build b/meson.build
> > > > index 9d9c11731f..b87704a83b 100644
> > > > --- a/meson.build
> > > > +++ b/meson.build
> > > > @@ -1329,7 +1329,7 @@ if get_option('spice') \
> > > > .require(pixman.found(),
> > > > error_message: 'cannot enable SPICE if pixman is not available') \
> > > > .allowed()
> > > > - spice = dependency('spice-server', version: '>=0.14.0',
> > > > + spice = dependency('spice-server', version: '>=0.14.3',
> > >
> > > I am okay with bumping dependency requirements, but the nicer
> > > alternative would be to allow the current version and check the
> > > existence of the new API/function instead.
> > >
> > I'm not familiar with how qemu handle new API dependency, but isn't it much
> > convenient to just bump lib version instead of a macro all around? And I can't
> > see similar macro in meson.build
>
> Yes it is generally simpler to bump requirements, but as Daniel
> hinted, we have policies about supporting older environments.
>
> For your series, I think we could simply have:
> if spice.found()
> config_host_data.set('HAVE_SPICE_QXL_GL_SCANOUT2',
> cc.has_function('spice_qxl_gl_scanout2', dependencies: spice))
> endif
>
This serial is more like a bug fix instead of feature adding. Because without
new spice-server, qemu spice gl scanout support is completely broken when
run on multi plane GPU driver. If not bumping spice version, user may still
suffer this problem when using new qemu and old spice. Is this OK?
>
> >
> > >
> > > > required: get_option('spice'),
> > > > method: 'pkg-config')
> > > > endif
> > > > diff --git a/ui/spice-display.c b/ui/spice-display.c
> > > > index b7016ece6a..46b6d5dfc9 100644
> > > > --- a/ui/spice-display.c
> > > > +++ b/ui/spice-display.c
> > > > @@ -28,6 +28,8 @@
> > > >
> > > > #include "ui/spice-display.h"
> > > >
> > > > +#include "standard-headers/drm/drm_fourcc.h"
> > > > +
> > > > bool spice_opengl;
> > > >
> > > > int qemu_spice_rect_is_empty(const QXLRect* r)
> > > > @@ -884,16 +886,11 @@ static void spice_gl_switch(DisplayChangeListener *dcl,
> > > > if (ssd->ds) {
> > > > uint32_t offset[DMABUF_MAX_PLANES], stride[DMABUF_MAX_PLANES];
> > > > int fd[DMABUF_MAX_PLANES], num_planes, fourcc;
> > > > + uint64_t modifier;
> > > >
> > > > surface_gl_create_texture(ssd->gls, ssd->ds);
> > > > if (!egl_dmabuf_export_texture(ssd->ds->texture, fd, (EGLint *)offset,
> > > > - (EGLint *)stride, &fourcc, &num_planes, NULL)) {
> > > > - surface_gl_destroy_texture(ssd->gls, ssd->ds);
> > > > - return;
> > > > - }
> > > > -
> > > > - if (num_planes > 1) {
> > > > - fprintf(stderr, "%s: does not support multi-plane texture\n", __func__);
> > > > + (EGLint *)stride, &fourcc, &num_planes, &modifier)) {
> > > > surface_gl_destroy_texture(ssd->gls, ssd->ds);
> > > > return;
> > > > }
> > > > @@ -904,10 +901,11 @@ static void spice_gl_switch(DisplayChangeListener *dcl,
> > > > fourcc);
> > > >
> > > > /* note: spice server will close the fd */
> > > > - spice_qxl_gl_scanout(&ssd->qxl, fd[0],
> > > > - surface_width(ssd->ds),
> > > > - surface_height(ssd->ds),
> > > > - stride[0], fourcc, false);
> > > > + spice_qxl_gl_scanout2(&ssd->qxl, fd,
> > > > + surface_width(ssd->ds),
> > > > + surface_height(ssd->ds),
> > > > + offset, stride, num_planes,
> > > > + fourcc, modifier, false);
> > > > ssd->have_surface = true;
> > > > ssd->have_scanout = false;
> > > >
> > > > @@ -930,7 +928,8 @@ 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);
> > > > - spice_qxl_gl_scanout(&ssd->qxl, -1, 0, 0, 0, 0, false);
> > > > + spice_qxl_gl_scanout2(&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);
> > > > ssd->have_surface = false;
> > > > ssd->have_scanout = false;
> > > > @@ -948,22 +947,21 @@ 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;
> > > > + uint64_t modifier;
> > > >
> > > > assert(tex_id);
> > > > if (!egl_dmabuf_export_texture(tex_id, fd, offset, stride, &fourcc,
> > > > - &num_planes, NULL)) {
> > > > + &num_planes, &modifier)) {
> > > > fprintf(stderr, "%s: failed to export dmabuf for texture\n", __func__);
> > > > return;
> > > > }
> > > > - if (num_planes > 1) {
> > > > - fprintf(stderr, "%s: does not support multi-plane dmabuf\n", __func__);
> > > > - return;
> > > > - }
> > > > +
> > > > trace_qemu_spice_gl_scanout_texture(ssd->qxl.id, w, h, fourcc);
> > > >
> > > > /* note: spice server will close the fd */
> > > > - spice_qxl_gl_scanout(&ssd->qxl, fd[0], backing_width, backing_height,
> > > > - stride[0], fourcc, y_0_top);
> > > > + spice_qxl_gl_scanout2(&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;
> > > > @@ -1034,11 +1032,10 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
> > > > uint32_t x, uint32_t y, uint32_t w, uint32_t h)
> > > > {
> > > > SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
> > > > - EGLint stride = 0, fourcc = 0;
> > > > + EGLint fourcc = 0;
> > > > bool render_cursor = false;
> > > > bool y_0_top = false; /* FIXME */
> > > > uint64_t cookie;
> > > > - int fd;
> > > > uint32_t width, height, texture;
> > > >
> > > > if (!ssd->have_scanout) {
> > > > @@ -1075,6 +1072,7 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
> > > > ssd->blit_fb.height != height) {
> > > > int fds[DMABUF_MAX_PLANES], num_planes;
> > > > uint32_t offsets[DMABUF_MAX_PLANES], strides[DMABUF_MAX_PLANES];
> > > > + uint64_t modifier;
> > > >
> > > > trace_qemu_spice_gl_render_dmabuf(ssd->qxl.id, width,
> > > > height);
> > > > @@ -1083,27 +1081,30 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
> > > > width, height);
> > > > if (!egl_dmabuf_export_texture(ssd->blit_fb.texture, fds,
> > > > (EGLint *)offsets, (EGLint *)strides,
> > > > - &fourcc, &num_planes, NULL)) {
> > > > + &fourcc, &num_planes, &modifier)) {
> > > > fprintf(stderr, "%s: failed to export dmabuf for texture\n", __func__);
> > > > return;
> > > > }
> > > > - if (num_planes > 1) {
> > > > - fprintf(stderr, "%s: does not support multi-plane dmabuf\n", __func__);
> > > > - return;
> > > > - }
> > > > - spice_qxl_gl_scanout(&ssd->qxl, fds[0], width, height,
> > > > - strides[0], fourcc, false);
> > > > +
> > > > + spice_qxl_gl_scanout2(&ssd->qxl, fds, width, height, offsets, strides,
> > > > + num_planes, fourcc, modifier, false);
> > > > }
> > > > } else {
> > > > - stride = qemu_dmabuf_get_stride(dmabuf)[0];
> > > > + int fds[DMABUF_MAX_PLANES];
> > > > +
> > > > fourcc = qemu_dmabuf_get_fourcc(dmabuf);
> > > > y_0_top = qemu_dmabuf_get_y0_top(dmabuf);
> > > > - qemu_dmabuf_dup_fd(dmabuf, &fd);
> > > > + qemu_dmabuf_dup_fd(dmabuf, fds);
> > > >
> > > > trace_qemu_spice_gl_forward_dmabuf(ssd->qxl.id, width, height);
> > > > /* note: spice server will close the fd, so hand over a dup */
> > > > - spice_qxl_gl_scanout(&ssd->qxl, fd, width, height,
> > > > - stride, fourcc, y_0_top);
> > > > + spice_qxl_gl_scanout2(&ssd->qxl, fds, width, height,
> > > > + qemu_dmabuf_get_offset(dmabuf),
> > > > + qemu_dmabuf_get_stride(dmabuf),
> > > > + qemu_dmabuf_get_num_planes(dmabuf),
> > > > + fourcc,
> > > > + qemu_dmabuf_get_modifier(dmabuf),
> > > > + y_0_top);
> > > > }
> > > > qemu_spice_gl_monitor_config(ssd, 0, 0, width, height);
> > > > ssd->guest_dmabuf_refresh = false;
> > > > --
> > > > 2.43.0
> > > >
> > > >
> > >
> > >
> > > --
> > > Marc-André Lureau
>
>
>
> --
> Marc-André Lureau
Hi
On Wed, Mar 26, 2025 at 5:46 AM Qiang Yu <yuq825@gmail.com> wrote:
>
> On Mon, Mar 24, 2025 at 10:02 PM Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> >
> > On Mon, Mar 24, 2025 at 5:35 PM Qiang Yu <yuq825@gmail.com> wrote:
> > >
> > > On Mon, Mar 24, 2025 at 5:30 PM Marc-André Lureau
> > > <marcandre.lureau@gmail.com> wrote:
> > > >
> > > > Hi
> > > >
> > > > On Mon, Mar 24, 2025 at 12:20 PM <yuq825@gmail.com> wrote:
> > > > >
> > > > > From: Qiang Yu <yuq825@gmail.com>
> > > > >
> > > > > Signed-off-by: Qiang Yu <yuq825@gmail.com>
> > > > > ---
> > > > > meson.build | 2 +-
> > > > > ui/spice-display.c | 65 +++++++++++++++++++++++-----------------------
> > > > > 2 files changed, 34 insertions(+), 33 deletions(-)
> > > > >
> > > > > diff --git a/meson.build b/meson.build
> > > > > index 9d9c11731f..b87704a83b 100644
> > > > > --- a/meson.build
> > > > > +++ b/meson.build
> > > > > @@ -1329,7 +1329,7 @@ if get_option('spice') \
> > > > > .require(pixman.found(),
> > > > > error_message: 'cannot enable SPICE if pixman is not available') \
> > > > > .allowed()
> > > > > - spice = dependency('spice-server', version: '>=0.14.0',
> > > > > + spice = dependency('spice-server', version: '>=0.14.3',
> > > >
> > > > I am okay with bumping dependency requirements, but the nicer
> > > > alternative would be to allow the current version and check the
> > > > existence of the new API/function instead.
> > > >
> > > I'm not familiar with how qemu handle new API dependency, but isn't it much
> > > convenient to just bump lib version instead of a macro all around? And I can't
> > > see similar macro in meson.build
> >
> > Yes it is generally simpler to bump requirements, but as Daniel
> > hinted, we have policies about supporting older environments.
> >
> > For your series, I think we could simply have:
> > if spice.found()
> > config_host_data.set('HAVE_SPICE_QXL_GL_SCANOUT2',
> > cc.has_function('spice_qxl_gl_scanout2', dependencies: spice))
> > endif
> >
> This serial is more like a bug fix instead of feature adding. Because without
> new spice-server, qemu spice gl scanout support is completely broken when
> run on multi plane GPU driver. If not bumping spice version, user may still
> suffer this problem when using new qemu and old spice. Is this OK?
Yes, the new QEMU should print a warning or error out on startup in
such situation.
> >
> > >
> > > >
> > > > > required: get_option('spice'),
> > > > > method: 'pkg-config')
> > > > > endif
> > > > > diff --git a/ui/spice-display.c b/ui/spice-display.c
> > > > > index b7016ece6a..46b6d5dfc9 100644
> > > > > --- a/ui/spice-display.c
> > > > > +++ b/ui/spice-display.c
> > > > > @@ -28,6 +28,8 @@
> > > > >
> > > > > #include "ui/spice-display.h"
> > > > >
> > > > > +#include "standard-headers/drm/drm_fourcc.h"
> > > > > +
> > > > > bool spice_opengl;
> > > > >
> > > > > int qemu_spice_rect_is_empty(const QXLRect* r)
> > > > > @@ -884,16 +886,11 @@ static void spice_gl_switch(DisplayChangeListener *dcl,
> > > > > if (ssd->ds) {
> > > > > uint32_t offset[DMABUF_MAX_PLANES], stride[DMABUF_MAX_PLANES];
> > > > > int fd[DMABUF_MAX_PLANES], num_planes, fourcc;
> > > > > + uint64_t modifier;
> > > > >
> > > > > surface_gl_create_texture(ssd->gls, ssd->ds);
> > > > > if (!egl_dmabuf_export_texture(ssd->ds->texture, fd, (EGLint *)offset,
> > > > > - (EGLint *)stride, &fourcc, &num_planes, NULL)) {
> > > > > - surface_gl_destroy_texture(ssd->gls, ssd->ds);
> > > > > - return;
> > > > > - }
> > > > > -
> > > > > - if (num_planes > 1) {
> > > > > - fprintf(stderr, "%s: does not support multi-plane texture\n", __func__);
> > > > > + (EGLint *)stride, &fourcc, &num_planes, &modifier)) {
> > > > > surface_gl_destroy_texture(ssd->gls, ssd->ds);
> > > > > return;
> > > > > }
> > > > > @@ -904,10 +901,11 @@ static void spice_gl_switch(DisplayChangeListener *dcl,
> > > > > fourcc);
> > > > >
> > > > > /* note: spice server will close the fd */
> > > > > - spice_qxl_gl_scanout(&ssd->qxl, fd[0],
> > > > > - surface_width(ssd->ds),
> > > > > - surface_height(ssd->ds),
> > > > > - stride[0], fourcc, false);
> > > > > + spice_qxl_gl_scanout2(&ssd->qxl, fd,
> > > > > + surface_width(ssd->ds),
> > > > > + surface_height(ssd->ds),
> > > > > + offset, stride, num_planes,
> > > > > + fourcc, modifier, false);
> > > > > ssd->have_surface = true;
> > > > > ssd->have_scanout = false;
> > > > >
> > > > > @@ -930,7 +928,8 @@ 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);
> > > > > - spice_qxl_gl_scanout(&ssd->qxl, -1, 0, 0, 0, 0, false);
> > > > > + spice_qxl_gl_scanout2(&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);
> > > > > ssd->have_surface = false;
> > > > > ssd->have_scanout = false;
> > > > > @@ -948,22 +947,21 @@ 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;
> > > > > + uint64_t modifier;
> > > > >
> > > > > assert(tex_id);
> > > > > if (!egl_dmabuf_export_texture(tex_id, fd, offset, stride, &fourcc,
> > > > > - &num_planes, NULL)) {
> > > > > + &num_planes, &modifier)) {
> > > > > fprintf(stderr, "%s: failed to export dmabuf for texture\n", __func__);
> > > > > return;
> > > > > }
> > > > > - if (num_planes > 1) {
> > > > > - fprintf(stderr, "%s: does not support multi-plane dmabuf\n", __func__);
> > > > > - return;
> > > > > - }
> > > > > +
> > > > > trace_qemu_spice_gl_scanout_texture(ssd->qxl.id, w, h, fourcc);
> > > > >
> > > > > /* note: spice server will close the fd */
> > > > > - spice_qxl_gl_scanout(&ssd->qxl, fd[0], backing_width, backing_height,
> > > > > - stride[0], fourcc, y_0_top);
> > > > > + spice_qxl_gl_scanout2(&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;
> > > > > @@ -1034,11 +1032,10 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
> > > > > uint32_t x, uint32_t y, uint32_t w, uint32_t h)
> > > > > {
> > > > > SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
> > > > > - EGLint stride = 0, fourcc = 0;
> > > > > + EGLint fourcc = 0;
> > > > > bool render_cursor = false;
> > > > > bool y_0_top = false; /* FIXME */
> > > > > uint64_t cookie;
> > > > > - int fd;
> > > > > uint32_t width, height, texture;
> > > > >
> > > > > if (!ssd->have_scanout) {
> > > > > @@ -1075,6 +1072,7 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
> > > > > ssd->blit_fb.height != height) {
> > > > > int fds[DMABUF_MAX_PLANES], num_planes;
> > > > > uint32_t offsets[DMABUF_MAX_PLANES], strides[DMABUF_MAX_PLANES];
> > > > > + uint64_t modifier;
> > > > >
> > > > > trace_qemu_spice_gl_render_dmabuf(ssd->qxl.id, width,
> > > > > height);
> > > > > @@ -1083,27 +1081,30 @@ static void qemu_spice_gl_update(DisplayChangeListener *dcl,
> > > > > width, height);
> > > > > if (!egl_dmabuf_export_texture(ssd->blit_fb.texture, fds,
> > > > > (EGLint *)offsets, (EGLint *)strides,
> > > > > - &fourcc, &num_planes, NULL)) {
> > > > > + &fourcc, &num_planes, &modifier)) {
> > > > > fprintf(stderr, "%s: failed to export dmabuf for texture\n", __func__);
> > > > > return;
> > > > > }
> > > > > - if (num_planes > 1) {
> > > > > - fprintf(stderr, "%s: does not support multi-plane dmabuf\n", __func__);
> > > > > - return;
> > > > > - }
> > > > > - spice_qxl_gl_scanout(&ssd->qxl, fds[0], width, height,
> > > > > - strides[0], fourcc, false);
> > > > > +
> > > > > + spice_qxl_gl_scanout2(&ssd->qxl, fds, width, height, offsets, strides,
> > > > > + num_planes, fourcc, modifier, false);
> > > > > }
> > > > > } else {
> > > > > - stride = qemu_dmabuf_get_stride(dmabuf)[0];
> > > > > + int fds[DMABUF_MAX_PLANES];
> > > > > +
> > > > > fourcc = qemu_dmabuf_get_fourcc(dmabuf);
> > > > > y_0_top = qemu_dmabuf_get_y0_top(dmabuf);
> > > > > - qemu_dmabuf_dup_fd(dmabuf, &fd);
> > > > > + qemu_dmabuf_dup_fd(dmabuf, fds);
> > > > >
> > > > > trace_qemu_spice_gl_forward_dmabuf(ssd->qxl.id, width, height);
> > > > > /* note: spice server will close the fd, so hand over a dup */
> > > > > - spice_qxl_gl_scanout(&ssd->qxl, fd, width, height,
> > > > > - stride, fourcc, y_0_top);
> > > > > + spice_qxl_gl_scanout2(&ssd->qxl, fds, width, height,
> > > > > + qemu_dmabuf_get_offset(dmabuf),
> > > > > + qemu_dmabuf_get_stride(dmabuf),
> > > > > + qemu_dmabuf_get_num_planes(dmabuf),
> > > > > + fourcc,
> > > > > + qemu_dmabuf_get_modifier(dmabuf),
> > > > > + y_0_top);
> > > > > }
> > > > > qemu_spice_gl_monitor_config(ssd, 0, 0, width, height);
> > > > > ssd->guest_dmabuf_refresh = false;
> > > > > --
> > > > > 2.43.0
> > > > >
> > > > >
> > > >
> > > >
> > > > --
> > > > Marc-André Lureau
> >
> >
> >
> > --
> > Marc-André Lureau
--
Marc-André Lureau
On Mon, Mar 24, 2025 at 01:30:16PM +0400, Marc-André Lureau wrote:
> Hi
>
> On Mon, Mar 24, 2025 at 12:20 PM <yuq825@gmail.com> wrote:
> >
> > From: Qiang Yu <yuq825@gmail.com>
Please provide non-empty commit messages.
> >
> > Signed-off-by: Qiang Yu <yuq825@gmail.com>
> > ---
> > meson.build | 2 +-
> > ui/spice-display.c | 65 +++++++++++++++++++++++-----------------------
> > 2 files changed, 34 insertions(+), 33 deletions(-)
> >
> > diff --git a/meson.build b/meson.build
> > index 9d9c11731f..b87704a83b 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -1329,7 +1329,7 @@ if get_option('spice') \
> > .require(pixman.found(),
> > error_message: 'cannot enable SPICE if pixman is not available') \
> > .allowed()
> > - spice = dependency('spice-server', version: '>=0.14.0',
> > + spice = dependency('spice-server', version: '>=0.14.3',
>
> I am okay with bumping dependency requirements, but the nicer
> alternative would be to allow the current version and check the
> existence of the new API/function instead.
Bumping the min version must be a separate commit that lists
the current versions in our supported build platforms[1], in order
to demonstrate the that the chosen min version doesn't exclude
any supported platform.
See commit 34d55725e664445ccd5621165b1ef805197a530e for how
this should be done.
0.14.3 looks like it would probably be fine as a choice though,
as Ubuntu 20.04 has aged out of our support matrix.
With regards,
Daniel
[1] https://www.qemu.org/docs/master/about/build-platforms.html
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
© 2016 - 2025 Red Hat, Inc.