Hello, On Thu, Aug 17, 2023 at 09:11:36AM +0200, Hans Verkuil wrote: > On 16/08/2023 19:48, Sakari Ailus wrote: > > On Wed, Aug 16, 2023 at 05:34:32PM +0300, Laurent Pinchart wrote: > >> On Wed, Aug 16, 2023 at 02:47:33PM +0200, Hans Verkuil wrote: > >>> Currently if VIDIOC_CREATE_BUFS is called to add new buffers, then the requested > >>> number of planes per buffer might be different from the already allocated buffers. > >>> > >>> However, this does not make a lot of sense and there are no drivers that allow > >>> for variable number of planes in the allocated buffers. > >>> > >>> While it is possible do this today, when such a buffer is queued it will fail > >>> in the buf_prepare() callback where the plane sizes are checked if those are > >>> large enough. If fewer planes were allocated, then the size for the missing > >>> planes are 0 and the check will return -EINVAL. > >>> > >>> But it is much better to do this check in VIDIOC_CREATE_BUFS. > >> > >> I don't think this is a good idea. One important use case for > >> VIDIOC_CREATE_BUFS is to allocate buffers for a different format than > >> the one currently configured for the device, to prepare for a future > >> capture (or output) session with a different configuration. This patch > >> would prevent that. > > > > I'd prefer to keep this capability in videobuf2, too. Although... one way > > achieve that could be to add a flag (or an integer field) in struct > > vb2_queue to tell vb2 core that the driver wants to do the num_planes > > checks by itself. > > Having a flag for this was something I intended to do once we have a > driver that actually supports this feature. I'm not aware of any driver > that needs this today. But I'll make a v2 adding this flag, it is simple > to do and doesn't hurt. I don't think it should be a driver decision, as it's a question of userspace usage. We shouldn't couple buffer allocation and buffer usage, VIDIOC_CREATE_BUFS needs to allow allocation of any buffer suitable for the hardware *in any configuration*, not just the active configuration. It's only at buffer prepare time that the fitness of a particular buffer for the active configuration of the device can be checked. > > It'd be also nicer, considering the end result, to configure this when > > setting up the queue, rather than based on the first buffer created. This > > would involve changing a large number of drivers though. > > "to configure this": what is "this"? The new flag? Or the number of planes? > And with "setting up the queue" do you mean the queue_setup callback or > when the vb2_queue is initialized? -- Regards, Laurent Pinchart
