RE: [RFC 0/1] Use dmabufs for display updates instead of pixman

["Kasireddy, Vivek" <vivek.kasireddy@intel.com>]

Hi Gerd,

> Yes, it surely makes sense to go into that direction.
> The patch as-is doesn't, it breaks the guest/host interface.
> That's ok-ish for a quick proof-of-concept, but clearly not merge-able.
> 
> > TODO:
> > - Use Blob resources for getting meta-data such as modifier, format, etc.
> 
> That is pretty much mandatory.  Without blob resources there is no concept of resources
> shared between host and guest in virtio-gpu, all data is explicitly copied with transfer
> commands.
[Kasireddy, Vivek] My understanding of virtio-gpu and the concept of resources is still
fairly limited but are blob resources really needed for non-Virgl use-cases -- other than
something like a dmabuf/scanout blob that shares the meta-data such as modifer? I
thought the main motivation for blob resources would be to avoid the explicit copy you
mentioned for Virgl workloads. 

> 
> Which implies quite a bit of work because we don't have blob resource support in qemu
> yet.
[Kasireddy, Vivek] I was scrubbing through old mailing list messages to understand the
motivation behind blob resources as to why they are needed and came across this:
https://gitlab.freedesktop.org/virgl/qemu/-/commits/virtio-gpu-next

Does your work above not count for anything?

> 
> > - Test with Virgil rendered BOs to see if this can be used in that case..
> 
> That also opens up the question how to go forward with virtio-gpu in general.  The object
> hierarchy we have right now (skipping pci + vga variants for simplicity):
> 
>   TYPE_VIRTIO_GPU_BASE (abstract base)
>    -> TYPE_VIRTIO_GPU (in-qemu implementation)
>    -> TYPE_VHOST_USER_GPU (vhost-user implementation)
> 
> When compiled with opengl + virgl TYPE_VIRTIO_GPU has a virgl=on/off property.
> Having a single device is not ideal for modular builds.
> because the hw-display-virtio-gpu.so module has a dependency on ui-opengl.so so that is
> needed (due to symbol references) even for the virgl=off case.  Also the code is a bit of a
> #ifdef mess.
> 
> I think we should split TYPE_VIRTIO_GPU into two devices.  Remove
> virgl+opengl support from TYPE_VIRTIO_GPU.  Add a new
> TYPE_VIRTIO_GPU_VIRGL, with either TYPE_VIRTIO_GPU or
> TYPE_VIRTIO_GPU_BASE as parent (not sure which is easier), have all opengl/virgl
> support code there.
> 
> I think when using opengl it makes sense to also require virgl, so we can use the
> virglrenderer library to manage blob resources (even when the actual rendering isn't done
> with virgl).  Also reduces the complexity and test matrix.
[Kasireddy, Vivek] When you say "using opengl" are you referring to the presentation of
the rendered buffer via dmabuf or pixman? If yes, I am not sure why this would need to
depend on Virgl. For our use-case(s) where we are using virtio-gpu in buffer sharing mode,
we'd still need opengl for submitting the dmabuf to UI, IIUC.

> 
> Maybe it even makes sense to deprecate in-qemu virgl support and focus exclusively on
> the vhost-user implementation, so we don't have to duplicate all work for both
> implementations.
[Kasireddy, Vivek] Is the vhost-user implementation better in terms of performance, generally? 

> > case, how do we make sure that Weston and Qemu UI are not using the same buffer at
> any given time?
> 
> There is graphic_hw_gl_block + graphic_hw_gl_flushed for syncronization.
> Right now this is only wired up in spice, and it is rather simple (just stalls virgl rendering
> instead of providing per-buffer syncronization).
[Kasireddy, Vivek] I guess that might work for Virgl rendering but not for our use-case. What
we need is a way to tell if the previously submitted dmabuf has been consumed by the Host 
compositor or not before we release/close it. Weston (wl_buffer.release event and fences) 
and EGL (sync and fences) do provide few options but I am not sure if GTK lets us use
any of those or not. Any recommendations? EGLSync objects?

On a different note, any particular reason why Qemu UI EGL implementation is limited to Xorg
and not extended to Wayland/Weston for which there is GTK glarea?

Thanks,
Vivek

> 
> take care,
>   Gerd