* [PATCH 1/3] media: subdev: Add v4l2_get_frame_desc_passthrough helper
2025-02-18 16:15 [PATCH 0/3] media: subdev: Add v4l2_get_frame_desc_passthrough and use it Tomi Valkeinen
@ 2025-02-18 16:15 ` Tomi Valkeinen
2025-03-13 10:53 ` Sakari Ailus
2025-02-18 16:15 ` [PATCH 2/3] media: i2c: ds90ub953: Use v4l2_get_frame_desc_passthrough Tomi Valkeinen
2025-02-18 16:15 ` [PATCH 3/3] media: i2c: ds90ub913: " Tomi Valkeinen
2 siblings, 1 reply; 5+ messages in thread
From: Tomi Valkeinen @ 2025-02-18 16:15 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Laurent Pinchart, Sakari Ailus
Cc: linux-media, linux-kernel, Tomi Valkeinen
Add a helper for v4l2_subdev_pad_ops.v4l2_get_frame_desc operation. The
helper can be used when the subdevice directly passes through the
streams.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
drivers/media/v4l2-core/v4l2-subdev.c | 96 +++++++++++++++++++++++++++++++++++
include/media/v4l2-subdev.h | 21 ++++++++
2 files changed, 117 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index cde1774c9098..f04de851d777 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -2500,6 +2500,102 @@ int v4l2_subdev_s_stream_helper(struct v4l2_subdev *sd, int enable)
}
EXPORT_SYMBOL_GPL(v4l2_subdev_s_stream_helper);
+int v4l2_get_frame_desc_passthrough(struct v4l2_subdev *sd, unsigned int pad,
+ struct v4l2_mbus_frame_desc *fd)
+{
+ struct v4l2_subdev_route *route;
+ struct v4l2_subdev_state *state;
+ int ret;
+ const struct media_pad *pads = sd->entity.pads;
+ struct media_pad *local_sink_pad;
+
+ if (WARN_ON(!(pads[pad].flags & MEDIA_PAD_FL_SOURCE)))
+ return -EINVAL;
+
+ state = v4l2_subdev_lock_and_get_active_state(sd);
+
+ /* Iterate over sink pads */
+ media_entity_for_each_pad(&sd->entity, local_sink_pad) {
+ struct v4l2_mbus_frame_desc source_fd;
+ bool have_source_fd = false;
+
+ if (!(local_sink_pad->flags & MEDIA_PAD_FL_SINK))
+ continue;
+
+ /*
+ * Copy frame desc entries for the streams going from the sink
+ * pad to the requested pad
+ */
+ for_each_active_route(&state->routing, route) {
+ struct v4l2_mbus_frame_desc_entry *source_entry = NULL;
+ unsigned int i;
+ struct media_pad *remote_source_pad;
+ struct v4l2_subdev *remote_sd;
+
+ if (route->source_pad != pad ||
+ route->sink_pad != local_sink_pad->index)
+ continue;
+
+ if (!have_source_fd) {
+ remote_source_pad = media_pad_remote_pad_unique(local_sink_pad);
+ if (!remote_source_pad) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ remote_sd = media_entity_to_v4l2_subdev(remote_source_pad->entity);
+ if (!remote_sd) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ ret = v4l2_subdev_call(remote_sd, pad,
+ get_frame_desc,
+ remote_source_pad->index,
+ &source_fd);
+ if (ret)
+ goto out_unlock;
+
+ have_source_fd = true;
+
+ if (fd->num_entries == 0) {
+ fd->type = source_fd.type;
+ } else if (fd->type != source_fd.type) {
+ ret = -EPIPE;
+ goto out_unlock;
+ }
+ }
+
+ for (i = 0; i < source_fd.num_entries; i++) {
+ if (source_fd.entry[i].stream == route->sink_stream) {
+ source_entry = &source_fd.entry[i];
+ break;
+ }
+ }
+
+ if (!source_entry) {
+ dev_err(sd->dev,
+ "Failed to find stream %u from source frame desc\n",
+ route->sink_stream);
+ ret = -EPIPE;
+ goto out_unlock;
+ }
+
+ fd->entry[fd->num_entries] = *source_entry;
+
+ fd->entry[fd->num_entries].stream = route->source_stream;
+
+ fd->num_entries++;
+ }
+ }
+
+out_unlock:
+ v4l2_subdev_unlock_state(state);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_get_frame_desc_passthrough);
+
#endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
#endif /* CONFIG_MEDIA_CONTROLLER */
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 2f2200875b03..d42fb3f55515 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -1746,6 +1746,27 @@ int v4l2_subdev_disable_streams(struct v4l2_subdev *sd, u32 pad,
*/
int v4l2_subdev_s_stream_helper(struct v4l2_subdev *sd, int enable);
+/**
+ * v4l2_get_frame_desc_passthrough() - Helper to implement the subdev
+ * v4l2_get_frame_desc operation in simple passthrough cases
+ * @sd: The subdevice
+ * @pad: The source pad index
+ * @fd: The mbus frame desc
+ *
+ * Subdevice drivers that only pass through the streams can use this helper
+ * to implement the &v4l2_subdev_pad_ops.v4l2_get_frame_desc operation.
+ *
+ * The helper will call get_frame_desc on the subdevice's sources, create a new
+ * frame desc which contains only the streams on the given source pad. The data
+ * for each frame desc entry is copied directly from the data provided from the
+ * calls to the subdevice's sources, with the exception of the 'stream' field
+ * which is set according to the subdevice's routing table.
+ *
+ * Return: 0 on success, or a negative error code otherwise.
+ */
+int v4l2_get_frame_desc_passthrough(struct v4l2_subdev *sd, unsigned int pad,
+ struct v4l2_mbus_frame_desc *fd);
+
#endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
#endif /* CONFIG_MEDIA_CONTROLLER */
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/3] media: subdev: Add v4l2_get_frame_desc_passthrough helper
2025-02-18 16:15 ` [PATCH 1/3] media: subdev: Add v4l2_get_frame_desc_passthrough helper Tomi Valkeinen
@ 2025-03-13 10:53 ` Sakari Ailus
0 siblings, 0 replies; 5+ messages in thread
From: Sakari Ailus @ 2025-03-13 10:53 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Mauro Carvalho Chehab, Laurent Pinchart, linux-media, linux-kernel
Moi,
Thanks for the set.
On Tue, Feb 18, 2025 at 06:15:43PM +0200, Tomi Valkeinen wrote:
> Add a helper for v4l2_subdev_pad_ops.v4l2_get_frame_desc operation. The
> helper can be used when the subdevice directly passes through the
> streams.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
> drivers/media/v4l2-core/v4l2-subdev.c | 96 +++++++++++++++++++++++++++++++++++
> include/media/v4l2-subdev.h | 21 ++++++++
> 2 files changed, 117 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index cde1774c9098..f04de851d777 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -2500,6 +2500,102 @@ int v4l2_subdev_s_stream_helper(struct v4l2_subdev *sd, int enable)
> }
> EXPORT_SYMBOL_GPL(v4l2_subdev_s_stream_helper);
>
> +int v4l2_get_frame_desc_passthrough(struct v4l2_subdev *sd, unsigned int pad,
Shouldn't this be called v4l2_subdev_get_frame_desc_passthrough? It's long
but that's unlikely to be a problem still.
> + struct v4l2_mbus_frame_desc *fd)
> +{
> + struct v4l2_subdev_route *route;
> + struct v4l2_subdev_state *state;
> + int ret;
> + const struct media_pad *pads = sd->entity.pads;
> + struct media_pad *local_sink_pad;
> +
> + if (WARN_ON(!(pads[pad].flags & MEDIA_PAD_FL_SOURCE)))
> + return -EINVAL;
> +
> + state = v4l2_subdev_lock_and_get_active_state(sd);
> +
> + /* Iterate over sink pads */
> + media_entity_for_each_pad(&sd->entity, local_sink_pad) {
> + struct v4l2_mbus_frame_desc source_fd;
> + bool have_source_fd = false;
> +
> + if (!(local_sink_pad->flags & MEDIA_PAD_FL_SINK))
> + continue;
> +
> + /*
> + * Copy frame desc entries for the streams going from the sink
> + * pad to the requested pad
> + */
> + for_each_active_route(&state->routing, route) {
> + struct v4l2_mbus_frame_desc_entry *source_entry = NULL;
> + unsigned int i;
> + struct media_pad *remote_source_pad;
> + struct v4l2_subdev *remote_sd;
> +
> + if (route->source_pad != pad ||
> + route->sink_pad != local_sink_pad->index)
> + continue;
> +
> + if (!have_source_fd) {
> + remote_source_pad = media_pad_remote_pad_unique(local_sink_pad);
> + if (!remote_source_pad) {
> + ret = -EINVAL;
> + goto out_unlock;
> + }
> +
> + remote_sd = media_entity_to_v4l2_subdev(remote_source_pad->entity);
> + if (!remote_sd) {
> + ret = -EINVAL;
Having dev_dbg()'s for all these cases would be nice. Otherwise it'll be
hard to figure out what went wrong.
> + goto out_unlock;
> + }
> +
> + ret = v4l2_subdev_call(remote_sd, pad,
> + get_frame_desc,
> + remote_source_pad->index,
> + &source_fd);
> + if (ret)
> + goto out_unlock;
> +
> + have_source_fd = true;
> +
> + if (fd->num_entries == 0) {
> + fd->type = source_fd.type;
> + } else if (fd->type != source_fd.type) {
> + ret = -EPIPE;
> + goto out_unlock;
> + }
> + }
> +
> + for (i = 0; i < source_fd.num_entries; i++) {
> + if (source_fd.entry[i].stream == route->sink_stream) {
> + source_entry = &source_fd.entry[i];
> + break;
> + }
> + }
> +
> + if (!source_entry) {
> + dev_err(sd->dev,
> + "Failed to find stream %u from source frame desc\n",
> + route->sink_stream);
> + ret = -EPIPE;
> + goto out_unlock;
> + }
> +
> + fd->entry[fd->num_entries] = *source_entry;
> +
> + fd->entry[fd->num_entries].stream = route->source_stream;
> +
> + fd->num_entries++;
Could you check you're not overflowing what's allocated?
> + }
> + }
> +
> +out_unlock:
> + v4l2_subdev_unlock_state(state);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_get_frame_desc_passthrough);
> +
> #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
>
> #endif /* CONFIG_MEDIA_CONTROLLER */
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 2f2200875b03..d42fb3f55515 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -1746,6 +1746,27 @@ int v4l2_subdev_disable_streams(struct v4l2_subdev *sd, u32 pad,
> */
> int v4l2_subdev_s_stream_helper(struct v4l2_subdev *sd, int enable);
>
> +/**
> + * v4l2_get_frame_desc_passthrough() - Helper to implement the subdev
> + * v4l2_get_frame_desc operation in simple passthrough cases
> + * @sd: The subdevice
> + * @pad: The source pad index
> + * @fd: The mbus frame desc
> + *
> + * Subdevice drivers that only pass through the streams can use this helper
> + * to implement the &v4l2_subdev_pad_ops.v4l2_get_frame_desc operation.
> + *
> + * The helper will call get_frame_desc on the subdevice's sources, create a new
> + * frame desc which contains only the streams on the given source pad. The data
> + * for each frame desc entry is copied directly from the data provided from the
> + * calls to the subdevice's sources, with the exception of the 'stream' field
> + * which is set according to the subdevice's routing table.
> + *
> + * Return: 0 on success, or a negative error code otherwise.
> + */
> +int v4l2_get_frame_desc_passthrough(struct v4l2_subdev *sd, unsigned int pad,
> + struct v4l2_mbus_frame_desc *fd);
> +
> #endif /* CONFIG_VIDEO_V4L2_SUBDEV_API */
>
> #endif /* CONFIG_MEDIA_CONTROLLER */
>
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/3] media: i2c: ds90ub953: Use v4l2_get_frame_desc_passthrough
2025-02-18 16:15 [PATCH 0/3] media: subdev: Add v4l2_get_frame_desc_passthrough and use it Tomi Valkeinen
2025-02-18 16:15 ` [PATCH 1/3] media: subdev: Add v4l2_get_frame_desc_passthrough helper Tomi Valkeinen
@ 2025-02-18 16:15 ` Tomi Valkeinen
2025-02-18 16:15 ` [PATCH 3/3] media: i2c: ds90ub913: " Tomi Valkeinen
2 siblings, 0 replies; 5+ messages in thread
From: Tomi Valkeinen @ 2025-02-18 16:15 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Laurent Pinchart, Sakari Ailus
Cc: linux-media, linux-kernel, Tomi Valkeinen
Use the new v4l2_get_frame_desc_passthrough helper for .get_frame_desc.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
drivers/media/i2c/ds90ub953.c | 61 +------------------------------------------
1 file changed, 1 insertion(+), 60 deletions(-)
diff --git a/drivers/media/i2c/ds90ub953.c b/drivers/media/i2c/ds90ub953.c
index 46569381b332..6622a150f802 100644
--- a/drivers/media/i2c/ds90ub953.c
+++ b/drivers/media/i2c/ds90ub953.c
@@ -489,65 +489,6 @@ static int ub953_set_routing(struct v4l2_subdev *sd,
return _ub953_set_routing(sd, state, routing);
}
-static int ub953_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
- struct v4l2_mbus_frame_desc *fd)
-{
- struct ub953_data *priv = sd_to_ub953(sd);
- struct v4l2_mbus_frame_desc source_fd;
- struct v4l2_subdev_route *route;
- struct v4l2_subdev_state *state;
- int ret;
-
- if (pad != UB953_PAD_SOURCE)
- return -EINVAL;
-
- ret = v4l2_subdev_call(priv->source_sd, pad, get_frame_desc,
- priv->source_sd_pad, &source_fd);
- if (ret)
- return ret;
-
- fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
-
- state = v4l2_subdev_lock_and_get_active_state(sd);
-
- for_each_active_route(&state->routing, route) {
- struct v4l2_mbus_frame_desc_entry *source_entry = NULL;
- unsigned int i;
-
- if (route->source_pad != pad)
- continue;
-
- for (i = 0; i < source_fd.num_entries; i++) {
- if (source_fd.entry[i].stream == route->sink_stream) {
- source_entry = &source_fd.entry[i];
- break;
- }
- }
-
- if (!source_entry) {
- dev_err(&priv->client->dev,
- "Failed to find stream from source frame desc\n");
- ret = -EPIPE;
- goto out_unlock;
- }
-
- fd->entry[fd->num_entries].stream = route->source_stream;
- fd->entry[fd->num_entries].flags = source_entry->flags;
- fd->entry[fd->num_entries].length = source_entry->length;
- fd->entry[fd->num_entries].pixelcode = source_entry->pixelcode;
- fd->entry[fd->num_entries].bus.csi2.vc =
- source_entry->bus.csi2.vc;
- fd->entry[fd->num_entries].bus.csi2.dt =
- source_entry->bus.csi2.dt;
-
- fd->num_entries++;
- }
-
-out_unlock:
- v4l2_subdev_unlock_state(state);
-
- return ret;
-}
static int ub953_set_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state,
@@ -723,7 +664,7 @@ static const struct v4l2_subdev_pad_ops ub953_pad_ops = {
.enable_streams = ub953_enable_streams,
.disable_streams = ub953_disable_streams,
.set_routing = ub953_set_routing,
- .get_frame_desc = ub953_get_frame_desc,
+ .get_frame_desc = v4l2_get_frame_desc_passthrough,
.get_fmt = v4l2_subdev_get_fmt,
.set_fmt = ub953_set_fmt,
};
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 3/3] media: i2c: ds90ub913: Use v4l2_get_frame_desc_passthrough
2025-02-18 16:15 [PATCH 0/3] media: subdev: Add v4l2_get_frame_desc_passthrough and use it Tomi Valkeinen
2025-02-18 16:15 ` [PATCH 1/3] media: subdev: Add v4l2_get_frame_desc_passthrough helper Tomi Valkeinen
2025-02-18 16:15 ` [PATCH 2/3] media: i2c: ds90ub953: Use v4l2_get_frame_desc_passthrough Tomi Valkeinen
@ 2025-02-18 16:15 ` Tomi Valkeinen
2 siblings, 0 replies; 5+ messages in thread
From: Tomi Valkeinen @ 2025-02-18 16:15 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Laurent Pinchart, Sakari Ailus
Cc: linux-media, linux-kernel, Tomi Valkeinen
Use the new v4l2_get_frame_desc_passthrough helper for .get_frame_desc.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
drivers/media/i2c/ds90ub913.c | 59 +------------------------------------------
1 file changed, 1 insertion(+), 58 deletions(-)
diff --git a/drivers/media/i2c/ds90ub913.c b/drivers/media/i2c/ds90ub913.c
index fd2d2d5272bf..9ae761d7e3a5 100644
--- a/drivers/media/i2c/ds90ub913.c
+++ b/drivers/media/i2c/ds90ub913.c
@@ -358,63 +358,6 @@ static int ub913_set_routing(struct v4l2_subdev *sd,
return _ub913_set_routing(sd, state, routing);
}
-static int ub913_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
- struct v4l2_mbus_frame_desc *fd)
-{
- struct ub913_data *priv = sd_to_ub913(sd);
- const struct v4l2_subdev_krouting *routing;
- struct v4l2_mbus_frame_desc source_fd;
- struct v4l2_subdev_route *route;
- struct v4l2_subdev_state *state;
- int ret;
-
- if (pad != UB913_PAD_SOURCE)
- return -EINVAL;
-
- ret = v4l2_subdev_call(priv->source_sd, pad, get_frame_desc,
- priv->source_sd_pad, &source_fd);
- if (ret)
- return ret;
-
- fd->type = V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL;
-
- state = v4l2_subdev_lock_and_get_active_state(sd);
-
- routing = &state->routing;
-
- for_each_active_route(routing, route) {
- unsigned int i;
-
- if (route->source_pad != pad)
- continue;
-
- for (i = 0; i < source_fd.num_entries; i++) {
- if (source_fd.entry[i].stream == route->sink_stream)
- break;
- }
-
- if (i == source_fd.num_entries) {
- dev_err(&priv->client->dev,
- "Failed to find stream from source frame desc\n");
- ret = -EPIPE;
- goto out_unlock;
- }
-
- fd->entry[fd->num_entries].stream = route->source_stream;
- fd->entry[fd->num_entries].flags = source_fd.entry[i].flags;
- fd->entry[fd->num_entries].length = source_fd.entry[i].length;
- fd->entry[fd->num_entries].pixelcode =
- source_fd.entry[i].pixelcode;
-
- fd->num_entries++;
- }
-
-out_unlock:
- v4l2_subdev_unlock_state(state);
-
- return ret;
-}
-
static int ub913_set_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state,
struct v4l2_subdev_format *format)
@@ -514,7 +457,7 @@ static const struct v4l2_subdev_pad_ops ub913_pad_ops = {
.enable_streams = ub913_enable_streams,
.disable_streams = ub913_disable_streams,
.set_routing = ub913_set_routing,
- .get_frame_desc = ub913_get_frame_desc,
+ .get_frame_desc = v4l2_get_frame_desc_passthrough,
.get_fmt = v4l2_subdev_get_fmt,
.set_fmt = ub913_set_fmt,
};
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread