mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Antoine Pichot <antoine.pichot@gmail.com>
To: Dave Stevenson <dave.stevenson@raspberrypi.com>
Cc: linux-media@vger.kernel.org, sakari.ailus@linux.intel.com,
	mchehab@kernel.org, hansg@kernel.org,
	andriy.shevchenko@linux.intel.com, dan.scally@ideasonboard.com,
	djrscally@gmail.com, dmanresa@gmail.com,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH v2 1/9] media: i2c: ov7251: Use in-range PLL1 dividers for the 319.2 MHz link
Date: Fri, 30 Oct 2026 23:26:28 +0900	[thread overview]
Message-ID: <20261030142628.2235190-1-antoine.pichot@gmail.com> (raw)
In-Reply-To: <CAPY8ntArdj338V8bcSxrmwKQecy-pAcCXMbkWy=qLckX=hawNQ@mail.gmail.com>

Hi Dave,

On Thu, Oct 29, 2026 at 07:06:31PM +0000, Dave Stevenson wrote:
> The cleanups do raise the question of what registers 0x3016-0x301c do,
> and which of the 2 variants is preferable (60 and 90fps both use the
> same values). The datasheet SC_CLKRSTx naming isn't exactly
> descriptive. I don't suppose you have any information?

The datasheet calls them the system control clock/reset registers
(SC_CLKRST0-6, table 7-1) and names every bit: per internal block, a
clock gate in bits 7-4 and a reset in bits 3-0. I checked them on the
Book 2 and believe they can safely move to ov7251_global_init_setting
for all three rates with the 60 and 90 fps values, 0x3016 = 0x10 and
0x00 in the other five: timing control clocked, every other gate and
every reset clear. Here is why:

a. Poked bit by bit mid-stream, any reset bit set holds its block
   (frames stop or come out empty), and only one gate matters for
   streaming, the timing control's (0x3016 bit 4): cleared, frames stop
   at every rate. Two other gates matter only to blocks the driver does
   not use, group hold and the OTP. Every other gate and the debug bits
   changed nothing I could see. None of this depends on the frame rate.

b. Written as whole values at 30, 60 and 90 fps, mid-stream and with
   the sensor restarted from software standby, this value, every gate
   set (the 30 fps table's 0xf0), the reset values and a set with only
   the documented gates on all streamed identically to the driver's
   values: same frame rate, lit and dark frame means, black level at
   gain 16 and 1023, test pattern bars, no sequence gaps, errors or
   kernel messages. So did a rebuilt driver started from power-on
   with the six lines in the init table, this value or 0xf0, right
   after the reset or at its end, and with the lines simply removed.
   Every bit clear gave no frames at any rate.

c. It is what the Windows driver for this module (ov7251.sys
   30.18305.6.5127) writes in all four of its modes, 30 fps included,
   what ov6211 and the other vendor tables I found use (Intel Aero,
   NVIDIA CyberDog, Allwinner, Unisoc, Sophgo, Ingenic, Cypress, at
   most one bit apart in 0x3018 and 0x301c), and what your rpi-5.15.y
   commit 63843c4bb331 [1] ran at all three rates. The 0xf0 set I only
   found in 30 fps tables derived from Qualcomm's auto-generated
   sensor library (ov7251_lib.h), which the 2018 driver's tables
   match: its 30 fps mode carried 0xf0 and its 90 fps mode 0x10/0x00,
   so the two variants look inherited, not chosen.

Not writing them at all works too, then; I prefer the value above
because it is the one everybody else runs. A driver that adds group
hold one day sets 0x301c bit 5.

Details follow: block names, bit map, method and results.

[1] https://github.com/raspberrypi/linux/commit/63843c4bb331c07492adcceb7c693fd39c26d88b

Details
=======

Block names, as abbreviated in the datasheet; the expansions are
OmniVision's usual ones, the confidence is mine:

  name    block                                        confidence
  fc      frame control (starts and ends each frame)   high, tested
  tc      timing control (line and pixel counters)     high, tested
  aec     automatic exposure control (unused here)     high
  blc     black level correction                       high
  isp     pixel pipeline                               high
  vfifo   output line buffer                           high, tested
  mipi    CSI-2 transmitter                            high, tested
  otp     one-time programmable memory                 high, read it
  grp     group hold (applies writes at a frame edge)  high, tested
  bist    built-in self-test                           high
  srb     serial register bus (I2C register block)     medium
  asram   asram_tst, internal RAM test logic           medium
  sync    snr_sync, frame sync to an external signal   medium
  ac, src, mipisc (mipi_sc): datasheet names only      low
  dbg     "Debug control", undocumented                n/a

Decoding, from table 7-1 of the datasheet (rev 2.12), which names
every bit: each register holds four blocks, their clock gates in bits
7-4 (sclk_, * = pclk_) and their resets in bits 3-0 (rst_), at the
same position except in 0x301a. The reset column is the value after
the software reset (0x0103), from the same table.

  reg    reset b7     b6     b5     b4     b3     b2     b1     b0
  0x3016 0xf0  fc     dbg    aec    tc     fc     dbg    aec    tc
  0x3017 0xf0  blc    isp    dbg    vfifo  blc    isp    dbg    vfifo
  0x3018 0xf0  dbg    mipi   ac     otp    dbg    mipi   ac     otp
  0x3019 0xf0  dbg    dbg    dbg    dbg    dbg    dbg    dbg    dbg
  0x301a 0xf0  dbg    dbg    vfifo* mipi*  dbg    mipisc dbg    dbg
  0x301b 0xf0  src    dbg    asram  sync   src    dbg    asram  sync
  0x301c 0xf2  bist   srb    grp    dbg    bist   dbg    grp    dbg

Method: v3 driver streaming at 30 fps, where the seven registers read
0xf0 (every gate set, every reset clear), frames captured with
v4l2-ctl, emitter pulsing alternate frames in a dark room or test
pattern on. From user space over I2C, one bit rewritten mid-stream
and the block's visible output compared before, during and after; a
reset bit held for 2 s. Group hold and the OTP, which a stream never
exercises, were driven directly: an exposure write under group hold
then launched, an OTP load into its buffer, one attempt per stream,
bit set against bit cleared. Nothing persists, the driver resets the
sensor at every power-on.

Then whole values, at 30, 60 and 90 fps each: every bit clear; the
timing control gate alone (0x3016 = 0x10, the rest 0x00); every gate
set (0xf0 in all six); every documented gate set and the debug bits
clear; and the reset values of the table above. The six registers
were written mid-stream, and written with the sensor in software
standby (0x0100) and restarted, the nearest thing to the init table
without a driver rebuild. Each run checked the frame rate, the lit
and dark frame means (strobe on alternate frames), the black level
at gain 16 and 1023 with a 1-line exposure, the test pattern bars,
the sequence numbers and the receiver's kernel messages against a
run with the driver's own values. Finally from power-on: the driver
rebuilt with the six lines moved to the init table (this value at
its end and right after the reset, 0xf0 at its end) or removed,
streamed at the three rates with the same checks.

Results (n/m = streams):

  clock gate cleared       effect
  0x301c b5 grp            group-hold write dropped 4/4 (applied 4/4
                           with the bit set), 30 and 60 fps
  0x3018 b4 otp            OTP load works 16/32 (16/16 with the bit
                           set), whatever the wait, rate or the other
                           five registers
  0x3017 b7 blc            none: black level still tracks a gain change
  0x3016 b6 dbg            none: strobe still fires
  0x3016 b4 tc             frames stop, no error, resume when set
                           again (30 and 60 fps)

  reset held for 2 s       effect
  0x3016 b3 fc, b0 tc,     frames stop, no error, resume on clear
  0x3017 b0 vfifo,         (mipi: one CRC error at release)
  0x3018 b2 mipi
  0x3017 b3 blc, b2 isp,   empty frames: a frame sync error and a
  0x3018 b1 ac,            0-byte payload per frame at the receiver,
  0x301b b3 src, b0 sync   sometimes a few CRC errors at release
  aec, otp, asram, bist,   nothing visible
  grp, mipisc, the debug
  bits, 0x3019 at 0x00
  and at 0xff

  whole value (6 runs each)        effect
  timing gate alone (0x10, 0x00)   all four: identical to the driver's
  every gate set (6x 0xf0)         own values in every run, same fps,
  documented gates set, debug      lit/dark means, black level at gain
  clear (b0 d0 70 30 b0 e0)        16 and 1023, bars, no gaps, errors
  reset values (0xf0, 0x301c 0xf2) or kernel messages
  every bit clear                  no frames at any rate; 0x3016 bit 4
                                   cleared alone does it, frames stop
                                   cleanly and resume when it is set

  cold start, rebuilt driver       effect
  this value at the end of the     all identical to the runs above:
  init table or right after the    registers read back as written,
  reset; 0xf0 at the end; lines    same fps, lit/dark means, black
  removed (reset values)           level, bars, no gaps, errors or
                                   kernel messages

(The standby restart itself truncates one frame at the receiver at
60 and 90 fps, with or without new values.)

So a reset bit at 1 holds its block; a clock gate at 1 keeps its block
clocked, and at 0 the chip clocks the image path anyway while
streaming, except the timing control, whose gate stops the frames
like its reset does. Every reset and the timing gate released
cleanly, the receiver never needed a restart.

Thanks,
Antoine

  parent reply	other threads:[~2026-10-30 14:26 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-28 11:21 [PATCH v2 0/9] media: i2c: ov7251: fixes and infrared strobe for Surface Book 2 Antoine Pichot
2026-10-28 11:21 ` [PATCH v2 1/9] media: i2c: ov7251: Use in-range PLL1 dividers for the 319.2 MHz link Antoine Pichot
2026-10-29 13:57   ` Andy Shevchenko
2026-10-29 19:06   ` Dave Stevenson
2026-10-30 14:26     ` Antoine Pichot
2026-10-30 14:26     ` Antoine Pichot [this message]
2026-10-28 11:21 ` [PATCH v2 2/9] media: i2c: ov7251: Fix link frequency index Antoine Pichot
2026-10-28 11:21 ` [PATCH v2 3/9] media: i2c: ov7251: Report the pixel array rate for both link frequencies Antoine Pichot
2026-10-29 13:59   ` Andy Shevchenko
2026-10-28 11:21 ` [PATCH v2 4/9] media: i2c: ov7251: Reset vblank when the frame interval changes Antoine Pichot
2026-10-28 11:21 ` [PATCH v2 5/9] media: i2c: ov7251: Use sub-device active state Antoine Pichot
2026-10-29 14:02   ` Andy Shevchenko
2026-10-30 14:26     ` Antoine Pichot
2026-10-28 11:21 ` [PATCH v2 6/9] media: i2c: ov7251: Add orientation and rotation controls Antoine Pichot
2026-10-28 11:21 ` [PATCH v2 7/9] media: i2c: ov7251: Move the strobe registers out of the mode tables Antoine Pichot
2026-10-28 11:21 ` [PATCH v2 8/9] media: i2c: ov7251: Add strobe output enable control Antoine Pichot
2026-10-30 12:12   ` Antoine Pichot
2026-10-28 11:21 ` [PATCH v2 9/9] media: ipu-bridge: Add a rotation quirk for the Surface Book 2 and Pro 7+ Antoine Pichot
2026-10-29 14:04   ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261030142628.2235190-1-antoine.pichot@gmail.com \
    --to=antoine.pichot@gmail.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=dan.scally@ideasonboard.com \
    --cc=dave.stevenson@raspberrypi.com \
    --cc=djrscally@gmail.com \
    --cc=dmanresa@gmail.com \
    --cc=hansg@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®