Re: [PATCH v2 0/2] Add Omnivision OV4689 image sensor driver

[Mikhail Rudenko <mike.rudenko@gmail.com>]

Hi Dave,

On 2022-09-22 at 11:43 +01, Dave Stevenson <dave.stevenson@raspberrypi.com> wrote:

> Hi Mikhail & Sakari
>
> On Thu, 22 Sept 2022 at 10:55, Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
>>
>> Hi Mikhail,
>>
>> On Sun, Sep 11, 2022 at 11:01:33PM +0300, Mikhail Rudenko wrote:
>> > Hello,
>> >
>> > this series implements support for Omnivision OV4689 image
>> > sensor. The Omnivision OV4689 is a high performance, 1/3-inch, 4
>> > megapixel image sensor. Ihis chip supports high frame rate speeds up
>> > to 90 fps at 2688x1520 resolution. It is programmable through an I2C
>> > interface, and sensor output is sent via 1/2/4 lane MIPI CSI-2
>> > connection.
>> >
>> > The driver is based on Rockchip BSP kernel [1]. It implements 4-lane CSI-2
>> > and single 2688x1520 @ 30 fps mode. The driver was tested on Rockchip
>> > 3399-based FriendlyElec NanoPi M4 board with MCAM400 camera module.
>> >
>> > While porting the driver, I stumbled upon two issues:
>> >
>> > (1) In the original driver, horizontal total size (HTS) was set to a
>> > value (2584) lower then the frame width (2688), resulting in negative
>> > hblank. In this driver, I increased HTS to 2688, but fps dropped from
>> > 29.88 to 28.73. What is the preferred way to handle this?
>>
>> If horizontal total size is less than the frame width, something is
>> certainly wrong there. You can't have negative horizontal blanking. Neither
>> it can be zero.
>
> Something certainly seems odd.
>
> To continue my thoughts from earlier in this patch set, Omnivision's
> Product Brief [1] states:
> The 1/3-inch OV4689 can capture full-resolution 4-megapixel high
> definition (HD) video at 90 frames per second (fps), 1080p HD at 120
> fps, and binned 720p HD at 180 fps
>
> The datasheet section 2.1 states:
> The OV4689 color image sensor is a high performance, 4 megapixel RAW
> image sensor that delivers 2688x1520 at 90 fps using OmniBSI-2™ pixel
> technology.
>
> So 4MP 90fps or 1080p120 should be achievable somehow.
>
> 2688x1520 @ 90fps is 367.7MPix/s, and that tallies quite nicely with
> table 2-9 listing the DAC PLL speed limitation of 360-378MHz. Exactly
> how that is then converted into PCLK or SCLK is unclear.
> Ideally you'd be able to contact an Omnivision FAE to confirm, but
> that means you need to be buying modules directly from them or
> otherwise have a business relationship.
> I do note that there is an NVidia Tegra driver for OV4689 at [2]. I
> wonder if analysis of that would reveal anything.
>
> I have just been looking at the ov9282 driver and the timings don't
> tally there either - configure it for 60fps and you get 30fps. The
> TIMING_HTS register appears to be in units of 2 pixels. The default is
> 0x2d8 (728 decimal) on a 1280x720 mode, but consider it as units of 2
> pixels and HTS of 1456 (1280 active and hblank of 176) does match up.
> It works in the general case too.
>
> Looking at the OV4689 datasheet again, the default for TIMING_HTS
> (0x380c/d) is 0x5f8 (1528 decimal) when HOUTPUT_SIZE (0x3808/9) is
> 0x1200 (4608 decimal). Whilst there are no guarantees that default
> register settings will stream in any sensible form, it does imply
> TIMING_HTS is not in units of 1 pixel, and potentially 4 pixels.
> From the Rockchip BSP driver it plainly does stream at 30 (or 29.88)
> fps with TIMING_HTS < HOUTPUT_SIZE, so a quick test of reducing it
> further would be worthwhile. What does the default of 0x2d8 give you
> as a frame rate, and are the images correct?
>

I've done some experimentation with timing registers and here is what I
found. First of all, there are two tables with default values of timing
registers (table 4-2 and table 6-9) in publicly available datasheet, and
they (values) are completely different. I'll just leave relevant parts
here for reference:

    Defaults (table 4-2):
    H_OUT_SIZE: 4608 (0x1200)
    V_OUT_SIZE: 3456 (0x0d80)
    HTS: 1528 (0x05f8)
    VTS: 3492 (0x0da4)

    Defaults (table 6-9):
    H_OUT_SIZE: 2688 (0xa80)
    V_OUT_SIZE: 1520 (0x5f0)
    HTS: 860 (0x35c)
    VTS: 1552 (0x610)

    Driver v2:
    H_OUT_SIZE: 2688
    V_OUT_SIZE: 1520
    HTS(0x380c/0d): 2688 (0x0a80)
    VTS(0x380e/0f): 1554 (0x0612)

Then I tried decreasing hts/vts and seeing what happens. The lowest
possible values before frame dropping started were hts=886 and vts=1552,
and the frame rate was 87.27 fps. Multiplying these three numbers yields
pixel rate of 120.0025e6.

So it looks like you are right, and the unit of HTS register is at least
4 pixels. Hence, in order to allow libcamera do correct timing
calculations, we should report pixel rate of 4*120e6 == 480e6, and
HBLANK of (4*HTS - H_OUT_SIZE). For 30 fps and VTS of 1554, this yields
HTS=2574 and HBLANK = (4*2574 - 2688) = 7608.

In fact, the above is what I'm going to implement in v3. Comments,
anyone?

> Just some thoughts.
>   Dave
>
> [1] https://www.ovt.com/wp-content/uploads/2022/01/OV4689-PB-v1.7-WEB.pdf
> [2] https://github.com/bogsen/STLinux-Kernel/blob/master/drivers/media/platform/tegra/ov4689.c
>

--
Best regards,
Mikhail Rudenko