On 11/20/2018 10:27 AM, Sakari Ailus wrote:
> Hi Hans,
>
> On Tue, Nov 20, 2018 at 09:58:43AM +0100, Hans Verkuil wrote:
>> Add new buffer capability flags to indicate if the VIDIOC_PREPARE_BUF or
>> VIDIOC_CREATE_BUFS ioctls are supported.
>
> Are there practical benefits from the change for the user space?
The more important ioctl to know about is PREPARE_BUF. I noticed this when working
on v4l2-compliance: the only way to know for an application if PREPARE_BUF exists
is by trying it, but then you already have prepared a buffer. That's not what you
want in the application, you need a way to know up front if prepare_buf is present
or not without having to actually execute it.
CREATE_BUFS was added because not all drivers support it. It can be dropped since
it is possible to test for the existence of CREATE_BUFS without actually allocating
anything, but if I'm adding V4L2_BUF_CAP_SUPPORTS_PREPARE_BUF anyway, then it is
trivial to add V4L2_BUF_CAP_SUPPORTS_CREATE_BUFS as well to avoid an additional
ioctl call.
Hmm, I should have explained this in the commit log.
Regards,
Hans
>
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>> ---
>> Note: the flag bits will change since there are two other patches that add
>> flags, so the numbering will change.
>> ---
>> diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
>> index d4bbbb0c60e8..abf925484aff 100644
>> --- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
>> +++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
>> @@ -112,6 +112,8 @@ any DMA in progress, an implicit
>> .. _V4L2-BUF-CAP-SUPPORTS-USERPTR:
>> .. _V4L2-BUF-CAP-SUPPORTS-DMABUF:
>> .. _V4L2-BUF-CAP-SUPPORTS-REQUESTS:
>> +.. _V4L2-BUF-CAP-SUPPORTS-PREPARE-BUF:
>> +.. _V4L2-BUF-CAP-SUPPORTS-CREATE-BUFS:
>>
>> .. cssclass:: longtable
>>
>> @@ -132,6 +134,12 @@ any DMA in progress, an implicit
>> * - ``V4L2_BUF_CAP_SUPPORTS_REQUESTS``
>> - 0x00000008
>> - This buffer type supports :ref:`requests <media-request-api>`.
>> + * - ``V4L2_BUF_CAP_SUPPORTS_PREPARE_BUF``
>> + - 0x00000010
>> + - This buffer type supports :ref:`VIDIOC_PREPARE_BUF`.
>> + * - ``V4L2_BUF_CAP_SUPPORTS_CREATE_BUFS``
>> + - 0x00000020
>> + - This buffer type supports :ref:`VIDIOC_CREATE_BUFS`.
>>
>> Return Value
>> ============
>> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
>> index a17033ab2c22..27c0fafca0bf 100644
>> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
>> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
>> @@ -871,6 +871,16 @@ static inline bool vb2_queue_is_busy(struct video_device *vdev, struct file *fil
>> return vdev->queue->owner && vdev->queue->owner != file->private_data;D_PACK
>> }
>>
>> +static void fill_buf_caps_vdev(struct video_device *vdev, u32 *caps)
>> +{
>> + *caps = 0;
>> + fill_buf_caps(vdev->queue, caps);
>> + if (vdev->ioctl_ops->vidioc_prepare_buf)
>> + *caps |= V4L2_BUF_CAP_SUPPORTS_PREPARE_BUF;
>> + if (vdev->ioctl_ops->vidioc_create_bufs)
>> + *caps |= V4L2_BUF_CAP_SUPPORTS_CREATE_BUFS;
>> +}
>> +
>> /* vb2 ioctl helpers */
>>
>> int vb2_ioctl_reqbufs(struct file *file, void *priv,
>> @@ -879,7 +889,7 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv,
>> struct video_device *vdev = video_devdata(file);
>> int res = vb2_verify_memory_type(vdev->queue, p->memory, p->type);
>>
>> - fill_buf_caps(vdev->queue, &p->capabilities);
>> + fill_buf_caps_vdev(vdev, &p->capabilities);
>> if (res)
>> return res;
>> if (vb2_queue_is_busy(vdev, file))
>> @@ -901,7 +911,7 @@ int vb2_ioctl_create_bufs(struct file *file, void *priv,
>> p->format.type);
>>
>> p->index = vdev->queue->num_buffers;
>> - fill_buf_caps(vdev->queue, &p->capabilities);
>> + fill_buf_caps_vdev(vdev, &p->capabilities);
>> /*
>> * If count == 0, then just check if memory and type are valid.
>> * Any -EBUSY result from vb2_verify_memory_type can be mapped to 0.
>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>> index c8e8ff810190..6648f8ba2277 100644
>> --- a/include/uapi/linux/videodev2.h
>> +++ b/include/uapi/linux/videodev2.h
>> @@ -879,6 +879,8 @@ struct v4l2_requestbuffers {
>> #define V4L2_BUF_CAP_SUPPORTS_USERPTR (1 << 1)
>> #define V4L2_BUF_CAP_SUPPORTS_DMABUF (1 << 2)
>> #define V4L2_BUF_CAP_SUPPORTS_REQUESTS (1 << 3)
>
> Could you align the previous lines to match the ones below?
>
>> +#define V4L2_BUF_CAP_SUPPORTS_PREPARE_BUF (1 << 4)
>> +#define V4L2_BUF_CAP_SUPPORTS_CREATE_BUFS (1 << 5)
>>
>> /**
>> * struct v4l2_plane - plane info for multi-planar buffers
>
