Add units to controls

The V4L2 control flags are a 32-bit bitmask. Use 32-bit hexadecimal numbers to specify the flags (was 16).

Signed-off-by Sakari Ailus <[email protected]>

---
 Documentation/media/uapi/v4l/vidioc-queryctrl.rst | 22 ++++++++++----------
 include/uapi/linux/videodev2.h                    | 22 ++++++++++----------
 2 files changed, 22 insertions(+), 22 deletions(-)

(quoted diff) +0 -0

b/Documentation/media/uapi/v4l/vidioc-queryctrl.rst index 5bd26e8c9a1a0..ff2d131223b84 100644

Documentation/media/uapi/v4l/vidioc-queryctrl.rst +7 -7

@@ -439,37 +439,37 @@ See also the examples in :ref:`control`.
 
     :widths:       3 1 4
 
     * - ``V4L2_CTRL_FLAG_DISABLED``
-      - 0x0001
+      - 0x00000001
       - This control is permanently disabled and should be ignored by the
 	application. Any attempt to change the control will result in an
 	``EINVAL`` error code.
     * - ``V4L2_CTRL_FLAG_GRABBED``
-      - 0x0002
+      - 0x00000002
       - This control is temporarily unchangeable, for example because
 	another application took over control of the respective resource.
 	Such controls may be displayed specially in a user interface.
 	Attempts to change the control may result in an ``EBUSY`` error code.
     * - ``V4L2_CTRL_FLAG_READ_ONLY``
-      - 0x0004
+      - 0x00000004
       - This control is permanently readable only. Any attempt to change
 	the control will result in an ``EINVAL`` error code.
     * - ``V4L2_CTRL_FLAG_UPDATE``
-      - 0x0008
+      - 0x00000008
       - A hint that changing this control may affect the value of other
 	controls within the same control class. Applications should update
 	their user interface accordingly.
     * - ``V4L2_CTRL_FLAG_INACTIVE``
-      - 0x0010
+      - 0x00000010
       - This control is not applicable to the current configuration and
 	should be displayed accordingly in a user interface. For example
 	the flag may be set on a MPEG audio level 2 bitrate control when
 	MPEG audio encoding level 1 was selected with another control.
     * - ``V4L2_CTRL_FLAG_SLIDER``
-      - 0x0020
+      - 0x00000020
       - A hint that this control is best represented as a slider-like
 	element in a user interface.
     * - ``V4L2_CTRL_FLAG_WRITE_ONLY``
-      - 0x0040
+      - 0x00000040
       - This control is permanently writable only. Any attempt to read the
 	control will result in an ``EACCES`` error code error code. This flag

Documentation/media/uapi/v4l/vidioc-queryctrl.rst +2 -2

 	is typically present for relative controls or action controls

@@ -477,7 +477,7 @@ See also the examples in :ref:`control`.
 	action (e. g. motor control) but no meaningful value can be
 	returned.
     * - ``V4L2_CTRL_FLAG_VOLATILE``
-      - 0x0080
+      - 0x00000080
       - This control is volatile, which means that the value of the
 	control changes continuously. A typical example would be the
 	current gain value if the device is in auto-gain mode. In such a

@@ -493,7 +493,7 @@ See also the examples in :ref:`control`.
 	   Setting a new value for a volatile control will *never* trigger a
 
 	   :ref:`V4L2_EVENT_CTRL_CH_VALUE <ctrl-changes-flags>` event.
 
     * - ``V4L2_CTRL_FLAG_HAS_PAYLOAD``
-      - 0x0100
+      - 0x00000100
       - This control has a pointer type, so its value has to be accessed

Documentation/media/uapi/v4l/vidioc-queryctrl.rst +2 -2

 	using one of the pointer fields of struct
 
 	:c:type:`v4l2_ext_control`. This flag is set

@@ -503,7 +503,7 @@ See also the examples in :ref:`control`.
     * .. _FLAG_EXECUTE_ON_WRITE:
 
       - ``V4L2_CTRL_FLAG_EXECUTE_ON_WRITE``
-      - 0x0200
+      - 0x00000200
       - The value provided to the control will be propagated to the driver
 	even if it remains constant. This is required when the control
 	represents an action on the hardware. For example: clearing an

@@ -512,7 +512,7 @@ See also the examples in :ref:`control`.
     * .. _FLAG_MODIFY_LAYOUT:
 
       - ``V4L2_CTRL_FLAG_MODIFY_LAYOUT``
-      - 0x0400
+      - 0x00000400
       - Changing this control value may modify the layout of the
         buffer (for video devices) or the media bus format (for

sub-devices).

include/uapi/linux/videodev2.h +11 -11

index 184e4dbe8f9c0..ae083978988f1 100644
@@ -1681,17 +1681,17 @@ struct v4l2_querymenu {
 } __attribute__ ((packed));
 
 /*  Control flags  */
-#define V4L2_CTRL_FLAG_DISABLED		0x0001
-#define V4L2_CTRL_FLAG_GRABBED		0x0002
-#define V4L2_CTRL_FLAG_READ_ONLY	0x0004
-#define V4L2_CTRL_FLAG_UPDATE		0x0008
-#define V4L2_CTRL_FLAG_INACTIVE		0x0010
-#define V4L2_CTRL_FLAG_SLIDER		0x0020
-#define V4L2_CTRL_FLAG_WRITE_ONLY	0x0040
-#define V4L2_CTRL_FLAG_VOLATILE		0x0080
-#define V4L2_CTRL_FLAG_HAS_PAYLOAD	0x0100
-#define V4L2_CTRL_FLAG_EXECUTE_ON_WRITE	0x0200
-#define V4L2_CTRL_FLAG_MODIFY_LAYOUT	0x0400
+#define V4L2_CTRL_FLAG_DISABLED		0x00000001
+#define V4L2_CTRL_FLAG_GRABBED		0x00000002
+#define V4L2_CTRL_FLAG_READ_ONLY	0x00000004
+#define V4L2_CTRL_FLAG_UPDATE		0x00000008
+#define V4L2_CTRL_FLAG_INACTIVE		0x00000010
+#define V4L2_CTRL_FLAG_SLIDER		0x00000020
+#define V4L2_CTRL_FLAG_WRITE_ONLY	0x00000040
+#define V4L2_CTRL_FLAG_VOLATILE		0x00000080
+#define V4L2_CTRL_FLAG_HAS_PAYLOAD	0x00000100
+#define V4L2_CTRL_FLAG_EXECUTE_ON_WRITE	0x00000200
+#define V4L2_CTRL_FLAG_MODIFY_LAYOUT	0x00000400
 
 /*  Query flags, to be ORed with the control ID */
 #define V4L2_CTRL_FLAG_NEXT_CTRL	0x80000000

The V4L2 control flags are a 32-bit bitmask. Use 32-bit hexadecimal numbers to specify the flags (was 16).

Signed-off-by Sakari Ailus <[email protected]>

---
 Documentation/media/uapi/v4l/vidioc-queryctrl.rst | 22 +++++++++++-----------
 include/uapi/linux/videodev2.h                    | 22 +++++++++++-----------
 2 files changed, 22 insertions(+), 22 deletions(-)

Documentation/media/uapi/v4l/vidioc-queryctrl.rst +11 -11

index 5bd26e8c9a1a0..ff2d131223b84 100644
@@ -439,37 +439,37 @@ See also the examples in :ref:`control`.
     :widths:       3 1 4
 
     * - ``V4L2_CTRL_FLAG_DISABLED``
-      - 0x0001
+      - 0x00000001
       - This control is permanently disabled and should be ignored by the
 	application. Any attempt to change the control will result in an
 	``EINVAL`` error code.
     * - ``V4L2_CTRL_FLAG_GRABBED``
-      - 0x0002
+      - 0x00000002
       - This control is temporarily unchangeable, for example because
 	another application took over control of the respective resource.
 	Such controls may be displayed specially in a user interface.
 	Attempts to change the control may result in an ``EBUSY`` error code.
     * - ``V4L2_CTRL_FLAG_READ_ONLY``
-      - 0x0004
+      - 0x00000004
       - This control is permanently readable only. Any attempt to change
 	the control will result in an ``EINVAL`` error code.
     * - ``V4L2_CTRL_FLAG_UPDATE``
-      - 0x0008
+      - 0x00000008
       - A hint that changing this control may affect the value of other
 	controls within the same control class. Applications should update
 	their user interface accordingly.
     * - ``V4L2_CTRL_FLAG_INACTIVE``
-      - 0x0010
+      - 0x00000010
       - This control is not applicable to the current configuration and
 	should be displayed accordingly in a user interface. For example
 	the flag may be set on a MPEG audio level 2 bitrate control when
 	MPEG audio encoding level 1 was selected with another control.
     * - ``V4L2_CTRL_FLAG_SLIDER``
-      - 0x0020
+      - 0x00000020
       - A hint that this control is best represented as a slider-like
 	element in a user interface.
     * - ``V4L2_CTRL_FLAG_WRITE_ONLY``
-      - 0x0040
+      - 0x00000040
       - This control is permanently writable only. Any attempt to read the
 	control will result in an ``EACCES`` error code error code. This flag
 	is typically present for relative controls or action controls

@@ -477,7 +477,7 @@ See also the examples in :ref:`control`.
 	action (e. g. motor control) but no meaningful value can be
 	returned.
     * - ``V4L2_CTRL_FLAG_VOLATILE``
-      - 0x0080
+      - 0x00000080
       - This control is volatile, which means that the value of the
 	control changes continuously. A typical example would be the
 	current gain value if the device is in auto-gain mode. In such a

@@ -493,7 +493,7 @@ See also the examples in :ref:`control`.
 	   Setting a new value for a volatile control will *never* trigger a
 	   :ref:`V4L2_EVENT_CTRL_CH_VALUE <ctrl-changes-flags>` event.
     * - ``V4L2_CTRL_FLAG_HAS_PAYLOAD``
-      - 0x0100
+      - 0x00000100
       - This control has a pointer type, so its value has to be accessed
 	using one of the pointer fields of struct
 	:c:type:`v4l2_ext_control`. This flag is set

@@ -503,7 +503,7 @@ See also the examples in :ref:`control`.
     * .. _FLAG_EXECUTE_ON_WRITE:
 
       - ``V4L2_CTRL_FLAG_EXECUTE_ON_WRITE``
-      - 0x0200
+      - 0x00000200
       - The value provided to the control will be propagated to the driver
 	even if it remains constant. This is required when the control
 	represents an action on the hardware. For example: clearing an

@@ -512,7 +512,7 @@ See also the examples in :ref:`control`.
     * .. _FLAG_MODIFY_LAYOUT:
 
       - ``V4L2_CTRL_FLAG_MODIFY_LAYOUT``
-      - 0x0400
+      - 0x00000400
       - Changing this control value may modify the layout of the
         buffer (for video devices) or the media bus format (for sub-devices).
 

include/uapi/linux/videodev2.h +11 -11

index 184e4dbe8f9c0..ae083978988f1 100644
@@ -1681,17 +1681,17 @@ struct v4l2_querymenu {
 } __attribute__ ((packed));
 
 /*  Control flags  */
-#define V4L2_CTRL_FLAG_DISABLED		0x0001
-#define V4L2_CTRL_FLAG_GRABBED		0x0002
-#define V4L2_CTRL_FLAG_READ_ONLY	0x0004
-#define V4L2_CTRL_FLAG_UPDATE		0x0008
-#define V4L2_CTRL_FLAG_INACTIVE		0x0010
-#define V4L2_CTRL_FLAG_SLIDER		0x0020
-#define V4L2_CTRL_FLAG_WRITE_ONLY	0x0040
-#define V4L2_CTRL_FLAG_VOLATILE		0x0080
-#define V4L2_CTRL_FLAG_HAS_PAYLOAD	0x0100
-#define V4L2_CTRL_FLAG_EXECUTE_ON_WRITE	0x0200
-#define V4L2_CTRL_FLAG_MODIFY_LAYOUT	0x0400
+#define V4L2_CTRL_FLAG_DISABLED		0x00000001
+#define V4L2_CTRL_FLAG_GRABBED		0x00000002
+#define V4L2_CTRL_FLAG_READ_ONLY	0x00000004
+#define V4L2_CTRL_FLAG_UPDATE		0x00000008
+#define V4L2_CTRL_FLAG_INACTIVE		0x00000010
+#define V4L2_CTRL_FLAG_SLIDER		0x00000020
+#define V4L2_CTRL_FLAG_WRITE_ONLY	0x00000040
+#define V4L2_CTRL_FLAG_VOLATILE		0x00000080
+#define V4L2_CTRL_FLAG_HAS_PAYLOAD	0x00000100
+#define V4L2_CTRL_FLAG_EXECUTE_ON_WRITE	0x00000200
+#define V4L2_CTRL_FLAG_MODIFY_LAYOUT	0x00000400
 
 /*  Query flags, to be ORed with the control ID */
 #define V4L2_CTRL_FLAG_NEXT_CTRL	0x80000000

Add support for exponential bases, prefixes as well as units for V4L2 controls. This makes it possible to convey information on the relation between the control value and the hardware feature being controlled.

Signed-off-by Sakari Ailus <[email protected]>

---
 include/uapi/linux/videodev2.h | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

include/uapi/linux/videodev2.h +0 -0

index ae083978988f1..23b02f2db85a1 100644

include/uapi/linux/videodev2.h +19 -1

+#define V4L2_CTRL_PREFIX_KILO		3
+#define V4L2_CTRL_PREFIX_MEGA		6
+#define V4L2_CTRL_PREFIX_GIGA		9
+
+/* V4L2 control units */
+#define V4L2_CTRL_UNIT_UNDEFINED	0
+#define V4L2_CTRL_UNIT_NONE		1
+#define V4L2_CTRL_UNIT_SECOND		2
+#define V4L2_CTRL_UNIT_AMPERE		3
+#define V4L2_CTRL_UNIT_LINE		4
+#define V4L2_CTRL_UNIT_PIXEL		5
+#define V4L2_CTRL_UNIT_PIXELS_PER_SEC	6
+#define V4L2_CTRL_UNIT_HZ		7
+
+
 /*  Used in the VIDIOC_QUERY_EXT_CTRL ioctl for querying extended controls */
 struct v4l2_query_ext_ctrl {
 	__u32		     id;

@@ -1666,7 +1692,10 @@ struct v4l2_query_ext_ctrl {
 	__u32                elems;
 	__u32                nr_of_dims;
 	__u32                dims[V4L2_CTRL_MAX_DIMS];
-	__u32		     reserved[32];
+	__u8		     base;
+	__s8		     prefix;
+	__u16		     unit;
+	__u32		     reserved[31];
 };
 
 /*  Used in the VIDIOC_QUERYMENU ioctl for querying menu items */

include/uapi/linux/videodev2.h struct v4l2_queryctrl { · line 1665

Quoted 5 lines · Sakari Ailus

include/uapi/linux/videodev2.h +5 -0

+/* V4L2 control unit prefixes */
+#define V4L2_CTRL_PREFIX_NANO		-9
+#define V4L2_CTRL_PREFIX_MICRO		-6
+#define V4L2_CTRL_PREFIX_MILLI		-3
+#define V4L2_CTRL_PREFIX_1		0

I would prefer PREFIX_NONE, since there is no prefix in this case.

I assume this prefix is only valid if the unit is not UNDEFINED and not NONE?

Is 'base' also dependent on a valid unit? (it doesn't appear to be)

Quoted 9 lines · Hans Verkuil

include/uapi/linux/videodev2.h struct v4l2_queryctrl { · line 1665

Quoted 5 lines · Sakari Ailus

include/uapi/linux/videodev2.h +5 -0

+/* V4L2 control unit prefixes */
+#define V4L2_CTRL_PREFIX_NANO		-9
+#define V4L2_CTRL_PREFIX_MICRO		-6
+#define V4L2_CTRL_PREFIX_MILLI		-3
+#define V4L2_CTRL_PREFIX_1		0

I would prefer PREFIX_NONE, since there is no prefix in this case.

Quoted 2 lines

I assume this prefix is only valid if the unit is not UNDEFINED and not NONE?

Why should it? The prefix is concerned with rescaling a value prior to presenting it to a user. Even a unitless quantity or a value of undefined unit can be reasonably scaled. Displaying a unit and scaling look like orthogonal concepts to me.

Quoted 2 lines

Is 'base' also dependent on a valid unit? (it doesn't appear to be)

I'd argue it should not depend on a valid unit like the prefix.

Quoted 4 lines · Sakari Ailus

Add support for exponential bases, prefixes as well as units for V4L2 controls. This makes it possible to convey information on the relation between the control value and the hardware feature being controlled.

Quoted 10 lines

Signed-off-by Sakari Ailus <[email protected]>

---
 include/uapi/linux/videodev2.h | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

include/uapi/linux/videodev2.h +0 -0

index ae083978988f1..23b02f2db85a1 100644

include/uapi/linux/videodev2.h struct v4l2_queryctrl { · line 1657

Quoted 6 lines

include/uapi/linux/videodev2.h +3 -0

@@ -1652,6 +1652,32 @@ struct v4l2_queryctrl {
      __u32                reserved[2];
 };
 
+/* V4L2 control exponential bases */
+#define V4L2_CTRL_BASE_UNDEFINED     0
+#define V4L2_CTRL_BASE_LINEAR                1

I'm not really sure you need BASE_LINEAR. That is effectively the same as UNDEFINED since what else can you do? It's also weird to have this as 'base' if the EXPONENTIAL flag is set.

I don't see why you need the EXPONENTIAL flag at all: if this is non-0, then you know the exponential base.

include/uapi/linux/videodev2.h struct v4l2_queryctrl { · line 1665

Quoted 9 lines

include/uapi/linux/videodev2.h +8 -0

+#define V4L2_CTRL_BASE_2             2
+#define V4L2_CTRL_BASE_10            10
+
+/* V4L2 control unit prefixes */
+#define V4L2_CTRL_PREFIX_NANO                -9
+#define V4L2_CTRL_PREFIX_MICRO               -6
+#define V4L2_CTRL_PREFIX_MILLI               -3
+#define V4L2_CTRL_PREFIX_1           0

I would prefer PREFIX_NONE, since there is no prefix in this case.

I assume this prefix is only valid if the unit is not UNDEFINED and not NONE?

Is 'base' also dependent on a valid unit? (it doesn't appear to be)

include/uapi/linux/videodev2.h struct v4l2_queryctrl { · line 1672

Quoted 7 lines

include/uapi/linux/videodev2.h +7 -0

+#define V4L2_CTRL_PREFIX_KILO                3
+#define V4L2_CTRL_PREFIX_MEGA                6
+#define V4L2_CTRL_PREFIX_GIGA                9
+
+/* V4L2 control units */
+#define V4L2_CTRL_UNIT_UNDEFINED     0
+#define V4L2_CTRL_UNIT_NONE          1

Quoted 10 lines

include/uapi/linux/videodev2.h +8 -0

+#define V4L2_CTRL_UNIT_SECOND                2
+#define V4L2_CTRL_UNIT_AMPERE                3
+#define V4L2_CTRL_UNIT_LINE          4
+#define V4L2_CTRL_UNIT_PIXEL         5
+#define V4L2_CTRL_UNIT_PIXELS_PER_SEC        6
+#define V4L2_CTRL_UNIT_HZ            7
+
+
 /*  Used in the VIDIOC_QUERY_EXT_CTRL ioctl for querying extended controls */
 struct v4l2_query_ext_ctrl {

include/uapi/linux/videodev2.h struct v4l2_query_ext_ctrl { · line 1696

Quoted 8 lines

include/uapi/linux/videodev2.h +2 -1

      __u32                id;

@@ -1666,7 +1692,10 @@ struct v4l2_query_ext_ctrl {
      __u32                elems;
      __u32                nr_of_dims;
      __u32                dims[V4L2_CTRL_MAX_DIMS];
-     __u32                reserved[32];
+     __u8                 base;
+     __s8                 prefix;

Add support for exponential bases, prefixes as well as units for V4L2 controls. This makes it possible to convey information on the relation between the control value and the hardware feature being controlled.

Signed-off-by Sakari Ailus <[email protected]>

Documentation/media/uapi/v4l/vidioc-queryctrl.rst See also the examples in :ref:`control`. · line 670

Quoted 8 lines · Sakari Ailus

Documentation/media/uapi/v4l/vidioc-queryctrl.rst +8 -0

+    * - ``V4L2_CTRL_UNIT_PIXEL``
+      - 5
+      - A pixel in sensor's pixel matrix. This is a unit of time commonly used
+        by camera sensors in e.g. exposure control, i.e. the time it takes for
+	a sensor to read a pixel from the sensor's pixel matrix.
+
+    * - ``V4L2_CTRL_UNIT_PIXEL``
+      - 6

The latter V4L2_CTRL_UNIT_PIXEL looks like a typo that should say V4L2_CTRL_UNIT_PIXELS_PER_SEC.

Document V4L2 control exponential bases, units and prefixes, as well as the control flag telling a control value is an exponent.

Signed-off-by Sakari Ailus <[email protected]>

---
 Documentation/media/uapi/v4l/extended-controls.rst |   2 +
 Documentation/media/uapi/v4l/vidioc-queryctrl.rst  | 152 ++++++++++++++++++++-
 Documentation/media/videodev2.h.rst.exceptions     |  22 +++
 3 files changed, 175 insertions(+), 1 deletion(-)

Documentation/media/uapi/v4l/extended-controls.rst +2 -0

index 9f7312bf33651..8461fd92d1b9e 100644
@@ -3460,6 +3460,8 @@ Image Process Control IDs
     by selecting the desired horizontal and vertical blanking. The unit
     of this control is Hz.
 
+.. _v4l2_cid_pixel_rate:
+
 ``V4L2_CID_PIXEL_RATE (64-bit integer)``
     Pixel rate in the source pads of the subdev. This control is
     read-only and its unit is pixels / second.

Documentation/media/uapi/v4l/vidioc-queryctrl.rst +15 -1

index ff2d131223b84..472378f5d7566 100644
@@ -269,8 +269,22 @@ See also the examples in :ref:`control`.
       - ``dims[V4L2_CTRL_MAX_DIMS]``
       - The size of each dimension. The first ``nr_of_dims`` elements of
 	this array must be non-zero, all remaining elements must be zero.
+    * - __u8
+      - ``base``
+      - The exponential base of the control value. Valid only if the
+	:ref:`V4L2_CTRL_FLAG_EXPONENTIAL <FLAG_EXPONENTIAL>` control flag is
+	set. This is an enumeration.
+    * - __u8
+      - ``prefix``
+      - Prefix of the unit. This is an enumeration.
+    * - __u16
+      - ``unit``
+      - Unit of the value. Together with the ``prefix`` as well as the ``base``
+	field (if :ref:`V4L2_CTRL_FLAG_EXPONENTIAL <FLAG_EXPONENTIAL>` is set),
+	defines the relation between the control value and the property of the
+	hardware being controlled. This is an enumeration.
     * - __u32
-      - ``reserved``\ [32]
+      - ``reserved``\ [31]
       - Reserved for future extensions. Applications and drivers must set
 	the array to zero.
 

Documentation/media/uapi/v4l/vidioc-queryctrl.rst +103 -0

+
+
+.. tabularcolumns:: |p{6.6cm}|p{2.2cm}|p{8.7cm}|
+
+.. _control-bases:
+
+.. cssclass:: longtable
+
+.. flat-table:: Control Exponential Bases
+    :header-rows:  1
+    :stub-columns: 0
+    :widths:       3 1 4
+
+    * - Base Name
+      - Value
+      - Description
+
+    * - ``V4L2_CTRL_BASE_UNDEFINED``
+      - 0
+      - The control exponential base is not defined.
+
+    * - ``V4L2_CTRL_BASE_LINEAR``
+      - 1
+      - The control is linear.
+
+    * - ``V4L2_CTRL_BASE_2``
+      - 2
+      - The base of the control is 2.
+
+    * - ``V4L2_CTRL_BASE_10``
+      - 10
+      - The base of the control is 10.
+
+.. tabularcolumns:: |p{6.6cm}|p{2.2cm}|p{8.7cm}|
+
+.. _control-prefixes:
+
+.. cssclass:: longtable
+
+.. flat-table:: Control Prefixes
+    :header-rows:  1
+    :stub-columns: 0
+    :widths:       3 1 4
+
+    * - Prefix Name
+      - Value
+      - Description
+
+    * - ``V4L2_CTRL_PREFIX_NANO``
+      - -9
+      - Nano
+
+    * - ``V4L2_CTRL_PREFIX_MICRO``
+      - -6
+      - Micro
+
+    * - ``V4L2_CTRL_PREFIX_MILLI``
+      - -3
+      - Milli
+
+    * - ``V4L2_CTRL_PREFIX_1``
+      - 0
+      - \-
+
+    * - ``V4L2_CTRL_PREFIX_KILO``
+      - 3
+      - Kilo
+
+    * - ``V4L2_CTRL_PREFIX_MEGA``
+      - 6
+      - Mega
+
+    * - ``V4L2_CTRL_PREFIX_GIGA``
+      - 9
+      - Giga
+
+.. tabularcolumns:: |p{6.6cm}|p{2.2cm}|p{8.7cm}|
+
+.. _control-units:
+
+.. cssclass:: longtable
+
+.. flat-table:: Control Units
+    :header-rows:  1
+    :stub-columns: 0
+    :widths:       3 1 4
+
+    * - Unit Name
+      - Value
+      - Description
+
+    * - ``V4L2_CTRL_UNIT_UNDEFINED``
+      - 0
+      - The unit of the control is not defined.
+
+    * - ``V4L2_CTRL_UNIT_NONE``
+      - 1
+      - The control has no unit.
+
+    * - ``V4L2_CTRL_UNIT_SECOND``
+      - 2
+      - Second
+

Documentation/media/uapi/v4l/vidioc-queryctrl.rst +6 -0

+      - 6
+      - Pixels per second
+
+    * - ``V4L2_CTRL_UNIT_HZ
+      - 7
+      - Hertz
 
 Return Value
 ============

Documentation/media/videodev2.h.rst.exceptions +11 -0

index 63fa131729c09..8183a4f3554b0 100644
@@ -343,11 +343,33 @@ replace define V4L2_CTRL_FLAG_VOLATILE control-flags
 replace define V4L2_CTRL_FLAG_HAS_PAYLOAD control-flags
 replace define V4L2_CTRL_FLAG_EXECUTE_ON_WRITE control-flags
 replace define V4L2_CTRL_FLAG_MODIFY_LAYOUT control-flags
+replace define V4L2_CTRL_FLAG_EXPONENTIAL control-flags
 
 replace define V4L2_CTRL_FLAG_NEXT_CTRL control
 replace define V4L2_CTRL_FLAG_NEXT_COMPOUND control
 replace define V4L2_CID_PRIVATE_BASE control
 
+# V4L2 control bases, prefixes and units
+replace define V4L2_CTRL_BASE_UNDEFINED control-bases
+replace define V4L2_CTRL_BASE_LINEAR control-bases
+replace define V4L2_CTRL_BASE_2 control-bases
+replace define V4L2_CTRL_BASE_10 control-bases
+
+replace define V4L2_CTRL_PREFIX_NANO control-prefixes
+replace define V4L2_CTRL_PREFIX_MICRO control-prefixes
+replace define V4L2_CTRL_PREFIX_MILLI control-prefixes
+replace define V4L2_CTRL_PREFIX_1 control-prefixes

Add support for conveying the information set by the driver to the user space.

Signed-off-by Sakari Ailus <[email protected]>

---
 drivers/media/v4l2-core/v4l2-ctrls.c | 3 +++
 include/media/v4l2-ctrls.h           | 2 ++
 2 files changed, 5 insertions(+)

drivers/media/v4l2-core/v4l2-ctrls.c +3 -0

index ee006d34c19f0..8d2931b0a4701 100644
@@ -2732,6 +2732,9 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_query_ext_ctr
 	qc->minimum = ctrl->minimum;
 	qc->maximum = ctrl->maximum;
 	qc->default_value = ctrl->default_value;
+	qc->base = ctrl->base;
+	qc->prefix = ctrl->prefix;
+	qc->unit = ctrl->unit;
 	if (ctrl->type == V4L2_CTRL_TYPE_MENU
 	    || ctrl->type == V4L2_CTRL_TYPE_INTEGER_MENU)
 		qc->step = 1;

include/media/v4l2-ctrls.h +0 -0

index f615ba1b29dd9..d6aaf45b70381 100644

Assign units for the controls exposed by the smiapp driver.

Signed-off-by Sakari Ailus <[email protected]>

---
 drivers/media/i2c/smiapp/smiapp-core.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

drivers/media/i2c/smiapp/smiapp-core.c +0 -7

index 99f3b295ae3c7..289313c232430 100644
@@ -562,17 +562,10 @@ static int smiapp_init_controls(struct smiapp_sensor *sensor)
 	sensor->vblank = v4l2_ctrl_new_std(
 		&sensor->pixel_array->ctrl_handler, &smiapp_ctrl_ops,
 		V4L2_CID_VBLANK, 0, 1, 1, 0);
-
-	if (sensor->vblank)
-		sensor->vblank->flags |= V4L2_CTRL_FLAG_UPDATE;
-
 	sensor->hblank = v4l2_ctrl_new_std(
 		&sensor->pixel_array->ctrl_handler, &smiapp_ctrl_ops,
 		V4L2_CID_HBLANK, 0, 1, 1, 0);
 
-	if (sensor->hblank)
-		sensor->hblank->flags |= V4L2_CTRL_FLAG_UPDATE;
-
 	sensor->pixel_rate_parray = v4l2_ctrl_new_std(
 		&sensor->pixel_array->ctrl_handler, &smiapp_ctrl_ops,
 		V4L2_CID_PIXEL_RATE, 1, INT_MAX, 1, 1);

drivers/media/i2c/smiapp/smiapp-core.c +4 -0

+	sensor->hblank->flags |= V4L2_CTRL_FLAG_UPDATE;
+	sensor->hblank->unit = V4L2_CTRL_UNIT_PIXEL;
+	sensor->pixel_rate_parray->unit = V4L2_CTRL_UNIT_PIXELS_PER_SEC;
+
 	sensor->pixel_array->sd.ctrl_handler =
 		&sensor->pixel_array->ctrl_handler;
 

This set adds a few things to the current control framework in terms of what kind of information the user space may have on controls. It adds support for units and prefixes, exponential base as well as information on whether a control is linear or exponential, to struct v4l2_query_ext_ctrl.

The smiapp driver gains support for the feature. In the near term, some controls could also be assigned the unit automatically. The pixel rate, for instance. Fewer driver changes would be needed this way. A driver could override the value if there's a need to.

I think I'll merge the undefined and no unit cases. Same for the exponential base actually --- the flag can be removed, too...

Regarding Ricardo's suggestion --- I was thinking of adding a control flag (yes, there are a few bits available) to tell how to round the value. The user could use the TRY_EXT_CTRLS IOCTL to figure out the next (or previous) control value by incrementing the current value and setting the appropriate flag. This is out of the scope of this set though.

Quoted 4 lines · Sakari Ailus

This set adds a few things to the current control framework in terms of what kind of information the user space may have on controls. It adds support for units and prefixes, exponential base as well as information on whether a control is linear or exponential, to struct v4l2_query_ext_ctrl.

That's a great improvement. Being able to give meaning to controls is great. However, I see two significant weaknesses in the approach being taken:

1. There are a number of controls whose value is not easily described as either linear or exponential. I'm faced with at least two controls that actually are floating point numbers. One with two bits for the exponent and one (strange) bit for the mantissa (no joke) and another with three bits for the exponent and four bits for the mantissa. Neither can suitably be represented.

Since the value ranges are small for the cases I mentioned, I looked into using an integer menu. The present approach does not allow for replacing an integer with an integer menu though. Each control id has a fixed type. I'm not sure how to solve this.

2. The present implementation places the responsibility of assigning units and scales into drivers. A number of controls (e.g. V4L2_CID_EXPOSURE_ABSOLUTE, V4L2_CID_AUDIO_LIMITER_RELEASE_TIME, V4L2_CID_PIXEL_RATE, ...) clearly state the scale and unit in the documentation. Having each and every driver set units and scales in the documented way will lead to duplication and buggy code. Having each driver choose unit and scale will lead to inconsistency. It would be very good to have a mechanism that puts the framework in charge of maintaining units and scales for the standard control ids.

What is a consumer supposed to do with a control that changes unit? The algorithm expected e.g. a duration. It might be able to convert to pixels, but what should it do if it gets back amperes? I argue that the unit should be a property of the control id and be documented (i.e. what is done now). If it is reported via an ioctl,

the framework needs to be in charge of setting it.

The story is much different for scales. Scaling the value up and down is something consumers can reasonably be expected to do. It allows shifting the value range such that the relevant values can be fully represented. Allowing drivers to change scales is much more reasonable. Still the framework should provide a default such that most drivers should not need any update.

I acknowledge that these expectations are high and see that they're partially covered in your later remarks.

Quoted 4 lines

The smiapp driver gains support for the feature. In the near term, some controls could also be assigned the unit automatically. The pixel rate, for instance. Fewer driver changes would be needed this way. A driver could override the value if there's a need to.

I believe that in the interest of keeping maintenance cost low, this should happen rather sooner than later. Just even adding the support to smiapp seems wrong when it would be possible to have the framework do the work.

Quoted 2 lines

I think I'll merge the undefined and no unit cases. Same for the exponential base actually --- the flag can be removed, too...

I'm not sure I understand. It reads like you are going to revert a significant part of the patch.

Quoted 6 lines

Regarding Ricardo's suggestion --- I was thinking of adding a control flag (yes, there are a few bits available) to tell how to round the value. The user could use the TRY_EXT_CTRLS IOCTL to figure out the next (or previous) control value by incrementing the current value and setting the appropriate flag. This is out of the scope of this set though.

This approach sounds really useful to me. Having control over the rounding would allow reading supported control values with reasonable effort. With such an approach, a very sparsely populated control becomes feasible and with integer64 controls that'd likely allow representing most exponential controls with linear values. If going this route, I don't see an application of V4L2_CTRL_FLAG_EXPONENTIAL.

Thus, I think that control over the rounding is tightly related to this patchset and needs to be discussed together.

Quoted 6 lines · Helmut Grohne

1. There are a number of controls whose value is not easily described as either linear or exponential. I'm faced with at least two controls that actually are floating point numbers. One with two bits for the exponent and one (strange) bit for the mantissa (no joke) and another with three bits for the exponent and four bits for the mantissa. Neither can suitably be represented.

The proposal does not address all potential situations, that's true. There's no way to try to represent everything out there (without enumerating the values) in an easily generalised way but something can be done.

There are devices such as some flash LED controllers where the flash current if simply a value you can pick from the list. It's currently implemented as an integer control. AFAIR the driver is drivers/leds/leds-aat1290.c .

The fact is that the unit is specific to hardware. The documentation documents something, and often suggests a unit, but if the hardware does something else, then that's what you get --- independently of what the documentation says.

Hence the need to convey it via the API.

Some controls could have the unit set by the framework if that makes sense.

Most drivers shouldn't actually need to touch this if they're fine with defaults (whenever a control has a default).

A macro or two, it's not a major change. From the user space point of view, does it make a difference if a control has no unit or when it's not known what the unit is?

Yes, I think [V4L2_CTRL_FLAG_EXPONENTIAL] can be dropped as I suggested.

Quoted 2 lines

Thus, I think that control over the rounding is tightly related to this patchset and needs to be discussed together.

It addresses some of the same problem area but the implementation is orthogonal to this.

Providing that would probably make the base field less useful: the valid control values could be enumerated by the user using TRY_EXT_CTRLS without the need to tell the valid values are powers of e.g. two.

I don't really have a strong opinion on that actually when it comes to the API itself. The imx208 driver could proceed to use linear relation between the control value and the digital gain. My worry is just the driver
implementation: this may not be entirely trivial. There's still no way to address this problem in a generic way otherwise.

Quoted 18 lines · Helmut Grohne

Quoted 5 lines · Sakari Ailus

This set adds a few things to the current control framework in terms of what kind of information the user space may have on controls. It adds support for units and prefixes, exponential base as well as information on whether a control is linear or exponential, to struct v4l2_query_ext_ctrl.

That's a great improvement. Being able to give meaning to controls is great. However, I see two significant weaknesses in the approach being taken:

1. There are a number of controls whose value is not easily described as either linear or exponential. I'm faced with at least two controls that actually are floating point numbers. One with two bits for the exponent and one (strange) bit for the mantissa (no joke) and another with three bits for the exponent and four bits for the mantissa. Neither can suitably be represented.

Quoted 4 lines

Since the value ranges are small for the cases I mentioned, I looked into using an integer menu. The present approach does not allow for replacing an integer with an integer menu though. Each control id has a fixed type. I'm not sure how to solve this.

The proposal does not address all potential situations, that's true. There's no way to try to represent everything out there (without enumerating the values) in an easily generalised way but something can be done.

There are devices such as some flash LED controllers where the flash current if simply a value you can pick from the list. It's currently implemented as an integer control. AFAIR the driver is drivers/leds/leds-aat1290.c .

Quoted 10 lines

2. The present implementation places the responsibility of assigning units and scales into drivers. A number of controls (e.g. V4L2_CID_EXPOSURE_ABSOLUTE, V4L2_CID_AUDIO_LIMITER_RELEASE_TIME, V4L2_CID_PIXEL_RATE, ...) clearly state the scale and unit in the documentation. Having each and every driver set units and scales in the documented way will lead to duplication and buggy code. Having each driver choose unit and scale will lead to inconsistency. It would be very good to have a mechanism that puts the framework in charge of maintaining units and scales for the standard control ids.

Quoted 5 lines

What is a consumer supposed to do with a control that changes unit? The algorithm expected e.g. a duration. It might be able to convert to pixels, but what should it do if it gets back amperes? I argue that the unit should be a property of the control id and be documented (i.e. what is done now). If it is reported via an ioctl,

The fact is that the unit is specific to hardware. The documentation documents something, and often suggests a unit, but if the hardware does something else, then that's what you get --- independently of what the documentation says.

Hence the need to convey it via the API.

Some controls could have the unit set by the framework if that makes sense. Most drivers shouldn't actually need to touch this if they're fine with defaults (whenever a control has a default).

Quoted 25 lines

the framework needs to be in charge of setting it.

The story is much different for scales. Scaling the value up and down is something consumers can reasonably be expected to do. It allows shifting the value range such that the relevant values can be fully represented. Allowing drivers to change scales is much more reasonable. Still the framework should provide a default such that most drivers should not need any update.

I acknowledge that these expectations are high and see that they're partially covered in your later remarks.

Quoted 4 lines

The smiapp driver gains support for the feature. In the near term, some controls could also be assigned the unit automatically. The pixel rate, for instance. Fewer driver changes would be needed this way. A driver could override the value if there's a need to.

I believe that in the interest of keeping maintenance cost low, this should happen rather sooner than later. Just even adding the support to smiapp seems wrong when it would be possible to have the framework do the work.

Quoted 2 lines

I think I'll merge the undefined and no unit cases. Same for the exponential base actually --- the flag can be removed, too...

Quoted 2 lines

I'm not sure I understand. It reads like you are going to revert a significant part of the patch.

A macro or two, it's not a major change. From the user space point of view, does it make a difference if a control has no unit or when it's not known what the unit is?

Quoted 10 lines

Quoted 6 lines

Regarding Ricardo's suggestion --- I was thinking of adding a control flag (yes, there are a few bits available) to tell how to round the value. The user could use the TRY_EXT_CTRLS IOCTL to figure out the next (or previous) control value by incrementing the current value and setting the appropriate flag. This is out of the scope of this set though.

This approach sounds really useful to me. Having control over the rounding would allow reading supported control values with reasonable effort.

Quoted 4 lines

With such an approach, a very sparsely populated control becomes feasible and with integer64 controls that'd likely allow representing most exponential controls with linear values. If going this route, I don't see an application of V4L2_CTRL_FLAG_EXPONENTIAL.

Yes, I think the flag can be dropped as I suggested.

Quoted 2 lines

Thus, I think that control over the rounding is tightly related to this patchset and needs to be discussed together.

It addresses some of the same problem area but the implementation is orthogonal to this.

Providing that would probably make the base field less useful: the valid control values could be enumerated by the user using TRY_EXT_CTRLS without the need to tell the valid values are powers of e.g. two.

I don't really have a strong opinion on that actually when it comes to the API itself. The imx208 driver could proceed to use linear relation between the control value and the digital gain. My worry is just the driver
implementation: this may not be entirely trivial. There's still no way to address this problem in a generic way otherwise.

Hi Sakari,

Quoted 24 lines · On Tuesday, 25 September 2018 15:30:31 EEST Sakari Ailus wrote:

Quoted 18 lines · Helmut Grohne

Quoted 5 lines · Sakari Ailus

This set adds a few things to the current control framework in terms of what kind of information the user space may have on controls. It adds support for units and prefixes, exponential base as well as information on whether a control is linear or exponential, to struct v4l2_query_ext_ctrl.

That's a great improvement. Being able to give meaning to controls is great. However, I see two significant weaknesses in the approach being taken:

1. There are a number of controls whose value is not easily described as either linear or exponential. I'm faced with at least two controls that actually are floating point numbers. One with two bits for the exponent and one (strange) bit for the mantissa (no joke) and another with three bits for the exponent and four bits for the mantissa. Neither can suitably be represented.

Quoted 4 lines

Since the value ranges are small for the cases I mentioned, I looked into using an integer menu. The present approach does not allow for replacing an integer with an integer menu though. Each control id has a fixed type. I'm not sure how to solve this.

Quoted 4 lines

The proposal does not address all potential situations, that's true. There's no way to try to represent everything out there (without enumerating the values) in an easily generalised way but something can be done.

[1]

Quoted 27 lines

There are devices such as some flash LED controllers where the flash current if simply a value you can pick from the list. It's currently implemented as an integer control. AFAIR the driver is drivers/leds/leds-aat1290.c .

Quoted 10 lines

2. The present implementation places the responsibility of assigning units and scales into drivers. A number of controls (e.g. V4L2_CID_EXPOSURE_ABSOLUTE, V4L2_CID_AUDIO_LIMITER_RELEASE_TIME, V4L2_CID_PIXEL_RATE, ...) clearly state the scale and unit in the documentation. Having each and every driver set units and scales in the documented way will lead to duplication and buggy code. Having each driver choose unit and scale will lead to inconsistency. It would be very good to have a mechanism that puts the framework in charge of maintaining units and scales for the standard control ids.

Quoted 5 lines

What is a consumer supposed to do with a control that changes unit? The algorithm expected e.g. a duration. It might be able to convert to pixels, but what should it do if it gets back amperes? I argue that the unit should be a property of the control id and be documented (i.e. what is done now). If it is reported via an ioctl,

The fact is that the unit is specific to hardware. The documentation documents something, and often suggests a unit, but if the hardware does something else, then that's what you get --- independently of what the documentation says.

Hence the need to convey it via the API.

Quoted 3 lines

Some controls could have the unit set by the framework if that makes sense. Most drivers shouldn't actually need to touch this if they're fine with defaults (whenever a control has a default).

I have to agree with Helmut here. If we don't handle the 95% of the common cases in the core, we'll never get it right. Drivers will be buggy in many ways. Which leads me to a related comment, we need v4l2-compliance support for this, to verify that each control reports valid information.

Quoted 26 lines

Quoted 26 lines

the framework needs to be in charge of setting it.

The story is much different for scales. Scaling the value up and down is something consumers can reasonably be expected to do. It allows shifting the value range such that the relevant values can be fully represented. Allowing drivers to change scales is much more reasonable. Still the framework should provide a default such that most drivers should not need any update.

I acknowledge that these expectations are high and see that they're partially covered in your later remarks.

Quoted 4 lines

The smiapp driver gains support for the feature. In the near term, some controls could also be assigned the unit automatically. The pixel rate, for instance. Fewer driver changes would be needed this way. A driver could override the value if there's a need to.

I believe that in the interest of keeping maintenance cost low, this should happen rather sooner than later. Just even adding the support to smiapp seems wrong when it would be possible to have the framework do the work.

Quoted 2 lines

I think I'll merge the undefined and no unit cases. Same for the exponential base actually --- the flag can be removed, too...

Quoted 17 lines

Quoted 2 lines

I'm not sure I understand. It reads like you are going to revert a significant part of the patch.

A macro or two, it's not a major change. From the user space point of view, does it make a difference if a control has no unit or when it's not known what the unit is?

Quoted 10 lines

Quoted 6 lines

Regarding Ricardo's suggestion --- I was thinking of adding a control flag (yes, there are a few bits available) to tell how to round the value. The user could use the TRY_EXT_CTRLS IOCTL to figure out the next (or previous) control value by incrementing the current value and setting the appropriate flag. This is out of the scope of this set though.

This approach sounds really useful to me. Having control over the rounding would allow reading supported control values with reasonable effort.

That would result in way too many ioctl calls though. If we design an API to report all valid values, we have to do it through a single ioctl call. I'm however not opposed to rounding flags, as there could be a separate use for that, but I'm concerned that drivers will not use them correctly and consistently if we don't implement support for them in the core (and for any integer control that can't be accurately represented by min, max and step, I don't think we can).

I'm also concerned about over-engineering a solution for a problem that I'm still not convinced does exist. Applications needing precise information about control will very likely be device-aware in my opinion (due to [1]). I'm open to debating this, in which case I would ask for real use cases, and a real implementation (in something else than a test program).

V4L2 is bloated with micro-features that have been badly designed and that are mostly unused, let's try to get it right this time.

Quoted 16 lines

Quoted 4 lines

With such an approach, a very sparsely populated control becomes feasible and with integer64 controls that'd likely allow representing most exponential controls with linear values. If going this route, I don't see an application of V4L2_CTRL_FLAG_EXPONENTIAL.

Yes, I think the flag can be dropped as I suggested.

Quoted 2 lines

Thus, I think that control over the rounding is tightly related to this patchset and needs to be discussed together.

It addresses some of the same problem area but the implementation is orthogonal to this.

Providing that would probably make the base field less useful: the valid control values could be enumerated by the user using TRY_EXT_CTRLS without the need to tell the valid values are powers of e.g. two.

Quoted 6 lines

I don't really have a strong opinion on that actually when it comes to the API itself. The imx208 driver could proceed to use linear relation between the control value and the digital gain. My worry is just the driver
implementation: this may not be entirely trivial. There's still no way to address this problem in a generic way otherwise.

Quoted 1 line · Sakari Ailus

Quoted 6 lines

Quoted 5 lines

Regarding Ricardo's suggestion --- I was thinking of adding a control flag (yes, there are a few bits available) to tell how to round the value. The user could use the TRY_EXT_CTRLS IOCTL to figure out the next (or previous) control value by incrementing the current value and setting the appropriate flag. This is out of the scope of this set though.

Quoted 6 lines

This approach sounds really useful to me. Having control over the rounding would allow reading supported control values with reasonable effort. With such an approach, a very sparsely populated control becomes feasible and with integer64 controls that'd likely allow representing most exponential controls with linear values. If going this route, I don't see an application of V4L2_CTRL_FLAG_EXPONENTIAL.

Yes, I think the flag can be dropped as I suggested.

Quoted 4 lines

Thus, I think that control over the rounding is tightly related to this patchset and needs to be discussed together.

It addresses some of the same problem area but the implementation is orthogonal to this.

Providing that would probably make the base field less useful: the valid control values could be enumerated by the user using TRY_EXT_CTRLS without the need to tell the valid values are powers of e.g. two.

I don't really have a strong opinion on that actually when it comes to the API itself. The imx208 driver could proceed to use linear relation between the control value and the digital gain. My worry is just the driver
implementation: this may not be entirely trivial. There's still no way to address this problem in a generic way otherwise.