Message-ID: <87czbwp9xx.fsf@gmail.com>
Date:   Fri, 16 Sep 2022 00:27:42 +0300
From:   Mikhail Rudenko <mike.rudenko@...il.com>
To:     Dave Stevenson <dave.stevenson@...pberrypi.com>
Cc:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Hans Verkuil <hverkuil-cisco@...all.nl>,
        Jacopo Mondi <jacopo@...ndi.org>,
        Shawn Tu <shawnx.tu@...el.com>, Jimmy Su <jimmy.su@...el.com>,
        Arnd Bergmann <arnd@...db.de>, Arec Kao <arec.kao@...el.com>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Marek Vasut <marex@...x.de>, linux-media@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 0/2] Add Omnivision OV4689 image sensor driver


Hi Dave,

On 2022-09-14 at 10:58 +01, Dave Stevenson <dave.stevenson@...pberrypi.com> wrote:
> Hi Mikhail
>
> On Sun, 11 Sept 2022 at 21:02, Mikhail Rudenko <mike.rudenko@...il.com> 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?
>
> This is one of the joys of sensors - they don't all work in the same way.
>
> I don't have an official datasheet for OV4689 from Omnivision, but
> found one on the internet [1]. That should allow you to reverse the
> PLL configuration to confirm that the pixel rate is the value you've
> computed based on link frequency (they aren't necessarily related). Do
> the frame rate calculations work using width + HBLANK, height +
> VBLANK, and pixel rate?
> The datasheet claims the sensor supports 2688x1520 @ 90 fps, so
> something doesn't hold true between 4 data lanes at 500MHz/1Gbit/s per
> lane when your default hts/vts is 2688x1554 and it only gives
> 28.73fps.

Seems like those 90 fps is about CSI throughput, not actual sensor
performance. I've checked the datasheet and the register values, and it
seems like the pixel clock is 126 Mhz in this configuration (the maximum
is 150 MHz according to the datasheet). This corresponds to a
theoretical fps of 30.16 at hts=2688 and vts=1554. At the same time the
observed fps is 28.73. I'm not sure where those 1.43 frames are lost,
hope to do more experimentation with VTS and HTS over the weekend.

> I have seen modes in sensors where the HTS register is in units of 2
> pixels, so what range of HTS (and VTS) values actually works on this
> sensor? (I don't see it documented, but I'm not surprised).
>
> [1] https://cdn.hackaday.io/files/19354828041536/OV4689-OmniVision.pdf
>
>> (2) The original driver exposes analog gain range 0x0 - 0x7ff, but the
>> gain is not linear across that range. Instead, it is piecewise linear
>> (and discontinuous). 0x0-0xff register values result in 0x-2x gain,
>> 0x100-0x1ff to 0x-4x, 0x300-0x3ff to 0x-8x, and 0x700-0x7ff to 0x-16x,
>> with more linear segments in between. Rockchip's camera engine code
>> chooses one of the above segments depenging on the desired gain
>> value. The question is, how should we proceed keeping in mind
>> libcamera use case? Should the whole 0x0-0x7ff be exposed as-is and
>> libcamera will do the mapping, or the driver will do the mapping
>> itself and expose some logical gain units not tied to the actual gain
>> register value? Meanwhile, this driver conservatively exposes only
>> 0x0-0xf8 gain register range.
>
> The datasheet linked above says "for the gain formula, please contact
> your local OmniVision FAE" :-(
> I would assume that the range is from 1x rather than 0x - people
> rarely want a totally black image that 0x would give. Or is it ranges
> of 1x - 2x, 2x - 4x, 4x - 8x, and 8x - 16x?

A picture is worth a thousand words, so I've attached the results of my
experimentation with the gain register. They were obtained with Rockchip
3399, with AEC, AGC and black level subtraction disabled. The image was
converted from 10-bit RGGB to 8-bit YUV 4:2:0 by the Rockchip ISP.

> Other sensors expose the full range of the register via
> V4L2_CID_ANALOGUE_GAIN, and require userspace (mainly libcamera now)
> to know how to convert a gain into the register value. If the gain
> range goes up to x16, then exposing that would be useful. I'd advocate
> just exposing the full range of 0x000 - 0x7ff, as then you can have
> the accuracy of 256 values between x1 to x2, but also the full range.

I also like this approach, although libcamera's CameraSensorHelper
doesn't support piecewise-linear gain code mapping yet. Nevertheless,
I believe exposing the full range is a good idea and will do so in v3.

> I might see if I can pick up one of these sensors and see if I can get
> it running on a Raspberry Pi. Thanks for trying to upstream this -
> it's nice to have such a range of sensor drivers to choose from.
>
>   Dave
>

Thanks for your elucidating tips!

--
Best regards,
Mikhail Rudenko


Download attachment "gain.png" of type "image/png" (49130 bytes)

Powered by blists - more mailing lists