Hi Hans,
On Wed, Jun 07, 2023 at 06:46:45PM +0200, Hans de Goede wrote:
> Quoting the OV2680 datasheet:
>
> "3.2 exposure and gain control
>
> In the OV2680, the exposure time and gain are set manually from an external
> controller. The OV2680 supports manual gain and exposure control only for
> normal applications, no auto mode."
>
> And indeed testing with the atomisp_ov2680 fork of ov2680.c has shown that
> auto-exposure and auto-gain do not work.
>
> Note that the code setting the auto-exposure flag was broken, callers
> of ov2680_exposure_set() were directly passing !!ctrls->auto_exp->val as
> "bool auto_exp" value, but ctrls->auto_exp is a menu control with:
>
> enum v4l2_exposure_auto_type {
> V4L2_EXPOSURE_AUTO = 0,
> V4L2_EXPOSURE_MANUAL = 1,
> ...
>
> So instead of passing !!ctrls->auto_exp->val they should have been passing
> ctrls->auto_exp->val == V4L2_EXPOSURE_AUTO, iow the passed value was
> inverted of what it should have been.
>
> Also remove ov2680_g_volatile_ctrl() since without auto support the gain
> and exposure controls are not volatile.
>
> This also fixes the control values not being properly applied in
> ov2680_mode_set(). The 800x600 mode register-list also sets gain,
> exposure and vflip overriding the last set ctrl values.
>
> ov2680_mode_set() does call ov2680_gain_set() and ov2680_exposure_set()
> but did this before writing the mode register-list, so these values
> would still be overridden by the mode register-list.
>
> Add a v4l2_ctrl_handler_setup() call after writing the mode register-list
> to restore all ctrl values. Also remove the ctrls->gain->is_new check from
> ov2680_gain_set() so that the gain always gets restored properly.
>
> Last since ov2680_mode_set() now calls v4l2_ctrl_handler_setup(), remove
> the v4l2_ctrl_handler_setup() call after ov2680_mode_restore() since
> ov2680_mode_restore() calls ov2680_mode_set().
>
> Fixes: 3ee47cad3e69 ("media: ov2680: Add Omnivision OV2680 sensor driver")
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
It is interesting, to say at least, to see this and the few following
patches. No-one has previously complained. Makes me also wonder how the
original driver was tested.
Could you also cc the original author who is also listed as the current
maintainter of the driver for v2?
--
Kind regards,
Sakari Ailus
