As of:
commit bf4a4b518c20 ("media: ov5640: Don't force the auto exposure state at
start time") auto-exposure got disabled before programming new capture modes to
the sensor. Unfortunately the function used to do that (ov5640_set_exposure())
does not enable/disable auto-exposure engine through register 0x3503[0] bit, but
programs registers [0x3500 - 0x3502] which represent the desired exposure time
when running with manual exposure. As a result, auto-exposure was not actually
disabled at all.
To actually disable auto-exposure, go through the control framework instead of
calling ov5640_set_exposure() function directly.
Also, as auto-gain and auto-exposure are disabled un-conditionally but only
restored to their previous values in ov5640_set_mode_direct() function, move
controls restoring so that their value is re-programmed opportunely after
either ov5640_set_mode_direct() or ov5640_set_mode_exposure_calc() have been
executed.
Fixes: bf4a4b518c20 ("media: ov5640: Don't force the auto exposure state at start time")
Signed-off-by: Jacopo Mondi <jacopo@xxxxxxxxxx>
---
Is it worth doing with auto-gain what we're doing with auto-exposure? Cache the
value and then re-program it instead of unconditionally disable/enable it?
Thanks
j
---
---
drivers/media/i2c/ov5640.c | 29 +++++++++++++----------------
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 12b3496..bc75cb7 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1588,25 +1588,13 @@ static int ov5640_set_mode_exposure_calc(struct ov5640_dev *sensor,
* change mode directly
*/
static int ov5640_set_mode_direct(struct ov5640_dev *sensor,
- const struct ov5640_mode_info *mode,
- s32 exposure)
+ const struct ov5640_mode_info *mode)
{
- int ret;
-
if (!mode->reg_data)
return -EINVAL;
/* Write capture setting */
- ret = ov5640_load_regs(sensor, mode);
- if (ret < 0)
- return ret;
-
- /* turn auto gain/exposure back on for direct mode */
- ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_gain, 1);
- if (ret)
- return ret;
-
- return __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp, exposure);
+ return ov5640_load_regs(sensor, mode);
}
static int ov5640_set_mode(struct ov5640_dev *sensor,
@@ -1626,7 +1614,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
return ret;
exposure = sensor->ctrls.auto_exp->val;
- ret = ov5640_set_exposure(sensor, V4L2_EXPOSURE_MANUAL);
+ ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp, V4L2_EXPOSURE_MANUAL);
if (ret)
return ret;
@@ -1642,12 +1630,21 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
* change inside subsampling or scaling
* download firmware directly
*/
- ret = ov5640_set_mode_direct(sensor, mode, exposure);
+ ret = ov5640_set_mode_direct(sensor, mode);
}
if (ret < 0)
return ret;
+ /* Restore auto-gain and auto-exposure after mode has changed. */
+ ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_gain, 1);
+ if (ret)
+ return ret;
+
+ ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp, exposure)
+ if (ret)
+ return ret;
+
ret = ov5640_set_binning(sensor, dn_mode != SCALING);
if (ret < 0)
return ret;
--
2.7.4
