Skip to content

Commit 6bc0fac

Browse files
widgetiiclaude
andauthored
kernel/isp/hi3516cv200: tasklet-defer openipc_frame_ts_push (closes #183) (#186)
Unblocks the gate from #182 and re-enables the openipc_frame_ts MIPI_FS hook on cv200 / hi3518ev200 without bricking /image.jpg. PR #155 (firmware bump #2126) introduced a synchronous openipc_frame_ts_push() call inside ISP_ISR's u32PortIntStatus branch; #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; #182 gated the cv200 hook off. The brick mechanism is NOT the rt_mutex_trylock WARN at rtmutex.c:1545 that #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 #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>
1 parent 14fc195 commit 6bc0fac

1 file changed

Lines changed: 57 additions & 17 deletions

File tree

  • kernel/isp/arch/hi3516cv200/firmware/drv

kernel/isp/arch/hi3516cv200/firmware/drv/isp.c

Lines changed: 57 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,42 @@ HI_U32 lsc_update_mode = 0;
134134

135135
spinlock_t g_stIspLock;
136136

137+
/*
138+
* Tasklet-deferred openipc_frame_ts_push for cv200 ISP_ISR.
139+
*
140+
* The cv200 ISR top half runs the entire ISP_IntBottomHalf synchronously
141+
* before returning, and that bottom half writes sensor exposure/gain over
142+
* i2c. Empirical reboot/power-cycle testing on the dlab hi3518ev200 bench
143+
* (10 power-cycles per state, JXF22 sensor, kernel 4.9.37) showed that
144+
* adding *any* extra work — even a fast spinlock_irqsave ring push —
145+
* inside the top half before the bottom half runs tips a timing race
146+
* downstream in the vendor VPSS/VENC chn 1 setup, raising the
147+
* /image.jpg HTTP-000 brick rate from 20% (baseline) to 60%. That's
148+
* why openhisilicon#155/#178 had to be gated off by #182.
149+
*
150+
* The race isn't the rt_mutex_trylock WARN that #183 originally blamed
151+
* (we confirmed that empirically by deferring i2c to a workqueue and
152+
* watching brick rate get *worse*). It's the µs budget itself.
153+
*
154+
* Fix: keep the ISR doing only a tasklet_hi_schedule() (a single bit
155+
* set + softirq raise, ~10s of cycles), and run openipc_frame_ts_push()
156+
* in the tasklet. Tasklets run in softirq context after the hardirq
157+
* returns, so the ISR hot path stays at near-zero added µs and the
158+
* downstream race doesn't fire.
159+
*
160+
* Timestamp precision trade-off: the push reads sched_clock() at
161+
* tasklet-run time (a few µs after the IRQ rather than inside it).
162+
* For 30 fps frame-edge events on a 33 ms cadence this is a negligible
163+
* shift — and openipc_frame_ts_push() already has a per-event-type
164+
* dedupe window so coalesced wake-ups don't double-fire.
165+
*/
166+
static struct tasklet_struct g_stIspFtTasklet;
167+
168+
static void ISP_FtTaskletFn(unsigned long data)
169+
{
170+
openipc_frame_ts_push((unsigned int)data, OPENIPC_FT_EVT_MIPI_FS);
171+
}
172+
137173
/****************************************************************************
138174
* INTERNAL FUNCTION DECLARATION *
139175
****************************************************************************/
@@ -2628,25 +2664,19 @@ static inline irqreturn_t ISP_ISR(int irq, void *id)
26282664
if (u32PortIntStatus)
26292665
{
26302666
/*
2631-
* openipc_frame_ts hook disabled on hi3516cv200 family pending
2632-
* a hardware-validation bench. The family includes hi3516cv200
2633-
* AND hi3518ev200 (same CHIPARCH=hi3516cv200, same open_isp.ko
2634-
* renamed to hi3518e_isp.ko at firmware install time). The hook
2635-
* adds a few µs to the ISR hot path which, on a real
2636-
* hi3518ev200 board, tips a latent i2c-from-hardirq race
2637-
* (rt_mutex_trylock WARN at rtmutex.c:1545 via
2638-
* hi_sensor_i2c_write → i2c_transfer); majestic can no longer
2639-
* read the sensor and /image.jpg returns HTTP 000.
2667+
* cv200's MIPI RX hardware doesn't expose a usable vsync bit,
2668+
* so we report this ISP-port FSTART IRQ as MIPI_FS-equivalent.
2669+
* It fires at the same pipeline point (frame start) as the
2670+
* MIPI_FS path on V4 SoCs. cv200 doesn't have an FEND IRQ
2671+
* source we can hook today — see kernel/isp/arch/hi3516cv200
2672+
* for the ISP register map.
26402673
*
2641-
* Re-enable here only after the i2c-in-IRQ path is fixed
2642-
* upstream OR after we've confirmed on real hi3518ev200 and
2643-
* hi3516cv200 hardware that the hook doesn't perturb the
2644-
* sensor i2c timing budget.
2645-
*
2646-
* Tracked: openipc/firmware#2128 (revert of opensdk bump),
2647-
* openhisilicon#178 follow-up.
2674+
* Deferred to a tasklet so the ISR hot path stays at near-zero
2675+
* added µs — see g_stIspFtTasklet comment near the top of this
2676+
* file for why direct openipc_frame_ts_push() here bricked the
2677+
* camera ~60% of boots (openhisilicon#155/#178/#182/#183).
26482678
*/
2649-
/* openipc_frame_ts_push(IspDev, OPENIPC_FT_EVT_MIPI_FS); */
2679+
tasklet_hi_schedule(&g_stIspFtTasklet);
26502680
HW_REG(IO_ADDRESS_PORT(VI_PT0_INT)) = VI_PT0_INT_FSTART;
26512681
}
26522682
/*When detect vi port's width&height changed,then reset isp*/
@@ -2968,6 +2998,11 @@ static int ISP_DRV_Init(void)
29682998
tasklet_init(&g_astIspDrvCtx[0].stIntSch.tsklet, ISP_IntBottomHalf, (unsigned long)&g_astIspDrvCtx[0]);
29692999
}
29703000

3001+
/* IspDev is always 0 on cv200 (single ISP). Passing it as the
3002+
* tasklet data lets the tasklet hand it to openipc_frame_ts_push()
3003+
* as the vi_chn parameter, matching the V4 / cv500 channel mapping. */
3004+
tasklet_init(&g_stIspFtTasklet, ISP_FtTaskletFn, 0);
3005+
29713006
/* alloc isp stat shandow mem for application use */
29723007
g_astIspDrvCtx[0].stStatShadowMem.u32Size = sizeof(ISP_STAT_S);
29733008
s32Ret = CMPI_MmzMallocNocache(HI_NULL, "ISP shadow mem", &g_astIspDrvCtx[0].stStatShadowMem.u32PhyAddr,
@@ -2987,6 +3022,11 @@ static int ISP_DRV_Init(void)
29873022
static int ISP_DRV_Exit(void)
29883023
{
29893024
free_irq(isp_irq, (void*)&g_astIspDrvCtx);
3025+
/* tasklet_kill must run *after* free_irq so no fresh IRQs can
3026+
* re-schedule it. It waits for any pending run to finish, so on
3027+
* return the tasklet is fully quiesced and safe to drop with the
3028+
* module. */
3029+
tasklet_kill(&g_stIspFtTasklet);
29903030
//iounmap((void*)reg_vicap_base_va);
29913031
//iounmap((void*)reg_isp_base_va);
29923032
ISP_ACM_DRV_Exit();

0 commit comments

Comments
 (0)