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