Hi Tomi, On Wed, Apr 21, 2021 at 04:04:22PM +0300, Tomi Valkeinen wrote: > On 18/04/2021 22:04, Laurent Pinchart wrote: > > On Thu, Apr 15, 2021 at 04:04:48PM +0300, Tomi Valkeinen wrote: > >> Add v4l2_subdev_get_format_dir() which can be used to find subdev format > >> for a specific stream on a multiplexed pad. The function will follow the > >> routes and links until it finds a non-multiplexed pad. > >> > >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx> > >> --- > >> drivers/media/v4l2-core/v4l2-subdev.c | 96 +++++++++++++++++++++++++++ > >> include/media/v4l2-subdev.h | 26 ++++++++ > >> 2 files changed, 122 insertions(+) > >> > >> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > >> index 7a4f71d8c6c3..430dbdaab080 100644 > >> --- a/drivers/media/v4l2-core/v4l2-subdev.c > >> +++ b/drivers/media/v4l2-core/v4l2-subdev.c > >> @@ -998,6 +998,102 @@ bool v4l2_subdev_has_route(struct v4l2_subdev_krouting *routing, > >> } > >> EXPORT_SYMBOL_GPL(v4l2_subdev_has_route); > >> > >> +int v4l2_subdev_get_format_dir(struct media_pad *pad, u16 stream, > >> + enum v4l2_direction dir, > >> + struct v4l2_subdev_format *fmt) > >> +{ > >> + struct device *dev = pad->entity->graph_obj.mdev->dev; > >> + int ret; > >> + int i; > >> + > >> + dev_dbg(dev, "%s '%s':%u:%u %s\n", __func__, > >> + pad->entity->name, pad->index, stream, > >> + dir == V4L2_DIR_SOURCEWARD ? "sourceward" : "sinkward"); > >> + > >> + while (true) { > >> + struct v4l2_subdev_krouting routing; > >> + struct v4l2_subdev_route *route; > >> + > >> + if (pad->entity->obj_type != MEDIA_ENTITY_TYPE_V4L2_SUBDEV) > >> + return -EINVAL; > >> + > >> + ret = v4l2_subdev_link_validate_get_format(pad, fmt); > >> + if (ret == 0) > >> + return 0; > >> + else if (ret != -ENOIOCTLCMD) > >> + return ret; > >> + > >> + if (pad->flags & > >> + (dir == V4L2_DIR_SINKWARD ? MEDIA_PAD_FL_SOURCE : > >> + MEDIA_PAD_FL_SINK)) { > >> + pad = media_entity_remote_pad(pad); > >> + > >> + if (!pad) > >> + return -EINVAL; > >> + > >> + if (pad->entity->obj_type != MEDIA_ENTITY_TYPE_V4L2_SUBDEV) > >> + return -EINVAL; > >> + > >> + ret = v4l2_subdev_link_validate_get_format(pad, fmt); > >> + if (ret == 0) > >> + return 0; > >> + else if (ret != -ENOIOCTLCMD) > >> + return ret; > >> + } > >> + > >> + ret = v4l2_subdev_get_krouting(media_entity_to_v4l2_subdev(pad->entity), &routing); > >> + if (ret) > >> + return ret; > >> + > >> + route = NULL; > >> + for (i = 0; i < routing.num_routes; ++i) { > >> + u16 near_pad = dir == V4L2_DIR_SINKWARD ? > >> + routing.routes[i].sink_pad : > >> + routing.routes[i].source_pad; > >> + u16 near_stream = dir == V4L2_DIR_SINKWARD ? > >> + routing.routes[i].sink_stream : > >> + routing.routes[i].source_stream; > >> + > >> + if (!(routing.routes[i].flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE)) > >> + continue; > >> + > >> + if (near_pad != pad->index) > >> + continue; > >> + > >> + if (near_stream != stream) > >> + continue; > >> + > >> + if (route) { > >> + dev_err(dev, > >> + "%s: '%s' has multiple active routes for stream %u\n", > >> + __func__, pad->entity->name, stream); > >> + v4l2_subdev_free_routing(&routing); > >> + return -EINVAL; > >> + } > >> + > >> + route = &routing.routes[i]; > >> + } > >> + > >> + if (!route) { > >> + dev_err(dev, "%s: no route found in '%s' for stream %u\n", > >> + __func__, pad->entity->name, stream); > >> + v4l2_subdev_free_routing(&routing); > >> + return -EINVAL; > >> + } > >> + > >> + if (dir == V4L2_DIR_SINKWARD) { > >> + pad = &pad->entity->pads[route->source_pad]; > >> + stream = route->source_stream; > >> + } else { > >> + pad = &pad->entity->pads[route->sink_pad]; > >> + stream = route->sink_stream; > >> + } > >> + > >> + v4l2_subdev_free_routing(&routing); > >> + } > >> +} > >> +EXPORT_SYMBOL_GPL(v4l2_subdev_get_format_dir); > >> + > >> int v4l2_subdev_link_validate(struct media_link *link) > >> { > >> struct v4l2_subdev *sink; > >> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > >> index 1843b77dd843..730631f9a091 100644 > >> --- a/include/media/v4l2-subdev.h > >> +++ b/include/media/v4l2-subdev.h > >> @@ -1239,4 +1239,30 @@ void v4l2_subdev_cpy_routing(struct v4l2_subdev_krouting *dst, > >> bool v4l2_subdev_has_route(struct v4l2_subdev_krouting *routing, > >> unsigned int pad0, unsigned int pad1); > >> > >> +/** > >> + * enum v4l2_direction - Direction either towards the source or the sink > >> + * > >> + * @V4L2_DIR_SOURCEWARD: Direction towards the source. > >> + * @V4L2_DIR_SINKWARD: Direction towards the sink. > >> + */ > >> +enum v4l2_direction { > >> + V4L2_DIR_SOURCEWARD, > >> + V4L2_DIR_SINKWARD, > >> +}; > >> + > >> +/** > >> + * v4l2_subdev_get_format_dir() - Find format by following streams > > > > The name is a bit cryptic, and the usage pattern error-prone. Can we do > > better ? In particular, if we could limit the usage of this function to > > be called on a non-multiplexed pad, we could drop the stream argument. > > Deducing the direction argument from the type of pad would also make the > > API simpler. > > Hmm, but that's not what the function does. It follows a specific > stream, from a multiplexed pad, so it has to get the stream as a parameter. > > We can't deduct the direction from the type of the pad. We can of course > define that given a source pad this function goes sourceward. But if > that's not what the caller wants, then the caller needs to first follow > the stream either direction to get a sink pad, and then call this > function, which doesn't make sense. What do the current callers need ? We don't have to implement something that is more generic or featureful than our needs dictate, as this is a very ad hoc function anyway. If we really need the full behaviour implemented here, we should at the very least rename the function, but I think it should be possible to do better overall, perhaps splitting the operation in the caller into cleaner functions. > >> + * @pad: The pad from which to start the search > >> + * @stream: The stream for which we want to find the format > >> + * @dir: The direction of the search > >> + * @fmt: Pointer to &struct v4l2_subdev_format where the found format is stored > >> + * > >> + * This function attempts to find v4l2_subdev_format for a specific stream on a > >> + * multiplexed pad by following the stream using routes and links to the specified > >> + * direction, until a non-multiplexed pad is found. > >> + */ > >> +int v4l2_subdev_get_format_dir(struct media_pad *pad, u16 stream, > >> + enum v4l2_direction dir, > >> + struct v4l2_subdev_format *fmt); > >> + > >> #endif -- Regards, Laurent Pinchart