[Spice-devel] [virtio-dev] Re: [PATCH v2 0/1] VirtIO video device specification

[Keiichi Watanabe]

Hi,

On Wed, Jan 15, 2020 at 8:26 PM Gerd Hoffmann <kraxel at redhat.com> wrote:
>
>   Hi,
>
> > > If you have (1) hardware you simply can't import buffers with arbitrary
> > > plane offsets, so I'd expect software would prefer the single buffer
> > > layout (i) over (ii), even when using another driver + dmabuf
> > > export/import, to be able to support as much hardware as possible.
> > > So (ii) might end up being unused in practice.
> > >
> > > But maybe not, was just an idea, feel free to scratch it.
> >
> > That's true, simple user space would often do that. However, if more
> > devices are in the game, often some extra alignment or padding between
> > planes is needed and that is not allowed by (1), even though all the
> > planes are in the same buffer.
> >
> > My suggestion, based on the latest V4L2 discussion on unifying the
> > UAPI of i) and ii), is that we may want to instead always specify
> > buffers on a per-plane basis. Any additional requirements would be
> > then validated by the host, which could check if the planes end up in
> > the same buffer (or different buffers for (3)) and/or at the right
> > offsets.
>
> Hmm, using (ii) the API, then check whenever your three plane buffers
> happen to have the correct layout for (1) hardware looks somewhat
> backwards to me.

Can't this problem be solved by adding "offset" field in virtio_video_mem_entry?

struct virtio_video_mem_entry {
  le64 addr;
  le32 length;
  le32 offset;
  u8 padding[4];
};

Here, "addr" must be the same in every mem_entry for (1) hardware.

>
> I'd suggest to use (i) API and allow the device specify alignment
> requirements.  So (1) hardware would say "need_align=0", whereas (3)
> hardware would probably say "need_align=PAGE_SIZE" so it can easily
> split the single buffer into three per-plane buffers.

Just to confirm, is "need_align" a field added in virtio_video_format_desc?
It sounds workable, too.

Best regards,
Keiichi


>
> cheers,
>   Gerd
>