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

Gerd Hoffmann
2020-01-20T10:48:08Z

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