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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxx>
---
  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@xxxxxxxxxxxxxxxx>

Thanks!

 Tomi





[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux