Linux Media Controller development
 help / color / mirror / Atom feed
* [PATCH 0/2] v4l2-compliance streams fixes
@ 2024-04-03  8:16 Tomi Valkeinen
  2024-04-03  8:16 ` [PATCH v4l-utils 1/2] v4l2-compliance: Fix use of routing on 32-bit platforms Tomi Valkeinen
  2024-04-03  8:16 ` [PATCH v4l-utils 2/2] v4l2-compliance: Fix streams use in testSubDevEnumFrameSize() Tomi Valkeinen
  0 siblings, 2 replies; 6+ messages in thread
From: Tomi Valkeinen @ 2024-04-03  8:16 UTC (permalink / raw)
  To: linux-media, Laurent Pinchart, Sakari Ailus, Hans Verkuil; +Cc: 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>


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

* [PATCH v4l-utils 1/2] v4l2-compliance: Fix use of routing on 32-bit platforms
  2024-04-03  8:16 [PATCH 0/2] v4l2-compliance streams fixes Tomi Valkeinen
@ 2024-04-03  8:16 ` Tomi Valkeinen
  2024-04-03  8:41   ` Laurent Pinchart
  2024-04-03  9:00   ` Hans Verkuil
  2024-04-03  8:16 ` [PATCH v4l-utils 2/2] v4l2-compliance: Fix streams use in testSubDevEnumFrameSize() Tomi Valkeinen
  1 sibling, 2 replies; 6+ messages in thread
From: Tomi Valkeinen @ 2024-04-03  8:16 UTC (permalink / raw)
  To: linux-media, Laurent Pinchart, Sakari Ailus, Hans Verkuil; +Cc: 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


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

* [PATCH v4l-utils 2/2] v4l2-compliance: Fix streams use in testSubDevEnumFrameSize()
  2024-04-03  8:16 [PATCH 0/2] v4l2-compliance streams fixes Tomi Valkeinen
  2024-04-03  8:16 ` [PATCH v4l-utils 1/2] v4l2-compliance: Fix use of routing on 32-bit platforms Tomi Valkeinen
@ 2024-04-03  8:16 ` Tomi Valkeinen
  2024-04-03  8:43   ` Laurent Pinchart
  1 sibling, 1 reply; 6+ messages in thread
From: Tomi Valkeinen @ 2024-04-03  8:16 UTC (permalink / raw)
  To: linux-media, Laurent Pinchart, Sakari Ailus, Hans Verkuil; +Cc: 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


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

* Re: [PATCH v4l-utils 1/2] v4l2-compliance: Fix use of routing on 32-bit platforms
  2024-04-03  8:16 ` [PATCH v4l-utils 1/2] v4l2-compliance: Fix use of routing on 32-bit platforms Tomi Valkeinen
@ 2024-04-03  8:41   ` Laurent Pinchart
  2024-04-03  9:00   ` Hans Verkuil
  1 sibling, 0 replies; 6+ messages in thread
From: Laurent Pinchart @ 2024-04-03  8:41 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-media, Sakari Ailus, Hans Verkuil

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

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

* Re: [PATCH v4l-utils 2/2] v4l2-compliance: Fix streams use in testSubDevEnumFrameSize()
  2024-04-03  8:16 ` [PATCH v4l-utils 2/2] v4l2-compliance: Fix streams use in testSubDevEnumFrameSize() Tomi Valkeinen
@ 2024-04-03  8:43   ` Laurent Pinchart
  0 siblings, 0 replies; 6+ messages in thread
From: Laurent Pinchart @ 2024-04-03  8:43 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-media, Sakari Ailus, Hans Verkuil

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

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

* Re: [PATCH v4l-utils 1/2] v4l2-compliance: Fix use of routing on 32-bit platforms
  2024-04-03  8:16 ` [PATCH v4l-utils 1/2] v4l2-compliance: Fix use of routing on 32-bit platforms Tomi Valkeinen
  2024-04-03  8:41   ` Laurent Pinchart
@ 2024-04-03  9:00   ` Hans Verkuil
  1 sibling, 0 replies; 6+ messages in thread
From: Hans Verkuil @ 2024-04-03  9:00 UTC (permalink / raw)
  To: Tomi Valkeinen, linux-media, Laurent Pinchart, Sakari Ailus

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));
>  
> 


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

end of thread, other threads:[~2024-04-03  9:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-03  8:16 [PATCH 0/2] v4l2-compliance streams fixes Tomi Valkeinen
2024-04-03  8:16 ` [PATCH v4l-utils 1/2] v4l2-compliance: Fix use of routing on 32-bit platforms Tomi Valkeinen
2024-04-03  8:41   ` Laurent Pinchart
2024-04-03  9:00   ` Hans Verkuil
2024-04-03  8:16 ` [PATCH v4l-utils 2/2] v4l2-compliance: Fix streams use in testSubDevEnumFrameSize() Tomi Valkeinen
2024-04-03  8:43   ` Laurent Pinchart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox