| Message ID | 20250506125715.232872-13-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Maintainer updates for May (testing, plugins, virtio-gpu) | expand |
On 2025/05/06 21:57, Alex Bennée wrote: > From: Dongwon Kim <dongwon.kim@intel.com> > > This partially reverts commit 77bf310084dad38b3a2badf01766c659056f1cf2 > which causes some guest display corruption when gtk-gl-area > is used for GTK rendering (e.g. Wayland Compositor) possibly due to > simulataneous accesses on the guest frame buffer by host compositor > and the guest. Simply reverting the part of the commit may re-introduce the problem the commit tried to solve, which will be a regression as the commit is already included in releases. I guess the problem is that the gl_block callback of GraphicHwOps is not properly implemented and it is what should be fixed. > > Fixes: 77bf310084 ("ui/gtk: Draw guest frame at refresh cycle") > Reported-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> > Reported-by: Alex Bennée <alex.bennee@linaro.org> > Tested-by: Alex Bennée <alex.bennee@linaro.org> > Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> > Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> > Cc: Marc-André Lureau <marcandre.lureau@redhat.com> > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> > Message-Id: <20250214170813.2234754-1-dongwon.kim@intel.com> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > ui/gtk-gl-area.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c > index 2c9a0db425..9f7dc697f2 100644 > --- a/ui/gtk-gl-area.c > +++ b/ui/gtk-gl-area.c > @@ -129,7 +129,6 @@ void gd_gl_area_refresh(DisplayChangeListener *dcl) > > if (vc->gfx.guest_fb.dmabuf && > qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) { > - gd_gl_area_draw(vc); > return; > } >
On 5/10/25 07:52, Akihiko Odaki wrote: > On 2025/05/06 21:57, Alex Bennée wrote: >> From: Dongwon Kim <dongwon.kim@intel.com> >> >> This partially reverts commit 77bf310084dad38b3a2badf01766c659056f1cf2 >> which causes some guest display corruption when gtk-gl-area >> is used for GTK rendering (e.g. Wayland Compositor) possibly due to >> simulataneous accesses on the guest frame buffer by host compositor >> and the guest. > > Simply reverting the part of the commit may re-introduce the problem the > commit tried to solve, which will be a regression as the commit is > already included in releases. > > I guess the problem is that the gl_block callback of GraphicHwOps is not > properly implemented and it is what should be fixed. The reverted commit made QEMU GTK GUI unusable under Wayland. It was fixing problem which requires very specific QEMU setup, while breaking generic setups. The offending change should be reverted as it introduced a bigger problem. A proper solution should be found, meanwhile QEMU GTK under Wayland should be restored, IMO. For the reference see [1]. First bug reports about a mirrored display problem were made to me on IRC a year ago and the root of the problem was identified only couple months ago. [1] https://lore.kernel.org/qemu-devel/5aedf1ad-d9b0-4edb-a050-f3d9bee9bccb@collabora.com/ As of today, the GTK problem isn't understood.
On 2025/05/10 21:12, Dmitry Osipenko wrote: > On 5/10/25 07:52, Akihiko Odaki wrote: >> On 2025/05/06 21:57, Alex Bennée wrote: >>> From: Dongwon Kim <dongwon.kim@intel.com> >>> >>> This partially reverts commit 77bf310084dad38b3a2badf01766c659056f1cf2 >>> which causes some guest display corruption when gtk-gl-area >>> is used for GTK rendering (e.g. Wayland Compositor) possibly due to >>> simulataneous accesses on the guest frame buffer by host compositor >>> and the guest. >> >> Simply reverting the part of the commit may re-introduce the problem the >> commit tried to solve, which will be a regression as the commit is >> already included in releases. >> >> I guess the problem is that the gl_block callback of GraphicHwOps is not >> properly implemented and it is what should be fixed. > > The reverted commit made QEMU GTK GUI unusable under Wayland. It was > fixing problem which requires very specific QEMU setup, while breaking > generic setups. The offending change should be reverted as it introduced > a bigger problem. A proper solution should be found, meanwhile QEMU GTK > under Wayland should be restored, IMO. > > For the reference see [1]. First bug reports about a mirrored display > problem were made to me on IRC a year ago and the root of the problem > was identified only couple months ago. > > [1] > https://lore.kernel.org/qemu-devel/5aedf1ad-d9b0-4edb-a050-f3d9bee9bccb@collabora.com/ That describes the context well. It should also be described with an inline comment so that we will not lose track of the known problem. > > As of today, the GTK problem isn't understood. > If your guess about simultaneous accesses on the guest frame buffer is correct, it's a bug of the emulated device, not GTK. Regards, Akihiko Odaki
> Subject: Re: [PATCH v2 12/14] ui/gtk-gl-area: Remove extra draw call in refresh > > On 2025/05/10 21:12, Dmitry Osipenko wrote: > > On 5/10/25 07:52, Akihiko Odaki wrote: > >> On 2025/05/06 21:57, Alex Bennée wrote: > >>> From: Dongwon Kim <dongwon.kim@intel.com> > >>> > >>> This partially reverts commit > >>> 77bf310084dad38b3a2badf01766c659056f1cf2 > >>> which causes some guest display corruption when gtk-gl-area is used > >>> for GTK rendering (e.g. Wayland Compositor) possibly due to > >>> simulataneous accesses on the guest frame buffer by host compositor > >>> and the guest. > >> > >> Simply reverting the part of the commit may re-introduce the problem > >> the commit tried to solve, which will be a regression as the commit > >> is already included in releases. > >> > >> I guess the problem is that the gl_block callback of GraphicHwOps is > >> not properly implemented and it is what should be fixed. > > > > The reverted commit made QEMU GTK GUI unusable under Wayland. It was > > fixing problem which requires very specific QEMU setup, while breaking > > generic setups. The offending change should be reverted as it > > introduced a bigger problem. A proper solution should be found, > > meanwhile QEMU GTK under Wayland should be restored, IMO. > > > > For the reference see [1]. First bug reports about a mirrored display > > problem were made to me on IRC a year ago and the root of the problem > > was identified only couple months ago. > > > > [1] > > https://lore.kernel.org/qemu-devel/5aedf1ad-d9b0-4edb-a050-f3d9bee9bcc > > b@collabora.com/ > > That describes the context well. It should also be described with an inline > comment so that we will not lose track of the known problem. [Kim, Dongwon] Do you mean we should have this info in the commit message? > > > > > As of today, the GTK problem isn't understood. > > > > If your guess about simultaneous accesses on the guest frame buffer is correct, > it's a bug of the emulated device, not GTK. [Kim, Dongwon] "Simultaneous accesses" was just a guess. This problem needs to be debugged further. BTW, the problem the original commit was trying to fix is some lockup due to suspended rendering on minimized or hidden GTK window. But according to Dmitry, this case wasn't hit in his setup with wayland compositor so I assume the risk of not having an extra draw in gtk-gl-area would be low. > > Regards, > Akihiko Odaki
diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c index 2c9a0db425..9f7dc697f2 100644 --- a/ui/gtk-gl-area.c +++ b/ui/gtk-gl-area.c @@ -129,7 +129,6 @@ void gd_gl_area_refresh(DisplayChangeListener *dcl) if (vc->gfx.guest_fb.dmabuf && qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) { - gd_gl_area_draw(vc); return; }