Skip to content

Commit ea2b7a5

Browse files
committed
audio: buffer: move dp_heap_user lifecycle to IPC and module adapter
The mod_heap_user (dp_heap_user) reference count management in comp_buffer_free() does not belong there - a buffer's free method should not be responsible for freeing a module's private heap. Additionally, in CONFIG_SOF_USERSPACE_LL builds, comp_buffer_free() may run in user-space context where rfree() of kernel-allocated memory is not ok. - Add dp_heap_put() helper to dp_schedule.h for reference-counted DP heap release - Move buffer-side client_count decrements to IPC layer (ipc_comp_disconnect, ipc_pipeline_module_free, ipc_comp_connect error paths) - Move raw data buffer client_count management to module_adapter_free/prepare with DP domain guard - Fix module_adapter_mem_free() where CONFIG_SOF_USERSPACE_LL incorrectly reassigned mod_heap for DP modules - Fix client_count increment guard in ipc_comp_connect() to check dp pointer rather than dp_heap Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
1 parent fed1761 commit ea2b7a5

4 files changed

Lines changed: 80 additions & 19 deletions

File tree

src/audio/buffers/comp_buffer.c

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -157,12 +157,6 @@ static void comp_buffer_free(struct sof_audio_buffer *audio_buffer)
157157

158158
sof_heap_free(heap, buffer->stream.addr);
159159
sof_heap_free(heap, buffer);
160-
if (heap) {
161-
struct dp_heap_user *mod_heap_user = container_of(heap, struct dp_heap_user, heap);
162-
163-
if (!--mod_heap_user->client_count)
164-
rfree(mod_heap_user);
165-
}
166160
}
167161

168162
APP_TASK_DATA static const struct source_ops comp_buffer_source_ops = {

src/audio/module_adapter/module_adapter.c

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -179,18 +179,15 @@ static void module_adapter_mem_free(struct processing_module *mod)
179179
sof_heap_free(mod_heap, mod->dev);
180180
LOG_INF("mod");
181181
#ifdef CONFIG_SOF_USERSPACE_LL
182-
mod_heap = zephyr_ll_user_heap();
183-
comp_cl_dbg(drv, "using ll user heap for module free");
182+
if (domain != COMP_PROCESSING_DOMAIN_DP) {
183+
mod_heap = zephyr_ll_user_heap();
184+
comp_cl_dbg(drv, "using ll user heap for module free");
185+
}
184186
#endif
185187
comp_cl_info(drv, "free mod %p with heap %p", mod, mod_heap);
186188
sof_heap_free(mod_heap, mod);
187-
if (domain == COMP_PROCESSING_DOMAIN_DP) {
188-
struct dp_heap_user *mod_heap_user = container_of(mod_heap, struct dp_heap_user,
189-
heap);
190-
191-
if (mod_heap && !--mod_heap_user->client_count)
192-
rfree(mod_heap_user);
193-
}
189+
if (domain == COMP_PROCESSING_DOMAIN_DP && mod_heap)
190+
dp_heap_put(mod_heap);
194191
}
195192

196193
/*
@@ -641,7 +638,8 @@ int module_adapter_prepare(struct comp_dev *dev)
641638
goto free;
642639
}
643640

644-
if (md->resources.heap && md->resources.heap != dev->drv->user_heap) {
641+
if (dev->ipc_config.proc_domain == COMP_PROCESSING_DOMAIN_DP &&
642+
md->resources.heap) {
645643
struct dp_heap_user *dp_user = container_of(md->resources.heap,
646644
struct dp_heap_user,
647645
heap);
@@ -687,6 +685,9 @@ int module_adapter_prepare(struct comp_dev *dev)
687685
list_item_del(&buffer->buffers_list);
688686
irq_local_enable(flags);
689687
buffer_free(buffer);
688+
if (dev->ipc_config.proc_domain == COMP_PROCESSING_DOMAIN_DP &&
689+
md->resources.heap)
690+
dp_heap_put(md->resources.heap);
690691
}
691692

692693
out_data_free:
@@ -1479,6 +1480,9 @@ void module_adapter_free(struct comp_dev *dev)
14791480
list_item_del(&buffer->buffers_list);
14801481
irq_local_enable(flags);
14811482
buffer_free(buffer);
1483+
if (dev->ipc_config.proc_domain == COMP_PROCESSING_DOMAIN_DP &&
1484+
mod->priv.resources.heap)
1485+
dp_heap_put(mod->priv.resources.heap);
14821486
}
14831487

14841488
mod_free(mod, mod->stream_params);

src/include/sof/schedule/dp_schedule.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#ifndef __SOF_SCHEDULE_DP_SCHEDULE_H__
99
#define __SOF_SCHEDULE_DP_SCHEDULE_H__
1010

11+
#include <rtos/alloc.h>
1112
#include <rtos/task.h>
1213
#include <sof/trace/trace.h>
1314
#include <user/trace.h>
@@ -125,6 +126,22 @@ struct dp_heap_user {
125126
unsigned int client_count; /* devices and buffers */
126127
};
127128

129+
/**
130+
* dp_heap_put() - Release a reference to a DP module heap.
131+
* @heap: The k_heap pointer belonging to a dp_heap_user.
132+
*
133+
* Decrements client_count and frees the dp_heap_user when it reaches zero.
134+
* Must only be called for heaps that are part of a dp_heap_user, i.e. heaps
135+
* allocated by module_adapter_dp_heap_new() for DP domain modules.
136+
*/
137+
static inline void dp_heap_put(struct k_heap *heap)
138+
{
139+
struct dp_heap_user *mod_heap_user = container_of(heap, struct dp_heap_user, heap);
140+
141+
if (!--mod_heap_user->client_count)
142+
rfree(mod_heap_user);
143+
}
144+
128145
#if CONFIG_ZEPHYR_DP_SCHEDULER
129146
int scheduler_dp_thread_ipc(struct processing_module *pmod, unsigned int cmd,
130147
const union scheduler_dp_thread_ipc_param *param);

src/ipc/ipc4/helper.c

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -457,22 +457,51 @@ __cold static int ipc_pipeline_module_free(uint32_t pipeline_id)
457457

458458
/* free sink buffer allocated by current component in bind function */
459459
comp_dev_for_each_consumer_safe(icd->cd, buffer, safe) {
460+
#if CONFIG_ZEPHYR_DP_SCHEDULER
461+
struct k_heap *buf_heap = buffer->audio_buffer.heap;
462+
struct comp_dev *orig_sink = comp_buffer_get_sink_component(buffer);
463+
bool buf_is_dp = buf_heap &&
464+
(icd->cd->ipc_config.proc_domain == COMP_PROCESSING_DOMAIN_DP ||
465+
(orig_sink && orig_sink->ipc_config.proc_domain ==
466+
COMP_PROCESSING_DOMAIN_DP));
467+
#endif
468+
460469
pipeline_disconnect(icd->cd, buffer, PPL_CONN_DIR_COMP_TO_BUFFER);
461470
struct comp_dev *sink = comp_buffer_get_sink_component(buffer);
462471

463472
/* free the buffer only when the sink module has also been disconnected */
464-
if (!sink)
473+
if (!sink) {
465474
buffer_free(buffer);
475+
#if CONFIG_ZEPHYR_DP_SCHEDULER
476+
if (buf_is_dp)
477+
dp_heap_put(buf_heap);
478+
#endif
479+
}
466480
}
467481

468482
/* free source buffer allocated by current component in bind function */
469483
comp_dev_for_each_producer_safe(icd->cd, buffer, safe) {
484+
#if CONFIG_ZEPHYR_DP_SCHEDULER
485+
struct k_heap *buf_heap = buffer->audio_buffer.heap;
486+
struct comp_dev *orig_source =
487+
comp_buffer_get_source_component(buffer);
488+
bool buf_is_dp = buf_heap &&
489+
(icd->cd->ipc_config.proc_domain == COMP_PROCESSING_DOMAIN_DP ||
490+
(orig_source && orig_source->ipc_config.proc_domain ==
491+
COMP_PROCESSING_DOMAIN_DP));
492+
#endif
493+
470494
pipeline_disconnect(icd->cd, buffer, PPL_CONN_DIR_BUFFER_TO_COMP);
471495
struct comp_dev *source = comp_buffer_get_source_component(buffer);
472496

473497
/* free the buffer only when the source module has also been disconnected */
474-
if (!source)
498+
if (!source) {
475499
buffer_free(buffer);
500+
#if CONFIG_ZEPHYR_DP_SCHEDULER
501+
if (buf_is_dp)
502+
dp_heap_put(buf_heap);
503+
#endif
504+
}
476505
}
477506

478507
if (!cpu_is_me(icd->core))
@@ -738,7 +767,7 @@ __cold int ipc_comp_connect(struct ipc *ipc, ipc_pipe_comp_connect *_connect)
738767
}
739768

740769
#if CONFIG_ZEPHYR_DP_SCHEDULER
741-
if (dp_heap) {
770+
if (dp) {
742771
struct dp_heap_user *dp_user = container_of(dp_heap, struct dp_heap_user, heap);
743772

744773
dp_user->client_count++;
@@ -782,6 +811,8 @@ __cold int ipc_comp_connect(struct ipc *ipc, ipc_pipe_comp_connect *_connect)
782811
buf_get_id(buffer));
783812
if (!ring_buffer) {
784813
buffer_free(buffer);
814+
if (dp)
815+
dp_heap_put(dp_heap);
785816
return IPC4_OUT_OF_MEMORY;
786817
}
787818

@@ -869,6 +900,10 @@ __cold int ipc_comp_connect(struct ipc *ipc, ipc_pipe_comp_connect *_connect)
869900
free:
870901
ll_unblock(cross_core_bind, flags);
871902
buffer_free(buffer);
903+
#if CONFIG_ZEPHYR_DP_SCHEDULER
904+
if (dp)
905+
dp_heap_put(dp_heap);
906+
#endif
872907
return IPC4_INVALID_RESOURCE_STATE;
873908
}
874909

@@ -950,6 +985,13 @@ __cold int ipc_comp_disconnect(struct ipc *ipc, ipc_pipe_comp_connect *_connect)
950985
#endif
951986
}
952987

988+
#if CONFIG_ZEPHYR_DP_SCHEDULER
989+
struct k_heap *buf_heap = buffer->audio_buffer.heap;
990+
bool buf_is_dp = buf_heap &&
991+
(src->ipc_config.proc_domain == COMP_PROCESSING_DOMAIN_DP ||
992+
sink->ipc_config.proc_domain == COMP_PROCESSING_DOMAIN_DP);
993+
#endif
994+
953995
pipeline_disconnect(src, buffer, PPL_CONN_DIR_COMP_TO_BUFFER);
954996
pipeline_disconnect(sink, buffer, PPL_CONN_DIR_BUFFER_TO_COMP);
955997
/* these might call comp_ipc4_bind_remote() if necessary */
@@ -965,6 +1007,10 @@ __cold int ipc_comp_disconnect(struct ipc *ipc, ipc_pipe_comp_connect *_connect)
9651007
ll_unblock(cross_core_unbind, flags);
9661008

9671009
buffer_free(buffer);
1010+
#if CONFIG_ZEPHYR_DP_SCHEDULER
1011+
if (buf_is_dp)
1012+
dp_heap_put(buf_heap);
1013+
#endif
9681014

9691015
if (ret || ret1)
9701016
return IPC4_INVALID_RESOURCE_ID;

0 commit comments

Comments
 (0)