[4/4] media: subdev: Support enable/disable_streams for non-streams subdevs
Commit Message
Currently a subdevice without streams support (V4L2_SUBDEV_FL_STREAMS),
e.g. a sensor subdevice with a single source pad, 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 subdevs without streams
support. As they don't support multiple streams, each pad must contain a
single stream, with stream ID 0, and rather than tracking enabled
streams, we can track enabled pads similarly to the
enable/disable_streams_fallback by using the sd->enabled_pads field.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
drivers/media/v4l2-core/v4l2-subdev.c | 86 +++++++++++++++++++++++------------
1 file changed, 58 insertions(+), 28 deletions(-)
Comments
Hi Tomi,
Thank you for the patch.
On Thu, Apr 04, 2024 at 01:50:03PM +0300, Tomi Valkeinen wrote:
> Currently a subdevice without streams support (V4L2_SUBDEV_FL_STREAMS),
> e.g. a sensor subdevice with a single source pad, 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 subdevs without streams
> support. As they don't support multiple streams, each pad must contain a
> single stream, with stream ID 0, and rather than tracking enabled
> streams, we can track enabled pads similarly to the
> enable/disable_streams_fallback by using the sd->enabled_pads field.
Could you explain in the commit message why this is desired ?
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
> drivers/media/v4l2-core/v4l2-subdev.c | 86 +++++++++++++++++++++++------------
> 1 file changed, 58 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 91298bb84e6b..d86f930be2c4 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -2158,21 +2158,32 @@ 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 (sd->flags & V4L2_SUBDEV_FL_STREAMS) {
> + 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;
> + if (cfg->pad != pad || !(streams_mask & BIT_ULL(cfg->stream)))
> + continue;
>
> - found_streams |= BIT_ULL(cfg->stream);
> + 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);
> + if (cfg->enabled) {
> + dev_dbg(dev, "stream %u already enabled on %s:%u\n",
> + cfg->stream, sd->entity.name, pad);
> + ret = -EALREADY;
> + goto done;
> + }
> + }
> + } else {
> + if (sd->enabled_pads & BIT_ULL(pad)) {
> + dev_dbg(dev, "stream 0 already enabled on %s:%u\n",
> + sd->entity.name, pad);
> ret = -EALREADY;
> goto done;
> }
> +
> + found_streams = BIT_ULL(0);
> }
>
> if (found_streams != streams_mask) {
> @@ -2194,12 +2205,16 @@ int v4l2_subdev_enable_streams(struct v4l2_subdev *sd, u32 pad,
> }
>
> /* 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->flags & V4L2_SUBDEV_FL_STREAMS) {
> + 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;
> + }
> + } else {
> + sd->enabled_pads |= BIT_ULL(pad);
> }
>
> done:
> @@ -2281,21 +2296,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 (sd->flags & V4L2_SUBDEV_FL_STREAMS) {
> + 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;
> + if (cfg->pad != pad || !(streams_mask & BIT_ULL(cfg->stream)))
> + continue;
>
> - found_streams |= BIT_ULL(cfg->stream);
> + 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);
> + if (!cfg->enabled) {
> + dev_dbg(dev, "stream %u already disabled on %s:%u\n",
> + cfg->stream, sd->entity.name, pad);
> + ret = -EALREADY;
> + goto done;
> + }
> + }
> + } else {
> + if (!(sd->enabled_pads & BIT_ULL(pad))) {
> + dev_dbg(dev, "stream 0 already disabled on %s:%u\n",
> + sd->entity.name, pad);
> ret = -EALREADY;
> goto done;
> }
> +
> + found_streams = BIT_ULL(0);
> }
>
> if (found_streams != streams_mask) {
> @@ -2317,12 +2343,16 @@ int v4l2_subdev_disable_streams(struct v4l2_subdev *sd, u32 pad,
> }
>
> /* 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->flags & V4L2_SUBDEV_FL_STREAMS) {
> + 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;
> + }
> + } else {
> + sd->enabled_pads &= ~BIT_ULL(pad);
> }
>
> done:
@@ -2158,21 +2158,32 @@ 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 (sd->flags & V4L2_SUBDEV_FL_STREAMS) {
+ 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;
+ if (cfg->pad != pad || !(streams_mask & BIT_ULL(cfg->stream)))
+ continue;
- found_streams |= BIT_ULL(cfg->stream);
+ 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);
+ if (cfg->enabled) {
+ dev_dbg(dev, "stream %u already enabled on %s:%u\n",
+ cfg->stream, sd->entity.name, pad);
+ ret = -EALREADY;
+ goto done;
+ }
+ }
+ } else {
+ if (sd->enabled_pads & BIT_ULL(pad)) {
+ dev_dbg(dev, "stream 0 already enabled on %s:%u\n",
+ sd->entity.name, pad);
ret = -EALREADY;
goto done;
}
+
+ found_streams = BIT_ULL(0);
}
if (found_streams != streams_mask) {
@@ -2194,12 +2205,16 @@ int v4l2_subdev_enable_streams(struct v4l2_subdev *sd, u32 pad,
}
/* 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->flags & V4L2_SUBDEV_FL_STREAMS) {
+ 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;
+ }
+ } else {
+ sd->enabled_pads |= BIT_ULL(pad);
}
done:
@@ -2281,21 +2296,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 (sd->flags & V4L2_SUBDEV_FL_STREAMS) {
+ 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;
+ if (cfg->pad != pad || !(streams_mask & BIT_ULL(cfg->stream)))
+ continue;
- found_streams |= BIT_ULL(cfg->stream);
+ 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);
+ if (!cfg->enabled) {
+ dev_dbg(dev, "stream %u already disabled on %s:%u\n",
+ cfg->stream, sd->entity.name, pad);
+ ret = -EALREADY;
+ goto done;
+ }
+ }
+ } else {
+ if (!(sd->enabled_pads & BIT_ULL(pad))) {
+ dev_dbg(dev, "stream 0 already disabled on %s:%u\n",
+ sd->entity.name, pad);
ret = -EALREADY;
goto done;
}
+
+ found_streams = BIT_ULL(0);
}
if (found_streams != streams_mask) {
@@ -2317,12 +2343,16 @@ int v4l2_subdev_disable_streams(struct v4l2_subdev *sd, u32 pad,
}
/* 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->flags & V4L2_SUBDEV_FL_STREAMS) {
+ 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;
+ }
+ } else {
+ sd->enabled_pads &= ~BIT_ULL(pad);
}
done: