From efd2f0061f0dcb2c63840929802d72d99855367c Mon Sep 17 00:00:00 2001 From: Phi Bang Nguyen Date: Wed, 19 Mar 2025 15:44:02 +0100 Subject: [PATCH 01/13] drivers: video: mipi_csi2rx: Explicitly set init priority The MIPI CSI-2 Rx needs to be initialized after the camera sensor which is generally initialized with CONFIG_VIDEO_INIT_PRIORITY. This is currently true "by chance" due to the order the linker links the object files. This linker order is not easily controlled, so use an explicit priority value to ensure this requirement. Signed-off-by: Phi Bang Nguyen --- drivers/video/Kconfig.mcux_mipi_csi2rx | 7 +++++++ drivers/video/video_mcux_mipi_csi2rx.c | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/video/Kconfig.mcux_mipi_csi2rx b/drivers/video/Kconfig.mcux_mipi_csi2rx index f9bf2f19f1efe..377590cb88778 100644 --- a/drivers/video/Kconfig.mcux_mipi_csi2rx +++ b/drivers/video/Kconfig.mcux_mipi_csi2rx @@ -8,3 +8,10 @@ config VIDEO_MCUX_MIPI_CSI2RX default y depends on DT_HAS_NXP_MIPI_CSI2RX_ENABLED select VIDEO_MCUX_CSI + +config VIDEO_MCUX_MIPI_CSI2RX_INIT_PRIORITY + int "NXP MCUX CSI-2 Rx init priority" + default 61 + depends on VIDEO_MCUX_MIPI_CSI2RX + help + Initialization priority for the MIPI CSI-2 Rx device. diff --git a/drivers/video/video_mcux_mipi_csi2rx.c b/drivers/video/video_mcux_mipi_csi2rx.c index 2e7bdd32a201e..fdad99db7927a 100644 --- a/drivers/video/video_mcux_mipi_csi2rx.c +++ b/drivers/video/video_mcux_mipi_csi2rx.c @@ -327,7 +327,8 @@ static int mipi_csi2rx_init(const struct device *dev) }; \ \ DEVICE_DT_INST_DEFINE(n, &mipi_csi2rx_init, NULL, &mipi_csi2rx_data_##n, \ - &mipi_csi2rx_config_##n, POST_KERNEL, CONFIG_VIDEO_INIT_PRIORITY, \ + &mipi_csi2rx_config_##n, POST_KERNEL, \ + CONFIG_VIDEO_MCUX_MIPI_CSI2RX_INIT_PRIORITY, \ &mipi_csi2rx_driver_api); \ \ VIDEO_DEVICE_DEFINE(mipi_csi2rx_##n, DEVICE_DT_INST_GET(n), SOURCE_DEV(n)); From 3682e16fe0236b46252ae08fe3b6491d407b33f8 Mon Sep 17 00:00:00 2001 From: Phi Bang Nguyen Date: Mon, 2 Dec 2024 15:44:15 +0100 Subject: [PATCH 02/13] drivers: video: mipi_csi2rx: Fix type range related to pixel rate Fix some type range related to pixel rate which can cause overflow. Signed-off-by: Phi Bang Nguyen --- drivers/video/video_mcux_mipi_csi2rx.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/drivers/video/video_mcux_mipi_csi2rx.c b/drivers/video/video_mcux_mipi_csi2rx.c index fdad99db7927a..2f87566efba31 100644 --- a/drivers/video/video_mcux_mipi_csi2rx.c +++ b/drivers/video/video_mcux_mipi_csi2rx.c @@ -37,7 +37,7 @@ struct mipi_csi2rx_data { }; struct mipi_csi2rx_tHsSettleEscClk_config { - uint64_t pixel_rate; + uint32_t pixel_rate; uint8_t tHsSettle_EscClk; }; @@ -193,21 +193,24 @@ static int mipi_csi2rx_get_frmival(const struct device *dev, struct video_frmiva return video_get_frmival(config->sensor_dev, frmival); } -static uint64_t mipi_csi2rx_cal_frame_size(const struct video_format *fmt) +static uint32_t mipi_csi2rx_cal_frame_size(const struct video_format *fmt) { return fmt->height * fmt->width * video_bits_per_pixel(fmt->pixelformat); } -static uint64_t mipi_csi2rx_estimate_pixel_rate(const struct video_frmival *cur_fmival, +static uint32_t mipi_csi2rx_estimate_pixel_rate(const struct video_frmival *cur_fmival, const struct video_frmival *fie_frmival, const struct video_format *cur_format, const struct video_format *fie_format, - uint64_t cur_pixel_rate, uint8_t laneNum) + uint32_t cur_pixel_rate, uint8_t laneNum) { - return mipi_csi2rx_cal_frame_size(cur_format) * fie_frmival->denominator * - cur_fmival->numerator * cur_pixel_rate / - (mipi_csi2rx_cal_frame_size(fie_format) * fie_frmival->numerator * - cur_fmival->denominator); + uint64_t numerator = mipi_csi2rx_cal_frame_size(cur_format) * fie_frmival->denominator * + cur_fmival->numerator * cur_pixel_rate; + + uint64_t denominator = mipi_csi2rx_cal_frame_size(fie_format) * fie_frmival->numerator * + cur_fmival->denominator; + + return numerator / denominator; } static int mipi_csi2rx_enum_frmival(const struct device *dev, struct video_frmival_enum *fie) @@ -215,7 +218,7 @@ static int mipi_csi2rx_enum_frmival(const struct device *dev, struct video_frmiv const struct mipi_csi2rx_config *config = dev->config; struct mipi_csi2rx_data *drv_data = dev->data; int ret; - uint64_t est_pixel_rate; + uint32_t est_pixel_rate; struct video_frmival cur_frmival; struct video_format cur_fmt; struct video_control sensor_rate = {.id = VIDEO_CID_PIXEL_RATE, .val64 = -1}; From 968a2142367511aa44c948c5f8619767a7a86795 Mon Sep 17 00:00:00 2001 From: Farah Fliss Date: Tue, 20 Aug 2024 11:26:38 +0200 Subject: [PATCH 03/13] drivers: video: mt9m114: Fix coding style Fix coding style in a variable naming. Signed-off-by: Farah Fliss Signed-off-by: Phi Bang Nguyen --- drivers/video/mt9m114.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/video/mt9m114.c b/drivers/video/mt9m114.c index ec855474409d2..5f1d0d255acbf 100644 --- a/drivers/video/mt9m114.c +++ b/drivers/video/mt9m114.c @@ -174,7 +174,7 @@ static struct mt9m114_reg mt9m114_1280_720[] = { {MT9M114_CAM_STAT_AE_INITIAL_WINDOW_YEND, 2, 0x008F}, /* 143 */ {/* NULL terminated */}}; -static struct mt9m114_resolution_config resolutionConfigs[] = { +static struct mt9m114_resolution_config resolution_configs[] = { {.width = 480, .height = 272, .params = mt9m114_480_272}, {.width = 640, .height = 480, .params = mt9m114_640_480}, {.width = 1280, .height = 720, .params = mt9m114_1280_720}, @@ -432,10 +432,10 @@ static int mt9m114_set_fmt(const struct device *dev, struct video_format *fmt) } /* Set output resolution */ - for (i = 0; i < ARRAY_SIZE(resolutionConfigs); i++) { - if (fmt->width == resolutionConfigs[i].width && - fmt->height == resolutionConfigs[i].height) { - ret = mt9m114_write_all(dev, resolutionConfigs[i].params); + for (i = 0; i < ARRAY_SIZE(resolution_configs); i++) { + if (fmt->width == resolution_configs[i].width && + fmt->height == resolution_configs[i].height) { + ret = mt9m114_write_all(dev, resolution_configs[i].params); if (ret) { LOG_ERR("Unable to set resolution"); return ret; From 529252b9afd06a5ca07499b2f44cd617a7b20d58 Mon Sep 17 00:00:00 2001 From: Farah Fliss Date: Thu, 30 May 2024 15:07:54 +0200 Subject: [PATCH 04/13] drivers: video: mt9m114: Make the driver multi-instance The mt9m114 camera driver used to be single-instance. Improve it to multi-instance. Signed-off-by: Farah Fliss Signed-off-by: Phi Bang Nguyen --- drivers/video/mt9m114.c | 45 +++++++++++++++++------------------------ 1 file changed, 19 insertions(+), 26 deletions(-) diff --git a/drivers/video/mt9m114.c b/drivers/video/mt9m114.c index 5f1d0d255acbf..7f01586d729e5 100644 --- a/drivers/video/mt9m114.c +++ b/drivers/video/mt9m114.c @@ -526,10 +526,16 @@ static int mt9m114_init_controls(const struct device *dev) static int mt9m114_init(const struct device *dev) { + const struct mt9m114_config *cfg = dev->config; struct video_format fmt; uint16_t val; int ret; + if (!device_is_ready(cfg->i2c.bus)) { + LOG_ERR("Bus device is not ready"); + return -ENODEV; + } + /* no power control, wait for camera ready */ k_sleep(K_MSEC(100)); @@ -573,29 +579,16 @@ static int mt9m114_init(const struct device *dev) return mt9m114_init_controls(dev); } -#if 1 /* Unique Instance */ - -static const struct mt9m114_config mt9m114_cfg_0 = { - .i2c = I2C_DT_SPEC_INST_GET(0), -}; - -static struct mt9m114_data mt9m114_data_0; - -static int mt9m114_init_0(const struct device *dev) -{ - const struct mt9m114_config *cfg = dev->config; - - if (!device_is_ready(cfg->i2c.bus)) { - LOG_ERR("Bus device is not ready"); - return -ENODEV; - } - - return mt9m114_init(dev); -} - -DEVICE_DT_INST_DEFINE(0, &mt9m114_init_0, NULL, &mt9m114_data_0, &mt9m114_cfg_0, POST_KERNEL, - CONFIG_VIDEO_INIT_PRIORITY, &mt9m114_driver_api); - -VIDEO_DEVICE_DEFINE(mt9m114, DEVICE_DT_INST_GET(0), NULL); - -#endif +#define MT9M114_INIT(n) \ + static struct mt9m114_data mt9m114_data_##n; \ + \ + static const struct mt9m114_config mt9m114_cfg_##n = { \ + .i2c = I2C_DT_SPEC_INST_GET(n), \ + }; \ + \ + DEVICE_DT_INST_DEFINE(n, &mt9m114_init, NULL, &mt9m114_data_##n, &mt9m114_cfg_##n, \ + POST_KERNEL, CONFIG_VIDEO_INIT_PRIORITY, &mt9m114_driver_api); \ + \ + VIDEO_DEVICE_DEFINE(mt9m114_##n, DEVICE_DT_INST_GET(n), NULL); + +DT_INST_FOREACH_STATUS_OKAY(MT9M114_INIT) From 87b8f7e5259248d011af6a10d19270c2e54835fc Mon Sep 17 00:00:00 2001 From: Phi Bang Nguyen Date: Mon, 2 Dec 2024 15:09:38 +0100 Subject: [PATCH 05/13] drivers: video: ov5640: Drop cur_pixrate internal variable Update the control value directly. No need for an internal variable. Signed-off-by: Phi Bang Nguyen --- drivers/video/ov5640.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/video/ov5640.c b/drivers/video/ov5640.c index 9610a9ccaa46e..56fc216f3b02f 100644 --- a/drivers/video/ov5640.c +++ b/drivers/video/ov5640.c @@ -164,7 +164,6 @@ struct ov5640_ctrls { struct ov5640_data { struct ov5640_ctrls ctrls; struct video_format fmt; - uint64_t cur_pixrate; uint16_t cur_frmrate; const struct ov5640_mode_config *cur_mode; }; @@ -811,10 +810,9 @@ static int ov5640_set_frmival(const struct device *dev, struct video_frmival *fr } drv_data->cur_frmrate = best_match; - drv_data->cur_pixrate = drv_data->cur_mode->mipi_frmrate_config[ind].pixelrate; /* Update pixerate control */ - drv_data->ctrls.pixel_rate.val = drv_data->cur_pixrate; + drv_data->ctrls.pixel_rate.val64 = drv_data->cur_mode->mipi_frmrate_config[ind].pixelrate; frmival->numerator = 1; frmival->denominator = best_match; From 145a8f17582f29cd64ab8cba8729a58e7346b939 Mon Sep 17 00:00:00 2001 From: Trung Hieu Le Date: Tue, 24 Dec 2024 14:55:48 +0100 Subject: [PATCH 06/13] drivers: video: ov5640: Fix HUE register write SDE_CTRL8_REG's value must be modified using modify_register. Signed-off-by: Trung Hieu Le Signed-off-by: Phi Bang Nguyen --- drivers/video/ov5640.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/video/ov5640.c b/drivers/video/ov5640.c index 56fc216f3b02f..6092e607239cf 100644 --- a/drivers/video/ov5640.c +++ b/drivers/video/ov5640.c @@ -992,9 +992,13 @@ static int ov5640_set_ctrl_hue(const struct device *dev, int value) sign = 0x02; } - struct ov5640_reg hue_params[] = {{SDE_CTRL8_REG, sign}, - {SDE_CTRL1_REG, abs(cos_coef)}, - {SDE_CTRL2_REG, abs(sin_coef)}}; + struct ov5640_reg hue_params[] = {{SDE_CTRL1_REG, abs(cos_coef) & 0xFF}, + {SDE_CTRL2_REG, abs(sin_coef) & 0xFF}}; + + ret = ov5640_modify_reg(&cfg->i2c, SDE_CTRL8_REG, 0x7F, sign); + if (ret < 0) { + return ret; + } return ov5640_write_multi_regs(&cfg->i2c, hue_params, ARRAY_SIZE(hue_params)); } From c7179c760bcb0aee53a38545e6198f737320c6b1 Mon Sep 17 00:00:00 2001 From: Trung Hieu Le Date: Mon, 30 Dec 2024 16:18:46 +0100 Subject: [PATCH 07/13] drivers: video: ov5640: Fix brightness control register Fix the sign register for brightness control Signed-off-by: Trung Hieu Le Signed-off-by: Phi Bang Nguyen --- drivers/video/ov5640.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/video/ov5640.c b/drivers/video/ov5640.c index 6092e607239cf..f5e6f0946791e 100644 --- a/drivers/video/ov5640.c +++ b/drivers/video/ov5640.c @@ -1021,15 +1021,18 @@ static int ov5640_set_ctrl_brightness(const struct device *dev, int value) { const struct ov5640_config *cfg = dev->config; - struct ov5640_reg brightness_params[] = {{SDE_CTRL8_REG, value >= 0 ? 0x01 : 0x09}, - {SDE_CTRL7_REG, abs(value) & 0xff}}; int ret = ov5640_modify_reg(&cfg->i2c, SDE_CTRL0_REG, BIT(2), BIT(2)); if (ret) { return ret; } - return ov5640_write_multi_regs(&cfg->i2c, brightness_params, ARRAY_SIZE(brightness_params)); + ret = ov5640_modify_reg(&cfg->i2c, SDE_CTRL8_REG, BIT(3), value >= 0 ? 0 : BIT(3)); + if (ret < 0) { + return ret; + } + + return ov5640_write_reg(&cfg->i2c, SDE_CTRL7_REG, (abs(value) << 4) & 0xf0); } static int ov5640_set_ctrl_contrast(const struct device *dev, int value) From 9c75c86732d4babc642ac75aef9f96c2526637b1 Mon Sep 17 00:00:00 2001 From: Trung Hieu Le Date: Mon, 30 Dec 2024 16:39:58 +0100 Subject: [PATCH 08/13] drivers: video: ov5640: Fix constrast value sign Fix sign's register for constrast value. Signed-off-by: Trung Hieu Le Signed-off-by: Phi Bang Nguyen --- drivers/video/ov5640.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/video/ov5640.c b/drivers/video/ov5640.c index f5e6f0946791e..88a83a8cf196b 100644 --- a/drivers/video/ov5640.c +++ b/drivers/video/ov5640.c @@ -1045,6 +1045,11 @@ static int ov5640_set_ctrl_contrast(const struct device *dev, int value) return ret; } + ret = ov5640_modify_reg(&cfg->i2c, SDE_CTRL6_REG, BIT(2), value >= 0 ? 0 : BIT(2)); + if (ret < 0) { + return ret; + } + return ov5640_write_reg(&cfg->i2c, SDE_CTRL6_REG, value & 0xff); } From c0adc978d4dc0763eb3161aada9034314d0bc376 Mon Sep 17 00:00:00 2001 From: Phi Bang Nguyen Date: Wed, 14 May 2025 15:46:39 +0200 Subject: [PATCH 09/13] drivers: video: Compute bits per pixel according to format Compute bits per pixel according to the pixel format instead of hardcoding it. Signed-off-by: Phi Bang Nguyen --- drivers/video/gc2145.c | 2 +- drivers/video/mt9m114.c | 2 +- drivers/video/ov2640.c | 2 +- drivers/video/ov5640.c | 2 +- drivers/video/ov7670.c | 2 +- drivers/video/ov7725.c | 2 +- drivers/video/video_emul_imager.c | 2 +- samples/drivers/video/capture/src/main.c | 2 +- samples/drivers/video/capture_to_lvgl/src/main.c | 2 +- tests/drivers/video/api/src/video_emul.c | 2 +- 10 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/video/gc2145.c b/drivers/video/gc2145.c index ad724a98983be..84d718358e29a 100644 --- a/drivers/video/gc2145.c +++ b/drivers/video/gc2145.c @@ -1183,7 +1183,7 @@ static int gc2145_init(const struct device *dev) fmt.pixelformat = VIDEO_PIX_FMT_RGB565; fmt.width = RESOLUTION_QVGA_W; fmt.height = RESOLUTION_QVGA_H; - fmt.pitch = RESOLUTION_QVGA_W * 2; + fmt.pitch = RESOLUTION_QVGA_W * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; ret = gc2145_set_fmt(dev, &fmt); if (ret) { diff --git a/drivers/video/mt9m114.c b/drivers/video/mt9m114.c index 7f01586d729e5..ca7b5f91d97c6 100644 --- a/drivers/video/mt9m114.c +++ b/drivers/video/mt9m114.c @@ -564,7 +564,7 @@ static int mt9m114_init(const struct device *dev) fmt.pixelformat = VIDEO_PIX_FMT_RGB565; fmt.width = 480; fmt.height = 272; - fmt.pitch = fmt.width * 2; + fmt.pitch = fmt.width * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; ret = mt9m114_set_fmt(dev, &fmt); if (ret) { diff --git a/drivers/video/ov2640.c b/drivers/video/ov2640.c index 7a660cbb64397..9b28a81454422 100644 --- a/drivers/video/ov2640.c +++ b/drivers/video/ov2640.c @@ -1032,7 +1032,7 @@ static int ov2640_init(const struct device *dev) fmt.pixelformat = VIDEO_PIX_FMT_RGB565; fmt.width = SVGA_HSIZE; fmt.height = SVGA_VSIZE; - fmt.pitch = SVGA_HSIZE * 2; + fmt.pitch = SVGA_HSIZE * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; ret = ov2640_set_fmt(dev, &fmt); if (ret) { LOG_ERR("Unable to configure default format"); diff --git a/drivers/video/ov5640.c b/drivers/video/ov5640.c index 88a83a8cf196b..9a9a3f33dff48 100644 --- a/drivers/video/ov5640.c +++ b/drivers/video/ov5640.c @@ -1433,7 +1433,7 @@ static int ov5640_init(const struct device *dev) fmt.width = 1280; fmt.height = 720; } - fmt.pitch = fmt.width * 2; + fmt.pitch = fmt.width * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; ret = ov5640_set_fmt(dev, &fmt); if (ret) { LOG_ERR("Unable to configure default format"); diff --git a/drivers/video/ov7670.c b/drivers/video/ov7670.c index 3b6ad6ac288ff..59414d6591c85 100644 --- a/drivers/video/ov7670.c +++ b/drivers/video/ov7670.c @@ -554,7 +554,7 @@ static int ov7670_init(const struct device *dev) fmt.pixelformat = VIDEO_PIX_FMT_YUYV; fmt.width = 640; fmt.height = 480; - fmt.pitch = fmt.width * 2; + fmt.pitch = fmt.width * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; ret = ov7670_set_fmt(dev, &fmt); if (ret < 0) { return ret; diff --git a/drivers/video/ov7725.c b/drivers/video/ov7725.c index 1f9834c79100a..dc9aab8fefeb4 100644 --- a/drivers/video/ov7725.c +++ b/drivers/video/ov7725.c @@ -592,7 +592,7 @@ static int ov7725_init(const struct device *dev) fmt.pixelformat = VIDEO_PIX_FMT_RGB565; fmt.width = 640; fmt.height = 480; - fmt.pitch = 640 * 2; + fmt.pitch = fmt.width * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; ret = ov7725_set_fmt(dev, &fmt); if (ret) { LOG_ERR("Unable to configure default format"); diff --git a/drivers/video/video_emul_imager.c b/drivers/video/video_emul_imager.c index 43b4982a89f92..a8be49545bd4a 100644 --- a/drivers/video/video_emul_imager.c +++ b/drivers/video/video_emul_imager.c @@ -368,7 +368,7 @@ int emul_imager_init(const struct device *dev) fmt.pixelformat = fmts[0].pixelformat; fmt.width = fmts[0].width_min; fmt.height = fmts[0].height_min; - fmt.pitch = fmt.width * 2; + fmt.pitch = fmt.width * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; ret = emul_imager_set_fmt(dev, &fmt); if (ret < 0) { diff --git a/samples/drivers/video/capture/src/main.c b/samples/drivers/video/capture/src/main.c index f5afea5925424..761e49a1cfc70 100644 --- a/samples/drivers/video/capture/src/main.c +++ b/samples/drivers/video/capture/src/main.c @@ -150,7 +150,7 @@ int main(void) #if CONFIG_VIDEO_FRAME_WIDTH fmt.width = CONFIG_VIDEO_FRAME_WIDTH; - fmt.pitch = fmt.width * 2; + fmt.pitch = fmt.width * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; #endif if (strcmp(CONFIG_VIDEO_PIXEL_FORMAT, "")) { diff --git a/samples/drivers/video/capture_to_lvgl/src/main.c b/samples/drivers/video/capture_to_lvgl/src/main.c index 81883577ad8f9..ecac099a80ea5 100644 --- a/samples/drivers/video/capture_to_lvgl/src/main.c +++ b/samples/drivers/video/capture_to_lvgl/src/main.c @@ -79,7 +79,7 @@ int main(void) /* Set format */ fmt.width = CONFIG_VIDEO_WIDTH; fmt.height = CONFIG_VIDEO_HEIGHT; - fmt.pitch = fmt.width * 2; + fmt.pitch = fmt.width * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; fmt.pixelformat = VIDEO_PIX_FMT_RGB565; if (video_set_format(video_dev, &fmt)) { diff --git a/tests/drivers/video/api/src/video_emul.c b/tests/drivers/video/api/src/video_emul.c index 3013ea84befbe..1c4d04d1338ba 100644 --- a/tests/drivers/video/api/src/video_emul.c +++ b/tests/drivers/video/api/src/video_emul.c @@ -158,7 +158,7 @@ ZTEST(video_common, test_video_vbuf) fmt.pixelformat = caps.format_caps[0].pixelformat; fmt.width = caps.format_caps[0].width_max; fmt.height = caps.format_caps[0].height_max; - fmt.pitch = fmt.width * 2; + fmt.pitch = fmt.width * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; fmt.type = type; zexpect_ok(video_set_format(rx_dev, &fmt)); From afdaa6a9019298562b909c913fd7430f402ed009 Mon Sep 17 00:00:00 2001 From: Phi Bang Nguyen Date: Wed, 14 May 2025 19:19:04 +0200 Subject: [PATCH 10/13] drivers: video: stm32-dcmi: Drop video_stm32_dcmi_is_fmt_valid Drop video_stm32_dcmi_is_fmt_valid() as it is not needed. In this function, (i) checking against a format based on another utility function video_bits_per_pixel() is not robust, this check is done in the sensor driver, (ii) checking against the heap size is not appropriate because this should be done when allocating buffers, not in get/set format. Signed-off-by: Phi Bang Nguyen --- drivers/video/video_stm32_dcmi.c | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/drivers/video/video_stm32_dcmi.c b/drivers/video/video_stm32_dcmi.c index f27a06da16b41..3add78cdad1e8 100644 --- a/drivers/video/video_stm32_dcmi.c +++ b/drivers/video/video_stm32_dcmi.c @@ -186,28 +186,12 @@ static int stm32_dcmi_enable_clock(const struct device *dev) return clock_control_on(dcmi_clock, (clock_control_subsys_t *)&config->pclken); } -static inline int video_stm32_dcmi_is_fmt_valid(uint32_t pixelformat, uint32_t pitch, - uint32_t height) -{ - if (video_bits_per_pixel(pixelformat) / BITS_PER_BYTE == 0 || - pitch * height > CONFIG_VIDEO_BUFFER_POOL_SZ_MAX) { - return -EINVAL; - } - - return 0; -} - static int video_stm32_dcmi_set_fmt(const struct device *dev, struct video_format *fmt) { const struct video_stm32_dcmi_config *config = dev->config; struct video_stm32_dcmi_data *data = dev->data; int ret; - ret = video_stm32_dcmi_is_fmt_valid(fmt->pixelformat, fmt->pitch, fmt->height); - if (ret < 0) { - return ret; - } - ret = video_set_format(config->sensor_dev, fmt); if (ret < 0) { return ret; @@ -234,11 +218,6 @@ static int video_stm32_dcmi_get_fmt(const struct device *dev, struct video_forma return ret; } - ret = video_stm32_dcmi_is_fmt_valid(fmt->pixelformat, fmt->pitch, fmt->height); - if (ret < 0) { - return ret; - } - data->fmt = *fmt; return 0; @@ -342,12 +321,6 @@ static int video_stm32_dcmi_enum_frmival(const struct device *dev, struct video_ const struct video_stm32_dcmi_config *config = dev->config; int ret; - ret = video_stm32_dcmi_is_fmt_valid(fie->format->pixelformat, fie->format->pitch, - fie->format->height); - if (ret < 0) { - return ret; - } - ret = video_enum_frmival(config->sensor_dev, fie); if (ret < 0) { return ret; From 33dcbe37cfd3593e8c6e9cfd218dd31fdd533598 Mon Sep 17 00:00:00 2001 From: Phi Bang Nguyen Date: Wed, 14 May 2025 19:46:14 +0200 Subject: [PATCH 11/13] drivers: video: Move format pitch setting to bridge drivers The format pitch (bytesperline) field is typically set by the bridge drivers, i.e. DMA, ISP drivers who actually handle the memory as they know exactly the memory layout constraints. Application just set the pixel format and resolution and must always read back this field to see what the driver actually sets (to allocate buffers for example). Also, drop format pitch setting in sensor drivers as this is not needed. Signed-off-by: Phi Bang Nguyen --- drivers/video/gc2145.c | 1 - drivers/video/mt9m114.c | 1 - drivers/video/ov2640.c | 1 - drivers/video/ov5640.c | 1 - drivers/video/ov7670.c | 1 - drivers/video/ov7725.c | 1 - drivers/video/video_emul_imager.c | 1 - drivers/video/video_emul_rx.c | 5 +++++ drivers/video/video_esp32_dvp.c | 12 +++++++++++- drivers/video/video_mcux_csi.c | 11 ++++------- drivers/video/video_mcux_smartdma.c | 18 +++++++++++++----- drivers/video/video_stm32_dcmi.c | 4 ++++ drivers/video/video_sw_generator.c | 2 ++ samples/drivers/video/capture/src/main.c | 1 - .../drivers/video/capture_to_lvgl/src/main.c | 1 - tests/drivers/video/api/src/video_common.c | 8 -------- tests/drivers/video/api/src/video_emul.c | 1 - 17 files changed, 39 insertions(+), 31 deletions(-) diff --git a/drivers/video/gc2145.c b/drivers/video/gc2145.c index 84d718358e29a..3d7a7bda219b7 100644 --- a/drivers/video/gc2145.c +++ b/drivers/video/gc2145.c @@ -1183,7 +1183,6 @@ static int gc2145_init(const struct device *dev) fmt.pixelformat = VIDEO_PIX_FMT_RGB565; fmt.width = RESOLUTION_QVGA_W; fmt.height = RESOLUTION_QVGA_H; - fmt.pitch = RESOLUTION_QVGA_W * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; ret = gc2145_set_fmt(dev, &fmt); if (ret) { diff --git a/drivers/video/mt9m114.c b/drivers/video/mt9m114.c index ca7b5f91d97c6..f591ee567a30f 100644 --- a/drivers/video/mt9m114.c +++ b/drivers/video/mt9m114.c @@ -564,7 +564,6 @@ static int mt9m114_init(const struct device *dev) fmt.pixelformat = VIDEO_PIX_FMT_RGB565; fmt.width = 480; fmt.height = 272; - fmt.pitch = fmt.width * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; ret = mt9m114_set_fmt(dev, &fmt); if (ret) { diff --git a/drivers/video/ov2640.c b/drivers/video/ov2640.c index 9b28a81454422..79adca53b1828 100644 --- a/drivers/video/ov2640.c +++ b/drivers/video/ov2640.c @@ -1032,7 +1032,6 @@ static int ov2640_init(const struct device *dev) fmt.pixelformat = VIDEO_PIX_FMT_RGB565; fmt.width = SVGA_HSIZE; fmt.height = SVGA_VSIZE; - fmt.pitch = SVGA_HSIZE * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; ret = ov2640_set_fmt(dev, &fmt); if (ret) { LOG_ERR("Unable to configure default format"); diff --git a/drivers/video/ov5640.c b/drivers/video/ov5640.c index 9a9a3f33dff48..219ec0316b892 100644 --- a/drivers/video/ov5640.c +++ b/drivers/video/ov5640.c @@ -1433,7 +1433,6 @@ static int ov5640_init(const struct device *dev) fmt.width = 1280; fmt.height = 720; } - fmt.pitch = fmt.width * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; ret = ov5640_set_fmt(dev, &fmt); if (ret) { LOG_ERR("Unable to configure default format"); diff --git a/drivers/video/ov7670.c b/drivers/video/ov7670.c index 59414d6591c85..27d5bc218f040 100644 --- a/drivers/video/ov7670.c +++ b/drivers/video/ov7670.c @@ -554,7 +554,6 @@ static int ov7670_init(const struct device *dev) fmt.pixelformat = VIDEO_PIX_FMT_YUYV; fmt.width = 640; fmt.height = 480; - fmt.pitch = fmt.width * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; ret = ov7670_set_fmt(dev, &fmt); if (ret < 0) { return ret; diff --git a/drivers/video/ov7725.c b/drivers/video/ov7725.c index dc9aab8fefeb4..eff4b21519fc3 100644 --- a/drivers/video/ov7725.c +++ b/drivers/video/ov7725.c @@ -592,7 +592,6 @@ static int ov7725_init(const struct device *dev) fmt.pixelformat = VIDEO_PIX_FMT_RGB565; fmt.width = 640; fmt.height = 480; - fmt.pitch = fmt.width * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; ret = ov7725_set_fmt(dev, &fmt); if (ret) { LOG_ERR("Unable to configure default format"); diff --git a/drivers/video/video_emul_imager.c b/drivers/video/video_emul_imager.c index a8be49545bd4a..a59c6cf532a0f 100644 --- a/drivers/video/video_emul_imager.c +++ b/drivers/video/video_emul_imager.c @@ -368,7 +368,6 @@ int emul_imager_init(const struct device *dev) fmt.pixelformat = fmts[0].pixelformat; fmt.width = fmts[0].width_min; fmt.height = fmts[0].height_min; - fmt.pitch = fmt.width * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; ret = emul_imager_set_fmt(dev, &fmt); if (ret < 0) { diff --git a/drivers/video/video_emul_rx.c b/drivers/video/video_emul_rx.c index 2a67559cb44a9..1048486974bd0 100644 --- a/drivers/video/video_emul_rx.c +++ b/drivers/video/video_emul_rx.c @@ -67,7 +67,9 @@ static int emul_rx_set_fmt(const struct device *const dev, struct video_format * } /* Cache the format selected locally to use it for getting the size of the buffer */ + fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE; data->fmt = *fmt; + return 0; } @@ -216,6 +218,9 @@ int emul_rx_init(const struct device *dev) return ret; } + data->fmt.pitch = + data->fmt.width * video_bits_per_pixel(data->fmt.pixelformat) / BITS_PER_BYTE; + k_fifo_init(&data->fifo_in); k_fifo_init(&data->fifo_out); k_work_init(&data->work, &emul_rx_worker); diff --git a/drivers/video/video_esp32_dvp.c b/drivers/video/video_esp32_dvp.c index d0eebe40b13a2..43420ffbfb4e9 100644 --- a/drivers/video/video_esp32_dvp.c +++ b/drivers/video/video_esp32_dvp.c @@ -269,6 +269,8 @@ static int video_esp32_get_fmt(const struct device *dev, struct video_format *fm return ret; } + fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE; + return 0; } @@ -276,14 +278,22 @@ static int video_esp32_set_fmt(const struct device *dev, struct video_format *fm { const struct video_esp32_config *cfg = dev->config; struct video_esp32_data *data = dev->data; + int ret; if (fmt == NULL) { return -EINVAL; } + ret = video_set_format(cfg->source_dev, fmt); + if (ret < 0) { + return ret; + } + + fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE; + data->video_format = *fmt; - return video_set_format(cfg->source_dev, fmt); + return 0; } static int video_esp32_enqueue(const struct device *dev, struct video_buffer *vbuf) diff --git a/drivers/video/video_mcux_csi.c b/drivers/video/video_mcux_csi.c index 866f58844de57..f4226ab58e126 100644 --- a/drivers/video/video_mcux_csi.c +++ b/drivers/video/video_mcux_csi.c @@ -133,16 +133,11 @@ static int video_mcux_csi_set_fmt(const struct device *dev, struct video_format { const struct video_mcux_csi_config *config = dev->config; struct video_mcux_csi_data *data = dev->data; - unsigned int bpp = video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE; status_t ret; struct video_format format = *fmt; - if (bpp == 0) { - return -EINVAL; - } - - data->csi_config.bytesPerPixel = bpp; - data->csi_config.linePitch_Bytes = fmt->pitch; + data->csi_config.bytesPerPixel = video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE; + data->csi_config.linePitch_Bytes = fmt->width * data->csi_config.bytesPerPixel; #if defined(CONFIG_VIDEO_MCUX_MIPI_CSI2RX) if (fmt->pixelformat != VIDEO_PIX_FMT_XRGB32 && fmt->pixelformat != VIDEO_PIX_FMT_XYUV32) { return -ENOTSUP; @@ -172,6 +167,8 @@ static int video_mcux_csi_set_fmt(const struct device *dev, struct video_format return -EIO; } + fmt->pitch = data->csi_config.linePitch_Bytes; + return 0; } diff --git a/drivers/video/video_mcux_smartdma.c b/drivers/video/video_mcux_smartdma.c index 5a6e65f6ebfa4..71666a10e5fc4 100644 --- a/drivers/video/video_mcux_smartdma.c +++ b/drivers/video/video_mcux_smartdma.c @@ -218,6 +218,7 @@ static const struct video_format_cap fmts[] = { static int nxp_video_sdma_set_format(const struct device *dev, struct video_format *fmt) { const struct nxp_video_sdma_config *config = dev->config; + int ret; if (fmt == NULL) { return -EINVAL; @@ -230,14 +231,20 @@ static int nxp_video_sdma_set_format(const struct device *dev, struct video_form if ((fmt->pixelformat != fmts[0].pixelformat) || (fmt->width != fmts[0].width_min) || - (fmt->height != fmts[0].height_min) || - (fmt->pitch != fmts[0].width_min * 2)) { + (fmt->height != fmts[0].height_min)) { LOG_ERR("Unsupported format"); return -ENOTSUP; } /* Forward format to sensor device */ - return video_set_format(config->sensor_dev, fmt); + ret = video_set_format(config->sensor_dev, fmt); + if (ret < 0) { + return ret; + } + + fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE; + + return 0; } static int nxp_video_sdma_get_format(const struct device *dev, struct video_format *fmt) @@ -267,8 +274,7 @@ static int nxp_video_sdma_get_format(const struct device *dev, struct video_form /* Verify that format is RGB565 */ if ((fmt->pixelformat != fmts[0].pixelformat) || (fmt->width != fmts[0].width_min) || - (fmt->height != fmts[0].height_min) || - (fmt->pitch != fmts[0].width_min * 2)) { + (fmt->height != fmts[0].height_min)) { /* Update format of sensor */ fmt->pixelformat = fmts[0].pixelformat; fmt->width = fmts[0].width_min; @@ -281,6 +287,8 @@ static int nxp_video_sdma_get_format(const struct device *dev, struct video_form } } + fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE; + return 0; } diff --git a/drivers/video/video_stm32_dcmi.c b/drivers/video/video_stm32_dcmi.c index 3add78cdad1e8..34f7768a65424 100644 --- a/drivers/video/video_stm32_dcmi.c +++ b/drivers/video/video_stm32_dcmi.c @@ -197,6 +197,8 @@ static int video_stm32_dcmi_set_fmt(const struct device *dev, struct video_forma return ret; } + fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE; + data->fmt = *fmt; return 0; @@ -218,6 +220,8 @@ static int video_stm32_dcmi_get_fmt(const struct device *dev, struct video_forma return ret; } + fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE; + data->fmt = *fmt; return 0; diff --git a/drivers/video/video_sw_generator.c b/drivers/video/video_sw_generator.c index 40ba10011bea5..2d3ab20cb812b 100644 --- a/drivers/video/video_sw_generator.c +++ b/drivers/video/video_sw_generator.c @@ -81,6 +81,8 @@ static int video_sw_generator_set_fmt(const struct device *dev, struct video_for return -ENOTSUP; } + fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE; + data->fmt = *fmt; return 0; diff --git a/samples/drivers/video/capture/src/main.c b/samples/drivers/video/capture/src/main.c index 761e49a1cfc70..ae6e0d4cdbd1c 100644 --- a/samples/drivers/video/capture/src/main.c +++ b/samples/drivers/video/capture/src/main.c @@ -150,7 +150,6 @@ int main(void) #if CONFIG_VIDEO_FRAME_WIDTH fmt.width = CONFIG_VIDEO_FRAME_WIDTH; - fmt.pitch = fmt.width * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; #endif if (strcmp(CONFIG_VIDEO_PIXEL_FORMAT, "")) { diff --git a/samples/drivers/video/capture_to_lvgl/src/main.c b/samples/drivers/video/capture_to_lvgl/src/main.c index ecac099a80ea5..cbd23cb30cd9b 100644 --- a/samples/drivers/video/capture_to_lvgl/src/main.c +++ b/samples/drivers/video/capture_to_lvgl/src/main.c @@ -79,7 +79,6 @@ int main(void) /* Set format */ fmt.width = CONFIG_VIDEO_WIDTH; fmt.height = CONFIG_VIDEO_HEIGHT; - fmt.pitch = fmt.width * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; fmt.pixelformat = VIDEO_PIX_FMT_RGB565; if (video_set_format(video_dev, &fmt)) { diff --git a/tests/drivers/video/api/src/video_common.c b/tests/drivers/video/api/src/video_common.c index 4f01ef2d5b599..a9dc98dd5f469 100644 --- a/tests/drivers/video/api/src/video_common.c +++ b/tests/drivers/video/api/src/video_common.c @@ -36,40 +36,34 @@ ZTEST(video_common, test_video_format_caps_index) fmt.width = 100; fmt.height = 100; - fmt.pitch = 100 * 2; ret = video_format_caps_index(fmts, &fmt, &idx); zassert_ok(ret, "expecting minimum value to match"); zassert_equal(idx, YUYV_A); fmt.width = 1000; fmt.height = 1000; - fmt.pitch = 1000 * 2; ret = video_format_caps_index(fmts, &fmt, &idx); zassert_ok(ret, "expecting maximum value to match"); zassert_equal(idx, YUYV_A); fmt.width = 1920; fmt.height = 1080; - fmt.pitch = 1920 * 2; ret = video_format_caps_index(fmts, &fmt, &idx); zassert_ok(ret, "expecting exact match to work"); zassert_equal(idx, YUYV_B); fmt.width = 1001; fmt.height = 1000; - fmt.pitch = 1001 * 2; ret = video_format_caps_index(fmts, &fmt, &idx); zassert_not_ok(ret, "expecting 1 above maximum width to mismatch"); fmt.width = 1000; fmt.height = 1001; - fmt.pitch = 1000 * 2; ret = video_format_caps_index(fmts, &fmt, &idx); zassert_not_ok(ret, "expecting 1 above maximum height to mismatch"); fmt.width = 1280; fmt.height = 720; - fmt.pitch = 1280 * 2; ret = video_format_caps_index(fmts, &fmt, &idx); zassert_not_ok(ret); zassert_not_ok(ret, "expecting wrong format to mismatch"); @@ -78,13 +72,11 @@ ZTEST(video_common, test_video_format_caps_index) fmt.width = 1000; fmt.height = 1000; - fmt.pitch = 1000 * 2; ret = video_format_caps_index(fmts, &fmt, &idx); zassert_not_ok(ret, "expecting wrong format to mismatch"); fmt.width = 1280; fmt.height = 720; - fmt.pitch = 1280 * 2; ret = video_format_caps_index(fmts, &fmt, &idx); zassert_ok(ret, "expecting exact match to work"); zassert_equal(idx, RGB565); diff --git a/tests/drivers/video/api/src/video_emul.c b/tests/drivers/video/api/src/video_emul.c index 1c4d04d1338ba..3e8b2a063ca48 100644 --- a/tests/drivers/video/api/src/video_emul.c +++ b/tests/drivers/video/api/src/video_emul.c @@ -158,7 +158,6 @@ ZTEST(video_common, test_video_vbuf) fmt.pixelformat = caps.format_caps[0].pixelformat; fmt.width = caps.format_caps[0].width_max; fmt.height = caps.format_caps[0].height_max; - fmt.pitch = fmt.width * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; fmt.type = type; zexpect_ok(video_set_format(rx_dev, &fmt)); From e79dbbf9669b0e675e7c65b52a0606e7b3b5612b Mon Sep 17 00:00:00 2001 From: Phi Bang Nguyen Date: Wed, 14 May 2025 23:15:45 +0200 Subject: [PATCH 12/13] drivers: video: mcux_csi: Don't set_format() in get_format() Do not set_format() when doing get_format(). This design seems initially to simplify the sample (just get_format() and everything works out of the box) but it makes thing incomprehensive and error prone. Signed-off-by: Phi Bang Nguyen --- drivers/video/video_mcux_csi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/video/video_mcux_csi.c b/drivers/video/video_mcux_csi.c index f4226ab58e126..4527beaf30210 100644 --- a/drivers/video/video_mcux_csi.c +++ b/drivers/video/video_mcux_csi.c @@ -184,8 +184,8 @@ static int video_mcux_csi_get_fmt(const struct device *dev, struct video_format #if defined(CONFIG_VIDEO_MCUX_MIPI_CSI2RX) video_pix_fmt_convert(fmt, true); #endif - /* align CSI with source fmt */ - return video_mcux_csi_set_fmt(dev, fmt); + + return 0; } return -EIO; From 115995015f19633fba692251cbf860b7a5bb6123 Mon Sep 17 00:00:00 2001 From: Phi Bang Nguyen Date: Wed, 14 May 2025 23:24:46 +0200 Subject: [PATCH 13/13] drivers: video: mcux_smartdma: Don't set_format() in get_format() Do not set_format() when doing get_format(). This design seems initially to simplify the sample (just get_format() and everything works out of the box) but it makes thing incomprehensive and error prone. Signed-off-by: Phi Bang Nguyen --- drivers/video/video_mcux_smartdma.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/drivers/video/video_mcux_smartdma.c b/drivers/video/video_mcux_smartdma.c index 71666a10e5fc4..bb4ac1025f248 100644 --- a/drivers/video/video_mcux_smartdma.c +++ b/drivers/video/video_mcux_smartdma.c @@ -275,16 +275,7 @@ static int nxp_video_sdma_get_format(const struct device *dev, struct video_form if ((fmt->pixelformat != fmts[0].pixelformat) || (fmt->width != fmts[0].width_min) || (fmt->height != fmts[0].height_min)) { - /* Update format of sensor */ - fmt->pixelformat = fmts[0].pixelformat; - fmt->width = fmts[0].width_min; - fmt->height = fmts[0].height_min; - fmt->pitch = fmts[0].width_min * 2; - ret = video_set_format(config->sensor_dev, fmt); - if (ret < 0) { - LOG_ERR("Sensor device does not support RGB565"); - return ret; - } + return -ENOTSUP; } fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE;