-
Notifications
You must be signed in to change notification settings - Fork 354
buffer: extend ability to allocate on specific heap to all functions #10719
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -156,7 +156,16 @@ static void comp_buffer_free(struct sof_audio_buffer *audio_buffer) | |||||||||
|
|
||||||||||
| struct mod_alloc_ctx *alloc = buffer->audio_buffer.alloc; | ||||||||||
|
|
||||||||||
| #ifdef CONFIG_SOF_USERSPACE_LL | ||||||||||
| assert(alloc); | ||||||||||
| if (alloc->vreg) | ||||||||||
| vregion_free(alloc->vreg, buffer->stream.addr); | ||||||||||
| else | ||||||||||
| sof_heap_free(alloc->heap, buffer->stream.addr); | ||||||||||
| #else | ||||||||||
| rfree(buffer->stream.addr); | ||||||||||
| #endif | ||||||||||
|
|
||||||||||
| if (alloc && alloc->vreg) { | ||||||||||
| vregion_free(alloc->vreg, buffer); | ||||||||||
| if (!vregion_put(alloc->vreg)) | ||||||||||
|
|
@@ -254,7 +263,15 @@ struct comp_buffer *buffer_alloc(struct mod_alloc_ctx *alloc, size_t size, uint3 | |||||||||
| return NULL; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| #ifdef CONFIG_SOF_USERSPACE_LL | ||||||||||
| assert(alloc); | ||||||||||
| if (alloc->vreg) | ||||||||||
| stream_addr = vregion_alloc_align(alloc->vreg, VREGION_MEM_TYPE_INTERIM, size, align); | ||||||||||
| else | ||||||||||
| stream_addr = sof_heap_alloc(alloc->heap, flags, size, align); | ||||||||||
| #else | ||||||||||
| stream_addr = rballoc_align(flags, size, align); | ||||||||||
| #endif | ||||||||||
| if (!stream_addr) { | ||||||||||
| tr_err(&buffer_tr, "could not alloc size = %zu bytes of flags = 0x%x", | ||||||||||
| size, flags); | ||||||||||
|
|
@@ -264,7 +281,15 @@ struct comp_buffer *buffer_alloc(struct mod_alloc_ctx *alloc, size_t size, uint3 | |||||||||
| buffer = buffer_alloc_struct(alloc, stream_addr, size, flags, is_shared); | ||||||||||
| if (!buffer) { | ||||||||||
| tr_err(&buffer_tr, "could not alloc buffer structure"); | ||||||||||
| #ifdef CONFIG_SOF_USERSPACE_LL | ||||||||||
| assert(alloc); | ||||||||||
| if (alloc->vreg) | ||||||||||
| vregion_free(alloc->vreg, stream_addr); | ||||||||||
| else | ||||||||||
| sof_heap_free(alloc->heap, stream_addr); | ||||||||||
| #else | ||||||||||
| rfree(stream_addr); | ||||||||||
| #endif | ||||||||||
| } | ||||||||||
|
|
||||||||||
| return buffer; | ||||||||||
|
|
@@ -292,7 +317,15 @@ struct comp_buffer *buffer_alloc_range(struct mod_alloc_ctx *alloc, size_t prefe | |||||||||
| preferred_size += minimum_size - preferred_size % minimum_size; | ||||||||||
|
|
||||||||||
| for (size = preferred_size; size >= minimum_size; size -= minimum_size) { | ||||||||||
| #ifdef CONFIG_SOF_USERSPACE_LL | ||||||||||
| assert(alloc); | ||||||||||
| if (alloc->vreg) | ||||||||||
| stream_addr = vregion_alloc_align(alloc->vreg, VREGION_MEM_TYPE_INTERIM, size, align); | ||||||||||
| else | ||||||||||
| stream_addr = sof_heap_alloc(alloc->heap, flags, size, align); | ||||||||||
| #else | ||||||||||
| stream_addr = rballoc_align(flags, size, align); | ||||||||||
| #endif | ||||||||||
| if (stream_addr) | ||||||||||
| break; | ||||||||||
| } | ||||||||||
|
|
@@ -308,7 +341,15 @@ struct comp_buffer *buffer_alloc_range(struct mod_alloc_ctx *alloc, size_t prefe | |||||||||
| buffer = buffer_alloc_struct(alloc, stream_addr, size, flags, is_shared); | ||||||||||
| if (!buffer) { | ||||||||||
| tr_err(&buffer_tr, "could not alloc buffer structure"); | ||||||||||
| #ifdef CONFIG_SOF_USERSPACE_LL | ||||||||||
| assert(alloc); | ||||||||||
| if (alloc->vreg) | ||||||||||
| vregion_free(alloc->vreg, stream_addr); | ||||||||||
| else | ||||||||||
| sof_heap_free(alloc->heap, stream_addr); | ||||||||||
| #else | ||||||||||
| rfree(stream_addr); | ||||||||||
| #endif | ||||||||||
| } | ||||||||||
|
|
||||||||||
| return buffer; | ||||||||||
|
|
@@ -329,6 +370,9 @@ void buffer_zero(struct comp_buffer *buffer) | |||||||||
| int buffer_set_size(struct comp_buffer *buffer, uint32_t size, uint32_t alignment) | ||||||||||
| { | ||||||||||
| void *new_ptr = NULL; | ||||||||||
| #ifdef CONFIG_SOF_USERSPACE_LL | ||||||||||
| struct mod_alloc_ctx *alloc = buffer->audio_buffer.alloc; | ||||||||||
| #endif | ||||||||||
|
|
||||||||||
| CORE_CHECK_STRUCT(&buffer->audio_buffer); | ||||||||||
|
|
||||||||||
|
|
@@ -341,14 +385,16 @@ int buffer_set_size(struct comp_buffer *buffer, uint32_t size, uint32_t alignmen | |||||||||
| if (size == audio_stream_get_size(&buffer->stream)) | ||||||||||
| return 0; | ||||||||||
|
|
||||||||||
|
||||||||||
| if (!alignment) | |
| alignment = PLATFORM_DCACHE_ALIGN; |
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.
The same semantics are now implemented by sof_heap_alloc() so 0 can be passed forward to it.
Copilot
AI
Apr 23, 2026
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.
buffer_set_size_range(): the resize loop decrements new_size until it drops below minimum_size. If all allocations fail, new_ptr stays NULL and new_size will end up < minimum_size (often 0), but the function later calls buffer_init_stream(buffer, new_size). That can set an invalid size despite the earlier validation. Consider handling the “no allocation succeeded” case explicitly: for grow requests return -ENOMEM without changing the stream; for shrink requests skip allocating a new block and just set the stream size to the chosen target (>= minimum_size).
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.
Fixed in V2.
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.
@kv2019i I'm thinking whether
flagscan containSOF_MEM_FLAG_COHERENT. I've so far been unable to find any such paths and IIUC the only possible source of "implicit" flags would be those, coming over IPC fromconst struct sof_ipc_buffer *descinbuffer_new(), but those seem to only be allowed to containSOF_BUF_*RUN_PERMITTEDflags. So, looks like this is correct and under no circumstances would we need to callvregion_alloc_coherent_align(). Maybe a comment?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.
Ack @lyakh , this is kind forced retirement of the flags. But it does make sense... we shouldn't have logic in user-space depending on coherent access. Synchronization, talking to hw, these should all go via a kernel/driver interface.