| Message ID | 20220301161156.1119557-35-tomi.valkeinen@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | v4l: routing and streams support | expand |
Hi Tomi, On Tue, Mar 01, 2022 at 06:11:54PM +0200, Tomi Valkeinen wrote: > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Add a helper function to translate streams between two pads of a subdev, > using the subdev's internal routing table. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > --- > drivers/media/v4l2-core/v4l2-subdev.c | 25 +++++++++++++++++++++++++ > include/media/v4l2-subdev.h | 23 +++++++++++++++++++++++ > 2 files changed, 48 insertions(+) > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > index e30338a53088..6a9fc62dacbf 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -1529,6 +1529,31 @@ v4l2_subdev_state_get_opposite_stream_format(struct v4l2_subdev_state *state, > } > EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_opposite_stream_format); > > +u64 v4l2_subdev_state_xlate_streams(const struct v4l2_subdev_state *state, > + u32 pad0, u32 pad1, u64 *streams) > +{ > + const struct v4l2_subdev_krouting *routing = &state->routing; > + struct v4l2_subdev_route *route; > + u64 streams0 = 0; > + u64 streams1 = 0; > + > + for_each_active_route(routing, route) { > + if (route->sink_pad == pad0 && route->source_pad == pad1 && > + (*streams & BIT(route->sink_stream))) { > + streams0 |= BIT(route->sink_stream); > + streams1 |= BIT(route->source_stream); > + } > + if (route->source_pad == pad0 && route->sink_pad == pad1 && > + (*streams & BIT(route->source_stream))) { > + streams0 |= BIT(route->source_stream); > + streams1 |= BIT(route->sink_stream); > + } > + } > + > + *streams = streams0; > + return streams1; I'll probably learn how this is used later, but I found it hard to immagine how the returned mask should be used. To a sink stream mask that contains multiple streams a source maks with multiple entries will be associated In example, the sink stream mask might look lik sink_stream_mask = {1, 0, 1, 0} So we are interested in translating stream 0 and 2 Assume 0->4 and 2->1 in the routing tabe, the resulting source stream mask will be source_stream_mask = {1, 0, 0, 1} How should the caller know what stream goes where ? > +} > + > int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state, > struct v4l2_subdev_format *format) > { > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > index 1eced0f47296..992debe116ac 100644 > --- a/include/media/v4l2-subdev.h > +++ b/include/media/v4l2-subdev.h > @@ -1497,6 +1497,29 @@ struct v4l2_mbus_framefmt * > v4l2_subdev_state_get_opposite_stream_format(struct v4l2_subdev_state *state, > u32 pad, u32 stream); > > +/** > + * v4l2_subdev_state_xlate_streams() - Translate streams from one pad to another Could we stress out that pads are on the same entity and this is not meant to translate over media links ? Is this necessary ? > + * > + * @state: Subdevice state > + * @pad0: The first pad > + * @pad1: The second pad > + * @streams: Streams bitmask on the first pad > + * > + * Streams on sink pads of a subdev are routed to source pads as expressed in > + * the subdev state routing table. Stream numbers don't necessarily match on > + * the sink and source side of a route. This function translates stream numbers > + * on @pad0, expressed as a bitmask in @streams, to the corresponding streams > + * on @pad1 using the routing table from the @state. It returns the stream mask > + * on @pad1, and updates @streams with the streams that have been found in the > + * routing table. > + * > + * @pad0 and @pad1 must be a sink and a source, in any order. > + * > + * Return: The bitmask of streams of @pad1 that are routed to @streams on @pad0. > + */ > +u64 v4l2_subdev_state_xlate_streams(const struct v4l2_subdev_state *state, > + u32 pad0, u32 pad1, u64 *streams); > + > /** > * v4l2_subdev_get_fmt() - Fill format based on state > * @sd: subdevice > -- > 2.25.1 >
On 16/03/2022 13:26, Jacopo Mondi wrote: > Hi Tomi, > > On Tue, Mar 01, 2022 at 06:11:54PM +0200, Tomi Valkeinen wrote: >> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> >> Add a helper function to translate streams between two pads of a subdev, >> using the subdev's internal routing table. >> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> >> --- >> drivers/media/v4l2-core/v4l2-subdev.c | 25 +++++++++++++++++++++++++ >> include/media/v4l2-subdev.h | 23 +++++++++++++++++++++++ >> 2 files changed, 48 insertions(+) >> >> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c >> index e30338a53088..6a9fc62dacbf 100644 >> --- a/drivers/media/v4l2-core/v4l2-subdev.c >> +++ b/drivers/media/v4l2-core/v4l2-subdev.c >> @@ -1529,6 +1529,31 @@ v4l2_subdev_state_get_opposite_stream_format(struct v4l2_subdev_state *state, >> } >> EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_opposite_stream_format); >> >> +u64 v4l2_subdev_state_xlate_streams(const struct v4l2_subdev_state *state, >> + u32 pad0, u32 pad1, u64 *streams) >> +{ >> + const struct v4l2_subdev_krouting *routing = &state->routing; >> + struct v4l2_subdev_route *route; >> + u64 streams0 = 0; >> + u64 streams1 = 0; >> + >> + for_each_active_route(routing, route) { >> + if (route->sink_pad == pad0 && route->source_pad == pad1 && >> + (*streams & BIT(route->sink_stream))) { >> + streams0 |= BIT(route->sink_stream); >> + streams1 |= BIT(route->source_stream); >> + } >> + if (route->source_pad == pad0 && route->sink_pad == pad1 && >> + (*streams & BIT(route->source_stream))) { >> + streams0 |= BIT(route->source_stream); >> + streams1 |= BIT(route->sink_stream); >> + } >> + } >> + >> + *streams = streams0; >> + return streams1; > > I'll probably learn how this is used later, but I found it hard to > immagine how the returned mask should be used. > > To a sink stream mask that contains multiple streams a source maks > with multiple entries will be associated > > In example, the sink stream mask might look lik > > sink_stream_mask = {1, 0, 1, 0} > > So we are interested in translating stream 0 and 2 > Assume 0->4 and 2->1 in the routing tabe, the resulting source stream > mask will be > > source_stream_mask = {1, 0, 0, 1} > > How should the caller know what stream goes where ? The use case for the function is for cases where all the streams go from one pad to another. Probably the main use is in subdevs with a single source and sink pad. This makes it easy to implement enable_streams() op: you just basically call v4l2_subdev_state_xlate_streams() to get the streams on the other side, and call v4l2_subdev_enable_streams(). Here's a version from ub913: > static int ub913_enable_streams(struct v4l2_subdev *sd, > struct v4l2_subdev_state *state, u32 pad, > u64 streams_mask) > { > struct ub913_data *priv = sd_to_ub913(sd); > struct media_pad *remote_pad; > u64 sink_streams; > int ret; > > if (streams_mask & priv->enabled_source_streams) > return -EALREADY; > > sink_streams = v4l2_subdev_state_xlate_streams( > state, UB913_PAD_SOURCE, UB913_PAD_SINK, &streams_mask); > > remote_pad = media_entity_remote_pad(&priv->pads[UB913_PAD_SINK]); > > ret = v4l2_subdev_enable_streams(priv->source_sd, remote_pad->index, > sink_streams); > if (ret) > return ret; > > priv->enabled_source_streams |= streams_mask; > > return 0; > } >> +} >> + >> int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state, >> struct v4l2_subdev_format *format) >> { >> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h >> index 1eced0f47296..992debe116ac 100644 >> --- a/include/media/v4l2-subdev.h >> +++ b/include/media/v4l2-subdev.h >> @@ -1497,6 +1497,29 @@ struct v4l2_mbus_framefmt * >> v4l2_subdev_state_get_opposite_stream_format(struct v4l2_subdev_state *state, >> u32 pad, u32 stream); >> >> +/** >> + * v4l2_subdev_state_xlate_streams() - Translate streams from one pad to another > > Could we stress out that pads are on the same entity and this is not > meant to translate over media links ? Is this necessary ? The streams on both sides of a link are identical, so there can be no translation in that case. The doc below explains the use, I'd rather not try to squeeze in a longer explanation in the title. But I think we should emphasize that this only makes sense on a simple routing case. >> + * >> + * @state: Subdevice state >> + * @pad0: The first pad >> + * @pad1: The second pad >> + * @streams: Streams bitmask on the first pad >> + * >> + * Streams on sink pads of a subdev are routed to source pads as expressed in >> + * the subdev state routing table. Stream numbers don't necessarily match on >> + * the sink and source side of a route. This function translates stream numbers >> + * on @pad0, expressed as a bitmask in @streams, to the corresponding streams >> + * on @pad1 using the routing table from the @state. It returns the stream mask >> + * on @pad1, and updates @streams with the streams that have been found in the >> + * routing table. >> + * >> + * @pad0 and @pad1 must be a sink and a source, in any order. >> + * >> + * Return: The bitmask of streams of @pad1 that are routed to @streams on @pad0. >> + */ >> +u64 v4l2_subdev_state_xlate_streams(const struct v4l2_subdev_state *state, >> + u32 pad0, u32 pad1, u64 *streams); >> + >> /** >> * v4l2_subdev_get_fmt() - Fill format based on state >> * @sd: subdevice >> -- >> 2.25.1 >>
On 17/03/2022 11:27, Tomi Valkeinen wrote: > On 16/03/2022 13:26, Jacopo Mondi wrote: >> Hi Tomi, >> >> On Tue, Mar 01, 2022 at 06:11:54PM +0200, Tomi Valkeinen wrote: >>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >>> >>> Add a helper function to translate streams between two pads of a subdev, >>> using the subdev's internal routing table. >>> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> >>> --- >>> drivers/media/v4l2-core/v4l2-subdev.c | 25 +++++++++++++++++++++++++ >>> include/media/v4l2-subdev.h | 23 +++++++++++++++++++++++ >>> 2 files changed, 48 insertions(+) >>> >>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c >>> b/drivers/media/v4l2-core/v4l2-subdev.c >>> index e30338a53088..6a9fc62dacbf 100644 >>> --- a/drivers/media/v4l2-core/v4l2-subdev.c >>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c >>> @@ -1529,6 +1529,31 @@ >>> v4l2_subdev_state_get_opposite_stream_format(struct v4l2_subdev_state >>> *state, >>> } >>> EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_opposite_stream_format); >>> >>> +u64 v4l2_subdev_state_xlate_streams(const struct v4l2_subdev_state >>> *state, >>> + u32 pad0, u32 pad1, u64 *streams) >>> +{ >>> + const struct v4l2_subdev_krouting *routing = &state->routing; >>> + struct v4l2_subdev_route *route; >>> + u64 streams0 = 0; >>> + u64 streams1 = 0; >>> + >>> + for_each_active_route(routing, route) { >>> + if (route->sink_pad == pad0 && route->source_pad == pad1 && >>> + (*streams & BIT(route->sink_stream))) { >>> + streams0 |= BIT(route->sink_stream); >>> + streams1 |= BIT(route->source_stream); >>> + } >>> + if (route->source_pad == pad0 && route->sink_pad == pad1 && >>> + (*streams & BIT(route->source_stream))) { >>> + streams0 |= BIT(route->source_stream); >>> + streams1 |= BIT(route->sink_stream); >>> + } >>> + } >>> + >>> + *streams = streams0; >>> + return streams1; >> >> I'll probably learn how this is used later, but I found it hard to >> immagine how the returned mask should be used. >> >> To a sink stream mask that contains multiple streams a source maks >> with multiple entries will be associated >> >> In example, the sink stream mask might look lik >> >> sink_stream_mask = {1, 0, 1, 0} >> >> So we are interested in translating stream 0 and 2 >> Assume 0->4 and 2->1 in the routing tabe, the resulting source stream >> mask will be >> >> source_stream_mask = {1, 0, 0, 1} >> >> How should the caller know what stream goes where ? > > The use case for the function is for cases where all the streams go from > one pad to another. Probably the main use is in subdevs with a single > source and sink pad. This makes it easy to implement enable_streams() > op: you just basically call v4l2_subdev_state_xlate_streams() to get the > streams on the other side, and call v4l2_subdev_enable_streams(). Here's > a version from ub913: > >> static int ub913_enable_streams(struct v4l2_subdev *sd, >> struct v4l2_subdev_state *state, u32 pad, >> u64 streams_mask) >> { >> struct ub913_data *priv = sd_to_ub913(sd); >> struct media_pad *remote_pad; >> u64 sink_streams; >> int ret; >> >> if (streams_mask & priv->enabled_source_streams) >> return -EALREADY; >> >> sink_streams = v4l2_subdev_state_xlate_streams( >> state, UB913_PAD_SOURCE, UB913_PAD_SINK, &streams_mask); >> >> remote_pad = media_entity_remote_pad(&priv->pads[UB913_PAD_SINK]); >> >> ret = v4l2_subdev_enable_streams(priv->source_sd, remote_pad->index, >> sink_streams); >> if (ret) >> return ret; >> >> priv->enabled_source_streams |= streams_mask; >> >> return 0; >> } > > > >>> +} >>> + >>> int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct >>> v4l2_subdev_state *state, >>> struct v4l2_subdev_format *format) >>> { >>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h >>> index 1eced0f47296..992debe116ac 100644 >>> --- a/include/media/v4l2-subdev.h >>> +++ b/include/media/v4l2-subdev.h >>> @@ -1497,6 +1497,29 @@ struct v4l2_mbus_framefmt * >>> v4l2_subdev_state_get_opposite_stream_format(struct >>> v4l2_subdev_state *state, >>> u32 pad, u32 stream); >>> >>> +/** >>> + * v4l2_subdev_state_xlate_streams() - Translate streams from one >>> pad to another >> >> Could we stress out that pads are on the same entity and this is not >> meant to translate over media links ? Is this necessary ? > > The streams on both sides of a link are identical, so there can be no > translation in that case. > > The doc below explains the use, I'd rather not try to squeeze in a > longer explanation in the title. > > But I think we should emphasize that this only makes sense on a simple > routing case. Ah, never mind. The function can be used in multi-pad cases too, so we don't need to emphasize the above. The function tells how a set of streams from pad A translate to a set of streams on pad B. So you only know the set of streams, not how a single stream goes. For that you need to look at the routing table. Tomi
Hi, On 07/07/2022 13:27, Tomasz Figa wrote: > Hi Tomi, Laurent, > > On Tue, Mar 01, 2022 at 06:11:54PM +0200, Tomi Valkeinen wrote: >> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> >> Add a helper function to translate streams between two pads of a subdev, >> using the subdev's internal routing table. >> > > Thanks for the patch! Please see my comments inline. > >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> >> --- >> drivers/media/v4l2-core/v4l2-subdev.c | 25 +++++++++++++++++++++++++ >> include/media/v4l2-subdev.h | 23 +++++++++++++++++++++++ >> 2 files changed, 48 insertions(+) >> >> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c >> index e30338a53088..6a9fc62dacbf 100644 >> --- a/drivers/media/v4l2-core/v4l2-subdev.c >> +++ b/drivers/media/v4l2-core/v4l2-subdev.c >> @@ -1529,6 +1529,31 @@ v4l2_subdev_state_get_opposite_stream_format(struct v4l2_subdev_state *state, >> } >> EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_opposite_stream_format); >> >> +u64 v4l2_subdev_state_xlate_streams(const struct v4l2_subdev_state *state, >> + u32 pad0, u32 pad1, u64 *streams) >> +{ >> + const struct v4l2_subdev_krouting *routing = &state->routing; >> + struct v4l2_subdev_route *route; >> + u64 streams0 = 0; >> + u64 streams1 = 0; >> + >> + for_each_active_route(routing, route) { >> + if (route->sink_pad == pad0 && route->source_pad == pad1 && >> + (*streams & BIT(route->sink_stream))) { >> + streams0 |= BIT(route->sink_stream); >> + streams1 |= BIT(route->source_stream); >> + } >> + if (route->source_pad == pad0 && route->sink_pad == pad1 && >> + (*streams & BIT(route->source_stream))) { >> + streams0 |= BIT(route->source_stream); >> + streams1 |= BIT(route->sink_stream); >> + } >> + } >> + >> + *streams = streams0; >> + return streams1; >> +} >> + >> int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state, >> struct v4l2_subdev_format *format) >> { >> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h >> index 1eced0f47296..992debe116ac 100644 >> --- a/include/media/v4l2-subdev.h >> +++ b/include/media/v4l2-subdev.h >> @@ -1497,6 +1497,29 @@ struct v4l2_mbus_framefmt * >> v4l2_subdev_state_get_opposite_stream_format(struct v4l2_subdev_state *state, >> u32 pad, u32 stream); >> >> +/** >> + * v4l2_subdev_state_xlate_streams() - Translate streams from one pad to another >> + * >> + * @state: Subdevice state >> + * @pad0: The first pad >> + * @pad1: The second pad >> + * @streams: Streams bitmask on the first pad >> + * >> + * Streams on sink pads of a subdev are routed to source pads as expressed in >> + * the subdev state routing table. Stream numbers don't necessarily match on >> + * the sink and source side of a route. This function translates stream numbers >> + * on @pad0, expressed as a bitmask in @streams, to the corresponding streams >> + * on @pad1 using the routing table from the @state. It returns the stream mask >> + * on @pad1, and updates @streams with the streams that have been found in the >> + * routing table. >> + * >> + * @pad0 and @pad1 must be a sink and a source, in any order. >> + * >> + * Return: The bitmask of streams of @pad1 that are routed to @streams on @pad0. > > It might be just me, but somehow I associate "translate" with turning a > name from one namespace into a corresponding name from another > namespace. In this case I initially assumed that this function is > supposed to accept stream IDs from pad0 and return corresponding > stream IDs from pad1. However, it looks like this is move like - "tell > me which streams on pad1 are connected to the given pad0 streams". Is > this correct? So is your point that as the function returns a bitmask, and not a mapping of the stream IDs, this is not a translate function? > If yes, maybe v4l2_subdev_state_get_routed_streams() be a better name? I think this name would also fit quite well for a function that returns a mapping of the streams. So... I don't have a strong opinion on this. To me, neither the current name nor the proposed one clearly explains alone what the function does. Tomi
On Thu, Jul 14, 2022 at 4:41 PM Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote: > > Hi, > > On 07/07/2022 13:27, Tomasz Figa wrote: > > Hi Tomi, Laurent, > > > > On Tue, Mar 01, 2022 at 06:11:54PM +0200, Tomi Valkeinen wrote: > >> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> > >> Add a helper function to translate streams between two pads of a subdev, > >> using the subdev's internal routing table. > >> > > > > Thanks for the patch! Please see my comments inline. > > > >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > >> --- > >> drivers/media/v4l2-core/v4l2-subdev.c | 25 +++++++++++++++++++++++++ > >> include/media/v4l2-subdev.h | 23 +++++++++++++++++++++++ > >> 2 files changed, 48 insertions(+) > >> > >> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > >> index e30338a53088..6a9fc62dacbf 100644 > >> --- a/drivers/media/v4l2-core/v4l2-subdev.c > >> +++ b/drivers/media/v4l2-core/v4l2-subdev.c > >> @@ -1529,6 +1529,31 @@ v4l2_subdev_state_get_opposite_stream_format(struct v4l2_subdev_state *state, > >> } > >> EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_opposite_stream_format); > >> > >> +u64 v4l2_subdev_state_xlate_streams(const struct v4l2_subdev_state *state, > >> + u32 pad0, u32 pad1, u64 *streams) > >> +{ > >> + const struct v4l2_subdev_krouting *routing = &state->routing; > >> + struct v4l2_subdev_route *route; > >> + u64 streams0 = 0; > >> + u64 streams1 = 0; > >> + > >> + for_each_active_route(routing, route) { > >> + if (route->sink_pad == pad0 && route->source_pad == pad1 && > >> + (*streams & BIT(route->sink_stream))) { > >> + streams0 |= BIT(route->sink_stream); > >> + streams1 |= BIT(route->source_stream); > >> + } > >> + if (route->source_pad == pad0 && route->sink_pad == pad1 && > >> + (*streams & BIT(route->source_stream))) { > >> + streams0 |= BIT(route->source_stream); > >> + streams1 |= BIT(route->sink_stream); > >> + } > >> + } > >> + > >> + *streams = streams0; > >> + return streams1; > >> +} > >> + > >> int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state, > >> struct v4l2_subdev_format *format) > >> { > >> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > >> index 1eced0f47296..992debe116ac 100644 > >> --- a/include/media/v4l2-subdev.h > >> +++ b/include/media/v4l2-subdev.h > >> @@ -1497,6 +1497,29 @@ struct v4l2_mbus_framefmt * > >> v4l2_subdev_state_get_opposite_stream_format(struct v4l2_subdev_state *state, > >> u32 pad, u32 stream); > >> > >> +/** > >> + * v4l2_subdev_state_xlate_streams() - Translate streams from one pad to another > >> + * > >> + * @state: Subdevice state > >> + * @pad0: The first pad > >> + * @pad1: The second pad > >> + * @streams: Streams bitmask on the first pad > >> + * > >> + * Streams on sink pads of a subdev are routed to source pads as expressed in > >> + * the subdev state routing table. Stream numbers don't necessarily match on > >> + * the sink and source side of a route. This function translates stream numbers > >> + * on @pad0, expressed as a bitmask in @streams, to the corresponding streams > >> + * on @pad1 using the routing table from the @state. It returns the stream mask > >> + * on @pad1, and updates @streams with the streams that have been found in the > >> + * routing table. > >> + * > >> + * @pad0 and @pad1 must be a sink and a source, in any order. > >> + * > >> + * Return: The bitmask of streams of @pad1 that are routed to @streams on @pad0. > > > > It might be just me, but somehow I associate "translate" with turning a > > name from one namespace into a corresponding name from another > > namespace. In this case I initially assumed that this function is > > supposed to accept stream IDs from pad0 and return corresponding > > stream IDs from pad1. However, it looks like this is move like - "tell > > me which streams on pad1 are connected to the given pad0 streams". Is > > this correct? > > So is your point that as the function returns a bitmask, and not a > mapping of the stream IDs, this is not a translate function? Yep. > > > If yes, maybe v4l2_subdev_state_get_routed_streams() be a better name? > > I think this name would also fit quite well for a function that returns > a mapping of the streams. So... I don't have a strong opinion on this. > To me, neither the current name nor the proposed one clearly explains > alone what the function does. Well, since it's a kernel function, it probably can be renamed later. Anyway, I forgot to place "nit:" before the comment. It's not really a strong opinion from me either. Best regards, Tomasz
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index e30338a53088..6a9fc62dacbf 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -1529,6 +1529,31 @@ v4l2_subdev_state_get_opposite_stream_format(struct v4l2_subdev_state *state, } EXPORT_SYMBOL_GPL(v4l2_subdev_state_get_opposite_stream_format); +u64 v4l2_subdev_state_xlate_streams(const struct v4l2_subdev_state *state, + u32 pad0, u32 pad1, u64 *streams) +{ + const struct v4l2_subdev_krouting *routing = &state->routing; + struct v4l2_subdev_route *route; + u64 streams0 = 0; + u64 streams1 = 0; + + for_each_active_route(routing, route) { + if (route->sink_pad == pad0 && route->source_pad == pad1 && + (*streams & BIT(route->sink_stream))) { + streams0 |= BIT(route->sink_stream); + streams1 |= BIT(route->source_stream); + } + if (route->source_pad == pad0 && route->sink_pad == pad1 && + (*streams & BIT(route->source_stream))) { + streams0 |= BIT(route->source_stream); + streams1 |= BIT(route->sink_stream); + } + } + + *streams = streams0; + return streams1; +} + int v4l2_subdev_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *state, struct v4l2_subdev_format *format) { diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 1eced0f47296..992debe116ac 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -1497,6 +1497,29 @@ struct v4l2_mbus_framefmt * v4l2_subdev_state_get_opposite_stream_format(struct v4l2_subdev_state *state, u32 pad, u32 stream); +/** + * v4l2_subdev_state_xlate_streams() - Translate streams from one pad to another + * + * @state: Subdevice state + * @pad0: The first pad + * @pad1: The second pad + * @streams: Streams bitmask on the first pad + * + * Streams on sink pads of a subdev are routed to source pads as expressed in + * the subdev state routing table. Stream numbers don't necessarily match on + * the sink and source side of a route. This function translates stream numbers + * on @pad0, expressed as a bitmask in @streams, to the corresponding streams + * on @pad1 using the routing table from the @state. It returns the stream mask + * on @pad1, and updates @streams with the streams that have been found in the + * routing table. + * + * @pad0 and @pad1 must be a sink and a source, in any order. + * + * Return: The bitmask of streams of @pad1 that are routed to @streams on @pad0. + */ +u64 v4l2_subdev_state_xlate_streams(const struct v4l2_subdev_state *state, + u32 pad0, u32 pad1, u64 *streams); + /** * v4l2_subdev_get_fmt() - Fill format based on state * @sd: subdevice