Skip to content

video: introduction of STM32 VENC driver for H264 hardware video compression #92884

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion boards/st/stm32n6570_dk/stm32n6570_dk_common.dtsi
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <zephyr/dt-bindings/flash_controller/xspi.h>
#include <zephyr/dt-bindings/gpio/raspberrypi-csi-connector.h>
#include <zephyr/dt-bindings/input/input-event-codes.h>
#include <zephyr/dt-bindings/memory-attr/memory-attr-sw.h>
#include <zephyr/dt-bindings/video/video-interfaces.h>
#include "arduino_r3_connector.dtsi"

Expand Down Expand Up @@ -42,7 +43,7 @@
compatible = "zephyr,memory-region";
reg = <0x90000000 DT_SIZE_M(32)>;
zephyr,memory-region = "PSRAM";
zephyr,memory-attr = <( DT_MEM_ARM(ATTR_MPU_RAM) )>;
zephyr,memory-attr = <( DT_MEM_ARM(ATTR_MPU_RAM_NOCACHE) | DT_MEM_SW_ALLOC_NON_CACHE )>;
};

leds: leds {
Expand Down
14 changes: 14 additions & 0 deletions drivers/video/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,20 @@ config VIDEO_I2C_RETRY_NUM
The default is to not retry. Board configuration files or user project can then
use the number of retries that matches their situation.

config VIDEO_BUFFER_USE_MEM_ATTR_HEAP
bool "Use mem attr api for video buffer"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bool "Use mem attr api for video buffer"
bool "Allocate video buffer from mem_attr heap"

+ maybe this should depends on MEM_ATTR_HEAP

default n

config VIDEO_BUFFER_MEM_SW_ATTRIBUTE
int "Mem SW attribute for video buffer"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
int "Mem SW attribute for video buffer"
int "Heap allocations attribute"

depends on VIDEO_BUFFER_USE_MEM_ATTR_HEAP
default 1
help
Mem SW attribute for video buffer:
1: ATTR_SW_ALLOC_CACHE
2: ATTR_SW_ALLOC_NON_CACHE
4: ATTR_SW_ALLOC_DMA
Comment on lines +70 to +73
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be best to avoid duplicating information from the header.
Proposal:

Suggested change
Mem SW attribute for video buffer:
1: ATTR_SW_ALLOC_CACHE
2: ATTR_SW_ALLOC_NON_CACHE
4: ATTR_SW_ALLOC_DMA
Attribute to request when performing allocations from mem_attr heap.
Refer to include/zephyr/dt-bindings/memory-attr/memory-attr-sw.h for
the list of allowed values (one of ATTR_SW_*).
For example, if you want to use ATTR_SW_ALLOC_CACHE defined as:
#define ATTR_SW_ALLOC_CACHE BIT(0)
This symbol should be set equal to 1 (equal to (1 << 0)).


source "drivers/video/Kconfig.esp32_dvp"

source "drivers/video/Kconfig.mcux_csi"
Expand Down
11 changes: 11 additions & 0 deletions drivers/video/video_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ LOG_MODULE_REGISTER(video_common, CONFIG_VIDEO_LOG_LEVEL);
#define VIDEO_COMMON_HEAP_ALLOC(align, size, timeout) \
shared_multi_heap_aligned_alloc(CONFIG_VIDEO_BUFFER_SMH_ATTRIBUTE, align, size)
#define VIDEO_COMMON_FREE(block) shared_multi_heap_free(block)
#elif defined(CONFIG_VIDEO_BUFFER_USE_MEM_ATTR_HEAP)
#include <zephyr/mem_mgmt/mem_attr_heap.h>
#include <zephyr/dt-bindings/memory-attr/memory-attr.h>

#define VIDEO_COMMON_HEAP_ALLOC(align, size, timeout) \
mem_attr_heap_aligned_alloc((CONFIG_VIDEO_BUFFER_MEM_SW_ATTRIBUTE << \
DT_MEM_SW_ATTR_SHIFT), align, size)
Comment on lines +33 to +35
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#define VIDEO_COMMON_HEAP_ALLOC(align, size, timeout) \
mem_attr_heap_aligned_alloc((CONFIG_VIDEO_BUFFER_MEM_SW_ATTRIBUTE << \
DT_MEM_SW_ATTR_SHIFT), align, size)
#define VIDEO_COMMON_HEAP_ALLOC(align, size, timeout) \
mem_attr_heap_aligned_alloc(DT_MEM_SW(CONFIG_VIDEO_BUFFER_MEM_SW_ATTRIBUTE), \
align, size)

#define VIDEO_COMMON_FREE(block) mem_attr_heap_free(block)
#else
K_HEAP_DEFINE(video_buffer_pool, CONFIG_VIDEO_BUFFER_POOL_SZ_MAX*CONFIG_VIDEO_BUFFER_POOL_NUM_MAX);
#define VIDEO_COMMON_HEAP_ALLOC(align, size, timeout) \
Expand All @@ -47,6 +55,9 @@ struct video_buffer *video_buffer_aligned_alloc(size_t size, size_t align, k_tim
struct mem_block *block;
int i;

#if defined(CONFIG_VIDEO_BUFFER_USE_MEM_ATTR_HEAP)
mem_attr_heap_pool_init();
#endif
Comment on lines +58 to +60
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: should check that return value is 0 or -EALREADY

/* find available video buffer */
for (i = 0; i < ARRAY_SIZE(video_buf); i++) {
if (video_buf[i].buffer == NULL) {
Expand Down
3 changes: 3 additions & 0 deletions include/zephyr/drivers/video.h
Original file line number Diff line number Diff line change
Expand Up @@ -1551,6 +1551,8 @@ int64_t video_get_csi_link_freq(const struct device *dev, uint8_t bpp, uint8_t l
*/
#define VIDEO_PIX_FMT_XYUV32 VIDEO_FOURCC('X', 'Y', 'U', 'V')

#define VIDEO_PIX_FMT_NV12 VIDEO_FOURCC('N', 'V', '1', '2')

/**
* @}
*/
Expand Down Expand Up @@ -1597,6 +1599,7 @@ static inline unsigned int video_bits_per_pixel(uint32_t pixfmt)
case VIDEO_PIX_FMT_SGRBG12P:
case VIDEO_PIX_FMT_SRGGB12P:
case VIDEO_PIX_FMT_Y12P:
case VIDEO_PIX_FMT_NV12:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added the whole multi buffer support of DCMIPP as well as description in video.h in the following PR which is still currently in draft so that I can fully test it:

#94081

return 12;
case VIDEO_PIX_FMT_SBGGR14P:
case VIDEO_PIX_FMT_SGBRG14P:
Expand Down
5 changes: 5 additions & 0 deletions west.yml
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,11 @@ manifest:
- name: zcbor
revision: 9b07780aca6fb21f82a241ba386ad9b379809337
path: modules/lib/zcbor
- name: zephyr-isp
url: [email protected]:AIS/mg-zephyr-isp.git
revision: master
path: Lib/zephyr-isp
submodules: true
Comment on lines +373 to +377
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Impacts the whole project, should be removed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dependency should be documented at board or sample level

# zephyr-keep-sorted-stop

self:
Expand Down