Hi,
On Wed, Jan 15, 2020 at 8:00 PM Tomasz Figa <tfiga@xxxxxxxxxxxx> wrote:
>
> On Mon, Jan 13, 2020 at 10:27 PM Gerd Hoffmann <kraxel@xxxxxxxxxx> wrote:
> >
> > Hi,
> >
> > > > Well, no. Tomasz Figa had splitted the devices into three groups:
> > > >
> > > > (1) requires single buffer.
> > > > (2) allows any layout (including the one (1) devices want).
> > > > (3) requires per-plane buffers.
> > > >
> > > > Category (3) devices are apparently rare and old. Both category (1)+(2)
> > > > devices can handle single buffers just fine. So maybe support only
> > > > that?
> > >
> > > From the guest implementation point of view, Linux V4L2 currently
> > > supports 2 cases, if used in allocation-mode (i.e. the buffers are
> > > allocated locally by V4L2):
> > >
> > > i) single buffer with plane offsets predetermined by the format, (can
> > > be handled by devices from category 1) and 2))
> > > ii) per-plane buffers with planes at the beginning of their own
> > > buffers. (can be handled by devices from category 2) and 3))
> > >
> > > Support for ii) is required if one wants to be able to import buffers
> > > with arbitrary plane offsets, so I'd consider it unavoidable.
> >
> > 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.
>
It sounds reasonable. Even in the protocol design we discussed so far,
the driver sends an array of struct virtio_video_mem_entry in the
RESOURE_CREATE command:
struct virtio_video_mem_entry {
le64 addr;
le32 length;
u8 padding[4];
};
struct virtio_video_resource_create {
struct virtio_video_cmd_hdr hdr;
le32 queue_type;
le32 resource_id;
le32 num_planes;
le32 num_entries[VIRTIO_VIDEO_MAX_PLANES];
u8 padding[4];
/* Followed by struct virtio_video_mem_entry entries[] */
};
Does it match with your idea? We may need an |offset| in virtio_video_mem_entry?
Also, we should redesign "enum virtio_video_planes_layout" that we
originally discussed.
How about changing it like this?
enum virtio_video_planes_layout {
VIRTIO_VIDEO_PLANES_LAYOUT_SINGLE_BUFFER = 1 << 0,
VIRTIO_VIDEO_PLANES_LAYOUT_PER_PLANE = 1 << 1,
};
So, the device can combine flags to tell which (1), (2) or (3) it is.
Then, the device check if an incoming RESOURE_CREATE request violates
the requirement.
Does it make sense?
Best regards,
Keiichi
> WDYT?
>
> Best regards,
> Tomasz
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel