Re: [PATCH v8 36/38] media: v4l: Add V4L2_SUBDEV_ROUTE_FL_IMMUTABLE sub-device routing flag

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Huomenta Tomi,

On Wed, Mar 13, 2024 at 09:34:13AM +0200, Tomi Valkeinen wrote:
> On 13/03/2024 09:25, Sakari Ailus wrote:
> > Add a flag to denote immutable routes, V4L2_SUBDEV_ROUTE_FL_IMMUTABLE.
> > Such routes cannot be changed and they're always active.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >   Documentation/userspace-api/media/v4l/dev-subdev.rst         | 3 ++-
> >   .../userspace-api/media/v4l/vidioc-subdev-g-routing.rst      | 5 +++++
> >   include/uapi/linux/v4l2-subdev.h                             | 5 +++++
> >   3 files changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > index 08495cc6f4a6..2f2423f676cf 100644
> > --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > @@ -572,7 +572,8 @@ internal pad always has a single stream only (0).
> >   Routes from an internal source pad to an external source pad are typically not
> >   modifiable but they can be activated and deactivated using the
> >   :ref:`V4L2_SUBDEV_ROUTE_FL_ACTIVE <v4l2-subdev-routing-flags>` flag, depending
> > -on driver capabilities.
> > +on driver capabilities. This capatibility is indicated by the
> > +:ref:`V4L2_SUBDEV_ROUTE_FL_IMMUTABLE <v4l2-subdev-routing-flags>` flag.
> >   Interaction between routes, streams, formats and selections
> >   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst
> > index 08b8d17cef3f..cd7735f9104e 100644
> > --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst
> > +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst
> > @@ -139,6 +139,11 @@ Also ``VIDIOC_SUBDEV_S_ROUTING`` may return more route than the user provided in
> >       * - V4L2_SUBDEV_ROUTE_FL_ACTIVE
> >         - 0x0001
> >         - The route is enabled. Set by applications.
> > +    * - V4L2_SUBDEV_ROUTE_FL_IMMUTABLE
> > +      - 0x0002
> > +      - The route is immutable. Set by the driver. The
> > +	``V4L2_SUBDEV_ROUTE_FL_ACTIVE`` flag of an immutable route may not be
> > +	changed.
> >   Return Value
> >   ============
> > diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
> > index ca543982460c..7e501cb45e4e 100644
> > --- a/include/uapi/linux/v4l2-subdev.h
> > +++ b/include/uapi/linux/v4l2-subdev.h
> > @@ -200,6 +200,11 @@ struct v4l2_subdev_capability {
> >    * on a video node.
> >    */
> >   #define V4L2_SUBDEV_ROUTE_FL_ACTIVE		(1U << 0)
> > +/*
> > + * Is the route immutable. The ACTIVE flag of an immutable route may not be
> > + * changed.
> > + */
> > +#define V4L2_SUBDEV_ROUTE_FL_IMMUTABLE		(1U << 1)
> >   /**
> >    * struct v4l2_subdev_route - A route inside a subdev
> 
> Is the route fully immutable? The sink/source stream ID cannot be changed
> (or any new fields we might come up with in the future)?

I think the new fields should be considered separately when they're added.
This also applies to the stream IDs, I'll add this to the documentation.

The naming of the flag is aligned with MC link flag with a similar purpose.

> 
> Hmm, or would a route with different stream IDs be a, well, different
> route...
> 
> The docs here only talk about the ACTIVE flag. Would
> V4L2_SUBDEV_ROUTE_FL_ALWAYS_ACTIVE be a better name, to be more explicit on
> the meaning?

I prefer immutable. I wonder what Laurent and Hans think.

-- 
Terveisin,

Sakari Ailus