diff mbox series

v4l2-compliance: add EOPNOTSUPP for create_bufs

Message ID 20231003224536.13199-1-deborah.brouwer@collabora.com
State New
Headers show
Series v4l2-compliance: add EOPNOTSUPP for create_bufs | expand

Commit Message

Deborah Brouwer Oct. 3, 2023, 10:45 p.m. UTC
If VIDIOC_CREATE_BUFS is supported on one queue but not the other, then
the driver should return EOPNOTSUPP for the unsupported queue only.

Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>
---
On the wave5 driver, v4l2-compliance -d0 -v now shows:
<snip>
Buffer ioctls:
        info: test buftype Video Capture Multiplanar
        info: VIDIOC_CREATE_BUFS not supported for Video Capture Multiplanar
        info: test buftype Video Output Multiplanar
    test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
    test VIDIOC_EXPBUF: OK
        info: could not test the Request API, no suitable control found
    test Requests: OK (Not Supported)

Total for wave5-dec device /dev/video0: 45, Succeeded: 45, Failed: 0, Warnings: 0

 utils/v4l2-compliance/v4l2-compliance.h     | 1 +
 utils/v4l2-compliance/v4l2-test-buffers.cpp | 8 ++++++++
 2 files changed, 9 insertions(+)

Comments

Hans Verkuil Oct. 4, 2023, 9:44 a.m. UTC | #1
Hi Deb,

On 10/4/23 00:45, Deborah Brouwer wrote:
> If VIDIOC_CREATE_BUFS is supported on one queue but not the other, then
> the driver should return EOPNOTSUPP for the unsupported queue only.
> 
> Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>
> ---
> On the wave5 driver, v4l2-compliance -d0 -v now shows:
> <snip>
> Buffer ioctls:
>         info: test buftype Video Capture Multiplanar
>         info: VIDIOC_CREATE_BUFS not supported for Video Capture Multiplanar
>         info: test buftype Video Output Multiplanar
>     test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
>     test VIDIOC_EXPBUF: OK
>         info: could not test the Request API, no suitable control found
>     test Requests: OK (Not Supported)
> 
> Total for wave5-dec device /dev/video0: 45, Succeeded: 45, Failed: 0, Warnings: 0
> 
>  utils/v4l2-compliance/v4l2-compliance.h     | 1 +
>  utils/v4l2-compliance/v4l2-test-buffers.cpp | 8 ++++++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/utils/v4l2-compliance/v4l2-compliance.h b/utils/v4l2-compliance/v4l2-compliance.h
> index 7caf254b..99c98916 100644
> --- a/utils/v4l2-compliance/v4l2-compliance.h
> +++ b/utils/v4l2-compliance/v4l2-compliance.h
> @@ -165,6 +165,7 @@ struct base_node {
>  	bool supports_orphaned_bufs;
>  	// support for this was introduced in 5.9
>  	bool might_support_cache_hints;
> +	bool create_bufs_on_one_queue_only;

No need to add this here.

>  };
>  
>  struct node : public base_node, public cv4l_fd {
> diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp
> index 6d592c9b..e709580b 100644
> --- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
> @@ -693,6 +693,14 @@ int testReqBufs(struct node *node)
>  				warn("VIDIOC_CREATE_BUFS not supported\n");
>  				break;
>  			}
> +			if (ret == EOPNOTSUPP) {
> +				/* VIDIOC_CREATE_BUFS is supported on one queue but not the other. */
> +				fail_on_test(node->create_bufs_on_one_queue_only);
> +				node->create_bufs_on_one_queue_only = true;
> +				info("VIDIOC_CREATE_BUFS not supported for %s\n",
> +				     buftype2s(q.g_type()).c_str());
> +				break;
> +			}

It is sufficient to have it as a local variable in this function.

>  
>  			memset(&crbufs, 0xff, sizeof(crbufs));
>  			node->g_fmt(crbufs.format, i);

In any case, the logic needs to be improved a bit:

keep track of the q.create_bufs(node, 0) return values: crbufs_enotty_cnt,
crbufs_eopnotsupp_cnt and crbufs_ok_cnt (for any ohter return values).

Fail if (crbufs_enotty_cnt && (crbufs_eopnotsupp_cnt + crbufs_ok_cnt), since
if ENOTTY is returned once, it has to be returned always.

Also fail if (crbufs_eopnotsupp_cnt && !crbufs_ok_cnt), since EOPNOTSUPP indicates
that it is supported in some cases, but not others, so you need to see at least
some successful calls.

I think that is a better test, because that checks for ENOTTY abuse as well.

Regards,

	Hans
diff mbox series

Patch

diff --git a/utils/v4l2-compliance/v4l2-compliance.h b/utils/v4l2-compliance/v4l2-compliance.h
index 7caf254b..99c98916 100644
--- a/utils/v4l2-compliance/v4l2-compliance.h
+++ b/utils/v4l2-compliance/v4l2-compliance.h
@@ -165,6 +165,7 @@  struct base_node {
 	bool supports_orphaned_bufs;
 	// support for this was introduced in 5.9
 	bool might_support_cache_hints;
+	bool create_bufs_on_one_queue_only;
 };
 
 struct node : public base_node, public cv4l_fd {
diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp
index 6d592c9b..e709580b 100644
--- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
+++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
@@ -693,6 +693,14 @@  int testReqBufs(struct node *node)
 				warn("VIDIOC_CREATE_BUFS not supported\n");
 				break;
 			}
+			if (ret == EOPNOTSUPP) {
+				/* VIDIOC_CREATE_BUFS is supported on one queue but not the other. */
+				fail_on_test(node->create_bufs_on_one_queue_only);
+				node->create_bufs_on_one_queue_only = true;
+				info("VIDIOC_CREATE_BUFS not supported for %s\n",
+				     buftype2s(q.g_type()).c_str());
+				break;
+			}
 
 			memset(&crbufs, 0xff, sizeof(crbufs));
 			node->g_fmt(crbufs.format, i);