Skip to content

hisilicon-osdrv-hi3516cv200: add GC2023 MIPI sensor variant - #2248

Open
phedoreanu wants to merge 1 commit into
OpenIPC:masterfrom
phedoreanu:gc2023-mipi
Open

hisilicon-osdrv-hi3516cv200: add GC2023 MIPI sensor variant#2248
phedoreanu wants to merge 1 commit into
OpenIPC:masterfrom
phedoreanu:gc2023-mipi

Conversation

@phedoreanu

Copy link
Copy Markdown

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.

Some GC2023 boards wire the sensor to the MIPI CSI-2 interface, not DVP
- e.g. VStarcam Hi3518EV200 cameras (2-lane RAW10). For those boards the
existing gc2023 entry in load_hisilicon is fatal: it muxes the parallel
VI pads (0x200f007c..0x200f0094) to VI_DATA/VI_VS/VI_HS, 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 (OpenIPC#2035/OpenIPC#2038), add a gc2023_mipi sensor
identity that only muxes I2C0 and sets the 24MHz sensor clock, plus a
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 tell for getting
this wrong is red and blue swapping). The plain gc2023 DVP entry is left
untouched.

Verified on a VStarcam Hi3518EV200: with this identity and ini, VI
IntCnt climbs and majestic delivers 1920x1080 at the sensor's native
timing. Note the shipped closed-source libsns_gc2023.so did not produce
frames on that unit (VENC timeout; it appears to program the sensor for
DVP output) - the working setup used the vendor's MIPI libsns. The
platform side this commit fixes is required either way, and the sensor
init details recovered from the vendor lib are documented in the
matching issue for a future open-source driver.
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

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

✨ Enhancement ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

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

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider

Great, no issues found!

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

Grey Divider

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 ↗

Grey Divider

Qodo Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant