From 71d8a7f926cd541b68374839aef3e722b1586edc Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Mon, 31 Mar 2025 20:02:56 +0000 Subject: [PATCH 1/4] 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 | 203 +++++++++++++++++++++++++++++- drivers/video/video_common.h | 233 +++++++++++++++++++++++++++++++++++ 3 files changed, 442 insertions(+), 2 deletions(-) create mode 100644 drivers/video/video_common.h diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index a3b163443628..a111d0e2cc52 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 retries after a failed I2C communication" + default 0 + help + If set to 0, only a single write attempt will be done with no retry. + The default is to not retry. Board configuration files or user project can then + use the number of retries that matches their situation. + 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 7ae75932ccdf..9c92ed09b028 100644 --- a/drivers/video/video_common.c +++ b/drivers/video/video_common.c @@ -1,14 +1,23 @@ /* * Copyright (c) 2019, Linaro Limited - * Copyright (c) 2024, tinyVision.ai Inc. + * Copyright (c) 2024-2025, tinyVision.ai Inc. * * SPDX-License-Identifier: Apache-2.0 */ #include -#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 @@ -166,3 +175,193 @@ void video_closest_frmival(const struct device *dev, struct video_frmival_enum * } } } + +static int video_read_reg_retry(const struct i2c_dt_spec *i2c, uint8_t *buf_w, size_t size_w, + uint8_t *buf_r, size_t size_r) +{ + int ret; + + for (int i = 0;; i++) { + ret = i2c_write_read_dt(i2c, buf_w, size_w, buf_r, size_r); + if (ret == 0) { + break; + } + if (i == CONFIG_VIDEO_I2C_RETRY_NUM) { + LOG_HEXDUMP_ERR(buf_w, size_w, "failed to write-read to I2C register"); + return ret; + } + + k_sleep(K_MSEC(1)); + } + + return 0; +} + +int video_read_cci_reg(const struct i2c_dt_spec *i2c, uint32_t reg_addr, uint32_t *reg_data) +{ + size_t addr_size = FIELD_GET(VIDEO_REG_ADDR_SIZE_MASK, reg_addr); + size_t data_size = FIELD_GET(VIDEO_REG_DATA_SIZE_MASK, reg_addr); + bool big_endian = FIELD_GET(VIDEO_REG_ENDIANNESS_MASK, reg_addr); + uint16_t addr = FIELD_GET(VIDEO_REG_ADDR_MASK, reg_addr); + uint8_t buf_w[sizeof(uint16_t)] = {0}; + uint8_t *data_ptr; + int ret; + + __ASSERT(addr_size > 0, "The address must have a address size flag"); + __ASSERT(data_size > 0, "The address must have a data size flag"); + + *reg_data = 0; + + if (big_endian) { + /* Casting between data sizes in big-endian requires re-aligning */ + data_ptr = (uint8_t *)reg_data + sizeof(*reg_data) - data_size; + } else { + /* Casting between data sizes in little-endian is a no-op */ + data_ptr = (uint8_t *)reg_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 = video_read_reg_retry(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"); + } + + *reg_data = big_endian ? sys_be32_to_cpu(*reg_data) : sys_le32_to_cpu(*reg_data); + + return 0; +} + +static int video_write_reg_retry(const struct i2c_dt_spec *i2c, uint8_t *buf_w, size_t size) +{ + int ret; + + for (int i = 0;; i++) { + ret = i2c_write_dt(i2c, buf_w, size); + if (ret == 0) { + break; + } + if (i == CONFIG_VIDEO_I2C_RETRY_NUM) { + LOG_HEXDUMP_ERR(buf_w, size, "failed to write to I2C register"); + return ret; + } + + k_sleep(K_MSEC(1)); + } + + return 0; +} + +int video_write_cci_reg(const struct i2c_dt_spec *i2c, uint32_t reg_addr, uint32_t reg_data) +{ + size_t addr_size = FIELD_GET(VIDEO_REG_ADDR_SIZE_MASK, reg_addr); + size_t data_size = FIELD_GET(VIDEO_REG_DATA_SIZE_MASK, reg_addr); + bool big_endian = FIELD_GET(VIDEO_REG_ENDIANNESS_MASK, reg_addr); + uint16_t addr = FIELD_GET(VIDEO_REG_ADDR_MASK, reg_addr); + uint8_t buf_w[sizeof(uint16_t) + sizeof(uint32_t)] = {0}; + uint8_t *data_ptr; + int ret; + + __ASSERT(addr_size > 0, "The address must have a address size flag"); + __ASSERT(data_size > 0, "The address must have a data size flag"); + + if (big_endian) { + /* Casting between data sizes in big-endian requires re-aligning */ + reg_data = sys_cpu_to_be32(reg_data); + data_ptr = (uint8_t *)®_data + sizeof(reg_data) - data_size; + } else { + /* Casting between data sizes in little-endian is a no-op */ + reg_data = sys_cpu_to_le32(reg_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_modify_cci_reg(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_multiregs(const struct i2c_dt_spec *i2c, const struct video_reg *regs, + size_t num_regs) +{ + int ret; + + for (int i = 0; i < num_regs; i++) { + ret = video_write_cci_reg(i2c, regs[i].addr, regs[i].data); + if (ret < 0) { + return ret; + } + } + + return 0; +} + +int video_write_cci_multiregs8(const struct i2c_dt_spec *i2c, const struct video_reg8 *regs, + size_t num_regs) +{ + int ret; + + for (int i = 0; i < num_regs; i++) { + ret = video_write_cci_reg(i2c, regs[i].addr | VIDEO_REG_ADDR8_DATA8, regs[i].data); + if (ret < 0) { + return ret; + } + } + + return 0; +} + +int video_write_cci_multiregs16(const struct i2c_dt_spec *i2c, const struct video_reg16 *regs, + size_t num_regs) +{ + int ret; + + for (int i = 0; i < num_regs; i++) { + ret = video_write_cci_reg(i2c, regs[i].addr | VIDEO_REG_ADDR16_DATA8, regs[i].data); + if (ret < 0) { + return ret; + } + } + + return 0; +} diff --git a/drivers/video/video_common.h b/drivers/video/video_common.h new file mode 100644 index 000000000000..7744a010eab6 --- /dev/null +++ b/drivers/video/video_common.h @@ -0,0 +1,233 @@ +/* + * 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 + +/** + * @brief Register for building tables supporting 8/16 bit address and 8/16/24/32 bit value sizes. + * + * A flag in the address indicates the size of the register address, and the size and endinaness of + * the value. + * + * If willing to save space on large register tables, a more compact version @ref video_reg8 and + * @ref video_reg16 can be used, however only supporting 8-bit values . + * + * The data field is in CPU-native endianness, and the library functions will perform the + * endianness swap as needed while transmitting data. + */ +struct video_reg { + /** Address of the register, and other flags if used with the @ref video_cci API. */ + uint32_t addr; + /** Value to write to this address */ + uint32_t data; +}; + +/** Register for building tables supporting 8-bit addresses and 8-bit value sizes */ +struct video_reg8 { + /** Address of the register */ + uint8_t addr; + /** Value to write to this address */ + uint8_t data; +}; + +/** Register for building tables supporting 16-bit addresses and 8-bit value sizes */ +struct video_reg16 { + /** Address of the register */ + uint16_t addr; + /** Value to write to this address */ + uint8_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. + * 2. 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 */ + +/** + * @defgroup video_cci_reg_flags Flags describing a Video CCI register + * + * For a given drivers, register sizes are not expected to change, so macros can be used when + * defining registers: + * + * @code{.c} + * #define SENSORNAME_REG8(addr) ((uint32_t)(addr) | VIDEO_REG_ADDR16_DATA8) + * #define SENSORNAME_REG16(addr) ((uint32_t)(addr) | VIDEO_REG_ADDR16_DATA16_LE) + * #define SENSORNAME_REG24(addr) ((uint32_t)(addr) | VIDEO_REG_ADDR16_DATA24_LE) + * @endcode + * + * Then the macros can be used directly in register definitions: + * + * @code{.c} + * #define SENSORNAME_REG_EXPOSURE_LEVEL SENSORNAME_REG16(0x3060) + * #define SENSORNAME_REG_AN_GAIN_LEVEL SENSORNAME_REG8(0x3062) + * ... + * video_write_cci_reg(&cfg->i2c, SENSORNAME_REG_EXPOSURE_LEVEL, 3000); + * video_write_cci_reg(&cfg->i2c, SENSORNAME_REG_AN_GAIN_LEVEL, 220); + * @endcode + * + * Or used directly inline: + * @code{.c} + * video_write_cci_reg(&cfg->i2c, SENSORNAME_REG16(0x3060), 3000); + * video_write_cci_reg(&cfg->i2c, SENSORNAME_REG8(0x3062), 220); + * @endcode + * + * As well as in register tables: + * + * @code{.c} + * struct video_reg init_regs[] = { + * {SENSORNAME_REG_EXPOSURE_LEVEL, 2000}, + * {SENSORNAME_REG_AN_GAIN_LEVEL, 180}, + * ... + * {0}, + * }; + * @endcode + * + * @{ + */ +/** 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 @p reg_data along with size information. + * @brief reg_data 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_data); + + +/** + * @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 @p reg_data 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_modify_cci_reg(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 @p reg_data along with size information. + * @brief reg_data 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_data); + +/** + * @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_REG_ADDR16_DATA8. 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. + * @brief num_regs Number of registers entries in the table. + */ +int video_write_cci_multiregs(const struct i2c_dt_spec *i2c, const struct video_reg *regs, + size_t num_regs); + +/** + * @brief Write a complete table of registers to a device one by one. + * + * The registers address are 8-bit wide and values are 8-bit wide. + * + * @brief i2c Reference to the video device on an I2C bus. + * @brief regs Array of address/value pairs to write to the device sequentially. + * @brief num_regs Number of registers entries in the table. + */ +int video_write_cci_multiregs8(const struct i2c_dt_spec *i2c, const struct video_reg8 *regs, + size_t num_regs); + +/** + * @brief Write a complete table of registers to a device one by one. + * + * The registers address are 16-bit wide and values are 8-bit wide. + * + * @brief i2c Reference to the video device on an I2C bus. + * @brief regs Array of address/value pairs to write to the device sequentially. + * @brief num_regs Number of registers entries in the table. + */ +int video_write_cci_multiregs16(const struct i2c_dt_spec *i2c, const struct video_reg16 *regs, + size_t num_regs); + +/** @} */ + +#endif /* ZEPHYR_DRIVERS_VIDEO_COMMON_H_ */ From 11d88c14d6a64e0c2f0eac8e0fdfad34be453939 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Wed, 12 Mar 2025 16:46:07 +0100 Subject: [PATCH 2/4] 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 | 103 +++++++++++++++++++--------- tests/drivers/video/api/app.overlay | 39 +++++------ tests/drivers/video/api/prj.conf | 2 + 4 files changed, 91 insertions(+), 54 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 43b4982a89f9..e13018ad66e0 100644 --- a/drivers/video/video_emul_imager.c +++ b/drivers/video/video_emul_imager.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include "video_ctrls.h" @@ -22,16 +23,16 @@ 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 @@ -39,9 +40,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, @@ -86,10 +84,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[] = { @@ -160,23 +158,18 @@ 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); } /* 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, @@ -344,12 +337,13 @@ static int emul_imager_init_controls(const struct device *dev) 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; } @@ -384,7 +378,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, \ @@ -394,3 +388,50 @@ int emul_imager_init(const struct device *dev) VIDEO_DEVICE_DEFINE(emul_imager_##inst, DEVICE_DT_INST_GET(inst), NULL); 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 e0b2fee641ed..0175ad9c2e76 100644 --- a/tests/drivers/video/api/prj.conf +++ b/tests/drivers/video/api/prj.conf @@ -10,3 +10,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 76c176208275807a0b01e5c1fc3f6eae9de3427f Mon Sep 17 00:00:00 2001 From: Phi Bang Nguyen Date: Fri, 21 Mar 2025 11:26:04 +0100 Subject: [PATCH 3/4] drivers: video: emul_rx: Explicitly set init priority The Emul 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.emul_rx | 7 +++++++ drivers/video/video_emul_rx.c | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/video/Kconfig.emul_rx b/drivers/video/Kconfig.emul_rx index 39425d9b136b..ea0c2d4eac3f 100644 --- a/drivers/video/Kconfig.emul_rx +++ b/drivers/video/Kconfig.emul_rx @@ -8,3 +8,10 @@ config VIDEO_EMUL_RX default y help Enable driver for the MIPI RX emulated DMA engine. + +config VIDEO_EMUL_RX_INIT_PRIORITY + int "Emul Rx init priority" + default 61 + depends on VIDEO_EMUL_RX + help + Initialization priority for the EMUL Rx device. diff --git a/drivers/video/video_emul_rx.c b/drivers/video/video_emul_rx.c index 2a67559cb44a..9a1cb77e6787 100644 --- a/drivers/video/video_emul_rx.c +++ b/drivers/video/video_emul_rx.c @@ -235,7 +235,8 @@ int emul_rx_init(const struct device *dev) }; \ \ DEVICE_DT_INST_DEFINE(n, &emul_rx_init, NULL, &emul_rx_data_##n, &emul_rx_cfg_##n, \ - POST_KERNEL, CONFIG_VIDEO_INIT_PRIORITY, &emul_rx_driver_api); \ + POST_KERNEL, CONFIG_VIDEO_EMUL_RX_INIT_PRIORITY, \ + &emul_rx_driver_api); \ \ VIDEO_DEVICE_DEFINE(emul_rx_##n, DEVICE_DT_INST_GET(n), SOURCE_DEV(n)); From 78999d63b5cc508fd475ae84ef7e51c358ffc340 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Tue, 1 Apr 2025 13:10:20 +0000 Subject: [PATCH 4/4] 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. Modify the emulated imager registers to make the need for the CCI library appear. Signed-off-by: Josuah Demangeon --- drivers/video/video_emul_imager.c | 206 ++++++++++++++++++------------ 1 file changed, 121 insertions(+), 85 deletions(-) diff --git a/drivers/video/video_emul_imager.c b/drivers/video/video_emul_imager.c index e13018ad66e0..63ba4b7aa4c1 100644 --- a/drivers/video/video_emul_imager.c +++ b/drivers/video/video_emul_imager.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024 tinyVision.ai Inc. + * Copyright (c) 2024-2025 tinyVision.ai Inc. * * SPDX-License-Identifier: Apache-2.0 */ @@ -18,36 +18,39 @@ #include #include +#include "video_common.h" #include "video_ctrls.h" #include "video_device.h" LOG_MODULE_REGISTER(video_emul_imager, CONFIG_VIDEO_LOG_LEVEL); -#define EMUL_IMAGER_REG_SENSOR_ID 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_REG8(addr) ((addr) | VIDEO_REG_ADDR8_DATA8) +#define EMUL_IMAGER_REG16(addr) ((addr) | VIDEO_REG_ADDR8_DATA16_BE) +#define EMUL_IMAGER_REG24(addr) ((addr) | VIDEO_REG_ADDR8_DATA24_BE) + +#define EMUL_IMAGER_REG_SENSOR_ID EMUL_IMAGER_REG16(0x00) +#define EMUL_IMAGER_SENSOR_ID 0x2025 +#define EMUL_IMAGER_REG_CTRL EMUL_IMAGER_REG8(0x02) +#define EMUL_IMAGER_REG_TIMING1 EMUL_IMAGER_REG16(0x04) +#define EMUL_IMAGER_REG_TIMING2 EMUL_IMAGER_REG16(0x06) +#define EMUL_IMAGER_REG_TIMING3 EMUL_IMAGER_REG16(0x08) +#define EMUL_IMAGER_REG_CUSTOM EMUL_IMAGER_REG24(0x10) +#define EMUL_IMAGER_REG_FORMAT EMUL_IMAGER_REG8(0x20) /* Custom control that is just an I2C write for example and test purpose */ #define EMUL_IMAGER_CID_CUSTOM (VIDEO_CID_PRIVATE_BASE + 0x01) +/* Helper to avoid repetition */ +#define EMUL_IMAGER_REG_LIST(array) {.regs = (array), .size = ARRAY_SIZE(array)} + enum emul_imager_fmt_id { RGB565_320x240, YUYV_320x240, }; -struct emul_imager_reg { - uint16_t addr; - uint8_t value; +struct emul_imager_reg_list { + const struct video_reg *regs; + size_t size; }; struct emul_imager_mode { @@ -56,8 +59,7 @@ struct emul_imager_mode { * 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]; - /* More fields can be added according to the needs of the sensor driver */ + const struct emul_imager_reg_list lists[3]; }; struct emul_imager_config { @@ -79,53 +81,109 @@ struct emul_imager_data { }; /* All the I2C registers sent on various scenario */ -static const struct emul_imager_reg emul_imager_init_regs[] = { - {EMUL_IMAGER_REG_CTRL, 0x00}, - {EMUL_IMAGER_REG_INIT1, 0x10}, - {EMUL_IMAGER_REG_INIT2, 0x00}, +static const struct video_reg8 emul_imager_init_regs[] = { /* Undocumented registers from the vendor */ - {0x80, 0x01}, - {0x84, 0x01}, - {0x85, 0x20}, - {0x89, 0x7f}, - {0}, + {0xe3, 0x60}, + {0x9e, 0xd3}, + {0x05, 0x5d}, + {0x39, 0xf7}, + {0xf0, 0xef}, + {0xe8, 0x40}, + {0x6d, 0x16}, + {0x16, 0x33}, + {0xeb, 0xa7}, + {0xb8, 0x1f}, + {0x45, 0xb7}, + {0x26, 0x3a}, + {0x6e, 0x32}, + {0x1b, 0x9e}, + {0xd3, 0xf7}, + {0xd3, 0xb3}, + {0x7b, 0x64}, + {0xa3, 0xaf}, + {0x3c, 0x6e}, + {0x11, 0x2d}, + {0x15, 0x67}, + {0xb9, 0xc8}, + {0x12, 0xc8}, + {0xa6, 0x31}, + {0x0e, 0x7c}, + {0x7b, 0x64}, + {0xf8, 0x5f}, + {0x44, 0x27}, + {0xc5, 0x9a}, + {0x8d, 0x54}, }; -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[] = { - {EMUL_IMAGER_REG_TIMING1, 0x32}, - {EMUL_IMAGER_REG_TIMING2, 0x24}, - {0}, +static const struct video_reg emul_imager_320x240[] = { + {EMUL_IMAGER_REG_TIMING1, 320}, + {EMUL_IMAGER_REG_TIMING2, 240}, +}; +static const struct video_reg emul_imager_160x120[] = { + {EMUL_IMAGER_REG_TIMING1, 160}, + {EMUL_IMAGER_REG_TIMING2, 120}, }; -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}, }; /* Description of "modes", that pick lists of registesr that will be all sentto the imager */ struct emul_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}}, + { + .fps = 15, + .lists = { + EMUL_IMAGER_REG_LIST(emul_imager_320x240), + EMUL_IMAGER_REG_LIST(emul_imager_rgb565), + EMUL_IMAGER_REG_LIST(emul_imager_15fps), + }, + }, + { + .fps = 30, + .lists = { + EMUL_IMAGER_REG_LIST(emul_imager_320x240), + EMUL_IMAGER_REG_LIST(emul_imager_rgb565), + EMUL_IMAGER_REG_LIST(emul_imager_30fps), + }, + }, + { + .fps = 60, + .lists = { + EMUL_IMAGER_REG_LIST(emul_imager_320x240), + EMUL_IMAGER_REG_LIST(emul_imager_rgb565), + EMUL_IMAGER_REG_LIST(emul_imager_60fps), + }, + }, {0}, }; + struct emul_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}}, + { + .fps = 15, + .lists = { + EMUL_IMAGER_REG_LIST(emul_imager_320x240), + EMUL_IMAGER_REG_LIST(emul_imager_yuyv), + EMUL_IMAGER_REG_LIST(emul_imager_15fps), + } + }, + { + .fps = 30, + .lists = { + EMUL_IMAGER_REG_LIST(emul_imager_320x240), + EMUL_IMAGER_REG_LIST(emul_imager_yuyv), + EMUL_IMAGER_REG_LIST(emul_imager_30fps), + } + }, {0}, }; @@ -155,47 +213,18 @@ 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); -} - -/* 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) -{ - 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, uint32_t id) { + const struct emul_imager_config *cfg = dev->config; struct emul_imager_data *data = dev->data; - return emul_imager_write_reg(dev, EMUL_IMAGER_REG_CUSTOM, data->ctrls.custom.val); + return video_write_cci_reg(&cfg->i2c, EMUL_IMAGER_REG_CUSTOM, data->ctrls.custom.val); } /* 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; @@ -207,7 +236,9 @@ 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]); + const struct emul_imager_reg_list *list = &mode->lists[i]; + + ret = video_write_cci_multiregs(&cfg->i2c, list->regs, list->size); if (ret < 0) { goto err; } @@ -312,7 +343,9 @@ static int emul_imager_get_caps(const struct device *dev, struct video_caps *cap static int emul_imager_set_stream(const struct device *dev, bool enable, enum video_buf_type type) { - 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) = { @@ -339,7 +372,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)) { @@ -347,13 +380,14 @@ 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_multiregs(&cfg->i2c, emul_imager_init_regs, + ARRAY_SIZE(emul_imager_init_regs)); if (ret < 0) { LOG_ERR("Could not set initial registers"); return ret; @@ -395,7 +429,9 @@ 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, + /* SENSOR_ID_LO */ + [0x00] = EMUL_IMAGER_SENSOR_ID & 0xff, + [0x01] = EMUL_IMAGER_SENSOR_ID >> 8, }; if (num_msgs == 0) {