Re: [PATCH 05/28] media/v4l2: remove V4L2-FLAG-MEMORY-NON-CONSISTENT

From: Sergey Senozhatsky
Date: Thu Sep 10 2020 - 05:49:53 EST


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.

If we decide to remove v4l2_requestbuffers and v4l2_create_buffers ->flags,
then we also need to rollback documentation changes.

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?


- 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[]);

--
2.28.0.526.ge36021eeef-goog