| Message ID | 20210524104408.599645-21-tomi.valkeinen@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series | v4l: subdev internal routing and streams | expand |
Hi Tomi, Thank you for the patch. On Mon, May 24, 2021 at 01:44:01PM +0300, Tomi Valkeinen wrote: > Add route flag to indicate that the route is a source route. This means > that the route does not lead anywhere, and the sink_pad and sink_stream > should not be used. I don't like this much. It's not a route if it doesn't lead anywhere, so this flag seems like a hack. If we need a way to discover streams on a source, I'd rather have an explicit operation to do so. Can't the get frame descriptor operation be used for this ? If the need is to find out that we're reaching the end of a pipeline while going through links and routes, I'd rather have a pad flag to indicate that the pad is an endpoint. > A sensor which provides multiple streams should implement get_routing > and use the flag to mark the routes as sources. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > --- > include/uapi/linux/v4l2-subdev.h | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h > index 45c01799e2cd..f20491e1f53f 100644 > --- a/include/uapi/linux/v4l2-subdev.h > +++ b/include/uapi/linux/v4l2-subdev.h > @@ -200,6 +200,13 @@ struct v4l2_subdev_capability { > */ > #define V4L2_SUBDEV_ROUTE_FL_IMMUTABLE BIT(1) > > +/** > + * Is the route a source endpoint? A source endpoint route doesn't come > + * from "anywhere", and the sink_pad and sink_stream fields are unused. > + * Set by the driver. > + */ > +#define V4L2_SUBDEV_ROUTE_FL_SOURCE BIT(2) > + > /** > * struct v4l2_subdev_route - A route inside a subdev > * -- Regards, Laurent Pinchart
Hi Tomi, On Sun, Jun 06, 2021 at 01:44:54AM +0300, Laurent Pinchart wrote: > On Mon, May 24, 2021 at 01:44:01PM +0300, Tomi Valkeinen wrote: > > Add route flag to indicate that the route is a source route. This means > > that the route does not lead anywhere, and the sink_pad and sink_stream > > should not be used. > > I don't like this much. It's not a route if it doesn't lead anywhere, so > this flag seems like a hack. If we need a way to discover streams on a > source, I'd rather have an explicit operation to do so. Can't the get > frame descriptor operation be used for this ? > > If the need is to find out that we're reaching the end of a pipeline > while going through links and routes, I'd rather have a pad flag to > indicate that the pad is an endpoint. Also, I can't see where this is being used, so maybe we can just drop it :-) > > A sensor which provides multiple streams should implement get_routing > > and use the flag to mark the routes as sources. > > > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > > --- > > include/uapi/linux/v4l2-subdev.h | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h > > index 45c01799e2cd..f20491e1f53f 100644 > > --- a/include/uapi/linux/v4l2-subdev.h > > +++ b/include/uapi/linux/v4l2-subdev.h > > @@ -200,6 +200,13 @@ struct v4l2_subdev_capability { > > */ > > #define V4L2_SUBDEV_ROUTE_FL_IMMUTABLE BIT(1) > > > > +/** > > + * Is the route a source endpoint? A source endpoint route doesn't come > > + * from "anywhere", and the sink_pad and sink_stream fields are unused. > > + * Set by the driver. > > + */ > > +#define V4L2_SUBDEV_ROUTE_FL_SOURCE BIT(2) > > + > > /** > > * struct v4l2_subdev_route - A route inside a subdev > > * -- Regards, Laurent Pinchart
Hi, On 06/06/2021 01:46, Laurent Pinchart wrote: > Hi Tomi, > > On Sun, Jun 06, 2021 at 01:44:54AM +0300, Laurent Pinchart wrote: >> On Mon, May 24, 2021 at 01:44:01PM +0300, Tomi Valkeinen wrote: >>> Add route flag to indicate that the route is a source route. This means >>> that the route does not lead anywhere, and the sink_pad and sink_stream >>> should not be used. >> >> I don't like this much. It's not a route if it doesn't lead anywhere, so >> this flag seems like a hack. If we need a way to discover streams on a >> source, I'd rather have an explicit operation to do so. Can't the get >> frame descriptor operation be used for this ? If the "route" word is the problem, perhaps we can rename it to... "streams"? If the sensor has one source pad with one or two streams, the userspace needs to be able to 1) configure if we have one or two streams 2) configure the stream IDs 3) configure the stream format. This is exactly the same as when configuring routes for a bridge, the only difference is that the sensor's streams come from inside the sensor, not from an external source. Somewhat related, I was wondering if the whole routing table should be dropped. Now that the stream endpoints each have a configuration, similar to pads in current drivers, perhaps the routing information could be just part of the stream endpoints configuration. So, stream 0 in pad 0 could have a config field that has the destination pad+stream tuple. The destination should, of course, have a matching but inverse destination configuration. The pad+stream tuples would be validated in the normal subdev validation step. I haven't looked at this closely, so I'm not sure if this would help or not. >> If the need is to find out that we're reaching the end of a pipeline >> while going through links and routes, I'd rather have a pad flag to >> indicate that the pad is an endpoint. A pad could have both source streams and "routed" streams. > Also, I can't see where this is being used, so maybe we can just drop it > :-) True, I seem to have dropped the use when I dropped my metadata hacks from the test sensor driver. But I think the flag, or something similar, is needed if this information is in the routing data (or whatever we rename it to). Tomi
diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h index 45c01799e2cd..f20491e1f53f 100644 --- a/include/uapi/linux/v4l2-subdev.h +++ b/include/uapi/linux/v4l2-subdev.h @@ -200,6 +200,13 @@ struct v4l2_subdev_capability { */ #define V4L2_SUBDEV_ROUTE_FL_IMMUTABLE BIT(1) +/** + * Is the route a source endpoint? A source endpoint route doesn't come + * from "anywhere", and the sink_pad and sink_stream fields are unused. + * Set by the driver. + */ +#define V4L2_SUBDEV_ROUTE_FL_SOURCE BIT(2) + /** * struct v4l2_subdev_route - A route inside a subdev *
Add route flag to indicate that the route is a source route. This means that the route does not lead anywhere, and the sink_pad and sink_stream should not be used. A sensor which provides multiple streams should implement get_routing and use the flag to mark the routes as sources. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> --- include/uapi/linux/v4l2-subdev.h | 7 +++++++ 1 file changed, 7 insertions(+)