| Message ID | 20240313072516.241106-37-sakari.ailus@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v8,01/38] media: mc: Add INTERNAL pad flag | expand |
Hello, On Wed, Mar 13, 2024 at 07:39:17AM +0000, Sakari Ailus wrote: > 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. That's not very clear, it sounds like V4L2_SUBDEV_ROUTE_FL_IMMUTABLE indicates that the route can be enabled/disabled. I'd rewrite this. > > > 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. May not be changed and will always be set ? > > > 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. I'm fine with either. IMMUTABLE is shorter, if that makes a difference.
Hi Laurent, On Thu, Mar 21, 2024 at 07:03:08PM +0200, Laurent Pinchart wrote: > Hello, > > On Wed, Mar 13, 2024 at 07:39:17AM +0000, Sakari Ailus wrote: > > 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. > > That's not very clear, it sounds like V4L2_SUBDEV_ROUTE_FL_IMMUTABLE > indicates that the route can be enabled/disabled. I'd rewrite this. I'll use instead: The :ref:`V4L2_SUBDEV_ROUTE_FL_IMMUTABLE <v4l2-subdev-routing-flags>` flag indicates that the ``V4L2_SUBDEV_ROUTE_FLAG_ACTIVE`` of the route may not be unset. > > > > > 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. > > May not be changed and will always be set ? How about "may not be unset"? > > > > > 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. > > I'm fine with either. IMMUTABLE is shorter, if that makes a difference. >
Hi Sakari, On Tue, Apr 09, 2024 at 01:21:16PM +0000, Sakari Ailus wrote: > On Thu, Mar 21, 2024 at 07:03:08PM +0200, Laurent Pinchart wrote: > > On Wed, Mar 13, 2024 at 07:39:17AM +0000, Sakari Ailus wrote: > > > 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. > > > > That's not very clear, it sounds like V4L2_SUBDEV_ROUTE_FL_IMMUTABLE > > indicates that the route can be enabled/disabled. I'd rewrite this. > > I'll use instead: > > The :ref:`V4L2_SUBDEV_ROUTE_FL_IMMUTABLE <v4l2-subdev-routing-flags>` flag > indicates that the ``V4L2_SUBDEV_ROUTE_FLAG_ACTIVE`` of the route may not > be unset. Sounds good to me. > > > > > 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. > > > > May not be changed and will always be set ? > > How about "may not be unset"? Even better :-) > > > > > 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. > > > > I'm fine with either. IMMUTABLE is shorter, if that makes a difference.
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
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(-)