[PATCH v5 0/2] New V4L2 control V4L2_CID_NOTIFY_GAINS

[PATCH v5 0/2] New V4L2 control V4L2_CID_NOTIFY_GAINS

[David Plowman]

Hello again

Thanks for all the discussion on the previous (v4) version of this
patch set. This new version addresses the points raised which were
principally in improving the documentation.

There was also mention of the v4l2_ctrl_fill function. I haven't
included this control there as the other "image source" controls
aren't listed and it seems to me that it's up to the sensor what
min/max/def to use, but please correct me on that if I've got that
wrong!

Best regards
David

David Plowman (2):
  media: v4l2-ctrls: Add V4L2_CID_NOTIFY_GAINS control
  media: v4l2-ctrls: Document V4L2_CID_NOTIFY_GAINS control

 .../media/v4l/ext-ctrls-image-source.rst      | 20 +++++++++++++++++++
 drivers/media/v4l2-core/v4l2-ctrls-defs.c     |  1 +
 include/uapi/linux/v4l2-controls.h            |  1 +
 3 files changed, 22 insertions(+)

-- 
2.30.2

[PATCH v5 1/2] media: v4l2-ctrls: Add V4L2_CID_NOTIFY_GAINS control

[David Plowman]

We add a new control V4L2_CID_NOTIFY_GAINS which allows the sensor to
be notified what gains will be applied to the different colour
channels by subsequent processing (such as by an ISP), even though the
sensor will not apply any of these gains itself.

For Bayer sensors this will be an array control taking 4 values which
are the 4 gains arranged in the fixed order B, Gb, Gr and R,
irrespective of the exact Bayer order of the sensor itself. The use of
an array makes it straightforward to extend this control to non-Bayer
sensors (for example, sensors with an RGBW pattern) in future.

The units are in all cases linear with the default value indicating a
gain of exactly 1. For example, if the default value were reported as
128 then the value 256 would represent a gain of exactly 2.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 drivers/media/v4l2-core/v4l2-ctrls-defs.c | 1 +
 include/uapi/linux/v4l2-controls.h        | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
index 421300e13a41..f87053c83249 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
@@ -1107,6 +1107,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_TEST_PATTERN_GREENR:	return "Green (Red) Pixel Value";
 	case V4L2_CID_TEST_PATTERN_BLUE:	return "Blue Pixel Value";
 	case V4L2_CID_TEST_PATTERN_GREENB:	return "Green (Blue) Pixel Value";
+	case V4L2_CID_NOTIFY_GAINS:		return "Notify Gains";
 
 	/* Image processing controls */
 	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 5532b5f68493..133e20444939 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -1118,6 +1118,7 @@ enum v4l2_jpeg_chroma_subsampling {
 #define V4L2_CID_TEST_PATTERN_BLUE		(V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 6)
 #define V4L2_CID_TEST_PATTERN_GREENB		(V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 7)
 #define V4L2_CID_UNIT_CELL_SIZE			(V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 8)
+#define V4L2_CID_NOTIFY_GAINS			(V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 9)
 
 
 /* Image processing controls */
-- 
2.30.2

[PATCH v5 2/2] media: v4l2-ctrls: Document V4L2_CID_NOTIFY_GAINS control

[David Plowman]

Add documentation for the V4L2_CID_NOTIFY_GAINS control.

This control is required by sensors that need to know what colour
gains will be applied to pixels by downstream processing (such as by
an ISP), though the sensor does not apply these gains itself.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 .../media/v4l/ext-ctrls-image-source.rst      | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-image-source.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-image-source.rst
index de43f5c8486d..ba83cc0867d2 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-image-source.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-image-source.rst
@@ -72,3 +72,23 @@ Image Source Control IDs
     * - __u32
       - ``height``
       - Height of the area.
+
+``V4L2_CID_NOTIFY_GAINS (integer array)``
+    The sensor is notified what gains will be applied to the different
+    colour channels by subsequent processing (such as by an ISP). The
+    sensor is merely informed of these values in case it performs
+    processing that requires them, but it does not apply them itself to
+    the output pixels.
+
+    The use of an array allows this control to be extended to sensors
+    with, for example, non-Bayer CFAs (colour filter arrays).
+
+    Currently it is defined only for Bayer sensors, and is an array
+    control taking 4 gain values, being the gains for each of the
+    Bayer channels. The gains are always in the order B, Gb, Gr and R,
+    irrespective of the exact Bayer order of the sensor itself.
+
+    The units for the gain values are linear, with the default value
+    representing a gain of exactly 1. For example, if this default value
+    is reported as being (say) 128, then a value of 256 would represent
+    a gain of exactly 2.
-- 
2.30.2

Re: [PATCH v5 2/2] media: v4l2-ctrls: Document V4L2_CID_NOTIFY_GAINS control

[Laurent Pinchart]

Hi David,

Thank you for the patch.

On Tue, Aug 10, 2021 at 10:38:11AM +0100, David Plowman wrote:
> Add documentation for the V4L2_CID_NOTIFY_GAINS control.
> 
> This control is required by sensors that need to know what colour
> gains will be applied to pixels by downstream processing (such as by
> an ISP), though the sensor does not apply these gains itself.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  .../media/v4l/ext-ctrls-image-source.rst      | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-image-source.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-image-source.rst
> index de43f5c8486d..ba83cc0867d2 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-image-source.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-image-source.rst
> @@ -72,3 +72,23 @@ Image Source Control IDs
>      * - __u32
>        - ``height``
>        - Height of the area.
> +
> +``V4L2_CID_NOTIFY_GAINS (integer array)``
> +    The sensor is notified what gains will be applied to the different
> +    colour channels by subsequent processing (such as by an ISP). The
> +    sensor is merely informed of these values in case it performs
> +    processing that requires them, but it does not apply them itself to
> +    the output pixels.
> +
> +    The use of an array allows this control to be extended to sensors
> +    with, for example, non-Bayer CFAs (colour filter arrays).
> +
> +    Currently it is defined only for Bayer sensors, and is an array
> +    control taking 4 gain values, being the gains for each of the
> +    Bayer channels. The gains are always in the order B, Gb, Gr and R,
> +    irrespective of the exact Bayer order of the sensor itself.

I'd swap the above two paragraphs.

> +
> +    The units for the gain values are linear, with the default value
> +    representing a gain of exactly 1. For example, if this default value
> +    is reported as being (say) 128, then a value of 256 would represent
> +    a gain of exactly 2.

Maybe 1.0 and 2.0 instead of 1 and 2 to show that the gains values
themselves can be fractional ?

With those two minor issues addressed if deemed to be worth it,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 1/2] media: v4l2-ctrls: Add V4L2_CID_NOTIFY_GAINS control

[Laurent Pinchart]

Hi David,

Thank you for the patch.

On Tue, Aug 10, 2021 at 10:38:10AM +0100, David Plowman wrote:
> We add a new control V4L2_CID_NOTIFY_GAINS which allows the sensor to
> be notified what gains will be applied to the different colour
> channels by subsequent processing (such as by an ISP), even though the
> sensor will not apply any of these gains itself.
> 
> For Bayer sensors this will be an array control taking 4 values which
> are the 4 gains arranged in the fixed order B, Gb, Gr and R,
> irrespective of the exact Bayer order of the sensor itself. The use of
> an array makes it straightforward to extend this control to non-Bayer
> sensors (for example, sensors with an RGBW pattern) in future.
> 
> The units are in all cases linear with the default value indicating a
> gain of exactly 1. For example, if the default value were reported as
> 128 then the value 256 would represent a gain of exactly 2.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/v4l2-core/v4l2-ctrls-defs.c | 1 +
>  include/uapi/linux/v4l2-controls.h        | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> index 421300e13a41..f87053c83249 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> @@ -1107,6 +1107,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_TEST_PATTERN_GREENR:	return "Green (Red) Pixel Value";
>  	case V4L2_CID_TEST_PATTERN_BLUE:	return "Blue Pixel Value";
>  	case V4L2_CID_TEST_PATTERN_GREENB:	return "Green (Blue) Pixel Value";
> +	case V4L2_CID_NOTIFY_GAINS:		return "Notify Gains";
>  
>  	/* Image processing controls */
>  	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 5532b5f68493..133e20444939 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -1118,6 +1118,7 @@ enum v4l2_jpeg_chroma_subsampling {
>  #define V4L2_CID_TEST_PATTERN_BLUE		(V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 6)
>  #define V4L2_CID_TEST_PATTERN_GREENB		(V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 7)
>  #define V4L2_CID_UNIT_CELL_SIZE			(V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 8)
> +#define V4L2_CID_NOTIFY_GAINS			(V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 9)
>  
>  
>  /* Image processing controls */

-- 
Regards,

Laurent Pinchart