DP: more preparation for application-level user-space (part 4.2 of #10287) #10446
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lyakh
requested review from
LaurentiuM1234,
RanderWang,
abonislawski,
bardliao,
dbaluta,
jxstelter,
kv2019i,
lbetlej,
lgirdwood,
marcinszkudlinski,
mmaka1,
pblaszko,
plbossart,
ranj063 and
singalsu
as code owners
There was a problem hiding this comment.
Pull request overview
This PR continues preparation for application-level user-space functionality (part 4.2 of #10287) by making fundamental changes to buffer memory allocation and scheduler notification handling.
- Adds heap parameter to buffer allocation functions to support custom heap allocation for DP modules
- Removes NOTIFIER_ID_LL_PRE_RUN notification and simplifies scheduler integration
- Improves library manager error handling and adds domain removal functionality
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/include/sof/audio/buffer.h | Adds struct k_heap parameter to buffer allocation function signatures |
| src/include/sof/lib_manager.h | Removes public API declaration for lib_manager_free_module |
| src/include/sof/llext_manager.h | Adds llext_manager_rm_domain function declaration |
| src/include/sof/lib/notifier.h | Removes NOTIFIER_ID_LL_PRE_RUN notification ID |
| src/ipc/ipc-helper.c | Updates buffer_new to accept heap parameter |
| src/ipc/ipc3/helper.c | Passes NULL heap to buffer_new for IPC3 buffers |
| src/ipc/ipc4/helper.c | Adds DP heap detection and passes appropriate heap to buffer creation |
| src/audio/buffers/comp_buffer.c | Implements heap-aware buffer allocation and deallocation |
| src/audio/module_adapter/module_adapter.c | Passes module heap to buffer_alloc |
| src/audio/src/src_common.c | Changes mod_balloc to mod_alloc and adds proper cleanup in src_free |
| src/audio/host-zephyr.c | Passes NULL heap to buffer_alloc_range |
| src/audio/host-legacy.c | Passes NULL heap to buffer_alloc |
| src/audio/dai-zephyr.c | Passes NULL heap to buffer_alloc_range |
| src/audio/dai-legacy.c | Passes NULL heap to buffer_alloc |
| src/audio/copier/copier_generic.c | Passes NULL heap to buffer_new |
| src/audio/chain_dma.c | Passes NULL heap to buffer_alloc |
| src/library_manager/lib_manager.c | Makes lib_manager_free_module static and adds documentation |
| src/library_manager/llext_manager.c | Adds NULL check for virtual_memory_regions, implements llext_manager_rm_domain, and improves error handling |
| src/schedule/ll_schedule.c | Removes NOTIFIER_ID_LL_PRE_RUN notification |
| src/schedule/zephyr_ll.c | Removes NOTIFIER_ID_LL_PRE_RUN notification |
| src/schedule/zephyr_dp_schedule.c | Simplifies tick handling, removes PRE_RUN registration, adds scheduler_dp_task_stop, renames task_cancel to task_stop |
| test/cmocka/src/util.h | Passes NULL heap to buffer_new in test utilities |
| test/cmocka/src/math/fft/fft.c | Passes NULL heap to buffer_new in all FFT tests |
| test/cmocka/src/audio/buffer/*.c | Passes NULL heap to buffer_new in all buffer tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /* | ||
| * \brief Free module | ||
| * | ||
| * param[in] component_id - component id coming from ipc config. This function reguires valid |
There was a problem hiding this comment.
Spelling error: "reguires" should be "requires".
Suggested change
| * param[in] component_id - component id coming from ipc config. This function reguires valid | |
| * param[in] component_id - component id coming from ipc config. This function requires valid |
Add a heap parameter to buffer allocation functions. This makes buffer structures accessible to the user-space. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
When a LLEXT module is freed, its partitions should be removed from any memory domains. Add a function for that. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
lgirdwood
approved these changes
Dec 16, 2025
| /* | ||
| * \brief Free module | ||
| * | ||
| * param[in] component_id - component id coming from ipc config. This function reguires valid |
kv2019i
approved these changes
Dec 16, 2025
src/schedule/zephyr_dp_schedule.c
Outdated
| int ret; | ||
|
|
||
| scheduler_dp_task_cancel(data, task); | ||
| scheduler_dp_task_stop(data, task); |
Collaborator
There was a problem hiding this comment.
This is somewhat confusing with commit message as the patch both removes one call to scheduler_dp_task_cancel() and also claims it is not used. But ack, I now get you mean task_cancel() is not called via the ops.
Collaborator
Author
There was a problem hiding this comment.
@kv2019i correct, but I had to drop that commit - it was breaking QB
SRC is relying on the managed memory freeing but the common SOF approach now is to free all managed heap allocations explicitly. Add the missing mod_free() calls. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Both lib_manager_module_create() and lib_manager_module_free() are defined and used in lib_manager.c exclusively, so both should be static functions. It is already the case for the former. Make lib_manager_module_free() static too. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Currently DP deadlines are recalculated in the beginning and the end of each LL scheduling tick. As discussed in thesofproject#10177 (comment) recalculating it in the beginning is superfluous. Remove it. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
This reverts commit 49f7b79. Commit 9fcc269 ("Audio: SRC: All memory allocations through module") to which it's referring wasn't wrong, the delay_lines buffer in SRC must also be allocated on the module heap to be accessible in the DP userspace mode. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
The check virtual_region == NULL after a
SYS_MM_DRV_MEMORY_REGION_FOREACH(virtual_memory_regions, virtual_region) {}
loop is wrong and useless - the only way virtual_region can be NULL
is if virtual_memory_regions == NULL and in that case the
SYS_MM_DRV_MEMORY_REGION_FOREACH() loop will cause an immediate
exception. Fix this by checking virtual_memory_regions != NULL before
the loop.
Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
jsarha
approved these changes
Dec 16, 2025
Contributor
jsarha
left a comment
jsarha
left a comment
There was a problem hiding this comment.
Did not find anything wrong.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part 4.2 of... Sorry, there's really no better way to describe this than just say that this is a subset of #10386 to make it smaller again...