Hi Hugues,
 one more comment on this patch..

On Mon, Aug 13, 2018 at 12:19:46PM +0200, Hugues Fruchet wrote:
> 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.fruc...@st.com>
> ---
>  drivers/media/i2c/ov5640.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index c110a6a..923cc30 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;
>
> @@ -1628,6 +1629,9 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
>       bool auto_exp =  sensor->ctrls.auto_exp->val == V4L2_EXPOSURE_AUTO;
>       int ret;
>
> +     if (!orig_mode)
> +             orig_mode = mode;
> +

Am I wrong or with the introduction of last_mode we could drop the
'orig_mode' parameter (which has confused me already :/ ) from the
set_mode() function?

Just set here 'orig_mode = sensor->last_mode' and make sure last_mode
is intialized properly at probe time...

Or is there some other value in keeping the orig_mode parameter here?

Thanks
   j

>       dn_mode = mode->dn_mode;
>       orig_dn_mode = orig_mode->dn_mode;
>
> @@ -1688,6 +1692,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
>               return ret;
>
>       sensor->pending_mode_change = false;
> +     sensor->last_mode = mode;
>
>       return 0;
>
> @@ -2551,7 +2556,8 @@ 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, sensor->last_mode);
> +
>                       if (ret)
>                               goto out;
>
> --
> 2.7.4
>

Attachment: signature.asc
Description: PGP signature

Reply via email to