hisilicon-osdrv-hi3516cv200: add GC2023 MIPI sensor variant

[phedoreanu]

Some GC2023 boards wire the sensor to MIPI CSI-2, not DVP — e.g. VStarcam Hi3518EV200 pan/tilt cameras (2-lane RAW10). For those boards the existing gc2023 entry in load_hisilicon is fatal: it muxes the parallel VI pads (0x200f007c..0x200f0094), which disables the MIPI PHY. The tell is /proc/umap/vi showing IntCnt=0 with TmgErr=0 and ccErrN=0 — VI receives no VSYNC at all, while bad ini timings would at least raise the error counters.

Following the OV2710 precedent (#2035/#2038), this adds:

• a gc2023_mipi sensor identity that only muxes I2C0 and sets the 24 MHz sensor clock — the plain gc2023 DVP entry is untouched;
• gc2023_mipi_1080p.ini, cloned from ov2735_mipi_1080p.ini (same SoC, same 2-lane RAW10 MIPI geometry) with the sensor identity swapped and Isp_Bayer corrected to 0 (RGGB — GC2023 is SRGGB10; the symptom of getting this wrong is red and blue swapping).

Verified on a VStarcam Hi3518EV200: with this identity and ini, VI IntCnt climbs and majestic delivers 1920×1080 at the sensor's native timing.

One honest caveat, detailed in #2243: the shipped closed-source libsns_gc2023.so did not produce frames on the tested unit (VENC timeout; it appears to program the sensor for DVP output) — the working setup used the vendor's MIPI libsns plus an init-table replay. The platform side fixed here is required either way, and #2243 carries the full recovered init table (123 writes, including the digital-gain 0xb1 trap) toward an open-source driver.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Add GC2023 MIPI (CSI-2) sensor variant for Hi3516CV200

✨ Enhancement ⚙️ Configuration changes 🕐 10-20 Minutes

AI Description

• Add new gc2023_mipi sensor identity to preserve MIPI pinmux/PHY operation.
• Introduce a 1080p MIPI RAW10 INI profile with correct GC2023 Bayer order.
• Keep existing gc2023 (DVP) behavior unchanged for parallel-wired boards.

Diagram

graph TD
  A[/"User selects sensor: gc2023_mipi"/] --> B["load_hisilicon (insert_sns)" ] --> C[("SoC pinmux/clock regs")]
  B --> D[/"gc2023_mipi_1080p.ini"/]
  D --> E["libsns_gc2023.so" ] --> F(("GC2023 sensor"))
  C --> G["VI + MIPI CSI-2" ]
  F --> G

  subgraph Legend
    direction LR
    _cfg[/"Config (.ini/env)"/] ~~~ _svc["Script/driver glue" ] ~~~ _hw[("HW regs") ] ~~~ _sns(("Sensor") )
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Auto-detect MIPI vs DVP at boot

• ➕ No need for separate gc2023_mipi identity; fewer user-facing sensor names.
• ➕ Could reduce misconfiguration risk on unknown boards.
• ➖ Detection is platform- and driver-dependent (PHY status, /proc counters), potentially flaky early-boot.
• ➖ Adds complexity to a critical init script path and raises risk of false positives.

2. Make `gc2023` conditional on INI input_mode

• ➕ Single sensor name while allowing both MIPI and DVP wiring.
• ➕ Keeps configuration centralized in INI.
• ➖ load_hisilicon currently keys off sensor identity and does not parse INI; would require new parsing logic or refactor.
• ➖ More invasive change with higher regression potential for existing boards.

Recommendation: Keep the PR’s approach (add a distinct gc2023_mipi identity plus a matching INI). It’s the lowest-risk fix because it avoids changing the existing gc2023 (DVP) behavior while preventing the fatal DVP pad muxing on MIPI-wired boards. Alternatives (auto-detection or INI-driven conditional pinmux) are feasible but would be more complex and riskier in early-boot hardware init.

Files changed (2) +290 / -2

Enhancement (1) +6 / -2

load_hisiliconAdd 'gc2023_mipi' to MIPI-safe pinmux/clock init path +6/-2

Add 'gc2023_mipi' to MIPI-safe pinmux/clock init path

• Extends the MIPI-variant sensor case list to include 'gc2023_mipi', ensuring only I2C0 muxing and sensor clock setup are applied. Adds inline documentation explaining that the existing 'gc2023' entry muxes DVP pads and can disable the MIPI PHY on CSI-2-wired boards.

general/package/hisilicon-osdrv-hi3516cv200/files/script/load_hisilicon

Other (1) +284 / -0

gc2023_mipi_1080p.iniIntroduce GC2023 MIPI 1080p RAW10 sensor profile +284/-0

Introduce GC2023 MIPI 1080p RAW10 sensor profile

• Adds a new INI profile for GC2023 over 2-lane MIPI CSI-2 at 1920x1080, including lane configuration and VI/VPSS/VENC defaults. Sets 'Isp_Bayer=0' (RGGB) to match GC2023 (SRGGB10) and avoid red/blue channel swapping.

general/package/hisilicon-osdrv-hi3516cv200/files/sensor/config/gc2023_mipi_1080p.ini

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can ask Qodo to dismiss a finding you disagree with, with your reason on record

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[flyrouter]

Thanks.
It looks like an interesting proposal for improvement.

[widgetii]

Thanks — this is unusually well-evidenced work, and the writeup in #2243 is genuinely useful material. I reviewed both halves separately, because they have very different risk and value profiles.

The load_hisilicon half: correct, and I want it

No regression risk, verified rather than assumed:

• case matching is exact-glob and the DVP gc2023 arm comes first, so no existing sensor value changes behaviour.
• Kernel side: open_sys_config for cv200 carries its own duplicate pinmux table (kernel/sys_config/hi3516cv200/sys_config.c) and falls through to printk("sensor_type '%s' is error!!!") for names it does not know — non-fatal, and the script's devmem writes run afterwards and win. gc2023, ov2735 and several others already hit that same branch today.
• The vendor hi3518e_sys.ko takes sensor as a bare charp with no name table, so an unknown value is inert there too.
• Affected defconfigs are hi3516cv200_lite, hi3518ev200_lite, hi3518ev200_ultimate, hi3518ev201_lite. The new .ini is installed unconditionally by the *.ini glob in the .mk: ~10 KB raw, ~3 KB compressed. Irrelevant even on the tight lite boards.

One asymmetry worth flagging: the OV2710 precedent you follow (#2038) updated both sys_config.c in openhisilicon and load_hisilicon. This PR only does the latter. That is harmless, but it means dmesg now gains a sensor_type 'gc2023_mipi' is error!!! line, which looks alarming to anyone reading a boot log. Adding the arm to sys_config.c as well would keep the two tables in sync and keep the log clean.

The .ini half: one real objection, and it is the header

From your own table in #2243, on the same unit with the same MIPI ini and the same fixed pinmux:

library	frames
vendor MIPI libsns_gc2023.so	yes
vendor DVP libsns_gc2023_dvp.so	none (VENC timeout)
OpenIPC libsns_gc2023.so	none (VENC timeout)

DllFile in this file points at row three. So anyone who follows this file's own instructions gets a VENC timeout. That is fine — it is a platform-side template, and it is useful to someone who supplies their own MIPI libsns and edits one line — but the header currently says "Verified on a VStarcam Hi3518EV200 board with the GC2023 wired to MIPI", which is not true of this file with this DllFile. Verification was done with a library that is not the one named here. That comment will cost somebody a day.

Before merge I would like just the header corrected (suggestion inline). The frame-rate and Bayer notes are questions, not blockers.

On "what is the point if the camera still does not work"

Answering it here since it will come up. The platform half is a prerequisite that no library can work without — without it the MIPI PHY is dead and the sensor is unreachable on these boards regardless of what userspace you throw at it — and it costs nothing. The .ini is scaffolding that the open driver your init table in #2243 makes possible will plug straight into. Both are worth having, as long as the file is honest about what does and does not work today. That is the whole of my ask.

[phedoreanu]

All asks addressed, and the naming experiment produced a more interesting answer than either of us guessed.

Header (b168796): your text taken essentially verbatim. The file now leads with the NOTE that the OpenIPC-shipped libsns_gc2023.so does not stream on a MIPI-wired board and that DllFile must point at a GC2023 MIPI library OpenIPC does not currently ship; verification credit goes to the vendor MIPI libsns, with a pointer to #2243. The same commit adds the Isp_FrameRate note and fixes the "below"/"above" slip in the load_hisilicon comment.

The naming experiment, run on the unit:

# fw_setenv sensor gc2023_mipi
# ipcinfo -s
gc2023
# majestic -s   (foreground)
DEBUG [sdk] try_sensor_config@5428: Autodetected sensor as 'gc2023_i2c'
DEBUG [sdk] find_sensor_config@5388: matched sensor config: gc2023_i2c_1080p.ini

The search key does not come from the sensor env at all: majestic autodetects by I2C probe and derives gc2023_i2c even with sensor=gc2023_mipi set. So gc2023_mipi_i2c_1080p.ini would not auto-resolve either — the only auto-matching name is a gc2023_i2c* prefix, and that key cannot distinguish a MIPI-wired board from a DVP-wired one (the suffix records how majestic talks to the sensor, not how video leaves it). Since this file needs a manually supplied MIPI libsns anyway, staying off the auto-match path and requiring the explicit cli -s seems right — the same situation ov2710_mipi_1080p.ini / ov2710_dc_1080p.ini are in against their probed ov2710_i2c key. Name kept.

sys_config.c asymmetry: fixed for real rather than flagged — OpenIPC/openhisilicon#207 adds the gc2023_mipi arm as a mirror of the ov2710_mipi one, so a gc2023_mipi boot loses the sensor_type 'gc2023_mipi' is error!!! line and the two pinmux tables stay in sync.

Isp_FrameRate 25 vs 30: a leftover of the clone — but a leftover the verification ran with. The working vendor-libsns session used this file with 25 in place, while the recovered init table is sensor_linear_1080p30_init. Your theory fits the AE oddities in #2243 (MaxLine=2, Error=-196), so I noted the tension next to the value rather than silently flipping to an untested 30; it should be revisited together with the AE limits once an open MIPI driver exists (#2243 carries the full init table for that).

And thank you for the closing paragraph — "prerequisite plus scaffolding, as long as the file is honest" is exactly the framing I will steal for the next one of these.