| Message ID | 20220131144444.129036-1-jacopo@jmondi.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: ov5640: Rework the clock tree programming for MIPI | expand |
Hi Jacopo, Thank you for the patch. On Mon, Jan 31, 2022 at 03:44:40PM +0100, Jacopo Mondi wrote: > Now that the frame duration can be controlled by tuning the VBLANK > duration, fix all modes to comply with the reported FPS. > > All modes run at 30 FPS except for full-resolution mode 2592x1944 > which runs at 15FPS. > > Tested on a 2 data lanes setup in UYVY and RGB565 modes. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > drivers/media/i2c/ov5640.c | 30 +++++++++++++++--------------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c > index 6eeb50724195..2176fa0b8eae 100644 > --- a/drivers/media/i2c/ov5640.c > +++ b/drivers/media/i2c/ov5640.c > @@ -611,8 +611,8 @@ ov5640_mode_data[OV5640_NUM_MODES] = { > .width = 160, > .height = 120, > }, > - .ppl = 1896, > - .vblank_def = 864, > + .ppl = 1600, > + .vblank_def = 878, The total number of pixels is changed from 1896 * (120+864) = 1865664 to 1600 * (120+878) = 1596800. The latter produces 30.06fps with a 48 Mp/s rate, which certainly seems better than the 25.73fps produced by the format. I wonder, though, if this has always been wrong, or if the incorrect frame rate got introduced earlier in this series. Also, how did you pick a htot value of 1600 ? > .reg_data = ov5640_setting_QQVGA_160_120, > .reg_data_size = ARRAY_SIZE(ov5640_setting_QQVGA_160_120), > .max_fps = OV5640_30_FPS > @@ -633,8 +633,8 @@ ov5640_mode_data[OV5640_NUM_MODES] = { > .width = 176, > .height = 144, > }, > - .ppl = 1896, > - .vblank_def = 840, > + .ppl = 1600, > + .vblank_def = 854, > .reg_data = ov5640_setting_QCIF_176_144, > .reg_data_size = ARRAY_SIZE(ov5640_setting_QCIF_176_144), > .max_fps = OV5640_30_FPS > @@ -655,8 +655,8 @@ ov5640_mode_data[OV5640_NUM_MODES] = { > .width = 320, > .height = 240, > }, > - .ppl = 1896, > - .vblank_def = 744, > + .ppl = 1600, > + .vblank_def = 760, > .reg_data = ov5640_setting_QVGA_320_240, > .reg_data_size = ARRAY_SIZE(ov5640_setting_QVGA_320_240), > .max_fps = OV5640_30_FPS > @@ -677,8 +677,8 @@ ov5640_mode_data[OV5640_NUM_MODES] = { > .width = 640, > .height = 480, > }, > - .ppl = 1896, > - .vblank_def = 600, > + .ppl = 1600, > + .vblank_def = 520, > .reg_data = ov5640_setting_VGA_640_480, > .reg_data_size = ARRAY_SIZE(ov5640_setting_VGA_640_480), > .max_fps = OV5640_60_FPS > @@ -700,7 +700,7 @@ ov5640_mode_data[OV5640_NUM_MODES] = { > .height = 480, > }, > .ppl = 1896, > - .vblank_def = 504, > + .vblank_def = 1206, > .reg_data = ov5640_setting_NTSC_720_480, > .reg_data_size = ARRAY_SIZE(ov5640_setting_NTSC_720_480), > .max_fps = OV5640_30_FPS > @@ -722,7 +722,7 @@ ov5640_mode_data[OV5640_NUM_MODES] = { > .height = 576, > }, > .ppl = 1896, > - .vblank_def = 408, > + .vblank_def = 1110, > .reg_data = ov5640_setting_PAL_720_576, > .reg_data_size = ARRAY_SIZE(ov5640_setting_PAL_720_576), > .max_fps = OV5640_30_FPS > @@ -744,7 +744,7 @@ ov5640_mode_data[OV5640_NUM_MODES] = { > .height = 768, > }, > .ppl = 1896, > - .vblank_def = 312, > + .vblank_def = 918, > .reg_data = ov5640_setting_XGA_1024_768, > .reg_data_size = ARRAY_SIZE(ov5640_setting_XGA_1024_768), > .max_fps = OV5640_30_FPS > @@ -765,8 +765,8 @@ ov5640_mode_data[OV5640_NUM_MODES] = { > .width = 1280, > .height = 720, > }, > - .ppl = 1896, > - .vblank_def = 20, > + .ppl = 1600, > + .vblank_def = 560, > .reg_data = ov5640_setting_720P_1280_720, > .reg_data_size = ARRAY_SIZE(ov5640_setting_720P_1280_720), > .max_fps = OV5640_30_FPS > @@ -787,8 +787,8 @@ ov5640_mode_data[OV5640_NUM_MODES] = { > .width = 1920, > .height = 1080, > }, > - .ppl = 2500, > - .vblank_def = 40, > + .ppl = 2234, > + .vblank_def = 24, > .reg_data = ov5640_setting_1080P_1920_1080, > .reg_data_size = ARRAY_SIZE(ov5640_setting_1080P_1920_1080), > .max_fps = OV5640_30_FPS
Hi Laurent, On Wed, Feb 02, 2022 at 11:48:52PM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Mon, Jan 31, 2022 at 03:44:40PM +0100, Jacopo Mondi wrote: > > Now that the frame duration can be controlled by tuning the VBLANK > > duration, fix all modes to comply with the reported FPS. > > > > All modes run at 30 FPS except for full-resolution mode 2592x1944 > > which runs at 15FPS. > > > > Tested on a 2 data lanes setup in UYVY and RGB565 modes. > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > drivers/media/i2c/ov5640.c | 30 +++++++++++++++--------------- > > 1 file changed, 15 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c > > index 6eeb50724195..2176fa0b8eae 100644 > > --- a/drivers/media/i2c/ov5640.c > > +++ b/drivers/media/i2c/ov5640.c > > @@ -611,8 +611,8 @@ ov5640_mode_data[OV5640_NUM_MODES] = { > > .width = 160, > > .height = 120, > > }, > > - .ppl = 1896, > > - .vblank_def = 864, > > + .ppl = 1600, > > + .vblank_def = 878, > > The total number of pixels is changed from 1896 * (120+864) = 1865664 to > 1600 * (120+878) = 1596800. The latter produces 30.06fps with a 48 Mp/s > rate, which certainly seems better than the 25.73fps produced by the > format. I wonder, though, if this has always been wrong, or if the > incorrect frame rate got introduced earlier in this series. > Ideally, the values there come from the existing tables. I could have introduced mistakes of course, but not changed those numbers internally. With the current implementation I recall the framerate was not accurate I can't tell if that's because the PLL is wrongly programmed or because of wrong timings. > Also, how did you pick a htot value of 1600 ? > mmm, it was a value that allowed me to range in a decent interval of vblank values. Found by experiments I have to admit, not planned. It could prbably be better estimated... > > .reg_data = ov5640_setting_QQVGA_160_120, > > .reg_data_size = ARRAY_SIZE(ov5640_setting_QQVGA_160_120), > > .max_fps = OV5640_30_FPS > > @@ -633,8 +633,8 @@ ov5640_mode_data[OV5640_NUM_MODES] = { > > .width = 176, > > .height = 144, > > }, > > - .ppl = 1896, > > - .vblank_def = 840, > > + .ppl = 1600, > > + .vblank_def = 854, > > .reg_data = ov5640_setting_QCIF_176_144, > > .reg_data_size = ARRAY_SIZE(ov5640_setting_QCIF_176_144), > > .max_fps = OV5640_30_FPS > > @@ -655,8 +655,8 @@ ov5640_mode_data[OV5640_NUM_MODES] = { > > .width = 320, > > .height = 240, > > }, > > - .ppl = 1896, > > - .vblank_def = 744, > > + .ppl = 1600, > > + .vblank_def = 760, > > .reg_data = ov5640_setting_QVGA_320_240, > > .reg_data_size = ARRAY_SIZE(ov5640_setting_QVGA_320_240), > > .max_fps = OV5640_30_FPS > > @@ -677,8 +677,8 @@ ov5640_mode_data[OV5640_NUM_MODES] = { > > .width = 640, > > .height = 480, > > }, > > - .ppl = 1896, > > - .vblank_def = 600, > > + .ppl = 1600, > > + .vblank_def = 520, > > .reg_data = ov5640_setting_VGA_640_480, > > .reg_data_size = ARRAY_SIZE(ov5640_setting_VGA_640_480), > > .max_fps = OV5640_60_FPS > > @@ -700,7 +700,7 @@ ov5640_mode_data[OV5640_NUM_MODES] = { > > .height = 480, > > }, > > .ppl = 1896, > > - .vblank_def = 504, > > + .vblank_def = 1206, > > .reg_data = ov5640_setting_NTSC_720_480, > > .reg_data_size = ARRAY_SIZE(ov5640_setting_NTSC_720_480), > > .max_fps = OV5640_30_FPS > > @@ -722,7 +722,7 @@ ov5640_mode_data[OV5640_NUM_MODES] = { > > .height = 576, > > }, > > .ppl = 1896, > > - .vblank_def = 408, > > + .vblank_def = 1110, > > .reg_data = ov5640_setting_PAL_720_576, > > .reg_data_size = ARRAY_SIZE(ov5640_setting_PAL_720_576), > > .max_fps = OV5640_30_FPS > > @@ -744,7 +744,7 @@ ov5640_mode_data[OV5640_NUM_MODES] = { > > .height = 768, > > }, > > .ppl = 1896, > > - .vblank_def = 312, > > + .vblank_def = 918, > > .reg_data = ov5640_setting_XGA_1024_768, > > .reg_data_size = ARRAY_SIZE(ov5640_setting_XGA_1024_768), > > .max_fps = OV5640_30_FPS > > @@ -765,8 +765,8 @@ ov5640_mode_data[OV5640_NUM_MODES] = { > > .width = 1280, > > .height = 720, > > }, > > - .ppl = 1896, > > - .vblank_def = 20, > > + .ppl = 1600, > > + .vblank_def = 560, > > .reg_data = ov5640_setting_720P_1280_720, > > .reg_data_size = ARRAY_SIZE(ov5640_setting_720P_1280_720), > > .max_fps = OV5640_30_FPS > > @@ -787,8 +787,8 @@ ov5640_mode_data[OV5640_NUM_MODES] = { > > .width = 1920, > > .height = 1080, > > }, > > - .ppl = 2500, > > - .vblank_def = 40, > > + .ppl = 2234, > > + .vblank_def = 24, > > .reg_data = ov5640_setting_1080P_1920_1080, > > .reg_data_size = ARRAY_SIZE(ov5640_setting_1080P_1920_1080), > > .max_fps = OV5640_30_FPS > > -- > Regards, > > Laurent Pinchart
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c index 6eeb50724195..2176fa0b8eae 100644 --- a/drivers/media/i2c/ov5640.c +++ b/drivers/media/i2c/ov5640.c @@ -611,8 +611,8 @@ ov5640_mode_data[OV5640_NUM_MODES] = { .width = 160, .height = 120, }, - .ppl = 1896, - .vblank_def = 864, + .ppl = 1600, + .vblank_def = 878, .reg_data = ov5640_setting_QQVGA_160_120, .reg_data_size = ARRAY_SIZE(ov5640_setting_QQVGA_160_120), .max_fps = OV5640_30_FPS @@ -633,8 +633,8 @@ ov5640_mode_data[OV5640_NUM_MODES] = { .width = 176, .height = 144, }, - .ppl = 1896, - .vblank_def = 840, + .ppl = 1600, + .vblank_def = 854, .reg_data = ov5640_setting_QCIF_176_144, .reg_data_size = ARRAY_SIZE(ov5640_setting_QCIF_176_144), .max_fps = OV5640_30_FPS @@ -655,8 +655,8 @@ ov5640_mode_data[OV5640_NUM_MODES] = { .width = 320, .height = 240, }, - .ppl = 1896, - .vblank_def = 744, + .ppl = 1600, + .vblank_def = 760, .reg_data = ov5640_setting_QVGA_320_240, .reg_data_size = ARRAY_SIZE(ov5640_setting_QVGA_320_240), .max_fps = OV5640_30_FPS @@ -677,8 +677,8 @@ ov5640_mode_data[OV5640_NUM_MODES] = { .width = 640, .height = 480, }, - .ppl = 1896, - .vblank_def = 600, + .ppl = 1600, + .vblank_def = 520, .reg_data = ov5640_setting_VGA_640_480, .reg_data_size = ARRAY_SIZE(ov5640_setting_VGA_640_480), .max_fps = OV5640_60_FPS @@ -700,7 +700,7 @@ ov5640_mode_data[OV5640_NUM_MODES] = { .height = 480, }, .ppl = 1896, - .vblank_def = 504, + .vblank_def = 1206, .reg_data = ov5640_setting_NTSC_720_480, .reg_data_size = ARRAY_SIZE(ov5640_setting_NTSC_720_480), .max_fps = OV5640_30_FPS @@ -722,7 +722,7 @@ ov5640_mode_data[OV5640_NUM_MODES] = { .height = 576, }, .ppl = 1896, - .vblank_def = 408, + .vblank_def = 1110, .reg_data = ov5640_setting_PAL_720_576, .reg_data_size = ARRAY_SIZE(ov5640_setting_PAL_720_576), .max_fps = OV5640_30_FPS @@ -744,7 +744,7 @@ ov5640_mode_data[OV5640_NUM_MODES] = { .height = 768, }, .ppl = 1896, - .vblank_def = 312, + .vblank_def = 918, .reg_data = ov5640_setting_XGA_1024_768, .reg_data_size = ARRAY_SIZE(ov5640_setting_XGA_1024_768), .max_fps = OV5640_30_FPS @@ -765,8 +765,8 @@ ov5640_mode_data[OV5640_NUM_MODES] = { .width = 1280, .height = 720, }, - .ppl = 1896, - .vblank_def = 20, + .ppl = 1600, + .vblank_def = 560, .reg_data = ov5640_setting_720P_1280_720, .reg_data_size = ARRAY_SIZE(ov5640_setting_720P_1280_720), .max_fps = OV5640_30_FPS @@ -787,8 +787,8 @@ ov5640_mode_data[OV5640_NUM_MODES] = { .width = 1920, .height = 1080, }, - .ppl = 2500, - .vblank_def = 40, + .ppl = 2234, + .vblank_def = 24, .reg_data = ov5640_setting_1080P_1920_1080, .reg_data_size = ARRAY_SIZE(ov5640_setting_1080P_1920_1080), .max_fps = OV5640_30_FPS
Now that the frame duration can be controlled by tuning the VBLANK duration, fix all modes to comply with the reported FPS. All modes run at 30 FPS except for full-resolution mode 2592x1944 which runs at 15FPS. Tested on a 2 data lanes setup in UYVY and RGB565 modes. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- drivers/media/i2c/ov5640.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)