Moi,
On Thu, Oct 05, 2023 at 03:59:05PM +0300, Tomi Valkeinen wrote:
> On 03/10/2023 15:07, Sakari Ailus wrote:
> > Document S_ROUTING behaviour for different devices.
> >
> > Generally in devices that produce streams the streams are static and some
>
> I'm not sure what "static" means here. "Generally in devices that produce
> streams there is a fixed amount of streams..."?
Perhaps this Wordnet definition fits:
3: showing little if any change; "a static population" [syn:
{static}, {stable}, {unchanging}]
Similarly, we have static links in MC.
>
> > can be enabled and disabled, whereas in devices that just transport them
> > or write them to memory, more configurability is allowed. Document this.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > ---
> > .../userspace-api/media/v4l/dev-subdev.rst | 21 +++++++++++++++++++
> > 1 file changed, 21 insertions(+)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > index fb73a95401c3..83993775237f 100644
> > --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > @@ -593,6 +593,27 @@ Any configurations of a stream within a pad, such as format or selections,
> > are independent of similar configurations on other streams. This is
> > subject to change in the future.
> > +Device types and routing setup
> > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +Different kinds of sub-devices have differing behaviour for route inactivation,
>
> Would "activation" convey the same, but be a bit clearer?
I'd think so.
>
> > +depending on the hardware. Devices generating the streams may often allow
>
> Maybe drop the "often".
Seems fine.
>
> > +enabling and disabling some of these or the configuration is fixed. If these
>
> "some of these" -> "some of the routes".
Yes.
>
> > +routes can be disabled, not declaring these routes (or declaring them without
>
> Here also, I think "the routes" is more readable than repeating "these
> routes".
Works for me.
>
> > +``VIDIOC_SUBDEV_STREAM_FL_ACTIVE`` flag set) in ``VIDIOC_SUBDEV_S_ROUTING`` will
>
> Why is the flag sentence in parenthesis? Aren't both options of the same
> value?
It's a long sentence. This is an alternative, indeed, and I think it's
easier to read this way.
>
> > +disable the routes while the sub-device driver retain the streams and their
>
> What does this mean? That even if the user disables a route, the driver must
> keep the configuration that was set earlier related to that route?
Yes. As the routes remain, so does the sub-device state.
Would you instead prefer to reset it to (some) defaults? I think driver
implementation will need some code in that case.
>
> > +configuration. The ``VIDIOC_SUBDEV_S_ROUTING`` will still return such routes
> > +back to the user in the routes array, with the ``V4L2_SUBDEV_STREAM_FL_ACTIVE``
> > +flag unset.
>
> So a generating device should always return all its routes with both
> G_ROUTING and S_ROUTING, right? But with disabled routes not having
Correct.
> VIDIOC_SUBDEV_STREAM_FL_ACTIVE. The text doesn't mention G_ROUTING at all.
G_ROUTING is sort of trivial in this sense --- it just returns the routes
to the user, and this is documented in the IOCTL documentation.
>
> > +Devices transporting the streams almost always have more configurability with
> > +respect to routing. Typically any route between the sub-device's sink and source
> > +pads is possible, and multiple routes (usually up to certain limited number) may
> > +be active simultaneously. For such devices, no routes are created by the driver
> > +and user-created routes are fully replaced when ``VIDIOC_SUBDEV_S_ROUTING`` is
> > +called on the sub-device. Such newly created routes have the device's default
> > +configuration for format and selection rectangles.
> > +
>
> I think this paragraph is ok. But could this whole section be restructured a
> bit, as the previous paragraph gets quite confusing. Maybe:
>
> First paragraph to explain the two different kinds of devices, and perhaps a
> mention that a route is considered disabled if either it does not exist in
> the routing table or if VIDIOC_SUBDEV_STREAM_FL_ACTIVE is not set.
If there's no route, the route isn't disabled. Or did you mean the routes
array for S_ROUTING IOCTL?
>
> Then a paragraph for generating devices, and then a paragraph for
> transporting devices.
I'll see what I can do.
--
Terveisin,
Sakari Ailus
