This is an automated email from the ASF dual-hosted git repository. acassis pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit f92b3d8d7947ec54259b6e17263e0e7ea2626406 Author: wangjianyu3 <[email protected]> AuthorDate: Sat Mar 21 10:00:00 2026 +0800 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 <[email protected]> --- arch/xtensa/src/esp32s3/esp32s3_cam.c | 136 ++++++++++++++++++++-------------- 1 file changed, 80 insertions(+), 56 deletions(-) diff --git a/arch/xtensa/src/esp32s3/esp32s3_cam.c b/arch/xtensa/src/esp32s3/esp32s3_cam.c index 969f887afb7..01aa3b34429 100644 --- a/arch/xtensa/src/esp32s3/esp32s3_cam.c +++ b/arch/xtensa/src/esp32s3/esp32s3_cam.c @@ -89,7 +89,7 @@ struct esp32s3_cam_s uint8_t cpu; int32_t dma_channel; - struct esp32s3_dmadesc_s dmadesc[ESP32S3_CAM_DMADESC_NUM]; + struct esp32s3_dmadesc_s *dmadesc; /* Heap-allocated DMA descriptors */ uint8_t *fb; /* Frame buffer */ uint32_t fb_size; /* Frame buffer size */ @@ -195,7 +195,13 @@ static int IRAM_ATTR cam_interrupt(int irq, void *context, void *arg) struct timeval ts; uint32_t regval; - /* Stop capture */ + /* Stop capture and DMA before invoking callback. + * The callback may call set_buf() which rewrites DMA + * descriptors. If DMA is still draining the CAM AFIFO + * it could read half-written descriptors and follow a + * corrupted pbuf pointer, writing pixel data over + * unrelated memory (e.g. g_cam_priv.data.ops). + */ regval = getreg32(LCD_CAM_CAM_CTRL1_REG); regval &= ~LCD_CAM_CAM_START_M; @@ -205,54 +211,72 @@ static int IRAM_ATTR cam_interrupt(int irq, void *context, void *arg) regval |= LCD_CAM_CAM_UPDATE_REG_M; putreg32(regval, LCD_CAM_CAM_CTRL_REG); + /* Stop DMA channel before callback to prevent race */ + + esp32s3_dma_reset_channel(priv->dma_channel, false); + gettimeofday(&ts, NULL); /* Notify frame complete */ priv->cb(0, priv->fb_size, &ts, priv->cb_arg); - /* Restart capture for next frame */ + /* Check if callback called stop_capture. With a + * single-buffer FIFO the V4L2 layer stops capture + * inside the callback; restarting DMA after that + * would run unsynchronized and corrupt memory. + */ - priv->vsync_cnt = 0; + if (!priv->capturing) + { + priv->vsync_cnt = 0; + } + else + { + /* Restart capture for next frame */ - /* Reset DMA */ + priv->vsync_cnt = 0; - esp32s3_dma_reset_channel(priv->dma_channel, false); + /* DMA channel was already reset before the + * callback above. Reset CAM + AFIFO now. + */ - /* Reset CAM + AFIFO */ + /* Reset CAM + AFIFO */ - regval = getreg32(LCD_CAM_CAM_CTRL1_REG); - regval |= LCD_CAM_CAM_RESET_M; - putreg32(regval, LCD_CAM_CAM_CTRL1_REG); - regval &= ~LCD_CAM_CAM_RESET_M; - putreg32(regval, LCD_CAM_CAM_CTRL1_REG); + regval = getreg32(LCD_CAM_CAM_CTRL1_REG); + regval |= LCD_CAM_CAM_RESET_M; + putreg32(regval, LCD_CAM_CAM_CTRL1_REG); + regval &= ~LCD_CAM_CAM_RESET_M; + putreg32(regval, LCD_CAM_CAM_CTRL1_REG); - regval |= LCD_CAM_CAM_AFIFO_RESET_M; - putreg32(regval, LCD_CAM_CAM_CTRL1_REG); - regval &= ~LCD_CAM_CAM_AFIFO_RESET_M; - putreg32(regval, LCD_CAM_CAM_CTRL1_REG); + regval |= LCD_CAM_CAM_AFIFO_RESET_M; + putreg32(regval, LCD_CAM_CAM_CTRL1_REG); + regval &= ~LCD_CAM_CAM_AFIFO_RESET_M; + putreg32(regval, LCD_CAM_CAM_CTRL1_REG); - /* Re-set REC_DATA_BYTELEN after reset */ + /* Re-set REC_DATA_BYTELEN after reset */ - regval &= ~LCD_CAM_CAM_REC_DATA_BYTELEN_M; - regval |= ((ESP32S3_CAM_DMA_BUFLEN - 1) - << LCD_CAM_CAM_REC_DATA_BYTELEN_S); - putreg32(regval, LCD_CAM_CAM_CTRL1_REG); + regval &= ~LCD_CAM_CAM_REC_DATA_BYTELEN_M; + regval |= ((ESP32S3_CAM_DMA_BUFLEN - 1) + << LCD_CAM_CAM_REC_DATA_BYTELEN_S); + putreg32(regval, LCD_CAM_CAM_CTRL1_REG); - /* Reload DMA descriptors */ + /* Reload DMA descriptors */ - esp32s3_dma_load(priv->dmadesc, priv->dma_channel, false); - esp32s3_dma_enable(priv->dma_channel, false); + esp32s3_dma_load(priv->dmadesc, priv->dma_channel, + false); + esp32s3_dma_enable(priv->dma_channel, false); - /* Restart */ + /* Restart */ - regval = getreg32(LCD_CAM_CAM_CTRL1_REG); - regval |= LCD_CAM_CAM_START_M; - putreg32(regval, LCD_CAM_CAM_CTRL1_REG); + regval = getreg32(LCD_CAM_CAM_CTRL1_REG); + regval |= LCD_CAM_CAM_START_M; + putreg32(regval, LCD_CAM_CAM_CTRL1_REG); - regval = getreg32(LCD_CAM_CAM_CTRL_REG); - regval |= LCD_CAM_CAM_UPDATE_REG_M; - putreg32(regval, LCD_CAM_CAM_CTRL_REG); + regval = getreg32(LCD_CAM_CAM_CTRL_REG); + regval |= LCD_CAM_CAM_UPDATE_REG_M; + putreg32(regval, LCD_CAM_CAM_CTRL_REG); + } } } } @@ -497,6 +521,23 @@ static int esp32s3_cam_init(struct imgdata_s *data) spin_lock_init(&priv->lock); + /* Allocate DMA descriptors from heap so they are isolated from + * the driver struct. If GDMA ever follows a stale/corrupted + * descriptor it will scribble on heap, not on g_cam_priv. + */ + + if (priv->dmadesc == NULL) + { + priv->dmadesc = kmm_memalign(4, + sizeof(struct esp32s3_dmadesc_s) * + ESP32S3_CAM_DMADESC_NUM); + if (priv->dmadesc == NULL) + { + snerr("ERROR: Failed to allocate DMA descriptors\n"); + return -ENOMEM; + } + } + /* Configure GPIO pins */ esp32s3_cam_gpio_config(); @@ -539,6 +580,14 @@ static int esp32s3_cam_uninit(struct imgdata_s *data) priv->dma_channel = -1; } + /* Free DMA descriptors */ + + if (priv->dmadesc) + { + kmm_free(priv->dmadesc); + priv->dmadesc = NULL; + } + /* Free frame buffer */ if (priv->fb) @@ -708,31 +757,6 @@ static int esp32s3_cam_start_capture(struct imgdata_s *data, priv->capturing = true; - syslog(LOG_INFO, "CAM start: CTRL=0x%08lx CTRL1=0x%08lx ENA=0x%08lx\n", - (unsigned long)getreg32(LCD_CAM_CAM_CTRL_REG), - (unsigned long)getreg32(LCD_CAM_CAM_CTRL1_REG), - (unsigned long)getreg32(LCD_CAM_LC_DMA_INT_ENA_REG)); - - /* Debug: poll RAW register to see if VSYNC appears */ - - for (int i = 0; i < 50; i++) - { - up_mdelay(20); - uint32_t raw = getreg32(LCD_CAM_LC_DMA_INT_RAW_REG); - uint32_t st = getreg32(LCD_CAM_LC_DMA_INT_ST_REG); - if (raw || st) - { - syslog(LOG_INFO, "CAM poll[%d]: raw=0x%08lx st=0x%08lx\n", - i, (unsigned long)raw, (unsigned long)st); - break; - } - - if (i == 49) - { - syslog(LOG_INFO, "CAM poll: no VSYNC after 1s\n"); - } - } - return OK; }
