DP: userspace: dynamic domains #10491
Conversation
We need to be able to specify allocation flags for object pools and to verify their consistency. Add a parameter for that. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
lyakh
requested review from
LaurentiuM1234,
dbaluta,
kv2019i,
lbetlej,
lgirdwood,
marcinszkudlinski,
mmaka1,
pblaszko and
plbossart
as code owners
There was a problem hiding this comment.
Pull request overview
This pull request implements dynamic memory domain allocation for DP (Data Processing) userspace modules using an object pool instead of a static per-core array. The goal is to fix a playback issue by allowing more flexible domain management.
Changes:
- Introduced
objpool_headstructure to track object pool state including allocation flags - Updated objpool API to accept allocation flags and enforce flag consistency across pool allocations
- Replaced static per-core memory domain array with dynamic allocation using object pool
- Added
mdomfield toprocessing_moduleto track the associated memory domain
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/include/sof/objpool.h | Introduced objpool_head structure and updated API signatures to support flags parameter |
| src/lib/objpool.c | Implemented flags consistency checking and updated allocation/free functions to use objpool_head |
| test/ztest/unit/objpool/test_objpool_ztest.c | Updated tests to use new objpool API with flags parameter |
| src/include/module/module/base.h | Added mdom pointer field to processing_module for CONFIG_SOF_USERSPACE_APPLICATION |
| src/schedule/zephyr_dp_schedule.h | Removed scheduler_dp_domain_init declaration (no longer needed) |
| src/schedule/zephyr_dp_schedule.c | Removed call to scheduler_dp_domain_init |
| src/schedule/zephyr_dp_schedule_application.c | Replaced static domain array with dynamic allocation using objpool, updated cleanup logic |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * 32 blocks are allocated every time. Note, that by design allocated blocks are | ||
| * never freed. See more below. | ||
| * TODO: @a flags are currently only used when allocating new object sets. | ||
| * Should add a check that they're consistent with already allocated objects. |
There was a problem hiding this comment.
The TODO comment states that flags consistency checks should be added, but the check already exists at line 63 in objpool.c. However, this TODO is still partially valid because the implementation is incomplete - the flags are checked but never stored. Consider updating this TODO to reflect the current state, or removing it once the flags storage issue is fixed.
| * Should add a check that they're consistent with already allocated objects. | |
| * The implementation checks that flags are consistent with already allocated | |
| * objects, but the flags are not stored; update the implementation to store | |
| * and enforce flags, then remove this TODO. |
A hard-coded memory domain per core was a reasonable starting implementation, but it certainly isn't suitable for the final version. This commit removes per-core domains and allocates them dynamically using the objpool API. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
| unsigned int pidx; | ||
| size_t size; | ||
| uintptr_t start; | ||
| struct k_mem_domain *mdom = objpool_alloc(&dp_mdom_head, sizeof(*mdom), |
There was a problem hiding this comment.
this is worth some comment explaining why its coherent when DP runs on a single core. i.e. to avoid any misunderstandings.
|
It looks good to introduce a separate allocator structure instead of using a raw list. It makes the design clearer and easier to extend in the future. One small suggestion: it might be worth renaming the parameter currently called 'head' to something that better reflects allocator object. Let me know what you think. |
3 participants
use an object pool to allocate memory domains dynamically. Should also fix https://sof-ci.01.org/sofpr/PR10490/build18590/devicetest/index.html?model=PTLH_RVP_NOCODEC&testcase=check-playback-10sec