[PATCH] media: ov5640: fix vblank unchange issue when work at dvp mode

[PATCH] media: ov5640: fix vblank unchange issue when work at dvp mode

[guoniu.zhou]

From: "Guoniu.zhou" <guoniu.zhou@nxp.com>

The value of V4L2_CID_VBLANK control is initialized to default vblank
value of 640x480 when driver probe. When OV5640 work at DVP mode, the
control value won't update and lead to sensor can't output data if the
resolution remain the same as last time since incorrect total vertical
size. So update it when there is a new value applied.

Fixes: bce93b827de6 ("media: ov5640: Add VBLANK control")
Signed-off-by: Guoniu.zhou <guoniu.zhou@nxp.com>
---
 drivers/media/i2c/ov5640.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 36b509714c8c..2f742f5f95fd 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -2854,12 +2854,22 @@ static int ov5640_try_fmt_internal(struct v4l2_subdev *sd,
 	return 0;
 }
 
+static void __v4l2_ctrl_vblank_update(struct ov5640_dev *sensor, u32 vblank)
+{
+	const struct ov5640_mode_info *mode = sensor->current_mode;
+
+	__v4l2_ctrl_modify_range(sensor->ctrls.vblank, OV5640_MIN_VBLANK,
+				 OV5640_MAX_VTS - mode->height, 1, vblank);
+
+	__v4l2_ctrl_s_ctrl(sensor->ctrls.vblank, vblank);
+}
+
 static int ov5640_update_pixel_rate(struct ov5640_dev *sensor)
 {
 	const struct ov5640_mode_info *mode = sensor->current_mode;
 	enum ov5640_pixel_rate_id pixel_rate_id = mode->pixel_rate;
 	struct v4l2_mbus_framefmt *fmt = &sensor->fmt;
-	const struct ov5640_timings *timings;
+	const struct ov5640_timings *timings = ov5640_timings(sensor, mode);
 	s32 exposure_val, exposure_max;
 	unsigned int hblank;
 	unsigned int i = 0;
@@ -2878,6 +2888,8 @@ static int ov5640_update_pixel_rate(struct ov5640_dev *sensor)
 		__v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate,
 					 ov5640_calc_pixel_rate(sensor));
 
+		__v4l2_ctrl_vblank_update(sensor, timings->vblank_def);
+
 		return 0;
 	}
 
@@ -2920,15 +2932,12 @@ static int ov5640_update_pixel_rate(struct ov5640_dev *sensor)
 	__v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate, pixel_rate);
 	__v4l2_ctrl_s_ctrl(sensor->ctrls.link_freq, i);
 
-	timings = ov5640_timings(sensor, mode);
 	hblank = timings->htot - mode->width;
 	__v4l2_ctrl_modify_range(sensor->ctrls.hblank,
 				 hblank, hblank, 1, hblank);
 
 	vblank = timings->vblank_def;
-	__v4l2_ctrl_modify_range(sensor->ctrls.vblank, OV5640_MIN_VBLANK,
-				 OV5640_MAX_VTS - mode->height, 1, vblank);
-	__v4l2_ctrl_s_ctrl(sensor->ctrls.vblank, vblank);
+	__v4l2_ctrl_vblank_update(sensor, vblank);
 
 	exposure_max = timings->crop.height + vblank - 4;
 	exposure_val = clamp_t(s32, sensor->ctrls.exposure->val,
-- 
2.37.1

Re: [PATCH] media: ov5640: fix vblank unchange issue when work at dvp mode

[Alain Volmat]

Hi Guoniu,

I came up to the same conclusion about wrong vblank when trying to make
the OV5640 work in DVP mode so I agree about this modification.

However I think other ctrls also have the same issue, at least
exposure.  I am wondering if we should keep the splitted code as
currently and add back the missing ctrl in the DVP mode path or
rework code to apply ctrl in both modes ?
Basically link_freq / pixelrate handling differ between DVP and MIPI
but it should be same handling for other ctrls I think.

Regards,
Alain

On Wed, Jul 19, 2023 at 03:30:12PM +0800, guoniu.zhou@oss.nxp.com wrote:
> From: "Guoniu.zhou" <guoniu.zhou@nxp.com>
> 
> The value of V4L2_CID_VBLANK control is initialized to default vblank
> value of 640x480 when driver probe. When OV5640 work at DVP mode, the
> control value won't update and lead to sensor can't output data if the
> resolution remain the same as last time since incorrect total vertical
> size. So update it when there is a new value applied.
> 
> Fixes: bce93b827de6 ("media: ov5640: Add VBLANK control")
> Signed-off-by: Guoniu.zhou <guoniu.zhou@nxp.com>
> ---
>  drivers/media/i2c/ov5640.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 36b509714c8c..2f742f5f95fd 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -2854,12 +2854,22 @@ static int ov5640_try_fmt_internal(struct v4l2_subdev *sd,
>  	return 0;
>  }
>  
> +static void __v4l2_ctrl_vblank_update(struct ov5640_dev *sensor, u32 vblank)
> +{
> +	const struct ov5640_mode_info *mode = sensor->current_mode;
> +
> +	__v4l2_ctrl_modify_range(sensor->ctrls.vblank, OV5640_MIN_VBLANK,
> +				 OV5640_MAX_VTS - mode->height, 1, vblank);
> +
> +	__v4l2_ctrl_s_ctrl(sensor->ctrls.vblank, vblank);
> +}
> +
>  static int ov5640_update_pixel_rate(struct ov5640_dev *sensor)
>  {
>  	const struct ov5640_mode_info *mode = sensor->current_mode;
>  	enum ov5640_pixel_rate_id pixel_rate_id = mode->pixel_rate;
>  	struct v4l2_mbus_framefmt *fmt = &sensor->fmt;
> -	const struct ov5640_timings *timings;
> +	const struct ov5640_timings *timings = ov5640_timings(sensor, mode);
>  	s32 exposure_val, exposure_max;
>  	unsigned int hblank;
>  	unsigned int i = 0;
> @@ -2878,6 +2888,8 @@ static int ov5640_update_pixel_rate(struct ov5640_dev *sensor)
>  		__v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate,
>  					 ov5640_calc_pixel_rate(sensor));
>  
> +		__v4l2_ctrl_vblank_update(sensor, timings->vblank_def);
> +
>  		return 0;
>  	}
>  
> @@ -2920,15 +2932,12 @@ static int ov5640_update_pixel_rate(struct ov5640_dev *sensor)
>  	__v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate, pixel_rate);
>  	__v4l2_ctrl_s_ctrl(sensor->ctrls.link_freq, i);
>  
> -	timings = ov5640_timings(sensor, mode);
>  	hblank = timings->htot - mode->width;
>  	__v4l2_ctrl_modify_range(sensor->ctrls.hblank,
>  				 hblank, hblank, 1, hblank);
>  
>  	vblank = timings->vblank_def;
> -	__v4l2_ctrl_modify_range(sensor->ctrls.vblank, OV5640_MIN_VBLANK,
> -				 OV5640_MAX_VTS - mode->height, 1, vblank);
> -	__v4l2_ctrl_s_ctrl(sensor->ctrls.vblank, vblank);
> +	__v4l2_ctrl_vblank_update(sensor, vblank);
>  
>  	exposure_max = timings->crop.height + vblank - 4;
>  	exposure_val = clamp_t(s32, sensor->ctrls.exposure->val,
> -- 
> 2.37.1
> 

RE: [PATCH] media: ov5640: fix vblank unchange issue when work at dvp mode

[G.N. Zhou (OSS)]

Hi Alain,

> -----Original Message-----
> From: Alain Volmat <alain.volmat@foss.st.com>
> Sent: 2023年7月19日 19:46
> To: G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.com>
> Cc: linux-media@vger.kernel.org; mchehab@kernel.org;
> slongerbeam@gmail.com; sakari.ailus@linux.intel.com;
> jacopo.mondi@ideasonboard.com; laurent.pinchart@ideasonboard.com
> Subject: Re: [PATCH] media: ov5640: fix vblank unchange issue when work at
> dvp mode
> 
> Caution: This is an external email. Please take care when clicking links or opening
> attachments. When in doubt, report the message using the 'Report this email'
> button
> 
> 
> Hi Guoniu,
> 
> I came up to the same conclusion about wrong vblank when trying to make the
> OV5640 work in DVP mode so I agree about this modification.
> 
> However I think other ctrls also have the same issue, at least exposure.  I am
> wondering if we should keep the splitted code as currently and add back the
> missing ctrl in the DVP mode path or rework code to apply ctrl in both modes ?
> Basically link_freq / pixelrate handling differ between DVP and MIPI but it should
> be same handling for other ctrls I think.

As you know, the patch is for VBLANK control added in " bce93b827de6 ("media: ov5640: Add VBLANK control")" and I prefer and follow "one patch do one thing" rule.

> 
> Regards,
> Alain
> 
> On Wed, Jul 19, 2023 at 03:30:12PM +0800, guoniu.zhou@oss.nxp.com wrote:
> > From: "Guoniu.zhou" <guoniu.zhou@nxp.com>
> >
> > The value of V4L2_CID_VBLANK control is initialized to default vblank
> > value of 640x480 when driver probe. When OV5640 work at DVP mode, the
> > control value won't update and lead to sensor can't output data if the
> > resolution remain the same as last time since incorrect total vertical
> > size. So update it when there is a new value applied.
> >
> > Fixes: bce93b827de6 ("media: ov5640: Add VBLANK control")
> > Signed-off-by: Guoniu.zhou <guoniu.zhou@nxp.com>
> > ---
> >  drivers/media/i2c/ov5640.c | 19 ++++++++++++++-----
> >  1 file changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> > index 36b509714c8c..2f742f5f95fd 100644
> > --- a/drivers/media/i2c/ov5640.c
> > +++ b/drivers/media/i2c/ov5640.c
> > @@ -2854,12 +2854,22 @@ static int ov5640_try_fmt_internal(struct
> v4l2_subdev *sd,
> >       return 0;
> >  }
> >
> > +static void __v4l2_ctrl_vblank_update(struct ov5640_dev *sensor, u32
> > +vblank) {
> > +     const struct ov5640_mode_info *mode = sensor->current_mode;
> > +
> > +     __v4l2_ctrl_modify_range(sensor->ctrls.vblank,
> OV5640_MIN_VBLANK,
> > +                              OV5640_MAX_VTS - mode->height, 1,
> > + vblank);
> > +
> > +     __v4l2_ctrl_s_ctrl(sensor->ctrls.vblank, vblank); }
> > +
> >  static int ov5640_update_pixel_rate(struct ov5640_dev *sensor)  {
> >       const struct ov5640_mode_info *mode = sensor->current_mode;
> >       enum ov5640_pixel_rate_id pixel_rate_id = mode->pixel_rate;
> >       struct v4l2_mbus_framefmt *fmt = &sensor->fmt;
> > -     const struct ov5640_timings *timings;
> > +     const struct ov5640_timings *timings = ov5640_timings(sensor,
> > + mode);
> >       s32 exposure_val, exposure_max;
> >       unsigned int hblank;
> >       unsigned int i = 0;
> > @@ -2878,6 +2888,8 @@ static int ov5640_update_pixel_rate(struct
> ov5640_dev *sensor)
> >               __v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate,
> >
> > ov5640_calc_pixel_rate(sensor));
> >
> > +             __v4l2_ctrl_vblank_update(sensor, timings->vblank_def);
> > +
> >               return 0;
> >       }
> >
> > @@ -2920,15 +2932,12 @@ static int ov5640_update_pixel_rate(struct
> ov5640_dev *sensor)
> >       __v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate, pixel_rate);
> >       __v4l2_ctrl_s_ctrl(sensor->ctrls.link_freq, i);
> >
> > -     timings = ov5640_timings(sensor, mode);
> >       hblank = timings->htot - mode->width;
> >       __v4l2_ctrl_modify_range(sensor->ctrls.hblank,
> >                                hblank, hblank, 1, hblank);
> >
> >       vblank = timings->vblank_def;
> > -     __v4l2_ctrl_modify_range(sensor->ctrls.vblank,
> OV5640_MIN_VBLANK,
> > -                              OV5640_MAX_VTS - mode->height, 1,
> vblank);
> > -     __v4l2_ctrl_s_ctrl(sensor->ctrls.vblank, vblank);
> > +     __v4l2_ctrl_vblank_update(sensor, vblank);
> >
> >       exposure_max = timings->crop.height + vblank - 4;
> >       exposure_val = clamp_t(s32, sensor->ctrls.exposure->val,
> > --
> > 2.37.1
> >

Re: [PATCH] media: ov5640: fix vblank unchange issue when work at dvp mode

[Alain Volmat]

Hi Guoniu,

On Thu, Jul 20, 2023 at 03:27:20AM +0000, G.N. Zhou (OSS) wrote:
> Hi Alain,
> 
> > -----Original Message-----
> > From: Alain Volmat <alain.volmat@foss.st.com>
> > Sent: 2023年7月19日 19:46
> > To: G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.com>
> > Cc: linux-media@vger.kernel.org; mchehab@kernel.org;
> > slongerbeam@gmail.com; sakari.ailus@linux.intel.com;
> > jacopo.mondi@ideasonboard.com; laurent.pinchart@ideasonboard.com
> > Subject: Re: [PATCH] media: ov5640: fix vblank unchange issue when work at
> > dvp mode
> > 
> > Caution: This is an external email. Please take care when clicking links or opening
> > attachments. When in doubt, report the message using the 'Report this email'
> > button
> > 
> > 
> > Hi Guoniu,
> > 
> > I came up to the same conclusion about wrong vblank when trying to make the
> > OV5640 work in DVP mode so I agree about this modification.
> > 
> > However I think other ctrls also have the same issue, at least exposure.  I am
> > wondering if we should keep the splitted code as currently and add back the
> > missing ctrl in the DVP mode path or rework code to apply ctrl in both modes ?
> > Basically link_freq / pixelrate handling differ between DVP and MIPI but it should
> > be same handling for other ctrls I think.
> 
> As you know, the patch is for VBLANK control added in " bce93b827de6 ("media: ov5640: Add VBLANK control")" and I prefer and follow "one patch do one thing" rule.

The exposure control has to be updated following a VBLANK update.
This is explained in the commit you are fixing.  So
I think that your fix should not only add the update of the vblank
but also update the exposure value.  You can have a look at the
commit bce93b827de6 ("media: ov5640: Add VBLANK control") especially the
update part in the ov5640_update_pixel_rate function.

While it might not be the most beautiful way to do it, probably a simple
goto and a label could also do the trick.

        if (!ov5640_is_csi2(sensor)) {
                __v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate,
                                         ov5640_calc_pixel_rate(sensor));

		goto update_ctrls;
        }

(...)

update_ctrls:
        vblank = timings->vblank_def;
        __v4l2_ctrl_modify_range(sensor->ctrls.vblank, OV5640_MIN_VBLANK,
                                 OV5640_MAX_VTS - mode->height, 1, vblank);
        __v4l2_ctrl_s_ctrl(sensor->ctrls.vblank, vblank);

        exposure_max = timings->crop.height + vblank - 4;
        exposure_val = clamp_t(s32, sensor->ctrls.exposure->val,
                               sensor->ctrls.exposure->minimum,
                               exposure_max);

(...)

Regards,
Alain

> 
> > 
> > Regards,
> > Alain
> > 
> > On Wed, Jul 19, 2023 at 03:30:12PM +0800, guoniu.zhou@oss.nxp.com wrote:
> > > From: "Guoniu.zhou" <guoniu.zhou@nxp.com>
> > >
> > > The value of V4L2_CID_VBLANK control is initialized to default vblank
> > > value of 640x480 when driver probe. When OV5640 work at DVP mode, the
> > > control value won't update and lead to sensor can't output data if the
> > > resolution remain the same as last time since incorrect total vertical
> > > size. So update it when there is a new value applied.
> > >
> > > Fixes: bce93b827de6 ("media: ov5640: Add VBLANK control")
> > > Signed-off-by: Guoniu.zhou <guoniu.zhou@nxp.com>
> > > ---
> > >  drivers/media/i2c/ov5640.c | 19 ++++++++++++++-----
> > >  1 file changed, 14 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> > > index 36b509714c8c..2f742f5f95fd 100644
> > > --- a/drivers/media/i2c/ov5640.c
> > > +++ b/drivers/media/i2c/ov5640.c
> > > @@ -2854,12 +2854,22 @@ static int ov5640_try_fmt_internal(struct
> > v4l2_subdev *sd,
> > >       return 0;
> > >  }
> > >
> > > +static void __v4l2_ctrl_vblank_update(struct ov5640_dev *sensor, u32
> > > +vblank) {
> > > +     const struct ov5640_mode_info *mode = sensor->current_mode;
> > > +
> > > +     __v4l2_ctrl_modify_range(sensor->ctrls.vblank,
> > OV5640_MIN_VBLANK,
> > > +                              OV5640_MAX_VTS - mode->height, 1,
> > > + vblank);
> > > +
> > > +     __v4l2_ctrl_s_ctrl(sensor->ctrls.vblank, vblank); }
> > > +
> > >  static int ov5640_update_pixel_rate(struct ov5640_dev *sensor)  {
> > >       const struct ov5640_mode_info *mode = sensor->current_mode;
> > >       enum ov5640_pixel_rate_id pixel_rate_id = mode->pixel_rate;
> > >       struct v4l2_mbus_framefmt *fmt = &sensor->fmt;
> > > -     const struct ov5640_timings *timings;
> > > +     const struct ov5640_timings *timings = ov5640_timings(sensor,
> > > + mode);
> > >       s32 exposure_val, exposure_max;
> > >       unsigned int hblank;
> > >       unsigned int i = 0;
> > > @@ -2878,6 +2888,8 @@ static int ov5640_update_pixel_rate(struct
> > ov5640_dev *sensor)
> > >               __v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate,
> > >
> > > ov5640_calc_pixel_rate(sensor));
> > >
> > > +             __v4l2_ctrl_vblank_update(sensor, timings->vblank_def);
> > > +
> > >               return 0;
> > >       }
> > >
> > > @@ -2920,15 +2932,12 @@ static int ov5640_update_pixel_rate(struct
> > ov5640_dev *sensor)
> > >       __v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate, pixel_rate);
> > >       __v4l2_ctrl_s_ctrl(sensor->ctrls.link_freq, i);
> > >
> > > -     timings = ov5640_timings(sensor, mode);
> > >       hblank = timings->htot - mode->width;
> > >       __v4l2_ctrl_modify_range(sensor->ctrls.hblank,
> > >                                hblank, hblank, 1, hblank);
> > >
> > >       vblank = timings->vblank_def;
> > > -     __v4l2_ctrl_modify_range(sensor->ctrls.vblank,
> > OV5640_MIN_VBLANK,
> > > -                              OV5640_MAX_VTS - mode->height, 1,
> > vblank);
> > > -     __v4l2_ctrl_s_ctrl(sensor->ctrls.vblank, vblank);
> > > +     __v4l2_ctrl_vblank_update(sensor, vblank);
> > >
> > >       exposure_max = timings->crop.height + vblank - 4;
> > >       exposure_val = clamp_t(s32, sensor->ctrls.exposure->val,
> > > --
> > > 2.37.1
> > >

RE: [PATCH] media: ov5640: fix vblank unchange issue when work at dvp mode

[G.N. Zhou (OSS)]

Hi Alain,

> -----Original Message-----
> From: Alain Volmat <alain.volmat@foss.st.com>
> Sent: 2023年7月20日 16:02
> To: G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.com>
> Cc: linux-media@vger.kernel.org; mchehab@kernel.org;
> slongerbeam@gmail.com; sakari.ailus@linux.intel.com;
> jacopo.mondi@ideasonboard.com; laurent.pinchart@ideasonboard.com
> Subject: Re: [PATCH] media: ov5640: fix vblank unchange issue when work at
> dvp mode
> 
> Caution: This is an external email. Please take care when clicking links or opening
> attachments. When in doubt, report the message using the 'Report this email'
> button
> 
> 
> Hi Guoniu,
> 
> On Thu, Jul 20, 2023 at 03:27:20AM +0000, G.N. Zhou (OSS) wrote:
> > Hi Alain,
> >
> > > -----Original Message-----
> > > From: Alain Volmat <alain.volmat@foss.st.com>
> > > Sent: 2023年7月19日 19:46
> > > To: G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.com>
> > > Cc: linux-media@vger.kernel.org; mchehab@kernel.org;
> > > slongerbeam@gmail.com; sakari.ailus@linux.intel.com;
> > > jacopo.mondi@ideasonboard.com; laurent.pinchart@ideasonboard.com
> > > Subject: Re: [PATCH] media: ov5640: fix vblank unchange issue when
> > > work at dvp mode
> > >
> > > Caution: This is an external email. Please take care when clicking
> > > links or opening attachments. When in doubt, report the message using the
> 'Report this email'
> > > button
> > >
> > >
> > > Hi Guoniu,
> > >
> > > I came up to the same conclusion about wrong vblank when trying to
> > > make the
> > > OV5640 work in DVP mode so I agree about this modification.
> > >
> > > However I think other ctrls also have the same issue, at least
> > > exposure.  I am wondering if we should keep the splitted code as
> > > currently and add back the missing ctrl in the DVP mode path or rework code
> to apply ctrl in both modes ?
> > > Basically link_freq / pixelrate handling differ between DVP and MIPI
> > > but it should be same handling for other ctrls I think.
> >
> > As you know, the patch is for VBLANK control added in " bce93b827de6
> ("media: ov5640: Add VBLANK control")" and I prefer and follow "one patch do
> one thing" rule.
> 
> The exposure control has to be updated following a VBLANK update.
> This is explained in the commit you are fixing.  So I think that your fix should not
> only add the update of the vblank but also update the exposure value.  You can
> have a look at the commit bce93b827de6 ("media: ov5640: Add VBLANK
> control") especially the update part in the ov5640_update_pixel_rate function.
> 
> While it might not be the most beautiful way to do it, probably a simple goto and
> a label could also do the trick.

When update VBLANK control value, actually, it does.

__v4l2_ctrl_s_ctrl(sensor->ctrls.vblank, vblank);
   Call ov5640_s_ctrl()
      case V4L2_CID_VBLANK:
          /* Update the exposure range to the newly programmed vblank. */
		  timings = ov5640_timings(sensor, mode);
		  exp_max = mode->height + ctrl->val - 4;
		  __v4l2_ctrl_modify_range(sensor->ctrls.exposure,
                         		sensor->ctrls.exposure->minimum,
                        			exp_max, sensor->ctrls.exposure->step,
                         		timings->vblank_def);

> 
>         if (!ov5640_is_csi2(sensor)) {
>                 __v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate,
> 
> ov5640_calc_pixel_rate(sensor));
> 
>                 goto update_ctrls;
>         }
> 
> (...)
> 
> update_ctrls:
>         vblank = timings->vblank_def;
>         __v4l2_ctrl_modify_range(sensor->ctrls.vblank,
> OV5640_MIN_VBLANK,
>                                  OV5640_MAX_VTS - mode->height, 1,
> vblank);
>         __v4l2_ctrl_s_ctrl(sensor->ctrls.vblank, vblank);
> 
>         exposure_max = timings->crop.height + vblank - 4;
>         exposure_val = clamp_t(s32, sensor->ctrls.exposure->val,
>                                sensor->ctrls.exposure->minimum,
>                                exposure_max);
> 
> (...)
> 
> Regards,
> Alain
> 
> >
> > >
> > > Regards,
> > > Alain
> > >
> > > On Wed, Jul 19, 2023 at 03:30:12PM +0800, guoniu.zhou@oss.nxp.com
> wrote:
> > > > From: "Guoniu.zhou" <guoniu.zhou@nxp.com>
> > > >
> > > > The value of V4L2_CID_VBLANK control is initialized to default
> > > > vblank value of 640x480 when driver probe. When OV5640 work at DVP
> > > > mode, the control value won't update and lead to sensor can't
> > > > output data if the resolution remain the same as last time since
> > > > incorrect total vertical size. So update it when there is a new value applied.
> > > >
> > > > Fixes: bce93b827de6 ("media: ov5640: Add VBLANK control")
> > > > Signed-off-by: Guoniu.zhou <guoniu.zhou@nxp.com>
> > > > ---
> > > >  drivers/media/i2c/ov5640.c | 19 ++++++++++++++-----
> > > >  1 file changed, 14 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/media/i2c/ov5640.c
> > > > b/drivers/media/i2c/ov5640.c index 36b509714c8c..2f742f5f95fd
> > > > 100644
> > > > --- a/drivers/media/i2c/ov5640.c
> > > > +++ b/drivers/media/i2c/ov5640.c
> > > > @@ -2854,12 +2854,22 @@ static int ov5640_try_fmt_internal(struct
> > > v4l2_subdev *sd,
> > > >       return 0;
> > > >  }
> > > >
> > > > +static void __v4l2_ctrl_vblank_update(struct ov5640_dev *sensor,
> > > > +u32
> > > > +vblank) {
> > > > +     const struct ov5640_mode_info *mode = sensor->current_mode;
> > > > +
> > > > +     __v4l2_ctrl_modify_range(sensor->ctrls.vblank,
> > > OV5640_MIN_VBLANK,
> > > > +                              OV5640_MAX_VTS - mode->height,
> 1,
> > > > + vblank);
> > > > +
> > > > +     __v4l2_ctrl_s_ctrl(sensor->ctrls.vblank, vblank); }
> > > > +
> > > >  static int ov5640_update_pixel_rate(struct ov5640_dev *sensor)  {
> > > >       const struct ov5640_mode_info *mode = sensor->current_mode;
> > > >       enum ov5640_pixel_rate_id pixel_rate_id = mode->pixel_rate;
> > > >       struct v4l2_mbus_framefmt *fmt = &sensor->fmt;
> > > > -     const struct ov5640_timings *timings;
> > > > +     const struct ov5640_timings *timings =
> > > > + ov5640_timings(sensor, mode);
> > > >       s32 exposure_val, exposure_max;
> > > >       unsigned int hblank;
> > > >       unsigned int i = 0;
> > > > @@ -2878,6 +2888,8 @@ static int ov5640_update_pixel_rate(struct
> > > ov5640_dev *sensor)
> > > >               __v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate,
> > > >
> > > > ov5640_calc_pixel_rate(sensor));
> > > >
> > > > +             __v4l2_ctrl_vblank_update(sensor,
> > > > + timings->vblank_def);
> > > > +
> > > >               return 0;
> > > >       }
> > > >
> > > > @@ -2920,15 +2932,12 @@ static int ov5640_update_pixel_rate(struct
> > > ov5640_dev *sensor)
> > > >       __v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate, pixel_rate);
> > > >       __v4l2_ctrl_s_ctrl(sensor->ctrls.link_freq, i);
> > > >
> > > > -     timings = ov5640_timings(sensor, mode);
> > > >       hblank = timings->htot - mode->width;
> > > >       __v4l2_ctrl_modify_range(sensor->ctrls.hblank,
> > > >                                hblank, hblank, 1, hblank);
> > > >
> > > >       vblank = timings->vblank_def;
> > > > -     __v4l2_ctrl_modify_range(sensor->ctrls.vblank,
> > > OV5640_MIN_VBLANK,
> > > > -                              OV5640_MAX_VTS - mode->height, 1,
> > > vblank);
> > > > -     __v4l2_ctrl_s_ctrl(sensor->ctrls.vblank, vblank);
> > > > +     __v4l2_ctrl_vblank_update(sensor, vblank);
> > > >
> > > >       exposure_max = timings->crop.height + vblank - 4;
> > > >       exposure_val = clamp_t(s32, sensor->ctrls.exposure->val,
> > > > --
> > > > 2.37.1
> > > >