Skip to content

Conversation

@avolmat-st
Copy link

The video buffer pool is placed in the default memory region. Due to that it wasn't possible to allow video buffer in other memory and multi-heap / shared-multi-heap was used.
Another alternative is simply to be able to indicate in which region the video buffer pool should be placed.
Similar approach is now available for LVGL buffers via dedicated config, making usage under the hood of the Z_GENERIC_SECTION mechanism.

Similar approach is proposed here, via addition two new KConfig for video subsystem VIDEO_BUFFER_POOL_ZEPHYR_REGION and VIDEO_BUFFER_POOL_ZEPHYR_REGION_NAME, to indicate where the video buffer pool should be placed.

This gives more flexibility on the whole soc memory allocation strategy, by simply partitioning the memory via DT, giving required caching properties as well.
Tricky part is that, depending on the memory, it might not be available yet from the very beginning, making it impossible to use the normal Z_HEAP_DEFINE_BY_SECT macro since this would lead to configuration the heap too early during the boot at a moment when the memory isn't yet available.
To overcome this, this manually configure the k_heap via the k_heap_init on the very first allocation request.

Two other commits are to move away from multi-heap / shared-multi-heap for LTDC / video buffer on STM32 platforms.
(if this is agreed, it would probably be necessary to either remove the SMH configuration of the MEMC XSPI/OSPI or at least make it configurable ?)

@avolmat-st
Copy link
Author

Forgot that STM32 VENC driver is using SMH for allocation of its internal buffers. Putting the PR in draft for the time being while figuring out a way for that.

@avolmat-st avolmat-st marked this pull request as draft December 7, 2025 15:54
@josuah
Copy link
Contributor

josuah commented Dec 7, 2025

It definitely sounds like an improvement, thank you!

Currently we have CONFIG_VIDEO_BUFFER_POOL_SZ_MAX * CONFIG_VIDEO_BUFFER_POOL_NUM_MAX to set the size, but currently this does not work (allocation fails) because there is overhead for the heap header struct, for alignment, and fragmentation can happen on top so malloc()/free() can randomly make the allocation fail at runtime (which is helped by reusing buffers).

Maybe a different API would be to use the video memory region only, with a value computed by the user. Devicetree allows #define and multiplications so things like (3 * 640 * 480 * 2) + (2 * 64 * 64 * 1) and even macro that adds padding would work.

* since the section might not be yet accessible from the beginning, making it impossible
* to initialize it if done via Z_HEAP_DEFINE_IN_SECT
*/
static char VIDEO_BUFFER_POOL_REGION_NAME
Copy link
Contributor

Choose a reason for hiding this comment

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

static const char

Copy link
Author

Choose a reason for hiding this comment

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

Actually having 2nd thought about this change in const. This is then given to k_heap_init which anyway will need a (void *) so, losing the const statement.


config VIDEO_BUFFER_USE_SHARED_MULTI_HEAP
bool "Use shared multi heap for video buffer"
default n
Copy link
Contributor

Choose a reason for hiding this comment

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

no need

Copy link
Contributor

@josuah josuah Dec 8, 2025

Choose a reason for hiding this comment

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

Do you mean to drop SMH support after this is merged?

Copy link
Author

Choose a reason for hiding this comment

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

I think this refer to the need of default n.

@ngphibang
Copy link
Contributor

Currently we have CONFIG_VIDEO_BUFFER_POOL_SZ_MAX * CONFIG_VIDEO_BUFFER_POOL_NUM_MAX to set the size, but currently this does not work (allocation fails) because there is overhead for the heap header struct, for alignment, and fragmentation can happen on top so malloc()/free() can randomly make the allocation fail at runtime (which is helped by reusing buffers).

I think this is because one of the two Kconfig exposed to user, the CONFIG_VIDEO_BUFFER_POOL_SZ_MAX does not make sense and cause the confusion, i.e., in fact, if we allocate less than CONFIG_VIDEO_BUFFER_POOL_NUM_MAX, the size of one buffer can still be greater than CONFIG_VIDEO_BUFFER_POOL_SZ_MAX. A simple fix is to expose

  • CONFIG_VIDEO_BUFFER_POOL_NUM_MAX : max numbers of buffers (for video_buf[] array)
  • CONFIG_VIDEO_BUFFER_POOL_HEAP_SIZE : size in bytes of the video buffer pool. This way user can specify the total size and does not need to adjust the header struct to their buffer size, etc.

Alain Volmat added 2 commits December 8, 2025 21:44
Addition of two options in order to select the Zephyr region
into which the video buffer pool should be placed.
CONFIG_VIDEO_BUFFER_POOL_ZEPHYR_REGION allows to indicate that the
video video pool should be placed in a specific ZEPHYR region which
name is CONFIG_VIDEO_BUFFER_POOL_ZEPHYR_REGION_NAME

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
Avoid default n since this is the default for a bool config

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
@avolmat-st avolmat-st force-pushed the pr-video-zephyr-region-support branch from 220eeab to 93a174b Compare December 8, 2025 21:18
@avolmat-st
Copy link
Author

@josuah, @ngphibang, I've added several commits in the PR, among them the last 5 commits which does the change from POOL_SZ_MAX into POOL_HEAP_SIZE. I tried to avoid break between commits so that there should be no situation where platform got broken during the move, leading to 5 commits to do that. This makes the transition harder to read finally so maybe not worth really and simple 2 commits could do the trick,with in such case git bisect breakage since between 2 commits making the transition this would get broken (it is probably better to not have ALL done in a single commit which would get pretty big).
Any opinion ?

Other added stuff are a fix in the VENC POOL_SZ_MAX value which was not correct but anyway unused since based on SMH.
I added as well new configurations in the VENC driver in order to be able to select the way to allocate the internal buffers needed by the VENC for its processing, making it possible to use either SMH (as previously), either a dedicated HEAP initialized by the driver. That done it becomes possible as for the VENC internal memory to be placed in any Zephyr region.

Alain Volmat added 2 commits December 9, 2025 12:22
Allow usage of either Shared-Multi-Heap based internal memory pool
allocation or allocation from a HEAP located optional in a
Zephyr region.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
The CONFIG_VIDEO_BUFFER_POOL_SZ_MAX represent the size of the
biggest buffer in the pool size and not the whole pool size.
For the stm32-venc this should be 1-000-000 and not 10-000-000.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
Alain Volmat added 7 commits December 9, 2025 12:22
Switch to usage of ZEPHYR_REGION for display and video
buffers instead of usage via the Shared-Multi-Heap API.
The ZEPHYR_REGION mechanism offer more flexibility than
the shared-multi-heap which is allocated on the whole
PSRAM.
If enabled, add allocation of the VENC internal buffers
from the PSRAM via the HEAP allocator.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
Switch to usage of ext-sdram property (ZEPHYR_REGION) for display
buffers instead of usage via the Shared-Multi-Heap API.
The ZEPHYR_REGION mechanism offer more flexibility than the
shared-multi-heap which is allocated on the whole PSRAM.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
Clarify the size of the video buffer pool by having a
dedicated CONFIG for it.  Until now the size of the
video buffer pool was equal to VIDEO_BUFFER_POOL_SZ_MAX
multiply by VIDEO_BUFFER_POOL_NUM_MAX.

This commit only add the description, the config doesn't
have yet any effect. Change will be added after all configs
are updated to define it in order to avoid breaking
platforms between 2 commits.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
In preparation to the renaming of CONFIG_VIDEO_BUFFER_POOL_SZ_MAX
into CONFIG_VIDEO_BUFFER_POOL_HEAP_SIZE, add the new CONFIG
in all conf files equal to POOL_SZ_MAX multiply by POOL_NUM_MAX.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
Update video common code and applications to rely on the
CONFIG_VIDEO_BUFFER_POOL_HEAP_SIZE instead of
CONFIG_VIDEO_BUFFER_POOL_SZ_MAX.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
Remove all CONFIG_VIDEO_BUFFER_POOL_SZ_MAX settings in
config files.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
Now that VIDEO_BUFFER_POOL_HEAP_SIZE is available is used
in all projects, VIDEO_BUFFER_POOL_SZ_MAX can be removed.

Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
@avolmat-st avolmat-st force-pushed the pr-video-zephyr-region-support branch from 93a174b to 209194c Compare December 9, 2025 11:24
@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 9, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Boards/SoCs area: Video Video subsystem platform: STM32 ST Micro STM32 Release Notes To be mentioned in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants