From 1dec42af2094a121814abca78b94b1edbc256186 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Mon, 31 Mar 2025 20:02:56 +0000 Subject: [PATCH 1/7] drivers: video: common: introduce CCI utilities Add a library for the Camera Common Interface, part of the MIPI CSI protocol standard defining methods to configure a camera device over I2C, such as which size for the register address/data. Signed-off-by: Josuah Demangeon --- drivers/video/Kconfig | 8 ++ drivers/video/video_common.c | 143 ++++++++++++++++++++++++++++++++++- drivers/video/video_common.h | 138 +++++++++++++++++++++++++++++++++ 3 files changed, 288 insertions(+), 1 deletion(-) create mode 100644 drivers/video/video_common.h diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index a3b163443628..ec6da8033b09 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -50,6 +50,14 @@ config VIDEO_BUFFER_SMH_ATTRIBUTE 1: SMH_REG_ATTR_NON_CACHEABLE 2: SMH_REG_ATTR_EXTERNAL +config VIDEO_I2C_RETRY_NUM + int "Number of attempts for retrying I2C communication if it failed" + default 3 + help + The default is there to reduce the chance of failure (i.e. occasional EMI) without + flooding the I2C bus upon error with too many retries. There is a 1ms wait time between + every retry. + source "drivers/video/Kconfig.esp32_dvp" source "drivers/video/Kconfig.mcux_csi" diff --git a/drivers/video/video_common.c b/drivers/video/video_common.c index 5028a5f67a1a..52e8fb2b836c 100644 --- a/drivers/video/video_common.c +++ b/drivers/video/video_common.c @@ -1,6 +1,6 @@ /* * Copyright (c) 2019, Linaro Limited - * Copyright (c) 2024, tinyVision.ai Inc. + * Copyright (c) 2024-2025, tinyVision.ai Inc. * * SPDX-License-Identifier: Apache-2.0 */ @@ -8,7 +8,16 @@ #include #include +#include +#include +#include #include +#include +#include + +#include "video_common.h" + +LOG_MODULE_REGISTER(video_common, CONFIG_VIDEO_LOG_LEVEL); #if defined(CONFIG_VIDEO_BUFFER_USE_SHARED_MULTI_HEAP) #include @@ -164,3 +173,135 @@ void video_closest_frmival(const struct device *dev, enum video_endpoint_id ep, } } } + +int video_read_cci_reg(const struct i2c_dt_spec *i2c, uint32_t flags, uint32_t *data) +{ + size_t addr_size = FIELD_GET(VIDEO_REG_ADDR_SIZE_MASK, flags); + size_t data_size = FIELD_GET(VIDEO_REG_DATA_SIZE_MASK, flags); + bool big_endian = FIELD_GET(VIDEO_REG_ENDIANNESS_MASK, flags); + uint16_t addr = FIELD_GET(VIDEO_REG_ADDR_MASK, flags); + uint8_t buf_w[sizeof(uint16_t)] = {0}; + uint8_t *data_ptr; + int ret; + + if (big_endian) { + /* Casting between data sizes in big-endian requires re-aligning */ + *data = 0; + data_ptr = (uint8_t *)data + sizeof(data) - data_size; + } else { + /* Casting between data sizes in little-endian is a no-op */ + *data = 0; + data_ptr = (uint8_t *)data; + } + + for (int i = 0; i < data_size; i++) { + if (addr_size == 1) { + buf_w[0] = addr + i; + } else { + sys_put_be16(addr + i, &buf_w[0]); + } + + ret = i2c_write_read_dt(i2c, buf_w, addr_size, &data_ptr[i], 1); + if (ret != 0) { + LOG_ERR("Failed to read from register 0x%x", addr + i); + return ret; + } + + LOG_HEXDUMP_DBG(buf_w, addr_size, "Data written to the I2C device..."); + LOG_HEXDUMP_DBG(&data_ptr[i], 1, "... data read back from the I2C device"); + } + + *data = big_endian ? sys_be32_to_cpu(*data) : sys_le32_to_cpu(*data); + + return 0; +} + +static int video_write_reg_retry(const struct i2c_dt_spec *i2c, uint8_t *buf_w, size_t size) +{ + int ret = 0; + + for (int i = 0; i < CONFIG_VIDEO_I2C_RETRY_NUM; i++) { + ret = i2c_write_dt(i2c, buf_w, size); + if (ret == 0) { + return 0; + } + + k_sleep(K_MSEC(1)); + } + + LOG_HEXDUMP_ERR(buf_w, size, "failed to write register configuration over I2C"); + + return ret; +} + +int video_write_cci_reg(const struct i2c_dt_spec *i2c, uint32_t flags, uint32_t data) +{ + size_t addr_size = FIELD_GET(VIDEO_REG_ADDR_SIZE_MASK, flags); + size_t data_size = FIELD_GET(VIDEO_REG_DATA_SIZE_MASK, flags); + bool big_endian = FIELD_GET(VIDEO_REG_ENDIANNESS_MASK, flags); + uint16_t addr = FIELD_GET(VIDEO_REG_ADDR_MASK, flags); + uint8_t buf_w[sizeof(uint16_t) + sizeof(uint32_t)] = {0}; + uint8_t *data_ptr; + int ret; + + if (big_endian) { + /* Casting between data sizes in big-endian requires re-aligning */ + data = sys_cpu_to_be32(data); + data_ptr = (uint8_t *)&data + sizeof(data) - data_size; + } else { + /* Casting between data sizes in little-endian is a no-op */ + data = sys_cpu_to_le32(data); + data_ptr = (uint8_t *)&data; + } + + for (int i = 0; i < data_size; i++) { + /* The address is always big-endian as per CCI standard */ + if (addr_size == 1) { + buf_w[0] = addr + i; + } else { + sys_put_be16(addr + i, &buf_w[0]); + } + + buf_w[addr_size] = data_ptr[i]; + + LOG_HEXDUMP_DBG(buf_w, addr_size + 1, "Data written to the I2C device"); + + ret = video_write_reg_retry(i2c, buf_w, addr_size + 1); + if (ret != 0) { + LOG_ERR("Failed to write to register 0x%x", addr + i); + return ret; + } + } + + return 0; +} + +int video_write_cci_field(const struct i2c_dt_spec *i2c, uint32_t reg_addr, uint32_t field_mask, + uint32_t field_value) +{ + uint32_t reg; + int ret; + + ret = video_read_cci_reg(i2c, reg_addr, ®); + if (ret != 0) { + return ret; + } + + return video_write_cci_reg(i2c, reg_addr, (reg & ~field_mask) | field_value); +} + +int video_write_cci_multi(const struct i2c_dt_spec *i2c, const struct video_reg *regs) +{ + int ret; + + for (int i = 0; regs[i].addr != 0; i++) { + ret = video_write_cci_reg(i2c, regs[i].addr, regs[i].data); + if (ret != 0) { + LOG_ERR("Failed to write 0x%04x to register 0x%02x", + regs[i].data, regs[i].addr); + return ret; + } + } + + return 0; +} diff --git a/drivers/video/video_common.h b/drivers/video/video_common.h new file mode 100644 index 000000000000..16d59fd4e4ec --- /dev/null +++ b/drivers/video/video_common.h @@ -0,0 +1,138 @@ +/* + * Copyright (c) 2025 tinyVision.ai Inc. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef ZEPHYR_DRIVERS_VIDEO_COMMON_H_ +#define ZEPHYR_DRIVERS_VIDEO_COMMON_H_ + +#include + +#include +#include +#include +#include +#include + +/** + * Type used by register tables that have either the address or value 16-bit wide. + */ +struct video_reg { + /** Address of the register to write to as well as */ + uint32_t addr; + /** Value to write in this address */ + uint32_t data; +}; + +/** + * @defgroup video_cci Video Camera Control Interface (CCI) handling + * + * The Camera Control Interface (CCI) is an I2C communication scheme part of MIPI-CSI. + * It defines how register addresses and register values are packed into I2C messages. + * + * After the I2C device address, I2C messages payload contain: + * + * 1. THe 8-bit or 16-bit of the address in big-endian, written to the device. + * 3. The 8-bit of the register data either read or written. + * + * To write to registers larger than 8-bit, multiple read/writes messages are issued. + * Endianness and segmentation of larger registers are defined on a per-sensor basis. + * + * @{ + */ + +/** @cond INTERNAL_HIDDEN */ +#define VIDEO_REG_ENDIANNESS_MASK (uint32_t)(GENMASK(24, 24)) +#define VIDEO_REG_ADDR_SIZE_MASK (uint32_t)(GENMASK(23, 20)) +#define VIDEO_REG_DATA_SIZE_MASK (uint32_t)(GENMASK(19, 16)) +#define VIDEO_REG_ADDR_MASK (uint32_t)(GENMASK(15, 0)) + +#define VIDEO_REG(addr_size, data_size, endianness) \ + (FIELD_PREP(VIDEO_REG_ADDR_SIZE_MASK, (addr_size)) | \ + FIELD_PREP(VIDEO_REG_DATA_SIZE_MASK, (data_size)) | \ + FIELD_PREP(VIDEO_REG_ENDIANNESS_MASK, (endianness))) +/** @endcond */ + +/** Flag a register as 8-bit address size, 8-bit data size */ +#define VIDEO_REG_ADDR8_DATA8 VIDEO_REG(1, 1, false) +/** Flag a register as 8-bit address size, 16-bit data size, little-endian */ +#define VIDEO_REG_ADDR8_DATA16_LE VIDEO_REG(1, 2, false) +/** Flag a register as 8-bit address size, 16-bit data size, big-endian */ +#define VIDEO_REG_ADDR8_DATA16_BE VIDEO_REG(1, 2, true) +/** Flag a register as 8-bit address size, 24-bit data size, little-endian */ +#define VIDEO_REG_ADDR8_DATA24_LE VIDEO_REG(1, 3, false) +/** Flag a register as 8-bit address size, 24-bit data size, big-endian */ +#define VIDEO_REG_ADDR8_DATA24_BE VIDEO_REG(1, 3, true) +/** Flag a register as 8-bit address size, 32-bit data size, little-endian */ +#define VIDEO_REG_ADDR8_DATA32_LE VIDEO_REG(1, 4, false) +/** Flag a register as 8-bit address size, 32-bit data size, big-endian */ +#define VIDEO_REG_ADDR8_DATA32_BE VIDEO_REG(1, 4, true) +/** Flag a register as 16-bit address size, 8-bit data size */ +#define VIDEO_REG_ADDR16_DATA8 VIDEO_REG(2, 1, false) +/** Flag a register as 16-bit address size, 16-bit data size, little-endian */ +#define VIDEO_REG_ADDR16_DATA16_LE VIDEO_REG(2, 2, false) +/** Flag a register as 16-bit address size, 16-bit data size, big-endian */ +#define VIDEO_REG_ADDR16_DATA16_BE VIDEO_REG(2, 2, true) +/** Flag a register as 16-bit address size, 24-bit data size, little-endian */ +#define VIDEO_REG_ADDR16_DATA24_LE VIDEO_REG(2, 3, false) +/** Flag a register as 16-bit address size, 24-bit data size, big-endian */ +#define VIDEO_REG_ADDR16_DATA24_BE VIDEO_REG(2, 3, true) +/** Flag a register as 16-bit address size, 32-bit data size, little-endian */ +#define VIDEO_REG_ADDR16_DATA32_LE VIDEO_REG(2, 4, false) +/** Flag a register as 16-bit address size, 32-bit data size, big-endian */ +#define VIDEO_REG_ADDR16_DATA32_BE VIDEO_REG(2, 4, true) + +/** + * @brief Write a Camera Control Interface register value with the specified address and size. + * + * The size of the register address and register data passed as flags in the high bits of + * @p reg_addr in the unused bits of the address. + * + * @brief i2c Reference to the video device on an I2C bus. + * @brief reg_addr Address of the register to fill with @reg_value along with size information. + * @brief reg_value Value to write at this address, the size to write is encoded in the address. + */ +int video_write_cci_reg(const struct i2c_dt_spec *i2c, uint32_t reg_addr, uint32_t reg_value); + + +/** + * @brief Perform a read-modify-write operation on a register given an address, mask and value. + * + * The size of the register address and register data passed as flags in the high bits of + * @p reg_addr in the unused bits of the address. + * + * @brief i2c Reference to the video device on an I2C bus. + * @brief reg_addr Address of the register to fill with @reg_value along with size information. + * @brief field_mask Mask of the field to insert into the existing value. + * @brief field_value Value to write at this address, the size to write is encoded in the address. + */ +int video_write_cci_field(const struct i2c_dt_spec *i2c, uint32_t reg_addr, uint32_t field_mask, + uint32_t field_value); + +/** + * @brief Read a Camera Control Interace register value from the specified address and size. + * + * The size of the register address and register data passed as flags in the high bits of + * @p reg_addr in the unused bits of the address. + * + * @brief i2c Reference to the video device on an I2C bus. + * @brief reg_addr Address of the register to fill with @reg_value along with size information. + * @brief reg_value Value to write at this address, the size to write is encoded in the address. + * This is a 32-bit integer pointer even when reading 8-bit or 16 bits value. + */ +int video_read_cci_reg(const struct i2c_dt_spec *i2c, uint32_t reg_addr, uint32_t *reg_value); + +/** + * @brief Write a complete table of registers to a device one by one. + * + * The address present in the registers need to be encoding the size information using the macros + * such as @ref VIDEO_ADDR16_REG8(). The last element must be empty (@c {0}) to mark the end of the + * table. + * + * @brief i2c Reference to the video device on an I2C bus. + * @brief regs Array of address/value pairs to write to the device sequentially. + */ +int video_write_cci_multi(const struct i2c_dt_spec *i2c, const struct video_reg *regs); + +#endif /* ZEPHYR_DRIVERS_VIDEO_COMMON_H_ */ From ef13afe8157c298325b892ce2b4d0bf547af61fb Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Mon, 31 Mar 2025 23:50:59 +0000 Subject: [PATCH 2/7] drivers: video: gc2145: switch to video CCI library Reduce the amount of code in the GC2145 driver by switching to the CCI helpers in video_common.c. The batch I2C write functions are not used to avoid increasing the size of the ROM, as struct video_reg is 8 bytes while the ad-hoc struct gc2145_reg is 2 bytes. Signed-off-by: Josuah Demangeon --- drivers/video/gc2145.c | 207 ++++++++++++----------------------------- 1 file changed, 57 insertions(+), 150 deletions(-) diff --git a/drivers/video/gc2145.c b/drivers/video/gc2145.c index 8ac3cc24033c..1bfccba9cd60 100644 --- a/drivers/video/gc2145.c +++ b/drivers/video/gc2145.c @@ -12,30 +12,36 @@ #include #include #include - #include + +#include "video_common.h" + LOG_MODULE_REGISTER(video_gc2145, CONFIG_VIDEO_LOG_LEVEL); -#define GC2145_REG_AMODE1 0x17 +#define GC2145_REG8(addr) ((addr) | VIDEO_REG_ADDR8_DATA8) +#define GC2145_REG16(addr) ((addr) | VIDEO_REG_ADDR8_DATA16_LE) + +#define GC2145_REG_AMODE1 GC2145_REG8(0x17) #define GC2145_AMODE1_WINDOW_MASK 0xFC -#define GC2145_REG_AMODE1_DEF 0x14 -#define GC2145_REG_OUTPUT_FMT 0x84 -#define GC2145_REG_OUTPUT_FMT_MASK 0x1F -#define GC2145_REG_OUTPUT_FMT_RGB565 0x06 -#define GC2145_REG_OUTPUT_FMT_YCBYCR 0x02 -#define GC2145_REG_SYNC_MODE 0x86 -#define GC2145_REG_SYNC_MODE_DEF 0x23 -#define GC2145_REG_SYNC_MODE_COL_SWITCH 0x10 -#define GC2145_REG_SYNC_MODE_ROW_SWITCH 0x20 -#define GC2145_REG_RESET 0xFE -#define GC2145_REG_SW_RESET 0x80 +#define GC2145_REG_AMODE1_DEF GC2145_REG8(0x14) +#define GC2145_REG_OUTPUT_FMT GC2145_REG8(0x84) +#define GC2145_REG_OUTPUT_FMT_MASK GC2145_REG8(0x1F) +#define GC2145_REG_OUTPUT_FMT_RGB565 GC2145_REG8(0x06) +#define GC2145_REG_OUTPUT_FMT_YCBYCR GC2145_REG8(0x02) +#define GC2145_REG_SYNC_MODE GC2145_REG8(0x86) +#define GC2145_REG_SYNC_MODE_DEF GC2145_REG8(0x23) +#define GC2145_REG_SYNC_MODE_COL_SWITCH GC2145_REG8(0x10) +#define GC2145_REG_SYNC_MODE_ROW_SWITCH GC2145_REG8(0x20) +#define GC2145_REG_RESET GC2145_REG8(0xFE) +#define GC2145_REG_SW_RESET GC2145_REG8(0x80) +#define GC2145_REG_CHIP_ID GC2145_REG8(0xF0) #define GC2145_SET_P0_REGS 0x00 -#define GC2145_REG_CROP_ENABLE 0x90 +#define GC2145_REG_CROP_ENABLE GC2145_REG8(0x90) #define GC2145_CROP_SET_ENABLE 0x01 -#define GC2145_REG_BLANK_WINDOW_BASE 0x09 -#define GC2145_REG_WINDOW_BASE 0x91 -#define GC2145_REG_SUBSAMPLE 0x99 -#define GC2145_REG_SUBSAMPLE_MODE 0x9A +#define GC2145_REG_BLANK_WINDOW_BASE GC2145_REG8(0x09) +#define GC2145_REG_WINDOW_BASE GC2145_REG8(0x91) +#define GC2145_REG_SUBSAMPLE GC2145_REG8(0x99) +#define GC2145_REG_SUBSAMPLE_MODE GC2145_REG8(0x9A) #define GC2145_SUBSAMPLE_MODE_SMOOTH 0x0E #define UXGA_HSIZE 1600 @@ -103,8 +109,8 @@ static const struct gc2145_reg default_regs[] = { {0x81, 0x26}, {0x82, 0xfa}, {0x83, 0x00}, - {GC2145_REG_OUTPUT_FMT, 0x06}, - {GC2145_REG_SYNC_MODE, 0x23}, + {(uint8_t)GC2145_REG_OUTPUT_FMT, 0x06}, + {(uint8_t)GC2145_REG_SYNC_MODE, 0x23}, {0x88, 0x03}, {0x89, 0x03}, {0x85, 0x08}, @@ -195,7 +201,7 @@ static const struct gc2145_reg default_regs[] = { {0x0c, 0x10}, {0x11, 0x10}, {0x13, 0x68}, - {GC2145_REG_OUTPUT_FMT, 0x00}, + {(uint8_t)GC2145_REG_OUTPUT_FMT, 0x00}, {0x1c, 0x11}, {0x1e, 0x61}, {0x1f, 0x35}, @@ -226,8 +232,8 @@ static const struct gc2145_reg default_regs[] = { {0x81, 0x08}, {0x82, 0x05}, {0x83, 0x08}, - {GC2145_REG_OUTPUT_FMT, 0x0a}, - {GC2145_REG_SYNC_MODE, 0xf0}, + {(uint8_t)GC2145_REG_OUTPUT_FMT, 0x0a}, + {(uint8_t)GC2145_REG_SYNC_MODE, 0xf0}, {0x87, 0x50}, {0x88, 0x15}, {0x89, 0xb0}, @@ -262,7 +268,7 @@ static const struct gc2145_reg default_regs[] = { {0x14, 0x27}, {0x15, 0x37}, {0x16, 0x45}, - {GC2145_REG_OUTPUT_FMT, 0x53}, + {(uint8_t)GC2145_REG_OUTPUT_FMT, 0x53}, {0x18, 0x69}, {0x19, 0x7d}, {0x1a, 0x8f}, @@ -718,54 +724,6 @@ static const struct video_format_cap fmts[] = { {0}, }; -static int gc2145_write_reg(const struct i2c_dt_spec *spec, uint8_t reg_addr, uint8_t value) -{ - int ret; - uint8_t tries = 3; - - /* - * It rarely happens that the camera does not respond with ACK signal. - * In that case it usually responds on 2nd try but there is a 3rd one - * just to be sure that the connection error is not caused by driver - * itself. - */ - do { - ret = i2c_reg_write_byte_dt(spec, reg_addr, value); - if (!ret) { - return 0; - } - /* If writing failed wait 5ms before next attempt */ - k_msleep(5); - } while (tries-- > 0); - - LOG_ERR("failed to write 0x%x to 0x%x,", value, reg_addr); - return ret; -} - -static int gc2145_read_reg(const struct i2c_dt_spec *spec, uint8_t reg_addr, uint8_t *value) -{ - int ret; - uint8_t tries = 3; - - /* - * It rarely happens that the camera does not respond with ACK signal. - * In that case it usually responds on 2nd try but there is a 3rd one - * just to be sure that the connection error is not caused by driver - * itself. - */ - do { - ret = i2c_reg_read_byte_dt(spec, reg_addr, value); - if (!ret) { - return 0; - } - /* If writing failed wait 5ms before next attempt */ - k_msleep(5); - } while (tries-- > 0); - - LOG_ERR("failed to read 0x%x register", reg_addr); - return ret; -} - static int gc2145_write_all(const struct device *dev, const struct gc2145_reg *regs, uint16_t reg_num) { @@ -774,7 +732,7 @@ static int gc2145_write_all(const struct device *dev, const struct gc2145_reg *r for (uint16_t i = 0; i < reg_num; i++) { int ret; - ret = gc2145_write_reg(&cfg->i2c, regs[i].addr, regs[i].value); + ret = video_write_cci_reg(&cfg->i2c, GC2145_REG8(regs[i].addr), regs[i].value); if (ret < 0) { return ret; } @@ -789,7 +747,7 @@ static int gc2145_soft_reset(const struct device *dev) const struct gc2145_config *cfg = dev->config; /* Initiate system reset */ - ret = gc2145_write_reg(&cfg->i2c, GC2145_REG_RESET, GC2145_REG_SW_RESET); + ret = video_write_cci_reg(&cfg->i2c, GC2145_REG_RESET, GC2145_REG_SW_RESET); k_msleep(300); @@ -798,87 +756,49 @@ static int gc2145_soft_reset(const struct device *dev) static int gc2145_set_ctrl_vflip(const struct device *dev, bool enable) { - int ret; const struct gc2145_config *cfg = dev->config; - uint8_t old_value; - - ret = gc2145_read_reg(&cfg->i2c, GC2145_REG_AMODE1, &old_value); - if (ret < 0) { - return ret; - } /* Set the vertical flip state */ - return gc2145_write_reg(&cfg->i2c, GC2145_REG_AMODE1, - (old_value & GC2145_AMODE1_WINDOW_MASK) | (enable << 1)); + return video_write_cci_field(&cfg->i2c, GC2145_REG_AMODE1, GC2145_AMODE1_WINDOW_MASK, + FIELD_PREP(GC2145_AMODE1_WINDOW_MASK, enable << 1)); } static int gc2145_set_ctrl_hmirror(const struct device *dev, bool enable) { - int ret; const struct gc2145_config *cfg = dev->config; - uint8_t old_value; - - ret = gc2145_read_reg(&cfg->i2c, GC2145_REG_AMODE1, &old_value); - if (ret < 0) { - return ret; - } /* Set the horizontal mirror state */ - return gc2145_write_reg(&cfg->i2c, GC2145_REG_AMODE1, - (old_value & GC2145_AMODE1_WINDOW_MASK) | enable); + return video_write_cci_field(&cfg->i2c, GC2145_REG_AMODE1, GC2145_AMODE1_WINDOW_MASK, + FIELD_PREP(GC2145_AMODE1_WINDOW_MASK, enable)); } -static int gc2145_set_window(const struct device *dev, uint16_t reg, uint16_t x, uint16_t y, - uint16_t w, uint16_t h) +static int gc2145_set_window(const struct device *dev, uint32_t reg, uint32_t x_offset, + uint32_t y_offset, uint32_t window_width, uint32_t window_height) { int ret; const struct gc2145_config *cfg = dev->config; - ret = gc2145_write_reg(&cfg->i2c, GC2145_REG_RESET, GC2145_SET_P0_REGS); - if (ret < 0) { - return ret; - } - - /* Y/row offset */ - ret = gc2145_write_reg(&cfg->i2c, reg++, y >> 8); + ret = video_write_cci_reg(&cfg->i2c, GC2145_REG_RESET, GC2145_SET_P0_REGS); if (ret < 0) { return ret; } - ret = gc2145_write_reg(&cfg->i2c, reg++, y & 0xff); + ret = video_write_cci_reg(&cfg->i2c, GC2145_REG16(reg + 0), y_offset); if (ret < 0) { return ret; } - /* X/col offset */ - ret = gc2145_write_reg(&cfg->i2c, reg++, x >> 8); + ret = video_write_cci_reg(&cfg->i2c, GC2145_REG16(reg + 2), x_offset); if (ret < 0) { return ret; } - ret = gc2145_write_reg(&cfg->i2c, reg++, x & 0xff); + ret = video_write_cci_reg(&cfg->i2c, GC2145_REG16(reg + 4), window_height); if (ret < 0) { return ret; } - /* Window height */ - ret = gc2145_write_reg(&cfg->i2c, reg++, h >> 8); - if (ret < 0) { - return ret; - } - - ret = gc2145_write_reg(&cfg->i2c, reg++, h & 0xff); - if (ret < 0) { - return ret; - } - - /* Window width */ - ret = gc2145_write_reg(&cfg->i2c, reg++, w >> 8); - if (ret < 0) { - return ret; - } - - ret = gc2145_write_reg(&cfg->i2c, reg++, w & 0xff); + ret = video_write_cci_reg(&cfg->i2c, GC2145_REG16(reg + 6), window_width); if (ret < 0) { return ret; } @@ -889,10 +809,9 @@ static int gc2145_set_window(const struct device *dev, uint16_t reg, uint16_t x, static int gc2145_set_output_format(const struct device *dev, int output_format) { int ret; - uint8_t old_value; const struct gc2145_config *cfg = dev->config; - ret = gc2145_write_reg(&cfg->i2c, GC2145_REG_RESET, GC2145_SET_P0_REGS); + ret = video_write_cci_reg(&cfg->i2c, GC2145_REG_RESET, GC2145_SET_P0_REGS); if (ret < 0) { return ret; } @@ -907,13 +826,8 @@ static int gc2145_set_output_format(const struct device *dev, int output_format) return -ENOTSUP; } - ret = gc2145_read_reg(&cfg->i2c, GC2145_REG_OUTPUT_FMT, &old_value); - if (ret < 0) { - return ret; - } - - ret = gc2145_write_reg(&cfg->i2c, GC2145_REG_OUTPUT_FMT, - (old_value & ~GC2145_REG_OUTPUT_FMT_MASK) | output_format); + ret = video_write_cci_field(&cfg->i2c, GC2145_REG_OUTPUT_FMT, GC2145_REG_OUTPUT_FMT_MASK, + output_format); if (ret < 0) { return ret; } @@ -965,31 +879,32 @@ static int gc2145_set_resolution(const struct device *dev, uint32_t w, uint32_t win_y = ((UXGA_VSIZE - win_h) / 2); /* Set readout window first. */ - ret = gc2145_set_window(dev, GC2145_REG_BLANK_WINDOW_BASE, win_x, win_y, win_w + 16, - win_h + 8); + ret = gc2145_set_window(dev, (uint8_t)GC2145_REG_BLANK_WINDOW_BASE, win_x, win_y, + win_w + 16, win_h + 8); if (ret < 0) { return ret; } /* Set cropping window next. */ - ret = gc2145_set_window(dev, GC2145_REG_WINDOW_BASE, x, y, w, h); + ret = gc2145_set_window(dev, (uint8_t)GC2145_REG_WINDOW_BASE, x, y, w, h); if (ret < 0) { return ret; } /* Enable crop */ - ret = gc2145_write_reg(&cfg->i2c, GC2145_REG_CROP_ENABLE, GC2145_CROP_SET_ENABLE); + ret = video_write_cci_reg(&cfg->i2c, GC2145_REG_CROP_ENABLE, GC2145_CROP_SET_ENABLE); if (ret < 0) { return ret; } /* Set Sub-sampling ratio and mode */ - ret = gc2145_write_reg(&cfg->i2c, GC2145_REG_SUBSAMPLE, ((r_ratio << 4) | c_ratio)); + ret = video_write_cci_reg(&cfg->i2c, GC2145_REG_SUBSAMPLE, ((r_ratio << 4) | c_ratio)); if (ret < 0) { return ret; } - ret = gc2145_write_reg(&cfg->i2c, GC2145_REG_SUBSAMPLE_MODE, GC2145_SUBSAMPLE_MODE_SMOOTH); + ret = video_write_cci_reg(&cfg->i2c, GC2145_REG_SUBSAMPLE_MODE, + GC2145_SUBSAMPLE_MODE_SMOOTH); if (ret < 0) { return ret; } @@ -1008,21 +923,13 @@ static uint8_t gc2145_check_connection(const struct device *dev) { int ret; const struct gc2145_config *cfg = dev->config; - uint8_t reg_chip_id[2]; - uint16_t chip_id; + uint32_t chip_id; - ret = gc2145_read_reg(&cfg->i2c, 0xf0, ®_chip_id[0]); + ret = video_read_cci_reg(&cfg->i2c, GC2145_REG_CHIP_ID, &chip_id); if (ret < 0) { return ret; } - ret = gc2145_read_reg(&cfg->i2c, 0xf1, ®_chip_id[1]); - if (ret < 0) { - return ret; - } - - chip_id = reg_chip_id[0] << 8 | reg_chip_id[1]; - if (chip_id != 0x2145 && chip_id != 0x2155) { LOG_WRN("Unexpected GC2145 chip ID: 0x%04x", chip_id); } @@ -1089,8 +996,8 @@ static int gc2145_set_stream(const struct device *dev, bool enable) { const struct gc2145_config *cfg = dev->config; - return enable ? gc2145_write_reg(&cfg->i2c, 0xf2, 0x0f) - : gc2145_write_reg(&cfg->i2c, 0xf2, 0x00); + return enable ? video_write_cci_reg(&cfg->i2c, GC2145_REG8(0xf2), 0x0f) + : video_write_cci_reg(&cfg->i2c, GC2145_REG8(0xf2), 0x00); } static int gc2145_get_caps(const struct device *dev, enum video_endpoint_id ep, From d2c9f4b3f3cdb4b5cf09cb53311a0bbde18b9fc0 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Wed, 12 Mar 2025 16:46:07 +0100 Subject: [PATCH 3/7] drivers: video: emul-imager: use the emulated I2C bus Replace the ad-hoc register emulation by the dedicated I2C emulator, making it usable with the same APIs as every other image sensors. Signed-off-by: Josuah Demangeon --- drivers/video/Kconfig.emul_imager | 1 + drivers/video/video_emul_imager.c | 105 +++++++++++++++++++--------- tests/drivers/video/api/app.overlay | 39 +++++------ tests/drivers/video/api/prj.conf | 2 + 4 files changed, 92 insertions(+), 55 deletions(-) diff --git a/drivers/video/Kconfig.emul_imager b/drivers/video/Kconfig.emul_imager index fc4956178337..853ed2eaa918 100644 --- a/drivers/video/Kconfig.emul_imager +++ b/drivers/video/Kconfig.emul_imager @@ -5,6 +5,7 @@ config VIDEO_EMUL_IMAGER bool "Software implementation of an imager" depends on DT_HAS_ZEPHYR_VIDEO_EMUL_IMAGER_ENABLED default y + depends on EMUL help Enable driver for the emulated Imager. A line buffer contains the color pattern within the imager data struct, at the first diff --git a/drivers/video/video_emul_imager.c b/drivers/video/video_emul_imager.c index c7e26a9ff54f..bae5d45009fa 100644 --- a/drivers/video/video_emul_imager.c +++ b/drivers/video/video_emul_imager.c @@ -15,20 +15,21 @@ #include #include #include +#include #include LOG_MODULE_REGISTER(video_emul_imager, CONFIG_VIDEO_LOG_LEVEL); -#define EMUL_IMAGER_REG_SENSOR_ID 0x0000 +#define EMUL_IMAGER_REG_SENSOR_ID 0x00 #define EMUL_IMAGER_SENSOR_ID 0x99 -#define EMUL_IMAGER_REG_CTRL 0x0001 -#define EMUL_IMAGER_REG_INIT1 0x0002 -#define EMUL_IMAGER_REG_INIT2 0x0003 -#define EMUL_IMAGER_REG_TIMING1 0x0004 -#define EMUL_IMAGER_REG_TIMING2 0x0005 -#define EMUL_IMAGER_REG_TIMING3 0x0006 -#define EMUL_IMAGER_REG_CUSTOM 0x0007 -#define EMUL_IMAGER_REG_FORMAT 0x000a +#define EMUL_IMAGER_REG_CTRL 0x01 +#define EMUL_IMAGER_REG_INIT1 0x02 +#define EMUL_IMAGER_REG_INIT2 0x03 +#define EMUL_IMAGER_REG_TIMING1 0x04 +#define EMUL_IMAGER_REG_TIMING2 0x05 +#define EMUL_IMAGER_REG_TIMING3 0x06 +#define EMUL_IMAGER_REG_CUSTOM 0x07 +#define EMUL_IMAGER_REG_FORMAT 0x0a #define EMUL_IMAGER_PATTERN_OFF 0x00 #define EMUL_IMAGER_PATTERN_BARS1 0x01 #define EMUL_IMAGER_PATTERN_BARS2 0x02 @@ -36,9 +37,6 @@ LOG_MODULE_REGISTER(video_emul_imager, CONFIG_VIDEO_LOG_LEVEL); /* Custom control that is just an I2C write for example and test purpose */ #define EMUL_IMAGER_CID_CUSTOM (VIDEO_CID_PRIVATE_BASE + 0x01) -/* Emulated register bank */ -uint8_t emul_imager_fake_regs[10]; - enum emul_imager_fmt_id { RGB565_320x240, YUYV_320x240, @@ -78,10 +76,10 @@ static const struct emul_imager_reg emul_imager_init_regs[] = { {EMUL_IMAGER_REG_INIT1, 0x10}, {EMUL_IMAGER_REG_INIT2, 0x00}, /* Undocumented registers from the vendor */ - {0x1200, 0x01}, - {0x1204, 0x01}, - {0x1205, 0x20}, - {0x1209, 0x7f}, + {0x80, 0x01}, + {0x84, 0x01}, + {0x85, 0x20}, + {0x89, 0x7f}, {0}, }; static const struct emul_imager_reg emul_imager_rgb565[] = { @@ -152,21 +150,15 @@ static const struct video_format_cap fmts[] = { /* Emulated I2C register interface, to replace with actual I2C calls for real hardware */ static int emul_imager_read_reg(const struct device *const dev, uint8_t reg_addr, uint8_t *value) { - LOG_DBG("Placeholder for I2C read from 0x%02x", reg_addr); - switch (reg_addr) { - case EMUL_IMAGER_REG_SENSOR_ID: - *value = EMUL_IMAGER_SENSOR_ID; - break; - default: - *value = emul_imager_fake_regs[reg_addr]; - } - return 0; + const struct emul_imager_config *cfg = dev->config; + + return i2c_write_read_dt(&cfg->i2c, ®_addr, 1, value, 1); } /* Helper to read a full integer directly from a register */ static int emul_imager_read_int(const struct device *const dev, uint8_t reg_addr, int *value) { - uint8_t val8; + uint8_t val8 = 0; int ret; ret = emul_imager_read_reg(dev, reg_addr, &val8); @@ -177,9 +169,10 @@ static int emul_imager_read_int(const struct device *const dev, uint8_t reg_addr /* Some sensors will need reg8 or reg16 variants. */ static int emul_imager_write_reg(const struct device *const dev, uint8_t reg_addr, uint8_t value) { - LOG_DBG("Placeholder for I2C write 0x%08x to 0x%02x", value, reg_addr); - emul_imager_fake_regs[reg_addr] = value; - return 0; + const struct emul_imager_config *cfg = dev->config; + uint8_t buf_w[] = {reg_addr, value}; + + return i2c_write_dt(&cfg->i2c, buf_w, 2); } static int emul_imager_write_multi(const struct device *const dev, @@ -383,12 +376,13 @@ static DEVICE_API(video, emul_imager_driver_api) = { int emul_imager_init(const struct device *dev) { + const struct emul_imager_config *cfg = dev->config; struct video_format fmt; uint8_t sensor_id; int ret; - if (/* !i2c_is_ready_dt(&cfg->i2c) */ false) { - /* LOG_ERR("Bus %s is not ready", cfg->i2c.bus->name); */ + if (!i2c_is_ready_dt(&cfg->i2c)) { + LOG_ERR("Bus %s is not ready", cfg->i2c.bus->name); return -ENODEV; } @@ -422,7 +416,7 @@ int emul_imager_init(const struct device *dev) static struct emul_imager_data emul_imager_data_##inst; \ \ static const struct emul_imager_config emul_imager_cfg_##inst = { \ - .i2c = /* I2C_DT_SPEC_INST_GET(inst) */ {0}, \ + .i2c = I2C_DT_SPEC_INST_GET(inst), \ }; \ \ DEVICE_DT_INST_DEFINE(inst, &emul_imager_init, NULL, &emul_imager_data_##inst, \ @@ -430,3 +424,50 @@ int emul_imager_init(const struct device *dev) &emul_imager_driver_api); DT_INST_FOREACH_STATUS_OKAY(EMUL_IMAGER_DEFINE) + +/* Simulated I2C bus */ + +static int emul_imager_transfer_i2c(const struct emul *target, struct i2c_msg msgs[], int num_msgs, + int addr) +{ + static uint8_t fake_regs[UINT8_MAX] = { + [EMUL_IMAGER_REG_SENSOR_ID] = EMUL_IMAGER_SENSOR_ID, + }; + + if (num_msgs == 0) { + CODE_UNREACHABLE; + } else if (num_msgs == 1 && + msgs[0].len == 2 && (msgs[0].flags & I2C_MSG_READ) == 0) { + /* Register write */ + fake_regs[msgs[0].buf[0]] = msgs[0].buf[1]; + } else if (num_msgs == 2 && + msgs[0].len == 1 && (msgs[0].flags & I2C_MSG_READ) == 0 && + msgs[1].len == 1 && (msgs[1].flags & I2C_MSG_READ) == 0) { + /* Register write */ + fake_regs[msgs[0].buf[0]] = msgs[1].buf[0]; + } else if (num_msgs == 2 && + msgs[0].len == 1 && (msgs[0].flags & I2C_MSG_READ) == 0 && + msgs[1].len == 1 && (msgs[1].flags & I2C_MSG_READ) != 0) { + /* Register read */ + msgs[1].buf[0] = fake_regs[msgs[0].buf[0]]; + } else { + LOG_ERR("Unsupported I2C operation of %u messages", num_msgs); + return -EIO; + } + + return 0; +} + +static const struct i2c_emul_api emul_imager_i2c_api = { + .transfer = emul_imager_transfer_i2c, +}; + +static int emul_imager_init_i2c(const struct emul *target, const struct device *dev) +{ + return 0; +} + +#define EMUL_I2C_DEFINE(inst) \ + EMUL_DT_INST_DEFINE(inst, &emul_imager_init_i2c, NULL, NULL, &emul_imager_i2c_api, NULL); + +DT_INST_FOREACH_STATUS_OKAY(EMUL_I2C_DEFINE) diff --git a/tests/drivers/video/api/app.overlay b/tests/drivers/video/api/app.overlay index 3e1e02e6a95a..8e6cce51748a 100644 --- a/tests/drivers/video/api/app.overlay +++ b/tests/drivers/video/api/app.overlay @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024 tinyVision.ai Inc. + * Copyright (c) 2024-2025 tinyVision.ai Inc. * SPDX-License-Identifier: Apache-2.0 */ @@ -8,31 +8,9 @@ #address-cells = <1>; #size-cells = <1>; - test_i2c: i2c@10002000 { - #address-cells = <1>; - #size-cells = <0>; - compatible = "vnd,i2c"; - reg = <0x10002000 0x1000>; - clock-frequency = <100000>; - status = "okay"; - - test_video_emul_imager: video_emul_imager@6 { - compatible = "zephyr,video-emul-imager"; - status = "okay"; - reg = <0x6>; - - port { - test_video_emul_imager_ep_out: endpoint { - remote-endpoint-label = "test_video_emul_rx_ep_in"; - }; - }; - }; - }; - test_video_emul_rx: video_emul_rx@10003000 { compatible = "zephyr,video-emul-rx"; reg = <0x10003000 0x1000>; - status = "okay"; port { #address-cells = <1>; @@ -51,3 +29,18 @@ }; }; }; + +&i2c0 { + status = "okay"; + + test_video_emul_imager: video_emul_imager@6 { + compatible = "zephyr,video-emul-imager"; + reg = <0x6>; + + port { + test_video_emul_imager_ep_out: endpoint { + remote-endpoint-label = "test_video_emul_rx_ep_in"; + }; + }; + }; +}; diff --git a/tests/drivers/video/api/prj.conf b/tests/drivers/video/api/prj.conf index 8a6c2151c273..f35903ca73d7 100644 --- a/tests/drivers/video/api/prj.conf +++ b/tests/drivers/video/api/prj.conf @@ -9,3 +9,5 @@ CONFIG_VIDEO_BUFFER_POOL_NUM_MAX=1 CONFIG_SYS_HEAP_BIG_ONLY=y CONFIG_VIDEO_LOG_LEVEL_DBG=y +CONFIG_EMUL=y +CONFIG_I2C=y From 2ece06a0055fc7ee37f1759e2bb93084fbb956a7 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Tue, 1 Apr 2025 13:10:20 +0000 Subject: [PATCH 4/7] drivers: video: emul_imager: switch to the CCI library for I2C Use the newly introduced CCI library instead of local implementation of I2C read/write commands. Signed-off-by: Josuah Demangeon --- drivers/video/video_emul_imager.c | 126 ++++++++++++------------------ 1 file changed, 48 insertions(+), 78 deletions(-) diff --git a/drivers/video/video_emul_imager.c b/drivers/video/video_emul_imager.c index bae5d45009fa..e352011c7340 100644 --- a/drivers/video/video_emul_imager.c +++ b/drivers/video/video_emul_imager.c @@ -18,21 +18,25 @@ #include #include +#include "video_common.h" + LOG_MODULE_REGISTER(video_emul_imager, CONFIG_VIDEO_LOG_LEVEL); -#define EMUL_IMAGER_REG_SENSOR_ID 0x00 +#define EMUL_IMAGER_REG8(addr) ((addr) | VIDEO_REG_ADDR8_DATA8) + +#define EMUL_IMAGER_REG_SENSOR_ID EMUL_IMAGER_REG8(0x00) #define EMUL_IMAGER_SENSOR_ID 0x99 -#define EMUL_IMAGER_REG_CTRL 0x01 -#define EMUL_IMAGER_REG_INIT1 0x02 -#define EMUL_IMAGER_REG_INIT2 0x03 -#define EMUL_IMAGER_REG_TIMING1 0x04 -#define EMUL_IMAGER_REG_TIMING2 0x05 -#define EMUL_IMAGER_REG_TIMING3 0x06 -#define EMUL_IMAGER_REG_CUSTOM 0x07 -#define EMUL_IMAGER_REG_FORMAT 0x0a -#define EMUL_IMAGER_PATTERN_OFF 0x00 -#define EMUL_IMAGER_PATTERN_BARS1 0x01 -#define EMUL_IMAGER_PATTERN_BARS2 0x02 +#define EMUL_IMAGER_REG_CTRL EMUL_IMAGER_REG8(0x01) +#define EMUL_IMAGER_REG_INIT1 EMUL_IMAGER_REG8(0x02) +#define EMUL_IMAGER_REG_INIT2 EMUL_IMAGER_REG8(0x03) +#define EMUL_IMAGER_REG_TIMING1 EMUL_IMAGER_REG8(0x04) +#define EMUL_IMAGER_REG_TIMING2 EMUL_IMAGER_REG8(0x05) +#define EMUL_IMAGER_REG_TIMING3 EMUL_IMAGER_REG8(0x06) +#define EMUL_IMAGER_REG_CUSTOM EMUL_IMAGER_REG8(0x07) +#define EMUL_IMAGER_REG_FORMAT EMUL_IMAGER_REG8(0x0a) +#define EMUL_IMAGER_PATTERN_OFF EMUL_IMAGER_REG8(0x00) +#define EMUL_IMAGER_PATTERN_BARS1 EMUL_IMAGER_REG8(0x01) +#define EMUL_IMAGER_PATTERN_BARS2 EMUL_IMAGER_REG8(0x02) /* Custom control that is just an I2C write for example and test purpose */ #define EMUL_IMAGER_CID_CUSTOM (VIDEO_CID_PRIVATE_BASE + 0x01) @@ -42,18 +46,13 @@ enum emul_imager_fmt_id { YUYV_320x240, }; -struct emul_imager_reg { - uint16_t addr; - uint8_t value; -}; - struct emul_imager_mode { uint8_t fps; /* List of registers lists to configure the various properties of the sensor. * This permits to deduplicate the list of registers in case some lare sections * are repeated across modes, such as the resolution for different FPS. */ - const struct emul_imager_reg *regs[3]; + const struct video_reg *regs[3]; /* More fields can be added according to the needs of the sensor driver */ }; @@ -71,39 +70,39 @@ struct emul_imager_data { }; /* All the I2C registers sent on various scenario */ -static const struct emul_imager_reg emul_imager_init_regs[] = { +static const struct video_reg emul_imager_init_regs[] = { {EMUL_IMAGER_REG_CTRL, 0x00}, {EMUL_IMAGER_REG_INIT1, 0x10}, {EMUL_IMAGER_REG_INIT2, 0x00}, /* Undocumented registers from the vendor */ - {0x80, 0x01}, - {0x84, 0x01}, - {0x85, 0x20}, - {0x89, 0x7f}, + {EMUL_IMAGER_REG8(0x80), 0x01}, + {EMUL_IMAGER_REG8(0x84), 0x01}, + {EMUL_IMAGER_REG8(0x85), 0x20}, + {EMUL_IMAGER_REG8(0x89), 0x7f}, {0}, }; -static const struct emul_imager_reg emul_imager_rgb565[] = { +static const struct video_reg emul_imager_rgb565[] = { {EMUL_IMAGER_REG_FORMAT, 0x01}, {0}, }; -static const struct emul_imager_reg emul_imager_yuyv[] = { +static const struct video_reg emul_imager_yuyv[] = { {EMUL_IMAGER_REG_FORMAT, 0x02}, {0}, }; -static const struct emul_imager_reg emul_imager_320x240[] = { +static const struct video_reg emul_imager_320x240[] = { {EMUL_IMAGER_REG_TIMING1, 0x32}, {EMUL_IMAGER_REG_TIMING2, 0x24}, {0}, }; -static const struct emul_imager_reg emul_imager_15fps[] = { +static const struct video_reg emul_imager_15fps[] = { {EMUL_IMAGER_REG_TIMING3, 15}, {0}, }; -static const struct emul_imager_reg emul_imager_30fps[] = { +static const struct video_reg emul_imager_30fps[] = { {EMUL_IMAGER_REG_TIMING3, 30}, {0}, }; -static const struct emul_imager_reg emul_imager_60fps[] = { +static const struct video_reg emul_imager_60fps[] = { {EMUL_IMAGER_REG_TIMING3, 60}, {0}, }; @@ -147,53 +146,13 @@ static const struct video_format_cap fmts[] = { {0}, }; -/* Emulated I2C register interface, to replace with actual I2C calls for real hardware */ -static int emul_imager_read_reg(const struct device *const dev, uint8_t reg_addr, uint8_t *value) -{ - const struct emul_imager_config *cfg = dev->config; - - return i2c_write_read_dt(&cfg->i2c, ®_addr, 1, value, 1); -} - -/* Helper to read a full integer directly from a register */ -static int emul_imager_read_int(const struct device *const dev, uint8_t reg_addr, int *value) -{ - uint8_t val8 = 0; - int ret; - - ret = emul_imager_read_reg(dev, reg_addr, &val8); - *value = val8; - return ret; -} - -/* Some sensors will need reg8 or reg16 variants. */ -static int emul_imager_write_reg(const struct device *const dev, uint8_t reg_addr, uint8_t value) +static int emul_imager_set_ctrl(const struct device *dev, unsigned int cid, void *value) { const struct emul_imager_config *cfg = dev->config; - uint8_t buf_w[] = {reg_addr, value}; - - return i2c_write_dt(&cfg->i2c, buf_w, 2); -} -static int emul_imager_write_multi(const struct device *const dev, - const struct emul_imager_reg *regs) -{ - int ret; - - for (int i = 0; regs[i].addr != 0; i++) { - ret = emul_imager_write_reg(dev, regs[i].addr, regs[i].value); - if (ret < 0) { - return ret; - } - } - return 0; -} - -static int emul_imager_set_ctrl(const struct device *dev, unsigned int cid, void *value) -{ switch (cid) { case EMUL_IMAGER_CID_CUSTOM: - return emul_imager_write_reg(dev, EMUL_IMAGER_REG_CUSTOM, (int)value); + return video_write_cci_reg(&cfg->i2c, EMUL_IMAGER_REG_CUSTOM, (int)value); default: return -ENOTSUP; } @@ -201,17 +160,26 @@ static int emul_imager_set_ctrl(const struct device *dev, unsigned int cid, void static int emul_imager_get_ctrl(const struct device *dev, unsigned int cid, void *value) { + const struct emul_imager_config *cfg = dev->config; + uint32_t reg = 0; + int ret; + switch (cid) { case EMUL_IMAGER_CID_CUSTOM: - return emul_imager_read_int(dev, EMUL_IMAGER_REG_CUSTOM, value); + ret = video_read_cci_reg(&cfg->i2c, EMUL_IMAGER_REG_CUSTOM, ®); + break; default: return -ENOTSUP; } + + *(uint32_t *)value = reg; + return ret; } /* Customize this function according to your "struct emul_imager_mode". */ static int emul_imager_set_mode(const struct device *dev, const struct emul_imager_mode *mode) { + const struct emul_imager_config *cfg = dev->config; struct emul_imager_data *data = dev->data; int ret; @@ -223,7 +191,7 @@ static int emul_imager_set_mode(const struct device *dev, const struct emul_imag /* Apply all the configuration registers for that mode */ for (int i = 0; i < 2; i++) { - ret = emul_imager_write_multi(dev, mode->regs[i]); + ret = video_write_cci_multi(&cfg->i2c, mode->regs[i]); if (ret < 0) { goto err; } @@ -359,7 +327,9 @@ static int emul_imager_get_caps(const struct device *dev, enum video_endpoint_id static int emul_imager_set_stream(const struct device *dev, bool enable) { - return emul_imager_write_reg(dev, EMUL_IMAGER_REG_CTRL, enable ? 1 : 0); + const struct emul_imager_config *cfg = dev->config; + + return video_write_cci_reg(&cfg->i2c, EMUL_IMAGER_REG_CTRL, enable ? 1 : 0); } static DEVICE_API(video, emul_imager_driver_api) = { @@ -378,7 +348,7 @@ int emul_imager_init(const struct device *dev) { const struct emul_imager_config *cfg = dev->config; struct video_format fmt; - uint8_t sensor_id; + uint32_t sensor_id; int ret; if (!i2c_is_ready_dt(&cfg->i2c)) { @@ -386,13 +356,13 @@ int emul_imager_init(const struct device *dev) return -ENODEV; } - ret = emul_imager_read_reg(dev, EMUL_IMAGER_REG_SENSOR_ID, &sensor_id); + ret = video_read_cci_reg(&cfg->i2c, EMUL_IMAGER_REG_SENSOR_ID, &sensor_id); if (ret < 0 || sensor_id != EMUL_IMAGER_SENSOR_ID) { LOG_ERR("Failed to get a correct sensor ID 0x%x", sensor_id); return ret; } - ret = emul_imager_write_multi(dev, emul_imager_init_regs); + ret = video_write_cci_multi(&cfg->i2c, emul_imager_init_regs); if (ret < 0) { LOG_ERR("Could not set initial registers"); return ret; @@ -431,7 +401,7 @@ static int emul_imager_transfer_i2c(const struct emul *target, struct i2c_msg ms int addr) { static uint8_t fake_regs[UINT8_MAX] = { - [EMUL_IMAGER_REG_SENSOR_ID] = EMUL_IMAGER_SENSOR_ID, + [EMUL_IMAGER_REG_SENSOR_ID & VIDEO_REG_ADDR_MASK] = EMUL_IMAGER_SENSOR_ID, }; if (num_msgs == 0) { From fd3677d26f4dae15c66a80b4ef14de617ac4471f Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Tue, 1 Apr 2025 17:49:34 +0000 Subject: [PATCH 5/7] drivers: video: common: fix video_closest_frmival() fie.index Fix bug introduced in 46a262ffe62d5e29bf54453de34104779d3e7385 where the fie.index field was expected to be incremented by the driver, while it is the responsibility of the caller to increment it. Signed-off-by: Josuah Demangeon --- drivers/video/video_common.c | 8 ++------ tests/drivers/video/api/prj.conf | 1 + tests/drivers/video/api/src/video_emul.c | 19 +++++++++++++------ 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/drivers/video/video_common.c b/drivers/video/video_common.c index 52e8fb2b836c..1343187b2526 100644 --- a/drivers/video/video_common.c +++ b/drivers/video/video_common.c @@ -144,7 +144,7 @@ void video_closest_frmival(const struct device *dev, enum video_endpoint_id ep, __ASSERT(match->type != VIDEO_FRMIVAL_TYPE_STEPWISE, "cannot find range matching the range, only a value matching the range"); - while (video_enum_frmival(dev, ep, &fie) == 0) { + for (fie.index = 0; video_enum_frmival(dev, ep, &fie) == 0; fie.index++) { struct video_frmival tmp = {0}; uint64_t diff_nsec = 0, a, b; @@ -164,12 +164,8 @@ void video_closest_frmival(const struct device *dev, enum video_endpoint_id ep, diff_nsec = a > b ? a - b : b - a; if (diff_nsec < best_diff_nsec) { best_diff_nsec = diff_nsec; + match->index = fie.index; memcpy(&match->discrete, &tmp, sizeof(tmp)); - - /* The video_enum_frmival() function will increment fie.index every time. - * Compensate for it to get the current index, not the next index. - */ - match->index = fie.index - 1; } } } diff --git a/tests/drivers/video/api/prj.conf b/tests/drivers/video/api/prj.conf index f35903ca73d7..0175ad9c2e76 100644 --- a/tests/drivers/video/api/prj.conf +++ b/tests/drivers/video/api/prj.conf @@ -1,4 +1,5 @@ CONFIG_ZTEST=y +CONFIG_ASSERT=y CONFIG_VIDEO=y # Just enough for a single frame in RGB565 format: 320 * 420 * 2 + some margin diff --git a/tests/drivers/video/api/src/video_emul.c b/tests/drivers/video/api/src/video_emul.c index 141627dd414c..7c7d2bf48142 100644 --- a/tests/drivers/video/api/src/video_emul.c +++ b/tests/drivers/video/api/src/video_emul.c @@ -83,10 +83,10 @@ ZTEST(video_common, test_video_frmival) /* Do a first enumeration of frame intervals, expected to work */ zexpect_ok(video_enum_frmival(imager_dev, VIDEO_EP_OUT, &fie)); - zexpect_equal(fie.index, 1, "fie's index should increment by one at every iteration"); + zexpect_equal(fie.index, 0, "fie's index should not increment on its own"); /* Test that every value of the frame interval enumerator can be applied */ - do { + for (fie.index = 0; video_enum_frmival(imager_dev, VIDEO_EP_OUT, &fie) == 0; fie.index++) { struct video_frmival q, a; uint32_t min, max, step; @@ -111,18 +111,25 @@ ZTEST(video_common, test_video_frmival) for (q.numerator = min; q.numerator <= max; q.numerator += step) { zexpect_ok(video_set_frmival(imager_dev, VIDEO_EP_OUT, &q)); zexpect_ok(video_get_frmival(imager_dev, VIDEO_EP_OUT, &a)); - zexpect_equal(video_frmival_nsec(&q), video_frmival_nsec(&a)); + zexpect_equal(video_frmival_nsec(&q), video_frmival_nsec(&a), + "query %u/%u (%u nsec) answer %u/%u (%u nsec, sw)", + q.numerator, q.denominator, video_frmival_nsec(&q), + a.numerator, a.denominator, video_frmival_nsec(&a)); } break; case VIDEO_FRMIVAL_TYPE_DISCRETE: /* There is just one frame interval to test */ - zexpect_ok(video_set_frmival(imager_dev, VIDEO_EP_OUT, &fie.discrete)); + memcpy(&q, &fie.discrete, sizeof(q)); + zexpect_ok(video_set_frmival(imager_dev, VIDEO_EP_OUT, &q)); zexpect_ok(video_get_frmival(imager_dev, VIDEO_EP_OUT, &a)); - zexpect_equal(video_frmival_nsec(&fie.discrete), video_frmival_nsec(&a)); + zexpect_equal(video_frmival_nsec(&fie.discrete), video_frmival_nsec(&a), + "query %u/%u (%u nsec) answer %u/%u (%u nsec, discrete)", + q.numerator, q.denominator, video_frmival_nsec(&q), + a.numerator, a.denominator, video_frmival_nsec(&a)); break; } - } while (video_enum_frmival(imager_dev, VIDEO_EP_OUT, &fie) == 0); + } } ZTEST(video_common, test_video_ctrl) From 541f190152fecd71935e87341315e3a604cf5038 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Tue, 1 Apr 2025 17:52:38 +0000 Subject: [PATCH 6/7] drivers: video: common: add shared implementation for imagers Imagers, also known as image sensors, have drivers that look very similar. Add common implementation that fits most image sensors, which can be bypassed wherever this does not make the implementation simpler. Signed-off-by: Josuah Demangeon --- drivers/video/video_common.c | 193 +++++++++++++++++++++++++++++++++++ drivers/video/video_common.h | 122 ++++++++++++++++++++++ 2 files changed, 315 insertions(+) diff --git a/drivers/video/video_common.c b/drivers/video/video_common.c index 1343187b2526..5ce40a250684 100644 --- a/drivers/video/video_common.c +++ b/drivers/video/video_common.c @@ -301,3 +301,196 @@ int video_write_cci_multi(const struct i2c_dt_spec *i2c, const struct video_reg return 0; } + +/* Common implementation for imagers (a.k.a. image sensor) drivers */ + +int video_imager_set_mode(const struct device *dev, const struct video_imager_mode *mode) +{ + const struct video_imager_config *cfg = dev->config; + struct video_imager_data *data = dev->data; + int ret; + + if (data->mode == mode) { + LOG_DBG("%s is arlready in the mode requested", dev->name); + return 0; + } + + /* Write each register table to the device */ + for (int i = 0; i < ARRAY_SIZE(mode->regs) && mode->regs[i] != NULL; i++) { + ret = cfg->write_multi(&cfg->i2c, mode->regs[i]); + if (ret != 0) { + LOG_ERR("Could not set %s to mode %p, %u FPS", dev->name, mode, mode->fps); + return ret; + } + } + + data->mode = mode; + + return 0; +} + +int video_imager_set_frmival(const struct device *dev, enum video_endpoint_id ep, + struct video_frmival *frmival) +{ + const struct video_imager_config *cfg = dev->config; + struct video_imager_data *data = dev->data; + struct video_frmival_enum fie = {.format = &data->fmt, .discrete = *frmival}; + + if (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL) { + return -EINVAL; + } + + video_closest_frmival(dev, ep, &fie); + + return video_imager_set_mode(dev, &cfg->modes[data->fmt_id][fie.index]); +} + +int video_imager_get_frmival(const struct device *dev, enum video_endpoint_id ep, + struct video_frmival *frmival) +{ + struct video_imager_data *data = dev->data; + + if (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL) { + return -EINVAL; + } + + frmival->numerator = 1; + frmival->denominator = data->mode->fps; + + return 0; +} + +int video_imager_enum_frmival(const struct device *dev, enum video_endpoint_id ep, + struct video_frmival_enum *fie) +{ + const struct video_imager_config *cfg = dev->config; + const struct video_imager_mode *modes; + size_t fmt_id = 0; + int ret; + + if (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL) { + return -EINVAL; + } + + ret = video_format_caps_index(cfg->fmts, fie->format, &fmt_id); + if (ret != 0) { + LOG_ERR("Format '%s' %ux%u not found for %s", + VIDEO_FOURCC_TO_STR(fie->format->pixelformat), + fie->format->width, fie->format->height, dev->name); + return ret; + } + + modes = cfg->modes[fmt_id]; + + for (int i = 0;; i++) { + if (modes[i].fps == 0) { + return -EINVAL; + } + + if (i == fie->index) { + fie->type = VIDEO_FRMIVAL_TYPE_DISCRETE; + fie->discrete.numerator = 1; + fie->discrete.denominator = modes[i].fps; + break; + } + } + + return 0; +} + +int video_imager_set_fmt(const struct device *const dev, enum video_endpoint_id ep, + struct video_format *fmt) +{ + const struct video_imager_config *cfg = dev->config; + struct video_imager_data *data = dev->data; + size_t fmt_id; + int ret; + + if (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL) { + LOG_ERR("Only the output endpoint is supported for %s", dev->name); + return -EINVAL; + } + + ret = video_format_caps_index(cfg->fmts, fmt, &fmt_id); + if (ret != 0) { + LOG_ERR("Format '%s' %ux%u not found for device %s", + VIDEO_FOURCC_TO_STR(fmt->pixelformat), fmt->width, fmt->height, dev->name); + return ret; + } + + ret = video_imager_set_mode(dev, &cfg->modes[fmt_id][0]); + if (ret != 0) { + return ret; + } + + data->fmt_id = fmt_id; + data->fmt = *fmt; + + return 0; +} + +int video_imager_get_fmt(const struct device *dev, enum video_endpoint_id ep, + struct video_format *fmt) +{ + struct video_imager_data *data = dev->data; + + if (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL) { + return -EINVAL; + } + + *fmt = data->fmt; + + return 0; +} + +int video_imager_get_caps(const struct device *dev, enum video_endpoint_id ep, + struct video_caps *caps) +{ + const struct video_imager_config *cfg = dev->config; + + if (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL) { + return -EINVAL; + } + + caps->format_caps = cfg->fmts; + + return 0; +} + +int video_imager_init(const struct device *dev, const struct video_reg *init_regs, + int default_fmt_idx) +{ + const struct video_imager_config *cfg = dev->config; + struct video_format fmt; + int ret; + + __ASSERT_NO_MSG(cfg->modes != NULL); + __ASSERT_NO_MSG(cfg->fmts != NULL); + + if (!device_is_ready(cfg->i2c.bus)) { + LOG_ERR("I2C bus device %s is not ready", cfg->i2c.bus->name); + return -ENODEV; + } + + if (init_regs != NULL) { + ret = cfg->write_multi(&cfg->i2c, init_regs); + if (ret != 0) { + LOG_ERR("Could not set %s initial registers", dev->name); + return ret; + } + } + + fmt.pixelformat = cfg->fmts[default_fmt_idx].pixelformat; + fmt.width = cfg->fmts[default_fmt_idx].width_max; + fmt.height = cfg->fmts[default_fmt_idx].height_max; + fmt.pitch = fmt.width * video_bits_per_pixel(fmt.pixelformat) / BITS_PER_BYTE; + + ret = video_set_format(dev, VIDEO_EP_OUT, &fmt); + if (ret != 0) { + LOG_ERR("Failed to set %s to default format '%s' %ux%u", + dev->name, VIDEO_FOURCC_TO_STR(fmt.pixelformat), fmt.width, fmt.height); + return ret; + } + + return 0; +} diff --git a/drivers/video/video_common.h b/drivers/video/video_common.h index 16d59fd4e4ec..017f75f8a97a 100644 --- a/drivers/video/video_common.h +++ b/drivers/video/video_common.h @@ -135,4 +135,126 @@ int video_read_cci_reg(const struct i2c_dt_spec *i2c, uint32_t reg_addr, uint32_ */ int video_write_cci_multi(const struct i2c_dt_spec *i2c, const struct video_reg *regs); +/** @} */ + +/** + * @defgroup video_imager Video Imager (image sensor) shared implementation + * + * This API is targeting image sensor driver developers. + * + * It provides a common implementation that only requires implementing a table of + * @ref video_imager_mode that lists the various resolution, frame rates and associated I2C + * configuration registers. + * + * The @c video_imager_... functions can be submitted directly to the @rev video_api. + * If a driver also needs to do extra work before or after applying a mode, it is possible + * to provide a custom wrapper or skip these default implementation altogether. + * + * @{ + */ + +/** + * @brief Table entry for an imaging mode of the sensor device. + * + * AA mode can be applied to an imager to configure a particular framerate, resolution, and pixel + * format. The index of the table of modes is meant to match the index of the table of formats. + */ +struct video_imager_mode { + /* FPS for this mode */ + uint16_t fps; + /* Multiple lists of registers to allow sharing common sets of registers across modes. */ + const struct video_reg *regs[3]; +}; + +/** + * @brief A video imager device is expected to have dev->data point to this structure. + * + * In order to support custom data structure, it is possible to store an extra pointer + * in the dev->config struct. See existing drivers for an example. + */ +struct video_imager_config { + /** List of all formats supported by this sensor */ + const struct video_format_cap *fmts; + /** Array of modes tables, one table per format cap lislted by "fmts" */ + const struct video_imager_mode **modes; + /** I2C device to write the registers to */ + struct i2c_dt_spec i2c; + /** Write a table of registers onto the device */ + int (*write_multi)(const struct i2c_dt_spec *i2c, const struct video_reg *regs); + /** Reference to a ; */ + struct video_imager_data *data; +}; + +/** + * @brief A video imager device is expected to have dev->data point to this structure. + * + * In order to support custom data structure, it is possible to store an extra pointer + * in the dev->config struct. See existing drivers for an example. + */ +struct video_imager_data { + /** Index of the currently active format in both modes[] and fmts[] */ + int fmt_id; + /** Currently active video format */ + struct video_format fmt; + /** Currently active operating mode as defined above */ + const struct video_imager_mode *mode; +}; + +/** + * @brief Initialize one row of a @struct video_format_cap with fixed width and height. + * + * The minimum and maximum are the same for both width and height fields. + * @param + */ +#define VIDEO_IMAGER_FORMAT_CAP(pixfmt, width, height) \ + { \ + .width_min = (width), .width_max = (width), .width_step = 0, \ + .height_min = (height), .height_max = (height), .height_step = 0, \ + .pixelformat = (pixfmt), \ + } + +/** + * @brief Set the operating mode of the imager as defined in @ref video_imager_mode. + * + * If the default immplementation for the video API are used, there is no need to explicitly call + * this function in the image sensor driver. + * + * @param dev Device that has a struct video_imager in @c dev->data. + * @param mode The mode to apply to the image sensor. + * @return 0 if successful, or negative error number otherwise. + */ +int video_imager_set_mode(const struct device *dev, const struct video_imager_mode *mode); + +/** @brief Default implementation for image drivers frame interval selection */ +int video_imager_set_frmival(const struct device *dev, enum video_endpoint_id ep, + struct video_frmival *frmival); +/** @brief Default implementation for image drivers frame interval query */ +int video_imager_get_frmival(const struct device *dev, enum video_endpoint_id ep, + struct video_frmival *frmival); +/** @brief Default implementation for image drivers frame interval enumeration */ +int video_imager_enum_frmival(const struct device *dev, enum video_endpoint_id ep, + struct video_frmival_enum *fie); +/** @brief Default implementation for image drivers format selection */ +int video_imager_set_fmt(const struct device *const dev, enum video_endpoint_id ep, + struct video_format *fmt); +/** @brief Default implementation for image drivers format query */ +int video_imager_get_fmt(const struct device *dev, enum video_endpoint_id ep, + struct video_format *fmt); +/** @brief Default implementation for image drivers format capabilities */ +int video_imager_get_caps(const struct device *dev, enum video_endpoint_id ep, + struct video_caps *caps); + +/** + * Initialize an imager by loading init_regs onto the device, and setting the default format. + * + * @param dev Device that has a struct video_imager in @c dev->data. + * @param init_regs If non-NULL, table of registers to configure at init. + * @param default_fmt_idx Default format index to apply at init. + * @return 0 if successful, or negative error number otherwise. + */ +int video_imager_init(const struct device *dev, const struct video_reg *init_regs, + int default_fmt_idx); + +/** @} */ + #endif /* ZEPHYR_DRIVERS_VIDEO_COMMON_H_ */ From b9ce6f405a3c4c35ffed836de3eff74365675845 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Tue, 1 Apr 2025 17:55:42 +0000 Subject: [PATCH 7/7] drivers: video: emul_imager: use the shared imager implementation Make the emulated imager use the shared implementation of all imagers, making shared implementation of the image sensor drivers be tested by CI. Signed-off-by: Josuah Demangeon --- drivers/video/video_emul_imager.c | 285 +++++++----------------------- drivers/video/video_emul_imager.h | 22 +++ drivers/video/video_emul_rx.c | 10 +- 3 files changed, 93 insertions(+), 224 deletions(-) create mode 100644 drivers/video/video_emul_imager.h diff --git a/drivers/video/video_emul_imager.c b/drivers/video/video_emul_imager.c index e352011c7340..cc9f914dcdef 100644 --- a/drivers/video/video_emul_imager.c +++ b/drivers/video/video_emul_imager.c @@ -46,27 +46,12 @@ enum emul_imager_fmt_id { YUYV_320x240, }; -struct emul_imager_mode { - uint8_t fps; - /* List of registers lists to configure the various properties of the sensor. - * This permits to deduplicate the list of registers in case some lare sections - * are repeated across modes, such as the resolution for different FPS. - */ - const struct video_reg *regs[3]; - /* More fields can be added according to the needs of the sensor driver */ -}; - -struct emul_imager_config { - struct i2c_dt_spec i2c; -}; - +/* Wrapper over the struct video_imager_data that extends it with other fields */ struct emul_imager_data { - /* First field is a line buffer for I/O emulation purpose */ - uint8_t framebuffer[320 * sizeof(uint16_t)]; - /* Other fields are shared with real hardware drivers */ - const struct emul_imager_mode *mode; - enum emul_imager_fmt_id fmt_id; - struct video_format fmt; + /* The first field is the default data used by all image sensors */ + struct video_imager_data imager; + /* A line buffer for I/O emulation purpose */ + uint8_t linebuffer[320 * sizeof(uint16_t)]; }; /* All the I2C registers sent on various scenario */ @@ -108,47 +93,50 @@ static const struct video_reg emul_imager_60fps[] = { }; /* Description of "modes", that pick lists of registesr that will be all sentto the imager */ -struct emul_imager_mode emul_imager_rgb565_320x240_modes[] = { +static struct video_imager_mode emul_imager_rgb565_320x240_modes[] = { {.fps = 15, .regs = {emul_imager_320x240, emul_imager_rgb565, emul_imager_15fps}}, {.fps = 30, .regs = {emul_imager_320x240, emul_imager_rgb565, emul_imager_30fps}}, {.fps = 60, .regs = {emul_imager_320x240, emul_imager_rgb565, emul_imager_60fps}}, {0}, }; -struct emul_imager_mode emul_imager_yuyv_320x240_modes[] = { +static struct video_imager_mode emul_imager_yuyv_320x240_modes[] = { {.fps = 15, .regs = {emul_imager_320x240, emul_imager_yuyv, emul_imager_15fps}}, {.fps = 30, .regs = {emul_imager_320x240, emul_imager_yuyv, emul_imager_30fps}}, {0}, }; -/* Summary of all the modes of all the frame formats, with indexes matching those of fmts[]. */ -static const struct emul_imager_mode *emul_imager_modes[] = { +static const struct video_imager_mode *emul_imager_modes[] = { [RGB565_320x240] = emul_imager_rgb565_320x240_modes, [YUYV_320x240] = emul_imager_yuyv_320x240_modes, }; - -/* Video device capabilities where the supported resolutions and pixel formats are listed. - * The format ID is used as index to fetch the matching mode from the list above. - */ -#define EMUL_IMAGER_VIDEO_FORMAT_CAP(format, width, height) \ - { \ - /* For a real imager, the width and height would be macro parameters */ \ - .pixelformat = (format), \ - .width_min = (width), \ - .width_max = (width), \ - .width_step = 0, \ - .height_min = (height), \ - .height_max = (height), \ - .height_step = 0, \ - } -static const struct video_format_cap fmts[] = { - [RGB565_320x240] = EMUL_IMAGER_VIDEO_FORMAT_CAP(VIDEO_PIX_FMT_RGB565, 320, 240), - [YUYV_320x240] = EMUL_IMAGER_VIDEO_FORMAT_CAP(VIDEO_PIX_FMT_YUYV, 320, 240), +static const struct video_format_cap emul_imager_fmts[] = { + [RGB565_320x240] = VIDEO_IMAGER_FORMAT_CAP(VIDEO_PIX_FMT_RGB565, 320, 240), + [YUYV_320x240] = VIDEO_IMAGER_FORMAT_CAP(VIDEO_PIX_FMT_YUYV, 320, 240), {0}, }; +static int emul_imager_set_fmt(const struct device *const dev, enum video_endpoint_id ep, + struct video_format *fmt) +{ + struct emul_imager_data *data = dev->data; + + /* For the purpose of simulation, fill the image line buffer with 50% gray, this data + * will be collected by the video_emul_rx driver. + */ + if (fmt->pixelformat == VIDEO_PIX_FMT_RGB565) { + for (int i = 0; i < sizeof(data->linebuffer) / sizeof(uint16_t); i++) { + ((uint16_t *)data->linebuffer)[i] = sys_cpu_to_le16(0x7bef); + } + } else { + memset(data->linebuffer, 0x7f, sizeof(data->linebuffer)); + } + + return video_imager_set_fmt(dev, ep, fmt); +} + static int emul_imager_set_ctrl(const struct device *dev, unsigned int cid, void *value) { - const struct emul_imager_config *cfg = dev->config; + const struct video_imager_config *cfg = dev->config; switch (cid) { case EMUL_IMAGER_CID_CUSTOM: @@ -160,7 +148,7 @@ static int emul_imager_set_ctrl(const struct device *dev, unsigned int cid, void static int emul_imager_get_ctrl(const struct device *dev, unsigned int cid, void *value) { - const struct emul_imager_config *cfg = dev->config; + const struct video_imager_config *cfg = dev->config; uint32_t reg = 0; int ret; @@ -176,178 +164,30 @@ static int emul_imager_get_ctrl(const struct device *dev, unsigned int cid, void return ret; } -/* Customize this function according to your "struct emul_imager_mode". */ -static int emul_imager_set_mode(const struct device *dev, const struct emul_imager_mode *mode) -{ - const struct emul_imager_config *cfg = dev->config; - struct emul_imager_data *data = dev->data; - int ret; - - if (data->mode == mode) { - return 0; - } - - LOG_DBG("Applying mode %p at %d FPS", mode, mode->fps); - - /* Apply all the configuration registers for that mode */ - for (int i = 0; i < 2; i++) { - ret = video_write_cci_multi(&cfg->i2c, mode->regs[i]); - if (ret < 0) { - goto err; - } - } - - data->mode = mode; - return 0; -err: - LOG_ERR("Could not apply mode %p (%u FPS)", mode, mode->fps); - return ret; -} - -static int emul_imager_set_frmival(const struct device *dev, enum video_endpoint_id ep, - struct video_frmival *frmival) -{ - struct emul_imager_data *data = dev->data; - struct video_frmival_enum fie = {.format = &data->fmt, .discrete = *frmival}; - - if (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL) { - return -EINVAL; - } - - video_closest_frmival(dev, ep, &fie); - LOG_DBG("Applying frame interval number %u", fie.index); - return emul_imager_set_mode(dev, &emul_imager_modes[data->fmt_id][fie.index]); -} - -static int emul_imager_get_frmival(const struct device *dev, enum video_endpoint_id ep, - struct video_frmival *frmival) -{ - struct emul_imager_data *data = dev->data; - - if (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL) { - return -EINVAL; - } - - frmival->numerator = 1; - frmival->denominator = data->mode->fps; - return 0; -} - -static int emul_imager_enum_frmival(const struct device *dev, enum video_endpoint_id ep, - struct video_frmival_enum *fie) -{ - const struct emul_imager_mode *mode; - size_t fmt_id; - int ret; - - if (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL) { - return -EINVAL; - } - - ret = video_format_caps_index(fmts, fie->format, &fmt_id); - if (ret < 0) { - return ret; - } - - mode = &emul_imager_modes[fmt_id][fie->index]; - - fie->type = VIDEO_FRMIVAL_TYPE_DISCRETE; - fie->discrete.numerator = 1; - fie->discrete.denominator = mode->fps; - fie->index++; - - return mode->fps == 0; -} - -static int emul_imager_set_fmt(const struct device *const dev, enum video_endpoint_id ep, - struct video_format *fmt) -{ - struct emul_imager_data *data = dev->data; - size_t fmt_id; - int ret; - - if (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL) { - return -EINVAL; - } - - if (memcmp(&data->fmt, fmt, sizeof(data->fmt)) == 0) { - return 0; - } - - ret = video_format_caps_index(fmts, fmt, &fmt_id); - if (ret < 0) { - LOG_ERR("Format %x %ux%u not found", fmt->pixelformat, fmt->width, fmt->height); - return ret; - } - - ret = emul_imager_set_mode(dev, &emul_imager_modes[fmt_id][0]); - if (ret < 0) { - return ret; - } - - /* For the purpose of simulation, fill the image line buffer with 50% gray, this data - * will be collected by the video_emul_rx driver. - */ - if (fmt->pixelformat == VIDEO_PIX_FMT_RGB565) { - for (int i = 0; i < fmt->width; i++) { - ((uint16_t *)data->framebuffer)[i] = sys_cpu_to_le16(0x7bef); - } - } else { - memset(data->framebuffer, 0x7f, fmt->pitch); - } - - data->fmt_id = fmt_id; - data->fmt = *fmt; - return 0; -} - -static int emul_imager_get_fmt(const struct device *dev, enum video_endpoint_id ep, - struct video_format *fmt) -{ - struct emul_imager_data *data = dev->data; - - if (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL) { - return -EINVAL; - } - - *fmt = data->fmt; - return 0; -} - -static int emul_imager_get_caps(const struct device *dev, enum video_endpoint_id ep, - struct video_caps *caps) -{ - if (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL) { - return -EINVAL; - } - - caps->format_caps = fmts; - return 0; -} - static int emul_imager_set_stream(const struct device *dev, bool enable) { - const struct emul_imager_config *cfg = dev->config; + const struct video_imager_config *cfg = dev->config; return video_write_cci_reg(&cfg->i2c, EMUL_IMAGER_REG_CTRL, enable ? 1 : 0); } static DEVICE_API(video, emul_imager_driver_api) = { + /* Local implementation */ .set_ctrl = emul_imager_set_ctrl, .get_ctrl = emul_imager_get_ctrl, - .set_frmival = emul_imager_set_frmival, - .get_frmival = emul_imager_get_frmival, - .enum_frmival = emul_imager_enum_frmival, - .set_format = emul_imager_set_fmt, - .get_format = emul_imager_get_fmt, - .get_caps = emul_imager_get_caps, .set_stream = emul_imager_set_stream, + .set_format = emul_imager_set_fmt, + /* Default implementation */ + .set_frmival = video_imager_set_frmival, + .get_frmival = video_imager_get_frmival, + .enum_frmival = video_imager_enum_frmival, + .get_format = video_imager_get_fmt, + .get_caps = video_imager_get_caps, }; int emul_imager_init(const struct device *dev) { - const struct emul_imager_config *cfg = dev->config; - struct video_format fmt; + const struct video_imager_config *cfg = dev->config; uint32_t sensor_id; int ret; @@ -358,43 +198,42 @@ int emul_imager_init(const struct device *dev) ret = video_read_cci_reg(&cfg->i2c, EMUL_IMAGER_REG_SENSOR_ID, &sensor_id); if (ret < 0 || sensor_id != EMUL_IMAGER_SENSOR_ID) { - LOG_ERR("Failed to get a correct sensor ID 0x%x", sensor_id); - return ret; - } - - ret = video_write_cci_multi(&cfg->i2c, emul_imager_init_regs); - if (ret < 0) { - LOG_ERR("Could not set initial registers"); + LOG_ERR("Failed to get a correct sensor ID 0x%x", sensor_id); return ret; } - fmt.pixelformat = fmts[0].pixelformat; - fmt.width = fmts[0].width_min; - fmt.height = fmts[0].height_min; - fmt.pitch = fmt.width * 2; - - ret = emul_imager_set_fmt(dev, VIDEO_EP_OUT, &fmt); - if (ret < 0) { - LOG_ERR("Failed to set to default format %x %ux%u", - fmt.pixelformat, fmt.width, fmt.height); - } - - return 0; + return video_imager_init(dev, emul_imager_init_regs, 0); } #define EMUL_IMAGER_DEFINE(inst) \ static struct emul_imager_data emul_imager_data_##inst; \ \ - static const struct emul_imager_config emul_imager_cfg_##inst = { \ + static struct video_imager_data video_imager_data_##inst; \ + \ + static const struct video_imager_config video_imager_cfg_##inst = { \ .i2c = I2C_DT_SPEC_INST_GET(inst), \ + .modes = emul_imager_modes, \ + .fmts = emul_imager_fmts, \ + .data = &video_imager_data_##inst, \ + .write_multi = video_write_cci_multi, \ }; \ \ DEVICE_DT_INST_DEFINE(inst, &emul_imager_init, NULL, &emul_imager_data_##inst, \ - &emul_imager_cfg_##inst, POST_KERNEL, CONFIG_VIDEO_INIT_PRIORITY, \ + &video_imager_cfg_##inst, POST_KERNEL, CONFIG_VIDEO_INIT_PRIORITY, \ &emul_imager_driver_api); DT_INST_FOREACH_STATUS_OKAY(EMUL_IMAGER_DEFINE) +/* Simulated MIPI bus */ + +const uint8_t *video_emul_imager_get_linebuffer(const struct device *dev, size_t *pitch) +{ + struct emul_imager_data *data = dev->data; + + *pitch = sizeof(data->linebuffer); + return data->linebuffer; +} + /* Simulated I2C bus */ static int emul_imager_transfer_i2c(const struct emul *target, struct i2c_msg msgs[], int num_msgs, diff --git a/drivers/video/video_emul_imager.h b/drivers/video/video_emul_imager.h new file mode 100644 index 000000000000..8649018f18e7 --- /dev/null +++ b/drivers/video/video_emul_imager.h @@ -0,0 +1,22 @@ +/* + * Copyright (c) 2025 tinyVision.ai Inc. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef ZEPHYR_DRIVERS_VIDEO_EMUL_IMAGER_H_ +#define ZEPHYR_DRIVERS_VIDEO_EMUL_IMAGER_H_ + +/** + * @brief Acces the 1-line buffer written to by the image sensor driver + * + * This 1-line buffer simulates the MIPI link going between an image sensor and a video receiver + * peripheral. + * + * @param dev Image sensor device to query the buffer from. + * @param pitch Pointer filled with the size of this buffer in bytes. + * @retval The 1-line buffer from this device. + */ +const uint8_t *video_emul_imager_get_linebuffer(const struct device *dev, size_t *pitch); + +#endif /* ZEPHYR_DRIVERS_VIDEO_EMUL_IMAGER_H_ */ diff --git a/drivers/video/video_emul_rx.c b/drivers/video/video_emul_rx.c index 2abb6fef32aa..eeea4f71fe95 100644 --- a/drivers/video/video_emul_rx.c +++ b/drivers/video/video_emul_rx.c @@ -15,6 +15,8 @@ #include #include +#include "video_emul_imager.h" + LOG_MODULE_REGISTER(video_emul_rx, CONFIG_VIDEO_LOG_LEVEL); struct emul_rx_config { @@ -146,10 +148,16 @@ static void emul_rx_worker(struct k_work *work) const struct emul_rx_config *cfg = dev->config; struct video_format *fmt = &data->fmt; struct video_buffer *vbuf = vbuf; + size_t pitch; + const uint8_t *linebuffer; LOG_DBG("Queueing a frame of %u bytes in format %x %ux%u", fmt->pitch * fmt->height, fmt->pixelformat, fmt->width, fmt->height); + linebuffer = video_emul_imager_get_linebuffer(cfg->source_dev, &pitch); + __ASSERT_NO_MSG(pitch == fmt->pitch); + __ASSERT_NO_MSG(linebuffer != NULL); + while ((vbuf = k_fifo_get(&data->fifo_in, K_NO_WAIT)) != NULL) { vbuf->bytesused = fmt->pitch * fmt->height; vbuf->line_offset = 0; @@ -161,7 +169,7 @@ static void emul_rx_worker(struct k_work *work) * the whole frame. vbuf->size is already checked in emul_rx_enqueue(). */ for (size_t i = 0; i + fmt->pitch <= vbuf->bytesused; i += fmt->pitch) { - memcpy(vbuf->buffer + i, cfg->source_dev->data, fmt->pitch); + memcpy(vbuf->buffer + i, linebuffer, fmt->pitch); } /* Once the buffer is completed, submit it to the video buffer */