diff mbox series

media: ov5640: fix incorrect frame frate issue for defulat VGA

Message ID 20230423021458.1138760-1-guoniu.zhou@nxp.com
State New
Headers show
Series media: ov5640: fix incorrect frame frate issue for defulat VGA | expand

Commit Message

G.N. Zhou April 23, 2023, 2:14 a.m. UTC
If runn OV5640 with 640x480@30 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>
---
 drivers/media/i2c/ov5640.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

G.N. Zhou May 5, 2023, 1:23 a.m. UTC | #1
Hi Jacopo Modi,

Sorry for replying so late.

Thanks for your comments and I agree with you. Will update a v2 patch to this issue. Thanks again. 

> -----Original Message-----
> From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Sent: 2023年4月27日 19:24
> To: G.N. Zhou <guoniu.zhou@nxp.com>
> Cc: linux-media@vger.kernel.org; mchehab@kernel.org;
> slongerbeam@gmail.com; laurent.pinchart@ideasonboard.com;
> jacopo.mondi@ideasonboard.com
> Subject: [EXT] Re: [PATCH] media: ov5640: fix incorrect frame frate issue for
> defulat VGA
> 
> Caution: EXT Email
> 
> Hi Guoniu
> 
> On Sun, Apr 23, 2023 at 10:14:58AM +0800, Guoniu.zhou wrote:
> > If runn OV5640 with 640x480@30 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>
> 
> The frame_interval calls are only used for parallel mode and are ignored in CSI-2
> mode. This means that the frame rate has to be controlled by adjusting blankings
> (and possibly LINK_FREQ which this driver registers in read-only mode though).
> 
> I recall the choice of that DEFAULT_LINK_FREQ was the one that allowed to
> obtain the highest frame rates, hence I think it's right to chose it by default.
> 
> Maybe this comment, that reports @30, is misleading
>          *
>          * default init sequence initialize sensor to
>          * YUV422 UYVY VGA@30fps
>          *
> 
> > ---
> >  drivers/media/i2c/ov5640.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> > index 1536649b9e90..80e1a2858abd 100644
> > --- a/drivers/media/i2c/ov5640.c
> > +++ b/drivers/media/i2c/ov5640.c
> > @@ -179,7 +179,7 @@ static const s64 ov5640_csi2_link_freqs[] = {  };
> >
> >  /* Link freq for default mode: UYVY 16 bpp, 2 data lanes. */
> > -#define OV5640_DEFAULT_LINK_FREQ     13
> > +#define OV5640_DEFAULT_LINK_FREQ     19
> >
> >  enum ov5640_format_mux {
> >       OV5640_FMT_MUX_YUV422 = 0,
> > --
> > 2.37.1
> >
diff mbox series

Patch

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 1536649b9e90..80e1a2858abd 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -179,7 +179,7 @@  static const s64 ov5640_csi2_link_freqs[] = {
 };
 
 /* Link freq for default mode: UYVY 16 bpp, 2 data lanes. */
-#define OV5640_DEFAULT_LINK_FREQ	13
+#define OV5640_DEFAULT_LINK_FREQ	19
 
 enum ov5640_format_mux {
 	OV5640_FMT_MUX_YUV422 = 0,