diff mbox series

media: v4l2-subdev: Support enable/disable_streams for single-pad subdevs

Message ID 20240325-single-pad-enable-streams-v1-1-142e19896a72@ideasonboard.com
State New
Headers show
Series media: v4l2-subdev: Support enable/disable_streams for single-pad subdevs | expand

Commit Message

Tomi Valkeinen March 25, 2024, 12:43 p.m. UTC 
Currently a subdevice with a single pad, e.g. a sensor subdevice, must
use the v4l2_subdev_video_ops.s_stream op, instead of
v4l2_subdev_pad_ops.enable/disable_streams. This is because the
enable/disable_streams machinery requires a routing table which a subdev
cannot have with a single pad.

Implement enable/disable_streams support for these single-pad subdevices
by assuming an implicit stream 0 when the subdevice has only one pad.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
Even though I did send this patch, I'm not sure if this is necessary.
s_stream works fine for the subdevs with a single pad. With the upcoming
internal pads, adding an internal pad to the subdev will create a
routing table, and enable/disable_streams would get "fixed" that way.

So perhaps the question is, do we want to support single-pad subdevs in
the future, in which case something like this patch is necessary, or
will all modern source subdev drivers have internal pads, in which
case this is not needed...
---
 drivers/media/v4l2-core/v4l2-subdev.c | 105 ++++++++++++++++++++++------------
 include/media/v4l2-subdev.h           |   4 +-
 2 files changed, 72 insertions(+), 37 deletions(-)


---
base-commit: e8f897f4afef0031fe618a8e94127a0934896aba
change-id: 20240325-single-pad-enable-streams-32a9a746ac5b

Best regards,
diff mbox series

Patch

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 4c6198c48dd6..ddc7ed69421c 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -2129,21 +2129,33 @@  int v4l2_subdev_enable_streams(struct v4l2_subdev *sd, u32 pad,
 	 * Verify that the requested streams exist and that they are not
 	 * already enabled.
 	 */
-	for (i = 0; i < state->stream_configs.num_configs; ++i) {
-		struct v4l2_subdev_stream_config *cfg =
-			&state->stream_configs.configs[i];
 
-		if (cfg->pad != pad || !(streams_mask & BIT_ULL(cfg->stream)))
-			continue;
-
-		found_streams |= BIT_ULL(cfg->stream);
-
-		if (cfg->enabled) {
+	if (sd->entity.num_pads == 1) {
+		if (sd->enabled_streams) {
 			dev_dbg(dev, "stream %u already enabled on %s:%u\n",
-				cfg->stream, sd->entity.name, pad);
+				0, sd->entity.name, pad);
 			ret = -EALREADY;
 			goto done;
 		}
+
+		found_streams = BIT_ULL(0);
+	} else {
+		for (i = 0; i < state->stream_configs.num_configs; ++i) {
+			struct v4l2_subdev_stream_config *cfg =
+				&state->stream_configs.configs[i];
+
+			if (cfg->pad != pad || !(streams_mask & BIT_ULL(cfg->stream)))
+				continue;
+
+			found_streams |= BIT_ULL(cfg->stream);
+
+			if (cfg->enabled) {
+				dev_dbg(dev, "stream %u already enabled on %s:%u\n",
+					cfg->stream, sd->entity.name, pad);
+				ret = -EALREADY;
+				goto done;
+			}
+		}
 	}
 
 	if (found_streams != streams_mask) {
@@ -2164,13 +2176,17 @@  int v4l2_subdev_enable_streams(struct v4l2_subdev *sd, u32 pad,
 		goto done;
 	}
 
-	/* Mark the streams as enabled. */
-	for (i = 0; i < state->stream_configs.num_configs; ++i) {
-		struct v4l2_subdev_stream_config *cfg =
-			&state->stream_configs.configs[i];
+	if (sd->entity.num_pads == 1) {
+		sd->enabled_streams |= streams_mask;
+	} else {
+		/* Mark the streams as enabled. */
+		for (i = 0; i < state->stream_configs.num_configs; ++i) {
+			struct v4l2_subdev_stream_config *cfg =
+				&state->stream_configs.configs[i];
 
-		if (cfg->pad == pad && (streams_mask & BIT_ULL(cfg->stream)))
-			cfg->enabled = true;
+			if (cfg->pad == pad && (streams_mask & BIT_ULL(cfg->stream)))
+				cfg->enabled = true;
+		}
 	}
 
 done:
@@ -2246,21 +2262,32 @@  int v4l2_subdev_disable_streams(struct v4l2_subdev *sd, u32 pad,
 	 * Verify that the requested streams exist and that they are not
 	 * already disabled.
 	 */
-	for (i = 0; i < state->stream_configs.num_configs; ++i) {
-		struct v4l2_subdev_stream_config *cfg =
-			&state->stream_configs.configs[i];
-
-		if (cfg->pad != pad || !(streams_mask & BIT_ULL(cfg->stream)))
-			continue;
-
-		found_streams |= BIT_ULL(cfg->stream);
-
-		if (!cfg->enabled) {
+	if (sd->entity.num_pads == 1) {
+		if (!sd->enabled_streams) {
 			dev_dbg(dev, "stream %u already disabled on %s:%u\n",
-				cfg->stream, sd->entity.name, pad);
+				0, sd->entity.name, pad);
 			ret = -EALREADY;
 			goto done;
 		}
+
+		found_streams = BIT_ULL(0);
+	} else {
+		for (i = 0; i < state->stream_configs.num_configs; ++i) {
+			struct v4l2_subdev_stream_config *cfg =
+				&state->stream_configs.configs[i];
+
+			if (cfg->pad != pad || !(streams_mask & BIT_ULL(cfg->stream)))
+				continue;
+
+			found_streams |= BIT_ULL(cfg->stream);
+
+			if (!cfg->enabled) {
+				dev_dbg(dev, "stream %u already disabled on %s:%u\n",
+					cfg->stream, sd->entity.name, pad);
+				ret = -EALREADY;
+				goto done;
+			}
+		}
 	}
 
 	if (found_streams != streams_mask) {
@@ -2281,13 +2308,17 @@  int v4l2_subdev_disable_streams(struct v4l2_subdev *sd, u32 pad,
 		goto done;
 	}
 
-	/* Mark the streams as disabled. */
-	for (i = 0; i < state->stream_configs.num_configs; ++i) {
-		struct v4l2_subdev_stream_config *cfg =
-			&state->stream_configs.configs[i];
+	if (sd->entity.num_pads == 1) {
+		sd->enabled_streams &= ~streams_mask;
+	} else {
+		/* Mark the streams as disabled. */
+		for (i = 0; i < state->stream_configs.num_configs; ++i) {
+			struct v4l2_subdev_stream_config *cfg =
+				&state->stream_configs.configs[i];
 
-		if (cfg->pad == pad && (streams_mask & BIT_ULL(cfg->stream)))
-			cfg->enabled = false;
+			if (cfg->pad == pad && (streams_mask & BIT_ULL(cfg->stream)))
+				cfg->enabled = false;
+		}
 	}
 
 done:
@@ -2325,8 +2356,12 @@  int v4l2_subdev_s_stream_helper(struct v4l2_subdev *sd, int enable)
 	 */
 	state = v4l2_subdev_lock_and_get_active_state(sd);
 
-	for_each_active_route(&state->routing, route)
-		source_mask |= BIT_ULL(route->source_stream);
+	if (sd->entity.num_pads == 1) {
+		source_mask = BIT_ULL(0);
+	} else {
+		for_each_active_route(&state->routing, route)
+			source_mask |= BIT_ULL(route->source_stream);
+	}
 
 	v4l2_subdev_unlock_state(state);
 
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index a9e6b8146279..39b230f7b3c8 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -1041,8 +1041,8 @@  struct v4l2_subdev_platform_data {
  *		  v4l2_subdev_init_finalize().
  * @enabled_streams: Bitmask of enabled streams used by
  *		     v4l2_subdev_enable_streams() and
- *		     v4l2_subdev_disable_streams() helper functions for fallback
- *		     cases.
+ *		     v4l2_subdev_disable_streams() helper functions. This is
+ *		     for fallback cases and for subdevs with single pads.
  *
  * Each instance of a subdev driver should create this struct, either
  * stand-alone or embedded in a larger struct.