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

[Gerd Hoffmann]

  Hi,

> > 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.

No.  virtio_video_mem_entry is basically a scatter list entry, you use
an *array* of these entries to describe your buffer (unless you are
using CMA due to hardware requirements, in this special case you have
only one entry in your array).

> > 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?

Given that different formats might have different alignment
requirements this looks like a good place to me.  Maybe rename to
plane_align to make clear what kind of alignment this is.

cheers,
  Gerd