All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] v4l-compliance: re-introduce NON_COHERENT and cache hints tests
@ 2021-07-09  9:22 Sergey Senozhatsky
  2021-09-10 12:20 ` Hans Verkuil
  0 siblings, 1 reply; 8+ messages in thread
From: Sergey Senozhatsky @ 2021-07-09  9:22 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Tomasz Figa, Ricardo Ribalda, linux-media, linux-kernel,
	Sergey Senozhatsky

This returns back non-coherent (previously known as NON_COHERENT)
memory flag and buffer cache management hints testing (for VB2_MEMORY_MMAP
buffers).

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 utils/common/cv4l-helpers.h                 |  8 +--
 utils/common/v4l-helpers.h                  |  8 ++-
 utils/v4l2-compliance/v4l2-test-buffers.cpp | 65 ++++++++++++++++++---
 3 files changed, 66 insertions(+), 15 deletions(-)

diff --git a/utils/common/cv4l-helpers.h b/utils/common/cv4l-helpers.h
index 712efde6..3cee372b 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)
+	int reqbufs(cv4l_fd *fd, unsigned count = 0, unsigned int flags = 0)
 	{
-		return v4l_queue_reqbufs(fd->g_v4l_fd(), this, count);
+		return v4l_queue_reqbufs(fd->g_v4l_fd(), this, count, flags);
 	}
 	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)
+	int create_bufs(cv4l_fd *fd, unsigned count, const v4l2_format *fmt = NULL, unsigned int flags = 0)
 	{
-		return v4l_queue_create_bufs(fd->g_v4l_fd(), this, count, fmt);
+		return v4l_queue_create_bufs(fd->g_v4l_fd(), this, count, fmt, flags);
 	}
 	int mmap_bufs(cv4l_fd *fd, unsigned from = 0)
 	{
diff --git a/utils/common/v4l-helpers.h b/utils/common/v4l-helpers.h
index f96b3c38..c09cd987 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)
+		struct v4l_queue *q, unsigned count, unsigned int flags = 0)
 {
 	struct v4l2_requestbuffers reqbufs;
 	int ret;
@@ -1523,6 +1523,7 @@ 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?
@@ -1547,7 +1548,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)
+		const struct v4l2_format *fmt, unsigned int flags = 0)
 {
 	struct v4l2_create_buffers createbufs;
 	int ret;
@@ -1555,6 +1556,7 @@ 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 {
@@ -1733,7 +1735,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);
+	v4l_queue_reqbufs(f, q, 0, 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 e40461bd..6997f40b 100644
--- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
+++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
@@ -663,6 +663,10 @@ 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 coherent;
+
+			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);
@@ -678,8 +682,17 @@ int testReqBufs(struct node *node)
 			reqbufs.count = 1;
 			reqbufs.type = i;
 			reqbufs.memory = m;
+			reqbufs.flags = V4L2_MEMORY_FLAG_NON_COHERENT;
 			fail_on_test(doioctl(node, VIDIOC_REQBUFS, &reqbufs));
-			fail_on_test(check_0(reqbufs.reserved, sizeof(reqbufs.reserved)));
+			coherent = reqbufs.flags & V4L2_MEMORY_FLAG_NON_COHERENT;
+			if (!cache_hints_cap) {
+				fail_on_test(coherent);
+			} else {
+				if (m == V4L2_MEMORY_MMAP)
+					fail_on_test(!coherent);
+				else
+					fail_on_test(coherent);
+			}
 			q.reqbufs(node);
 
 			ret = q.create_bufs(node, 0);
@@ -692,9 +705,32 @@ int testReqBufs(struct node *node)
 			node->g_fmt(crbufs.format, i);
 			crbufs.count = 1;
 			crbufs.memory = m;
+			crbufs.flags = V4L2_MEMORY_FLAG_NON_COHERENT;
 			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());
+
+			coherent = crbufs.flags & V4L2_MEMORY_FLAG_NON_COHERENT;
+			if (!cache_hints_cap) {
+				fail_on_test(coherent);
+			} else {
+				if (m == V4L2_MEMORY_MMAP)
+					fail_on_test(!coherent);
+				else
+					fail_on_test(coherent);
+			}
+
+			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));
@@ -1207,10 +1243,16 @@ static int setupMmap(struct node *node, cv4l_queue &q)
 		fail_on_test(buf.querybuf(node, i));
 		fail_on_test(buf.check(q, Unqueued, i));
 
-		flags = buf.g_flags();
-		flags |= V4L2_BUF_FLAG_NO_CACHE_INVALIDATE;
-		flags |= V4L2_BUF_FLAG_NO_CACHE_CLEAN;
-		buf.s_flags(flags);
+		/*
+		 * Do not set cache hints for all the buffers, but only on
+		 * some of them, so that we can test more cases.
+		 */
+		if (i == 0) {
+			flags = buf.g_flags();
+			flags |= V4L2_BUF_FLAG_NO_CACHE_INVALIDATE;
+			flags |= V4L2_BUF_FLAG_NO_CACHE_CLEAN;
+			buf.s_flags(flags);
+		}
 
 		for (unsigned p = 0; p < buf.g_num_planes(); p++) {
 			// Try a random offset
@@ -1250,8 +1292,15 @@ static int setupMmap(struct node *node, cv4l_queue &q)
 		}
 		flags = buf.g_flags();
 		if (cache_hints) {
-			fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE));
-			fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN));
+			if (i == 0) {
+				/* We do expect cache hints on this buffer */
+				fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE));
+				fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN));
+			} else {
+				/* We expect no cache hints on this buffer */
+				fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE);
+				fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN);
+			}
 		} else if (node->might_support_cache_hints) {
 			fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE);
 			fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN);
@@ -1341,7 +1390,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);
+			q.create_bufs(node, 2, &cur_fmt, V4L2_MEMORY_FLAG_NON_COHERENT);
 			fail_on_test(setupMmap(node, q));
 			q.munmap_bufs(node);
 			q.reqbufs(node, 2);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] v4l-compliance: re-introduce NON_COHERENT and cache hints tests
  2021-07-09  9:22 [PATCH] v4l-compliance: re-introduce NON_COHERENT and cache hints tests Sergey Senozhatsky
@ 2021-09-10 12:20 ` Hans Verkuil
  2021-09-10 12:48   ` Sergey Senozhatsky
  0 siblings, 1 reply; 8+ messages in thread
From: Hans Verkuil @ 2021-09-10 12:20 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Tomasz Figa, Ricardo Ribalda, linux-media, linux-kernel

Hi Sergey,

I've applied the "[PATCHv6 0/8] videobuf2: support new noncontiguous DMA API" series
to my kernel, and applied this patch to v4l-utils.

Running 'test-media vim2m' in contrib/test results in the following compliance failures:

Streaming ioctls:
        test read/write: OK (Not Supported)
        test blocking wait: OK
        Video Capture: Captured 8 buffers
        test MMAP (no poll): OK
        Video Capture: Captured 8 buffers
        test MMAP (select): OK
        Video Capture: Captured 8 buffers
        test MMAP (epoll): OK
        Video Capture: Captured 8 buffers
        test USERPTR (no poll): OK
        Video Capture: Captured 8 buffers
        test USERPTR (select): OK
                fail: v4l2-test-buffers.cpp(1869): !(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE)
                fail: v4l2-test-buffers.cpp(1932): setupDmaBuf(expbuf_node, node, q, exp_q)
        test DMABUF (no poll): FAIL
                fail: v4l2-test-buffers.cpp(1869): !(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE)
                fail: v4l2-test-buffers.cpp(1932): setupDmaBuf(expbuf_node, node, q, exp_q)
        test DMABUF (select): FAIL

The same happens with e.g. vivid, but vim2m is quicker to test.

I'm not sure whether this is a bug in this v4l2-compliance patch or whether it is
a bug in the v6 series, but it should be checked.

Regards,

	Hans

On 09/07/2021 11:22, Sergey Senozhatsky wrote:
> This returns back non-coherent (previously known as NON_COHERENT)
> memory flag and buffer cache management hints testing (for VB2_MEMORY_MMAP
> buffers).
> 
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> ---
>  utils/common/cv4l-helpers.h                 |  8 +--
>  utils/common/v4l-helpers.h                  |  8 ++-
>  utils/v4l2-compliance/v4l2-test-buffers.cpp | 65 ++++++++++++++++++---
>  3 files changed, 66 insertions(+), 15 deletions(-)
> 
> diff --git a/utils/common/cv4l-helpers.h b/utils/common/cv4l-helpers.h
> index 712efde6..3cee372b 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)
> +	int reqbufs(cv4l_fd *fd, unsigned count = 0, unsigned int flags = 0)
>  	{
> -		return v4l_queue_reqbufs(fd->g_v4l_fd(), this, count);
> +		return v4l_queue_reqbufs(fd->g_v4l_fd(), this, count, flags);
>  	}
>  	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)
> +	int create_bufs(cv4l_fd *fd, unsigned count, const v4l2_format *fmt = NULL, unsigned int flags = 0)
>  	{
> -		return v4l_queue_create_bufs(fd->g_v4l_fd(), this, count, fmt);
> +		return v4l_queue_create_bufs(fd->g_v4l_fd(), this, count, fmt, flags);
>  	}
>  	int mmap_bufs(cv4l_fd *fd, unsigned from = 0)
>  	{
> diff --git a/utils/common/v4l-helpers.h b/utils/common/v4l-helpers.h
> index f96b3c38..c09cd987 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)
> +		struct v4l_queue *q, unsigned count, unsigned int flags = 0)
>  {
>  	struct v4l2_requestbuffers reqbufs;
>  	int ret;
> @@ -1523,6 +1523,7 @@ 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?
> @@ -1547,7 +1548,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)
> +		const struct v4l2_format *fmt, unsigned int flags = 0)
>  {
>  	struct v4l2_create_buffers createbufs;
>  	int ret;
> @@ -1555,6 +1556,7 @@ 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 {
> @@ -1733,7 +1735,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);
> +	v4l_queue_reqbufs(f, q, 0, 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 e40461bd..6997f40b 100644
> --- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
> @@ -663,6 +663,10 @@ 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 coherent;
> +
> +			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);
> @@ -678,8 +682,17 @@ int testReqBufs(struct node *node)
>  			reqbufs.count = 1;
>  			reqbufs.type = i;
>  			reqbufs.memory = m;
> +			reqbufs.flags = V4L2_MEMORY_FLAG_NON_COHERENT;
>  			fail_on_test(doioctl(node, VIDIOC_REQBUFS, &reqbufs));
> -			fail_on_test(check_0(reqbufs.reserved, sizeof(reqbufs.reserved)));
> +			coherent = reqbufs.flags & V4L2_MEMORY_FLAG_NON_COHERENT;
> +			if (!cache_hints_cap) {
> +				fail_on_test(coherent);
> +			} else {
> +				if (m == V4L2_MEMORY_MMAP)
> +					fail_on_test(!coherent);
> +				else
> +					fail_on_test(coherent);
> +			}
>  			q.reqbufs(node);
>  
>  			ret = q.create_bufs(node, 0);
> @@ -692,9 +705,32 @@ int testReqBufs(struct node *node)
>  			node->g_fmt(crbufs.format, i);
>  			crbufs.count = 1;
>  			crbufs.memory = m;
> +			crbufs.flags = V4L2_MEMORY_FLAG_NON_COHERENT;
>  			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());
> +
> +			coherent = crbufs.flags & V4L2_MEMORY_FLAG_NON_COHERENT;
> +			if (!cache_hints_cap) {
> +				fail_on_test(coherent);
> +			} else {
> +				if (m == V4L2_MEMORY_MMAP)
> +					fail_on_test(!coherent);
> +				else
> +					fail_on_test(coherent);
> +			}
> +
> +			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));
> @@ -1207,10 +1243,16 @@ static int setupMmap(struct node *node, cv4l_queue &q)
>  		fail_on_test(buf.querybuf(node, i));
>  		fail_on_test(buf.check(q, Unqueued, i));
>  
> -		flags = buf.g_flags();
> -		flags |= V4L2_BUF_FLAG_NO_CACHE_INVALIDATE;
> -		flags |= V4L2_BUF_FLAG_NO_CACHE_CLEAN;
> -		buf.s_flags(flags);
> +		/*
> +		 * Do not set cache hints for all the buffers, but only on
> +		 * some of them, so that we can test more cases.
> +		 */
> +		if (i == 0) {
> +			flags = buf.g_flags();
> +			flags |= V4L2_BUF_FLAG_NO_CACHE_INVALIDATE;
> +			flags |= V4L2_BUF_FLAG_NO_CACHE_CLEAN;
> +			buf.s_flags(flags);
> +		}
>  
>  		for (unsigned p = 0; p < buf.g_num_planes(); p++) {
>  			// Try a random offset
> @@ -1250,8 +1292,15 @@ static int setupMmap(struct node *node, cv4l_queue &q)
>  		}
>  		flags = buf.g_flags();
>  		if (cache_hints) {
> -			fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE));
> -			fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN));
> +			if (i == 0) {
> +				/* We do expect cache hints on this buffer */
> +				fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE));
> +				fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN));
> +			} else {
> +				/* We expect no cache hints on this buffer */
> +				fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE);
> +				fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN);
> +			}
>  		} else if (node->might_support_cache_hints) {
>  			fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE);
>  			fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN);
> @@ -1341,7 +1390,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);
> +			q.create_bufs(node, 2, &cur_fmt, V4L2_MEMORY_FLAG_NON_COHERENT);
>  			fail_on_test(setupMmap(node, q));
>  			q.munmap_bufs(node);
>  			q.reqbufs(node, 2);
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] v4l-compliance: re-introduce NON_COHERENT and cache hints tests
  2021-09-10 12:20 ` Hans Verkuil
@ 2021-09-10 12:48   ` Sergey Senozhatsky
  2021-09-10 13:31     ` Sergey Senozhatsky
  0 siblings, 1 reply; 8+ messages in thread
From: Sergey Senozhatsky @ 2021-09-10 12:48 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Sergey Senozhatsky, Tomasz Figa, Ricardo Ribalda, linux-media,
	linux-kernel

Hello,

On (21/09/10 14:20), Hans Verkuil wrote:
> Hi Sergey,
> 
> I've applied the "[PATCHv6 0/8] videobuf2: support new noncontiguous DMA API" series
> to my kernel, and applied this patch to v4l-utils.
> 
> Running 'test-media vim2m' in contrib/test results in the following compliance failures:
> 
> Streaming ioctls:
>         test read/write: OK (Not Supported)
>         test blocking wait: OK
>         Video Capture: Captured 8 buffers
>         test MMAP (no poll): OK
>         Video Capture: Captured 8 buffers
>         test MMAP (select): OK
>         Video Capture: Captured 8 buffers
>         test MMAP (epoll): OK
>         Video Capture: Captured 8 buffers
>         test USERPTR (no poll): OK
>         Video Capture: Captured 8 buffers
>         test USERPTR (select): OK
>                 fail: v4l2-test-buffers.cpp(1869): !(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE)
>                 fail: v4l2-test-buffers.cpp(1932): setupDmaBuf(expbuf_node, node, q, exp_q)
>         test DMABUF (no poll): FAIL
>                 fail: v4l2-test-buffers.cpp(1869): !(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE)
>                 fail: v4l2-test-buffers.cpp(1932): setupDmaBuf(expbuf_node, node, q, exp_q)
>         test DMABUF (select): FAIL
> 
> The same happens with e.g. vivid, but vim2m is quicker to test.
> 
> I'm not sure whether this is a bug in this v4l2-compliance patch or whether it is
> a bug in the v6 series, but it should be checked.

Looking into it now. I ran v4l2-compliance, but not "contrib/test/test-media"

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] v4l-compliance: re-introduce NON_COHERENT and cache hints tests
  2021-09-10 12:48   ` Sergey Senozhatsky
@ 2021-09-10 13:31     ` Sergey Senozhatsky
  2021-09-10 14:16       ` Sergey Senozhatsky
  0 siblings, 1 reply; 8+ messages in thread
From: Sergey Senozhatsky @ 2021-09-10 13:31 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Tomasz Figa, Ricardo Ribalda, linux-media, linux-kernel,
	Sergey Senozhatsky

On (21/09/10 21:48), Sergey Senozhatsky wrote:
> > Streaming ioctls:
> >         test read/write: OK (Not Supported)
> >         test blocking wait: OK
> >         Video Capture: Captured 8 buffers
> >         test MMAP (no poll): OK
> >         Video Capture: Captured 8 buffers
> >         test MMAP (select): OK
> >         Video Capture: Captured 8 buffers
> >         test MMAP (epoll): OK
> >         Video Capture: Captured 8 buffers
> >         test USERPTR (no poll): OK
> >         Video Capture: Captured 8 buffers
> >         test USERPTR (select): OK
> >                 fail: v4l2-test-buffers.cpp(1869): !(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE)
> >                 fail: v4l2-test-buffers.cpp(1932): setupDmaBuf(expbuf_node, node, q, exp_q)
> >         test DMABUF (no poll): FAIL
> >                 fail: v4l2-test-buffers.cpp(1869): !(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE)
> >                 fail: v4l2-test-buffers.cpp(1932): setupDmaBuf(expbuf_node, node, q, exp_q)
> >         test DMABUF (select): FAIL
> > 
> > The same happens with e.g. vivid, but vim2m is quicker to test.
> > 
> > I'm not sure whether this is a bug in this v4l2-compliance patch or whether it is
> > a bug in the v6 series, but it should be checked.
> 
> Looking into it now. I ran v4l2-compliance, but not "contrib/test/test-media"

AFAICT the problem is in v4l2-compliance patch.

We clear request flags if queue does not support user-space cache hints:

	 q->allow_cache_hints && q->memory == VB2_MEMORY_MMAP

But for DMABUF buffers (only) we set cache hints internally in
set_buffer_cache_hints() and always skip cache sync/flush on
prepare/finish regardless of what is passed from the user-space:

       if (q->memory == VB2_MEMORY_DMABUF) {
               vb->skip_cache_sync_on_finish = 1;
               vb->skip_cache_sync_on_prepare = 1;
               return;
       }

Technically we don't support user-space cache hints for DMABUF, so we
clear passed user-space cache hint flags.

I think the fix should look like this (tested with "test-media vivid"):

---

diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp
index 9b87c90f..baa306f1 100644
--- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
+++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
@@ -1865,9 +1865,10 @@ static int setupDmaBuf(struct node *expbuf_node, struct node *node,
 				fail_on_test(!buf.g_bytesused(p));
 		}
 		flags = buf.g_flags();
-		/* We always skip cache sync/flush for DMABUF memory type */
-		fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE));
-		fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN));
+
+		/* Make sure that flags are cleared */
+		fail_on_test((flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE));
+		fail_on_test((flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN));
 		fail_on_test(flags & V4L2_BUF_FLAG_DONE);
 		fail_on_test(buf.querybuf(node, i));
 		fail_on_test(buf.check(q, Queued, i));

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] v4l-compliance: re-introduce NON_COHERENT and cache hints tests
  2021-09-10 13:31     ` Sergey Senozhatsky
@ 2021-09-10 14:16       ` Sergey Senozhatsky
  0 siblings, 0 replies; 8+ messages in thread
From: Sergey Senozhatsky @ 2021-09-10 14:16 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Tomasz Figa, Ricardo Ribalda, linux-media, linux-kernel,
	Sergey Senozhatsky

On (21/09/10 22:31), Sergey Senozhatsky wrote:
> > Looking into it now. I ran v4l2-compliance, but not "contrib/test/test-media"
> 
> AFAICT the problem is in v4l2-compliance patch.
> 
> We clear request flags if queue does not support user-space cache hints:
> 
> 	 q->allow_cache_hints && q->memory == VB2_MEMORY_MMAP
> 
> But for DMABUF buffers (only) we set cache hints internally in
> set_buffer_cache_hints() and always skip cache sync/flush on
> prepare/finish regardless of what is passed from the user-space:
> 
>        if (q->memory == VB2_MEMORY_DMABUF) {
>                vb->skip_cache_sync_on_finish = 1;
>                vb->skip_cache_sync_on_prepare = 1;
>                return;
>        }
> 
> Technically we don't support user-space cache hints for DMABUF, so we
> clear passed user-space cache hint flags.
> 
> I think the fix should look like this (tested with "test-media vivid"):
> 
> ---
> 
> diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp
> index 9b87c90f..baa306f1 100644
> --- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
> @@ -1865,9 +1865,10 @@ static int setupDmaBuf(struct node *expbuf_node, struct node *node,
>  				fail_on_test(!buf.g_bytesused(p));
>  		}
>  		flags = buf.g_flags();
> -		/* We always skip cache sync/flush for DMABUF memory type */
> -		fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE));
> -		fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN));
> +
> +		/* Make sure that flags are cleared */
> +		fail_on_test((flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE));
> +		fail_on_test((flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN));
>  		fail_on_test(flags & V4L2_BUF_FLAG_DONE);
>  		fail_on_test(buf.querybuf(node, i));
>  		fail_on_test(buf.check(q, Queued, i));


Alternatively, we can do something like below on the kernel side instead:
do nothing in v4l2 for DMABUF and preserve b->flags (if user-space has
passed cache management flags).

// But I think it'll be better to clear b->flags cache hints for DMABUF. To
// indicate that we don't accept cache-hints for DMABUF.


---

diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 6edf4508c636..fc8e31b7dced 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -345,6 +345,13 @@ static void set_buffer_cache_hints(struct vb2_queue *q,
 				   struct vb2_buffer *vb,
 				   struct v4l2_buffer *b)
 {
+	/*
+	 * No user-space cache hints for DMABUF, but we preserve b->flags
+	 * cache hints (if user-space has passed any).
+	 */
+	if (q->memory == VB2_MEMORY_DMABUF)
+		return;
+
 	if (!vb2_queue_allows_cache_hints(q)) {
 		/*
 		 * Clear buffer cache flags if queue does not support user

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH] v4l-compliance: re-introduce NON_COHERENT and cache hints tests
@ 2021-04-27 13:20 Sergey Senozhatsky
  0 siblings, 0 replies; 8+ messages in thread
From: Sergey Senozhatsky @ 2021-04-27 13:20 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Tomasz Figa, Ricardo Ribalda, linux-media, linux-kernel,
	Sergey Senozhatsky

This returns back non-coherent (previously known as NON_COHERENT)
memory flag and buffer cache management hints testing (for VB2_MEMORY_MMAP
buffers).

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 utils/common/cv4l-helpers.h                 |  8 +--
 utils/common/v4l-helpers.h                  |  8 ++-
 utils/v4l2-compliance/v4l2-test-buffers.cpp | 65 ++++++++++++++++++---
 3 files changed, 66 insertions(+), 15 deletions(-)

diff --git a/utils/common/cv4l-helpers.h b/utils/common/cv4l-helpers.h
index 712efde6..3cee372b 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)
+	int reqbufs(cv4l_fd *fd, unsigned count = 0, unsigned int flags = 0)
 	{
-		return v4l_queue_reqbufs(fd->g_v4l_fd(), this, count);
+		return v4l_queue_reqbufs(fd->g_v4l_fd(), this, count, flags);
 	}
 	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)
+	int create_bufs(cv4l_fd *fd, unsigned count, const v4l2_format *fmt = NULL, unsigned int flags = 0)
 	{
-		return v4l_queue_create_bufs(fd->g_v4l_fd(), this, count, fmt);
+		return v4l_queue_create_bufs(fd->g_v4l_fd(), this, count, fmt, flags);
 	}
 	int mmap_bufs(cv4l_fd *fd, unsigned from = 0)
 	{
diff --git a/utils/common/v4l-helpers.h b/utils/common/v4l-helpers.h
index f96b3c38..c09cd987 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)
+		struct v4l_queue *q, unsigned count, unsigned int flags = 0)
 {
 	struct v4l2_requestbuffers reqbufs;
 	int ret;
@@ -1523,6 +1523,7 @@ 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?
@@ -1547,7 +1548,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)
+		const struct v4l2_format *fmt, unsigned int flags = 0)
 {
 	struct v4l2_create_buffers createbufs;
 	int ret;
@@ -1555,6 +1556,7 @@ 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 {
@@ -1733,7 +1735,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);
+	v4l_queue_reqbufs(f, q, 0, 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 e40461bd..6997f40b 100644
--- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
+++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
@@ -663,6 +663,10 @@ 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);
@@ -678,8 +682,17 @@ int testReqBufs(struct node *node)
 			reqbufs.count = 1;
 			reqbufs.type = i;
 			reqbufs.memory = m;
+			reqbufs.flags = V4L2_MEMORY_FLAG_NON_COHERENT;
 			fail_on_test(doioctl(node, VIDIOC_REQBUFS, &reqbufs));
-			fail_on_test(check_0(reqbufs.reserved, sizeof(reqbufs.reserved)));
+			consistent = reqbufs.flags & V4L2_MEMORY_FLAG_NON_COHERENT;
+			if (!cache_hints_cap) {
+				fail_on_test(consistent);
+			} else {
+				if (m == V4L2_MEMORY_MMAP)
+					fail_on_test(!consistent);
+				else
+					fail_on_test(consistent);
+			}
 			q.reqbufs(node);
 
 			ret = q.create_bufs(node, 0);
@@ -692,9 +705,32 @@ int testReqBufs(struct node *node)
 			node->g_fmt(crbufs.format, i);
 			crbufs.count = 1;
 			crbufs.memory = m;
+			crbufs.flags = V4L2_MEMORY_FLAG_NON_COHERENT;
 			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_MEMORY_FLAG_NON_COHERENT;
+			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));
@@ -1207,10 +1243,16 @@ static int setupMmap(struct node *node, cv4l_queue &q)
 		fail_on_test(buf.querybuf(node, i));
 		fail_on_test(buf.check(q, Unqueued, i));
 
-		flags = buf.g_flags();
-		flags |= V4L2_BUF_FLAG_NO_CACHE_INVALIDATE;
-		flags |= V4L2_BUF_FLAG_NO_CACHE_CLEAN;
-		buf.s_flags(flags);
+		/*
+		 * Do not set cache hints for all the buffers, but only on
+		 * some of them, so that we can test more cases.
+		 */
+		if (i == 0) {
+			flags = buf.g_flags();
+			flags |= V4L2_BUF_FLAG_NO_CACHE_INVALIDATE;
+			flags |= V4L2_BUF_FLAG_NO_CACHE_CLEAN;
+			buf.s_flags(flags);
+		}
 
 		for (unsigned p = 0; p < buf.g_num_planes(); p++) {
 			// Try a random offset
@@ -1250,8 +1292,15 @@ static int setupMmap(struct node *node, cv4l_queue &q)
 		}
 		flags = buf.g_flags();
 		if (cache_hints) {
-			fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE));
-			fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN));
+			if (i == 0) {
+				/* We do expect cache hints on this buffer */
+				fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE));
+				fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN));
+			} else {
+				/* We expect no cache hints on this buffer */
+				fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE);
+				fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN);
+			}
 		} else if (node->might_support_cache_hints) {
 			fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE);
 			fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN);
@@ -1341,7 +1390,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);
+			q.create_bufs(node, 2, &cur_fmt, V4L2_MEMORY_FLAG_NON_COHERENT);
 			fail_on_test(setupMmap(node, q));
 			q.munmap_bufs(node);
 			q.reqbufs(node, 2);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] v4l-compliance: re-introduce NON_COHERENT and cache hints tests
  2021-03-02  0:49 ` [PATCH] v4l-compliance: re-introduce NON_COHERENT and cache hints tests Sergey Senozhatsky
@ 2021-03-02  0:50   ` Sergey Senozhatsky
  0 siblings, 0 replies; 8+ messages in thread
From: Sergey Senozhatsky @ 2021-03-02  0:50 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Hans Verkuil, Tomasz Figa, Christoph Hellwig,
	Mauro Carvalho Chehab, linux-media, linux-kernel

On (21/03/02 09:49), Sergey Senozhatsky wrote:
> This returns back non-coherent (previously known as NON_COHERENT)
							^^^
							NON_CONSISTENT...

> memory flag and buffer cache management hints testing (for VB2_MEMORY_MMAP
> buffers).

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] v4l-compliance: re-introduce NON_COHERENT and cache hints tests
  2021-03-02  0:46 [PATCH 0/8] videobuf2: support new noncontiguous DMA API Sergey Senozhatsky
@ 2021-03-02  0:49 ` Sergey Senozhatsky
  2021-03-02  0:50   ` Sergey Senozhatsky
  0 siblings, 1 reply; 8+ messages in thread
From: Sergey Senozhatsky @ 2021-03-02  0:49 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Tomasz Figa, Christoph Hellwig, Mauro Carvalho Chehab,
	linux-media, linux-kernel, Sergey Senozhatsky

This returns back non-coherent (previously known as NON_COHERENT)
memory flag and buffer cache management hints testing (for VB2_MEMORY_MMAP
buffers).

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 utils/common/cv4l-helpers.h                 |  8 +--
 utils/common/v4l-helpers.h                  |  8 ++-
 utils/v4l2-compliance/v4l2-test-buffers.cpp | 65 ++++++++++++++++++---
 3 files changed, 66 insertions(+), 15 deletions(-)

diff --git a/utils/common/cv4l-helpers.h b/utils/common/cv4l-helpers.h
index 712efde6..3cee372b 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)
+	int reqbufs(cv4l_fd *fd, unsigned count = 0, unsigned int flags = 0)
 	{
-		return v4l_queue_reqbufs(fd->g_v4l_fd(), this, count);
+		return v4l_queue_reqbufs(fd->g_v4l_fd(), this, count, flags);
 	}
 	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)
+	int create_bufs(cv4l_fd *fd, unsigned count, const v4l2_format *fmt = NULL, unsigned int flags = 0)
 	{
-		return v4l_queue_create_bufs(fd->g_v4l_fd(), this, count, fmt);
+		return v4l_queue_create_bufs(fd->g_v4l_fd(), this, count, fmt, flags);
 	}
 	int mmap_bufs(cv4l_fd *fd, unsigned from = 0)
 	{
diff --git a/utils/common/v4l-helpers.h b/utils/common/v4l-helpers.h
index f96b3c38..c09cd987 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)
+		struct v4l_queue *q, unsigned count, unsigned int flags = 0)
 {
 	struct v4l2_requestbuffers reqbufs;
 	int ret;
@@ -1523,6 +1523,7 @@ 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?
@@ -1547,7 +1548,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)
+		const struct v4l2_format *fmt, unsigned int flags = 0)
 {
 	struct v4l2_create_buffers createbufs;
 	int ret;
@@ -1555,6 +1556,7 @@ 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 {
@@ -1733,7 +1735,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);
+	v4l_queue_reqbufs(f, q, 0, 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 e40461bd..6555c0cb 100644
--- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
+++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
@@ -663,6 +663,10 @@ 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);
@@ -678,8 +682,17 @@ int testReqBufs(struct node *node)
 			reqbufs.count = 1;
 			reqbufs.type = i;
 			reqbufs.memory = m;
+			reqbufs.flags = V4L2_FLAG_MEMORY_NON_COHERENT;
 			fail_on_test(doioctl(node, VIDIOC_REQBUFS, &reqbufs));
-			fail_on_test(check_0(reqbufs.reserved, sizeof(reqbufs.reserved)));
+			consistent = reqbufs.flags & V4L2_FLAG_MEMORY_NON_COHERENT;
+			if (!cache_hints_cap) {
+				fail_on_test(consistent);
+			} else {
+				if (m == V4L2_MEMORY_MMAP)
+					fail_on_test(!consistent);
+				else
+					fail_on_test(consistent);
+			}
 			q.reqbufs(node);
 
 			ret = q.create_bufs(node, 0);
@@ -692,9 +705,32 @@ int testReqBufs(struct node *node)
 			node->g_fmt(crbufs.format, i);
 			crbufs.count = 1;
 			crbufs.memory = m;
+			crbufs.flags = V4L2_FLAG_MEMORY_NON_COHERENT;
 			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_COHERENT;
+			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));
@@ -1207,10 +1243,16 @@ static int setupMmap(struct node *node, cv4l_queue &q)
 		fail_on_test(buf.querybuf(node, i));
 		fail_on_test(buf.check(q, Unqueued, i));
 
-		flags = buf.g_flags();
-		flags |= V4L2_BUF_FLAG_NO_CACHE_INVALIDATE;
-		flags |= V4L2_BUF_FLAG_NO_CACHE_CLEAN;
-		buf.s_flags(flags);
+		/*
+		 * Do not set cache hints for all the buffers, but only on
+		 * some of them, so that we can test more cases.
+		 */
+		if (i == 0) {
+			flags = buf.g_flags();
+			flags |= V4L2_BUF_FLAG_NO_CACHE_INVALIDATE;
+			flags |= V4L2_BUF_FLAG_NO_CACHE_CLEAN;
+			buf.s_flags(flags);
+		}
 
 		for (unsigned p = 0; p < buf.g_num_planes(); p++) {
 			// Try a random offset
@@ -1250,8 +1292,15 @@ static int setupMmap(struct node *node, cv4l_queue &q)
 		}
 		flags = buf.g_flags();
 		if (cache_hints) {
-			fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE));
-			fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN));
+			if (i == 0) {
+				/* We do expect cache hints on this buffer */
+				fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE));
+				fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN));
+			} else {
+				/* We expect no cache hints on this buffer */
+				fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE);
+				fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN);
+			}
 		} else if (node->might_support_cache_hints) {
 			fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE);
 			fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN);
@@ -1341,7 +1390,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);
+			q.create_bufs(node, 2, &cur_fmt, V4L2_FLAG_MEMORY_NON_COHERENT);
 			fail_on_test(setupMmap(node, q));
 			q.munmap_bufs(node);
 			q.reqbufs(node, 2);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-09-10 14:16 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-09  9:22 [PATCH] v4l-compliance: re-introduce NON_COHERENT and cache hints tests Sergey Senozhatsky
2021-09-10 12:20 ` Hans Verkuil
2021-09-10 12:48   ` Sergey Senozhatsky
2021-09-10 13:31     ` Sergey Senozhatsky
2021-09-10 14:16       ` Sergey Senozhatsky
  -- strict thread matches above, loose matches on Subject: below --
2021-04-27 13:20 Sergey Senozhatsky
2021-03-02  0:46 [PATCH 0/8] videobuf2: support new noncontiguous DMA API Sergey Senozhatsky
2021-03-02  0:49 ` [PATCH] v4l-compliance: re-introduce NON_COHERENT and cache hints tests Sergey Senozhatsky
2021-03-02  0:50   ` Sergey Senozhatsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.