Re: [PATCH v4 05/12] media: ov5640: Compute the clock rate at runtime

[Jacopo Mondi <jacopo@jmondi.org>]

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

Sorry again,

On Mon, Feb 25, 2019 at 10:21:51AM +0100, Jacopo Mondi wrote:
> Hello Maxime, Benoit,
>   sorry for chiming in, but I'm a bit confused...
>
> On Fri, Feb 22, 2019 at 04:04:21PM +0100, Maxime Ripard wrote:
> > On Fri, Feb 22, 2019 at 08:54:56AM -0600, Benoit Parrot wrote:
> > > Maxime Ripard <maxime.ripard@bootlin.com> wrote on Fri [2019-Feb-22 15:39:59 +0100]:
> > > > On Thu, Feb 21, 2019 at 10:20:20AM -0600, Benoit Parrot wrote:
> > > > > Hi Maxime,
> > > > >
> > > > > A couple of questions,
> > > > >
> > > > > Maxime Ripard <maxime.ripard@bootlin.com> wrote on Thu [2018-Oct-11 04:21:00 -0500]:
> > > > > > The clock rate, while hardcoded until now, is actually a function of the
> > > > > > resolution, framerate and bytes per pixel. Now that we have an algorithm to
> > > > > > adjust our clock rate, we can select it dynamically when we change the
> > > > > > mode.
> > > > > >
> > > > > > This changes a bit the clock rate being used, with the following effect:
> > > > > >
> > > > > > +------+------+------+------+-----+-----------------+----------------+-----------+
> > > > > > | Hact | Vact | Htot | Vtot | FPS | Hardcoded clock | Computed clock | Deviation |
> > > > > > +------+------+------+------+-----+-----------------+----------------+-----------+
> > > > > > |  640 |  480 | 1896 | 1080 |  15 |        56000000 |       61430400 | 8.84 %    |
> > > > > > |  640 |  480 | 1896 | 1080 |  30 |       112000000 |      122860800 | 8.84 %    |
> > > > > > | 1024 |  768 | 1896 | 1080 |  15 |        56000000 |       61430400 | 8.84 %    |
> > > > > > | 1024 |  768 | 1896 | 1080 |  30 |       112000000 |      122860800 | 8.84 %    |
> > > > > > |  320 |  240 | 1896 |  984 |  15 |        56000000 |       55969920 | 0.05 %    |
> > > > > > |  320 |  240 | 1896 |  984 |  30 |       112000000 |      111939840 | 0.05 %    |
> > > > > > |  176 |  144 | 1896 |  984 |  15 |        56000000 |       55969920 | 0.05 %    |
> > > > > > |  176 |  144 | 1896 |  984 |  30 |       112000000 |      111939840 | 0.05 %    |
> > > > > > |  720 |  480 | 1896 |  984 |  15 |        56000000 |       55969920 | 0.05 %    |
> > > > > > |  720 |  480 | 1896 |  984 |  30 |       112000000 |      111939840 | 0.05 %    |
> > > > > > |  720 |  576 | 1896 |  984 |  15 |        56000000 |       55969920 | 0.05 %    |
> > > > > > |  720 |  576 | 1896 |  984 |  30 |       112000000 |      111939840 | 0.05 %    |
> > > > > > | 1280 |  720 | 1892 |  740 |  15 |        42000000 |       42002400 | 0.01 %    |
> > > > > > | 1280 |  720 | 1892 |  740 |  30 |        84000000 |       84004800 | 0.01 %    |
> > > > > > | 1920 | 1080 | 2500 | 1120 |  15 |        84000000 |       84000000 | 0.00 %    |
> > > > > > | 1920 | 1080 | 2500 | 1120 |  30 |       168000000 |      168000000 | 0.00 %    |
> > > > > > | 2592 | 1944 | 2844 | 1944 |  15 |        84000000 |      165862080 | 49.36 %   |
> > > > > > +------+------+------+------+-----+-----------------+----------------+-----------+
> > > > >
> > > > > Is the computed clock above the same for both parallel and CSI2?
> > > > >
> > > > > I want to add controls for PIXEL_RATE and LINK_FREQ, would you have any
> > > > > quick pointer on taking the computed clock and translating that into the
> > > > > PIXEL_RATE and LINK_FREQ values?
> > > > >
> > > > > I am trying to use this sensor with TI CAL driver which at the moment uses
> > > > > the PIXEL_RATE values in order to compute ths_settle and ths_term values
> > > > > needed to program the DPHY properly. This is similar in behavior as the way
> > > > > omap3isp relies on this info as well.
> > > >
> > > > I haven't looked that much into the csi-2 case, but the pixel rate
> > > > should be the same at least.
> > >
> > > I'll have to study the way the computed clock is actually calculated for
> > > either case, but if they yield the same number then I would be surprised
> > > that the pixel rate would be the same as in parallel mode you get 8 data
> > > bits per clock whereas in CSI2 using 2 data lanes you get 4 data bits per
> > > clock.
> >
> > The bus rate will be different, but the pixel rate is the same: you
> > have as many pixels per frames and as many frames per seconds in the
> > parallel and CSI cases.
> >
>
> I agree with that, but..
>
> > > So just to be certain here the "Computed clock" column above would be the
> > > pixel clock frequency?
> >
> > it is
>
> ...it seems to me the Computed clock column is actually the "byte clock".
>
> From a simple calculation for the 640x480@15FPS case:
> "Computed clock" = 1896 * 1080 * 15 * 2 = 61430400
>
> While, in my understanding, the pixel clock would just be
> pixel_clock = HTOT * VTOT * FPS = 1896 * 1080 * 15 = 30715200
>
> So I suspect the "* 2" there is the number of bytes per pixel.
>
> That would match what's also reported here
> file:///home/jmondi/project/renesas/linux/linux-build/Documentation/output/media/kapi/csi2.html?highlight=link_freq
>

Of course that is a link to the local copy of the documentation,
what I actually meant to link was:
https://www.kernel.org/doc/html/latest/media/kapi/csi2.html?highlight=link_freq

Sorry!
   j


> Where:
> link_freq = (pixel_rate * bpp) / (2 * nr_lanes)
>
> So if I were to calculate PIXEL_RATE and LINK_FREQ in this driver,
> that would be:
> PIXEL_RATE = mode->vtot * mode->htot * ov5640_framerates[sensor->current_fr];
> LINK_FREQ = PIXEL_RATE * 16 / ( 2 * sensor->ep.bus.mipi_csi2.num_data_lanes);
> (assuming, as the driver does now, all formats have 16bpp)
>
> Does this match your understanding as well?
>
> Thanks
>   j
>
> >
> > Maxime
> >
> > --
> > Maxime Ripard, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
>
>



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