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

From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Tomasz Figa <tfiga@chromium.org>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Christoph Hellwig <hch@lst.de>,
	Linux Media Mailing List <linux-media@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Subject: Re: [PATCH 05/28] media/v4l2: remove V4L2-FLAG-MEMORY-NON-CONSISTENT
Date: Fri, 11 Sep 2020 00:38:02 +0900	[thread overview]
Message-ID: <20200910153802.GE97481@google.com> (raw)
In-Reply-To: <20200910144830.GD97481@google.com>

On (20/09/10 23:48), Sergey Senozhatsky wrote:
> 
> I've a kernel patch (I think I got all the pieces). This is not a
> format submission yet, because I don't have v4l2-compliance patch
> yet, so didn't really test it.
> 

I guess,,, the v4l-utils patch should look something like this:
====

From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Subject: [PATCH] v4l-compliance: remove NON_CONSISTENT hint test

Kernel support for V4L2_FLAG_MEMORY_NON_CONSISTENT has been
reverted, so we need to to remove the memory consistency
(coherency) tests from the test-buffers code.

Note, the buffer cache management hints support is still there
and should be tested.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 utils/common/cv4l-helpers.h                 |  8 ++---
 utils/common/v4l-helpers.h                  |  8 ++---
 utils/v4l2-compliance/v4l2-test-buffers.cpp | 40 ++-------------------
 3 files changed, 9 insertions(+), 47 deletions(-)

diff --git a/utils/common/cv4l-helpers.h b/utils/common/cv4l-helpers.h
index 3cee372b..712efde6 100644
--- a/utils/common/cv4l-helpers.h
+++ b/utils/common/cv4l-helpers.h
@@ -754,17 +754,17 @@ public:
 	int g_fd(unsigned index, unsigned plane) const { return v4l_queue_g_fd(this, index, plane); }
 	void s_fd(unsigned index, unsigned plane, int fd) { v4l_queue_s_fd(this, index, plane, fd); }
 
-	int reqbufs(cv4l_fd *fd, unsigned count = 0, unsigned int flags = 0)
+	int reqbufs(cv4l_fd *fd, unsigned count = 0)
 	{
-		return v4l_queue_reqbufs(fd->g_v4l_fd(), this, count, flags);
+		return v4l_queue_reqbufs(fd->g_v4l_fd(), this, count);
 	}
 	bool has_create_bufs(cv4l_fd *fd) const
 	{
 		return v4l_queue_has_create_bufs(fd->g_v4l_fd(), this);
 	}
-	int create_bufs(cv4l_fd *fd, unsigned count, const v4l2_format *fmt = NULL, unsigned int flags = 0)
+	int create_bufs(cv4l_fd *fd, unsigned count, const v4l2_format *fmt = NULL)
 	{
-		return v4l_queue_create_bufs(fd->g_v4l_fd(), this, count, fmt, flags);
+		return v4l_queue_create_bufs(fd->g_v4l_fd(), this, count, fmt);
 	}
 	int mmap_bufs(cv4l_fd *fd, unsigned from = 0)
 	{
diff --git a/utils/common/v4l-helpers.h b/utils/common/v4l-helpers.h
index c09cd987..f96b3c38 100644
--- a/utils/common/v4l-helpers.h
+++ b/utils/common/v4l-helpers.h
@@ -1515,7 +1515,7 @@ static inline int v4l_queue_querybufs(struct v4l_fd *f, struct v4l_queue *q, uns
 }
 
 static inline int v4l_queue_reqbufs(struct v4l_fd *f,
-		struct v4l_queue *q, unsigned count, unsigned int flags = 0)
+		struct v4l_queue *q, unsigned count)
 {
 	struct v4l2_requestbuffers reqbufs;
 	int ret;
@@ -1523,7 +1523,6 @@ static inline int v4l_queue_reqbufs(struct v4l_fd *f,
 	reqbufs.type = q->type;
 	reqbufs.memory = q->memory;
 	reqbufs.count = count;
-	reqbufs.flags = flags;
 	/*
 	 * Problem: if REQBUFS returns an error, did it free any old
 	 * buffers or not?
@@ -1548,7 +1547,7 @@ static inline bool v4l_queue_has_create_bufs(struct v4l_fd *f, const struct v4l_
 
 static inline int v4l_queue_create_bufs(struct v4l_fd *f,
 		struct v4l_queue *q, unsigned count,
-		const struct v4l2_format *fmt, unsigned int flags = 0)
+		const struct v4l2_format *fmt)
 {
 	struct v4l2_create_buffers createbufs;
 	int ret;
@@ -1556,7 +1555,6 @@ static inline int v4l_queue_create_bufs(struct v4l_fd *f,
 	createbufs.format.type = q->type;
 	createbufs.memory = q->memory;
 	createbufs.count = count;
-	createbufs.flags = flags;
 	if (fmt) {
 		createbufs.format = *fmt;
 	} else {
@@ -1735,7 +1733,7 @@ static inline void v4l_queue_free(struct v4l_fd *f, struct v4l_queue *q)
 	v4l_ioctl(f, VIDIOC_STREAMOFF, &q->type);
 	v4l_queue_release_bufs(f, q, 0);
 	v4l_queue_close_exported_fds(q);
-	v4l_queue_reqbufs(f, q, 0, 0);
+	v4l_queue_reqbufs(f, q, 0);
 }
 
 static inline void v4l_queue_buffer_update(const struct v4l_queue *q,
diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp
index aca0eb68..1651e95c 100644
--- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
+++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
@@ -653,10 +653,6 @@ int testReqBufs(struct node *node)
 		fail_on_test(q.reqbufs(node, 0));
 
 		for (m = V4L2_MEMORY_MMAP; m <= V4L2_MEMORY_DMABUF; m++) {
-			bool cache_hints_cap = false;
-			bool consistent;
-
-			cache_hints_cap = q.g_capabilities() & V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS;
 			if (!(node->valid_memorytype & (1 << m)))
 				continue;
 			cv4l_queue q2(i, m);
@@ -672,17 +668,8 @@ int testReqBufs(struct node *node)
 			reqbufs.count = 1;
 			reqbufs.type = i;
 			reqbufs.memory = m;
-			reqbufs.flags = V4L2_FLAG_MEMORY_NON_CONSISTENT;
 			fail_on_test(doioctl(node, VIDIOC_REQBUFS, &reqbufs));
-			consistent = reqbufs.flags & V4L2_FLAG_MEMORY_NON_CONSISTENT;
-			if (!cache_hints_cap) {
-				fail_on_test(consistent);
-			} else {
-				if (m == V4L2_MEMORY_MMAP)
-					fail_on_test(!consistent);
-				else
-					fail_on_test(consistent);
-			}
+			fail_on_test(check_0(reqbufs.reserved, sizeof(reqbufs.reserved)));
 			q.reqbufs(node);
 
 			ret = q.create_bufs(node, 0);
@@ -695,32 +682,9 @@ int testReqBufs(struct node *node)
 			node->g_fmt(crbufs.format, i);
 			crbufs.count = 1;
 			crbufs.memory = m;
-			crbufs.flags = V4L2_FLAG_MEMORY_NON_CONSISTENT;
 			fail_on_test(doioctl(node, VIDIOC_CREATE_BUFS, &crbufs));
 			fail_on_test(check_0(crbufs.reserved, sizeof(crbufs.reserved)));
 			fail_on_test(crbufs.index != q.g_buffers());
-
-			consistent = crbufs.flags & V4L2_FLAG_MEMORY_NON_CONSISTENT;
-			if (!cache_hints_cap) {
-				fail_on_test(consistent);
-			} else {
-				if (m == V4L2_MEMORY_MMAP)
-					fail_on_test(!consistent);
-				else
-					fail_on_test(consistent);
-			}
-
-			if (cache_hints_cap) {
-				/*
-				 * Different memory consistency model. Should fail for MMAP
-				 * queues which support cache hints.
-				 */
-				crbufs.flags = 0;
-				if (m == V4L2_MEMORY_MMAP)
-					fail_on_test(doioctl(node, VIDIOC_CREATE_BUFS, &crbufs) != EINVAL);
-				else
-					fail_on_test(doioctl(node, VIDIOC_CREATE_BUFS, &crbufs));
-			}
 			q.reqbufs(node);
 
 			fail_on_test(q.create_bufs(node, 1));
@@ -1352,7 +1316,7 @@ int testMmap(struct node *node, struct node *node_m2m_cap, unsigned frame_count,
 			have_createbufs = false;
 		if (have_createbufs) {
 			q.reqbufs(node);
-			q.create_bufs(node, 2, &cur_fmt, V4L2_FLAG_MEMORY_NON_CONSISTENT);
+			q.create_bufs(node, 2, &cur_fmt);
 			fail_on_test(setupMmap(node, q));
 			q.munmap_bufs(node);
 			q.reqbufs(node, 2);
-- 
2.28.0

