[PATCH 1/2] media: subdev: Split V4L2_SUBDEV_ROUTING_NO_STREAM_MIX

[PATCH 1/2] media: subdev: Split V4L2_SUBDEV_ROUTING_NO_STREAM_MIX

[Tomi Valkeinen]

V4L2_SUBDEV_ROUTING_NO_STREAM_MIX routing validation flag means that all
routes from a sink pad must go to the same source pad and all routes
going to the same source pad must originate from the same sink pad.

This does not cover all use cases. For example, if a device routes
all streams from a single sink pad to any of the source pads, but
streams from multiple sink pads can go to the same source pad, the
current flag is too restrictive.

Split the flag into two parts, V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX
and V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX, which add the restriction
only on one side of the device. Together they mean the same as
V4L2_SUBDEV_ROUTING_NO_STREAM_MIX.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 17 +++++++++++++----
 include/media/v4l2-subdev.h           | 16 +++++++++++++---
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index dff1d9be7841..bc3678337048 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -1693,10 +1693,11 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
 		}
 
 		/*
-		 * V4L2_SUBDEV_ROUTING_NO_STREAM_MIX: Streams on the same pad
-		 * may not be routed to streams on different pads.
+		 * V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX: Streams on the same
+		 * sink pad may not be routed to streams on different source
+		 * pads.
 		 */
-		if (disallow & V4L2_SUBDEV_ROUTING_NO_STREAM_MIX) {
+		if (disallow & V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX) {
 			if (remote_pads[route->sink_pad] != U32_MAX &&
 			    remote_pads[route->sink_pad] != route->source_pad) {
 				dev_dbg(sd->dev,
@@ -1705,6 +1706,15 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
 				goto out;
 			}
 
+			remote_pads[route->sink_pad] = route->source_pad;
+		}
+
+		/*
+		 * V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX: Streams on the same
+		 * source pad may not be routed to streams on different sink
+		 * pads.
+		 */
+		if (disallow & V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX) {
 			if (remote_pads[route->source_pad] != U32_MAX &&
 			    remote_pads[route->source_pad] != route->sink_pad) {
 				dev_dbg(sd->dev,
@@ -1713,7 +1723,6 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
 				goto out;
 			}
 
-			remote_pads[route->sink_pad] = route->source_pad;
 			remote_pads[route->source_pad] = route->sink_pad;
 		}
 
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 17773be4a4ee..a4331e0a5aeb 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -1643,19 +1643,29 @@ u64 v4l2_subdev_state_xlate_streams(const struct v4l2_subdev_state *state,
  * @V4L2_SUBDEV_ROUTING_NO_N_TO_1:
  *	multiple input streams may not be routed to the same output stream
  *	(stream merging)
- * @V4L2_SUBDEV_ROUTING_NO_STREAM_MIX:
- *	streams on the same pad may not be routed to streams on different pads
+ * @V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX:
+ *	streams on the same sink pad may not be routed to streams on different
+ *	source pads
+ * @V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX:
+ *	streams on the same source pad may not be routed to streams on different
+ *	sink pads
  * @V4L2_SUBDEV_ROUTING_ONLY_1_TO_1:
  *	only non-overlapping 1-to-1 stream routing is allowed (a combination of
  *	@V4L2_SUBDEV_ROUTING_NO_1_TO_N and @V4L2_SUBDEV_ROUTING_NO_N_TO_1)
+ * @V4L2_SUBDEV_ROUTING_NO_STREAM_MIX:
+ *	streams on the same pad may not be routed to streams on different pads
  */
 enum v4l2_subdev_routing_restriction {
 	V4L2_SUBDEV_ROUTING_NO_1_TO_N = BIT(0),
 	V4L2_SUBDEV_ROUTING_NO_N_TO_1 = BIT(1),
-	V4L2_SUBDEV_ROUTING_NO_STREAM_MIX = BIT(2),
+	V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX = BIT(2),
+	V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX = BIT(3),
 	V4L2_SUBDEV_ROUTING_ONLY_1_TO_1 =
 		V4L2_SUBDEV_ROUTING_NO_1_TO_N |
 		V4L2_SUBDEV_ROUTING_NO_N_TO_1,
+	V4L2_SUBDEV_ROUTING_NO_STREAM_MIX =
+		V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX |
+		V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX,
 };
 
 /**
-- 
2.34.1

[PATCH 2/2] media: subdev: Add V4L2_SUBDEV_ROUTING_NO_MULTIPLEXING

[Tomi Valkeinen]

A common case with subdev routing is that on the subdevice just before
the DMA engines (video nodes), no multiplexing is allowed on the source
pads, as the DMA engine can only handle a single stream.

In some other situations one might also want to do the same check on the
sink side.

Add new routing validation flags to check these:
V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING and
V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 36 ++++++++++++++++++++++++---
 include/media/v4l2-subdev.h           |  9 +++++++
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index bc3678337048..ae74a48dd2ba 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -1664,7 +1664,8 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
 	unsigned int i, j;
 	int ret = -EINVAL;
 
-	if (disallow & V4L2_SUBDEV_ROUTING_NO_STREAM_MIX) {
+	if (disallow & (V4L2_SUBDEV_ROUTING_NO_STREAM_MIX |
+			V4L2_SUBDEV_ROUTING_NO_MULTIPLEXING)) {
 		remote_pads = kcalloc(sd->entity.num_pads, sizeof(*remote_pads),
 				      GFP_KERNEL);
 		if (!remote_pads)
@@ -1705,8 +1706,6 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
 					i, "sink");
 				goto out;
 			}
-
-			remote_pads[route->sink_pad] = route->source_pad;
 		}
 
 		/*
@@ -1722,7 +1721,38 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
 					i, "source");
 				goto out;
 			}
+		}
+
+		/*
+		 * V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING: Pads on the sink
+		 * side can not do stream multiplexing, i.e. there can be only
+		 * a single stream in a sink pad.
+		 */
+		if (disallow & V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING) {
+			if (remote_pads[route->sink_pad] != U32_MAX) {
+				dev_dbg(sd->dev,
+					"route %u attempts to multiplex on %s pad %u\n",
+					i, "sink", route->sink_pad);
+				goto out;
+			}
+		}
 
+		/*
+		 * V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING: Pads on the
+		 * source side can not do stream multiplexing, i.e. there can
+		 * be only a single stream in a source pad.
+		 */
+		if (disallow & V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING) {
+			if (remote_pads[route->source_pad] != U32_MAX) {
+				dev_dbg(sd->dev,
+					"route %u attempts to multiplex on %s pad %u\n",
+					i, "source", route->source_pad);
+				goto out;
+			}
+		}
+
+		if (remote_pads) {
+			remote_pads[route->sink_pad] = route->source_pad;
 			remote_pads[route->source_pad] = route->sink_pad;
 		}
 
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index a4331e0a5aeb..4a8d45e2c804 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -1649,6 +1649,10 @@ u64 v4l2_subdev_state_xlate_streams(const struct v4l2_subdev_state *state,
  * @V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX:
  *	streams on the same source pad may not be routed to streams on different
  *	sink pads
+ * @V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING:
+ *	source pads may not contain multiplexed streams
+ * @V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING:
+ *	sink pads may not contain multiplexed streams
  * @V4L2_SUBDEV_ROUTING_ONLY_1_TO_1:
  *	only non-overlapping 1-to-1 stream routing is allowed (a combination of
  *	@V4L2_SUBDEV_ROUTING_NO_1_TO_N and @V4L2_SUBDEV_ROUTING_NO_N_TO_1)
@@ -1660,12 +1664,17 @@ enum v4l2_subdev_routing_restriction {
 	V4L2_SUBDEV_ROUTING_NO_N_TO_1 = BIT(1),
 	V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX = BIT(2),
 	V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX = BIT(3),
+	V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING = BIT(4),
+	V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING = BIT(5),
 	V4L2_SUBDEV_ROUTING_ONLY_1_TO_1 =
 		V4L2_SUBDEV_ROUTING_NO_1_TO_N |
 		V4L2_SUBDEV_ROUTING_NO_N_TO_1,
 	V4L2_SUBDEV_ROUTING_NO_STREAM_MIX =
 		V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX |
 		V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX,
+	V4L2_SUBDEV_ROUTING_NO_MULTIPLEXING =
+		V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING |
+		V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING,
 };
 
 /**
-- 
2.34.1

Re: [PATCH 2/2] media: subdev: Add V4L2_SUBDEV_ROUTING_NO_MULTIPLEXING

[Tomi Valkeinen]

On 28/02/2023 11:23, Tomi Valkeinen wrote:
> A common case with subdev routing is that on the subdevice just before
> the DMA engines (video nodes), no multiplexing is allowed on the source
> pads, as the DMA engine can only handle a single stream.
> 
> In some other situations one might also want to do the same check on the
> sink side.
> 
> Add new routing validation flags to check these:
> V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING and
> V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>   drivers/media/v4l2-core/v4l2-subdev.c | 36 ++++++++++++++++++++++++---
>   include/media/v4l2-subdev.h           |  9 +++++++
>   2 files changed, 42 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index bc3678337048..ae74a48dd2ba 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -1664,7 +1664,8 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
>   	unsigned int i, j;
>   	int ret = -EINVAL;
>   
> -	if (disallow & V4L2_SUBDEV_ROUTING_NO_STREAM_MIX) {
> +	if (disallow & (V4L2_SUBDEV_ROUTING_NO_STREAM_MIX |
> +			V4L2_SUBDEV_ROUTING_NO_MULTIPLEXING)) {
>   		remote_pads = kcalloc(sd->entity.num_pads, sizeof(*remote_pads),
>   				      GFP_KERNEL);
>   		if (!remote_pads)
> @@ -1705,8 +1706,6 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
>   					i, "sink");
>   				goto out;
>   			}
> -
> -			remote_pads[route->sink_pad] = route->source_pad;
>   		}
>   
>   		/*
> @@ -1722,7 +1721,38 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
>   					i, "source");
>   				goto out;
>   			}
> +		}
> +
> +		/*
> +		 * V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING: Pads on the sink
> +		 * side can not do stream multiplexing, i.e. there can be only
> +		 * a single stream in a sink pad.
> +		 */
> +		if (disallow & V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING) {
> +			if (remote_pads[route->sink_pad] != U32_MAX) {
> +				dev_dbg(sd->dev,
> +					"route %u attempts to multiplex on %s pad %u\n",
> +					i, "sink", route->sink_pad);
> +				goto out;
> +			}
> +		}
>   
> +		/*
> +		 * V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING: Pads on the
> +		 * source side can not do stream multiplexing, i.e. there can
> +		 * be only a single stream in a source pad.
> +		 */
> +		if (disallow & V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING) {
> +			if (remote_pads[route->source_pad] != U32_MAX) {
> +				dev_dbg(sd->dev,
> +					"route %u attempts to multiplex on %s pad %u\n",
> +					i, "source", route->source_pad);
> +				goto out;
> +			}
> +		}
> +
> +		if (remote_pads) {
> +			remote_pads[route->sink_pad] = route->source_pad;
>   			remote_pads[route->source_pad] = route->sink_pad;
>   		}
>   
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index a4331e0a5aeb..4a8d45e2c804 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -1649,6 +1649,10 @@ u64 v4l2_subdev_state_xlate_streams(const struct v4l2_subdev_state *state,
>    * @V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX:
>    *	streams on the same source pad may not be routed to streams on different
>    *	sink pads
> + * @V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING:
> + *	source pads may not contain multiplexed streams
> + * @V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING:
> + *	sink pads may not contain multiplexed streams
>    * @V4L2_SUBDEV_ROUTING_ONLY_1_TO_1:
>    *	only non-overlapping 1-to-1 stream routing is allowed (a combination of
>    *	@V4L2_SUBDEV_ROUTING_NO_1_TO_N and @V4L2_SUBDEV_ROUTING_NO_N_TO_1)
> @@ -1660,12 +1664,17 @@ enum v4l2_subdev_routing_restriction {
>   	V4L2_SUBDEV_ROUTING_NO_N_TO_1 = BIT(1),
>   	V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX = BIT(2),
>   	V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX = BIT(3),
> +	V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING = BIT(4),
> +	V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING = BIT(5),
>   	V4L2_SUBDEV_ROUTING_ONLY_1_TO_1 =
>   		V4L2_SUBDEV_ROUTING_NO_1_TO_N |
>   		V4L2_SUBDEV_ROUTING_NO_N_TO_1,
>   	V4L2_SUBDEV_ROUTING_NO_STREAM_MIX =
>   		V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX |
>   		V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX,
> +	V4L2_SUBDEV_ROUTING_NO_MULTIPLEXING =
> +		V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING |
> +		V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING,
>   };
>   
>   /**

I missed the V4L2_SUBDEV_ROUTING_NO_MULTIPLEXING value in the kdocs. I 
will add that.

  Tomi

Re: [PATCH 1/2] media: subdev: Split V4L2_SUBDEV_ROUTING_NO_STREAM_MIX

[Jacopo Mondi]

Hi Tomi

On Tue, Feb 28, 2023 at 11:23:45AM +0200, Tomi Valkeinen wrote:
> V4L2_SUBDEV_ROUTING_NO_STREAM_MIX routing validation flag means that all
> routes from a sink pad must go to the same source pad and all routes
> going to the same source pad must originate from the same sink pad.
>
> This does not cover all use cases. For example, if a device routes
> all streams from a single sink pad to any of the source pads, but
> streams from multiple sink pads can go to the same source pad, the
> current flag is too restrictive.
>
> Split the flag into two parts, V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX
> and V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX, which add the restriction
> only on one side of the device. Together they mean the same as
> V4L2_SUBDEV_ROUTING_NO_STREAM_MIX.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 17 +++++++++++++----
>  include/media/v4l2-subdev.h           | 16 +++++++++++++---
>  2 files changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index dff1d9be7841..bc3678337048 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -1693,10 +1693,11 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
>  		}
>
>  		/*
> -		 * V4L2_SUBDEV_ROUTING_NO_STREAM_MIX: Streams on the same pad
> -		 * may not be routed to streams on different pads.
> +		 * V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX: Streams on the same
> +		 * sink pad may not be routed to streams on different source

nit: it was already like this, but as the flag checks for a condition
that is forbidden I would use "Streams on the same sink pad -shall-
not be routed to streams on -a- different source pad"

> +		 * pads.
>  		 */
> -		if (disallow & V4L2_SUBDEV_ROUTING_NO_STREAM_MIX) {
> +		if (disallow & V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX) {
>  			if (remote_pads[route->sink_pad] != U32_MAX &&
>  			    remote_pads[route->sink_pad] != route->source_pad) {
>  				dev_dbg(sd->dev,
> @@ -1705,6 +1706,15 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
>  				goto out;
>  			}
>
> +			remote_pads[route->sink_pad] = route->source_pad;
> +		}
> +
> +		/*
> +		 * V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX: Streams on the same
> +		 * source pad may not be routed to streams on different sink
> +		 * pads.
> +		 */
> +		if (disallow & V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX) {
>  			if (remote_pads[route->source_pad] != U32_MAX &&
>  			    remote_pads[route->source_pad] != route->sink_pad) {
>  				dev_dbg(sd->dev,
> @@ -1713,7 +1723,6 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
>  				goto out;
>  			}
>
> -			remote_pads[route->sink_pad] = route->source_pad;
>  			remote_pads[route->source_pad] = route->sink_pad;
>  		}
>
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 17773be4a4ee..a4331e0a5aeb 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -1643,19 +1643,29 @@ u64 v4l2_subdev_state_xlate_streams(const struct v4l2_subdev_state *state,
>   * @V4L2_SUBDEV_ROUTING_NO_N_TO_1:
>   *	multiple input streams may not be routed to the same output stream
>   *	(stream merging)
> - * @V4L2_SUBDEV_ROUTING_NO_STREAM_MIX:
> - *	streams on the same pad may not be routed to streams on different pads
> + * @V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX:
> + *	streams on the same sink pad may not be routed to streams on different
> + *	source pads

Same comment on s/may not/shall not/
Up to you, really

> + * @V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX:
> + *	streams on the same source pad may not be routed to streams on different
> + *	sink pads

I would prefer the way it is described in the commit message:

        streams on the same source pad must originate from the same
        sink pad


>   * @V4L2_SUBDEV_ROUTING_ONLY_1_TO_1:
>   *	only non-overlapping 1-to-1 stream routing is allowed (a combination of
>   *	@V4L2_SUBDEV_ROUTING_NO_1_TO_N and @V4L2_SUBDEV_ROUTING_NO_N_TO_1)
> + * @V4L2_SUBDEV_ROUTING_NO_STREAM_MIX:
> + *	streams on the same pad may not be routed to streams on different pads

        streams on a pad shall all be routed to the same opposite pad

All suggestions, take whatever you like the most

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

>   */
>  enum v4l2_subdev_routing_restriction {
>  	V4L2_SUBDEV_ROUTING_NO_1_TO_N = BIT(0),
>  	V4L2_SUBDEV_ROUTING_NO_N_TO_1 = BIT(1),
> -	V4L2_SUBDEV_ROUTING_NO_STREAM_MIX = BIT(2),
> +	V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX = BIT(2),
> +	V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX = BIT(3),
>  	V4L2_SUBDEV_ROUTING_ONLY_1_TO_1 =
>  		V4L2_SUBDEV_ROUTING_NO_1_TO_N |
>  		V4L2_SUBDEV_ROUTING_NO_N_TO_1,
> +	V4L2_SUBDEV_ROUTING_NO_STREAM_MIX =
> +		V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX |
> +		V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX,
>  };
>
>  /**
> --
> 2.34.1
>

Re: [PATCH 2/2] media: subdev: Add V4L2_SUBDEV_ROUTING_NO_MULTIPLEXING

[Jacopo Mondi]

Hi Tomi

On Tue, Feb 28, 2023 at 11:23:46AM +0200, Tomi Valkeinen wrote:
> A common case with subdev routing is that on the subdevice just before
> the DMA engines (video nodes), no multiplexing is allowed on the source
> pads, as the DMA engine can only handle a single stream.
>
> In some other situations one might also want to do the same check on the
> sink side.
>
> Add new routing validation flags to check these:
> V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING and
> V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 36 ++++++++++++++++++++++++---
>  include/media/v4l2-subdev.h           |  9 +++++++
>  2 files changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index bc3678337048..ae74a48dd2ba 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -1664,7 +1664,8 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
>  	unsigned int i, j;
>  	int ret = -EINVAL;
>
> -	if (disallow & V4L2_SUBDEV_ROUTING_NO_STREAM_MIX) {
> +	if (disallow & (V4L2_SUBDEV_ROUTING_NO_STREAM_MIX |
> +			V4L2_SUBDEV_ROUTING_NO_MULTIPLEXING)) {
>  		remote_pads = kcalloc(sd->entity.num_pads, sizeof(*remote_pads),

I wonder if we shoulnd't simply allocate the remote pads array and be
done with it.

Do we have maximum number of routes ? Can we even allocate statically ?

not strictly related to this patch though


>  				      GFP_KERNEL);
>  		if (!remote_pads)
> @@ -1705,8 +1706,6 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
>  					i, "sink");
>  				goto out;
>  			}
> -
> -			remote_pads[route->sink_pad] = route->source_pad;
>  		}
>
>  		/*
> @@ -1722,7 +1721,38 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
>  					i, "source");
>  				goto out;
>  			}
> +		}
> +
> +		/*
> +		 * V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING: Pads on the sink
> +		 * side can not do stream multiplexing, i.e. there can be only
> +		 * a single stream in a sink pad.
> +		 */
> +		if (disallow & V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING) {
> +			if (remote_pads[route->sink_pad] != U32_MAX) {
> +				dev_dbg(sd->dev,
> +					"route %u attempts to multiplex on %s pad %u\n",
> +					i, "sink", route->sink_pad);

nit: in all this dev_dbg() there's no really need to specify "sink" or
"source" as arguments.

> +				goto out;
> +			}
> +		}
>
> +		/*
> +		 * V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING: Pads on the
> +		 * source side can not do stream multiplexing, i.e. there can
> +		 * be only a single stream in a source pad.
> +		 */
> +		if (disallow & V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING) {
> +			if (remote_pads[route->source_pad] != U32_MAX) {
> +				dev_dbg(sd->dev,
> +					"route %u attempts to multiplex on %s pad %u\n",
> +					i, "source", route->source_pad);
> +				goto out;
> +			}
> +		}
> +
> +		if (remote_pads) {
> +			remote_pads[route->sink_pad] = route->source_pad;
>  			remote_pads[route->source_pad] = route->sink_pad;
>  		}
>
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index a4331e0a5aeb..4a8d45e2c804 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -1649,6 +1649,10 @@ u64 v4l2_subdev_state_xlate_streams(const struct v4l2_subdev_state *state,
>   * @V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX:
>   *	streams on the same source pad may not be routed to streams on different
>   *	sink pads
> + * @V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING:
> + *	source pads may not contain multiplexed streams
> + * @V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING:
> + *	sink pads may not contain multiplexed streams

Again as a suggestion

 * @V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING:
 *	a single stream shall be routed to source pads
 * @V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING:
 *	a single stream shall originate from sink pads

>   * @V4L2_SUBDEV_ROUTING_ONLY_1_TO_1:
>   *	only non-overlapping 1-to-1 stream routing is allowed (a combination of
>   *	@V4L2_SUBDEV_ROUTING_NO_1_TO_N and @V4L2_SUBDEV_ROUTING_NO_N_TO_1)
> @@ -1660,12 +1664,17 @@ enum v4l2_subdev_routing_restriction {
>  	V4L2_SUBDEV_ROUTING_NO_N_TO_1 = BIT(1),
>  	V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX = BIT(2),
>  	V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX = BIT(3),
> +	V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING = BIT(4),
> +	V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING = BIT(5),
>  	V4L2_SUBDEV_ROUTING_ONLY_1_TO_1 =
>  		V4L2_SUBDEV_ROUTING_NO_1_TO_N |
>  		V4L2_SUBDEV_ROUTING_NO_N_TO_1,
>  	V4L2_SUBDEV_ROUTING_NO_STREAM_MIX =
>  		V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX |
>  		V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX,
> +	V4L2_SUBDEV_ROUTING_NO_MULTIPLEXING =
> +		V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING |
> +		V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING,

As you noticed alreayd this is missing doc

All nits/suggestions
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

>  };
>
>  /**
> --
> 2.34.1
>

Re: [PATCH 1/2] media: subdev: Split V4L2_SUBDEV_ROUTING_NO_STREAM_MIX

[Tomi Valkeinen]

On 01/03/2023 12:37, Jacopo Mondi wrote:
> Hi Tomi
> 
> On Tue, Feb 28, 2023 at 11:23:45AM +0200, Tomi Valkeinen wrote:
>> V4L2_SUBDEV_ROUTING_NO_STREAM_MIX routing validation flag means that all
>> routes from a sink pad must go to the same source pad and all routes
>> going to the same source pad must originate from the same sink pad.
>>
>> This does not cover all use cases. For example, if a device routes
>> all streams from a single sink pad to any of the source pads, but
>> streams from multiple sink pads can go to the same source pad, the
>> current flag is too restrictive.
>>
>> Split the flag into two parts, V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX
>> and V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX, which add the restriction
>> only on one side of the device. Together they mean the same as
>> V4L2_SUBDEV_ROUTING_NO_STREAM_MIX.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> ---
>>   drivers/media/v4l2-core/v4l2-subdev.c | 17 +++++++++++++----
>>   include/media/v4l2-subdev.h           | 16 +++++++++++++---
>>   2 files changed, 26 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
>> index dff1d9be7841..bc3678337048 100644
>> --- a/drivers/media/v4l2-core/v4l2-subdev.c
>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
>> @@ -1693,10 +1693,11 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
>>   		}
>>
>>   		/*
>> -		 * V4L2_SUBDEV_ROUTING_NO_STREAM_MIX: Streams on the same pad
>> -		 * may not be routed to streams on different pads.
>> +		 * V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX: Streams on the same
>> +		 * sink pad may not be routed to streams on different source
> 
> nit: it was already like this, but as the flag checks for a condition
> that is forbidden I would use "Streams on the same sink pad -shall-
> not be routed to streams on -a- different source pad"

I'm fine with that. There were already other flags, so I'll change the 
wording for them too.

>> +		 * pads.
>>   		 */
>> -		if (disallow & V4L2_SUBDEV_ROUTING_NO_STREAM_MIX) {
>> +		if (disallow & V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX) {
>>   			if (remote_pads[route->sink_pad] != U32_MAX &&
>>   			    remote_pads[route->sink_pad] != route->source_pad) {
>>   				dev_dbg(sd->dev,
>> @@ -1705,6 +1706,15 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
>>   				goto out;
>>   			}
>>
>> +			remote_pads[route->sink_pad] = route->source_pad;
>> +		}
>> +
>> +		/*
>> +		 * V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX: Streams on the same
>> +		 * source pad may not be routed to streams on different sink
>> +		 * pads.
>> +		 */
>> +		if (disallow & V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX) {
>>   			if (remote_pads[route->source_pad] != U32_MAX &&
>>   			    remote_pads[route->source_pad] != route->sink_pad) {
>>   				dev_dbg(sd->dev,
>> @@ -1713,7 +1723,6 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
>>   				goto out;
>>   			}
>>
>> -			remote_pads[route->sink_pad] = route->source_pad;
>>   			remote_pads[route->source_pad] = route->sink_pad;
>>   		}
>>
>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>> index 17773be4a4ee..a4331e0a5aeb 100644
>> --- a/include/media/v4l2-subdev.h
>> +++ b/include/media/v4l2-subdev.h
>> @@ -1643,19 +1643,29 @@ u64 v4l2_subdev_state_xlate_streams(const struct v4l2_subdev_state *state,
>>    * @V4L2_SUBDEV_ROUTING_NO_N_TO_1:
>>    *	multiple input streams may not be routed to the same output stream
>>    *	(stream merging)
>> - * @V4L2_SUBDEV_ROUTING_NO_STREAM_MIX:
>> - *	streams on the same pad may not be routed to streams on different pads
>> + * @V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX:
>> + *	streams on the same sink pad may not be routed to streams on different
>> + *	source pads
> 
> Same comment on s/may not/shall not/
> Up to you, really
> 
>> + * @V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX:
>> + *	streams on the same source pad may not be routed to streams on different
>> + *	sink pads
> 
> I would prefer the way it is described in the commit message:
> 
>          streams on the same source pad must originate from the same
>          sink pad

Hmm, yes... As the flags are negatives (_NO_), I tried to document them 
as negatives too. But perhaps it's clearer to use positive style in the 
docs.

Or how about "all streams on a source pad must originate from a single 
sink pad".

And for the sink side:

"all streams from a sink pad must be routed to a single source pad"

> 
>>    * @V4L2_SUBDEV_ROUTING_ONLY_1_TO_1:
>>    *	only non-overlapping 1-to-1 stream routing is allowed (a combination of
>>    *	@V4L2_SUBDEV_ROUTING_NO_1_TO_N and @V4L2_SUBDEV_ROUTING_NO_N_TO_1)
>> + * @V4L2_SUBDEV_ROUTING_NO_STREAM_MIX:
>> + *	streams on the same pad may not be routed to streams on different pads
> 
>          streams on a pad shall all be routed to the same opposite pad

I think this would be better if it somehow emphasizes that it's both 
ways. But I can't come up with a good idea right now...

  Tomi

Re: [PATCH 2/2] media: subdev: Add V4L2_SUBDEV_ROUTING_NO_MULTIPLEXING

[Tomi Valkeinen]

On 01/03/2023 12:47, Jacopo Mondi wrote:
> Hi Tomi
> 
> On Tue, Feb 28, 2023 at 11:23:46AM +0200, Tomi Valkeinen wrote:
>> A common case with subdev routing is that on the subdevice just before
>> the DMA engines (video nodes), no multiplexing is allowed on the source
>> pads, as the DMA engine can only handle a single stream.
>>
>> In some other situations one might also want to do the same check on the
>> sink side.
>>
>> Add new routing validation flags to check these:
>> V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING and
>> V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>>   drivers/media/v4l2-core/v4l2-subdev.c | 36 ++++++++++++++++++++++++---
>>   include/media/v4l2-subdev.h           |  9 +++++++
>>   2 files changed, 42 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
>> index bc3678337048..ae74a48dd2ba 100644
>> --- a/drivers/media/v4l2-core/v4l2-subdev.c
>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
>> @@ -1664,7 +1664,8 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
>>   	unsigned int i, j;
>>   	int ret = -EINVAL;
>>
>> -	if (disallow & V4L2_SUBDEV_ROUTING_NO_STREAM_MIX) {
>> +	if (disallow & (V4L2_SUBDEV_ROUTING_NO_STREAM_MIX |
>> +			V4L2_SUBDEV_ROUTING_NO_MULTIPLEXING)) {
>>   		remote_pads = kcalloc(sd->entity.num_pads, sizeof(*remote_pads),
> 
> I wonder if we shoulnd't simply allocate the remote pads array and be
> done with it.

Well, it's difficult to say what will be the most common uses... So I 
think this is a sensible optimization for the time being.

> Do we have maximum number of routes ? Can we even allocate statically ?

No, I don't think we can do that.

> not strictly related to this patch though
> 
> 
>>   				      GFP_KERNEL);
>>   		if (!remote_pads)
>> @@ -1705,8 +1706,6 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
>>   					i, "sink");
>>   				goto out;
>>   			}
>> -
>> -			remote_pads[route->sink_pad] = route->source_pad;
>>   		}
>>
>>   		/*
>> @@ -1722,7 +1721,38 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
>>   					i, "source");
>>   				goto out;
>>   			}
>> +		}
>> +
>> +		/*
>> +		 * V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING: Pads on the sink
>> +		 * side can not do stream multiplexing, i.e. there can be only
>> +		 * a single stream in a sink pad.
>> +		 */
>> +		if (disallow & V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING) {
>> +			if (remote_pads[route->sink_pad] != U32_MAX) {
>> +				dev_dbg(sd->dev,
>> +					"route %u attempts to multiplex on %s pad %u\n",
>> +					i, "sink", route->sink_pad);
> 
> nit: in all this dev_dbg() there's no really need to specify "sink" or
> "source" as arguments.

This is an optimization Laurent likes... Note that the format strings 
are identical in this and the below prints. So there will be only one 
instance in the binary. If the sink/source would be "inlined" into the 
format string, there would be two full strings in the binary.

Whether this optimization makes sense or not... Can't say =).

>> +				goto out;
>> +			}
>> +		}
>>
>> +		/*
>> +		 * V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING: Pads on the
>> +		 * source side can not do stream multiplexing, i.e. there can
>> +		 * be only a single stream in a source pad.
>> +		 */
>> +		if (disallow & V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING) {
>> +			if (remote_pads[route->source_pad] != U32_MAX) {
>> +				dev_dbg(sd->dev,
>> +					"route %u attempts to multiplex on %s pad %u\n",
>> +					i, "source", route->source_pad);
>> +				goto out;
>> +			}
>> +		}
>> +
>> +		if (remote_pads) {
>> +			remote_pads[route->sink_pad] = route->source_pad;
>>   			remote_pads[route->source_pad] = route->sink_pad;
>>   		}
>>
>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>> index a4331e0a5aeb..4a8d45e2c804 100644
>> --- a/include/media/v4l2-subdev.h
>> +++ b/include/media/v4l2-subdev.h
>> @@ -1649,6 +1649,10 @@ u64 v4l2_subdev_state_xlate_streams(const struct v4l2_subdev_state *state,
>>    * @V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX:
>>    *	streams on the same source pad may not be routed to streams on different
>>    *	sink pads
>> + * @V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING:
>> + *	source pads may not contain multiplexed streams
>> + * @V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING:
>> + *	sink pads may not contain multiplexed streams
> 
> Again as a suggestion
> 
>   * @V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING:
>   *	a single stream shall be routed to source pads
>   * @V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING:
>   *	a single stream shall originate from sink pads

Yep.

>>    * @V4L2_SUBDEV_ROUTING_ONLY_1_TO_1:
>>    *	only non-overlapping 1-to-1 stream routing is allowed (a combination of
>>    *	@V4L2_SUBDEV_ROUTING_NO_1_TO_N and @V4L2_SUBDEV_ROUTING_NO_N_TO_1)
>> @@ -1660,12 +1664,17 @@ enum v4l2_subdev_routing_restriction {
>>   	V4L2_SUBDEV_ROUTING_NO_N_TO_1 = BIT(1),
>>   	V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX = BIT(2),
>>   	V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX = BIT(3),
>> +	V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING = BIT(4),
>> +	V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING = BIT(5),
>>   	V4L2_SUBDEV_ROUTING_ONLY_1_TO_1 =
>>   		V4L2_SUBDEV_ROUTING_NO_1_TO_N |
>>   		V4L2_SUBDEV_ROUTING_NO_N_TO_1,
>>   	V4L2_SUBDEV_ROUTING_NO_STREAM_MIX =
>>   		V4L2_SUBDEV_ROUTING_NO_SINK_STREAM_MIX |
>>   		V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX,
>> +	V4L2_SUBDEV_ROUTING_NO_MULTIPLEXING =
>> +		V4L2_SUBDEV_ROUTING_NO_SINK_MULTIPLEXING |
>> +		V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING,
> 
> As you noticed alreayd this is missing doc
> 
> All nits/suggestions
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks!

  Tomi