-
Notifications
You must be signed in to change notification settings - Fork 8.4k
video: allow placement of the video buffer pool in a ZEPHYR REGION (and adapt STM32N6 / STM32H7RS config) #100628
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
base: main
Are you sure you want to change the base?
video: allow placement of the video buffer pool in a ZEPHYR REGION (and adapt STM32N6 / STM32H7RS config) #100628
Conversation
|
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. |
|
It definitely sounds like an improvement, thank you! Currently we have Maybe a different API would be to use the video memory region only, with a value computed by the user. Devicetree allows |
drivers/video/video_common.c
Outdated
| * 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static const char
There was a problem hiding this comment.
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.
drivers/video/Kconfig
Outdated
|
|
||
| config VIDEO_BUFFER_USE_SHARED_MULTI_HEAP | ||
| bool "Use shared multi heap for video buffer" | ||
| default n |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
I think this is because one of the two Kconfig exposed to user, the
|
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>
220eeab to
93a174b
Compare
|
@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). Other added stuff are a fix in the VENC POOL_SZ_MAX value which was not correct but anyway unused since based on SMH. |
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>
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>
93a174b to
209194c
Compare
|



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 ?)