From 5d256fd07a49f80c4b1a9c33b39317391246dcce Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Wed, 2 Jul 2025 21:05:17 +0000 Subject: [PATCH] WIP: convert zephyr,video-sw-generator and sample to RTIO Signed-off-by: Josuah Demangeon --- drivers/video/Kconfig | 1 + drivers/video/video_common.c | 25 ++- drivers/video/video_sw_generator.c | 99 +++--------- include/zephyr/drivers/video.h | 192 +++++------------------ samples/drivers/video/capture/src/main.c | 48 ++++-- 5 files changed, 119 insertions(+), 246 deletions(-) diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index 924e2156273c4..cd780ae9b43cc 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -8,6 +8,7 @@ # menuconfig VIDEO bool "Video drivers" + imply RTIO help Enable support for the VIDEO. diff --git a/drivers/video/video_common.c b/drivers/video/video_common.c index 37ed899b8029a..70a1f01b31bfa 100644 --- a/drivers/video/video_common.c +++ b/drivers/video/video_common.c @@ -9,10 +9,11 @@ #include #include -#include #include +#include #include #include +#include #include #include @@ -41,6 +42,28 @@ struct mem_block { static struct mem_block video_block[CONFIG_VIDEO_BUFFER_POOL_NUM_MAX]; + +static void video_iodev_submit(struct rtio_iodev_sqe *iodev_sqe) +{ + const struct device *dev = iodev_sqe->sqe.iodev->data; + const struct video_driver_api *api; + + __ASSERT_NO_MSG(dev != NULL); + __ASSERT_NO_MSG(dev->api != NULL); + + api = dev->api; + if (api->iodev_submit == NULL) { + rtio_iodev_sqe_err(iodev_sqe, -ENODEV); + return; + } + + api->iodev_submit(dev, iodev_sqe); +} + +const struct rtio_iodev_api _video_iodev_api = { + .submit = video_iodev_submit, +}; + struct video_buffer *video_buffer_aligned_alloc(size_t size, size_t align, k_timeout_t timeout) { struct video_buffer *vbuf = NULL; diff --git a/drivers/video/video_sw_generator.c b/drivers/video/video_sw_generator.c index ff176e0ad3480..6a7f9a0755cb2 100644 --- a/drivers/video/video_sw_generator.c +++ b/drivers/video/video_sw_generator.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -38,11 +39,9 @@ struct video_sw_generator_data { const struct device *dev; struct sw_ctrls ctrls; struct video_format fmt; - struct k_fifo fifo_in; - struct k_fifo fifo_out; + struct mpsc io_q; struct k_work_delayable work; int pattern; - struct k_poll_signal *sig; uint32_t frame_rate; }; @@ -291,73 +290,45 @@ static void video_sw_generator_worker(struct k_work *work) struct k_work_delayable *dwork = k_work_delayable_from_work(work); struct video_sw_generator_data *data; struct video_buffer *vbuf; + struct mpsc_node *node; + struct rtio_iodev_sqe *iodev_sqe; + struct rtio_sqe *sqe; data = CONTAINER_OF(dwork, struct video_sw_generator_data, work); k_work_reschedule(&data->work, K_MSEC(1000 / data->frame_rate)); - vbuf = k_fifo_get(&data->fifo_in, K_NO_WAIT); - if (vbuf == NULL) { + node = mpsc_pop(&data->io_q); + if (node == NULL) { return; } + iodev_sqe = CONTAINER_OF(node, struct rtio_iodev_sqe, q); + sqe = &iodev_sqe->sqe; + if (sqe->op != RTIO_OP_RX) { + LOG_ERR("Invalid operation %d of length %u for submission %p", + sqe->op, sqe->rx.buf_len, (void *)iodev_sqe); + rtio_iodev_sqe_err(iodev_sqe, -EINVAL); + return; + } + + vbuf = sqe->userdata; + switch (data->pattern) { case VIDEO_PATTERN_COLOR_BAR: video_sw_generator_fill(data->dev, vbuf); break; } - k_fifo_put(&data->fifo_out, vbuf); - - if (IS_ENABLED(CONFIG_POLL) && data->sig) { - k_poll_signal_raise(data->sig, VIDEO_BUF_DONE); - } - - k_yield(); -} - -static int video_sw_generator_enqueue(const struct device *dev, struct video_buffer *vbuf) -{ - struct video_sw_generator_data *data = dev->data; - - k_fifo_put(&data->fifo_in, vbuf); - - return 0; -} - -static int video_sw_generator_dequeue(const struct device *dev, struct video_buffer **vbuf, - k_timeout_t timeout) -{ - struct video_sw_generator_data *data = dev->data; - - *vbuf = k_fifo_get(&data->fifo_out, timeout); - if (*vbuf == NULL) { - return -EAGAIN; - } - - return 0; + rtio_iodev_sqe_ok(iodev_sqe, 0); } -static int video_sw_generator_flush(const struct device *dev, bool cancel) +static void video_sw_generator_iodev_submit(const struct device *dev, + struct rtio_iodev_sqe *iodev_sqe) { struct video_sw_generator_data *data = dev->data; - struct video_buffer *vbuf; - - if (!cancel) { - /* wait for all buffer to be processed */ - do { - k_sleep(K_MSEC(1)); - } while (!k_fifo_is_empty(&data->fifo_in)); - } else { - while ((vbuf = k_fifo_get(&data->fifo_in, K_NO_WAIT))) { - k_fifo_put(&data->fifo_out, vbuf); - if (IS_ENABLED(CONFIG_POLL) && data->sig) { - k_poll_signal_raise(data->sig, VIDEO_BUF_ABORTED); - } - } - } - return 0; + mpsc_push(&data->io_q, &iodev_sqe->q); } static int video_sw_generator_get_caps(const struct device *dev, struct video_caps *caps) @@ -371,21 +342,6 @@ static int video_sw_generator_get_caps(const struct device *dev, struct video_ca return 0; } -#ifdef CONFIG_POLL -static int video_sw_generator_set_signal(const struct device *dev, struct k_poll_signal *sig) -{ - struct video_sw_generator_data *data = dev->data; - - if (data->sig && sig != NULL) { - return -EALREADY; - } - - data->sig = sig; - - return 0; -} -#endif - static int video_sw_generator_set_frmival(const struct device *dev, struct video_frmival *frmival) { struct video_sw_generator_data *data = dev->data; @@ -439,16 +395,11 @@ static DEVICE_API(video, video_sw_generator_driver_api) = { .set_format = video_sw_generator_set_fmt, .get_format = video_sw_generator_get_fmt, .set_stream = video_sw_generator_set_stream, - .flush = video_sw_generator_flush, - .enqueue = video_sw_generator_enqueue, - .dequeue = video_sw_generator_dequeue, .get_caps = video_sw_generator_get_caps, .set_frmival = video_sw_generator_set_frmival, .get_frmival = video_sw_generator_get_frmival, .enum_frmival = video_sw_generator_enum_frmival, -#ifdef CONFIG_POLL - .set_signal = video_sw_generator_set_signal, -#endif + .iodev_submit = video_sw_generator_iodev_submit, }; static int video_sw_generator_init_controls(const struct device *dev) @@ -464,10 +415,10 @@ static int video_sw_generator_init(const struct device *dev) struct video_sw_generator_data *data = dev->data; data->dev = dev; - k_fifo_init(&data->fifo_in); - k_fifo_init(&data->fifo_out); k_work_init_delayable(&data->work, video_sw_generator_worker); + mpsc_init(&data->io_q); + return video_sw_generator_init_controls(dev); } diff --git a/include/zephyr/drivers/video.h b/include/zephyr/drivers/video.h index 20de3754887ab..3708fdb01c3a1 100644 --- a/include/zephyr/drivers/video.h +++ b/include/zephyr/drivers/video.h @@ -23,10 +23,11 @@ * @{ */ -#include #include -#include +#include +#include +#include #include #ifdef __cplusplus @@ -39,6 +40,15 @@ extern "C" { */ #define LINE_COUNT_HEIGHT (-1) +/** + * @brief Statically define an input or output video stream. + * + * @param name Name of the RTIO iodev that will be defined for this stream. + * @param dt_node Devicetree node referring to the device to stream buffers from/to. + */ +#define VIDEO_STREAM_DEFINE(name, dt_node) \ + RTIO_IODEV_DEFINE(name, &_video_iodev_api, DEVICE_DT_GET(dt_node)) + struct video_control; /** @@ -115,7 +125,7 @@ struct video_caps { enum video_buf_type type; /** list of video format capabilities (zero terminated). */ const struct video_format_cap *format_caps; - /** minimal count of video buffers to enqueue before being able to start + /** minimal count of video buffers to submit before being able to start * the stream. */ uint8_t min_vbuf_count; @@ -137,6 +147,22 @@ struct video_caps { int16_t max_line_count; }; +/** + * @brief Interface between a video device and the application + * + * A video interface to send or receive frames with a video device from the application. + * As a consequence, input or output, so memory-to-memory (M2M) devices need two interfaces. + * + * The application can call @ref VIDEO_DT_INPUT_DEFINE and @ref VIDEO_DT_OUTPUT_DEFINE to + * instanciate video interfaces along with the associated @ref iodev maintaining the I/O state. + */ +struct video_stream { + /** Video device with which this interface communicates */ + const struct device *dev; + /** Type of buffer the interace exchanges with the application, such as input or output */ + enum video_buf_type type; +}; + /** * @struct video_buffer * @brief Video buffer structure @@ -144,10 +170,7 @@ struct video_caps { * Represent a video frame. */ struct video_buffer { - /** type of the buffer */ enum video_buf_type type; - /** pointer to driver specific data. */ - void *driver_data; /** pointer to the start of the buffer. */ uint8_t *buffer; /** index of the buffer, optionally set by the application */ @@ -316,30 +339,11 @@ typedef int (*video_api_frmival_t)(const struct device *dev, struct video_frmiva typedef int (*video_api_enum_frmival_t)(const struct device *dev, struct video_frmival_enum *fie); /** - * @typedef video_api_enqueue_t - * @brief Enqueue a buffer in the driver’s incoming queue. - * - * See video_enqueue() for argument descriptions. - */ -typedef int (*video_api_enqueue_t)(const struct device *dev, struct video_buffer *buf); - -/** - * @typedef video_api_dequeue_t - * @brief Dequeue a buffer from the driver’s outgoing queue. + * @brief Callback API for submitting work to a video device with RTIO * - * See video_dequeue() for argument descriptions. + * @param dev Device */ -typedef int (*video_api_dequeue_t)(const struct device *dev, struct video_buffer **buf, - k_timeout_t timeout); - -/** - * @typedef video_api_flush_t - * @brief Flush endpoint buffers, buffer are moved from incoming queue to - * outgoing queue. - * - * See video_flush() for argument descriptions. - */ -typedef int (*video_api_flush_t)(const struct device *dev, bool cancel); +typedef void (*video_api_iodev_submit_t)(const struct device *dev, struct rtio_iodev_sqe *iodev_sqe); /** * @typedef video_api_set_stream_t @@ -373,14 +377,6 @@ typedef int (*video_api_ctrl_t)(const struct device *dev, uint32_t cid); */ typedef int (*video_api_get_caps_t)(const struct device *dev, struct video_caps *caps); -/** - * @typedef video_api_set_signal_t - * @brief Register/Unregister poll signal for buffer events. - * - * See video_set_signal() for argument descriptions. - */ -typedef int (*video_api_set_signal_t)(const struct device *dev, struct k_poll_signal *sig); - /** * @typedef video_api_selection_t * @brief Get/Set video selection (crop / compose) @@ -389,6 +385,9 @@ typedef int (*video_api_set_signal_t)(const struct device *dev, struct k_poll_si */ typedef int (*video_api_selection_t)(const struct device *dev, struct video_selection *sel); +/* The video iodev API implementation */ +extern const struct rtio_iodev_api _video_iodev_api; + __subsystem struct video_driver_api { /* mandatory callbacks */ video_api_format_t set_format; @@ -396,17 +395,14 @@ __subsystem struct video_driver_api { video_api_set_stream_t set_stream; video_api_get_caps_t get_caps; /* optional callbacks */ - video_api_enqueue_t enqueue; - video_api_dequeue_t dequeue; - video_api_flush_t flush; video_api_ctrl_t set_ctrl; video_api_ctrl_t get_volatile_ctrl; - video_api_set_signal_t set_signal; video_api_frmival_t set_frmival; video_api_frmival_t get_frmival; video_api_enum_frmival_t enum_frmival; video_api_selection_t set_selection; video_api_selection_t get_selection; + video_api_iodev_submit_t iodev_submit; }; /** @@ -558,97 +554,11 @@ static inline int video_enum_frmival(const struct device *dev, struct video_frmi return api->enum_frmival(dev, fie); } -/** - * @brief Enqueue a video buffer. - * - * Enqueue an empty (capturing) or filled (output) video buffer in the driver’s - * endpoint incoming queue. - * - * @param dev Pointer to the device structure for the driver instance. - * @param buf Pointer to the video buffer. - * - * @retval 0 Is successful. - * @retval -EINVAL If parameters are invalid. - * @retval -EIO General input / output error. - */ -static inline int video_enqueue(const struct device *dev, struct video_buffer *buf) -{ - const struct video_driver_api *api = (const struct video_driver_api *)dev->api; - - __ASSERT_NO_MSG(dev != NULL); - __ASSERT_NO_MSG(buf != NULL); - __ASSERT_NO_MSG(buf->buffer != NULL); - - api = (const struct video_driver_api *)dev->api; - if (api->enqueue == NULL) { - return -ENOSYS; - } - - return api->enqueue(dev, buf); -} - -/** - * @brief Dequeue a video buffer. - * - * Dequeue a filled (capturing) or displayed (output) buffer from the driver’s - * endpoint outgoing queue. - * - * @param dev Pointer to the device structure for the driver instance. - * @param buf Pointer a video buffer pointer. - * @param timeout Timeout - * - * @retval 0 Is successful. - * @retval -EINVAL If parameters are invalid. - * @retval -EIO General input / output error. - */ -static inline int video_dequeue(const struct device *dev, struct video_buffer **buf, - k_timeout_t timeout) -{ - const struct video_driver_api *api; - - __ASSERT_NO_MSG(dev != NULL); - __ASSERT_NO_MSG(buf != NULL); - - api = (const struct video_driver_api *)dev->api; - if (api->dequeue == NULL) { - return -ENOSYS; - } - - return api->dequeue(dev, buf, timeout); -} - -/** - * @brief Flush endpoint buffers. - * - * A call to flush finishes when all endpoint buffers have been moved from - * incoming queue to outgoing queue. Either because canceled or fully processed - * through the video function. - * - * @param dev Pointer to the device structure for the driver instance. - * @param cancel If true, cancel buffer processing instead of waiting for - * completion. - * - * @retval 0 Is successful, -ERRNO code otherwise. - */ -static inline int video_flush(const struct device *dev, bool cancel) -{ - const struct video_driver_api *api; - - __ASSERT_NO_MSG(dev != NULL); - - api = (const struct video_driver_api *)dev->api; - if (api->flush == NULL) { - return -ENOSYS; - } - - return api->flush(dev, cancel); -} - /** * @brief Start the video device function. * * video_stream_start is called to enter ‘streaming’ state (capture, output...). - * The driver may receive buffers with video_enqueue() before video_stream_start + * The driver may receive buffers with video_iodev_submit() before video_stream_start * is called. If driver/device needs a minimum number of buffers before being * able to start streaming, then driver set the min_vbuf_count to the related * endpoint capabilities. @@ -698,7 +608,6 @@ static inline int video_stream_stop(const struct device *dev, enum video_buf_typ } ret = api->set_stream(dev, false, type); - video_flush(dev, true); return ret; } @@ -791,33 +700,6 @@ int video_query_ctrl(struct video_ctrl_query *cq); */ void video_print_ctrl(const struct video_ctrl_query *const cq); -/** - * @brief Register/Unregister k_poll signal for a video endpoint. - * - * Register a poll signal to the endpoint, which will be signaled on frame - * completion (done, aborted, error). Registering a NULL poll signal - * unregisters any previously registered signal. - * - * @param dev Pointer to the device structure for the driver instance. - * @param sig Pointer to k_poll_signal - * - * @retval 0 Is successful, -ERRNO code otherwise. - */ -static inline int video_set_signal(const struct device *dev, struct k_poll_signal *sig) -{ - const struct video_driver_api *api; - - __ASSERT_NO_MSG(dev != NULL); - __ASSERT_NO_MSG(sig != NULL); - - api = (const struct video_driver_api *)dev->api; - if (api->set_signal == NULL) { - return -ENOSYS; - } - - return api->set_signal(dev, sig); -} - /** * @brief Set video selection (crop/compose). * diff --git a/samples/drivers/video/capture/src/main.c b/samples/drivers/video/capture/src/main.c index 5e07318b08531..0bb14f7847497 100644 --- a/samples/drivers/video/capture/src/main.c +++ b/samples/drivers/video/capture/src/main.c @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -26,6 +27,9 @@ LOG_MODULE_REGISTER(main, CONFIG_LOG_DEFAULT_LEVEL); #error No camera chosen in devicetree. Missing "--shield" or "--snippet video-sw-generator" flag? #endif +RTIO_DEFINE(rtio, CONFIG_VIDEO_BUFFER_POOL_NUM_MAX, CONFIG_VIDEO_BUFFER_POOL_NUM_MAX); +VIDEO_STREAM_DEFINE(iodev_camera, DT_CHOSEN(zephyr_camera)); + #if DT_HAS_CHOSEN(zephyr_display) static inline int display_setup(const struct device *const display_dev, const uint32_t pixfmt) { @@ -91,7 +95,6 @@ static inline void video_display_frame(const struct device *const display_dev, int main(void) { - struct video_buffer *buffers[CONFIG_VIDEO_BUFFER_POOL_NUM_MAX]; struct video_buffer *vbuf = &(struct video_buffer){}; const struct device *video_dev; struct video_format fmt; @@ -284,19 +287,34 @@ int main(void) } /* Alloc video buffers and enqueue for capture */ - for (i = 0; i < ARRAY_SIZE(buffers); i++) { + while (true) { + struct rtio_sqe *sqe; + + sqe = rtio_sqe_acquire(&rtio); + if (sqe == NULL) { + break; + } + /* * For some hardwares, such as the PxP used on i.MX RT1170 to do image rotation, * buffer alignment is needed in order to achieve the best performance */ - buffers[i] = video_buffer_aligned_alloc(bsize, CONFIG_VIDEO_BUFFER_POOL_ALIGN, - K_FOREVER); - if (buffers[i] == NULL) { + vbuf = video_buffer_aligned_alloc(bsize, CONFIG_VIDEO_BUFFER_POOL_ALIGN, + K_FOREVER); + if (vbuf == NULL) { LOG_ERR("Unable to alloc video buffer"); return 0; } - buffers[i]->type = type; - video_enqueue(video_dev, buffers[i]); + + vbuf->type = type; + + /* Turn the buffer into an RTIO submission */ + rtio_sqe_prep_read(sqe, &iodev_camera, RTIO_PRIO_NORM, + vbuf->buffer, vbuf->size, vbuf); + sqe->flags |= RTIO_SQE_MULTISHOT; + + /* Submit it to RTIO but do not wait them to complete immediately */ + rtio_submit(&rtio, 0); } /* Start video capture */ @@ -310,10 +328,11 @@ int main(void) /* Grab video frames */ vbuf->type = type; while (1) { - err = video_dequeue(video_dev, &vbuf, K_FOREVER); - if (err) { - LOG_ERR("Unable to dequeue video buf"); - return 0; + struct rtio_cqe *cqe = rtio_cqe_consume_block(&rtio); + struct video_buffer *vbuf = cqe->userdata; + + if (cqe->result < 0) { + LOG_ERR("I/O operation failed"); } LOG_DBG("Got frame %u! size: %u; timestamp %u ms", @@ -329,10 +348,7 @@ int main(void) video_display_frame(display_dev, vbuf, fmt); #endif - err = video_enqueue(video_dev, vbuf); - if (err) { - LOG_ERR("Unable to requeue video buf"); - return 0; - } + /* The vbuf will be re-queued thanks to RTIO_SQE_MULTISHOT */ + rtio_cqe_release(&rtio, cqe); } }