[v6,5/8] v4l2-ctl/compliance: Add simple routing test
Commit Message
Add a simple test for VIDIOC_SUBDEV_G_ROUTING/VIDIOC_SUBDEV_S_ROUTING.
We can't (at least at the moment) really know here what kind of routings
the driver would accept, but we can test a VIDIOC_SUBDEV_G_ROUTING call,
followed by a VIDIOC_SUBDEV_S_ROUTING call with the routing we
received.
Additionally, we can check that the returned pads and flags look fine,
and also that setting obviously invalid routing will fail.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
utils/v4l2-compliance/v4l2-compliance.cpp | 12 ++++
utils/v4l2-compliance/v4l2-compliance.h | 1 +
utils/v4l2-compliance/v4l2-test-subdevs.cpp | 74 +++++++++++++++++++++
3 files changed, 87 insertions(+)
Comments
On 20/07/2023 09:50, Tomi Valkeinen wrote:
> Add a simple test for VIDIOC_SUBDEV_G_ROUTING/VIDIOC_SUBDEV_S_ROUTING.
>
> We can't (at least at the moment) really know here what kind of routings
> the driver would accept, but we can test a VIDIOC_SUBDEV_G_ROUTING call,
> followed by a VIDIOC_SUBDEV_S_ROUTING call with the routing we
> received.
>
> Additionally, we can check that the returned pads and flags look fine,
> and also that setting obviously invalid routing will fail.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
> utils/v4l2-compliance/v4l2-compliance.cpp | 12 ++++
> utils/v4l2-compliance/v4l2-compliance.h | 1 +
> utils/v4l2-compliance/v4l2-test-subdevs.cpp | 74 +++++++++++++++++++++
> 3 files changed, 87 insertions(+)
>
> diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp
> index e8359b2f..4b232314 100644
> --- a/utils/v4l2-compliance/v4l2-compliance.cpp
> +++ b/utils/v4l2-compliance/v4l2-compliance.cpp
> @@ -1249,6 +1249,18 @@ void testNode(struct node &node, struct node &node_m2m_cap, struct node &expbuf_
> node.is_passthrough_subdev = has_source && has_sink;
>
> if (has_routes) {
> + printf("Sub-Device routing ioctls:\n");
> +
> + for (unsigned which = V4L2_SUBDEV_FORMAT_TRY;
> + which <= V4L2_SUBDEV_FORMAT_ACTIVE; which++) {
> +
> + printf("\ttest %s VIDIOC_SUBDEV_G_ROUTING/VIDIOC_SUBDEV_S_ROUTING: %s\n",
> + which ? "Active" : "Try",
> + ok(testSubDevRouting(&node, which)));
> + }
> +
> + printf("\n");
> +
> for (unsigned which = V4L2_SUBDEV_FORMAT_TRY;
> which <= V4L2_SUBDEV_FORMAT_ACTIVE; which++) {
>
> diff --git a/utils/v4l2-compliance/v4l2-compliance.h b/utils/v4l2-compliance/v4l2-compliance.h
> index 0cd43980..35b2274b 100644
> --- a/utils/v4l2-compliance/v4l2-compliance.h
> +++ b/utils/v4l2-compliance/v4l2-compliance.h
> @@ -375,6 +375,7 @@ int testSubDevEnum(struct node *node, unsigned which, unsigned pad, unsigned str
> int testSubDevFormat(struct node *node, unsigned which, unsigned pad, unsigned stream);
> int testSubDevSelection(struct node *node, unsigned which, unsigned pad, unsigned stream);
> int testSubDevFrameInterval(struct node *node, unsigned pad, unsigned stream);
> +int testSubDevRouting(struct node *node, unsigned which);
>
> // Buffer ioctl tests
> int testReqBufs(struct node *node);
> diff --git a/utils/v4l2-compliance/v4l2-test-subdevs.cpp b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
> index 5545b0dc..e59d67f7 100644
> --- a/utils/v4l2-compliance/v4l2-test-subdevs.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
> @@ -551,3 +551,77 @@ int testSubDevSelection(struct node *node, unsigned which, unsigned pad, unsigne
>
> return have_sel ? 0 : ENOTTY;
> }
> +
> +int testSubDevRouting(struct node *node, unsigned which)
> +{
> + const uint32_t all_route_flags_mask = V4L2_SUBDEV_ROUTE_FL_ACTIVE;
> + struct v4l2_subdev_routing routing = {};
> + struct v4l2_subdev_route routes[NUM_ROUTES_MAX] = {};
> + unsigned int i;
> + int ret;
> +
> + routing.which = which;
> + routing.routes = (__u64)&routes;
> + routing.num_routes = 0;
> + memset(routing.reserved, 0xff, sizeof(routing.reserved));
> +
> + /*
> + * First test that G_ROUTING either returns success, or ENOSPC and
> + * updates num_routes.
> + */
> +
> + ret = doioctl(node, VIDIOC_SUBDEV_G_ROUTING, &routing);
> + fail_on_test(ret && ret != ENOSPC);
> + fail_on_test(ret == ENOSPC && routing.num_routes == 0);
> + fail_on_test(check_0(routing.reserved, sizeof(routing.reserved)));
> +
> + if (routing.num_routes)
> + return 0;
Shouldn't this be 'if (!routing.num_routes)'?
> +
> + /* Then get the actual routes */
> +
> + routing.num_routes = NUM_ROUTES_MAX;
> + fail_on_test(doioctl(node, VIDIOC_SUBDEV_G_ROUTING, &routing));
I assume that num_routes is always updated to the actual number of routes, right?
That's not actually explained clearly in the spec. It says that if the app provided
num_routes is too small, then it is updated, but it says nothing about what happens
if the app provided value is too large.
Assuming I am right, then I would rewrite this code as follows:
__u32 num_routes = routing.num_routes;
routing.num_routes = num_routes + 1;
fail_on_test(doioctl(node, VIDIOC_SUBDEV_G_ROUTING, &routing));
fail_on_test(routing.num_routes != num_routes);
> +
> + /* Check the validity of route pads and flags */
> +
> + if (node->pads) {
> + for (i = 0; i < routing.num_routes; ++i) {
> + const struct v4l2_subdev_route *route = &routes[i];
> + const struct media_pad_desc *sink;
> + const struct media_pad_desc *source;
> +
> + fail_on_test(route->sink_pad >= node->entity.pads);
> + fail_on_test(route->source_pad >= node->entity.pads);
> +
> + sink = &node->pads[route->sink_pad];
> + source = &node->pads[route->source_pad];
> +
> + fail_on_test(!(sink->flags & MEDIA_PAD_FL_SINK));
> + fail_on_test(!(source->flags & MEDIA_PAD_FL_SOURCE));
> + fail_on_test(route->flags & ~all_route_flags_mask);
> + }
> + }
> +
> + /* Set the same routes back, which should always succeed. */
> +
> + memset(routing.reserved, 0xff, sizeof(routing.reserved));
> + fail_on_test(doioctl(node, VIDIOC_SUBDEV_S_ROUTING, &routing));
> + fail_on_test(check_0(routing.reserved, sizeof(routing.reserved)));
> +
> + /* Test setting invalid pads */
> +
> + if (node->pads) {
> + for (i = 0; i < routing.num_routes; ++i) {
> + struct v4l2_subdev_route *route = &routes[i];
> +
> + route->sink_pad = node->entity.pads + 1;
> + }
> +
> + memset(routing.reserved, 0xff, sizeof(routing.reserved));
> + fail_on_test(doioctl(node, VIDIOC_SUBDEV_S_ROUTING, &routing) != EINVAL);
> + fail_on_test(check_0(routing.reserved, sizeof(routing.reserved)));
> + }
> +
> + return 0;
> +}
Regards,
Hans
On 02/08/2023 16:54, Hans Verkuil wrote:
> On 20/07/2023 09:50, Tomi Valkeinen wrote:
>> Add a simple test for VIDIOC_SUBDEV_G_ROUTING/VIDIOC_SUBDEV_S_ROUTING.
>>
>> We can't (at least at the moment) really know here what kind of routings
>> the driver would accept, but we can test a VIDIOC_SUBDEV_G_ROUTING call,
>> followed by a VIDIOC_SUBDEV_S_ROUTING call with the routing we
>> received.
>>
>> Additionally, we can check that the returned pads and flags look fine,
>> and also that setting obviously invalid routing will fail.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>> utils/v4l2-compliance/v4l2-compliance.cpp | 12 ++++
>> utils/v4l2-compliance/v4l2-compliance.h | 1 +
>> utils/v4l2-compliance/v4l2-test-subdevs.cpp | 74 +++++++++++++++++++++
>> 3 files changed, 87 insertions(+)
>>
>> diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp
>> index e8359b2f..4b232314 100644
>> --- a/utils/v4l2-compliance/v4l2-compliance.cpp
>> +++ b/utils/v4l2-compliance/v4l2-compliance.cpp
>> @@ -1249,6 +1249,18 @@ void testNode(struct node &node, struct node &node_m2m_cap, struct node &expbuf_
>> node.is_passthrough_subdev = has_source && has_sink;
>>
>> if (has_routes) {
>> + printf("Sub-Device routing ioctls:\n");
>> +
>> + for (unsigned which = V4L2_SUBDEV_FORMAT_TRY;
>> + which <= V4L2_SUBDEV_FORMAT_ACTIVE; which++) {
>> +
>> + printf("\ttest %s VIDIOC_SUBDEV_G_ROUTING/VIDIOC_SUBDEV_S_ROUTING: %s\n",
>> + which ? "Active" : "Try",
>> + ok(testSubDevRouting(&node, which)));
>> + }
>> +
>> + printf("\n");
>> +
>> for (unsigned which = V4L2_SUBDEV_FORMAT_TRY;
>> which <= V4L2_SUBDEV_FORMAT_ACTIVE; which++) {
>>
>> diff --git a/utils/v4l2-compliance/v4l2-compliance.h b/utils/v4l2-compliance/v4l2-compliance.h
>> index 0cd43980..35b2274b 100644
>> --- a/utils/v4l2-compliance/v4l2-compliance.h
>> +++ b/utils/v4l2-compliance/v4l2-compliance.h
>> @@ -375,6 +375,7 @@ int testSubDevEnum(struct node *node, unsigned which, unsigned pad, unsigned str
>> int testSubDevFormat(struct node *node, unsigned which, unsigned pad, unsigned stream);
>> int testSubDevSelection(struct node *node, unsigned which, unsigned pad, unsigned stream);
>> int testSubDevFrameInterval(struct node *node, unsigned pad, unsigned stream);
>> +int testSubDevRouting(struct node *node, unsigned which);
>>
>> // Buffer ioctl tests
>> int testReqBufs(struct node *node);
>> diff --git a/utils/v4l2-compliance/v4l2-test-subdevs.cpp b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
>> index 5545b0dc..e59d67f7 100644
>> --- a/utils/v4l2-compliance/v4l2-test-subdevs.cpp
>> +++ b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
>> @@ -551,3 +551,77 @@ int testSubDevSelection(struct node *node, unsigned which, unsigned pad, unsigne
>>
>> return have_sel ? 0 : ENOTTY;
>> }
>> +
>> +int testSubDevRouting(struct node *node, unsigned which)
>> +{
>> + const uint32_t all_route_flags_mask = V4L2_SUBDEV_ROUTE_FL_ACTIVE;
>> + struct v4l2_subdev_routing routing = {};
>> + struct v4l2_subdev_route routes[NUM_ROUTES_MAX] = {};
>> + unsigned int i;
>> + int ret;
>> +
>> + routing.which = which;
>> + routing.routes = (__u64)&routes;
>> + routing.num_routes = 0;
>> + memset(routing.reserved, 0xff, sizeof(routing.reserved));
>> +
>> + /*
>> + * First test that G_ROUTING either returns success, or ENOSPC and
>> + * updates num_routes.
>> + */
>> +
>> + ret = doioctl(node, VIDIOC_SUBDEV_G_ROUTING, &routing);
>> + fail_on_test(ret && ret != ENOSPC);
>> + fail_on_test(ret == ENOSPC && routing.num_routes == 0);
>> + fail_on_test(check_0(routing.reserved, sizeof(routing.reserved)));
>> +
>> + if (routing.num_routes)
>> + return 0;
>
> Shouldn't this be 'if (!routing.num_routes)'?
Yes...
>> +
>> + /* Then get the actual routes */
>> +
>> + routing.num_routes = NUM_ROUTES_MAX;
>> + fail_on_test(doioctl(node, VIDIOC_SUBDEV_G_ROUTING, &routing));
>
> I assume that num_routes is always updated to the actual number of routes, right?
If VIDIOC_SUBDEV_G_ROUTING succeeds, yes, num_routes is updated.
> That's not actually explained clearly in the spec. It says that if the app provided
> num_routes is too small, then it is updated, but it says nothing about what happens
> if the app provided value is too large.
Ok. I need to update the doc.
> Assuming I am right, then I would rewrite this code as follows:
>
> __u32 num_routes = routing.num_routes;
> routing.num_routes = num_routes + 1;
> fail_on_test(doioctl(node, VIDIOC_SUBDEV_G_ROUTING, &routing));
> fail_on_test(routing.num_routes != num_routes);
Yes, I think this looks fine.
Btw, you use __u32 above. Is there any style guide for these? I used
uint32_t in this function for another variable, and I'd use it here too.
>> +
>> + /* Check the validity of route pads and flags */
>> +
>> + if (node->pads) {
>> + for (i = 0; i < routing.num_routes; ++i) {
>> + const struct v4l2_subdev_route *route = &routes[i];
>> + const struct media_pad_desc *sink;
>> + const struct media_pad_desc *source;
>> +
>> + fail_on_test(route->sink_pad >= node->entity.pads);
>> + fail_on_test(route->source_pad >= node->entity.pads);
>> +
>> + sink = &node->pads[route->sink_pad];
>> + source = &node->pads[route->source_pad];
>> +
>> + fail_on_test(!(sink->flags & MEDIA_PAD_FL_SINK));
>> + fail_on_test(!(source->flags & MEDIA_PAD_FL_SOURCE));
>> + fail_on_test(route->flags & ~all_route_flags_mask);
>> + }
>> + }
>> +
>> + /* Set the same routes back, which should always succeed. */
>> +
>> + memset(routing.reserved, 0xff, sizeof(routing.reserved));
>> + fail_on_test(doioctl(node, VIDIOC_SUBDEV_S_ROUTING, &routing));
>> + fail_on_test(check_0(routing.reserved, sizeof(routing.reserved)));
>> +
>> + /* Test setting invalid pads */
>> +
>> + if (node->pads) {
>> + for (i = 0; i < routing.num_routes; ++i) {
>> + struct v4l2_subdev_route *route = &routes[i];
>> +
>> + route->sink_pad = node->entity.pads + 1;
>> + }
>> +
>> + memset(routing.reserved, 0xff, sizeof(routing.reserved));
>> + fail_on_test(doioctl(node, VIDIOC_SUBDEV_S_ROUTING, &routing) != EINVAL);
>> + fail_on_test(check_0(routing.reserved, sizeof(routing.reserved)));
After fixing the num_routes check, I noticed that this one is broken
too. If S_ROUTING fails early enough, the reserved field won't get
cleared, so we can't have this check here.
>> + }
>> +
>> + return 0;
>> +}
>
> Regards,
>
> Hans
On 08/08/2023 08:56, Tomi Valkeinen wrote:
>> I assume that num_routes is always updated to the actual number of
>> routes, right?
>
> If VIDIOC_SUBDEV_G_ROUTING succeeds, yes, num_routes is updated.
>
>> That's not actually explained clearly in the spec. It says that if the
>> app provided
>> num_routes is too small, then it is updated, but it says nothing about
>> what happens
>> if the app provided value is too large.
>
> Ok. I need to update the doc.
Actually, I have already sent a patch for that, "[PATCH] media:
Documentation: Fix [GS]_ROUTING documentation" on 20th July.
Tomi
On 08/08/2023 07:56, Tomi Valkeinen wrote:
> On 02/08/2023 16:54, Hans Verkuil wrote:
>> On 20/07/2023 09:50, Tomi Valkeinen wrote:
>>> Add a simple test for VIDIOC_SUBDEV_G_ROUTING/VIDIOC_SUBDEV_S_ROUTING.
>>>
>>> We can't (at least at the moment) really know here what kind of routings
>>> the driver would accept, but we can test a VIDIOC_SUBDEV_G_ROUTING call,
>>> followed by a VIDIOC_SUBDEV_S_ROUTING call with the routing we
>>> received.
>>>
>>> Additionally, we can check that the returned pads and flags look fine,
>>> and also that setting obviously invalid routing will fail.
>>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>>> ---
>>> utils/v4l2-compliance/v4l2-compliance.cpp | 12 ++++
>>> utils/v4l2-compliance/v4l2-compliance.h | 1 +
>>> utils/v4l2-compliance/v4l2-test-subdevs.cpp | 74 +++++++++++++++++++++
>>> 3 files changed, 87 insertions(+)
>>>
>>> diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp
>>> index e8359b2f..4b232314 100644
>>> --- a/utils/v4l2-compliance/v4l2-compliance.cpp
>>> +++ b/utils/v4l2-compliance/v4l2-compliance.cpp
>>> @@ -1249,6 +1249,18 @@ void testNode(struct node &node, struct node &node_m2m_cap, struct node &expbuf_
>>> node.is_passthrough_subdev = has_source && has_sink;
>>> if (has_routes) {
>>> + printf("Sub-Device routing ioctls:\n");
>>> +
>>> + for (unsigned which = V4L2_SUBDEV_FORMAT_TRY;
>>> + which <= V4L2_SUBDEV_FORMAT_ACTIVE; which++) {
>>> +
>>> + printf("\ttest %s VIDIOC_SUBDEV_G_ROUTING/VIDIOC_SUBDEV_S_ROUTING: %s\n",
>>> + which ? "Active" : "Try",
>>> + ok(testSubDevRouting(&node, which)));
>>> + }
>>> +
>>> + printf("\n");
>>> +
>>> for (unsigned which = V4L2_SUBDEV_FORMAT_TRY;
>>> which <= V4L2_SUBDEV_FORMAT_ACTIVE; which++) {
>>> diff --git a/utils/v4l2-compliance/v4l2-compliance.h b/utils/v4l2-compliance/v4l2-compliance.h
>>> index 0cd43980..35b2274b 100644
>>> --- a/utils/v4l2-compliance/v4l2-compliance.h
>>> +++ b/utils/v4l2-compliance/v4l2-compliance.h
>>> @@ -375,6 +375,7 @@ int testSubDevEnum(struct node *node, unsigned which, unsigned pad, unsigned str
>>> int testSubDevFormat(struct node *node, unsigned which, unsigned pad, unsigned stream);
>>> int testSubDevSelection(struct node *node, unsigned which, unsigned pad, unsigned stream);
>>> int testSubDevFrameInterval(struct node *node, unsigned pad, unsigned stream);
>>> +int testSubDevRouting(struct node *node, unsigned which);
>>> // Buffer ioctl tests
>>> int testReqBufs(struct node *node);
>>> diff --git a/utils/v4l2-compliance/v4l2-test-subdevs.cpp b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
>>> index 5545b0dc..e59d67f7 100644
>>> --- a/utils/v4l2-compliance/v4l2-test-subdevs.cpp
>>> +++ b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
>>> @@ -551,3 +551,77 @@ int testSubDevSelection(struct node *node, unsigned which, unsigned pad, unsigne
>>> return have_sel ? 0 : ENOTTY;
>>> }
>>> +
>>> +int testSubDevRouting(struct node *node, unsigned which)
>>> +{
>>> + const uint32_t all_route_flags_mask = V4L2_SUBDEV_ROUTE_FL_ACTIVE;
>>> + struct v4l2_subdev_routing routing = {};
>>> + struct v4l2_subdev_route routes[NUM_ROUTES_MAX] = {};
>>> + unsigned int i;
>>> + int ret;
>>> +
>>> + routing.which = which;
>>> + routing.routes = (__u64)&routes;
>>> + routing.num_routes = 0;
>>> + memset(routing.reserved, 0xff, sizeof(routing.reserved));
>>> +
>>> + /*
>>> + * First test that G_ROUTING either returns success, or ENOSPC and
>>> + * updates num_routes.
>>> + */
>>> +
>>> + ret = doioctl(node, VIDIOC_SUBDEV_G_ROUTING, &routing);
>>> + fail_on_test(ret && ret != ENOSPC);
>>> + fail_on_test(ret == ENOSPC && routing.num_routes == 0);
>>> + fail_on_test(check_0(routing.reserved, sizeof(routing.reserved)));
>>> +
>>> + if (routing.num_routes)
>>> + return 0;
>>
>> Shouldn't this be 'if (!routing.num_routes)'?
>
> Yes...
>
>>> +
>>> + /* Then get the actual routes */
>>> +
>>> + routing.num_routes = NUM_ROUTES_MAX;
>>> + fail_on_test(doioctl(node, VIDIOC_SUBDEV_G_ROUTING, &routing));
>>
>> I assume that num_routes is always updated to the actual number of routes, right?
>
> If VIDIOC_SUBDEV_G_ROUTING succeeds, yes, num_routes is updated.
>
>> That's not actually explained clearly in the spec. It says that if the app provided
>> num_routes is too small, then it is updated, but it says nothing about what happens
>> if the app provided value is too large.
>
> Ok. I need to update the doc.
>
>> Assuming I am right, then I would rewrite this code as follows:
>>
>> __u32 num_routes = routing.num_routes;
>> routing.num_routes = num_routes + 1;
>> fail_on_test(doioctl(node, VIDIOC_SUBDEV_G_ROUTING, &routing));
>> fail_on_test(routing.num_routes != num_routes);
>
> Yes, I think this looks fine.
>
> Btw, you use __u32 above. Is there any style guide for these? I used uint32_t in this function for another variable, and I'd use it here too.
It is derived from a kernel structure, and the kernel API uses __u32. So I prefer to
use __u32 for such things. I personally think that is good practice since it helps
indicate that it is a kernel-API-related variable.
I also like it because it is shorter than uint32_t :-)
Regards,
Hans
>
>>> +
>>> + /* Check the validity of route pads and flags */
>>> +
>>> + if (node->pads) {
>>> + for (i = 0; i < routing.num_routes; ++i) {
>>> + const struct v4l2_subdev_route *route = &routes[i];
>>> + const struct media_pad_desc *sink;
>>> + const struct media_pad_desc *source;
>>> +
>>> + fail_on_test(route->sink_pad >= node->entity.pads);
>>> + fail_on_test(route->source_pad >= node->entity.pads);
>>> +
>>> + sink = &node->pads[route->sink_pad];
>>> + source = &node->pads[route->source_pad];
>>> +
>>> + fail_on_test(!(sink->flags & MEDIA_PAD_FL_SINK));
>>> + fail_on_test(!(source->flags & MEDIA_PAD_FL_SOURCE));
>>> + fail_on_test(route->flags & ~all_route_flags_mask);
>>> + }
>>> + }
>>> +
>>> + /* Set the same routes back, which should always succeed. */
>>> +
>>> + memset(routing.reserved, 0xff, sizeof(routing.reserved));
>>> + fail_on_test(doioctl(node, VIDIOC_SUBDEV_S_ROUTING, &routing));
>>> + fail_on_test(check_0(routing.reserved, sizeof(routing.reserved)));
>>> +
>>> + /* Test setting invalid pads */
>>> +
>>> + if (node->pads) {
>>> + for (i = 0; i < routing.num_routes; ++i) {
>>> + struct v4l2_subdev_route *route = &routes[i];
>>> +
>>> + route->sink_pad = node->entity.pads + 1;
>>> + }
>>> +
>>> + memset(routing.reserved, 0xff, sizeof(routing.reserved));
>>> + fail_on_test(doioctl(node, VIDIOC_SUBDEV_S_ROUTING, &routing) != EINVAL);
>>> + fail_on_test(check_0(routing.reserved, sizeof(routing.reserved)));
>
> After fixing the num_routes check, I noticed that this one is broken too. If S_ROUTING fails early enough, the reserved field won't get cleared, so we can't have this check here.
>
>>> + }
>>> +
>>> + return 0;
>>> +}
>>
>> Regards,
>>
>> Hans
>
@@ -1249,6 +1249,18 @@ void testNode(struct node &node, struct node &node_m2m_cap, struct node &expbuf_
node.is_passthrough_subdev = has_source && has_sink;
if (has_routes) {
+ printf("Sub-Device routing ioctls:\n");
+
+ for (unsigned which = V4L2_SUBDEV_FORMAT_TRY;
+ which <= V4L2_SUBDEV_FORMAT_ACTIVE; which++) {
+
+ printf("\ttest %s VIDIOC_SUBDEV_G_ROUTING/VIDIOC_SUBDEV_S_ROUTING: %s\n",
+ which ? "Active" : "Try",
+ ok(testSubDevRouting(&node, which)));
+ }
+
+ printf("\n");
+
for (unsigned which = V4L2_SUBDEV_FORMAT_TRY;
which <= V4L2_SUBDEV_FORMAT_ACTIVE; which++) {
@@ -375,6 +375,7 @@ int testSubDevEnum(struct node *node, unsigned which, unsigned pad, unsigned str
int testSubDevFormat(struct node *node, unsigned which, unsigned pad, unsigned stream);
int testSubDevSelection(struct node *node, unsigned which, unsigned pad, unsigned stream);
int testSubDevFrameInterval(struct node *node, unsigned pad, unsigned stream);
+int testSubDevRouting(struct node *node, unsigned which);
// Buffer ioctl tests
int testReqBufs(struct node *node);
@@ -551,3 +551,77 @@ int testSubDevSelection(struct node *node, unsigned which, unsigned pad, unsigne
return have_sel ? 0 : ENOTTY;
}
+
+int testSubDevRouting(struct node *node, unsigned which)
+{
+ const uint32_t all_route_flags_mask = V4L2_SUBDEV_ROUTE_FL_ACTIVE;
+ struct v4l2_subdev_routing routing = {};
+ struct v4l2_subdev_route routes[NUM_ROUTES_MAX] = {};
+ unsigned int i;
+ int ret;
+
+ routing.which = which;
+ routing.routes = (__u64)&routes;
+ routing.num_routes = 0;
+ memset(routing.reserved, 0xff, sizeof(routing.reserved));
+
+ /*
+ * First test that G_ROUTING either returns success, or ENOSPC and
+ * updates num_routes.
+ */
+
+ ret = doioctl(node, VIDIOC_SUBDEV_G_ROUTING, &routing);
+ fail_on_test(ret && ret != ENOSPC);
+ fail_on_test(ret == ENOSPC && routing.num_routes == 0);
+ fail_on_test(check_0(routing.reserved, sizeof(routing.reserved)));
+
+ if (routing.num_routes)
+ return 0;
+
+ /* Then get the actual routes */
+
+ routing.num_routes = NUM_ROUTES_MAX;
+ fail_on_test(doioctl(node, VIDIOC_SUBDEV_G_ROUTING, &routing));
+
+ /* Check the validity of route pads and flags */
+
+ if (node->pads) {
+ for (i = 0; i < routing.num_routes; ++i) {
+ const struct v4l2_subdev_route *route = &routes[i];
+ const struct media_pad_desc *sink;
+ const struct media_pad_desc *source;
+
+ fail_on_test(route->sink_pad >= node->entity.pads);
+ fail_on_test(route->source_pad >= node->entity.pads);
+
+ sink = &node->pads[route->sink_pad];
+ source = &node->pads[route->source_pad];
+
+ fail_on_test(!(sink->flags & MEDIA_PAD_FL_SINK));
+ fail_on_test(!(source->flags & MEDIA_PAD_FL_SOURCE));
+ fail_on_test(route->flags & ~all_route_flags_mask);
+ }
+ }
+
+ /* Set the same routes back, which should always succeed. */
+
+ memset(routing.reserved, 0xff, sizeof(routing.reserved));
+ fail_on_test(doioctl(node, VIDIOC_SUBDEV_S_ROUTING, &routing));
+ fail_on_test(check_0(routing.reserved, sizeof(routing.reserved)));
+
+ /* Test setting invalid pads */
+
+ if (node->pads) {
+ for (i = 0; i < routing.num_routes; ++i) {
+ struct v4l2_subdev_route *route = &routes[i];
+
+ route->sink_pad = node->entity.pads + 1;
+ }
+
+ memset(routing.reserved, 0xff, sizeof(routing.reserved));
+ fail_on_test(doioctl(node, VIDIOC_SUBDEV_S_ROUTING, &routing) != EINVAL);
+ fail_on_test(check_0(routing.reserved, sizeof(routing.reserved)));
+ }
+
+ return 0;
+}