| Message ID | 1455873289-349-11-git-send-email-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fr, 2016-02-19 at 10:14 +0100, Gerd Hoffmann wrote: > Current spice client expects we create a primary surface, > even if we do display updates using dma-bufs exclusively. > > So just do that to get things going. > > Not fully clear whenever that is intentional or a bug on > the spice side, so I keep this as separate patch for now. > > I think this should either be squashed into the previous > patch, or dropped after fixing things on the spice side. Ping Marc? Any comment on this? thanks, Gerd
Hi
On Fri, Mar 18, 2016 at 2:17 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Ping Marc? Any comment on this?
Could you send a rebased series, for the patches that lead to the
issue? I assume "render DisplaySurface via opengl" patch?
On Fr, 2016-03-18 at 14:45 +0100, Marc-André Lureau wrote: > Hi > > On Fri, Mar 18, 2016 at 2:17 PM, Gerd Hoffmann <kraxel@redhat.com> wrote: > > Ping Marc? Any comment on this? > > > Could you send a rebased series, for the patches that lead to the > issue? Update pushed to https://www.kraxel.org/cgit/qemu/log/?h=work/virgl with some cherry-picks to fix nettle build issue and to sidestep conflict with in-flight pull request. That work better for testing. I'll resent the patch series once the temporary issues with master are settled. There hasn't changed much in the code though, other than rebasing and minor tweaks to adapt to the include changes. > I assume "render DisplaySurface via opengl" patch? Yes, that one. cheers, Gerd
Hi On Mon, Mar 21, 2016 at 11:24 AM, Gerd Hoffmann <kraxel@redhat.com> wrote: > On Fr, 2016-03-18 at 14:45 +0100, Marc-André Lureau wrote: >> Hi >> >> On Fri, Mar 18, 2016 at 2:17 PM, Gerd Hoffmann <kraxel@redhat.com> wrote: >> > Ping Marc? Any comment on this? >> >> >> Could you send a rebased series, for the patches that lead to the >> issue? > > Update pushed to https://www.kraxel.org/cgit/qemu/log/?h=work/virgl > with some cherry-picks to fix nettle build issue and to sidestep > conflict with in-flight pull request. > > That work better for testing. I'll resent the patch series once the > temporary issues with master are settled. There hasn't changed much in > the code though, other than rebasing and minor tweaks to adapt to the > include changes. > >> I assume "render DisplaySurface via opengl" patch? > > Yes, that one. I have done some more testing and sent a series for spice-gtk fixing display with gl scanout-only case. And a minor patch to spice server to solve a cursor initialization when there is no canvas. Your series works ok with that, only when doing a reboot, virtio_gpu_virgl_reset() calls spice_qxl_gl_scanout( fd = -1), and later spice_gl_refresh() calls spice_qxl_gl_draw_async() and reaches the following error: (process:21117): Spice-CRITICAL **: red-qxl.c:900:spice_qxl_gl_draw_async: condition `qxl_state->scanout.drm_dma_buf_fd != -1' failed
Hi, Resuming to work on this after 2.6 freeze break ... > I have done some more testing and sent a series for spice-gtk fixing > display with gl scanout-only case. And a minor patch to spice server > to solve a cursor initialization when there is no canvas. Your series > works ok with that, only when doing a reboot, virtio_gpu_virgl_reset() > calls spice_qxl_gl_scanout( fd = -1), and later spice_gl_refresh() > calls spice_qxl_gl_draw_async() and reaches the following error: > > (process:21117): Spice-CRITICAL **: > red-qxl.c:900:spice_qxl_gl_draw_async: condition > `qxl_state->scanout.drm_dma_buf_fd != -1' failed Hmm, that is sort-of intentional. It's valid for scanouts to not be backed by a resource, and in that case qemu_spice_gl_scanout() calls spice_qxl_gl_scanout() with fd=-1. So, what do you think we should do here? Fix spice to handle that case? Should qemu do something else instead? Such as not calling spice_qxl_gl_scanout() to keep the previous dma-buf alive? cheers, Gerd
Hi On Mon, May 23, 2016 at 3:52 PM, Gerd Hoffmann <kraxel@redhat.com> wrote: > Hi, > > Resuming to work on this after 2.6 freeze break ... > >> I have done some more testing and sent a series for spice-gtk fixing >> display with gl scanout-only case. And a minor patch to spice server >> to solve a cursor initialization when there is no canvas. Your series >> works ok with that, only when doing a reboot, virtio_gpu_virgl_reset() >> calls spice_qxl_gl_scanout( fd = -1), and later spice_gl_refresh() >> calls spice_qxl_gl_draw_async() and reaches the following error: >> >> (process:21117): Spice-CRITICAL **: >> red-qxl.c:900:spice_qxl_gl_draw_async: condition >> `qxl_state->scanout.drm_dma_buf_fd != -1' failed > > Hmm, that is sort-of intentional. It's valid for scanouts to not be > backed by a resource, and in that case qemu_spice_gl_scanout() calls > spice_qxl_gl_scanout() with fd=-1. Ok, then I suppose the display should be marked as non-ready, and the client should reflect that (blank display or "not ready" message) > > So, what do you think we should do here? Fix spice to handle that case? > Should qemu do something else instead? Such as not calling > spice_qxl_gl_scanout() to keep the previous dma-buf alive? I can't really make sense of a call to spice_qxl_gl_draw_async() if there is no scanout backing. So I can imagine the fix is on qemu side. Do you have an up to date branch for testing? thanks
On Mo, 2016-05-23 at 16:03 +0200, Marc-André Lureau wrote: > Hi > > On Mon, May 23, 2016 at 3:52 PM, Gerd Hoffmann <kraxel@redhat.com> wrote: > > Hi, > > > > Resuming to work on this after 2.6 freeze break ... > > > >> I have done some more testing and sent a series for spice-gtk fixing > >> display with gl scanout-only case. And a minor patch to spice server > >> to solve a cursor initialization when there is no canvas. What is the upstream status of this? Built a bunch of fresh packages today, including new spice-server and spice-gtk from master branch. And a bunch spice-gtk dependencies, due to soname change. Still not working without the dummy primary surface. > >> (process:21117): Spice-CRITICAL **: > >> red-qxl.c:900:spice_qxl_gl_draw_async: condition > >> `qxl_state->scanout.drm_dma_buf_fd != -1' failed > I can't really make sense of a call to spice_qxl_gl_draw_async() if > there is no scanout backing. So I can imagine the fix is on qemu side. Indeed. That one should be fixed now. > Do you have an up to date branch for testing? Pushed fresh branch to the usual work/virgl location. cheers, Gerd
diff --git a/ui/spice-display.c b/ui/spice-display.c index 15d7906..96beb02 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -856,13 +856,27 @@ static void spice_gl_switch(DisplayChangeListener *dcl, { SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl); EGLint stride, fourcc; + bool resize = true; int fd; + if (ssd->ds && new_surface && + surface_width(ssd->ds) == surface_width(new_surface) && + surface_height(ssd->ds) == surface_height(new_surface) && + surface_format(ssd->ds) == surface_format(new_surface)) { + resize = false; + } + if (ssd->ds) { surface_gl_destroy_texture(ssd->gls, ssd->ds); + if (resize) { + qemu_spice_destroy_host_primary(ssd); + } } ssd->ds = new_surface; if (ssd->ds) { + if (resize) { + qemu_spice_create_host_primary(ssd); + } surface_gl_create_texture(ssd->gls, ssd->ds); fd = egl_get_fd_for_texture(ssd->ds->texture, &stride, &fourcc);
Current spice client expects we create a primary surface, even if we do display updates using dma-bufs exclusively. So just do that to get things going. Not fully clear whenever that is intentional or a bug on the spice side, so I keep this as separate patch for now. I think this should either be squashed into the previous patch, or dropped after fixing things on the spice side. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- ui/spice-display.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)