On 09/08/2021 14:19, Laurent Pinchart wrote:
> Hi Hans,
>
> On Mon, Aug 09, 2021 at 01:05:00PM +0200, Hans Verkuil wrote:
>> On 09/08/2021 11:34, 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 units are in all cases linear with the default value indicating a
>>> gain of exactly 1.
>>
>> So a value of 2 means a gain of 2? Or are these fixed point values? How do
>> I represent a gain of 1.5?
>
> No, the default value corresponds to a x1.0 gain, but it's not 1. If the
> default is, let's say, 128, then x2.0 will be 256.
Ah, now I get it. Perhaps a small example of this in the documentation patch will
help clarify this.
>
>>> Signed-off-by: David Plowman <david.plowman@xxxxxxxxxxxxxxx>
>>> ---
>>> 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! */
>>
>> Since this is a standard control, it should also be configured correctly in
>> v4l2_ctrl_fill().
>>
>> Instead of an array, would a compound control (aka a struct) be better? Then you can
>> explicitly have field names g, gb, gr and r.
>>
>> Is there a specific reason we want an array instead of that? I'm not opposed, but
>> I'd like to see a rationale for that.
>
> Bayer ia only one of the possible CFA patterns for sensors. With a
> structure containing named b, gb, gr and r fields, we wouldn't be able
> to support, for instance, RGBW filters. It's not clear at this point
> what other CFA patterns will be seen in sensors that require this
> feature, but an array control will be able to more easily support these
> use cases.
OK. It is probably a good idea to mention this in the commit log at least.
Regards,
Hans
>
>>> 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 */
>
