| @@ -0,0 +1,93 @@
+From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+To: linux-media@vger.kernel.org
+Cc: Jacopo Mondi <jacopo.mondi@ideasonboard.com>,
+ Sakari Ailus <sakari.ailus@iki.fi>,
+ Paul Elder <paul.elder@ideasonboard.com>,
+ Steve Longerbeam <slongerbeam@gmail.com>,
+ hugues.fruchet@st.com, prabhakar.mahadev-lad.rj@bp.renesas.com,
+ aford173@gmail.com, festevam@gmail.com, eddy.khan@vergesense.com,
+ paul.kocialkowski@bootlin.com, eugen.hristev@microchip.com
+Subject: [PATCH v2] media: ov5640: Fix analogue gain control
+Date: Mon, 28 Nov 2022 10:02:01 +0200 [thread overview]
+Message-ID: <20221128080201.15104-1-laurent.pinchart@ideasonboard.com> (raw)
+
+From: Paul Elder <paul.elder@ideasonboard.com>
+
+Gain control is badly documented in publicly available (including
+leaked) documentation.
+
+There is an AGC pre-gain in register 0x3a13, expressed as a 6-bit value
+(plus an enable bit in bit 6). The driver hardcodes it to 0x43, which
+one application note states is equal to x1.047. The documentation also
+states that 0x40 is equel to x1.000. The pre-gain thus seems to be
+expressed as in 1/64 increments, and thus ranges from x1.00 to x1.984.
+What the pre-gain does is however unspecified.
+
+There is then an AGC gain limit, in registers 0x3a18 and 0x3a19,
+expressed as a 10-bit "real gain format" value. One application note
+sets it to 0x00f8 and states it is equal to x15.5, so it appears to be
+expressed in 1/16 increments, up to x63.9375.
+
+The manual gain is stored in registers 0x350a and 0x350b, also as a
+10-bit "real gain format" value. It is documented in the application
+note as a Q6.4 values, up to x63.9375.
+
+One version of the datasheet indicates that the sensor supports a
+digital gain:
+
+ The OV5640 supports 1/2/4 digital gain. Normally, the gain is
+ controlled automatically by the automatic gain control (AGC) block.
+
+It isn't clear how that would be controlled manually.
+
+There appears to be no indication regarding whether the gain controlled
+through registers 0x350a and 0x350b is an analogue gain only or also
+includes digital gain. The words "real gain" don't necessarily mean
+"combined analogue and digital gains". Some OmniVision sensors (such as
+the OV8858) are documented as supoprting different formats for the gain
+values, selectable through a register bit, and they are called "real
+gain format" and "sensor gain format". For that sensor, we have (one of)
+the gain registers documented as
+
+ 0x3503[2]=0, gain[7:0] is real gain format, where low 4 bits are
+ fraction bits, for example, 0x10 is 1x gain, 0x28 is 2.5x gain
+
+ If 0x3503[2]=1, gain[7:0] is sensor gain format, gain[7:4] is coarse
+ gain, 00000: 1x, 00001: 2x, 00011: 4x, 00111: 8x, gain[7] is 1,
+ gain[3:0] is fine gain. For example, 0x10 is 1x gain, 0x30 is 2x gain,
+ 0x70 is 4x gain
+
+(The second part of the text makes little sense)
+
+"Real gain" may thus refer to the combination of the coarse and fine
+analogue gains as a single value.
+
+The OV5640 0x350a and 0x350b registers thus appear to control analogue
+gain. The driver incorrectly uses V4L2_CID_GAIN as V4L2 has a specific
+control for analogue gain, V4L2_CID_ANALOGUE_GAIN. Use it.
+
+If registers 0x350a and 0x350b are later found to control digital gain
+as well, the driver could then restrict the range of the analogue gain
+control value to lower than x64 and add a separate digital gain control.
+
+Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
+Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
+---
+ drivers/media/i2c/ov5640.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
+index 2d740397a5d4..c65c391bc1eb 100644
+--- a/drivers/media/i2c/ov5640.c
++++ b/drivers/media/i2c/ov5640.c
+@@ -3458,7 +3458,7 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
+ /* Auto/manual gain */
+ ctrls->auto_gain = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_AUTOGAIN,
+ 0, 1, 1, 1);
+- ctrls->gain = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_GAIN,
++ ctrls->gain = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_ANALOGUE_GAIN,
+ 0, 1023, 1, 0);
+
+ ctrls->saturation = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_SATURATION,
+--
|
