Skip to content

Commit cc5e917

Browse files
arch/xtensa/esp32s3: fix CAM DMA race and heap-allocate descriptors
- Remove debug polling loop from esp32s3_cam_start_capture() that busy-waited on DMA status register. - Fix DMA ISR race: stop DMA before invoking capture callback to prevent ISR re-triggering while callback processes the buffer. Check priv->capturing before restarting DMA in ISR. - Move DMA descriptors from struct to heap allocation, avoiding stack/BSS alignment issues with cache-line-aligned descriptors. Signed-off-by: wangjianyu3 <wangjianyu3@xiaomi.com>
1 parent 2a4c5c8 commit cc5e917

1 file changed

Lines changed: 83 additions & 57 deletions

File tree

arch/xtensa/src/esp32s3/esp32s3_cam.c

Lines changed: 83 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ struct esp32s3_cam_s
9090
uint8_t cpu;
9191
int32_t dma_channel;
9292

93-
struct esp32s3_dmadesc_s dmadesc[ESP32S3_CAM_DMADESC_NUM];
93+
struct esp32s3_dmadesc_s *dmadesc; /* Heap-allocated DMA descriptors */
9494

9595
uint8_t *fb; /* Frame buffer */
9696
uint32_t fb_size; /* Frame buffer size */
@@ -196,7 +196,13 @@ static int IRAM_ATTR cam_interrupt(int irq, void *context, void *arg)
196196
struct timeval ts;
197197
uint32_t regval;
198198

199-
/* Stop capture */
199+
/* Stop capture and DMA before invoking callback.
200+
* The callback may call set_buf() which rewrites DMA
201+
* descriptors. If DMA is still draining the CAM AFIFO
202+
* it could read half-written descriptors and follow a
203+
* corrupted pbuf pointer, writing pixel data over
204+
* unrelated memory (e.g. g_cam_priv.data.ops).
205+
*/
200206

201207
regval = getreg32(LCD_CAM_CAM_CTRL1_REG);
202208
regval &= ~LCD_CAM_CAM_START_M;
@@ -206,54 +212,72 @@ static int IRAM_ATTR cam_interrupt(int irq, void *context, void *arg)
206212
regval |= LCD_CAM_CAM_UPDATE_REG_M;
207213
putreg32(regval, LCD_CAM_CAM_CTRL_REG);
208214

215+
/* Stop DMA channel before callback to prevent race */
216+
217+
esp32s3_dma_reset_channel(priv->dma_channel, false);
218+
209219
gettimeofday(&ts, NULL);
210220

211221
/* Notify frame complete */
212222

213223
priv->cb(0, priv->fb_size, &ts, priv->cb_arg);
214224

215-
/* Restart capture for next frame */
225+
/* Check if callback called stop_capture. With a
226+
* single-buffer FIFO the V4L2 layer stops capture
227+
* inside the callback; restarting DMA after that
228+
* would run unsynchronized and corrupt memory.
229+
*/
216230

217-
priv->vsync_cnt = 0;
231+
if (!priv->capturing)
232+
{
233+
priv->vsync_cnt = 0;
234+
}
235+
else
236+
{
237+
/* Restart capture for next frame */
218238

219-
/* Reset DMA */
239+
priv->vsync_cnt = 0;
220240

221-
esp32s3_dma_reset_channel(priv->dma_channel, false);
241+
/* DMA channel was already reset before the
242+
* callback above. Reset CAM + AFIFO now.
243+
*/
222244

223-
/* Reset CAM + AFIFO */
245+
/* Reset CAM + AFIFO */
224246

225-
regval = getreg32(LCD_CAM_CAM_CTRL1_REG);
226-
regval |= LCD_CAM_CAM_RESET_M;
227-
putreg32(regval, LCD_CAM_CAM_CTRL1_REG);
228-
regval &= ~LCD_CAM_CAM_RESET_M;
229-
putreg32(regval, LCD_CAM_CAM_CTRL1_REG);
247+
regval = getreg32(LCD_CAM_CAM_CTRL1_REG);
248+
regval |= LCD_CAM_CAM_RESET_M;
249+
putreg32(regval, LCD_CAM_CAM_CTRL1_REG);
250+
regval &= ~LCD_CAM_CAM_RESET_M;
251+
putreg32(regval, LCD_CAM_CAM_CTRL1_REG);
230252

231-
regval |= LCD_CAM_CAM_AFIFO_RESET_M;
232-
putreg32(regval, LCD_CAM_CAM_CTRL1_REG);
233-
regval &= ~LCD_CAM_CAM_AFIFO_RESET_M;
234-
putreg32(regval, LCD_CAM_CAM_CTRL1_REG);
253+
regval |= LCD_CAM_CAM_AFIFO_RESET_M;
254+
putreg32(regval, LCD_CAM_CAM_CTRL1_REG);
255+
regval &= ~LCD_CAM_CAM_AFIFO_RESET_M;
256+
putreg32(regval, LCD_CAM_CAM_CTRL1_REG);
235257

236-
/* Re-set REC_DATA_BYTELEN after reset */
258+
/* Re-set REC_DATA_BYTELEN after reset */
237259

238-
regval &= ~LCD_CAM_CAM_REC_DATA_BYTELEN_M;
239-
regval |= ((ESP32S3_CAM_DMA_BUFLEN - 1)
240-
<< LCD_CAM_CAM_REC_DATA_BYTELEN_S);
241-
putreg32(regval, LCD_CAM_CAM_CTRL1_REG);
260+
regval &= ~LCD_CAM_CAM_REC_DATA_BYTELEN_M;
261+
regval |= ((ESP32S3_CAM_DMA_BUFLEN - 1)
262+
<< LCD_CAM_CAM_REC_DATA_BYTELEN_S);
263+
putreg32(regval, LCD_CAM_CAM_CTRL1_REG);
242264

243-
/* Reload DMA descriptors */
265+
/* Reload DMA descriptors */
244266

245-
esp32s3_dma_load(priv->dmadesc, priv->dma_channel, false);
246-
esp32s3_dma_enable(priv->dma_channel, false);
267+
esp32s3_dma_load(priv->dmadesc, priv->dma_channel,
268+
false);
269+
esp32s3_dma_enable(priv->dma_channel, false);
247270

248-
/* Restart */
271+
/* Restart */
249272

250-
regval = getreg32(LCD_CAM_CAM_CTRL1_REG);
251-
regval |= LCD_CAM_CAM_START_M;
252-
putreg32(regval, LCD_CAM_CAM_CTRL1_REG);
273+
regval = getreg32(LCD_CAM_CAM_CTRL1_REG);
274+
regval |= LCD_CAM_CAM_START_M;
275+
putreg32(regval, LCD_CAM_CAM_CTRL1_REG);
253276

254-
regval = getreg32(LCD_CAM_CAM_CTRL_REG);
255-
regval |= LCD_CAM_CAM_UPDATE_REG_M;
256-
putreg32(regval, LCD_CAM_CAM_CTRL_REG);
277+
regval = getreg32(LCD_CAM_CAM_CTRL_REG);
278+
regval |= LCD_CAM_CAM_UPDATE_REG_M;
279+
putreg32(regval, LCD_CAM_CAM_CTRL_REG);
280+
}
257281
}
258282
}
259283
}
@@ -512,6 +536,23 @@ static int esp32s3_cam_init(struct imgdata_s *data)
512536

513537
spin_lock_init(&priv->lock);
514538

539+
/* Allocate DMA descriptors from heap so they are isolated from
540+
* the driver struct. If GDMA ever follows a stale/corrupted
541+
* descriptor it will scribble on heap, not on g_cam_priv.
542+
*/
543+
544+
if (priv->dmadesc == NULL)
545+
{
546+
priv->dmadesc = kmm_memalign(4,
547+
sizeof(struct esp32s3_dmadesc_s) *
548+
ESP32S3_CAM_DMADESC_NUM);
549+
if (priv->dmadesc == NULL)
550+
{
551+
snerr("ERROR: Failed to allocate DMA descriptors\n");
552+
return -ENOMEM;
553+
}
554+
}
555+
515556
/* Configure GPIO pins */
516557

517558
esp32s3_cam_gpio_config();
@@ -522,7 +563,9 @@ static int esp32s3_cam_init(struct imgdata_s *data)
522563

523564
/* Configure CAM controller */
524565

525-
return esp32s3_cam_config(priv);
566+
int ret = esp32s3_cam_config(priv);
567+
568+
return ret;
526569
}
527570

528571
/****************************************************************************
@@ -554,6 +597,14 @@ static int esp32s3_cam_uninit(struct imgdata_s *data)
554597
priv->dma_channel = -1;
555598
}
556599

600+
/* Free DMA descriptors */
601+
602+
if (priv->dmadesc)
603+
{
604+
kmm_free(priv->dmadesc);
605+
priv->dmadesc = NULL;
606+
}
607+
557608
/* Free frame buffer */
558609

559610
if (priv->fb)
@@ -723,31 +774,6 @@ static int esp32s3_cam_start_capture(struct imgdata_s *data,
723774

724775
priv->capturing = true;
725776

726-
syslog(LOG_INFO, "CAM start: CTRL=0x%08lx CTRL1=0x%08lx ENA=0x%08lx\n",
727-
(unsigned long)getreg32(LCD_CAM_CAM_CTRL_REG),
728-
(unsigned long)getreg32(LCD_CAM_CAM_CTRL1_REG),
729-
(unsigned long)getreg32(LCD_CAM_LC_DMA_INT_ENA_REG));
730-
731-
/* Debug: poll RAW register to see if VSYNC appears */
732-
733-
for (int i = 0; i < 50; i++)
734-
{
735-
up_mdelay(20);
736-
uint32_t raw = getreg32(LCD_CAM_LC_DMA_INT_RAW_REG);
737-
uint32_t st = getreg32(LCD_CAM_LC_DMA_INT_ST_REG);
738-
if (raw || st)
739-
{
740-
syslog(LOG_INFO, "CAM poll[%d]: raw=0x%08lx st=0x%08lx\n",
741-
i, (unsigned long)raw, (unsigned long)st);
742-
break;
743-
}
744-
745-
if (i == 49)
746-
{
747-
syslog(LOG_INFO, "CAM poll: no VSYNC after 1s\n");
748-
}
749-
}
750-
751777
return OK;
752778
}
753779

0 commit comments

Comments
 (0)