All of
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <>
Cc: Greg Kroah-Hartman <>,, Paul Elder <>,
	Laurent Pinchart <>,
	Jacopo Mondi <>,
	Jai Luthra <>,
	Sakari Ailus <>,
	Mauro Carvalho Chehab <>,
	Sasha Levin <>
Subject: [PATCH 4.14 18/21] media: ov5640: Fix analogue gain control
Date: Wed, 15 Mar 2023 13:12:41 +0100	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

From: Paul Elder <>

[ Upstream commit afa4805799c1d332980ad23339fdb07b5e0cf7e0 ]

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 <>
Signed-off-by: Laurent Pinchart <>
Reviewed-by: Jacopo Mondi <>
Reviewed-by: Jai Luthra <>
Signed-off-by: Sakari Ailus <>
Signed-off-by: Mauro Carvalho Chehab <>
Signed-off-by: Sasha Levin <>
 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 eb0331b8a5833..b78e35425d14f 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -2002,7 +2002,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,

  parent reply	other threads:[~2023-03-15 12:15 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-15 12:12 [PATCH 4.14 00/21] 4.14.310-rc1 review Greg Kroah-Hartman
2023-03-15 12:12 ` [PATCH 4.14 01/21] fs: prevent out-of-bounds array speculation when closing a file descriptor Greg Kroah-Hartman
2023-03-15 12:12 ` [PATCH 4.14 02/21] x86/CPU/AMD: Disable XSAVES on AMD family 0x17 Greg Kroah-Hartman
2023-03-15 12:12 ` [PATCH 4.14 03/21] ext4: fix RENAME_WHITEOUT handling for inline directories Greg Kroah-Hartman
2023-03-15 12:12 ` [PATCH 4.14 04/21] ext4: fix another off-by-one fsmap error on 1k block filesystems Greg Kroah-Hartman
2023-03-15 12:12 ` [PATCH 4.14 05/21] ext4: move where set the MAY_INLINE_DATA flag is set Greg Kroah-Hartman
2023-03-15 12:12 ` [PATCH 4.14 06/21] ext4: fix WARNING in ext4_update_inline_data Greg Kroah-Hartman
2023-03-15 12:12 ` [PATCH 4.14 07/21] ext4: zero i_disksize when initializing the bootloader inode Greg Kroah-Hartman
2023-03-15 12:12 ` [PATCH 4.14 08/21] nfc: change order inside nfc_se_io error path Greg Kroah-Hartman
2023-03-15 12:12 ` [PATCH 4.14 09/21] nfc: fdp: add null check of devm_kmalloc_array in fdp_nci_i2c_read_device_properties Greg Kroah-Hartman
2023-03-15 12:12 ` [PATCH 4.14 10/21] ila: do not generate empty messages in ila_xlat_nl_cmd_get_mapping() Greg Kroah-Hartman
2023-03-15 12:12 ` [PATCH 4.14 11/21] net: caif: Fix use-after-free in cfusbl_device_notify() Greg Kroah-Hartman
2023-03-15 12:12 ` [PATCH 4.14 12/21] clk: qcom: mmcc-apq8084: remove spdm clocks Greg Kroah-Hartman
2023-03-15 12:12 ` [PATCH 4.14 13/21] MIPS: Fix a compilation issue Greg Kroah-Hartman
2023-03-15 12:12 ` [PATCH 4.14 14/21] alpha: fix R_ALPHA_LITERAL reloc for large modules Greg Kroah-Hartman
2023-03-15 12:12 ` [PATCH 4.14 15/21] macintosh: windfarm: Use unsigned type for 1-bit bitfields Greg Kroah-Hartman
2023-03-15 12:12 ` [PATCH 4.14 16/21] PCI: Add SolidRun vendor ID Greg Kroah-Hartman
2023-03-15 12:12 ` [PATCH 4.14 17/21] PCI: Avoid FLR for SolidRun SNET DPU rev 1 Greg Kroah-Hartman
2023-03-15 12:12 ` Greg Kroah-Hartman [this message]
2023-03-15 12:12 ` [PATCH 4.14 19/21] tipc: improve function tipc_wait_for_cond() Greg Kroah-Hartman
2023-03-15 12:12 ` [PATCH 4.14 20/21] drm/i915: Dont use BAR mappings for ring buffers with LLC Greg Kroah-Hartman
2023-03-15 12:12   ` [Intel-gfx] " Greg Kroah-Hartman
2023-03-15 12:12 ` [PATCH 4.14 21/21] x86/cpu: Fix LFENCE serialization check in init_amd() Greg Kroah-Hartman
2023-03-15 13:55 ` [PATCH 4.14 00/21] 4.14.310-rc1 review Chris Paterson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.