diff mbox series

[v2] media: ov5640: fix incorrect frame frate issue for defulat VGA

Message ID 20230505020114.764715-1-guoniu.zhou@oss.nxp.com (mailing list archive)
State New, archived
Headers show
Series [v2] media: ov5640: fix incorrect frame frate issue for defulat VGA | expand

Commit Message

G.N. Zhou (OSS) May 5, 2023, 2:01 a.m. UTC
From: "Guoniu.zhou" <guoniu.zhou@nxp.com>

If run OV5640 with default setting after power up, the real frame
rate for it is 60, not 30. The reason is default frame interval
parameter initialized in probe is 30 but default link frequency
is to generate 60 frame rate, so correct it.

Signed-off-by: Guoniu.zhou <guoniu.zhou@nxp.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
v1->v2:
  1) fix typo issue(s/runn/run)
  2) keep original OV5640 default link frequency
  3) correct comments and frame_interval parameters to match actual
     frame rate
---
 drivers/media/i2c/ov5640.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Jacopo Mondi May 5, 2023, 6:45 a.m. UTC | #1
Hello Guoniu Zhou

On Fri, May 05, 2023 at 10:01:14AM +0800, G.N. Zhou (OSS) wrote:
> From: "Guoniu.zhou" <guoniu.zhou@nxp.com>
>
> If run OV5640 with default setting after power up, the real frame
> rate for it is 60, not 30. The reason is default frame interval
> parameter initialized in probe is 30 but default link frequency
> is to generate 60 frame rate, so correct it.
>
> Signed-off-by: Guoniu.zhou <guoniu.zhou@nxp.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

I don't recall having given a tag :)

> ---
> v1->v2:
>   1) fix typo issue(s/runn/run)
>   2) keep original OV5640 default link frequency
>   3) correct comments and frame_interval parameters to match actual
>      frame rate
> ---
>  drivers/media/i2c/ov5640.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 1536649b9e90..5c01bb9414c9 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -3851,11 +3851,11 @@ static int ov5640_probe(struct i2c_client *client)
>
>  	/*
>  	 * default init sequence initialize sensor to
> -	 * YUV422 UYVY VGA@30fps
> +	 * YUV422 UYVY VGA@60fps
>  	 */
>  	sensor->frame_interval.numerator = 1;
> -	sensor->frame_interval.denominator = ov5640_framerates[OV5640_30_FPS];
> -	sensor->current_fr = OV5640_30_FPS;
> +	sensor->frame_interval.denominator = ov5640_framerates[OV5640_60_FPS];
> +	sensor->current_fr = OV5640_60_FPS;

If you want to change also the frame_interval setting (which is again
used in parallel mode only) should you also change the .def_fps value
in the ov5640_mode_data[VGA] entry ?

To be honest I would leave parallel on 30FPS, unless you really want
to and have a setup to test.

We could simply update the comment with

+	 * YUV422 UYVY VGA (30FPS in parallel mode, 60 in MIPI CSI-2  mode)

What do you think ?


>  	sensor->current_mode =
>  		&ov5640_mode_data[OV5640_MODE_VGA_640_480];
>  	sensor->last_mode = sensor->current_mode;
> --
> 2.37.1
>
Sakari Ailus May 8, 2023, 9:47 a.m. UTC | #2
On Fri, May 05, 2023 at 08:45:55AM +0200, Jacopo Mondi wrote:
> +	 * YUV422 UYVY VGA (30FPS in parallel mode, 60 in MIPI CSI-2  mode)
> 
> What do you think ?

Please also fix subject (spelling).
G.N. Zhou (OSS) May 9, 2023, 6:23 a.m. UTC | #3
Agree, will update soon, thanks.

Best Regards
G.N Zhou


> -----Original Message-----
> From: Sakari Ailus <sakari.ailus@iki.fi>
> Sent: 2023年5月8日 17:48
> To: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Cc: G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.com>; linux-media@vger.kernel.org;
> mchehab@kernel.org; slongerbeam@gmail.com;
> laurent.pinchart@ideasonboard.com
> Subject: Re: [PATCH v2] media: ov5640: fix incorrect frame frate issue for defulat
> VGA
> 
> 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
> 
> 
> On Fri, May 05, 2023 at 08:45:55AM +0200, Jacopo Mondi wrote:
> > +      * YUV422 UYVY VGA (30FPS in parallel mode, 60 in MIPI CSI-2
> mode)
> >
> > What do you think ?
> 
> Please also fix subject (spelling).
> 
> --
> Sakari Ailus
diff mbox series

Patch

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 1536649b9e90..5c01bb9414c9 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -3851,11 +3851,11 @@  static int ov5640_probe(struct i2c_client *client)
 
 	/*
 	 * default init sequence initialize sensor to
-	 * YUV422 UYVY VGA@30fps
+	 * YUV422 UYVY VGA@60fps
 	 */
 	sensor->frame_interval.numerator = 1;
-	sensor->frame_interval.denominator = ov5640_framerates[OV5640_30_FPS];
-	sensor->current_fr = OV5640_30_FPS;
+	sensor->frame_interval.denominator = ov5640_framerates[OV5640_60_FPS];
+	sensor->current_fr = OV5640_60_FPS;
 	sensor->current_mode =
 		&ov5640_mode_data[OV5640_MODE_VGA_640_480];
 	sensor->last_mode = sensor->current_mode;