Re: [PATCH v3 00/12] media: ov5640: Misc cleanup and improvements

[Maxime Ripard <maxime.ripard@bootlin.com>]

[-- Attachment #1: Type: text/plain, Size: 2703 bytes --]

Hi!

On Mon, Oct 01, 2018 at 02:12:31PM +0000, Hugues FRUCHET wrote:
> >> This is working perfectly fine on my parallel setup and allows me to
> >> well support VGA@30fps (instead 27) and also support XGA(1024x768)@15fps
> >> that I never seen working before.
> >> So at least for the parallel setup, this serie is working fine for all
> >> the discrete resolutions and framerate exposed by the driver for the moment:
> >> * QCIF 176x144 15/30fps
> >> * QVGA 320x240 15/30fps
> >> * VGA 640x480 15/30fps
> >> * 480p 720x480 15/30fps
> >> * XGA 1024x768 15/30fps
> >> * 720p 1280x720 15/30fps
> >> * 1080p 1920x1080 15/30fps
> >> * 5Mp 2592x1944 15fps
> > 
> > I'm glad this is working for you as well. I guess I'll resubmit these
> > patches, but this time making sure someone with a CSI setup tests
> > before merging. I crtainly don't want to repeat the previous disaster.
> > 
> > Do you have those patches rebased somewhere? I'm not quite sure how to
> > fix the conflict with the v4l2_find_nearest_size addition.
> > 
> >> Moreover I'm not clear on relationship between rate and pixel clock
> >> frequency.
> >> I've understood that to DVP_PCLK_DIVIDER (0x3824) register
> >> and VFIFO_CTRL0C (0x460c) affects the effective pixel clock frequency.
> >> All the resolutions up to 720x576 are forcing a manual value of 2 for
> >> divider (0x460c=0x22), but including 720p and more, the divider value is
> >> controlled by "auto-mode" (0x460c=0x20)... from what I measured and
> >> understood, for those resolutions, the divider must be set to 1 in order
> >> that your rate computation match the effective pixel clock on output,
> >> see [2].
> >>
> >> So I wonder if this PCLK divider register should be included
> >> or not into your rate computation, what do you think ?
> > 
> > Have you tried change the PCLK divider while in auto-mode? IIRC, I did
> > that and it was affecting the PCLK rate on my scope, but I wouldn't be
> > definitive about it.
> 
> I have tested to change PCLK divider while in auto mode but no effect.
> 
> > Can we always set the mode to auto and divider to 1, even for the
> > lower resolutions?
>
> This is breaking 176x144@30fps on my side, because of pixel clock too 
> high (112MHz vs 70 MHz max).

Ok.

> Instead of using auto mode, my proposal was the inverse: use manual mode 
> with the proper divider to fit the max pixel clock constraint.

Oh. That would work for me too yeah. How do you want to deal with it?
Should I send your rebased patches, and you add that change as a
subsequent patch?

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]