hi3516ev200/sc2235: repeat DVP pad setup after stream start

[HeytalePazguato]

What

Four register writes added at the end of sc2235_linear_1080p30_init() (hi3516ev200): after the existing stream start, repeat the DVP pad setup (0x3d08=0x01, 0x3640=0x01, 0x3641=0x02) and start streaming again (0x0100=0x01).

Nothing else in the init table changes.

Why

On the Imou Cue 2 (IPC-C22EN: Hi3516EV200 + SC2235 on the DVP pads) the current sequence alone gives the SoC no pixel clock: the sensor answers on I2C and initialises, but /proc/umap/vi never counts a frame interrupt.

Where the sequence comes from

The camera's stock firmware (Dahua LiteOS). Its SC2235 init table (114 {u16 reg, u16 val} entries in the firmware blob) writes these three pad registers after the first 0x0100=0x01 and then writes 0x0100=0x01 a second time. There is no datasheet reasoning behind the exact values; they are the vendor's.

This PR is only that delta, as asked in OpenIPC/firmware#2446. The first version of that PR replaced the whole table with the vendor one; that is dropped.

Testing

• Built into a hi3516ev200 image (OpenIPC/firmware master + Add Imou Cue 2 (IPC-C22EN) device profile builder#160), flashed to one Imou Cue 2 with the overlay erased: video from cold boot with no runtime scripts, streams into Frigate, night mode, two-way audio.
• Not tested on any other SC2235 board. libsns_sc2235 for hi3516ev200 is also used by gk7205v200 (HISILICON_OPENSDK_SENSORS_gk7205v200), so those boards get the extra writes too.

Related: OpenIPC/firmware#2446, OpenIPC/builder#160.

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

PR Summary by Qodo

Restore SC2235 DVP pixel clock on Imou Cue 2

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Repeat vendor DVP pad writes after stream start to restore the Imou Cue 2 pixel clock.
• Leave the existing initialization table intact; the shared sensor library also serves other
 boards.

Diagram

sequenceDiagram
    participant Init as SC2235 init
    participant Sensor as SC2235 sensor
    participant VI as SoC VI
    Init->>Sensor: Start streaming
    Init->>Sensor: Repeat DVP pad setup
    Init->>Sensor: Reassert stream on
    Sensor-->>VI: Pixel clock and frames

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Gate the extra writes by board

• ➕ Avoids changing initialization on other boards using the shared sensor library.
• ➖ Requires a reliable board-identification or configuration path and additional integration work.

Recommendation: Keep the narrowly scoped vendor-derived sequence: it fixes the tested board without replacing the established register table. Because the library is shared, validate other SC2235 boards when possible and consider board-specific gating if a regression appears.

Files changed (1) +10 / -0

Bug fix (1) +10 / -0

sc2235_sensor_ctl.cRepeat DVP pad configuration after initial stream start +10/-0

Repeat DVP pad configuration after initial stream start

• Adds a 20 ms delay, writes the vendor DVP pad values to registers 0x3d08, 0x3640, and 0x3641, then writes stream-on again. The earlier initialization sequence remains unchanged.

libraries/sensor/hi3516ev200/smart_sc2235/sc2235_sensor_ctl.c

[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 turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[openipc-ai]

Thanks for tracking this down. The change itself looks fine, but one part of the description doesn't hold.

This doesn't reach gk7205v200 boards. The description says libsns_sc2235 for hi3516ev200 is shared with gk7205v200. It isn't: gk7205v200 has its own copy at libraries/sensor/gk7205v200/smart_sc2235/sc2235_sensor_ctl.c, which this PR doesn't touch.

• libraries/Makefile has no sensor filter for gk7205v200, so both trees are built.
• The firmware's hisilicon-opensdk.mk installs gk7205v200 sensor libraries from libraries/sensor/$(OPENIPC_SOC_FAMILY)/, which is the unchanged copy. HISILICON_OPENSDK_SENSORS_gk7205v200 only reuses the hi3516ev200 list of sensor names, not the files.

So a GK7205V200 board with an SC2235 on the DVP pads and the same missing pixel clock will still get no VI interrupts after this merges. The fix actually reaches hi3516ev200 boards only, and the two copies, which are byte-identical on main today, start to diverge.

Suggestion: unify the driver rather than patching both copies. libraries/sensor/hi3516ev200/ and libraries/sensor/gk7205v200/ are byte-identical on main: all 30 drivers, with diff -rq reporting nothing. The Goke naming is already handled by include/hicompat.h. #230 did this for gk7205v500: it deleted the duplicated driver and pointed CHIPARCH=gk7205v500 at sensor/hi3516ev200 via a SUBDIRS filter in libraries/Makefile, so a sensor fix lands once for every V4 target. That PR also shows why the split is risky: the MIS2008 Dgain out-of-bounds read it fixed was in the shared V4 source, and with separate copies it would have to be fixed twice. The same approach for gk7205v200 would mean:

1. Add a gk7205v200 clause to libraries/Makefile that filters SUBDIRS to ./sensor/hi3516ev200/%.
2. Remove libraries/sensor/gk7205v200/.
3. Point the gk7205v200 sensor install in the firmware's hisilicon-opensdk.mk at sensor/hi3516ev200, the same way the gk7205v500 block does.

If you'd rather keep this PR small, applying the same change to libraries/sensor/gk7205v200/smart_sc2235/sc2235_sensor_ctl.c (and correcting the description) would do for now, with the unification done as a follow-up.

Minor, not blocking:

• 0x3d08=0x01 repeats the write already made at line 329.
• 0x3640 and the stronger 0x3641=0x02 pad drive now apply to every hi3516ev200 SC2235 board, but they were tested on one board. If you can confirm on a second board, or on a module where the pixel clock already worked, that would lower the risk of breaking other boards.

[openipc-ai]

Retracting my main finding — it was wrong, and I am sorry for the noise.

I claimed gk7205v200 has its own copy of this driver at libraries/sensor/gk7205v200/smart_sc2235/sc2235_sensor_ctl.c and that this PR therefore reaches hi3516ev200 only. That is not true. libraries/sensor/gk7205v200 is a symlink to hi3516ev200, not a directory:

$ git ls-tree HEAD libraries/sensor/gk7205v200
120000 blob b7803faa97109bd8c6b8a298907f812e215aaf9b	libraries/sensor/gk7205v200
$ git cat-file -p b7803fa
hi3516ev200
$ ls -i libraries/sensor/{gk7205v200,hi3516ev200}/smart_sc2235/sc2235_sensor_ctl.c
99248001 libraries/sensor/gk7205v200/smart_sc2235/sc2235_sensor_ctl.c
99248001 libraries/sensor/hi3516ev200/smart_sc2235/sc2235_sensor_ctl.c

Same inode. There is one file, and libraries/sensor/$(OPENIPC_SOC_FAMILY)/ in the firmware's hisilicon-opensdk.mk resolves through the symlink to it. So this PR already reaches gk7205v200, and your description was right — it is mine that was wrong. The unification I suggested has already been done, with a symlink rather than a SUBDIRS filter, which is also why libraries/Makefile has no gk7205v200 clause and nothing looked out of place.

Please do not act on that suggestion. Adding a second copy of the file, or replacing the symlink with a directory, would create exactly the divergence I was worried about. Nothing here needs changing on that account.

How I got it wrong is worth recording: I checked with diff -rq libraries/sensor/hi3516ev200 libraries/sensor/gk7205v200, which reported no differences and which I read as "two identical copies". That command cannot tell a copy from a symlink to the same directory — it was comparing the tree with itself, so it could only ever report nothing. ls -la would have settled it in one line.

Also withdrawing the 0x3d08 note. I said it repeats the write already made at line 329. It does, and that is the point — the whole mechanism of this PR is repeating the pad setup after the sensor is streaming. Consistent, not redundant.

What still stands, and it is the only thing: 0x3640=0x01 and the stronger 0x3641=0x02 now apply to every hi3516ev200 SC2235 board, on the evidence of one camera. A check on a second board — ideally one where the pixel clock already worked, so a regression would show — is what would make this safe to land for everyone else. If nobody has one to hand, saying so is a reasonable answer too; the current behaviour on those boards is a sensor that works, so the risk is real but small and the fix is real and needed.

[openipc-ai]

Withdrawing the last note too — the risk it describes has no target.

I asked for a check on a second hi3516ev200 SC2235 board, on the grounds that 0x3640=0x01 and the stronger 0x3641=0x02 would reach every board using that sensor. Having gone looking for those boards, there are none:

• no br-ext-chip-hisilicon or br-ext-chip-goke defconfig names sc2235
• across every device in OpenIPC/builder, not one customizer.sh sets fw_setenv sensor sc2235
• the only two mentions in builder are exclusion lists, which prune libsns_sc2235.so to save flash

libsns_sc2235.so is built and shipped for this family because it is in the shared V4 sensor name list, but no camera selects it. The Imou Cue 2 is the first OpenIPC device to use this sensor, so there is no existing board with a working pixel clock that the stronger pad drive could regress — the objection I raised only bites when something is already relying on the old values, and nothing is.

There is also no hi3516ev200 board in our lab to test on, so a second-board check was never going to come from this side regardless.

So: nothing outstanding from me. Approving and merging. Sorry again for the detour on the gk7205v200 point — the symlink was there to be seen and I did not look properly.

[openipc-ai]

Approving. The delta is the right shape — the stock sequence is left alone and the DVP pad setup is repeated after the sensor is streaming, which is what the camera needs and what its vendor firmware does. 35 checks green.

Both of my earlier findings are withdrawn above: libraries/sensor/gk7205v200 is a symlink to hi3516ev200, so this already reaches that family and the description was accurate; and no OpenIPC configuration selects SC2235 today, so the stronger pad drive cannot regress an existing board.