Re: [PATCH v6] media: i2c: Add HM1092 camera sensor driver

Jake S <[email protected]>
Newsgroups org.kernel.vger.linux-media
Message-ID <[email protected]>
From: Jake Steinman <[email protected]>

Hi Ramshouriesh,

The comparison you asked for is below. Short version: your 180 MHz
derivation is independently confirmed by the Dell platform, which reaches
the same place by a different route.

I regenerated 238 (address, value) pairs from the Dell hm1092.sys mode
descriptor and diffed them against your 185 Qualcomm entries.

  registers present in both      178
    identical value              162
    different value               16
  Qualcomm only                    4
  Dell only                        5

162 of 178 identical across two unrelated vendors is, I think, the strongest
evidence either of us has that we are reading the sensor correctly and not
transcribing someone's platform quirks.


THE PLL, AND WHY OUR NUMBERS DIFFER SLIGHTLY

The clock block is where the two diverge, and it resolves cleanly:

  0x030d (pre-divider)   Qualcomm 0x0c = 12     Dell 0x0a = 10
  0x030f (multiplier)    Qualcomm 0x5a = 90     Dell 0x5e = 94

  Qualcomm: 24.0 MHz * 90 / 12 = 180,000,000 Hz
  Dell:     19.2 MHz * 94 / 10 = 180,480,000 Hz

Both land on ~180 MHz DDR from different EXTCLKs. Your ASUS power sequence
requests 24 MHz; Dell's INT3472 supplies 19.2 MHz, which is why the driver I
have been running publishes 180,480,000 rather than your 180,000,000. Neither
is wrong; they are different boards.

The implication for v6 is that EXTCLK is not a property of the sensor. A
driver that hardcodes 24 MHz will compute the wrong link frequency on Dell
hardware, and one that hardcodes 19.2 MHz will be wrong on yours. Reading
clock-frequency from firmware and deriving link_freq from the PLL registers
is the only form that is correct on both. If v6 already does that, ignore me.

The same applies to the init table: because the PLL registers differ, the two
platforms' tables are not interchangeable verbatim even though 162 of the
other registers agree.


TIMING

  0x0340/41 VTS    Qualcomm 0x02ee =  750    Dell 0x02e5 =  741
  0x0342/43 HTS    Qualcomm 0x0650 = 1616    Dell 0x0654 = 1620
  0x034c/4d out W  Qualcomm 0x0230 =  560    Dell 0x0288 =  648
  0x034e/4f out H  Qualcomm 0x0168 =  360    Dell 0x0170 =  368

  Dell: 36,096,000 / (1620 * 741) = 30.069 fps

which agrees with what I measure on the hardware. Your 29.703 fps for
560x360 and my 30.069 for 648x368 are the same sensor behaving consistently.

The remaining differences are all windowing  --  0x0344-0x034b crop, which is
what you would expect from two platforms choosing different output sizes.


EXPOSURE AND GAIN

Your register choices match what the Dell table does:

  0x0104 group hold        both present
  0x0202/0x0203 exposure   both present, 16-bit split across two writes
  0x0205 analogue gain     Dell only in my capture
  0x020e/0x020f dig gain   Dell only in my capture

The four your table lacks are the ones Windows only writes during the runtime
AE loop rather than at init, so their absence from a static init dump is
expected rather than a discrepancy.

One caution from experience: my Dell capture is a whole Windows Hello session
 --  init, stream-on, the AE loop and the teardown, including a final
0x0100 = 0x00. Replaying it verbatim and then calling stream-on made the
sensor arm, disarm and re-arm, which no real session does. If your Qualcomm
dump came from a similar capture rather than from the static table, it is
worth checking for a mid-table 0x0100 write before trusting the ordering.


IPU-BRIDGE

Agreed, follow-up patch on top of v6. I can write and test that one  --  the
IPU7 path is what I have hardware for. For reference the entry that works
here is:

  IPU_SENSOR_CONFIG("HIMX1092", 1, 180480000)

which will need to become two entries, or a firmware-derived value, once both
platforms are supported.


ATTRIBUTION

Co-developed-by, please, with my Signed-off-by. The register map, the control
register assignments and the link-frequency analysis are my work, and you have
said v6 folds them in directly  --  that is co-development rather than a report.

  Co-developed-by: Jake Steinman <[email protected]>
  Signed-off-by: Jake Steinman <[email protected]>

Keep Reported-by as well if you like; the two are not exclusive and the
original finding was a separate contribution from the control work.

Worth stating for the list, since it is relevant to how much weight to give
these numbers: the link-frequency root cause here was not a guess. V4L2 defines
LINK_FREQ as the DDR clock, while the vendor mode descriptors publish the
per-lane BIT rate  --  twice the value. Every published figure for this sensor was
2x too high, isys ran the D-PHY at 721 Mbps against a sensor sending ~361, and
nothing framed. Correcting it to 180,480,000 produced the first frames anyone
has captured from an HM1092 on Linux, and that driver has been running here
since. Your independent derivation from the Qualcomm blob landing on the same
number by a different EXTCLK is what makes me confident it generalises.

The comparison data is here rather than inline:

  https://gist.github.com/jibsta210/db457ed67e24454e834b9638f6d38037

  hm1092-dell-vs-qualcomm.txt   the full diff, all 16 differing registers
  hm1092-dell-table.txt         238 (address, value) pairs regenerated from
                                the Dell hm1092.sys mode descriptor, in order

Regards,
Jake
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.