Hi Steve, (stripping the CC list a bit and adding Sakari Ailus) On Thursday 02 Feb 2017 11:12:41 Steve Longerbeam wrote: > On 02/02/2017 10:58 AM, Russell King - ARM Linux wrote: [snip] > > It seems to me that if you don't take account of the existing queue > > size, your camif_queue_setup() has the side effect that each time > > either of these are called. Hence, the vb2 queue increases by the > > same amount each time, which is probably what you don't want. > > > > The documentation on queue_setup() leaves much to be desired: > > * @queue_setup: called from VIDIOC_REQBUFS() and VIDIOC_CREATE_BUFS() > > * handlers before memory allocation. It can be called > > * twice: if the original number of requested buffers > > * could not be allocated, then it will be called a > > * second time with the actually allocated number of > > * buffers to verify if that is OK. > > * The driver should return the required number of buffers > > * in \*num_buffers, the required number of planes per > > * buffer in \*num_planes, the size of each plane should be > > * set in the sizes\[\] array and optional per-plane > > * allocator specific device in the alloc_devs\[\] array. > > * When called from VIDIOC_REQBUFS,() \*num_planes == 0, > > * the driver has to use the currently configured format to > > * determine the plane sizes and \*num_buffers is the total > > * number of buffers that are being allocated. When called > > * from VIDIOC_CREATE_BUFS,() \*num_planes != 0 and it > > * describes the requested number of planes and sizes\[\] > > * contains the requested plane sizes. If either > > * \*num_planes or the requested sizes are invalid callback > > * must return %-EINVAL. In this case \*num_buffers are > > * being allocated additionally to q->num_buffers. > > > > That's really really ambiguous, because the "In this case" part doesn't > > really tell you which case it's talking about - but it seems to me looking > > at the code that it's referring to the VIDIOC_CREATE_BUFS case. > > Yes, I caught this when adding fixes from v4l2-compliance testing, which > is not part of the version 3 driver. I agree it is a confusing API. When > called from VIDIOC_CREATE_BUFS (indicated by *num_planes != 0), > *num_buffers is supposed to be requested buffers _in addition_ to > already requested q->num_buffers, which is important info and > should be emphasized a little more than the "oh by the way" fashion > in the prototype description, IMHO. Hans, Sakari, any opinion ? [snip] -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
