On 10/09/2020 11:49, Sergey Senozhatsky wrote:
> Hi,
>
> On (20/09/08 23:58), Tomasz Figa wrote:
>>
>> Given the above, we would like to make changes that affect the UAPI.
>> Would you still be able to revert this series?
>>
>
> If we want to apply only "media/v4l2: remove V4L2-FLAG-MEMORY-NON-CONSISTENT"
> patch and keep the rest of the buffer cache hints series in the kernel, then
> I'd add one or two more patches. We don't need ->flags argument in
> create_bufs() and reqbufs() functions, that argument was introduced in order
> to pass in the requested coherency flag.
>
>
> Now.
>
> Then we have a question - do we need flags member in struct
> v4l2_requestbuffers and v4l2_create_buffers or shall we just
> return back those 4 bytes to reserved[]? We pass BUF_FLAG_NO_CACHE_INVALIDATE
> and V4L2_BUF_FLAG_NO_CACHE_SYNC in struct v4l2_buffer.flags.
Drop the now unused flags member in v4l2_requestbuffers and v4l2_create_buffers.
>
> If we decide to remove v4l2_requestbuffers and v4l2_create_buffers ->flags,
> then we also need to rollback documentation changes.
Correct.
>
> Then we need to change CLEAR_AFTER_FIELD(foo, capabilities) in
> v4l2-ioctl to zero out reserved[] areas in v4l2_requestbuffers
> and v4l2_create_buffers. I think v4l2_create_buffers is fine,
> but requstbuffers has flags and reversed[1] in the union so for
> requestbuffers we simply removed the CLEAR_AFTER_FIELD() and
> hence dropped the corresponding check from v4l-compliance.
>
> So, do we plan on using .flags in v4l2_requestbuffers and
> v4l2_create_buffers?
Perhaps, but this patch is meant to revert *all* changes relating to
V4L2_FLAG_MEMORY_NON_CONSISTENT. We don't want to have unused fields
in the public API.
Regards,
Hans
>
>
> - create_bufs()/reqbufs() flags argument removal patch
> (no struct-s/documentation cleanup yet).
>
> ====8<====
> Subject: [PATCH] remove redundant flags argument
>
> ---
> drivers/media/common/videobuf2/videobuf2-core.c | 10 +++++-----
> drivers/media/common/videobuf2/videobuf2-v4l2.c | 6 ++----
> include/media/videobuf2-core.h | 6 ++----
> 3 files changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
> b/drivers/media/common/videobuf2/videobuf2-core.c
> index 66a41cef33c1..4eab6d81cce1 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -722,7 +722,7 @@ int vb2_verify_memory_type(struct vb2_queue *q,
> EXPORT_SYMBOL(vb2_verify_memory_type);
>
> int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
> - unsigned int flags, unsigned int *count)
> + unsigned int *count)
> {
> unsigned int num_buffers, allocated_buffers, num_planes = 0;
> unsigned plane_sizes[VB2_MAX_PLANES] = { };
> @@ -861,7 +861,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
> memory,
> EXPORT_SYMBOL_GPL(vb2_core_reqbufs);
>
> int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
> - unsigned int flags, unsigned int *count,
> + unsigned int *count,
> unsigned int requested_planes,
> const unsigned int requested_sizes[])
> {
> @@ -2547,7 +2547,7 @@ static int __vb2_init_fileio(struct vb2_queue *q, int
> read)
> fileio->memory = VB2_MEMORY_MMAP;
> fileio->type = q->type;
> q->fileio = fileio;
> - ret = vb2_core_reqbufs(q, fileio->memory, 0, &fileio->count);
> + ret = vb2_core_reqbufs(q, fileio->memory, &fileio->count);
> if (ret)
> goto err_kfree;
>
> @@ -2604,7 +2604,7 @@ static int __vb2_init_fileio(struct vb2_queue *q, int
> read)
>
> err_reqbufs:
> fileio->count = 0;
> - vb2_core_reqbufs(q, fileio->memory, 0, &fileio->count);
> + vb2_core_reqbufs(q, fileio->memory, &fileio->count);
>
> err_kfree:
> q->fileio = NULL;
> @@ -2624,7 +2624,7 @@ static int __vb2_cleanup_fileio(struct vb2_queue *q)
> vb2_core_streamoff(q, q->type);
> q->fileio = NULL;
> fileio->count = 0;
> - vb2_core_reqbufs(q, fileio->memory, 0, &fileio->count);
> + vb2_core_reqbufs(q, fileio->memory, &fileio->count);
> kfree(fileio);
> dprintk(q, 3, "file io emulator closed\n");
> }
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index d2879f53455b..96d3b2b2aa31 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -728,8 +728,7 @@ int vb2_reqbufs(struct vb2_queue *q, struct
> v4l2_requestbuffers *req)
> int ret = vb2_verify_memory_type(q, req->memory, req->type);
>
> fill_buf_caps(q, &req->capabilities);
> - return ret ? ret : vb2_core_reqbufs(q, req->memory,
> - req->flags, &req->count);
> + return ret ? ret : vb2_core_reqbufs(q, req->memory, &req->count);
> }
> EXPORT_SYMBOL_GPL(vb2_reqbufs);
>
> @@ -804,7 +803,6 @@ int vb2_create_bufs(struct vb2_queue *q, struct
> v4l2_create_buffers *create)
> if (requested_sizes[i] == 0)
> return -EINVAL;
> return ret ? ret : vb2_core_create_bufs(q, create->memory,
> - create->flags,
> &create->count,
> requested_planes,
> requested_sizes);
> @@ -993,7 +991,7 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv,
> return res;
> if (vb2_queue_is_busy(vdev, file))
> return -EBUSY;
> - res = vb2_core_reqbufs(vdev->queue, p->memory, p->flags, &p->count);
> + res = vb2_core_reqbufs(vdev->queue, p->memory, &p->count);
> /* If count == 0, then the owner has released all buffers and he
> is no longer owner of the queue. Otherwise we have a new owner. */
> if (res == 0)
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 4c7f25b07e93..bbb3f26fbde9 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -744,7 +744,6 @@ void vb2_core_querybuf(struct vb2_queue *q, unsigned int
> index, void *pb);
> * vb2_core_reqbufs() - Initiate streaming.
> * @q: pointer to &struct vb2_queue with videobuf2 queue.
> * @memory: memory type, as defined by &enum vb2_memory.
> - * @flags: auxiliary queue/buffer management flags.
> * @count: requested buffer count.
> *
> * Videobuf2 core helper to implement VIDIOC_REQBUF() operation. It is called
> @@ -769,13 +768,12 @@ void vb2_core_querybuf(struct vb2_queue *q, unsigned
> int index, void *pb);
> * Return: returns zero on success; an error code otherwise.
> */
> int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
> - unsigned int flags, unsigned int *count);
> + unsigned int *count);
>
> /**
> * vb2_core_create_bufs() - Allocate buffers and any required auxiliary
> structs
> * @q: pointer to &struct vb2_queue with videobuf2 queue.
> * @memory: memory type, as defined by &enum vb2_memory.
> - * @flags: auxiliary queue/buffer management flags.
> * @count: requested buffer count.
> * @requested_planes: number of planes requested.
> * @requested_sizes: array with the size of the planes.
> @@ -793,7 +791,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
> memory,
> * Return: returns zero on success; an error code otherwise.
> */
> int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
> - unsigned int flags, unsigned int *count,
> + unsigned int *count,
> unsigned int requested_planes,
> const unsigned int requested_sizes[]);
>
>
