[PATCH 0/2] v4l2-compliance streams fixes

[PATCH 0/2] v4l2-compliance streams fixes

[Tomi Valkeinen]

Two streams related fixes for v4l2-compliance.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
Tomi Valkeinen (2):
      v4l2-compliance: Fix use of routing on 32-bit platforms
      v4l2-compliance: Fix streams use in testSubDevEnumFrameSize()

 utils/v4l2-compliance/v4l2-compliance.cpp   |  4 ++--
 utils/v4l2-compliance/v4l2-test-subdevs.cpp | 12 ++++++------
 2 files changed, 8 insertions(+), 8 deletions(-)
---
base-commit: a04dfa5b72df01c6dbdf68fd9365e2d913fe5a0f
change-id: 20240403-v4l2-compliance-streams-fixes-e90e49fcab8b

Best regards,
-- 
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

[PATCH v4l-utils 1/2] v4l2-compliance: Fix use of routing on 32-bit platforms

[Tomi Valkeinen]

At the moment we do:

routing.routes = (__u64)&routes;

On 32-bit platforms the 32-bit address will be sign-extended, possibly
resulting setting the address to, e.g., 0xff000000 -> 0xffffffffff000000.

Fix this by first converting the address to uintptr_t.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 utils/v4l2-compliance/v4l2-compliance.cpp   | 4 ++--
 utils/v4l2-compliance/v4l2-test-subdevs.cpp | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp
index 2cf97909..82615c17 100644
--- a/utils/v4l2-compliance/v4l2-compliance.cpp
+++ b/utils/v4l2-compliance/v4l2-compliance.cpp
@@ -1274,7 +1274,7 @@ void testNode(struct node &node, struct node &node_m2m_cap, struct node &expbuf_
 				which <= V4L2_SUBDEV_FORMAT_ACTIVE; which++) {
 
 				sd_routing[which].which = which;
-				sd_routing[which].routes = (__u64)sd_routes[which];
+				sd_routing[which].routes = (__u64)(uintptr_t)sd_routes[which];
 				sd_routing[which].num_routes = NUM_ROUTES_MAX;
 
 				ret = doioctl(&node, VIDIOC_SUBDEV_G_ROUTING, &sd_routing[which]);
@@ -1305,7 +1305,7 @@ void testNode(struct node &node, struct node &node_m2m_cap, struct node &expbuf_
 					routes = sd_routes[which];
 				} else {
 					dummy_routing.num_routes = 1;
-					dummy_routing.routes = (__u64)&dummy_routes;
+					dummy_routing.routes = (__u64)(uintptr_t)&dummy_routes;
 					dummy_routes[0].source_pad = pad;
 					dummy_routes[0].source_stream = 0;
 					dummy_routes[0].sink_pad = pad;
diff --git a/utils/v4l2-compliance/v4l2-test-subdevs.cpp b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
index ebca1b94..fe7a9e1d 100644
--- a/utils/v4l2-compliance/v4l2-test-subdevs.cpp
+++ b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
@@ -586,7 +586,7 @@ int testSubDevRouting(struct node *node, unsigned which)
 	int ret;
 
 	routing.which = which;
-	routing.routes = (__u64)&routes;
+	routing.routes = (__u64)(uintptr_t)&routes;
 	routing.num_routes = 0;
 	memset(routing.reserved, 0xff, sizeof(routing.reserved));
 

-- 
2.34.1

Re: [PATCH v4l-utils 1/2] v4l2-compliance: Fix use of routing on 32-bit platforms

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Wed, Apr 03, 2024 at 11:16:28AM +0300, Tomi Valkeinen wrote:
> At the moment we do:
> 
> routing.routes = (__u64)&routes;
> 
> On 32-bit platforms the 32-bit address will be sign-extended, possibly
> resulting setting the address to, e.g., 0xff000000 -> 0xffffffffff000000.
> 
> Fix this by first converting the address to uintptr_t.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  utils/v4l2-compliance/v4l2-compliance.cpp   | 4 ++--
>  utils/v4l2-compliance/v4l2-test-subdevs.cpp | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp
> index 2cf97909..82615c17 100644
> --- a/utils/v4l2-compliance/v4l2-compliance.cpp
> +++ b/utils/v4l2-compliance/v4l2-compliance.cpp
> @@ -1274,7 +1274,7 @@ void testNode(struct node &node, struct node &node_m2m_cap, struct node &expbuf_
>  				which <= V4L2_SUBDEV_FORMAT_ACTIVE; which++) {
>  
>  				sd_routing[which].which = which;
> -				sd_routing[which].routes = (__u64)sd_routes[which];
> +				sd_routing[which].routes = (__u64)(uintptr_t)sd_routes[which];
>  				sd_routing[which].num_routes = NUM_ROUTES_MAX;
>  
>  				ret = doioctl(&node, VIDIOC_SUBDEV_G_ROUTING, &sd_routing[which]);
> @@ -1305,7 +1305,7 @@ void testNode(struct node &node, struct node &node_m2m_cap, struct node &expbuf_
>  					routes = sd_routes[which];
>  				} else {
>  					dummy_routing.num_routes = 1;
> -					dummy_routing.routes = (__u64)&dummy_routes;
> +					dummy_routing.routes = (__u64)(uintptr_t)&dummy_routes;
>  					dummy_routes[0].source_pad = pad;
>  					dummy_routes[0].source_stream = 0;
>  					dummy_routes[0].sink_pad = pad;
> diff --git a/utils/v4l2-compliance/v4l2-test-subdevs.cpp b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
> index ebca1b94..fe7a9e1d 100644
> --- a/utils/v4l2-compliance/v4l2-test-subdevs.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
> @@ -586,7 +586,7 @@ int testSubDevRouting(struct node *node, unsigned which)
>  	int ret;
>  
>  	routing.which = which;
> -	routing.routes = (__u64)&routes;
> +	routing.routes = (__u64)(uintptr_t)&routes;
>  	routing.num_routes = 0;
>  	memset(routing.reserved, 0xff, sizeof(routing.reserved));
>  
> 

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4l-utils 1/2] v4l2-compliance: Fix use of routing on 32-bit platforms

[Hans Verkuil]

On 03/04/2024 10:16, Tomi Valkeinen wrote:
> At the moment we do:
> 
> routing.routes = (__u64)&routes;
> 
> On 32-bit platforms the 32-bit address will be sign-extended, possibly
> resulting setting the address to, e.g., 0xff000000 -> 0xffffffffff000000.
> 
> Fix this by first converting the address to uintptr_t.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  utils/v4l2-compliance/v4l2-compliance.cpp   | 4 ++--
>  utils/v4l2-compliance/v4l2-test-subdevs.cpp | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp
> index 2cf97909..82615c17 100644
> --- a/utils/v4l2-compliance/v4l2-compliance.cpp
> +++ b/utils/v4l2-compliance/v4l2-compliance.cpp
> @@ -1274,7 +1274,7 @@ void testNode(struct node &node, struct node &node_m2m_cap, struct node &expbuf_
>  				which <= V4L2_SUBDEV_FORMAT_ACTIVE; which++) {
>  
>  				sd_routing[which].which = which;
> -				sd_routing[which].routes = (__u64)sd_routes[which];
> +				sd_routing[which].routes = (__u64)(uintptr_t)sd_routes[which];

You can drop the (__u64) cast. Same for the places below. The cast to uintptr_t
is sufficient and indeed required.

Regards,

	Hans

>  				sd_routing[which].num_routes = NUM_ROUTES_MAX;
>  
>  				ret = doioctl(&node, VIDIOC_SUBDEV_G_ROUTING, &sd_routing[which]);
> @@ -1305,7 +1305,7 @@ void testNode(struct node &node, struct node &node_m2m_cap, struct node &expbuf_
>  					routes = sd_routes[which];
>  				} else {
>  					dummy_routing.num_routes = 1;
> -					dummy_routing.routes = (__u64)&dummy_routes;
> +					dummy_routing.routes = (__u64)(uintptr_t)&dummy_routes;
>  					dummy_routes[0].source_pad = pad;
>  					dummy_routes[0].source_stream = 0;
>  					dummy_routes[0].sink_pad = pad;
> diff --git a/utils/v4l2-compliance/v4l2-test-subdevs.cpp b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
> index ebca1b94..fe7a9e1d 100644
> --- a/utils/v4l2-compliance/v4l2-test-subdevs.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
> @@ -586,7 +586,7 @@ int testSubDevRouting(struct node *node, unsigned which)
>  	int ret;
>  
>  	routing.which = which;
> -	routing.routes = (__u64)&routes;
> +	routing.routes = (__u64)(uintptr_t)&routes;
>  	routing.num_routes = 0;
>  	memset(routing.reserved, 0xff, sizeof(routing.reserved));
>  
> 

[PATCH v4l-utils 2/2] v4l2-compliance: Fix streams use in testSubDevEnumFrameSize()

[Tomi Valkeinen]

We don't pass the stream number to testSubDevEnumFrameSize(), which
instead always uses stream number 0. This causes failures when the
subdevice uses streams.

Fix this by adding stream parameter, and passing the correct stream ID.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 utils/v4l2-compliance/v4l2-test-subdevs.cpp | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/utils/v4l2-compliance/v4l2-test-subdevs.cpp b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
index fe7a9e1d..b2667a3b 100644
--- a/utils/v4l2-compliance/v4l2-test-subdevs.cpp
+++ b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
@@ -121,7 +121,7 @@ static int testSubDevEnumFrameInterval(struct node *node, unsigned which,
 }
 
 static int testSubDevEnumFrameSize(struct node *node, unsigned which,
-				   unsigned pad, unsigned code)
+				   unsigned pad, unsigned stream, unsigned code)
 {
 	struct v4l2_subdev_frame_size_enum fse;
 	unsigned num_sizes;
@@ -130,7 +130,7 @@ static int testSubDevEnumFrameSize(struct node *node, unsigned which,
 	memset(&fse, 0, sizeof(fse));
 	fse.which = which;
 	fse.pad = pad;
-	fse.stream = 0;
+	fse.stream = stream;
 	fse.code = code;
 	ret = doioctl(node, VIDIOC_SUBDEV_ENUM_FRAME_SIZE, &fse);
 	node->has_subdev_enum_fsize |= (ret != ENOTTY) << which;
@@ -140,7 +140,7 @@ static int testSubDevEnumFrameSize(struct node *node, unsigned which,
 		memset(&fie, 0, sizeof(fie));
 		fie.which = which;
 		fie.pad = pad;
-		fie.stream = 0;
+		fie.stream = stream;
 		fie.code = code;
 		fail_on_test(doioctl(node, VIDIOC_SUBDEV_ENUM_FRAME_INTERVAL, &fie) != ENOTTY);
 		return ret;
@@ -156,7 +156,7 @@ static int testSubDevEnumFrameSize(struct node *node, unsigned which,
 	memset(&fse, 0xff, sizeof(fse));
 	fse.which = which;
 	fse.pad = pad;
-	fse.stream = 0;
+	fse.stream = stream;
 	fse.code = code;
 	fse.index = 0;
 	fail_on_test(doioctl(node, VIDIOC_SUBDEV_ENUM_FRAME_SIZE, &fse));
@@ -266,7 +266,7 @@ int testSubDevEnum(struct node *node, unsigned which, unsigned pad, unsigned str
 		fail_on_test(mbus_core_enum.stream != stream);
 		fail_on_test(mbus_core_enum.index != i);
 
-		ret = testSubDevEnumFrameSize(node, which, pad, mbus_core_enum.code);
+		ret = testSubDevEnumFrameSize(node, which, pad, stream, mbus_core_enum.code);
 		fail_on_test(ret && ret != ENOTTY);
 	}
 	return 0;

-- 
2.34.1

Re: [PATCH v4l-utils 2/2] v4l2-compliance: Fix streams use in testSubDevEnumFrameSize()

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Wed, Apr 03, 2024 at 11:16:29AM +0300, Tomi Valkeinen wrote:
> We don't pass the stream number to testSubDevEnumFrameSize(), which
> instead always uses stream number 0. This causes failures when the
> subdevice uses streams.
> 
> Fix this by adding stream parameter, and passing the correct stream ID.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  utils/v4l2-compliance/v4l2-test-subdevs.cpp | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/utils/v4l2-compliance/v4l2-test-subdevs.cpp b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
> index fe7a9e1d..b2667a3b 100644
> --- a/utils/v4l2-compliance/v4l2-test-subdevs.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
> @@ -121,7 +121,7 @@ static int testSubDevEnumFrameInterval(struct node *node, unsigned which,
>  }
>  
>  static int testSubDevEnumFrameSize(struct node *node, unsigned which,
> -				   unsigned pad, unsigned code)
> +				   unsigned pad, unsigned stream, unsigned code)
>  {
>  	struct v4l2_subdev_frame_size_enum fse;
>  	unsigned num_sizes;
> @@ -130,7 +130,7 @@ static int testSubDevEnumFrameSize(struct node *node, unsigned which,
>  	memset(&fse, 0, sizeof(fse));
>  	fse.which = which;
>  	fse.pad = pad;
> -	fse.stream = 0;
> +	fse.stream = stream;
>  	fse.code = code;
>  	ret = doioctl(node, VIDIOC_SUBDEV_ENUM_FRAME_SIZE, &fse);
>  	node->has_subdev_enum_fsize |= (ret != ENOTTY) << which;
> @@ -140,7 +140,7 @@ static int testSubDevEnumFrameSize(struct node *node, unsigned which,
>  		memset(&fie, 0, sizeof(fie));
>  		fie.which = which;
>  		fie.pad = pad;
> -		fie.stream = 0;
> +		fie.stream = stream;
>  		fie.code = code;
>  		fail_on_test(doioctl(node, VIDIOC_SUBDEV_ENUM_FRAME_INTERVAL, &fie) != ENOTTY);
>  		return ret;
> @@ -156,7 +156,7 @@ static int testSubDevEnumFrameSize(struct node *node, unsigned which,
>  	memset(&fse, 0xff, sizeof(fse));
>  	fse.which = which;
>  	fse.pad = pad;
> -	fse.stream = 0;
> +	fse.stream = stream;
>  	fse.code = code;
>  	fse.index = 0;
>  	fail_on_test(doioctl(node, VIDIOC_SUBDEV_ENUM_FRAME_SIZE, &fse));
> @@ -266,7 +266,7 @@ int testSubDevEnum(struct node *node, unsigned which, unsigned pad, unsigned str
>  		fail_on_test(mbus_core_enum.stream != stream);
>  		fail_on_test(mbus_core_enum.index != i);
>  
> -		ret = testSubDevEnumFrameSize(node, which, pad, mbus_core_enum.code);
> +		ret = testSubDevEnumFrameSize(node, which, pad, stream, mbus_core_enum.code);
>  		fail_on_test(ret && ret != ENOTTY);
>  	}
>  	return 0;
> 

-- 
Regards,

Laurent Pinchart