michaelh/zephyr
1
0
Fork 0
Code Releases Activity

video: stm32-dcmi: correct get/set fmt handling

Browse source 
This commit mainly correct the get/set format handling and how
DCMI format is stored within the driver.  struct video_format
within the data structure is used to store the format.
Reworked way to handle get format to avoid calling the sensor
set_fmt whenever performing the get_fmt.
Slightly adjusted code to as much as possible reuse return
values provided by functions.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
This commit is contained in:
Alain Volmat 2025-05-08 00:51:22 +02:00 • committed by Anas Nashif
commit df93e20414
1 changed files with 39 additions and 42 deletions
Download patch file Download diff file Expand all files Collapse all files

81
drivers/video/video_stm32_dcmi.c
View file

@ -43,10 +43,6 @@ struct video_stm32_dcmi_data {
struct video_format fmt; struct video_format fmt;
struct k_fifo fifo_in; struct k_fifo fifo_in;
struct k_fifo fifo_out; struct k_fifo fifo_out;
uint32_t pixel_format;
uint32_t height;
uint32_t width;
uint32_t pitch;
struct video_buffer *vbuf; struct video_buffer *vbuf;
}; };
@ -127,7 +123,7 @@ static int stm32_dma_init(const struct device *dev)
/* /*
* DMA configuration * DMA configuration
* Due to use of QSPI HAL API in current driver, * Due to use of DMA HAL API in current driver,
* both HAL and Zephyr DMA drivers should be configured. * both HAL and Zephyr DMA drivers should be configured.
* The required configuration for Zephyr DMA driver should only provide * The required configuration for Zephyr DMA driver should only provide
* the minimum information to inform the DMA slot will be in used and * the minimum information to inform the DMA slot will be in used and
@ -179,7 +175,6 @@ static int stm32_dcmi_enable_clock(const struct device *dev)
{ {
const struct video_stm32_dcmi_config *config = dev->config; const struct video_stm32_dcmi_config *config = dev->config;
const struct device *dcmi_clock = DEVICE_DT_GET(STM32_CLOCK_CONTROL_NODE); const struct device *dcmi_clock = DEVICE_DT_GET(STM32_CLOCK_CONTROL_NODE);
int err;
if (!device_is_ready(dcmi_clock)) { if (!device_is_ready(dcmi_clock)) {
LOG_ERR("clock control device not ready"); LOG_ERR("clock control device not ready");
@ -187,10 +182,15 @@ static int stm32_dcmi_enable_clock(const struct device *dev)
} }
/* Turn on DCMI peripheral clock */ /* Turn on DCMI peripheral clock */
err = clock_control_on(dcmi_clock, (clock_control_subsys_t *) &config->pclken); return clock_control_on(dcmi_clock, (clock_control_subsys_t *)&config->pclken);
if (err < 0) { }
LOG_ERR("Failed to enable DCMI clock. Error %d", err);
return err; static inline int video_stm32_dcmi_is_fmt_valid(uint32_t pixelformat, uint32_t pitch,
uint32_t height)
{
if (video_bits_per_pixel(pixelformat) / BITS_PER_BYTE == 0 ||
pitch * height > CONFIG_VIDEO_BUFFER_POOL_SZ_MAX) {
return -EINVAL;
} }
return 0; return 0;
@ -202,25 +202,24 @@ static int video_stm32_dcmi_set_fmt(const struct device *dev,
{ {
const struct video_stm32_dcmi_config *config = dev->config; const struct video_stm32_dcmi_config *config = dev->config;
struct video_stm32_dcmi_data *data = dev->data; struct video_stm32_dcmi_data *data = dev->data;
unsigned int bpp = video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE; int ret;
if (bpp == 0 || (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL)) { if (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL) {
return -EINVAL; return -EINVAL;
} }
if ((fmt->pitch * fmt->height) > CONFIG_VIDEO_BUFFER_POOL_SZ_MAX) { ret = video_stm32_dcmi_is_fmt_valid(fmt->pixelformat, fmt->pitch, fmt->height);
return -EINVAL; if (ret < 0) {
return ret;
} }
data->pixel_format = fmt->pixelformat; ret = video_set_format(config->sensor_dev, ep, fmt);
data->pitch = fmt->pitch; if (ret < 0) {
data->height = fmt->height; return ret;
data->width = fmt->width;
if (video_set_format(config->sensor_dev, ep, fmt)) {
return -EIO;
} }
data->fmt = *fmt;
return 0; return 0;
} }
@ -230,33 +229,38 @@ static int video_stm32_dcmi_get_fmt(const struct device *dev,
{ {
struct video_stm32_dcmi_data *data = dev->data; struct video_stm32_dcmi_data *data = dev->data;
const struct video_stm32_dcmi_config *config = dev->config; const struct video_stm32_dcmi_config *config = dev->config;
int ret;
if (fmt == NULL || (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL)) { if (fmt == NULL || (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL)) {
return -EINVAL; return -EINVAL;
} }
if (!video_get_format(config->sensor_dev, ep, fmt)) { /* Align DCMI format with the one provided by the sensor */
/* align DCMI with sensor fmt */ ret = video_get_format(config->sensor_dev, ep, fmt);
return video_stm32_dcmi_set_fmt(dev, ep, fmt); if (ret < 0) {
return ret;
} }
fmt->pixelformat = data->pixel_format; ret = video_stm32_dcmi_is_fmt_valid(fmt->pixelformat, fmt->pitch, fmt->height);
fmt->height = data->height; if (ret < 0) {
fmt->width = data->width; return ret;
fmt->pitch = data->pitch; }
data->fmt = *fmt;
return 0; return 0;
} }
static int video_stm32_dcmi_set_stream(const struct device *dev, bool enable) static int video_stm32_dcmi_set_stream(const struct device *dev, bool enable)
{ {
int err;
struct video_stm32_dcmi_data *data = dev->data; struct video_stm32_dcmi_data *data = dev->data;
const struct video_stm32_dcmi_config *config = dev->config; const struct video_stm32_dcmi_config *config = dev->config;
int err;
if (!enable) { if (!enable) {
if (video_stream_stop(config->sensor_dev)) { err = video_stream_stop(config->sensor_dev);
return -EIO; if (err < 0) {
return err;
} }
err = HAL_DCMI_Stop(&data->hdcmi); err = HAL_DCMI_Stop(&data->hdcmi);
@ -285,11 +289,7 @@ static int video_stm32_dcmi_set_stream(const struct device *dev, bool enable)
return -EIO; return -EIO;
} }
if (video_stream_start(config->sensor_dev)) { return video_stream_start(config->sensor_dev);
return -EIO;
}
return 0;
} }
static int video_stm32_dcmi_enqueue(const struct device *dev, static int video_stm32_dcmi_enqueue(const struct device *dev,
@ -297,7 +297,7 @@ static int video_stm32_dcmi_enqueue(const struct device *dev,
struct video_buffer *vbuf) struct video_buffer *vbuf)
{ {
struct video_stm32_dcmi_data *data = dev->data; struct video_stm32_dcmi_data *data = dev->data;
const uint32_t buffer_size = data->pitch * data->height; const uint32_t buffer_size = data->fmt.pitch * data->fmt.height;
if (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL) { if (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL) {
return -EINVAL; return -EINVAL;
@ -339,7 +339,6 @@ static int video_stm32_dcmi_get_caps(const struct device *dev,
struct video_caps *caps) struct video_caps *caps)
{ {
const struct video_stm32_dcmi_config *config = dev->config; const struct video_stm32_dcmi_config *config = dev->config;
int ret = -ENODEV;
if (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL) { if (ep != VIDEO_EP_OUT && ep != VIDEO_EP_ALL) {
return -EINVAL; return -EINVAL;
@ -349,9 +348,7 @@ static int video_stm32_dcmi_get_caps(const struct device *dev,
caps->min_line_count = caps->max_line_count = LINE_COUNT_HEIGHT; caps->min_line_count = caps->max_line_count = LINE_COUNT_HEIGHT;
/* Forward the message to the sensor device */ /* Forward the message to the sensor device */
ret = video_get_caps(config->sensor_dev, ep, caps); return video_get_caps(config->sensor_dev, ep, caps);
return ret;
} }
static DEVICE_API(video, video_stm32_dcmi_driver_api) = { static DEVICE_API(video, video_stm32_dcmi_driver_api) = {
@ -479,7 +476,7 @@ static int video_stm32_dcmi_init(const struct device *dev)
err = stm32_dcmi_enable_clock(dev); err = stm32_dcmi_enable_clock(dev);
if (err < 0) { if (err < 0) {
LOG_ERR("Clock enabling failed."); LOG_ERR("Clock enabling failed.");
return -EIO; return err;
} }
data->dev = dev; data->dev = dev;