* [PATCH v3 1/5] media: ov5640: fix exposure regression
2018-09-11 13:48 [PATCH v3 0/5] Fix OV5640 exposure & gain Hugues Fruchet
@ 2018-09-11 13:48 ` Hugues Fruchet
2018-09-11 13:48 ` [PATCH v3 2/5] media: ov5640: fix auto gain & exposure when changing mode Hugues Fruchet
` (7 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Hugues Fruchet @ 2018-09-11 13:48 UTC (permalink / raw)
To: Steve Longerbeam, Sakari Ailus, Jacopo Mondi, Hans Verkuil,
Mauro Carvalho Chehab
Cc: linux-media, Hugues Fruchet, Benjamin Gaignard
fixes: bf4a4b518c20 ("media: ov5640: Don't force the auto exposure state at start time").
Symptom was black image when capturing HD or 5Mp picture
due to manual exposure set to 1 while it was intended to
set autoexposure to "manual", fix this.
Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/i2c/ov5640.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 1ecbb7a..4b9da8b 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -938,6 +938,12 @@ static int ov5640_load_regs(struct ov5640_dev *sensor,
return ret;
}
+static int ov5640_set_autoexposure(struct ov5640_dev *sensor, bool on)
+{
+ return ov5640_mod_reg(sensor, OV5640_REG_AEC_PK_MANUAL,
+ BIT(0), on ? 0 : BIT(0));
+}
+
/* read exposure, in number of line periods */
static int ov5640_get_exposure(struct ov5640_dev *sensor)
{
@@ -1593,7 +1599,7 @@ static int ov5640_set_mode_exposure_calc(struct ov5640_dev *sensor,
*/
static int ov5640_set_mode_direct(struct ov5640_dev *sensor,
const struct ov5640_mode_info *mode,
- s32 exposure)
+ bool auto_exp)
{
int ret;
@@ -1610,7 +1616,8 @@ static int ov5640_set_mode_direct(struct ov5640_dev *sensor,
if (ret)
return ret;
- return __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp, exposure);
+ return __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp, auto_exp ?
+ V4L2_EXPOSURE_AUTO : V4L2_EXPOSURE_MANUAL);
}
static int ov5640_set_mode(struct ov5640_dev *sensor,
@@ -1618,7 +1625,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
{
const struct ov5640_mode_info *mode = sensor->current_mode;
enum ov5640_downsize_mode dn_mode, orig_dn_mode;
- s32 exposure;
+ bool auto_exp = sensor->ctrls.auto_exp->val == V4L2_EXPOSURE_AUTO;
int ret;
dn_mode = mode->dn_mode;
@@ -1629,8 +1636,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
if (ret)
return ret;
- exposure = sensor->ctrls.auto_exp->val;
- ret = ov5640_set_exposure(sensor, V4L2_EXPOSURE_MANUAL);
+ ret = ov5640_set_autoexposure(sensor, false);
if (ret)
return ret;
@@ -1646,7 +1652,7 @@ 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, auto_exp);
}
if (ret < 0)
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 2/5] media: ov5640: fix auto gain & exposure when changing mode
2018-09-11 13:48 [PATCH v3 0/5] Fix OV5640 exposure & gain Hugues Fruchet
2018-09-11 13:48 ` [PATCH v3 1/5] media: ov5640: fix exposure regression Hugues Fruchet
@ 2018-09-11 13:48 ` Hugues Fruchet
2018-09-11 13:48 ` [PATCH v3 3/5] media: ov5640: fix wrong binning value in exposure calculation Hugues Fruchet
` (6 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Hugues Fruchet @ 2018-09-11 13:48 UTC (permalink / raw)
To: Steve Longerbeam, Sakari Ailus, Jacopo Mondi, Hans Verkuil,
Mauro Carvalho Chehab
Cc: linux-media, Hugues Fruchet, Benjamin Gaignard
Ensure that auto gain and auto exposure are well restored
when changing mode.
Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
drivers/media/i2c/ov5640.c | 96 ++++++++++++++++++++++++++--------------------
1 file changed, 54 insertions(+), 42 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 4b9da8b..7c569de 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1000,6 +1000,18 @@ static int ov5640_get_gain(struct ov5640_dev *sensor)
return gain & 0x3ff;
}
+static int ov5640_set_gain(struct ov5640_dev *sensor, int gain)
+{
+ return ov5640_write_reg16(sensor, OV5640_REG_AEC_PK_REAL_GAIN,
+ (u16)gain & 0x3ff);
+}
+
+static int ov5640_set_autogain(struct ov5640_dev *sensor, bool on)
+{
+ return ov5640_mod_reg(sensor, OV5640_REG_AEC_PK_MANUAL,
+ BIT(1), on ? 0 : BIT(1));
+}
+
static int ov5640_set_stream_dvp(struct ov5640_dev *sensor, bool on)
{
int ret;
@@ -1577,7 +1589,7 @@ static int ov5640_set_mode_exposure_calc(struct ov5640_dev *sensor,
}
/* set capture gain */
- ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.gain, cap_gain16);
+ ret = ov5640_set_gain(sensor, cap_gain16);
if (ret)
return ret;
@@ -1590,7 +1602,7 @@ static int ov5640_set_mode_exposure_calc(struct ov5640_dev *sensor,
}
/* set exposure */
- return __v4l2_ctrl_s_ctrl(sensor->ctrls.exposure, cap_shutter);
+ return ov5640_set_exposure(sensor, cap_shutter);
}
/*
@@ -1598,26 +1610,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,
- bool auto_exp)
+ 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, auto_exp ?
- V4L2_EXPOSURE_AUTO : V4L2_EXPOSURE_MANUAL);
+ return ov5640_load_regs(sensor, mode);
}
static int ov5640_set_mode(struct ov5640_dev *sensor,
@@ -1625,6 +1624,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
{
const struct ov5640_mode_info *mode = sensor->current_mode;
enum ov5640_downsize_mode dn_mode, orig_dn_mode;
+ bool auto_gain = sensor->ctrls.auto_gain->val == 1;
bool auto_exp = sensor->ctrls.auto_exp->val == V4L2_EXPOSURE_AUTO;
int ret;
@@ -1632,19 +1632,23 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
orig_dn_mode = orig_mode->dn_mode;
/* auto gain and exposure must be turned off when changing modes */
- ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_gain, 0);
- if (ret)
- return ret;
+ if (auto_gain) {
+ ret = ov5640_set_autogain(sensor, false);
+ if (ret)
+ return ret;
+ }
- ret = ov5640_set_autoexposure(sensor, false);
- if (ret)
- return ret;
+ if (auto_exp) {
+ ret = ov5640_set_autoexposure(sensor, false);
+ if (ret)
+ goto restore_auto_gain;
+ }
if ((dn_mode == SUBSAMPLING && orig_dn_mode == SCALING) ||
(dn_mode == SCALING && orig_dn_mode == SUBSAMPLING)) {
/*
* change between subsampling and scaling
- * go through exposure calucation
+ * go through exposure calculation
*/
ret = ov5640_set_mode_exposure_calc(sensor, mode);
} else {
@@ -1652,11 +1656,16 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
* change inside subsampling or scaling
* download firmware directly
*/
- ret = ov5640_set_mode_direct(sensor, mode, auto_exp);
+ ret = ov5640_set_mode_direct(sensor, mode);
}
-
if (ret < 0)
- return ret;
+ goto restore_auto_exp_gain;
+
+ /* restore auto gain and exposure */
+ if (auto_gain)
+ ov5640_set_autogain(sensor, true);
+ if (auto_exp)
+ ov5640_set_autoexposure(sensor, true);
ret = ov5640_set_timings(sensor, mode);
if (ret < 0)
@@ -1681,6 +1690,15 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
sensor->pending_mode_change = false;
return 0;
+
+restore_auto_exp_gain:
+ if (auto_exp)
+ ov5640_set_autoexposure(sensor, true);
+restore_auto_gain:
+ if (auto_gain)
+ ov5640_set_autogain(sensor, true);
+
+ return ret;
}
static int ov5640_set_framefmt(struct ov5640_dev *sensor,
@@ -2141,20 +2159,20 @@ static int ov5640_set_ctrl_white_balance(struct ov5640_dev *sensor, int awb)
return ret;
}
-static int ov5640_set_ctrl_exposure(struct ov5640_dev *sensor, int exp)
+static int ov5640_set_ctrl_exposure(struct ov5640_dev *sensor,
+ enum v4l2_exposure_auto_type auto_exposure)
{
struct ov5640_ctrls *ctrls = &sensor->ctrls;
- bool auto_exposure = (exp == V4L2_EXPOSURE_AUTO);
+ bool auto_exp = (auto_exposure == V4L2_EXPOSURE_AUTO);
int ret = 0;
if (ctrls->auto_exp->is_new) {
- ret = ov5640_mod_reg(sensor, OV5640_REG_AEC_PK_MANUAL,
- BIT(0), auto_exposure ? 0 : BIT(0));
+ ret = ov5640_set_autoexposure(sensor, auto_exp);
if (ret)
return ret;
}
- if (!auto_exposure && ctrls->exposure->is_new) {
+ if (!auto_exp && ctrls->exposure->is_new) {
u16 max_exp;
ret = ov5640_read_reg16(sensor, OV5640_REG_AEC_PK_VTS,
@@ -2174,25 +2192,19 @@ static int ov5640_set_ctrl_exposure(struct ov5640_dev *sensor, int exp)
return ret;
}
-static int ov5640_set_ctrl_gain(struct ov5640_dev *sensor, int auto_gain)
+static int ov5640_set_ctrl_gain(struct ov5640_dev *sensor, bool auto_gain)
{
struct ov5640_ctrls *ctrls = &sensor->ctrls;
int ret = 0;
if (ctrls->auto_gain->is_new) {
- ret = ov5640_mod_reg(sensor, OV5640_REG_AEC_PK_MANUAL,
- BIT(1),
- ctrls->auto_gain->val ? 0 : BIT(1));
+ ret = ov5640_set_autogain(sensor, auto_gain);
if (ret)
return ret;
}
- if (!auto_gain && ctrls->gain->is_new) {
- u16 gain = (u16)ctrls->gain->val;
-
- ret = ov5640_write_reg16(sensor, OV5640_REG_AEC_PK_REAL_GAIN,
- gain & 0x3ff);
- }
+ if (!auto_gain && ctrls->gain->is_new)
+ ret = ov5640_set_gain(sensor, ctrls->gain->val);
return ret;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 3/5] media: ov5640: fix wrong binning value in exposure calculation
2018-09-11 13:48 [PATCH v3 0/5] Fix OV5640 exposure & gain Hugues Fruchet
2018-09-11 13:48 ` [PATCH v3 1/5] media: ov5640: fix exposure regression Hugues Fruchet
2018-09-11 13:48 ` [PATCH v3 2/5] media: ov5640: fix auto gain & exposure when changing mode Hugues Fruchet
@ 2018-09-11 13:48 ` Hugues Fruchet
2018-09-11 13:48 ` [PATCH v3 4/5] media: ov5640: fix auto controls values when switching to manual mode Hugues Fruchet
` (5 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Hugues Fruchet @ 2018-09-11 13:48 UTC (permalink / raw)
To: Steve Longerbeam, Sakari Ailus, Jacopo Mondi, Hans Verkuil,
Mauro Carvalho Chehab
Cc: linux-media, Hugues Fruchet, Benjamin Gaignard
ov5640_set_mode_exposure_calc() is checking binning value but
binning value read is buggy, fix this.
Rename ov5640_binning_on() to ov5640_get_binning() as per other
similar functions.
Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/i2c/ov5640.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 7c569de..9fb17b5 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1349,7 +1349,7 @@ static int ov5640_set_ae_target(struct ov5640_dev *sensor, int target)
return ov5640_write_reg(sensor, OV5640_REG_AEC_CTRL1F, fast_low);
}
-static int ov5640_binning_on(struct ov5640_dev *sensor)
+static int ov5640_get_binning(struct ov5640_dev *sensor)
{
u8 temp;
int ret;
@@ -1357,8 +1357,8 @@ static int ov5640_binning_on(struct ov5640_dev *sensor)
ret = ov5640_read_reg(sensor, OV5640_REG_TIMING_TC_REG21, &temp);
if (ret)
return ret;
- temp &= 0xfe;
- return temp ? 1 : 0;
+
+ return temp & BIT(0);
}
static int ov5640_set_binning(struct ov5640_dev *sensor, bool enable)
@@ -1468,7 +1468,7 @@ static int ov5640_set_mode_exposure_calc(struct ov5640_dev *sensor,
if (ret < 0)
return ret;
prev_shutter = ret;
- ret = ov5640_binning_on(sensor);
+ ret = ov5640_get_binning(sensor);
if (ret < 0)
return ret;
if (ret && mode->id != OV5640_MODE_720P_1280_720 &&
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 4/5] media: ov5640: fix auto controls values when switching to manual mode
2018-09-11 13:48 [PATCH v3 0/5] Fix OV5640 exposure & gain Hugues Fruchet
` (2 preceding siblings ...)
2018-09-11 13:48 ` [PATCH v3 3/5] media: ov5640: fix wrong binning value in exposure calculation Hugues Fruchet
@ 2018-09-11 13:48 ` Hugues Fruchet
2018-09-11 13:48 ` [PATCH v3 5/5] media: ov5640: fix restore of last mode set Hugues Fruchet
` (4 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Hugues Fruchet @ 2018-09-11 13:48 UTC (permalink / raw)
To: Steve Longerbeam, Sakari Ailus, Jacopo Mondi, Hans Verkuil,
Mauro Carvalho Chehab
Cc: linux-media, Hugues Fruchet, Benjamin Gaignard
When switching from auto to manual mode, V4L2 core is calling
g_volatile_ctrl() in manual mode in order to get the manual initial value.
Remove the manual mode check/return to not break this behaviour.
Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
drivers/media/i2c/ov5640.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 9fb17b5..c110a6a 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -2277,16 +2277,12 @@ static int ov5640_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
switch (ctrl->id) {
case V4L2_CID_AUTOGAIN:
- if (!ctrl->val)
- return 0;
val = ov5640_get_gain(sensor);
if (val < 0)
return val;
sensor->ctrls.gain->val = val;
break;
case V4L2_CID_EXPOSURE_AUTO:
- if (ctrl->val == V4L2_EXPOSURE_MANUAL)
- return 0;
val = ov5640_get_exposure(sensor);
if (val < 0)
return val;
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 5/5] media: ov5640: fix restore of last mode set
2018-09-11 13:48 [PATCH v3 0/5] Fix OV5640 exposure & gain Hugues Fruchet
` (3 preceding siblings ...)
2018-09-11 13:48 ` [PATCH v3 4/5] media: ov5640: fix auto controls values when switching to manual mode Hugues Fruchet
@ 2018-09-11 13:48 ` Hugues Fruchet
2018-09-14 16:00 ` [PATCH v3 0/5] Fix OV5640 exposure & gain jacopo mondi
` (3 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Hugues Fruchet @ 2018-09-11 13:48 UTC (permalink / raw)
To: Steve Longerbeam, Sakari Ailus, Jacopo Mondi, Hans Verkuil,
Mauro Carvalho Chehab
Cc: linux-media, Hugues Fruchet, Benjamin Gaignard
Mode setting depends on last mode set, in particular
because of exposure calculation when downscale mode
change between subsampling and scaling.
At stream on the last mode was wrongly set to current mode,
so no change was detected and exposure calculation
was not made, fix this.
Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
drivers/media/i2c/ov5640.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index c110a6a..427c2e7 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -225,6 +225,7 @@ struct ov5640_dev {
struct v4l2_mbus_framefmt fmt;
const struct ov5640_mode_info *current_mode;
+ const struct ov5640_mode_info *last_mode;
enum ov5640_frame_rate current_fr;
struct v4l2_fract frame_interval;
@@ -1619,10 +1620,10 @@ static int ov5640_set_mode_direct(struct ov5640_dev *sensor,
return ov5640_load_regs(sensor, mode);
}
-static int ov5640_set_mode(struct ov5640_dev *sensor,
- const struct ov5640_mode_info *orig_mode)
+static int ov5640_set_mode(struct ov5640_dev *sensor)
{
const struct ov5640_mode_info *mode = sensor->current_mode;
+ const struct ov5640_mode_info *orig_mode = sensor->last_mode;
enum ov5640_downsize_mode dn_mode, orig_dn_mode;
bool auto_gain = sensor->ctrls.auto_gain->val == 1;
bool auto_exp = sensor->ctrls.auto_exp->val == V4L2_EXPOSURE_AUTO;
@@ -1688,6 +1689,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
return ret;
sensor->pending_mode_change = false;
+ sensor->last_mode = mode;
return 0;
@@ -1713,6 +1715,7 @@ static int ov5640_restore_mode(struct ov5640_dev *sensor)
ret = ov5640_load_regs(sensor, &ov5640_mode_init_data);
if (ret < 0)
return ret;
+ sensor->last_mode = &ov5640_mode_init_data;
ret = ov5640_mod_reg(sensor, OV5640_REG_SYS_ROOT_DIVIDER, 0x3f,
(ilog2(OV5640_SCLK2X_ROOT_DIVIDER_DEFAULT) << 2) |
@@ -1721,7 +1724,7 @@ static int ov5640_restore_mode(struct ov5640_dev *sensor)
return ret;
/* now restore the last capture mode */
- ret = ov5640_set_mode(sensor, &ov5640_mode_init_data);
+ ret = ov5640_set_mode(sensor);
if (ret < 0)
return ret;
@@ -2551,7 +2554,7 @@ static int ov5640_s_stream(struct v4l2_subdev *sd, int enable)
if (sensor->streaming == !enable) {
if (enable && sensor->pending_mode_change) {
- ret = ov5640_set_mode(sensor, sensor->current_mode);
+ ret = ov5640_set_mode(sensor);
if (ret)
goto out;
@@ -2667,6 +2670,7 @@ static int ov5640_probe(struct i2c_client *client,
sensor->current_mode =
&ov5640_mode_data[OV5640_30_FPS][OV5640_MODE_VGA_640_480];
sensor->pending_mode_change = true;
+ sensor->last_mode = sensor->current_mode;
sensor->ae_target = 52;
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v3 0/5] Fix OV5640 exposure & gain
2018-09-11 13:48 [PATCH v3 0/5] Fix OV5640 exposure & gain Hugues Fruchet
` (4 preceding siblings ...)
2018-09-11 13:48 ` [PATCH v3 5/5] media: ov5640: fix restore of last mode set Hugues Fruchet
@ 2018-09-14 16:00 ` jacopo mondi
2018-09-14 16:07 ` jacopo mondi
` (2 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: jacopo mondi @ 2018-09-14 16:00 UTC (permalink / raw)
To: Hugues Fruchet
Cc: Steve Longerbeam, Sakari Ailus, Hans Verkuil,
Mauro Carvalho Chehab, linux-media, Benjamin Gaignard
[-- Attachment #1: Type: text/plain, Size: 5081 bytes --]
Hi Hugues,
thanks for the patches
On Tue, Sep 11, 2018 at 03:48:16PM +0200, Hugues Fruchet wrote:
> This patch serie fixes some problems around exposure & gain in OV5640 driver.
>
> The 4th patch about autocontrols requires also a fix in v4l2-ctrls.c:
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg133164.html
>
> Here is the test procedure used for exposure & gain controls check:
> 1) Preview in background
> $> gst-launch-1.0 v4l2src ! "video/x-raw, width=640, Height=480" ! queue ! waylandsink -e &
> 2) Check gain & exposure values
> $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)"
> exposure (int) : min=0 max=65535 step=1 default=0 value=330 flags=inactive, volatile
> gain (int) : min=0 max=1023 step=1 default=0 value=19 flags=inactive, volatile
> 3) Put finger in front of camera and check that gain/exposure values are changing:
> $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)"
> exposure (int) : min=0 max=65535 step=1 default=0 value=660 flags=inactive, volatile
> gain (int) : min=0 max=1023 step=1 default=0 value=37 flags=inactive, volatile
> 4) switch to manual mode, image exposition must not change
> $> v4l2-ctl --set-ctrl=gain_automatic=0
> $> v4l2-ctl --set-ctrl=auto_exposure=1
> Note the "1" for manual exposure.
>
> 5) Check current gain/exposure values:
> $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)"
> exposure (int) : min=0 max=65535 step=1 default=0 value=330
> gain (int) : min=0 max=1023 step=1 default=0 value=20
>
> 6) Put finger behind camera and check that gain/exposure values are NOT changing:
> $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)"
> exposure (int) : min=0 max=65535 step=1 default=0 value=330
> gain (int) : min=0 max=1023 step=1 default=0 value=20
> 7) Update exposure, check that it is well changed on display and that same value is returned:
> $> v4l2-ctl --set-ctrl=exposure=100
> $> v4l2-ctl --get-ctrl=exposure
> exposure: 100
>
> 9) Update gain, check that it is well changed on display and that same value is returned:
> $> v4l2-ctl --set-ctrl=gain=10
> $> v4l2-ctl --get-ctrl=gain
> gain: 10
>
> 10) Switch back to auto gain/exposure, verify that image is correct and values returned are correct:
> $> v4l2-ctl --set-ctrl=gain_automatic=1
> $> v4l2-ctl --set-ctrl=auto_exposure=0
> $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)"
> exposure (int) : min=0 max=65535 step=1 default=0 value=330 flags=inactive, volatile
> gain (int) : min=0 max=1023 step=1 default=0 value=22 flags=inactive, volatile
> Note the "0" for auto exposure.
>
I've tested on my side and I can confirm the exposure and gain when in
auto-mode changes as expected, and it is possible to switch back and
forth between auto and manual modes.
The patches also fixes an issue when capturing frames, as the first
two/three frames where always black in my setup before this series.
(While streaming to gstreamers' fakesink)
# v4l2-ctl --get-ctrl "exposure" --get-ctrl "gain" -d /dev/video4
exposure: 885
gain: 50
(Point a light in front of the sensor)
# v4l2-ctl --get-ctrl "exposure" --get-ctrl "gain" -d /dev/video4
exposure: 17
gain: 19
(Disable auto-gain and auto-exposure)
# v4l2-ctl -d /dev/video4 --set-ctrl=auto_exposure=1
# v4l2-ctl -d /dev/video4 --set-ctrl=gain_automatic=0
# v4l2-ctl -d /dev/video4 --set-ctrl=exposure=100
# v4l2-ctl --get-ctrl "exposure" --get-ctrl "gain" -d /dev/video4
exposure: 100
gain: 46
(Re-enable auto-exp and auto-gain)
# v4l2-ctl -d /dev/video4 --set-ctrl=auto_exposure=0
# v4l2-ctl -d /dev/video4 --set-ctrl=gain_automatic=1
# v4l2-ctl --get-ctrl "exposure" --get-ctrl "gain" -d /dev/video4
exposure: 885
gain: 46
(Finger on the sensor)
# v4l2-ctl --get-ctrl "exposure" --get-ctrl "gain" -d /dev/video4
exposure: 885
gain: 248
(Point a light on the sensor)
exposure: 16
gain: 19
So please add my
Tested-by: Jacopo Mondi <jacopo@jmondi.org>
to this series.
Thanks
j
> ===========
> = history =
> ===========
> version 3:
> - Change patch 5/5 by removing set_mode() orig_mode parameter as per jacopo' suggestion:
> https://www.spinics.net/lists/linux-media/msg139457.html
>
> version 2:
> - Fix patch 3/5 commit comment and rename binning function as per jacopo' suggestion:
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg133272.html
>
> Hugues Fruchet (5):
> media: ov5640: fix exposure regression
> media: ov5640: fix auto gain & exposure when changing mode
> media: ov5640: fix wrong binning value in exposure calculation
> media: ov5640: fix auto controls values when switching to manual mode
> media: ov5640: fix restore of last mode set
>
> drivers/media/i2c/ov5640.c | 128 ++++++++++++++++++++++++++-------------------
> 1 file changed, 73 insertions(+), 55 deletions(-)
>
> --
> 2.7.4
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v3 0/5] Fix OV5640 exposure & gain
2018-09-11 13:48 [PATCH v3 0/5] Fix OV5640 exposure & gain Hugues Fruchet
` (5 preceding siblings ...)
2018-09-14 16:00 ` [PATCH v3 0/5] Fix OV5640 exposure & gain jacopo mondi
@ 2018-09-14 16:07 ` jacopo mondi
2018-09-15 23:02 ` Sakari Ailus
2018-09-14 17:49 ` Steve Longerbeam
2018-09-14 18:25 ` Nicolas Dufresne
8 siblings, 1 reply; 14+ messages in thread
From: jacopo mondi @ 2018-09-14 16:07 UTC (permalink / raw)
To: Sakari Ailus, hugues.fruchet
Cc: Steve Longerbeam, Hans Verkuil, Mauro Carvalho Chehab,
linux-media, Benjamin Gaignard
[-- Attachment #1: Type: text/plain, Size: 4420 bytes --]
Hi Sakari,
On Tue, Sep 11, 2018 at 03:48:16PM +0200, Hugues Fruchet wrote:
> This patch serie fixes some problems around exposure & gain in OV5640 driver.
As you offered to collect this series and my CSI-2 fixes I have just
re-sent, you might be interested in this branch:
git://jmondi.org/linux
engicam-imx6q/media-master/ov5640/csi2_init_v4_exposure_v3
I have there re-based this series on top of mine, which is in turn
based on latest media master, where this series do not apply as-is
afaict.
I have added to Hugues' patches my reviewed-by and tested-by tags.
If you prefer to I can send you a pull request, or if you want to have
a chance to review the whole patch list please refer to the above
branch.
Let me know if I can help speeding up the inclusion of these two
series as they fix two real issues of MIPI CSI-2 capture operations
for this sensor.
Thanks
j
>
> The 4th patch about autocontrols requires also a fix in v4l2-ctrls.c:
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg133164.html
>
> Here is the test procedure used for exposure & gain controls check:
> 1) Preview in background
> $> gst-launch-1.0 v4l2src ! "video/x-raw, width=640, Height=480" ! queue ! waylandsink -e &
> 2) Check gain & exposure values
> $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)"
> exposure (int) : min=0 max=65535 step=1 default=0 value=330 flags=inactive, volatile
> gain (int) : min=0 max=1023 step=1 default=0 value=19 flags=inactive, volatile
> 3) Put finger in front of camera and check that gain/exposure values are changing:
> $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)"
> exposure (int) : min=0 max=65535 step=1 default=0 value=660 flags=inactive, volatile
> gain (int) : min=0 max=1023 step=1 default=0 value=37 flags=inactive, volatile
> 4) switch to manual mode, image exposition must not change
> $> v4l2-ctl --set-ctrl=gain_automatic=0
> $> v4l2-ctl --set-ctrl=auto_exposure=1
> Note the "1" for manual exposure.
>
> 5) Check current gain/exposure values:
> $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)"
> exposure (int) : min=0 max=65535 step=1 default=0 value=330
> gain (int) : min=0 max=1023 step=1 default=0 value=20
>
> 6) Put finger behind camera and check that gain/exposure values are NOT changing:
> $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)"
> exposure (int) : min=0 max=65535 step=1 default=0 value=330
> gain (int) : min=0 max=1023 step=1 default=0 value=20
> 7) Update exposure, check that it is well changed on display and that same value is returned:
> $> v4l2-ctl --set-ctrl=exposure=100
> $> v4l2-ctl --get-ctrl=exposure
> exposure: 100
>
> 9) Update gain, check that it is well changed on display and that same value is returned:
> $> v4l2-ctl --set-ctrl=gain=10
> $> v4l2-ctl --get-ctrl=gain
> gain: 10
>
> 10) Switch back to auto gain/exposure, verify that image is correct and values returned are correct:
> $> v4l2-ctl --set-ctrl=gain_automatic=1
> $> v4l2-ctl --set-ctrl=auto_exposure=0
> $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)"
> exposure (int) : min=0 max=65535 step=1 default=0 value=330 flags=inactive, volatile
> gain (int) : min=0 max=1023 step=1 default=0 value=22 flags=inactive, volatile
> Note the "0" for auto exposure.
>
> ===========
> = history =
> ===========
> version 3:
> - Change patch 5/5 by removing set_mode() orig_mode parameter as per jacopo' suggestion:
> https://www.spinics.net/lists/linux-media/msg139457.html
>
> version 2:
> - Fix patch 3/5 commit comment and rename binning function as per jacopo' suggestion:
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg133272.html
>
> Hugues Fruchet (5):
> media: ov5640: fix exposure regression
> media: ov5640: fix auto gain & exposure when changing mode
> media: ov5640: fix wrong binning value in exposure calculation
> media: ov5640: fix auto controls values when switching to manual mode
> media: ov5640: fix restore of last mode set
>
> drivers/media/i2c/ov5640.c | 128 ++++++++++++++++++++++++++-------------------
> 1 file changed, 73 insertions(+), 55 deletions(-)
>
> --
> 2.7.4
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v3 0/5] Fix OV5640 exposure & gain
2018-09-14 16:07 ` jacopo mondi
@ 2018-09-15 23:02 ` Sakari Ailus
2018-09-17 7:47 ` jacopo mondi
0 siblings, 1 reply; 14+ messages in thread
From: Sakari Ailus @ 2018-09-15 23:02 UTC (permalink / raw)
To: jacopo mondi
Cc: hugues.fruchet, Steve Longerbeam, Hans Verkuil,
Mauro Carvalho Chehab, linux-media, Benjamin Gaignard
Hi Jacopo, Hugues,
On Fri, Sep 14, 2018 at 06:07:12PM +0200, jacopo mondi wrote:
> Hi Sakari,
>
> On Tue, Sep 11, 2018 at 03:48:16PM +0200, Hugues Fruchet wrote:
> > This patch serie fixes some problems around exposure & gain in OV5640 driver.
>
> As you offered to collect this series and my CSI-2 fixes I have just
> re-sent, you might be interested in this branch:
>
> git://jmondi.org/linux
> engicam-imx6q/media-master/ov5640/csi2_init_v4_exposure_v3
>
> I have there re-based this series on top of mine, which is in turn
> based on latest media master, where this series do not apply as-is
> afaict.
>
> I have added to Hugues' patches my reviewed-by and tested-by tags.
> If you prefer to I can send you a pull request, or if you want to have
> a chance to review the whole patch list please refer to the above
> branch.
>
> Let me know if I can help speeding up the inclusion of these two
> series as they fix two real issues of MIPI CSI-2 capture operations
> for this sensor.
I've pushed the patches here:
<URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=for-4.20-5>
There was a merge commit and a few extra patches in your tree; I threw them
out. :-)
I also edited the commit messages slightly (format; no change in content)
--- the patches are as-is. I'll still check they look right before sending
a pull request, likely on Monday.
--
Regards,
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 0/5] Fix OV5640 exposure & gain
2018-09-15 23:02 ` Sakari Ailus
@ 2018-09-17 7:47 ` jacopo mondi
2018-09-17 11:40 ` Sakari Ailus
0 siblings, 1 reply; 14+ messages in thread
From: jacopo mondi @ 2018-09-17 7:47 UTC (permalink / raw)
To: Sakari Ailus
Cc: hugues.fruchet, Steve Longerbeam, Hans Verkuil,
Mauro Carvalho Chehab, linux-media, Benjamin Gaignard
[-- Attachment #1: Type: text/plain, Size: 1782 bytes --]
Hi Sakari,
thanks for handling this
On Sun, Sep 16, 2018 at 02:02:30AM +0300, Sakari Ailus wrote:
> Hi Jacopo, Hugues,
>
> On Fri, Sep 14, 2018 at 06:07:12PM +0200, jacopo mondi wrote:
> > Hi Sakari,
> >
> > On Tue, Sep 11, 2018 at 03:48:16PM +0200, Hugues Fruchet wrote:
> > > This patch serie fixes some problems around exposure & gain in OV5640 driver.
> >
> > As you offered to collect this series and my CSI-2 fixes I have just
> > re-sent, you might be interested in this branch:
> >
> > git://jmondi.org/linux
> > engicam-imx6q/media-master/ov5640/csi2_init_v4_exposure_v3
> >
> > I have there re-based this series on top of mine, which is in turn
> > based on latest media master, where this series do not apply as-is
> > afaict.
> >
> > I have added to Hugues' patches my reviewed-by and tested-by tags.
> > If you prefer to I can send you a pull request, or if you want to have
> > a chance to review the whole patch list please refer to the above
> > branch.
> >
> > Let me know if I can help speeding up the inclusion of these two
> > series as they fix two real issues of MIPI CSI-2 capture operations
> > for this sensor.
>
> I've pushed the patches here:
>
> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=for-4.20-5>
>
> There was a merge commit and a few extra patches in your tree; I threw them
> out. :-)
Yeah, those are a few patches I need for my testing platform... Forgot to
remove them, hope you didn't spend too much time on this.
>
> I also edited the commit messages slightly (format; no change in content)
> --- the patches are as-is. I'll still check they look right before sending
> a pull request, likely on Monday.
Thanks, let me now if I can help.
Cheers
j
>
> --
> Regards,
>
> Sakari Ailus
> sakari.ailus@linux.intel.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v3 0/5] Fix OV5640 exposure & gain
2018-09-17 7:47 ` jacopo mondi
@ 2018-09-17 11:40 ` Sakari Ailus
2018-09-24 8:11 ` Hugues FRUCHET
0 siblings, 1 reply; 14+ messages in thread
From: Sakari Ailus @ 2018-09-17 11:40 UTC (permalink / raw)
To: jacopo mondi
Cc: Sakari Ailus, hugues.fruchet, Steve Longerbeam, Hans Verkuil,
Mauro Carvalho Chehab, linux-media, Benjamin Gaignard
On Mon, Sep 17, 2018 at 09:47:19AM +0200, jacopo mondi wrote:
> Hi Sakari,
> thanks for handling this
>
> On Sun, Sep 16, 2018 at 02:02:30AM +0300, Sakari Ailus wrote:
> > Hi Jacopo, Hugues,
> >
> > On Fri, Sep 14, 2018 at 06:07:12PM +0200, jacopo mondi wrote:
> > > Hi Sakari,
> > >
> > > On Tue, Sep 11, 2018 at 03:48:16PM +0200, Hugues Fruchet wrote:
> > > > This patch serie fixes some problems around exposure & gain in OV5640 driver.
> > >
> > > As you offered to collect this series and my CSI-2 fixes I have just
> > > re-sent, you might be interested in this branch:
> > >
> > > git://jmondi.org/linux
> > > engicam-imx6q/media-master/ov5640/csi2_init_v4_exposure_v3
> > >
> > > I have there re-based this series on top of mine, which is in turn
> > > based on latest media master, where this series do not apply as-is
> > > afaict.
> > >
> > > I have added to Hugues' patches my reviewed-by and tested-by tags.
> > > If you prefer to I can send you a pull request, or if you want to have
> > > a chance to review the whole patch list please refer to the above
> > > branch.
> > >
> > > Let me know if I can help speeding up the inclusion of these two
> > > series as they fix two real issues of MIPI CSI-2 capture operations
> > > for this sensor.
> >
> > I've pushed the patches here:
> >
> > <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=for-4.20-5>
> >
> > There was a merge commit and a few extra patches in your tree; I threw them
> > out. :-)
>
> Yeah, those are a few patches I need for my testing platform... Forgot to
> remove them, hope you didn't spend too much time on this.
No, it was rather easy to remove them. I've sent a pull request on these:
<URL:https://patchwork.linuxtv.org/patch/52091/>
--
Sakari Ailus
e-mail: sakari.ailus@iki.fi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 0/5] Fix OV5640 exposure & gain
2018-09-17 11:40 ` Sakari Ailus
@ 2018-09-24 8:11 ` Hugues FRUCHET
0 siblings, 0 replies; 14+ messages in thread
From: Hugues FRUCHET @ 2018-09-24 8:11 UTC (permalink / raw)
To: Sakari Ailus, jacopo mondi
Cc: Sakari Ailus, Steve Longerbeam, Hans Verkuil,
Mauro Carvalho Chehab, linux-media@vger.kernel.org,
Benjamin Gaignard
Many thanks for this work Jacopo and Sakari,
On my side I've made some other improvements/fixes on OV5640 that I will
push in the coming days.
BR,
Hugues.
On 09/17/2018 01:40 PM, Sakari Ailus wrote:
> On Mon, Sep 17, 2018 at 09:47:19AM +0200, jacopo mondi wrote:
>> Hi Sakari,
>> thanks for handling this
>>
>> On Sun, Sep 16, 2018 at 02:02:30AM +0300, Sakari Ailus wrote:
>>> Hi Jacopo, Hugues,
>>>
>>> On Fri, Sep 14, 2018 at 06:07:12PM +0200, jacopo mondi wrote:
>>>> Hi Sakari,
>>>>
>>>> On Tue, Sep 11, 2018 at 03:48:16PM +0200, Hugues Fruchet wrote:
>>>>> This patch serie fixes some problems around exposure & gain in OV5640 driver.
>>>>
>>>> As you offered to collect this series and my CSI-2 fixes I have just
>>>> re-sent, you might be interested in this branch:
>>>>
>>>> git://jmondi.org/linux
>>>> engicam-imx6q/media-master/ov5640/csi2_init_v4_exposure_v3
>>>>
>>>> I have there re-based this series on top of mine, which is in turn
>>>> based on latest media master, where this series do not apply as-is
>>>> afaict.
>>>>
>>>> I have added to Hugues' patches my reviewed-by and tested-by tags.
>>>> If you prefer to I can send you a pull request, or if you want to have
>>>> a chance to review the whole patch list please refer to the above
>>>> branch.
>>>>
>>>> Let me know if I can help speeding up the inclusion of these two
>>>> series as they fix two real issues of MIPI CSI-2 capture operations
>>>> for this sensor.
>>>
>>> I've pushed the patches here:
>>>
>>> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=for-4.20-5>
>>>
>>> There was a merge commit and a few extra patches in your tree; I threw them
>>> out. :-)
>>
>> Yeah, those are a few patches I need for my testing platform... Forgot to
>> remove them, hope you didn't spend too much time on this.
>
> No, it was rather easy to remove them. I've sent a pull request on these:
>
> <URL:https://patchwork.linuxtv.org/patch/52091/>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 0/5] Fix OV5640 exposure & gain
2018-09-11 13:48 [PATCH v3 0/5] Fix OV5640 exposure & gain Hugues Fruchet
` (6 preceding siblings ...)
2018-09-14 16:07 ` jacopo mondi
@ 2018-09-14 17:49 ` Steve Longerbeam
2018-09-14 18:25 ` Nicolas Dufresne
8 siblings, 0 replies; 14+ messages in thread
From: Steve Longerbeam @ 2018-09-14 17:49 UTC (permalink / raw)
To: Hugues Fruchet, Steve Longerbeam, Sakari Ailus, Jacopo Mondi,
Hans Verkuil, Mauro Carvalho Chehab
Cc: linux-media, Benjamin Gaignard
Hi Hughes,
The whole series,
Acked-by: Steve Longerbeam <steve_longerbeam@mentor.com>
and
Tested-by: Steve Longerbeam <steve_longerbeam@mentor.com>
on i.MX6q SabreSD with MIPI CSI-2 OV5640 module
On 09/11/2018 06:48 AM, Hugues Fruchet wrote:
> This patch serie fixes some problems around exposure & gain in OV5640 driver.
>
> The 4th patch about autocontrols requires also a fix in v4l2-ctrls.c:
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg133164.html
>
> Here is the test procedure used for exposure & gain controls check:
> 1) Preview in background
> $> gst-launch-1.0 v4l2src ! "video/x-raw, width=640, Height=480" ! queue ! waylandsink -e &
> 2) Check gain & exposure values
> $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)"
> exposure (int) : min=0 max=65535 step=1 default=0 value=330 flags=inactive, volatile
> gain (int) : min=0 max=1023 step=1 default=0 value=19 flags=inactive, volatile
> 3) Put finger in front of camera and check that gain/exposure values are changing:
> $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)"
> exposure (int) : min=0 max=65535 step=1 default=0 value=660 flags=inactive, volatile
> gain (int) : min=0 max=1023 step=1 default=0 value=37 flags=inactive, volatile
> 4) switch to manual mode, image exposition must not change
> $> v4l2-ctl --set-ctrl=gain_automatic=0
> $> v4l2-ctl --set-ctrl=auto_exposure=1
> Note the "1" for manual exposure.
>
> 5) Check current gain/exposure values:
> $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)"
> exposure (int) : min=0 max=65535 step=1 default=0 value=330
> gain (int) : min=0 max=1023 step=1 default=0 value=20
>
> 6) Put finger behind camera and check that gain/exposure values are NOT changing:
> $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)"
> exposure (int) : min=0 max=65535 step=1 default=0 value=330
> gain (int) : min=0 max=1023 step=1 default=0 value=20
> 7) Update exposure, check that it is well changed on display and that same value is returned:
> $> v4l2-ctl --set-ctrl=exposure=100
> $> v4l2-ctl --get-ctrl=exposure
> exposure: 100
>
> 9) Update gain, check that it is well changed on display and that same value is returned:
> $> v4l2-ctl --set-ctrl=gain=10
> $> v4l2-ctl --get-ctrl=gain
> gain: 10
>
> 10) Switch back to auto gain/exposure, verify that image is correct and values returned are correct:
> $> v4l2-ctl --set-ctrl=gain_automatic=1
> $> v4l2-ctl --set-ctrl=auto_exposure=0
> $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)"
> exposure (int) : min=0 max=65535 step=1 default=0 value=330 flags=inactive, volatile
> gain (int) : min=0 max=1023 step=1 default=0 value=22 flags=inactive, volatile
> Note the "0" for auto exposure.
>
> ===========
> = history =
> ===========
> version 3:
> - Change patch 5/5 by removing set_mode() orig_mode parameter as per jacopo' suggestion:
> https://www.spinics.net/lists/linux-media/msg139457.html
>
> version 2:
> - Fix patch 3/5 commit comment and rename binning function as per jacopo' suggestion:
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg133272.html
>
> Hugues Fruchet (5):
> media: ov5640: fix exposure regression
> media: ov5640: fix auto gain & exposure when changing mode
> media: ov5640: fix wrong binning value in exposure calculation
> media: ov5640: fix auto controls values when switching to manual mode
> media: ov5640: fix restore of last mode set
>
> drivers/media/i2c/ov5640.c | 128 ++++++++++++++++++++++++++-------------------
> 1 file changed, 73 insertions(+), 55 deletions(-)
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v3 0/5] Fix OV5640 exposure & gain
2018-09-11 13:48 [PATCH v3 0/5] Fix OV5640 exposure & gain Hugues Fruchet
` (7 preceding siblings ...)
2018-09-14 17:49 ` Steve Longerbeam
@ 2018-09-14 18:25 ` Nicolas Dufresne
8 siblings, 0 replies; 14+ messages in thread
From: Nicolas Dufresne @ 2018-09-14 18:25 UTC (permalink / raw)
To: Hugues Fruchet, Steve Longerbeam, Sakari Ailus, Jacopo Mondi,
Hans Verkuil, Mauro Carvalho Chehab
Cc: linux-media, Benjamin Gaignard
[-- Attachment #1: Type: text/plain, Size: 4007 bytes --]
Interesting, I just hit this problem yesterday. Same module as Steve,
with MIPI CSI-2 OV5640 (on Sabre Lite).
Tested-By: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Le mardi 11 septembre 2018 à 15:48 +0200, Hugues Fruchet a écrit :
> This patch serie fixes some problems around exposure & gain in OV5640
> driver.
>
> The 4th patch about autocontrols requires also a fix in v4l2-ctrls.c:
>
https://www.mail-archive.com/linux-media@vger.kernel.org/msg133164.html
>
> Here is the test procedure used for exposure & gain controls check:
> 1) Preview in background
> $> gst-launch-1.0 v4l2src ! "video/x-raw, width=640, Height=480" !
> queue ! waylandsink -e &
> 2) Check gain & exposure values
> $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)"
> exposure (int) : min=0 max=65535 step=1
> default=0 value=330 flags=inactive, volatile
> gain (int) : min=0 max=1023 step=1
> default=0 value=19 flags=inactive, volatile
> 3) Put finger in front of camera and check that gain/exposure values
> are changing:
> $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)"
> exposure (int) : min=0 max=65535 step=1
> default=0 value=660 flags=inactive, volatile
> gain (int) : min=0 max=1023 step=1
> default=0 value=37 flags=inactive, volatile
> 4) switch to manual mode, image exposition must not change
> $> v4l2-ctl --set-ctrl=gain_automatic=0
> $> v4l2-ctl --set-ctrl=auto_exposure=1
> Note the "1" for manual exposure.
>
> 5) Check current gain/exposure values:
> $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)"
> exposure (int) : min=0 max=65535 step=1
> default=0 value=330
> gain (int) : min=0 max=1023 step=1
> default=0 value=20
>
> 6) Put finger behind camera and check that gain/exposure values are
> NOT changing:
> $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)"
> exposure (int) : min=0 max=65535 step=1
> default=0 value=330
> gain (int) : min=0 max=1023 step=1
> default=0 value=20
> 7) Update exposure, check that it is well changed on display and that
> same value is returned:
> $> v4l2-ctl --set-ctrl=exposure=100
> $> v4l2-ctl --get-ctrl=exposure
> exposure: 100
>
> 9) Update gain, check that it is well changed on display and that
> same value is returned:
> $> v4l2-ctl --set-ctrl=gain=10
> $> v4l2-ctl --get-ctrl=gain
> gain: 10
>
> 10) Switch back to auto gain/exposure, verify that image is correct
> and values returned are correct:
> $> v4l2-ctl --set-ctrl=gain_automatic=1
> $> v4l2-ctl --set-ctrl=auto_exposure=0
> $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)"
> exposure (int) : min=0 max=65535 step=1
> default=0 value=330 flags=inactive, volatile
> gain (int) : min=0 max=1023 step=1
> default=0 value=22 flags=inactive, volatile
> Note the "0" for auto exposure.
>
> ===========
> = history =
> ===========
> version 3:
> - Change patch 5/5 by removing set_mode() orig_mode parameter as
> per jacopo' suggestion:
> https://www.spinics.net/lists/linux-media/msg139457.html
>
> version 2:
> - Fix patch 3/5 commit comment and rename binning function as per
> jacopo' suggestion:
>
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg133272.html
>
> Hugues Fruchet (5):
> media: ov5640: fix exposure regression
> media: ov5640: fix auto gain & exposure when changing mode
> media: ov5640: fix wrong binning value in exposure calculation
> media: ov5640: fix auto controls values when switching to manual
> mode
> media: ov5640: fix restore of last mode set
>
> drivers/media/i2c/ov5640.c | 128 ++++++++++++++++++++++++++---------
> ----------
> 1 file changed, 73 insertions(+), 55 deletions(-)
>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread