Re: [PATCH v3 0/5] Fix OV5640 exposure & gain

[jacopo mondi <jacopo@jmondi.org>]

[-- 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 --]