From 99a871eb37595f1620c49526411973b3bee05d24 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Wed, 18 Sep 2024 02:30:50 +0200 Subject: [PATCH 1/2] drivers: video: introduce "GET" sub-operations The video_get_ctrl() API permits to retrieve a value from a device using standard CIDs from . The CIDs do not come with a range information, and to know which value to apply to a video driver, knowing the minimum and maximum value before-hand is required. This prevents building generic layers that handle any video devices, such as protocols such as USB UVC, GigE Vision, or anything making use of "zephyr,camera" chosen node. This commit introduces extra flags added to the CIDs that indicates to the target device that instead of returning the current value, they should return the minimum, maximum, or default value instead, with the same type as the current value. The GET_CUR operation having a flag of 0x00, this makes all drivers implicitly support this new API, with an opt-in migration to also support the extra controls, correctly rejecting the unsupported extra operations by default. Signed-off-by: Josuah Demangeon --- include/zephyr/drivers/video-controls.h | 25 +++++++++++++++++++++++++ include/zephyr/drivers/video.h | 2 +- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/include/zephyr/drivers/video-controls.h b/include/zephyr/drivers/video-controls.h index 7912ab0d91cd..04f9542db5e5 100644 --- a/include/zephyr/drivers/video-controls.h +++ b/include/zephyr/drivers/video-controls.h @@ -42,6 +42,31 @@ extern "C" { * @} */ +/** + * @name Control Get operations + * + * Extra flags for video controls to inquire about the dimensions of an existing + * control: the minimum, maximum, or default value. + * + * For instance, OR-ing @c VIDEO_CID_CAMERA_EXPOSURE and @c VIDEO_CTRL_GET_MAX + * permits to query the maximum exposure time instead of the current exposure + * time. + * + * If no Control Get flag is added to a CID, the behavior is to fetch the current + * value as with @ref VIDEO_CTRL_GET_CUR. + * These must only be used along with the @ref video_get_ctrl() API. + * + * @{ + */ +#define VIDEO_CTRL_GET_CUR 0x00000000 /**< Get the current value */ +#define VIDEO_CTRL_GET_MIN 0x00001000 /**< Get the minimum value */ +#define VIDEO_CTRL_GET_MAX 0x00002000 /**< Get the maximum value */ +#define VIDEO_CTRL_GET_DEF 0x00003000 /**< Get the default value */ +#define VIDEO_CTRL_GET_MASK 0x0000f000 /**< Mask for get operations */ +/** + * @} + */ + /** * @name Generic class control IDs * @{ diff --git a/include/zephyr/drivers/video.h b/include/zephyr/drivers/video.h index b148f19a4a3c..664a8905b81b 100644 --- a/include/zephyr/drivers/video.h +++ b/include/zephyr/drivers/video.h @@ -16,7 +16,7 @@ * @brief Video Interface * @defgroup video_interface Video Interface * @since 2.1 - * @version 1.0.0 + * @version 1.1.0 * @ingroup io_interfaces * @{ */ From 3bd46c61493237d546131a0cbdbee3ce0537990b Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Sun, 6 Oct 2024 17:12:43 +0000 Subject: [PATCH 2/2] drivers: video: common: add helpers to check video control range This allows implementing the VIDEO_CTRL_GET_MIN/MAX/DEF controls on drivers with minimum modification of the drivers. A typical usage in a video_get_ctrl() implementation would be: ret = video_get_ctrl_range(cid, value, 0, 320, 9); if (ret <= 0) { return ret; } return sensor_read_u32(&cfg->i2c, EXPOSURE_REG, value, 2); A typical usage in a video_set_ctrl() implementation would be: ret = video_check_range_u32(dev, cid, (uint32_t)value); if (ret < 0) { return ret; } return sensor_read_reg(&cfg->i2c, EXPOSURE_REG, value, 2); Signed-off-by: Josuah Demangeon --- drivers/video/video_common.c | 87 ++++++++++++++++++++++++++++++++++++ drivers/video/video_common.h | 68 ++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 drivers/video/video_common.h diff --git a/drivers/video/video_common.c b/drivers/video/video_common.c index 6ff4c7b93385..bec34c044e42 100644 --- a/drivers/video/video_common.c +++ b/drivers/video/video_common.c @@ -6,6 +6,9 @@ #include #include +#include + +#include "video_common.h" #if defined(CONFIG_VIDEO_BUFFER_USE_SHARED_MULTI_HEAP) #include @@ -83,3 +86,87 @@ void video_buffer_release(struct video_buffer *vbuf) VIDEO_COMMON_FREE(block->data); } } + +int video_get_range_int(unsigned int cid, int *value, int min, int max, int def) +{ + switch (cid & VIDEO_CTRL_GET_MASK) { + case VIDEO_CTRL_GET_MIN: + *value = min; + return 0; + case VIDEO_CTRL_GET_MAX: + *value = max; + return 0; + case VIDEO_CTRL_GET_DEF: + *value = def; + return 0; + case VIDEO_CTRL_GET_CUR: + return 1; + default: + return -ENOTSUP; + } +} + +int video_get_range_int64(unsigned int cid, int64_t *value, int64_t min, int64_t max, int64_t def) +{ + switch (cid & VIDEO_CTRL_GET_MASK) { + case VIDEO_CTRL_GET_MIN: + *value = min; + return 0; + case VIDEO_CTRL_GET_MAX: + *value = max; + return 0; + case VIDEO_CTRL_GET_DEF: + *value = def; + return 0; + case VIDEO_CTRL_GET_CUR: + return 1; + default: + return -ENOTSUP; + } +} + +int video_check_range_int(const struct device *dev, unsigned int cid, int value) +{ + int min; + int max; + int ret; + + ret = video_get_ctrl(dev, cid | VIDEO_CTRL_GET_MIN, &min); + if (ret < 0) { + return ret; + } + + ret = video_get_ctrl(dev, cid | VIDEO_CTRL_GET_MAX, &max); + if (ret < 0) { + return ret; + } + + if (value < min || value > max || min > max) { + return -ERANGE; + } + + return 0; +} + +int video_check_range_int64(const struct device *dev, unsigned int cid, int64_t value) +{ + int64_t min; + int64_t max; + int ret; + + ret = video_get_ctrl(dev, cid | VIDEO_CTRL_GET_MIN, &min); + if (ret < 0) { + return ret; + } + + ret = video_get_ctrl(dev, cid | VIDEO_CTRL_GET_MAX, &max); + if (ret < 0) { + return ret; + } + + if (value < min || value > max || min > max) { + return -ERANGE; + } + + return 0; +} diff --git a/drivers/video/video_common.h b/drivers/video/video_common.h new file mode 100644 index 000000000000..45dfc42088d5 --- /dev/null +++ b/drivers/video/video_common.h @@ -0,0 +1,68 @@ +/* + * Copyright (c) 2024, tinyVision.ai Inc. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef ZEPHYR_INCLUDE_VIDEO_COMMON_H +#define ZEPHYR_INCLUDE_VIDEO_COMMON_H + +/** + * @brief Provide the minimum/maximum/default integer value depending on the CID. + * + * Video CIDs can contain sub-operations. This function facilitates + * implementation of video controls in the drivers by handling these + * the range-related CIDs. + * + * @param cid The Video Control ID which contains the operation. + * @param value The value, selected among @p min, @p max and @p def. + * @param min The minimum value returned if the CID requested it. + * @param max The maximum value returned if the CID requested it. + * @param def The default value returned if the CID requested it. + * + * @return 0 if the operation was regarding a range CID and the value could + * be set. In which case, there is nothing else to do than returning 0. + * @return 1 if the operation is not for the range, but the current value, + * in which case it is the duty of the driver to query the current + * value to the hardware + * @return A negative error code if an error occurred. + * + * @{ + */ + +/** Signed integer version */ +int video_get_range_int(unsigned int cid, int *value, int min, int max, int def); + +/** Signed 64-bit version */ +int video_get_range_int64(unsigned int cid, int64_t *value, int64_t min, int64_t max, int64_t def); + +/** + * @} + */ + +/** + * @brief Check if the integer value is within range for this CID. + * + * Before setting a video control, a driver might be interested in checking + * if it is within a valid range. This function facilitates it by reusing the + * video_get_ctrl() API using @c VIDEO_CTRL_GET_MIN and @c VIDEO_CTRL_GET_MAX + * to validate the input. + * + * @param dev The video device to query to learn about the min and max. + * @param cid The CID for which to check the range. + * @param value The integer value that must be matched against the range. + * + * @{ + */ + +/** Signed integer version */ +int video_check_range_int(const struct device *dev, unsigned int cid, int value); + +/** Signed 64-bit version */ +int video_check_range_int64(const struct device *dev, unsigned int cid, int64_t value); + +/** + * @} + */ + +#endif /* ZEPHYR_INCLUDE_VIDEO_COMMON_H */