| Message ID | 20210415130450.421168-23-tomi.valkeinen@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series | v4l: subdev internal routing | expand |
Hi Tomi, Thank you for the patch. 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@ideasonboard.com> > --- > 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. > + * @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
Hi Laurent, On 18/04/2021 22:04, Laurent Pinchart wrote: > Hi Tomi, > > Thank you for the patch. > > 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@ideasonboard.com> >> --- >> 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. >> + * @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 >
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@ideasonboard.com> > >> --- > >> 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
On 29/04/2021 04:43, Laurent Pinchart wrote: > 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@ideasonboard.com> >>>> --- >>>> 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. The link validation will call the function with both V4L2_DIR_SOURCEWARD and V4L2_DIR_SINKWARD. DS90UB960 driver also uses the function, with V4L2_DIR_SOURCEWARD. The name may not be the most clear one, but it's not easy to invent a name for something like this =). Why do you think the usage pattern is error prone? Tomi
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 + * @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
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@ideasonboard.com> --- drivers/media/v4l2-core/v4l2-subdev.c | 96 +++++++++++++++++++++++++++ include/media/v4l2-subdev.h | 26 ++++++++ 2 files changed, 122 insertions(+)