* [PATCH 1/3] virtio-gpu: rutabaga: Properly set stride when copying resources
2024-06-05 15:28 [PATCH 0/3] virtio-gpu: Enable virglrenderer backend for rutabaga Weifeng Liu
@ 2024-06-05 15:28 ` Weifeng Liu
2024-06-10 7:44 ` Marc-André Lureau
2024-06-05 15:28 ` [PATCH 2/3] virtio-gpu: rutabaga: Poll rutabaga upon events Weifeng Liu
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Weifeng Liu @ 2024-06-05 15:28 UTC (permalink / raw)
To: qemu-devel, Gurchetan Singh, Michael S. Tsirkin, Gerd Hoffmann
Cc: Weifeng Liu, Antonio Caggiano, Huang Rui, Dmitry Osipenko,
Marc-André Lureau
The stride is not correctly assigned when copying pixel data, causing
images being displayed incomplete when using 2d component of rutabaga.
Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
---
hw/display/virtio-gpu-rutabaga.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
index 17bf701a21..2ba6869606 100644
--- a/hw/display/virtio-gpu-rutabaga.c
+++ b/hw/display/virtio-gpu-rutabaga.c
@@ -53,6 +53,7 @@ virtio_gpu_rutabaga_update_cursor(VirtIOGPU *g, struct virtio_gpu_scanout *s,
transfer.z = 0;
transfer.w = res->width;
transfer.h = res->height;
+ transfer.stride = res->width * 4;
transfer.d = 1;
transfer_iovec.iov_base = s->current_cursor->data;
@@ -273,6 +274,7 @@ rutabaga_cmd_resource_flush(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd)
transfer.z = 0;
transfer.w = res->width;
transfer.h = res->height;
+ transfer.stride = pixman_image_get_stride(res->image);
transfer.d = 1;
transfer_iovec.iov_base = pixman_image_get_data(res->image);
@@ -382,6 +384,7 @@ rutabaga_cmd_transfer_to_host_2d(VirtIOGPU *g,
transfer.z = 0;
transfer.w = t2d.r.width;
transfer.h = t2d.r.height;
+ transfer.stride = t2d.r.width * 4;
transfer.d = 1;
result = rutabaga_resource_transfer_write(vr->rutabaga, 0, t2d.resource_id,
--
2.45.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/3] virtio-gpu: rutabaga: Properly set stride when copying resources
2024-06-05 15:28 ` [PATCH 1/3] virtio-gpu: rutabaga: Properly set stride when copying resources Weifeng Liu
@ 2024-06-10 7:44 ` Marc-André Lureau
2024-06-14 23:50 ` Gurchetan Singh
0 siblings, 1 reply; 9+ messages in thread
From: Marc-André Lureau @ 2024-06-10 7:44 UTC (permalink / raw)
To: Weifeng Liu, Gurchetan Singh
Cc: qemu-devel, Michael S. Tsirkin, Gerd Hoffmann, Antonio Caggiano,
Huang Rui, Dmitry Osipenko
[-- Attachment #1: Type: text/plain, Size: 1893 bytes --]
Hi
On Wed, Jun 5, 2024 at 7:30 PM Weifeng Liu <weifeng.liu.z@gmail.com> wrote:
> The stride is not correctly assigned when copying pixel data, causing
> images being displayed incomplete when using 2d component of rutabaga.
>
> Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
> ---
> hw/display/virtio-gpu-rutabaga.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/hw/display/virtio-gpu-rutabaga.c
> b/hw/display/virtio-gpu-rutabaga.c
> index 17bf701a21..2ba6869606 100644
> --- a/hw/display/virtio-gpu-rutabaga.c
> +++ b/hw/display/virtio-gpu-rutabaga.c
> @@ -53,6 +53,7 @@ virtio_gpu_rutabaga_update_cursor(VirtIOGPU *g, struct
> virtio_gpu_scanout *s,
> transfer.z = 0;
> transfer.w = res->width;
> transfer.h = res->height;
> + transfer.stride = res->width * 4;
>
ok, stride defined by QEMUCursor layout
> transfer.d = 1;
>
> transfer_iovec.iov_base = s->current_cursor->data;
> @@ -273,6 +274,7 @@ rutabaga_cmd_resource_flush(VirtIOGPU *g, struct
> virtio_gpu_ctrl_command *cmd)
> transfer.z = 0;
> transfer.w = res->width;
> transfer.h = res->height;
> + transfer.stride = pixman_image_get_stride(res->image);
> transfer.d = 1;
>
ok (destination image stride)
> transfer_iovec.iov_base = pixman_image_get_data(res->image);
> @@ -382,6 +384,7 @@ rutabaga_cmd_transfer_to_host_2d(VirtIOGPU *g,
> transfer.z = 0;
> transfer.w = t2d.r.width;
> transfer.h = t2d.r.height;
> + transfer.stride = t2d.r.width * 4;
>
here however, it's unclear to me what the stride could be, I think it could
depend on resource format (virgl doesn't set stride either).
Gurchetan?
> transfer.d = 1;
>
> result = rutabaga_resource_transfer_write(vr->rutabaga, 0,
> t2d.resource_id,
> --
> 2.45.0
>
>
>
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 3085 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] virtio-gpu: rutabaga: Properly set stride when copying resources
2024-06-10 7:44 ` Marc-André Lureau
@ 2024-06-14 23:50 ` Gurchetan Singh
0 siblings, 0 replies; 9+ messages in thread
From: Gurchetan Singh @ 2024-06-14 23:50 UTC (permalink / raw)
To: Marc-André Lureau
Cc: Weifeng Liu, qemu-devel, Michael S. Tsirkin, Gerd Hoffmann,
Antonio Caggiano, Huang Rui, Dmitry Osipenko
[-- Attachment #1: Type: text/plain, Size: 2570 bytes --]
On Mon, Jun 10, 2024 at 12:44 AM Marc-André Lureau <
marcandre.lureau@gmail.com> wrote:
> Hi
>
> On Wed, Jun 5, 2024 at 7:30 PM Weifeng Liu <weifeng.liu.z@gmail.com>
> wrote:
>
>> The stride is not correctly assigned when copying pixel data, causing
>> images being displayed incomplete when using 2d component of rutabaga.
>>
>> Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
>> ---
>> hw/display/virtio-gpu-rutabaga.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/hw/display/virtio-gpu-rutabaga.c
>> b/hw/display/virtio-gpu-rutabaga.c
>> index 17bf701a21..2ba6869606 100644
>> --- a/hw/display/virtio-gpu-rutabaga.c
>> +++ b/hw/display/virtio-gpu-rutabaga.c
>> @@ -53,6 +53,7 @@ virtio_gpu_rutabaga_update_cursor(VirtIOGPU *g, struct
>> virtio_gpu_scanout *s,
>> transfer.z = 0;
>> transfer.w = res->width;
>> transfer.h = res->height;
>> + transfer.stride = res->width * 4;
>>
>
> ok, stride defined by QEMUCursor layout
>
>
>> transfer.d = 1;
>>
>> transfer_iovec.iov_base = s->current_cursor->data;
>> @@ -273,6 +274,7 @@ rutabaga_cmd_resource_flush(VirtIOGPU *g, struct
>> virtio_gpu_ctrl_command *cmd)
>> transfer.z = 0;
>> transfer.w = res->width;
>> transfer.h = res->height;
>> + transfer.stride = pixman_image_get_stride(res->image);
>> transfer.d = 1;
>>
>
> ok (destination image stride)
>
>
>> transfer_iovec.iov_base = pixman_image_get_data(res->image);
>> @@ -382,6 +384,7 @@ rutabaga_cmd_transfer_to_host_2d(VirtIOGPU *g,
>> transfer.z = 0;
>> transfer.w = t2d.r.width;
>> transfer.h = t2d.r.height;
>> + transfer.stride = t2d.r.width * 4;
>>
>
> here however, it's unclear to me what the stride could be, I think it
> could depend on resource format (virgl doesn't set stride either).
>
> Gurchetan?
>
gfxstream does more or less the same thing internally to deal with the lack
of stride. Since virtio-gpu KMS only supports 4-bpp formats, this is fine,
so r-b for this particular patch.
That said, we actually don't use virtio-gpu-rutabaga for 2D mode or VirGL
mode, preferring to use the other more supported modes in QEMU for those
use cases. I think there's actually some CI support for those in many
places. So suggest just waiting until virglrenderer uprev lands in QEMU.
>
>
>
>> transfer.d = 1;
>>
>> result = rutabaga_resource_transfer_write(vr->rutabaga, 0,
>> t2d.resource_id,
>> --
>> 2.45.0
>>
>>
>>
>
> --
> Marc-André Lureau
>
[-- Attachment #2: Type: text/html, Size: 4285 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] virtio-gpu: rutabaga: Poll rutabaga upon events
2024-06-05 15:28 [PATCH 0/3] virtio-gpu: Enable virglrenderer backend for rutabaga Weifeng Liu
2024-06-05 15:28 ` [PATCH 1/3] virtio-gpu: rutabaga: Properly set stride when copying resources Weifeng Liu
@ 2024-06-05 15:28 ` Weifeng Liu
2024-06-05 15:28 ` [PATCH 3/3] virtio-gpu: rutabaga: Add options to enable virgl and venus contexts Weifeng Liu
2024-06-06 10:43 ` [PATCH 0/3] virtio-gpu: Enable virglrenderer backend for rutabaga Alex Bennée
3 siblings, 0 replies; 9+ messages in thread
From: Weifeng Liu @ 2024-06-05 15:28 UTC (permalink / raw)
To: qemu-devel, Gurchetan Singh, Michael S. Tsirkin, Gerd Hoffmann
Cc: Weifeng Liu, Antonio Caggiano, Huang Rui, Dmitry Osipenko,
Marc-André Lureau
To make virglrenderer work properly, we must poll it periodically or
when the event fd is readable, but this functionality is absent now.
This change registers the event fd as a gsource to the main loop and
attaches polling function as callback to the gsource.
Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
---
hw/display/virtio-gpu-rutabaga.c | 97 +++++++++++++++++++++++++++++++-
include/hw/virtio/virtio-gpu.h | 1 +
2 files changed, 97 insertions(+), 1 deletion(-)
diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
index 2ba6869606..ecb146315a 100644
--- a/hw/display/virtio-gpu-rutabaga.c
+++ b/hw/display/virtio-gpu-rutabaga.c
@@ -899,6 +899,93 @@ virtio_gpu_rutabaga_aio_cb(void *opaque)
g_free(data);
}
+
+static void
+virtio_gpu_fence_poll(void *opaque)
+{
+ VirtIOGPU *g = opaque;
+ VirtIOGPURutabaga *vr = VIRTIO_GPU_RUTABAGA(g);
+
+ rutabaga_poll(vr->rutabaga);
+}
+
+typedef void (*vu_watch_cb) (VirtIOGPURutabaga *dev, int condition, void *data);
+
+static void
+event_poll_cb(VirtIOGPURutabaga *dev, int condition, void *data)
+{
+ virtio_gpu_fence_poll(data);
+}
+
+typedef struct VrSrc {
+ GSource parent;
+ VirtIOGPURutabaga *dev;
+ GPollFD gfd;
+} VrSrc;
+
+static gboolean
+vr_src_prepare(GSource *gsrc, gint *timeout)
+{
+ g_assert(timeout);
+
+ *timeout = -1;
+ return FALSE;
+}
+
+static gboolean
+vr_src_check(GSource *gsrc)
+{
+ VrSrc *src = (VrSrc *)gsrc;
+
+ g_assert(src);
+
+ return src->gfd.revents & src->gfd.events;
+}
+
+static gboolean
+vr_src_dispatch(GSource *gsrc, GSourceFunc cb, gpointer data)
+{
+ VrSrc *src = (VrSrc *)gsrc;
+
+ g_assert(src);
+
+ ((vu_watch_cb)cb)(src->dev, src->gfd.revents, data);
+
+ return G_SOURCE_CONTINUE;
+}
+
+static GSourceFuncs vug_src_funcs = {
+ vr_src_prepare,
+ vr_src_check,
+ vr_src_dispatch,
+ NULL
+};
+
+static GSource *
+vr_source_new(VirtIOGPURutabaga *dev, int fd, GIOCondition cond,
+ vu_watch_cb vu_cb, gpointer data)
+{
+ GSource *gsrc;
+ VrSrc *src;
+ guint id;
+
+ g_assert(fd >= 0);
+ g_assert(vu_cb);
+
+ gsrc = g_source_new(&vug_src_funcs, sizeof(VrSrc));
+ g_source_set_callback(gsrc, (GSourceFunc)vu_cb, data, NULL);
+ src = (VrSrc *)gsrc;
+ src->dev = dev;
+ src->gfd.fd = fd;
+ src->gfd.events = cond;
+
+ g_source_add_poll(gsrc, &src->gfd);
+ id = g_source_attach(gsrc, g_main_context_get_thread_default());
+ g_assert(id);
+
+ return gsrc;
+}
+
static void
virtio_gpu_rutabaga_fence_cb(uint64_t user_data,
const struct rutabaga_fence *fence)
@@ -954,6 +1041,7 @@ virtio_gpu_rutabaga_debug_cb(uint64_t user_data,
static bool virtio_gpu_rutabaga_init(VirtIOGPU *g, Error **errp)
{
int result;
+ int poll_descriptor;
struct rutabaga_builder builder = { 0 };
struct rutabaga_channel channel = { 0 };
struct rutabaga_channels channels = { 0 };
@@ -1031,7 +1119,14 @@ static bool virtio_gpu_rutabaga_init(VirtIOGPU *g, Error **errp)
error_setg_errno(errp, -result, "Failed to init rutabaga");
return false;
}
-
+ result = rutabaga_poll_descriptor(vr->rutabaga, &poll_descriptor);
+ if (result) {
+ error_setg_errno(errp, -result, "Failed to get rutabaga poll descriptor");
+ return false;
+ }
+ if (poll_descriptor >= 0)
+ vr->poll_event_source = vr_source_new(vr, poll_descriptor, G_IO_IN,
+ event_poll_cb, g);
return true;
}
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 7a59379f5a..47a4347a9e 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -261,6 +261,7 @@ struct VirtIOGPURutabaga {
char *wsi;
bool headless;
uint32_t num_capsets;
+ GSource *poll_event_source;
struct rutabaga *rutabaga;
};
--
2.45.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/3] virtio-gpu: rutabaga: Add options to enable virgl and venus contexts
2024-06-05 15:28 [PATCH 0/3] virtio-gpu: Enable virglrenderer backend for rutabaga Weifeng Liu
2024-06-05 15:28 ` [PATCH 1/3] virtio-gpu: rutabaga: Properly set stride when copying resources Weifeng Liu
2024-06-05 15:28 ` [PATCH 2/3] virtio-gpu: rutabaga: Poll rutabaga upon events Weifeng Liu
@ 2024-06-05 15:28 ` Weifeng Liu
2024-06-06 10:43 ` [PATCH 0/3] virtio-gpu: Enable virglrenderer backend for rutabaga Alex Bennée
3 siblings, 0 replies; 9+ messages in thread
From: Weifeng Liu @ 2024-06-05 15:28 UTC (permalink / raw)
To: qemu-devel, Gurchetan Singh, Michael S. Tsirkin, Gerd Hoffmann
Cc: Weifeng Liu, Antonio Caggiano, Huang Rui, Dmitry Osipenko,
Marc-André Lureau
With this change, people will be able to use parameter like the one
below to add start virglrenderer backed virtio-gpu-rutabaga device:
-device virtio-vga-rutabaga,venus=on,virgl2=on,wsi=surfaceless
Performance being suboptimal though, this would be the first step.
Signed-off-by: Weifeng Liu <weifeng.liu.z@gmail.com>
---
hw/display/virtio-gpu-rutabaga.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
index ecb146315a..85bc33af8c 100644
--- a/hw/display/virtio-gpu-rutabaga.c
+++ b/hw/display/virtio-gpu-rutabaga.c
@@ -1203,6 +1203,10 @@ static Property virtio_gpu_rutabaga_properties[] = {
RUTABAGA_CAPSET_GFXSTREAM_GLES, false),
DEFINE_PROP_BIT64("x-gfxstream-composer", VirtIOGPURutabaga, capset_mask,
RUTABAGA_CAPSET_GFXSTREAM_COMPOSER, false),
+ DEFINE_PROP_BIT64("venus", VirtIOGPURutabaga, capset_mask,
+ RUTABAGA_CAPSET_VENUS, false),
+ DEFINE_PROP_BIT64("virgl2", VirtIOGPURutabaga, capset_mask,
+ RUTABAGA_CAPSET_VIRGL2, false),
DEFINE_PROP_STRING("wayland-socket-path", VirtIOGPURutabaga,
wayland_socket_path),
DEFINE_PROP_STRING("wsi", VirtIOGPURutabaga, wsi),
--
2.45.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 0/3] virtio-gpu: Enable virglrenderer backend for rutabaga
2024-06-05 15:28 [PATCH 0/3] virtio-gpu: Enable virglrenderer backend for rutabaga Weifeng Liu
` (2 preceding siblings ...)
2024-06-05 15:28 ` [PATCH 3/3] virtio-gpu: rutabaga: Add options to enable virgl and venus contexts Weifeng Liu
@ 2024-06-06 10:43 ` Alex Bennée
2024-06-07 6:47 ` Weifeng Liu
3 siblings, 1 reply; 9+ messages in thread
From: Alex Bennée @ 2024-06-06 10:43 UTC (permalink / raw)
To: Weifeng Liu
Cc: qemu-devel, Gurchetan Singh, Michael S. Tsirkin, Gerd Hoffmann,
Antonio Caggiano, Huang Rui, Dmitry Osipenko,
Marc-André Lureau
Weifeng Liu <weifeng.liu.z@gmail.com> writes:
> Greetings,
>
> I'd like to introduce you my attempt to enable virglrenderer backend for
> rutabaga empowered virtio-gpu device. I am aware that there have been
> effort in supporting venus in virtio-gpu-virgl.c [1], but there is no
> reason to prevent us from leveraging the virglrenderer component in
> rutabaga_gfx, especially it being not very hard to add this
> functionality.
>
> Generally, the gap is the polling capability, i.e., virglrenderer
> requires the main thread (namely the GPU command handling thread) to
> poll virglrenderer at proper moments, which is not yet supported in
> virtio-gpu-rutabaga device. This patch set try to add this so that
> virglrenderer backend (including virgl and venus) can work as expected.
>
> Slight change to rutabaga_gfx_ffi is also a requirement, which is
> included in [2].
>
> Further effort is required to tune the performance, since copying is
> present before the rendered images get displayed. But I still think this
> patch set could be a good starting point for the pending work.
>
> For those interested in setting up environment and playing around with
> this patch set, here is guideline in brief:
>
> 1. Clone the master/main branch of virglrenderer, compile and install it.
>
> git clone https://gitlab.freedesktop.org/virgl/virglrenderer
> cd virglrenderer
> meson setup builddir \
> --prefix=$INSTALL_DIR/virglrenderer \
> -Dvenus=true
> ninja -C builddir install
>
> 2. Clone the patched CrosVM, build and install rutabaga_gfx_ffi.
>
> git clone -b rutabaga_ffi_virgl https://github.com/phreer/crosvm.git
> cd crosvm/rutabaga_gfx/ffi
> export PKG_CONFIG_PATH=$INSTALL_DIR/virglrenderer/lib64/pkgconfig/
> meson setup builddir/ \
> --prefix $HOME/install/rutabaga_gfx/rutabaga_gfx_ffi/ \
> -Dvirglrenderer=true
> ninja -C builddir install
Is there a PR going in for this? The moving parts for rutabaga are
complex enough I think we need support upstream before merging this.
Is this branch where I should be getting the poll helpers from?
cc -m64 @qemu-system-arm.rsp
/usr/bin/ld: libcommon.fa.p/hw_display_virtio-gpu-rutabaga.c.o: in function `virtio_gpu_fence_poll':
/home/alex/lsrc/qemu.git/builds/vulkan/../../hw/display/virtio-gpu-rutabaga.c:909: undefined reference to `rutabaga_poll'
/usr/bin/ld: libcommon.fa.p/hw_display_virtio-gpu-rutabaga.c.o: in function `virtio_gpu_rutabaga_init':
/home/alex/lsrc/qemu.git/builds/vulkan/../../hw/display/virtio-gpu-rutabaga.c:1122: undefined reference to `rutabaga_poll_descriptor'
collect2: error: ld returned 1 exit status
ninja: build stopped: subcommand failed.
> 3. Applied this patch set to QEMU, build and install it:
>
> cd qemu
> # Apply this patch set atop main branch ...
> mkdir builddir; cd builddir
> ../configure --prefix=$INSTALL_DIR/qemu \
> --target-list=x86_64-softmmu \
> --disable-virglrenderer \
> --enable-rutabaga_gfx
> ninja -C builddir install
>
> 4. If you are lucky and everything goes fine, you are prepared to launch
> VM with virglrenderer backed virtio-gpu-rutabaga device:
>
> export LD_LIBRARY_PATH=$INSTALL_DIR/virglrenderer/lib64/:$LD_LIBRARY_PATH
> export LD_LIBRARY_PATH=$INSTALL_DIR/rutabaga_gfx_ffi/lib64/:$LD_LIBRARY_PATH
> $INSTALL_DIR/qemu/bin/qemu-system-x86_64
> $QEMU -d guest_errors -enable-kvm -M q35 -smp 4 -m $MEM \
> -object memory-backend-memfd,id=mem1,size=$MEM \
> -machine memory-backend=mem1 \
> -device virtio-vga-rutabaga,venus=on,virgl2=on,wsi=surfaceless,hostmem=$MEM \
>
This should go into docs/system/devices/virtio-gpu.rst with some
explanation. Is there anything we need on the guest side or does this
skip the encapsulating requirements of wayland?
> Note:
>
> - You might need this patch set [3] to avoid KVM bad address error when
> you are running on a GPU using TTM for memory management.
>
> [1] https://lore.kernel.org/all/dba6eb97-e1d1-4694-bfb6-e72db95715dd@daynix.com/T/
> [2] https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5599645/1
> [3] https://lore.kernel.org/kvm/20240229025759.1187910-1-stevensd@google.com/
>
> Weifeng Liu (3):
> virtio-gpu: rutabaga: Properly set stride when copying resources
> virtio-gpu: rutabaga: Poll rutabaga upon events
> virtio-gpu: rutabaga: Add options to enable virgl and venus contexts
>
> hw/display/virtio-gpu-rutabaga.c | 104 ++++++++++++++++++++++++++++++-
> include/hw/virtio/virtio-gpu.h | 1 +
> 2 files changed, 104 insertions(+), 1 deletion(-)
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 0/3] virtio-gpu: Enable virglrenderer backend for rutabaga
2024-06-06 10:43 ` [PATCH 0/3] virtio-gpu: Enable virglrenderer backend for rutabaga Alex Bennée
@ 2024-06-07 6:47 ` Weifeng Liu
2024-06-07 11:10 ` Alex Bennée
0 siblings, 1 reply; 9+ messages in thread
From: Weifeng Liu @ 2024-06-07 6:47 UTC (permalink / raw)
To: Alex Bennée
Cc: qemu-devel, Gurchetan Singh, Michael S. Tsirkin, Gerd Hoffmann,
Antonio Caggiano, Huang Rui, Dmitry Osipenko,
Marc-André Lureau
Hi Alex,
On Thu, 2024-06-06 at 11:43 +0100, Alex Bennée wrote:
> Weifeng Liu <weifeng.liu.z@gmail.com> writes:
>
> > Greetings,
> >
> > I'd like to introduce you my attempt to enable virglrenderer backend for
> > rutabaga empowered virtio-gpu device. I am aware that there have been
> > effort in supporting venus in virtio-gpu-virgl.c [1], but there is no
> > reason to prevent us from leveraging the virglrenderer component in
> > rutabaga_gfx, especially it being not very hard to add this
> > functionality.
> >
> > Generally, the gap is the polling capability, i.e., virglrenderer
> > requires the main thread (namely the GPU command handling thread) to
> > poll virglrenderer at proper moments, which is not yet supported in
> > virtio-gpu-rutabaga device. This patch set try to add this so that
> > virglrenderer backend (including virgl and venus) can work as expected.
> >
> > Slight change to rutabaga_gfx_ffi is also a requirement, which is
> > included in [2].
> >
> > Further effort is required to tune the performance, since copying is
> > present before the rendered images get displayed. But I still think this
> > patch set could be a good starting point for the pending work.
> >
> > For those interested in setting up environment and playing around with
> > this patch set, here is guideline in brief:
> >
> > 1. Clone the master/main branch of virglrenderer, compile and install it.
> >
> > git clone https://gitlab.freedesktop.org/virgl/virglrenderer
> > cd virglrenderer
> > meson setup builddir \
> > --prefix=$INSTALL_DIR/virglrenderer \
> > -Dvenus=true
> > ninja -C builddir install
> >
> > 2. Clone the patched CrosVM, build and install rutabaga_gfx_ffi.
> >
> > git clone -b rutabaga_ffi_virgl https://github.com/phreer/crosvm.git
> > cd crosvm/rutabaga_gfx/ffi
> > export PKG_CONFIG_PATH=$INSTALL_DIR/virglrenderer/lib64/pkgconfig/
> > meson setup builddir/ \
> > --prefix $HOME/install/rutabaga_gfx/rutabaga_gfx_ffi/ \
> > -Dvirglrenderer=true
> > ninja -C builddir install
>
> Is there a PR going in for this? The moving parts for rutabaga are
> complex enough I think we need support upstream before merging this.
>
It's true that this patch set depends on the change of
rutabaga_gfx_ffi. I am trying get the modifications of
crosvm/rubataga_gfx_ffi merged in upstream, please refer to this link:
https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5599645
> Is this branch where I should be getting the poll helpers from?
>
> cc -m64 @qemu-system-arm.rsp
> /usr/bin/ld: libcommon.fa.p/hw_display_virtio-gpu-rutabaga.c.o: in function `virtio_gpu_fence_poll':
> /home/alex/lsrc/qemu.git/builds/vulkan/../../hw/display/virtio-gpu-rutabaga.c:909: undefined reference to `rutabaga_poll'
> /usr/bin/ld: libcommon.fa.p/hw_display_virtio-gpu-rutabaga.c.o: in function `virtio_gpu_rutabaga_init':
> /home/alex/lsrc/qemu.git/builds/vulkan/../../hw/display/virtio-gpu-rutabaga.c:1122: undefined reference to `rutabaga_poll_descriptor'
> collect2: error: ld returned 1 exit status
> ninja: build stopped: subcommand failed.
>
The required patches are applied to the rutabaga_ffi_virgl branch of my
clone of crosvm already, so please check out to that branch.
>
> > 3. Applied this patch set to QEMU, build and install it:
> >
> > cd qemu
> > # Apply this patch set atop main branch ...
> > mkdir builddir; cd builddir
> > ../configure --prefix=$INSTALL_DIR/qemu \
> > --target-list=x86_64-softmmu \
> > --disable-virglrenderer \
> > --enable-rutabaga_gfx
> > ninja -C builddir install
> >
> > 4. If you are lucky and everything goes fine, you are prepared to launch
> > VM with virglrenderer backed virtio-gpu-rutabaga device:
> >
> > export LD_LIBRARY_PATH=$INSTALL_DIR/virglrenderer/lib64/:$LD_LIBRARY_PATH
> > export LD_LIBRARY_PATH=$INSTALL_DIR/rutabaga_gfx_ffi/lib64/:$LD_LIBRARY_PATH
> > $INSTALL_DIR/qemu/bin/qemu-system-x86_64
> > $QEMU -d guest_errors -enable-kvm -M q35 -smp 4 -m $MEM \
> > -object memory-backend-memfd,id=mem1,size=$MEM \
> > -machine memory-backend=mem1 \
> > -device virtio-vga-rutabaga,venus=on,virgl2=on,wsi=surfaceless,hostmem=$MEM \
> >
>
> This should go into docs/system/devices/virtio-gpu.rst with some
> explanation. Is there anything we need on the guest side or does this
> skip the encapsulating requirements of wayland?
>
Yeah, it's a good idea to add doc to explain the usage, thanks!
Best regards,
Weifeng
> > Note:
> >
> > - You might need this patch set [3] to avoid KVM bad address error when
> > you are running on a GPU using TTM for memory management.
> >
> > [1] https://lore.kernel.org/all/dba6eb97-e1d1-4694-bfb6-e72db95715dd@daynix.com/T/
> > [2] https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5599645/1
> > [3] https://lore.kernel.org/kvm/20240229025759.1187910-1-stevensd@google.com/
> >
> > Weifeng Liu (3):
> > virtio-gpu: rutabaga: Properly set stride when copying resources
> > virtio-gpu: rutabaga: Poll rutabaga upon events
> > virtio-gpu: rutabaga: Add options to enable virgl and venus contexts
> >
> > hw/display/virtio-gpu-rutabaga.c | 104 ++++++++++++++++++++++++++++++-
> > include/hw/virtio/virtio-gpu.h | 1 +
> > 2 files changed, 104 insertions(+), 1 deletion(-)
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] virtio-gpu: Enable virglrenderer backend for rutabaga
2024-06-07 6:47 ` Weifeng Liu
@ 2024-06-07 11:10 ` Alex Bennée
0 siblings, 0 replies; 9+ messages in thread
From: Alex Bennée @ 2024-06-07 11:10 UTC (permalink / raw)
To: Weifeng Liu
Cc: qemu-devel, Gurchetan Singh, Michael S. Tsirkin, Gerd Hoffmann,
Antonio Caggiano, Huang Rui, Dmitry Osipenko,
Marc-André Lureau
Weifeng Liu <weifeng.liu.z@gmail.com> writes:
> Hi Alex,
>
> On Thu, 2024-06-06 at 11:43 +0100, Alex Bennée wrote:
>> Weifeng Liu <weifeng.liu.z@gmail.com> writes:
>>
>> > Greetings,
>> >
>> > I'd like to introduce you my attempt to enable virglrenderer backend for
>> > rutabaga empowered virtio-gpu device. I am aware that there have been
>> > effort in supporting venus in virtio-gpu-virgl.c [1], but there is no
>> > reason to prevent us from leveraging the virglrenderer component in
>> > rutabaga_gfx, especially it being not very hard to add this
>> > functionality.
>> >
>> > Generally, the gap is the polling capability, i.e., virglrenderer
>> > requires the main thread (namely the GPU command handling thread) to
>> > poll virglrenderer at proper moments, which is not yet supported in
>> > virtio-gpu-rutabaga device. This patch set try to add this so that
>> > virglrenderer backend (including virgl and venus) can work as expected.
>> >
>> > Slight change to rutabaga_gfx_ffi is also a requirement, which is
>> > included in [2].
>> >
>> > Further effort is required to tune the performance, since copying is
>> > present before the rendered images get displayed. But I still think this
>> > patch set could be a good starting point for the pending work.
>> >
>> > For those interested in setting up environment and playing around with
>> > this patch set, here is guideline in brief:
>> >
>> > 1. Clone the master/main branch of virglrenderer, compile and install it.
>> >
>> > git clone https://gitlab.freedesktop.org/virgl/virglrenderer
>> > cd virglrenderer
>> > meson setup builddir \
>> > --prefix=$INSTALL_DIR/virglrenderer \
>> > -Dvenus=true
>> > ninja -C builddir install
>> >
>> > 2. Clone the patched CrosVM, build and install rutabaga_gfx_ffi.
>> >
>> > git clone -b rutabaga_ffi_virgl https://github.com/phreer/crosvm.git
>> > cd crosvm/rutabaga_gfx/ffi
>> > export PKG_CONFIG_PATH=$INSTALL_DIR/virglrenderer/lib64/pkgconfig/
>> > meson setup builddir/ \
>> > --prefix $HOME/install/rutabaga_gfx/rutabaga_gfx_ffi/ \
>> > -Dvirglrenderer=true
>> > ninja -C builddir install
>>
>> Is there a PR going in for this? The moving parts for rutabaga are
>> complex enough I think we need support upstream before merging this.
>>
>
> It's true that this patch set depends on the change of
> rutabaga_gfx_ffi. I am trying get the modifications of
> crosvm/rubataga_gfx_ffi merged in upstream, please refer to this link:
> https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5599645
>
>> Is this branch where I should be getting the poll helpers from?
>>
>> cc -m64 @qemu-system-arm.rsp
>> /usr/bin/ld: libcommon.fa.p/hw_display_virtio-gpu-rutabaga.c.o: in function `virtio_gpu_fence_poll':
>> /home/alex/lsrc/qemu.git/builds/vulkan/../../hw/display/virtio-gpu-rutabaga.c:909: undefined reference to `rutabaga_poll'
>> /usr/bin/ld: libcommon.fa.p/hw_display_virtio-gpu-rutabaga.c.o: in function `virtio_gpu_rutabaga_init':
>> /home/alex/lsrc/qemu.git/builds/vulkan/../../hw/display/virtio-gpu-rutabaga.c:1122: undefined reference to `rutabaga_poll_descriptor'
>> collect2: error: ld returned 1 exit status
>> ninja: build stopped: subcommand failed.
>>
>
> The required patches are applied to the rutabaga_ffi_virgl branch of my
> clone of crosvm already, so please check out to that branch.
I thought I had - maybe my install got polluted. I shall try again.
>
>>
>> > 3. Applied this patch set to QEMU, build and install it:
>> >
>> > cd qemu
>> > # Apply this patch set atop main branch ...
>> > mkdir builddir; cd builddir
>> > ../configure --prefix=$INSTALL_DIR/qemu \
>> > --target-list=x86_64-softmmu \
>> > --disable-virglrenderer \
>> > --enable-rutabaga_gfx
>> > ninja -C builddir install
>> >
>> > 4. If you are lucky and everything goes fine, you are prepared to launch
>> > VM with virglrenderer backed virtio-gpu-rutabaga device:
>> >
>> > export LD_LIBRARY_PATH=$INSTALL_DIR/virglrenderer/lib64/:$LD_LIBRARY_PATH
>> > export LD_LIBRARY_PATH=$INSTALL_DIR/rutabaga_gfx_ffi/lib64/:$LD_LIBRARY_PATH
>> > $INSTALL_DIR/qemu/bin/qemu-system-x86_64
>> > $QEMU -d guest_errors -enable-kvm -M q35 -smp 4 -m $MEM \
>> > -object memory-backend-memfd,id=mem1,size=$MEM \
>> > -machine memory-backend=mem1 \
>> > -device virtio-vga-rutabaga,venus=on,virgl2=on,wsi=surfaceless,hostmem=$MEM \
>> >
>>
>> This should go into docs/system/devices/virtio-gpu.rst with some
>> explanation. Is there anything we need on the guest side or does this
>> skip the encapsulating requirements of wayland?
>>
>
> Yeah, it's a good idea to add doc to explain the usage, thanks!
>
> Best regards,
> Weifeng
>
>> > Note:
>> >
>> > - You might need this patch set [3] to avoid KVM bad address error when
>> > you are running on a GPU using TTM for memory management.
>> >
>> > [1] https://lore.kernel.org/all/dba6eb97-e1d1-4694-bfb6-e72db95715dd@daynix.com/T/
>> > [2] https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5599645/1
>> > [3] https://lore.kernel.org/kvm/20240229025759.1187910-1-stevensd@google.com/
>> >
>> > Weifeng Liu (3):
>> > virtio-gpu: rutabaga: Properly set stride when copying resources
>> > virtio-gpu: rutabaga: Poll rutabaga upon events
>> > virtio-gpu: rutabaga: Add options to enable virgl and venus contexts
>> >
>> > hw/display/virtio-gpu-rutabaga.c | 104 ++++++++++++++++++++++++++++++-
>> > include/hw/virtio/virtio-gpu.h | 1 +
>> > 2 files changed, 104 insertions(+), 1 deletion(-)
>>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 9+ messages in thread