On 11/18/19 2:52 PM, Boris Brezillon wrote:
> Hello Hans,
>
> On Mon, 18 Nov 2019 14:06:40 +0100
> Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
>
>> Here is a proposal for a new VIDIOC_DESTROY_BUFS ioctl:
>
> Thanks for sending this RFC.
>
>>
>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>> index c7c1179eea65..1a80d1119768 100644
>> --- a/include/uapi/linux/videodev2.h
>> +++ b/include/uapi/linux/videodev2.h
>> @@ -2423,6 +2423,19 @@ struct v4l2_create_buffers {
>> __u32 reserved[7];
>> };
>>
>> +/**
>> + * struct v4l2_destroy_buffers - VIDIOC_DESTROY_BUFS argument
>> + * @type: stream type
>> + * @index: index of the first buffer to destroy
>> + * @count: number of consecutive buffers starting from @index to destroy
>> + */
>> +struct v4l2_destroy_buffers {
>> + __u32 type;
>> + __u32 index;
>> + __u32 count;
>> +};
>> +
>> +
>> /*
>> * I O C T L C O D E S F O R V I D E O D E V I C E S
>> *
>> @@ -2522,6 +2535,7 @@ struct v4l2_create_buffers {
>> #define VIDIOC_DBG_G_CHIP_INFO _IOWR('V', 102, struct v4l2_dbg_chip_info)
>>
>> #define VIDIOC_QUERY_EXT_CTRL _IOWR('V', 103, struct v4l2_query_ext_ctrl)
>> +#define VIDIOC_DESTROY_BUFS _IOW ('V', 104, struct v4l2_destroy_buffers)
>>
>> /* Reminder: when adding new ioctls please add support for them to
>> drivers/media/v4l2-core/v4l2-compat-ioctl32.c as well! */
>>
>>
>>
>> So this basically just destroys buffers [index..index+count-1]. Does nothing if
>> count == 0. All buffers in the sequence must be dequeued or it will return
>> -EBUSY and do nothing.
>>
>> If some of the buffers in that range are already destroyed, or in fact were
>> never created, then they will be ignored. I.e., DESTROY_BUFS won't return
>> an error in that case.
>
> Sounds good to me.
>
>>
>>
>> VIDIOC_CREATE_BUFS will need a few changes:
>>
>> CREATE_BUFS will try to find a range of <count> free consecutive buffers.
>> If that's not available, then it will reduce <count> to the count of the
>> maximum freely available consecutive buffers. If <count> is 0, then it
>> will set <index> to the maximum index of an existing buffer + 1.
>>
>> As long as DESTROY_BUFS isn't used, then CREATE_BUFS acts exactly the same
>> as it does today.
>
> Sounds good too.
>
>>
>> I would also like to extend struct v4l2_create_buffers with a new field:
>> __u32 max_index. This is a maximum index possible, typically VIDEO_MAX_FRAME-1.
>
> Shouldn't max_buffers be a property of the queue, set through a separate
> ioctl()? BTW, how would you decrease the queue depth?
> CREATE_BUFS.{count=0,max_index=<new-depth>}?
I think the name might be confusing: cap_max_index might be better: this is just
a read-only capability: i.e. how many buffers can userspace create? Currently
this is 32, but in the future drivers should be able to allow for more buffers.
It should be something they tell vb2.
Regards,
Hans
