kernel/isp/hi3516cv200: tasklet-defer openipc_frame_ts_push (closes #183, unblocks gate from #182)#186
Merged
widgetii merged 1 commit intoMay 24, 2026
Conversation
…penIPC#183) Unblocks the gate from OpenIPC#182 and re-enables the openipc_frame_ts MIPI_FS hook on cv200 / hi3518ev200 without bricking /image.jpg. PR OpenIPC#155 (firmware bump #2126) introduced a synchronous openipc_frame_ts_push() call inside ISP_ISR's u32PortIntStatus branch; OpenIPC#178 extended it. The few µs added to the cv200 ISR top half tipped a timing race downstream in the vendor VPSS / VENC chn 1 startup, raising the /image.jpg HTTP-000 brick rate on hi3518ev200 from ~20 % (latent baseline) to ~60 %. firmware#2128 urgently reverted; OpenIPC#182 gated the cv200 hook off. The brick mechanism is NOT the rt_mutex_trylock WARN at rtmutex.c:1545 that OpenIPC#183 originally blamed. We confirmed empirically: deferring the i2c writes to a workqueue (cv500-pattern) silences the WARN but makes the brick *worse* (40-60 % rate). The race is purely about µs cost in the cv200 ISR hot path before the synchronous ISP_IntBottomHalf runs. Fix: replace the direct openipc_frame_ts_push() with tasklet_hi_schedule(). tasklet_hi_schedule() from hardirq is ~10s of cycles (single bit set + softirq raise); the actual push then runs in softirq context after the hardirq returns, so the ISR hot path stays at near-zero added µs and the downstream race doesn't fire. Empirical validation on dlab hi3518ev200 (JXF22 sensor, kernel 4.9.37), 10 power-cycles per state, /image.jpg http_code via init-script-started majestic: | state | success | brick | | baseline (no hook, with OpenIPC#182 gate on) | 8/10 | 20% | | push direct in ISR + cv500-style i2c defer | 4/10 | 60% | | tasklet defer for push + cv500-style i2c defer | 6/10 | 40% | | this patch — tasklet defer alone (no i2c changes) | 10/10 | 0% | The tasklet-defer-alone fix is BETTER than baseline (which still intermittently bricks 20 % from the pre-frame_ts era) AND keeps the frame_ts hook enabled on cv200. Timestamp precision: the push reads sched_clock() at tasklet-run time (~µs after the IRQ rather than inside it). Negligible for 30 fps frame-edge events on a 33 ms cadence. openipc_frame_ts_push() already has a per-event-type dedupe window absorbing any coalescing from multiple ISR firings between tasklet runs. The rt_mutex_trylock WARN still fires once per boot on cv200 — it's cosmetic, not the brick cause. A cv500-pattern workqueue defer for i2c-from-hardirq to silence the WARN was prototyped and tested; it regresses the brick rate so it does NOT land here. Tracked as future work in openhisilicon#185. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
830bb2e to
c684130
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Unblocks the gate from #182 and re-enables the
openipc_frame_tsMIPI_FS hook on cv200 / hi3518ev200 without bricking/image.jpg. Single-file patch (+57/-17) inkernel/isp/arch/hi3516cv200/firmware/drv/isp.c.What broke and why
PR #155 (firmware bump #2126) introduced a synchronous
openipc_frame_ts_push()call inside the cv200ISP_ISRtop half; #178 extended it. The few µs added to the ISR hot path tipped a timing race downstream in vendor VPSS / VENC chn 1 startup, raising the/image.jpgHTTP-000 brick rate on hi3518ev200 from ~20 % (latent baseline) to ~60 %. firmware#2128 reverted; #182 gated the cv200 hook off as a backstop.The brick mechanism is not the
rt_mutex_trylockWARN atrtmutex.c:1545that #183 originally blamed. Empirically: deferring the i2c writes to a workqueue (cv500-pattern) silences the WARN but makes the brick worse (40-60 % rate). The race is purely about µs cost in the cv200 ISR top half before the synchronousISP_IntBottomHalfruns.Fix
Replace the direct
openipc_frame_ts_push()withtasklet_hi_schedule(). From hardirq this is ~10s of cycles (a single bit set + softirq raise); the actual push then runs in softirq context after the hardirq returns, so the ISR hot path stays at near-zero added µs and the downstream race doesn't fire.Timestamp precision: the push reads
sched_clock()at tasklet-run time (~µs after the IRQ rather than inside it). Negligible for 30 fps frame-edge events on a 33 ms cadence.openipc_frame_ts_push()already has a per-event-type dedupe window absorbing any coalescing from multiple ISR firings between tasklet runs.Empirical validation
Dlab hi3518ev200 bench (JXF22 sensor, kernel 4.9.37), 10 power-cycles per state,
/image.jpghttp_code via init-script-started majestic:The tasklet-defer-alone fix is better than baseline (which still intermittently bricks 20 % from the pre-frame_ts era) AND keeps the frame_ts hook enabled on cv200.
What this PR does NOT do
kernel/sensor_i2c/hi3516cv200/sensor_i2c.c. The cv500-pattern workqueue defer for i2c-from-hardirq was tested and regresses brick rate; it's tracked as future work in cv200 sensor_i2c.c: port cv500-style workqueue defer to silence the rt_mutex_trylock WARN #185.rt_mutex_trylockWARN atrtmutex.c:1545— that's cosmetic, fires once per boot on cv200, not the brick cause.osal_request_irqwithOSAL_IRQ_WAKE_THREAD).Test plan
References
🤖 Generated with Claude Code