Skip to content

Part 1 of userspace DP subset PR #10302 - #10350

Merged
lgirdwood merged 6 commits into
thesofproject:mainfrom
lyakh:user-p1_1
Nov 5, 2025
Merged

Part 1 of userspace DP subset PR #10302#10350
lgirdwood merged 6 commits into
thesofproject:mainfrom
lyakh:user-p1_1

Conversation

@lyakh

@lyakhlyakh commented Nov 3, 2025

Copy link
Copy Markdown
Collaborator

The simpler part of #10302 - no integration with the driver heap API

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces new heap allocation APIs and refactors memory allocation in the module adapter to consolidate allocation/deallocation logic into helper functions.

  • Adds sof_heap_alloc() and sof_heap_free() functions for heap-specific memory allocation with flag support
  • Refactors module adapter memory allocation/deallocation into dedicated helper functions
  • Extends module allocation API with mod_alloc_ext() to support allocation flags (e.g., coherency control)

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
zephyr/lib/alloc.cAdds sof_heap_alloc() and sof_heap_free() functions for managing heap allocations with flag support
zephyr/include/rtos/alloc.hDeclares new heap allocation functions
test/cmocka/src/common_mocks.cProvides mock implementations of heap allocation functions for testing
src/platform/library/lib/alloc.cAdds stub implementations using standard malloc/free
src/lib/alloc.cAdds stub implementations using standard malloc/free
src/include/sof/audio/module_adapter/module/generic.hAdds mod_alloc_ext() declaration and converts mod_alloc_align() to inline wrapper; fixes typo
src/audio/module_adapter/module_adapter.cExtracts memory allocation/free logic into helper functions
src/audio/module_adapter/module/generic.cRenames mod_alloc_align() to mod_alloc_ext() with flags parameter; fixes typos
posix/include/rtos/kernel.hAdds stub k_heap struct definition
posix/include/rtos/alloc.hAdds forward declarations and stub functions for heap allocation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

void *sof_heap_alloc(struct k_heap *heap, uint32_t flags, size_t bytes,
size_t alignment)
{
return malloc(bytes);

CopilotAINov 3, 2025

Copy link

Choose a reason for hiding this comment

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

The alignment parameter is not used in this implementation. While this is a stub implementation, it should either align the allocated memory or document why alignment is not respected. Consider using aligned_alloc() or documenting this limitation.

Suggested change
returnmalloc(bytes);
/*Usealigned_alloctorespectthealignmentparameter.
*Note: aligned_allocrequiresthatbytesisamultipleofalignment.
*/
if (alignment&& (bytes % alignment!=0)) {
bytes= ((bytes+alignment-1) / alignment) *alignment;
}
returnaligned_alloc(alignment ? alignment : sizeof(void*), bytes);

Copilot uses AI. Check for mistakes.
Comment threadsrc/lib/alloc.c
void *sof_heap_alloc(struct k_heap *heap, uint32_t flags, size_t bytes,
size_t alignment)
{
return malloc(bytes);

CopilotAINov 3, 2025

Copy link

Choose a reason for hiding this comment

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

The alignment parameter is not used in this implementation. While this is a stub implementation, it should either align the allocated memory or document why alignment is not respected. Consider using aligned_alloc() or documenting this limitation.

Suggested change
returnmalloc(bytes);
returnaligned_alloc(alignment, bytes);

Copilot uses AI. Check for mistakes.
Comment on lines +125 to +133
void WEAK *sof_heap_alloc(struct k_heap *heap, uint32_t flags, size_t bytes,
size_t alignment)
{
(void)heap;
(void)flags;
(void)alignment;

return malloc(bytes);
}

CopilotAINov 3, 2025

Copy link

Choose a reason for hiding this comment

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

The alignment parameter is marked as unused but is not respected by the underlying malloc() call. This could lead to misaligned memory in tests. Consider using aligned_alloc() to honor the alignment requirement or documenting that alignment is not supported in test mocks.

Copilot uses AI. Check for mistakes.

@lgirdwoodlgirdwood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@lyakh are you able to drop the 1st patch atm, and get the others that don't depend on patch 1 merged as part 1. Then we can follow up with heaps.

@lyakh
lyakhforce-pushed the user-p1_1 branch 3 times, most recently from 8592072 to 15c2b47CompareNovember 4, 2025 09:30

@kv2019ikv2019i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@lyakh can you add some human readable summary in the title? this is now part 1 of 1000, and the 1000 is another "part y of 1002" of PR. it would be nice for the summary to immediately describe what this PR is about.

@lgirdwood

Copy link
Copy Markdown
Member

@lyakh are you able to drop the 1st patch atm, and get the others that don't depend on patch 1 merged as part 1. Then we can follow up with heaps.

Ok, I'm good after chat.

@lgirdwood

Copy link
Copy Markdown
Member

SOFCI TEST


if (!mod) {
comp_err(dev, "failed to allocate memory for module");
goto err;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Would you consider moving the code from the 'err' label to this location?

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

I'm usually all for "early returns," i.e. for

if (fail)
return;

instead of

 if (fail)
goto err;
...
return 0;
err:
return -EINVAL;

but if there's some cleanup to be done in (all) error cases, I usually prefer a goto. When there's only one error case - well, that's a grey zone for me... But since we're likely to extend this function in the future, I'd rather keep it this way.

Add sof_heap_alloc() and sof_heap_free() to allocate and free memory
on a private heap.
Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Memory, allocated using the module_driver_heap_*() API, should be
freed using module_driver_heap_free(), not rfree().
Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Allocate and free memory for module and device objects and for
input pins to separate functions to support varying allocation
methods.
Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Add a definition of struct k_heap and SOF k_heap wrappers
sof_heap_alloc() and sof_heap_free() for host native builds.
Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Add a new mod_alloc_ext() allocation function with support for
allocator flags.
Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
s/allocatd/allocated/
Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
@lyakh

lyakh commented Nov 5, 2025

Copy link
Copy Markdown
CollaboratorAuthor

SOFCI TEST

@lyakhlyakh changed the title Part 1 of #10302Part 1 of userspace DP subset PR #10302Nov 5, 2025
@lgirdwood
lgirdwood merged commit 9c32699 into thesofproject:mainNov 5, 2025
35 of 42 checks passed
@lyakh
lyakh deleted the user-p1_1 branch November 5, 2025 13:45
@kv2019i

Copy link
Copy Markdown
Collaborator

@lgirdwood@lyakh Hmm, you broke the doxygen CI run with this PR. Now we have this failing a required check in all PRs.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@lyakh@lgirdwood@kv2019i@softwarecki