Re: [RFC PATCH v2 1/2] virtio-gpu: add resource create blob

[Gurchetan Singh <gurchetansingh@chromium.org>]

On Wed, May 13, 2020 at 12:03 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>   Hi,
>
> > > So add a note that this is for future planar format support?
> > > Or do we want add planar formats now?
> >
> > No YUV support for now.  I was mostly thinking about compressed
> > standard formats (AB24, AR24) with a header + data plane
> > (I915_FORMAT_MOD_Y_TILED_CCS etc.).  The modifier can be queried from
> > virglrenderer in the distant future to avoid modifiers in the guest
> > (which is another can of worms).  Will write a note about that.
>
> Ok, good.
>
> > > Also: should resource_id an array too?  So we have the option to store
> > > each plane in a different resource (simliar to drm_framebuffer in the
> > > linux kernel)?
> >
> > It's a cost vs. level of future-proofing tradeoff.
> >
> > Pros:
> > + To mirror the KMS/wayland APIs, a resource_id array +
> > VIRTIO_GPU_CMD_SET_SCANOUT_BLOBS makes sense.
> > Cons
> > - The trend seems to be one buffer.  I've only encountered disjoint
> > YUV images on deprecated platforms, and reviewing the modifiers in
> > <drm_fourcc.h> seem to be one buffer too.
>
> Ok.  I guess it makes sense to stick to one buffer then.  An API which
> isn't used in practice is a maintenance burden we better avoid ...

Cool.  I left out SCANOUT_FLUSH in v3 since AFAICT we'll only have one
blob resource per scanout, and that blob resource will encompass the
entire window (since we set_scanout when display parameters change).
And to update the scanout blob resource, we can just call
RESOURCE_FLUSH.

Though SCANOUT_FLUSH does make sense if you want multiple blob
resources per scanout surface, so if there's a desire for some
additional functionality, LMK.

Also, TRANSFER_BLOB is also another maintenance vs. future proofing
tradeoff -- for example, emulated coherent memory[1] (for platforms
without the host-visible region or external memory extensions or
shared guest) may benefit from such an API.  However, the
implementation of that doesn't exist and may not for a while (focusing
on true zero copy for now), and the virglrenderer impl of the
hypercall will likely just be a stub for now.  So if that's
undesirable, LMK.

[1] https://lists.freedesktop.org/archives/dri-devel/2019-April/215731.htm

>
> take care,
>   Gerd
>