* [PATCH] hw/display: fix virgl reset regression
@ 2021-07-02 12:32 marcandre.lureau
2021-07-22 12:11 ` Gerd Hoffmann
0 siblings, 1 reply; 4+ messages in thread
From: marcandre.lureau @ 2021-07-02 12:32 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, kraxel
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Before commit 49afbca3b00e8e517d54964229a794b51768deaf ("virtio-gpu: drop
use_virgl_renderer"), use_virgl_renderer was preventing calling GL
functions from non-GL context threads. The innocuously looking
g->parent_obj.use_virgl_renderer = false;
was set the first time virtio_gpu_gl_reset() was called, during
pc_machine_reset() in the main thread. Further virtio_gpu_gl_reset()
calls in IO threads, without associated GL context, were thus skipping
GL calls and avoided warnings or crashes (see also
https://gitlab.freedesktop.org/virgl/virglrenderer/-/issues/226).
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
include/hw/virtio/virtio-gpu.h | 1 +
hw/display/virtio-gpu-gl.c | 22 +++++++++++-----------
hw/display/virtio-gpu-virgl.c | 8 ++++++--
3 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index bcf54d970f..24c6628944 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -279,6 +279,7 @@ int virtio_gpu_update_dmabuf(VirtIOGPU *g,
void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd);
void virtio_gpu_virgl_fence_poll(VirtIOGPU *g);
+void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g);
void virtio_gpu_virgl_reset(VirtIOGPU *g);
int virtio_gpu_virgl_init(VirtIOGPU *g);
int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g);
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index c973d4824b..2bf5e568f1 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -53,12 +53,7 @@ static void virtio_gpu_gl_update_cursor_data(VirtIOGPU *g,
static void virtio_gpu_gl_flushed(VirtIOGPUBase *b)
{
VirtIOGPU *g = VIRTIO_GPU(b);
- VirtIOGPUGL *gl = VIRTIO_GPU_GL(b);
- if (gl->renderer_reset) {
- gl->renderer_reset = false;
- virtio_gpu_virgl_reset(g);
- }
virtio_gpu_process_cmdq(g);
}
@@ -76,6 +71,10 @@ static void virtio_gpu_gl_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
virtio_gpu_virgl_init(g);
gl->renderer_inited = true;
}
+ if (gl->renderer_reset) {
+ gl->renderer_reset = false;
+ virtio_gpu_virgl_reset(g);
+ }
cmd = virtqueue_pop(vq, sizeof(struct virtio_gpu_ctrl_command));
while (cmd) {
@@ -97,12 +96,13 @@ static void virtio_gpu_gl_reset(VirtIODevice *vdev)
virtio_gpu_reset(vdev);
- if (gl->renderer_inited) {
- if (g->parent_obj.renderer_blocked) {
- gl->renderer_reset = true;
- } else {
- virtio_gpu_virgl_reset(g);
- }
+ /*
+ * GL functions must be called with the associated GL context in main
+ * thread, and when the renderer is unblocked.
+ */
+ if (gl->renderer_inited && !gl->renderer_reset) {
+ virtio_gpu_virgl_reset_scanout(g);
+ gl->renderer_reset = true;
}
}
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 46b56f94d9..0d87de65d7 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -588,17 +588,21 @@ void virtio_gpu_virgl_fence_poll(VirtIOGPU *g)
virtio_gpu_fence_poll(g);
}
-void virtio_gpu_virgl_reset(VirtIOGPU *g)
+void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g)
{
int i;
- virgl_renderer_reset();
for (i = 0; i < g->parent_obj.conf.max_outputs; i++) {
dpy_gfx_replace_surface(g->parent_obj.scanout[i].con, NULL);
dpy_gl_scanout_disable(g->parent_obj.scanout[i].con);
}
}
+void virtio_gpu_virgl_reset(VirtIOGPU *g)
+{
+ virgl_renderer_reset();
+}
+
int virtio_gpu_virgl_init(VirtIOGPU *g)
{
int ret;
--
2.29.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] hw/display: fix virgl reset regression
2021-07-02 12:32 [PATCH] hw/display: fix virgl reset regression marcandre.lureau
@ 2021-07-22 12:11 ` Gerd Hoffmann
2021-07-22 12:51 ` Marc-André Lureau
0 siblings, 1 reply; 4+ messages in thread
From: Gerd Hoffmann @ 2021-07-22 12:11 UTC (permalink / raw)
To: marcandre.lureau; +Cc: qemu-devel
On Fri, Jul 02, 2021 at 04:32:21PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Before commit 49afbca3b00e8e517d54964229a794b51768deaf ("virtio-gpu: drop
> use_virgl_renderer"), use_virgl_renderer was preventing calling GL
> functions from non-GL context threads. The innocuously looking
>
> g->parent_obj.use_virgl_renderer = false;
>
> was set the first time virtio_gpu_gl_reset() was called, during
> pc_machine_reset() in the main thread. Further virtio_gpu_gl_reset()
> calls in IO threads, without associated GL context, were thus skipping
> GL calls and avoided warnings or crashes (see also
> https://gitlab.freedesktop.org/virgl/virglrenderer/-/issues/226).
Conflicts with patch by Akihiko Odaki fixing the same thing or a
related issue:
virtio-gpu: Call Virgl only in the main thread
https://patchwork.ozlabs.org/project/qemu-devel/patch/20210617113520.25973-1-akihiko.odaki@gmail.com/
Can you have a look please and suggest how to handle this?
thanks,
Gerd
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] hw/display: fix virgl reset regression
2021-07-22 12:11 ` Gerd Hoffmann
@ 2021-07-22 12:51 ` Marc-André Lureau
2021-07-22 13:16 ` Marc-André Lureau
0 siblings, 1 reply; 4+ messages in thread
From: Marc-André Lureau @ 2021-07-22 12:51 UTC (permalink / raw)
To: Gerd Hoffmann, Akihiko Odaki; +Cc: QEMU
[-- Attachment #1: Type: text/plain, Size: 2020 bytes --]
Hi
On Thu, Jul 22, 2021 at 4:12 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> On Fri, Jul 02, 2021 at 04:32:21PM +0400, marcandre.lureau@redhat.com
> wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Before commit 49afbca3b00e8e517d54964229a794b51768deaf ("virtio-gpu: drop
> > use_virgl_renderer"), use_virgl_renderer was preventing calling GL
> > functions from non-GL context threads. The innocuously looking
> >
> > g->parent_obj.use_virgl_renderer = false;
> >
> > was set the first time virtio_gpu_gl_reset() was called, during
> > pc_machine_reset() in the main thread. Further virtio_gpu_gl_reset()
> > calls in IO threads, without associated GL context, were thus skipping
> > GL calls and avoided warnings or crashes (see also
> > https://gitlab.freedesktop.org/virgl/virglrenderer/-/issues/226).
>
> Conflicts with patch by Akihiko Odaki fixing the same thing or a
> related issue:
>
> virtio-gpu: Call Virgl only in the main thread
>
>
> https://patchwork.ozlabs.org/project/qemu-devel/patch/20210617113520.25973-1-akihiko.odaki@gmail.com/
>
> Can you have a look please and suggest how to handle this?
>
Thanks, I didn't notice we were trying to solve the same issue.
Akihiko's patch indeed seems to solve the crash, but doesn't solve the
flood of asserts (on wayland):
qemu-system-x86_64: Gtk: gtk_gl_area_make_current: assertion
'gtk_widget_get_realized (widget)' failed
qemu-system-x86_64: Gdk: gdk_window_create_gl_context: assertion
'GDK_IS_WINDOW (window)' failed
qemu-system-x86_64: Gdk: gdk_gl_context_set_required_version: assertion
'GDK_IS_GL_CONTEXT (context)' failed
... and many more
My patch cleans it for me, I would suggest to take mine.
Fwiw, I just tested also on X11, and we have another regression that seems
unrelated:
qemu-system-x86_64: ../src/dispatch_common.c:858: epoxy_get_proc_address:
Assertion `0 && "Couldn't find current GLX or EGL context.\n"' failed.
sigh..
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 3100 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] hw/display: fix virgl reset regression
2021-07-22 12:51 ` Marc-André Lureau
@ 2021-07-22 13:16 ` Marc-André Lureau
0 siblings, 0 replies; 4+ messages in thread
From: Marc-André Lureau @ 2021-07-22 13:16 UTC (permalink / raw)
To: Gerd Hoffmann, Akihiko Odaki; +Cc: QEMU
[-- Attachment #1: Type: text/plain, Size: 2358 bytes --]
Hi
On Thu, Jul 22, 2021 at 4:51 PM Marc-André Lureau <
marcandre.lureau@gmail.com> wrote:
> Hi
>
> On Thu, Jul 22, 2021 at 4:12 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>> On Fri, Jul 02, 2021 at 04:32:21PM +0400, marcandre.lureau@redhat.com
>> wrote:
>> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
>> >
>> > Before commit 49afbca3b00e8e517d54964229a794b51768deaf ("virtio-gpu:
>> drop
>> > use_virgl_renderer"), use_virgl_renderer was preventing calling GL
>> > functions from non-GL context threads. The innocuously looking
>> >
>> > g->parent_obj.use_virgl_renderer = false;
>> >
>> > was set the first time virtio_gpu_gl_reset() was called, during
>> > pc_machine_reset() in the main thread. Further virtio_gpu_gl_reset()
>> > calls in IO threads, without associated GL context, were thus skipping
>> > GL calls and avoided warnings or crashes (see also
>> > https://gitlab.freedesktop.org/virgl/virglrenderer/-/issues/226).
>>
>> Conflicts with patch by Akihiko Odaki fixing the same thing or a
>> related issue:
>>
>> virtio-gpu: Call Virgl only in the main thread
>>
>>
>> https://patchwork.ozlabs.org/project/qemu-devel/patch/20210617113520.25973-1-akihiko.odaki@gmail.com/
>>
>> Can you have a look please and suggest how to handle this?
>>
>
> Thanks, I didn't notice we were trying to solve the same issue.
>
> Akihiko's patch indeed seems to solve the crash, but doesn't solve the
> flood of asserts (on wayland):
> qemu-system-x86_64: Gtk: gtk_gl_area_make_current: assertion
> 'gtk_widget_get_realized (widget)' failed
> qemu-system-x86_64: Gdk: gdk_window_create_gl_context: assertion
> 'GDK_IS_WINDOW (window)' failed
> qemu-system-x86_64: Gdk: gdk_gl_context_set_required_version: assertion
> 'GDK_IS_GL_CONTEXT (context)' failed
> ... and many more
>
> My patch cleans it for me, I would suggest to take mine.
>
> Fwiw, I just tested also on X11, and we have another regression that seems
> unrelated:
> qemu-system-x86_64: ../src/dispatch_common.c:858: epoxy_get_proc_address:
> Assertion `0 && "Couldn't find current GLX or EGL context.\n"' failed.
>
> sigh..
>
>
That assert is fixed with "vl: add virtio-vga-gl to the default_list" patch
(
https://patchew.org/QEMU/20210701062421.721414-1-marcandre.lureau@redhat.com/
).
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 3812 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-07-22 13:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-07-02 12:32 [PATCH] hw/display: fix virgl reset regression marcandre.lureau
2021-07-22 12:11 ` Gerd Hoffmann
2021-07-22 12:51 ` Marc-André Lureau
2021-07-22 13:16 ` Marc-André Lureau
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).