Hi Maxime, On 09/28/2018 06:05 PM, Maxime Ripard wrote: > Hi Hugues, > > On Thu, Sep 27, 2018 at 03:59:04PM +0000, Hugues FRUCHET wrote: >> Hi Maxime & all OV5640 stakeholders, >> >> I've just pushed a new patchset also related to rate/pixel clock >> handling [1], based on your V3 great work: >> > media: ov5640: Adjust the clock based on the expected rate >> > media: ov5640: Remove the clocks registers initialization >> > media: ov5640: Remove redundant defines >> > media: ov5640: Remove redundant register setup >> > media: ov5640: Compute the clock rate at runtime >> > media: ov5640: Remove pixel clock rates >> > media: ov5640: Enhance FPS handling >> >> 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). Instead of using auto mode, my proposal was the inverse: use manual mode with the proper divider to fit the max pixel clock constraint. BR, Hugues.