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.
> > 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.
> > 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
