Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220911200147.375198-1-mike.rudenko@gmail.com> <YywwqFi+2Nah2RpY@paasikivi.fi.intel.com>
In-Reply-To: <YywwqFi+2Nah2RpY@paasikivi.fi.intel.com>
From:   Dave Stevenson <dave.stevenson@raspberrypi.com>
Date:   Thu, 22 Sep 2022 11:43:14 +0100
Message-ID: <CAPY8ntAvos4Et4c5mAiw=6Wb4b53p2PLRX_Jw03bHckpD+e-sg@mail.gmail.com>
Subject: Re: [PATCH v2 0/2] Add Omnivision OV4689 image sensor driver
To:     Sakari Ailus <sakari.ailus@linux.intel.com>
Cc:     Mikhail Rudenko <mike.rudenko@gmail.com>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Hans Verkuil <hverkuil-cisco@xs4all.nl>,
        Jacopo Mondi <jacopo@jmondi.org>,
        Shawn Tu <shawnx.tu@intel.com>, Jimmy Su <jimmy.su@intel.com>,
        Arnd Bergmann <arnd@arndb.de>, Arec Kao <arec.kao@intel.com>,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        Marek Vasut <marex@denx.de>, linux-media@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

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 CS=
I-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. Neith=
er
> 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=E2=84=A2 pix=
el
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?

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/plat=
form/tegra/ov4689.c


> >
> > (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.
>
> --
> Sakari Ailus