On 27/09/2022 13:23, Sakari Ailus wrote:
<snip>
+All stream configurations are reset when ``VIDIOC_SUBDEV_S_ROUTING`` is called. This
+means that the userspace mut reconfigure all streams after calling the ioctl
+with e.g. ``VIDIOC_SUBDEV_S_FMT``.
How about this:
Calling ``VIDIOC_SUBDEV_S_ROUTING`` will cause the selections and subdev
formats being propagated from the sink pads towards the sources.
Hmm, but that's not true. The selections and formats will be zeroed, unless
the driver initializes them to a value. There's no propagation done.
They need to be propagated. The driver is responsible for maintaining a
valid configuration for the processing steps in a sub-device, and with
routes that must apply to routes as well.
Hmm, no, they don't need to be propagated. The driver needs to
initialize the formats and selections to valid configuration, that is
true, but it doesn't mean the driver propagates settings from the sink
pads to the source pads. In theory the formats on sink and source sides
could be different.
I guess you could say that the driver initializes the sink side streams
and then propagates those to the source side, but at least to me that
gives the wrong impression. What the driver does is initialize the
configuration, or reset the configuration to defaults (which it could do
with propagation if it so wants).
The framework provides v4l2_subdev_set_routing_with_fmt() helper to
achieve this, which sets the given format to all streams.
Anyway, I think we can talk about propagation if that helps, but I think
the main point there is that the settings are reset or initialized.
+
+A special case for routing are routes marked with
+``V4L2_SUBDEV_ROUTE_FL_SOURCE`` flag. These routes are used to describe
+source endpoints on sensors and the sink fields are unused.
+
+When inspecting routes through ``VIDIOC_SUBDEV_G_ROUTING`` and the application
+provided ``num_routes`` is not big enough to contain all the available routes
+the subdevice exposes, drivers return the ENOSPC error code and adjust the
+value of the ``num_routes`` field. Application should then reserve enough memory
+for all the route entries and call ``VIDIOC_SUBDEV_G_ROUTING`` again.
+
+.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{8.7cm}|
+
+.. c:type:: v4l2_subdev_routing
+
+.. flat-table:: struct v4l2_subdev_routing
+ :header-rows: 0
+ :stub-columns: 0
+ :widths: 1 1 2
+
+ * - __u32
+ - ``which``
+ - Format to modified, from enum
+ :ref:`v4l2_subdev_format_whence <v4l2-subdev-format-whence>`.
+ * - struct :c:type:`v4l2_subdev_route`
+ - ``routes[]``
+ - Array of struct :c:type:`v4l2_subdev_route` entries
+ * - __u32
+ - ``num_routes``
+ - Number of entries of the routes array
+ * - __u32
+ - ``reserved``\ [5]
+ - Reserved for future extensions. Applications and drivers must set
+ the array to zero.
+
+.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{8.7cm}|
+
+.. c:type:: v4l2_subdev_route
+
+.. flat-table:: struct v4l2_subdev_route
+ :header-rows: 0
+ :stub-columns: 0
+ :widths: 1 1 2
+
+ * - __u32
+ - ``sink_pad``
+ - Sink pad number.
+ * - __u32
+ - ``sink_stream``
+ - Sink pad stream number.
+ * - __u32
+ - ``source_pad``
+ - Source pad number.
+ * - __u32
+ - ``source_stream``
+ - Source pad stream number.
+ * - __u32
+ - ``flags``
+ - Route enable/disable flags
+ :ref:`v4l2_subdev_routing_flags <v4l2-subdev-routing-flags>`.
+ * - __u32
+ - ``reserved``\ [5]
+ - Reserved for future extensions. Applications and drivers must set
+ the array to zero.
+
+.. tabularcolumns:: |p{6.6cm}|p{2.2cm}|p{8.7cm}|
+
+.. _v4l2-subdev-routing-flags:
+
+.. flat-table:: enum v4l2_subdev_routing_flags
+ :header-rows: 0
+ :stub-columns: 0
+ :widths: 3 1 4
+
+ * - V4L2_SUBDEV_ROUTE_FL_ACTIVE
+ - 0
+ - The route is enabled. Set by applications.
+ * - V4L2_SUBDEV_ROUTE_FL_IMMUTABLE
How about calling this STATIC instead of IMMUTABLE? IMMUTABLE is used as a
link flag to mean a link that may not be changed in any way. In this case
we rather want to say that the route is always there, albeit you can still
enable or disable it.
If we think there's a need for this, I can add it back and name it static. I
think what it then should mean is that the user can enable/disable it and
also set the stream id, but the route must always exist.
But the static routes are recognised by the stream ID only, aren't they?
I think we'll definitely need a way to determine which routes are always
there and which ones can be removed at will.
That's the V4L2_SUBDEV_ROUTE_FL_SOURCE. V4L2_SUBDEV_ROUTE_FL_SOURCE
routes are always there, and the user can only enable or disable them.
This is why I dropped IMMUTABLE, as SOURCE is already immutable (or
rather, static), and I don't see a need for a non-source route to be static.
But as I said above, I haven't figured out a use for this.
+ - 1
+ - The route is immutable. Set by the driver.
+ * - V4L2_SUBDEV_ROUTE_FL_SOURCE
+ - 2
+ - The route is a source route, and the ``sink_pad`` and ``sink_stream``
+ fields are unused. Set by the driver.
+
+Return Value
+============
+
+On success 0 is returned, on error -1 and the ``errno`` variable is set
+appropriately. The generic error codes are described at the
+:ref:`Generic Error Codes <gen-errors>` chapter.
+
+ENOSPC
+ The number of provided route entries is less than the available ones.
What does "available ones" mean in this context? More than is supported?
Wouldn't E2BIG be the appropriate code in that case?
Good question. I don't think I wrote this part =). ENOSPC refers to the case
where VIDIOC_SUBDEV_G_ROUTING is called without enough space for the routing
table. So "available ones" mean the routes in the subdev's routing table,
and "provided route entries" refers to the userspace target routing table.
It sounds pretty odd, and obviously needs a clarification.
I think I actually can think what this did mean. It means that the
num_routes is smaller than the number of routes in a routing table when
G_ROUTING is called. For that I think ENOSPC is the right code actually.
But also I think we need to document what happens when there are too many
routes. For that E2BIG would be appropriate.
v4l2-ioctl.c returns EINVAL if there are over 256 routes. The drivers
should, of course, do additional check if needed. In v4l2-ioctl.c it
seems common to return EINVAL if there's too much data, but we can of
course define E2BIG for routing ioctls.
Should we have a field for telling which route was the bad one, if it was
one of them? That can be done later, too, if we'll ever need it.
Hmm maybe, although I wonder how often the drivers can say that this
particular route is the problem, and what would the userspace do with
that information...
Do you have any examples in mind?
Tomi
