From 4cf34aeff851dea8b211f8789e222356e4374321 Mon Sep 17 00:00:00 2001 From: Kurt Eckhardt Date: Sat, 9 Aug 2025 14:41:40 -0700 Subject: [PATCH 1/3] video: stm32_dcmi: support for CROP/Snapshot Added support to video_stm32_dcmi for the new set and get selection. These implementations simply forward the message to the underlying camera object if they support these messages. Also added support for a snapshot mode instead of always using continuous capture mode. Tried to make it semi-transparent when you desire it to be The stm32_dcmi code now also allows you to work with only one buffer. This will force it into snapshot mode. likewise if you call the video_set_stream and have not added any buffers yet, it will also set it into this mode. You can also specify to use snapshot mode, in the device tree, like: ``` &dcmi { snapshot-mode; }; ``` This commit also has some recovery from DMA errors in snapshot mode, in addition it appears to recover in many of the cases in continuous mode as well. At least it is a start to resolve some of the hangs. Updated: video_stm32_dcmi_get_caps. Signed-off-by: Kurt Eckhardt --- drivers/video/video_stm32_dcmi.c | 160 ++++++++++++++++++++++---- dts/bindings/video/st,stm32-dcmi.yaml | 4 + 2 files changed, 143 insertions(+), 21 deletions(-) diff --git a/drivers/video/video_stm32_dcmi.c b/drivers/video/video_stm32_dcmi.c index 4a763d779be34..2c9b9ac6e8d58 100644 --- a/drivers/video/video_stm32_dcmi.c +++ b/drivers/video/video_stm32_dcmi.c @@ -24,8 +24,8 @@ LOG_MODULE_REGISTER(video_stm32_dcmi, CONFIG_VIDEO_LOG_LEVEL); -#if CONFIG_VIDEO_BUFFER_POOL_NUM_MAX < 2 -#error "The minimum required number of buffers for video_stm32 is 2" +#if CONFIG_VIDEO_BUFFER_POOL_NUM_MAX < 1 +#error "The minimum required number of buffers for video_stm32 is 1" #endif typedef void (*irq_config_func_t)(const struct device *dev); @@ -45,6 +45,8 @@ struct video_stm32_dcmi_data { struct k_fifo fifo_in; struct k_fifo fifo_out; struct video_buffer *vbuf; + bool snapshot_mode : 1; + bool restart_dma : 1; }; struct video_stm32_dcmi_config { @@ -53,11 +55,22 @@ struct video_stm32_dcmi_config { const struct pinctrl_dev_config *pctrl; const struct device *sensor_dev; const struct stream dma; + bool snapshot_mode; }; +static void stm32_dcmi_process_dma_error(DCMI_HandleTypeDef *hdcmi) +{ + struct video_stm32_dcmi_data *dev_data = + CONTAINER_OF(hdcmi, struct video_stm32_dcmi_data, hdcmi); + + dev_data->restart_dma = true; + k_fifo_cancel_wait(&dev_data->fifo_out); +} + void HAL_DCMI_ErrorCallback(DCMI_HandleTypeDef *hdcmi) { - LOG_WRN("%s", __func__); + LOG_WRN("%s %p", __func__, hdcmi); + stm32_dcmi_process_dma_error(hdcmi); } void HAL_DCMI_FrameEventCallback(DCMI_HandleTypeDef *hdcmi) @@ -66,8 +79,17 @@ void HAL_DCMI_FrameEventCallback(DCMI_HandleTypeDef *hdcmi) CONTAINER_OF(hdcmi, struct video_stm32_dcmi_data, hdcmi); struct video_buffer *vbuf; - HAL_DCMI_Suspend(hdcmi); + if (dev_data->snapshot_mode) { + /* we remove the buffer from the camera and add it to fifo_out */ + vbuf = dev_data->vbuf; + dev_data->vbuf = NULL; + vbuf->timestamp = k_uptime_get_32(); + k_fifo_put(&dev_data->fifo_out, vbuf); + return; + } + /* Not in snapshot_mode */ + HAL_DCMI_Suspend(hdcmi); vbuf = k_fifo_get(&dev_data->fifo_in, K_NO_WAIT); if (vbuf == NULL) { @@ -94,21 +116,18 @@ static void stm32_dcmi_isr(const struct device *dev) static void dcmi_dma_callback(const struct device *dev, void *arg, uint32_t channel, int status) { DMA_HandleTypeDef *hdma = arg; + DCMI_HandleTypeDef *hdcmi = (DCMI_HandleTypeDef *)hdma->Parent; ARG_UNUSED(dev); if (status < 0) { LOG_ERR("DMA callback error with channel %d.", channel); + stm32_dcmi_process_dma_error(hdcmi); } HAL_DMA_IRQHandler(hdma); } -void HAL_DMA_ErrorCallback(DMA_HandleTypeDef *hdma) -{ - LOG_WRN("%s", __func__); -} - static int stm32_dma_init(const struct device *dev) { struct video_stm32_dcmi_data *data = dev->data; @@ -251,22 +270,29 @@ static int video_stm32_dcmi_set_stream(const struct device *dev, bool enable, return 0; } - data->vbuf = k_fifo_get(&data->fifo_in, K_NO_WAIT); + if (!config->snapshot_mode && (CONFIG_VIDEO_BUFFER_POOL_NUM_MAX != 1)) { + data->vbuf = k_fifo_get(&data->fifo_in, K_NO_WAIT); - if (data->vbuf == NULL) { - LOG_ERR("Failed to dequeue a DCMI buffer."); - return -ENOMEM; + if (data->vbuf == NULL) { + data->snapshot_mode = true; + LOG_WRN("No buffers, assume snapshot mode."); + } } /* Set the frame control */ data->hdcmi.Instance->CR &= ~(DCMI_CR_FCRC_0 | DCMI_CR_FCRC_1); data->hdcmi.Instance->CR |= STM32_DCMI_GET_CAPTURE_RATE(data->capture_rate); - err = HAL_DCMI_Start_DMA(&data->hdcmi, DCMI_MODE_CONTINUOUS, - (uint32_t)data->vbuf->buffer, data->vbuf->bytesused / 4); - if (err != HAL_OK) { - LOG_ERR("Failed to start DCMI DMA"); - return -EIO; + /* don't start the DCMI DMA if we are in Snapshot mode */ + if (!data->snapshot_mode) { + err = HAL_DCMI_Start_DMA(&data->hdcmi, DCMI_MODE_CONTINUOUS, + (uint32_t)data->vbuf->buffer, data->vbuf->bytesused / 4); + if (err != HAL_OK) { + LOG_ERR("Failed to start DCMI DMA"); + return -EIO; + } + } else { + LOG_DBG("Snapshot mode active"); } return video_stream_start(config->sensor_dev, type); @@ -289,31 +315,99 @@ static int video_stm32_dcmi_enqueue(const struct device *dev, struct video_buffe return 0; } +static int video_stm32_restart_dma_after_error(struct video_stm32_dcmi_data *data) { + LOG_DBG("Restart DMA after Error!"); + /* Lets try to recover by stopping and maybe restart */ + if (HAL_DCMI_Stop(&data->hdcmi) != HAL_OK) { + LOG_WRN("HAL_DCMI_Stop FAILED!"); + } + + if (data->snapshot_mode) { + if (data->vbuf != NULL) { + k_fifo_put(&data->fifo_in, data->vbuf); + data->vbuf = NULL; + } + } else { + /* error on last transfer try to restart it */ + if (HAL_DCMI_Start_DMA(&data->hdcmi, DCMI_MODE_CONTINUOUS, + (uint32_t)data->vbuf->buffer, + data->vbuf->size / 4) != HAL_OK) { + LOG_WRN("Snapshot: HAL_DCMI_Start_DMA FAILED!"); + return -EIO; + } + } + data->restart_dma = false; + return 0; +} + static int video_stm32_dcmi_dequeue(const struct device *dev, struct video_buffer **vbuf, k_timeout_t timeout) { struct video_stm32_dcmi_data *data = dev->data; + int err; + + if (data->snapshot_mode) { + /* See if we were already called and have an active buffer */ + if (data->vbuf == NULL) { + data->vbuf = k_fifo_get(&data->fifo_in, K_NO_WAIT); + if (data->vbuf == NULL) { + LOG_WRN("Snapshot: No Buffers available!"); + return -ENOMEM; + } + + if (HAL_DCMI_Start_DMA(&data->hdcmi, DCMI_MODE_SNAPSHOT, + (uint32_t)data->vbuf->buffer, + data->vbuf->size / 4) != HAL_OK) { + LOG_WRN("Snapshot: HAL_DCMI_Start_DMA FAILED!"); + return -EIO; + } + } else { + LOG_DBG("Snapshot: restart after timeout"); + } + } else { + err = video_stm32_restart_dma_after_error(data); + if (err < 0) { + return err; + } + } *vbuf = k_fifo_get(&data->fifo_out, timeout); + + if (data->restart_dma) { + err = video_stm32_restart_dma_after_error(data); + if (err < 0) { + return err; + } + } + if (*vbuf == NULL) { return -EAGAIN; } + if (data->snapshot_mode) { + if (HAL_DCMI_Stop(&data->hdcmi) != HAL_OK) { + LOG_WRN("Snapshot: HAL_DCMI_Stop FAILED!"); + } + } return 0; } static int video_stm32_dcmi_get_caps(const struct device *dev, struct video_caps *caps) { const struct video_stm32_dcmi_config *config = dev->config; + int ret; + + ret = video_get_caps(config->sensor_dev, caps); /* 2 buffers are needed for DCMI_MODE_CONTINUOUS */ caps->min_vbuf_count = 2; /* DCMI produces full frames */ - caps->min_line_count = caps->max_line_count = LINE_COUNT_HEIGHT; + caps->min_vbuf_count = config->snapshot_mode ? 1 : 2; + caps->min_line_count = LINE_COUNT_HEIGHT; + caps->max_line_count = LINE_COUNT_HEIGHT; - /* Forward the message to the sensor device */ - return video_get_caps(config->sensor_dev, caps); + return ret; } static int video_stm32_dcmi_enum_frmival(const struct device *dev, struct video_frmival_enum *fie) @@ -412,6 +506,20 @@ static int video_stm32_dcmi_get_frmival(const struct device *dev, struct video_f return 0; } +static int video_stm32_dcmi_set_selection(const struct device *dev, struct video_selection *sel) +{ + const struct video_stm32_dcmi_config *config = dev->config; + + return video_set_selection(config->sensor_dev, sel); +} + +static int video_stm32_dcmi_get_selection(const struct device *dev, struct video_selection *sel) +{ + const struct video_stm32_dcmi_config *config = dev->config; + + return video_get_selection(config->sensor_dev, sel); +} + static DEVICE_API(video, video_stm32_dcmi_driver_api) = { .set_format = video_stm32_dcmi_set_fmt, .get_format = video_stm32_dcmi_get_fmt, @@ -422,6 +530,8 @@ static DEVICE_API(video, video_stm32_dcmi_driver_api) = { .enum_frmival = video_stm32_dcmi_enum_frmival, .set_frmival = video_stm32_dcmi_set_frmival, .get_frmival = video_stm32_dcmi_get_frmival, + .set_selection = video_stm32_dcmi_set_selection, + .get_selection = video_stm32_dcmi_get_selection, }; static void video_stm32_dcmi_irq_config_func(const struct device *dev) @@ -504,6 +614,7 @@ static const struct video_stm32_dcmi_config video_stm32_dcmi_config_0 = { .irq_config = video_stm32_dcmi_irq_config_func, .pctrl = PINCTRL_DT_INST_DEV_CONFIG_GET(0), .sensor_dev = SOURCE_DEV(0), + .snapshot_mode = DT_INST_PROP(0, snapshot_mode), DCMI_DMA_CHANNEL(0, PERIPHERAL, MEMORY) }; @@ -534,6 +645,13 @@ static int video_stm32_dcmi_init(const struct device *dev) return err; } + /* See if we should initialize to only support snapshot mode or not */ + data->restart_dma = false; + data->snapshot_mode = config->snapshot_mode; + if (CONFIG_VIDEO_BUFFER_POOL_NUM_MAX == 1) { + LOG_DBG("Only one buffer so snapshot mode only"); + data->snapshot_mode = true; + } data->dev = dev; k_fifo_init(&data->fifo_in); k_fifo_init(&data->fifo_out); diff --git a/dts/bindings/video/st,stm32-dcmi.yaml b/dts/bindings/video/st,stm32-dcmi.yaml index 41da6c7659f9a..b402c9098fba7 100644 --- a/dts/bindings/video/st,stm32-dcmi.yaml +++ b/dts/bindings/video/st,stm32-dcmi.yaml @@ -46,6 +46,10 @@ properties: STM32_DMA_MEM_INC | STM32_DMA_PERIPH_8BITS | STM32_DMA_MEM_32BITS | STM32_DMA_PRIORITY_HIGH) STM32_DMA_FIFO_1_4>; + snapshot-mode: + type: boolean + description: set the driver into snapshot mode, default is off. + child-binding: child-binding: include: video-interfaces.yaml From dd4de94635b901d61c307a110c67b42366052be9 Mon Sep 17 00:00:00 2001 From: Kurt Eckhardt Date: Sat, 9 Aug 2025 14:47:43 -0700 Subject: [PATCH 2/3] video: gc2145: support for CROP Implements the set_selection and get_selection APIs Which are forwarded to it from video_stm32_dcmi as part of the Pull request. It uses the new messages to allow you to set a crop window on top of the current format window. It also then allows you to move this crop window around in the frame window. With this driver I also updated it to allow any resolution from the displays min to max limits. static const struct video_format_cap fmts[] = { GC2145_VIDEO_FORMAT_CAP_HL(128, 1600, 128, 1200, VIDEO_PIX_FMT_RGB565), GC2145_VIDEO_FORMAT_CAP_HL(128, 1600, 128, 1200, VIDEO_PIX_FMT_YUYV), When the resolution is set, it computes the scale factor. Using the set_selection(VIDEO_SEL_TGT_CROP) allows you define a crop window within the format window. It clamps the ratio to a max of 3 as some other drivers limit it saying it helps with frame rates. Signed-off-by: Kurt Eckhardt --- drivers/video/gc2145.c | 232 +++++++++++++++++++++++-------- drivers/video/video_stm32_dcmi.c | 3 +- 2 files changed, 176 insertions(+), 59 deletions(-) diff --git a/drivers/video/gc2145.c b/drivers/video/gc2145.c index 28bdcdc1f1ce9..48042b3bfccdb 100644 --- a/drivers/video/gc2145.c +++ b/drivers/video/gc2145.c @@ -768,14 +768,23 @@ struct gc2145_ctrls { struct gc2145_data { struct gc2145_ctrls ctrls; struct video_format fmt; + struct video_rect crop; + uint16_t format_width; + uint16_t format_height; + uint8_t ratio; }; -#define GC2145_VIDEO_FORMAT_CAP(width, height, format) \ - { \ - .pixelformat = format, .width_min = width, .width_max = width, \ - .height_min = height, .height_max = height, .width_step = 0, .height_step = 0, \ +#define GC2145_VIDEO_FORMAT_CAP_HL(width_l, width_h, height_l, height_h, format, step_w, step_h) \ + { \ + .pixelformat = (format), \ + .width_min = (width_l), .width_max = (width_h), \ + .height_min = (height_l), .height_max = (height_h), \ + .width_step = (step_w), .height_step = (step_h), \ } +#define GC2145_VIDEO_FORMAT_CAP(width, height, format) \ + GC2145_VIDEO_FORMAT_CAP_HL((width), (width), (height), (height), (format), 0, 0) + #define RESOLUTION_QVGA_W 320 #define RESOLUTION_QVGA_H 240 @@ -785,6 +794,13 @@ struct gc2145_data { #define RESOLUTION_UXGA_W 1600 #define RESOLUTION_UXGA_H 1200 +#define RESOLUTION_MAX_W RESOLUTION_UXGA_W +#define RESOLUTION_MAX_H RESOLUTION_UXGA_H + +/* Min not defined - smallest seen implementation is for QQVGA */ +#define RESOLUTION_MIN_W 160 +#define RESOLUTION_MIN_H 120 + static const struct video_format_cap fmts[] = { GC2145_VIDEO_FORMAT_CAP(RESOLUTION_QVGA_W, RESOLUTION_QVGA_H, VIDEO_PIX_FMT_RGB565), GC2145_VIDEO_FORMAT_CAP(RESOLUTION_VGA_W, RESOLUTION_VGA_H, VIDEO_PIX_FMT_RGB565), @@ -792,6 +808,11 @@ static const struct video_format_cap fmts[] = { GC2145_VIDEO_FORMAT_CAP(RESOLUTION_QVGA_W, RESOLUTION_QVGA_H, VIDEO_PIX_FMT_YUYV), GC2145_VIDEO_FORMAT_CAP(RESOLUTION_VGA_W, RESOLUTION_VGA_H, VIDEO_PIX_FMT_YUYV), GC2145_VIDEO_FORMAT_CAP(RESOLUTION_UXGA_W, RESOLUTION_UXGA_H, VIDEO_PIX_FMT_YUYV), + /* Add catchall resolution */ + GC2145_VIDEO_FORMAT_CAP_HL(RESOLUTION_MIN_W, RESOLUTION_MAX_W, RESOLUTION_MIN_H, + RESOLUTION_MAX_H, VIDEO_PIX_FMT_RGB565, 1, 1), + GC2145_VIDEO_FORMAT_CAP_HL(RESOLUTION_MIN_W, RESOLUTION_MAX_W, RESOLUTION_MIN_H, + RESOLUTION_MAX_H, VIDEO_PIX_FMT_YUYV, 1, 1), {0}, }; @@ -843,44 +864,74 @@ static int gc2145_set_output_format(const struct device *dev, int output_format) return 0; } +static int gc2145_set_crop_registers(const struct gc2145_config *cfg, uint16_t x, uint16_t y, + uint16_t w, uint16_t h) +{ + int ret; + + ret = video_write_cci_reg(&cfg->i2c, GC2145_REG_OUT_WIN_ROW_START, y); + if (ret < 0) { + return ret; + } + ret = video_write_cci_reg(&cfg->i2c, GC2145_REG_OUT_WIN_COL_START, x); + if (ret < 0) { + return ret; + } + ret = video_write_cci_reg(&cfg->i2c, GC2145_REG_OUT_WIN_HEIGHT, h); + if (ret < 0) { + return ret; + } + ret = video_write_cci_reg(&cfg->i2c, GC2145_REG_OUT_WIN_WIDTH, w); + if (ret < 0) { + return ret; + } + + /* Enable crop */ + return video_write_cci_reg(&cfg->i2c, GC2145_REG_CROP_ENABLE, GC2145_CROP_SET_ENABLE); +} + static int gc2145_set_resolution(const struct device *dev, uint32_t w, uint32_t h) { const struct gc2145_config *cfg = dev->config; + struct gc2145_data *drv_data = dev->data; int ret; uint16_t win_w; uint16_t win_h; - uint16_t c_ratio; - uint16_t r_ratio; - uint16_t x; - uint16_t y; uint16_t win_x; uint16_t win_y; - /* Add the subsampling factor depending on resolution */ - switch (w) { - case RESOLUTION_QVGA_W: - c_ratio = 3; - r_ratio = 3; - break; - case RESOLUTION_VGA_W: - c_ratio = 2; - r_ratio = 2; - break; - case RESOLUTION_UXGA_W: - c_ratio = 1; - r_ratio = 1; - break; - default: - LOG_ERR("Unsupported resolution %d %d", w, h); + if ((w == 0) || (h == 0)) { return -EIO; - }; + } + + /* If we are called from set_format, then we compute ratio and initialize crop */ + drv_data->ratio = MIN(RESOLUTION_UXGA_W / w, RESOLUTION_UXGA_H / h); + + /* make sure we don't end up with ratio of 0 */ + if (drv_data->ratio == 0) { + return -EIO; + } + + /* Restrict ratio to 3 for faster refresh ? */ + if (drv_data->ratio > 3) { + drv_data->ratio = 3; + } + + /* remember the width and height passed in */ + drv_data->format_width = w; + drv_data->format_height = h; + + /* Default to crop rectangle being same size as passed in resolution */ + drv_data->crop.left = 0; + drv_data->crop.top = 0; + drv_data->crop.width = w; + drv_data->crop.height = h; /* Calculates the window boundaries to obtain the desired resolution */ - win_w = w * c_ratio; - win_h = h * r_ratio; - x = (((win_w / c_ratio) - w) / 2); - y = (((win_h / r_ratio) - h) / 2); + + win_w = w * drv_data->ratio; + win_h = h * drv_data->ratio; win_x = ((UXGA_HSIZE - win_w) / 2); win_y = ((UXGA_VSIZE - win_h) / 2); @@ -909,31 +960,14 @@ static int gc2145_set_resolution(const struct device *dev, uint32_t w, uint32_t } /* Set cropping window next. */ - ret = video_write_cci_reg(&cfg->i2c, GC2145_REG_OUT_WIN_ROW_START, y); - if (ret < 0) { - return ret; - } - ret = video_write_cci_reg(&cfg->i2c, GC2145_REG_OUT_WIN_COL_START, x); - if (ret < 0) { - return ret; - } - ret = video_write_cci_reg(&cfg->i2c, GC2145_REG_OUT_WIN_HEIGHT, h); - if (ret < 0) { - return ret; - } - ret = video_write_cci_reg(&cfg->i2c, GC2145_REG_OUT_WIN_WIDTH, w); - if (ret < 0) { - return ret; - } - - /* Enable crop */ - ret = video_write_cci_reg(&cfg->i2c, GC2145_REG_CROP_ENABLE, GC2145_CROP_SET_ENABLE); + ret = gc2145_set_crop_registers(cfg, 0, 0, w, h); if (ret < 0) { return ret; } /* Set Sub-sampling ratio and mode */ - ret = video_write_cci_reg(&cfg->i2c, GC2145_REG_SUBSAMPLE, ((r_ratio << 4) | c_ratio)); + ret = video_write_cci_reg(&cfg->i2c, GC2145_REG_SUBSAMPLE, + (drv_data->ratio << 4) | drv_data->ratio); if (ret < 0) { return ret; } @@ -954,6 +988,52 @@ static int gc2145_set_resolution(const struct device *dev, uint32_t w, uint32_t return 0; } +static int gc2145_set_crop(const struct device *dev, struct video_selection *sel) +{ + /* set the crop, start off with most of a duplicate of set resolution */ + int ret; + const struct gc2145_config *cfg = dev->config; + struct gc2145_data *drv_data = dev->data; + + /* Verify the passed in rectangle is valid */ + if (((sel->rect.left + sel->rect.width) > drv_data->format_width) || + ((sel->rect.top + sel->rect.height) > drv_data->format_height)) { + LOG_INF("(%u %u) %ux%u > %ux%u", sel->rect.left, sel->rect.top, + sel->rect.width, sel->rect.height, + drv_data->format_width, drv_data->format_height); + return -EINVAL; + } + + /* if rectangle passed in is same as current, simply return */ + if ((drv_data->crop.left == sel->rect.left) && (drv_data->crop.top == sel->rect.top) && + (drv_data->crop.width == sel->rect.width) && + (drv_data->crop.height == sel->rect.height)) { + return 0; + } + + /* save out the updated crop window registers */ + ret = video_write_cci_reg(&cfg->i2c, GC2145_REG8(GC2145_REG_RESET), + GC2145_REG_RESET_P0_REGS); + if (ret < 0) { + return ret; + } + + ret = gc2145_set_crop_registers(cfg, sel->rect.left, sel->rect.top, + sel->rect.width, sel->rect.height); + if (ret < 0) { + return ret; + } + + /* Only if valid do we update our crop rectangle */ + drv_data->crop = sel->rect; + + /* enqueue/dequeue depend on this being set as well as the crop */ + drv_data->fmt.width = drv_data->crop.width; + drv_data->fmt.height = drv_data->crop.height; + + return 0; +} + static int gc2145_check_connection(const struct device *dev) { const struct gc2145_config *cfg = dev->config; @@ -1047,7 +1127,7 @@ static int gc2145_set_fmt(const struct device *dev, struct video_format *fmt) { struct gc2145_data *drv_data = dev->data; const struct gc2145_config *cfg = dev->config; - size_t res = ARRAY_SIZE(fmts); + size_t idx; int ret; if (memcmp(&drv_data->fmt, fmt, sizeof(drv_data->fmt)) == 0) { @@ -1056,14 +1136,7 @@ static int gc2145_set_fmt(const struct device *dev, struct video_format *fmt) } /* Check if camera is capable of handling given format */ - for (int i = 0; i < ARRAY_SIZE(fmts) - 1; i++) { - if (fmts[i].width_min == fmt->width && fmts[i].height_min == fmt->height && - fmts[i].pixelformat == fmt->pixelformat) { - res = i; - break; - } - } - if (res == ARRAY_SIZE(fmts)) { + if (video_format_caps_index(fmts, fmt, &idx) < 0) { LOG_ERR("Image format not supported"); return -ENOTSUP; } @@ -1171,12 +1244,55 @@ static int gc2145_set_ctrl(const struct device *dev, uint32_t id) } } +static int gc2145_set_selection(const struct device *dev, struct video_selection *sel) +{ + LOG_DBG("called: (%u %u)", sel->type, sel->target); + if (sel->type != VIDEO_BUF_TYPE_OUTPUT) { + return -EINVAL; + } + + if (sel->target != VIDEO_SEL_TGT_CROP) { + return -EINVAL; + } + + return gc2145_set_crop(dev, sel); +} + +static int gc2145_get_selection(const struct device *dev, struct video_selection *sel) +{ + LOG_DBG("called: (%u %u)", sel->type, sel->target); + if (sel->type != VIDEO_BUF_TYPE_OUTPUT) { + return -EINVAL; + } + + struct gc2145_data *drv_data = dev->data; + + switch (sel->target) { + case VIDEO_SEL_TGT_CROP: + sel->rect = drv_data->crop; + break; + + case VIDEO_SEL_TGT_NATIVE_SIZE: + sel->rect.top = 0; + sel->rect.left = 0; + sel->rect.width = drv_data->format_width; + sel->rect.height = drv_data->format_height; + break; + default: + return -EINVAL; + } + + return 0; +} + static DEVICE_API(video, gc2145_driver_api) = { .set_format = gc2145_set_fmt, .get_format = gc2145_get_fmt, .get_caps = gc2145_get_caps, .set_stream = gc2145_set_stream, .set_ctrl = gc2145_set_ctrl, + .set_selection = gc2145_set_selection, + .get_selection = gc2145_get_selection, }; static int gc2145_init_controls(const struct device *dev) diff --git a/drivers/video/video_stm32_dcmi.c b/drivers/video/video_stm32_dcmi.c index 2c9b9ac6e8d58..d0dec5656139f 100644 --- a/drivers/video/video_stm32_dcmi.c +++ b/drivers/video/video_stm32_dcmi.c @@ -315,7 +315,8 @@ static int video_stm32_dcmi_enqueue(const struct device *dev, struct video_buffe return 0; } -static int video_stm32_restart_dma_after_error(struct video_stm32_dcmi_data *data) { +static int video_stm32_restart_dma_after_error(struct video_stm32_dcmi_data *data) +{ LOG_DBG("Restart DMA after Error!"); /* Lets try to recover by stopping and maybe restart */ if (HAL_DCMI_Stop(&data->hdcmi) != HAL_OK) { From a27e1a259d1338367f56150753e944926e15d2bc Mon Sep 17 00:00:00 2001 From: Kurt Eckhardt Date: Mon, 15 Sep 2025 13:57:49 -0700 Subject: [PATCH 3/3] video: stm32_dcmi: snapshot recover cleanup Error recovery code done in callback for both modes. Timeout fixes/changes in snapshot. Two different timeouts. First is a timeout for the call to dequeue. Which can return before a snapshot completes. Additional calls to dequeue will continue to wait if necessary, for a frame to be received. A second timeout was added, which can be configured as parameter to DCMI. This handles cases where the snapshot start completes successfully, but does not return a frame in the prescribed time frame. It currently defaults to one second. Signed-off-by: Kurt Eckhardt --- drivers/video/video_stm32_dcmi.c | 142 +++++++++++++++----------- dts/bindings/video/st,stm32-dcmi.yaml | 7 +- 2 files changed, 87 insertions(+), 62 deletions(-) diff --git a/drivers/video/video_stm32_dcmi.c b/drivers/video/video_stm32_dcmi.c index d0dec5656139f..583163c9cbdf4 100644 --- a/drivers/video/video_stm32_dcmi.c +++ b/drivers/video/video_stm32_dcmi.c @@ -45,8 +45,8 @@ struct video_stm32_dcmi_data { struct k_fifo fifo_in; struct k_fifo fifo_out; struct video_buffer *vbuf; - bool snapshot_mode : 1; - bool restart_dma : 1; + uint32_t snapshot_start_time; + bool snapshot_mode; }; struct video_stm32_dcmi_config { @@ -56,6 +56,7 @@ struct video_stm32_dcmi_config { const struct device *sensor_dev; const struct stream dma; bool snapshot_mode; + int snapshot_timeout; }; static void stm32_dcmi_process_dma_error(DCMI_HandleTypeDef *hdcmi) @@ -63,8 +64,21 @@ static void stm32_dcmi_process_dma_error(DCMI_HandleTypeDef *hdcmi) struct video_stm32_dcmi_data *dev_data = CONTAINER_OF(hdcmi, struct video_stm32_dcmi_data, hdcmi); - dev_data->restart_dma = true; - k_fifo_cancel_wait(&dev_data->fifo_out); + LOG_WRN("Restart DMA after Error!"); + + /* Lets try to recover by stopping and restart */ + if (HAL_DCMI_Stop(&dev_data->hdcmi) != HAL_OK) { + LOG_WRN("HAL_DCMI_Stop FAILED!"); + return; + } + + if (HAL_DCMI_Start_DMA(&dev_data->hdcmi, + dev_data->snapshot_mode ? DCMI_MODE_SNAPSHOT : DCMI_MODE_CONTINUOUS, + (uint32_t)dev_data->vbuf->buffer, + dev_data->vbuf->size / 4) != HAL_OK) { + LOG_WRN("Continuous: HAL_DCMI_Start_DMA FAILED!"); + return; + } } void HAL_DCMI_ErrorCallback(DCMI_HandleTypeDef *hdcmi) @@ -85,6 +99,7 @@ void HAL_DCMI_FrameEventCallback(DCMI_HandleTypeDef *hdcmi) dev_data->vbuf = NULL; vbuf->timestamp = k_uptime_get_32(); k_fifo_put(&dev_data->fifo_out, vbuf); + LOG_DBG("event SH put: %p", vbuf); return; } @@ -270,7 +285,7 @@ static int video_stm32_dcmi_set_stream(const struct device *dev, bool enable, return 0; } - if (!config->snapshot_mode && (CONFIG_VIDEO_BUFFER_POOL_NUM_MAX != 1)) { + if (!data->snapshot_mode && (CONFIG_VIDEO_BUFFER_POOL_NUM_MAX != 1)) { data->vbuf = k_fifo_get(&data->fifo_in, K_NO_WAIT); if (data->vbuf == NULL) { @@ -302,7 +317,6 @@ static int video_stm32_dcmi_enqueue(const struct device *dev, struct video_buffe { struct video_stm32_dcmi_data *data = dev->data; const uint32_t buffer_size = data->fmt.pitch * data->fmt.height; - if (buffer_size > vbuf->size) { return -EINVAL; } @@ -315,82 +329,88 @@ static int video_stm32_dcmi_enqueue(const struct device *dev, struct video_buffe return 0; } -static int video_stm32_restart_dma_after_error(struct video_stm32_dcmi_data *data) +static int video_stm32_dcmi_snapshot(const struct device *dev, struct video_buffer **vbuf, + k_timeout_t timeout) { - LOG_DBG("Restart DMA after Error!"); - /* Lets try to recover by stopping and maybe restart */ - if (HAL_DCMI_Stop(&data->hdcmi) != HAL_OK) { - LOG_WRN("HAL_DCMI_Stop FAILED!"); - } + struct video_stm32_dcmi_data *data = dev->data; + const struct video_stm32_dcmi_config *config = dev->config; - if (data->snapshot_mode) { - if (data->vbuf != NULL) { - k_fifo_put(&data->fifo_in, data->vbuf); - data->vbuf = NULL; + LOG_DBG("dequeue snapshot: %p %llu\n", data->vbuf, timeout.ticks); + + /* See if we were already called and have an active buffer */ + if (data->vbuf == NULL) { + /* check first to see if we already have a buffer returned */ + *vbuf = k_fifo_get(&data->fifo_out, K_NO_WAIT); + if (*vbuf != NULL) { + LOG_DBG("k_fifo_get returned: %p\n", *vbuf); + if (HAL_DCMI_Stop(&data->hdcmi) != HAL_OK) { + LOG_WRN("Snapshot: HAL_DCMI_Stop FAILED!"); + } + return 0; } - } else { - /* error on last transfer try to restart it */ - if (HAL_DCMI_Start_DMA(&data->hdcmi, DCMI_MODE_CONTINUOUS, + data->vbuf = k_fifo_get(&data->fifo_in, K_NO_WAIT); + LOG_DBG("\tcamera buf: %p\n", data->vbuf); + if (data->vbuf == NULL) { + LOG_WRN("Snapshot: No Buffers available!"); + return -ENOMEM; + } + + if (HAL_DCMI_Start_DMA(&data->hdcmi, DCMI_MODE_SNAPSHOT, (uint32_t)data->vbuf->buffer, data->vbuf->size / 4) != HAL_OK) { LOG_WRN("Snapshot: HAL_DCMI_Start_DMA FAILED!"); return -EIO; } + + /* remember when we started this request */ + data->snapshot_start_time = k_uptime_get_32(); } - data->restart_dma = false; - return 0; -} -static int video_stm32_dcmi_dequeue(const struct device *dev, struct video_buffer **vbuf, - k_timeout_t timeout) -{ - struct video_stm32_dcmi_data *data = dev->data; - int err; + *vbuf = k_fifo_get(&data->fifo_out, timeout); + LOG_DBG("k_fifo_get returned: %p\n", *vbuf); - if (data->snapshot_mode) { - /* See if we were already called and have an active buffer */ - if (data->vbuf == NULL) { - data->vbuf = k_fifo_get(&data->fifo_in, K_NO_WAIT); - if (data->vbuf == NULL) { - LOG_WRN("Snapshot: No Buffers available!"); - return -ENOMEM; + if (*vbuf == NULL) { + uint32_t time_since_start_time = + (uint32_t)(k_uptime_get_32() - data->snapshot_start_time); + if (time_since_start_time > config->snapshot_timeout) { + LOG_WRN("Snapshot: Timed out!"); + if (HAL_DCMI_Stop(&data->hdcmi) != HAL_OK) { + LOG_WRN("Snapshot: HAL_DCMI_Stop FAILED!"); + } + /* verify it did not come in during this procuessing */ + *vbuf = k_fifo_get(&data->fifo_out, K_NO_WAIT); + if (*vbuf) { + return 0; } - if (HAL_DCMI_Start_DMA(&data->hdcmi, DCMI_MODE_SNAPSHOT, - (uint32_t)data->vbuf->buffer, - data->vbuf->size / 4) != HAL_OK) { - LOG_WRN("Snapshot: HAL_DCMI_Start_DMA FAILED!"); - return -EIO; + if (data->vbuf != NULL) { + k_fifo_put(&data->fifo_in, data->vbuf); + data->vbuf = NULL; } - } else { - LOG_DBG("Snapshot: restart after timeout"); - } - } else { - err = video_stm32_restart_dma_after_error(data); - if (err < 0) { - return err; } - } - - *vbuf = k_fifo_get(&data->fifo_out, timeout); - if (data->restart_dma) { - err = video_stm32_restart_dma_after_error(data); - if (err < 0) { - return err; - } + return -EAGAIN; } - if (*vbuf == NULL) { - return -EAGAIN; + if (HAL_DCMI_Stop(&data->hdcmi) != HAL_OK) { + LOG_WRN("Snapshot: HAL_DCMI_Stop FAILED!"); } + return 0; +} + +static int video_stm32_dcmi_dequeue(const struct device *dev, struct video_buffer **vbuf, + k_timeout_t timeout) +{ + struct video_stm32_dcmi_data *data = dev->data; if (data->snapshot_mode) { - if (HAL_DCMI_Stop(&data->hdcmi) != HAL_OK) { - LOG_WRN("Snapshot: HAL_DCMI_Stop FAILED!"); - } + return video_stm32_dcmi_snapshot(dev, vbuf, timeout); } - return 0; + + *vbuf = k_fifo_get(&data->fifo_out, timeout); + LOG_DBG("k_fifo_get returned: %p\n", *vbuf); + + return (*vbuf == NULL) ? -EAGAIN : 0; } static int video_stm32_dcmi_get_caps(const struct device *dev, struct video_caps *caps) @@ -616,6 +636,7 @@ static const struct video_stm32_dcmi_config video_stm32_dcmi_config_0 = { .pctrl = PINCTRL_DT_INST_DEV_CONFIG_GET(0), .sensor_dev = SOURCE_DEV(0), .snapshot_mode = DT_INST_PROP(0, snapshot_mode), + .snapshot_timeout = DT_INST_PROP(0, snapshot_timeout), DCMI_DMA_CHANNEL(0, PERIPHERAL, MEMORY) }; @@ -647,7 +668,6 @@ static int video_stm32_dcmi_init(const struct device *dev) } /* See if we should initialize to only support snapshot mode or not */ - data->restart_dma = false; data->snapshot_mode = config->snapshot_mode; if (CONFIG_VIDEO_BUFFER_POOL_NUM_MAX == 1) { LOG_DBG("Only one buffer so snapshot mode only"); diff --git a/dts/bindings/video/st,stm32-dcmi.yaml b/dts/bindings/video/st,stm32-dcmi.yaml index b402c9098fba7..c245827dfa40c 100644 --- a/dts/bindings/video/st,stm32-dcmi.yaml +++ b/dts/bindings/video/st,stm32-dcmi.yaml @@ -48,7 +48,12 @@ properties: snapshot-mode: type: boolean - description: set the driver into snapshot mode, default is off. + description: Set DCMI to Snapshot mode, instead of the default Capture mode. + + snapshot-timeout: + type: int + description: Set capture timeout in microseconds, default is 1000. + default: 1000 child-binding: child-binding: