Re: vb2_queue type question

[Hans Verkuil <hverkuil@xs4all.nl>]

On 22/03/2021 10:18, Tomi Valkeinen wrote:
> Hi Hans,
> 
> We were discussing this with Laurent and Sakari, I thought I'd ask if 
> you have any feedback on this.
> 
> struct vb2_queue has 'type' field, so you can only use a queue for 
> buffers of a single type. struct video_device has 'queue' field, so you 
> can only use a single queue for a video_device instance.
> 
> TI's SoCs have a CSI-2 receiver, with a bunch of DMA engines. The HW 
> doesn't care if we are currently capturing pixel buffers or metadata 
> buffers (I don't have experience with other HW, but I imagine this 
> shouldn't be a rare case). However, due to vb2_queue, the driver needs 
> to decide which one to support, which limits the possible use cases.
> 
> I was browsing the code, and afaics the type field doesn't do much. It 
> is, of course, used to reject queuing buffers of wrong type, and also 
> (mostly in mem-2-mem code) to find out if functions are called in input 
> or output context.
> 
> The latter one could be easily removed by just comparing the given queue 
> pointer to a stored pointer (e.g. queue == priv->input_queue).
> 
> Do you see any problems if we were to change the type field to 
> type_mask, allowing multiple buffer types per queue? Or even remove the 
> vb2_queue->type. This raises some questions, like should a queue contain 
> only buffers of a single type or can it contain a mix of buffers (I 
> think it shouldn't contain a mix of buffers), or can a queue's type_mask 
> contain both input and output types (I don't see why not).
> 
> An alternate which I tried was creating two vb2_queues, and switching 
> the video_device->queue at runtime based on set_format. It kind of 
> works, but I think the behavior is a bit unclear, and it might be 
> difficult to catch all the corner cases.

A vb2_queue basically represents a buffer queue that will be fed to a
DMA engine. It assumes that all the buffers are of the same format,
which typically is tied directly to the type.

The type of a vb2_queue can be changed if you like, but once buffers
are allocated it is fixed and can't be changed again until all buffers
are released. So you can't mix buffers of different types.

This is actually done in the vivid driver: see vidioc_s_fmt_vbi_cap()
and vidioc_s_fmt_sliced_vbi_cap(): depending on the format the queue
type will be set to either capture raw or sliced VBI.

The ivtv driver does the same thing.

So as long as vb2_is_busy() returns false, you are free to change the
queue type.

There is no need for a type_mask or anything like that. That's up to
the bridge driver to check. The vb2_queue type is there to ensure that
userspace isn't trying to mix buffers of different types, but as long
as no buffers are allocated it doesn't do anything and you are free to
change it.

Regards,

	Hans