Skip to content

Per pipeline vregions/mod_alloc_ctx #11164

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.

Sign up for GitHub

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

Open
jsarha wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Per pipeline vregions/mod_alloc_ctx #11164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd6bffd
pipeline: replace heap member with alloc context
Sep 3, 2026
af084f9
pipeline: allocate shared vregion for LL modules in the pipeline
Sep 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 65 additions & 35 deletions src/audio/module_adapter/module_adapter.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,15 @@ static struct vregion *module_adapter_dp_heap_new(const struct comp_ipc_config *
static
struct processing_module *module_adapter_mem_alloc(const struct comp_driver *drv,
const struct comp_ipc_config *config,
const struct module_ext_init_data *ext_init)
const struct module_ext_init_data *ext_init,
struct mod_alloc_ctx *ppl_alloc)
{
struct k_heap *mod_heap;
struct vregion *mod_vreg;
struct processing_module *mod;
struct mod_alloc_ctx *alloc;
struct comp_dev *dev;
bool use_ppl_alloc = ppl_alloc && config->proc_domain == COMP_PROCESSING_DOMAIN_LL;
/*
* For DP shared modules the struct processing_module object must be
* accessible from all cores. Unfortunately at this point there's no
Expand All @@ -93,7 +96,12 @@ struct processing_module *module_adapter_mem_alloc(const struct comp_driver *drv
uint32_t flags = config->proc_domain == COMP_PROCESSING_DOMAIN_DP ?
SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT : SOF_MEM_FLAG_USER;

if (config->proc_domain == COMP_PROCESSING_DOMAIN_DP && IS_ENABLED(CONFIG_SOF_VREGIONS) &&
if (use_ppl_alloc) {
/* LL modules share the pipeline's alloc context */
mod_heap = ppl_alloc->heap;
mod_vreg = ppl_alloc->vreg;
vregion_get(mod_vreg);
} else if (config->proc_domain == COMP_PROCESSING_DOMAIN_DP && IS_ENABLED(CONFIG_SOF_VREGIONS) &&
IS_ENABLED(CONFIG_USERSPACE) && !IS_ENABLED(CONFIG_SOF_USERSPACE_USE_DRIVER_HEAP)) {
mod_vreg = module_adapter_dp_heap_new(config, ext_init);
if (!mod_vreg) {
Expand Down Expand Up @@ -127,14 +135,18 @@ struct processing_module *module_adapter_mem_alloc(const struct comp_driver *drv
goto emod;
}

struct mod_alloc_ctx *alloc = sof_heap_alloc(mod_heap, flags, sizeof(*alloc), 0);
if (use_ppl_alloc) {
alloc = ppl_alloc;
} else {
alloc = sof_heap_alloc(mod_heap, flags, sizeof(*alloc), 0);
if (!alloc)
goto ealloc;

if (!alloc)
goto ealloc;
alloc->heap = mod_heap;
alloc->vreg = mod_vreg;
}

memset(mod, 0, sizeof(*mod));
alloc->heap = mod_heap;
alloc->vreg = mod_vreg;
mod->priv.resources.alloc = alloc;
mod_resource_init(mod);

Expand Down Expand Up @@ -163,7 +175,8 @@ struct processing_module *module_adapter_mem_alloc(const struct comp_driver *drv
return mod;

edev:
sof_heap_free(mod_heap, alloc);
if (!use_ppl_alloc)
sof_heap_free(mod_heap, alloc);
ealloc:
if (mod_vreg)
vregion_free(mod_vreg, mod);
Expand All @@ -178,26 +191,33 @@ struct processing_module *module_adapter_mem_alloc(const struct comp_driver *drv
static void module_adapter_mem_free(struct processing_module *mod)
{
struct mod_alloc_ctx *alloc = mod->priv.resources.alloc;
struct k_heap *mod_heap = alloc->heap;
bool ppl_alloc = mod->dev->ipc_config.proc_domain == COMP_PROCESSING_DOMAIN_LL &&
mod->dev->pipeline && mod->dev->pipeline->alloc == alloc;
Comment thread
jsarha marked this conversation as resolved.

/*
* In principle it shouldn't even be needed to free individual objects
* on the module heap since we're freeing the heap itself too
*/
#if CONFIG_IPC_MAJOR_4
sof_heap_free(mod_heap, mod->priv.cfg.input_pins);
sof_heap_free(alloc->heap, mod->priv.cfg.input_pins);
#endif
if (alloc->vreg) {
struct vregion *mod_vreg = alloc->vreg;
sof_ctx_free(alloc, mod->dev);
sof_ctx_free(alloc, mod);

vregion_free(mod_vreg, mod->dev);
vregion_free(mod_vreg, mod);
if (!vregion_put(mod_vreg))
if (ppl_alloc) {
/* alloc belongs to pipeline, just release vregion reference */
vregion_put(alloc->vreg);
} else if (alloc->vreg) {
/*
* This is DP userpsace case
* Only remove the alloc ctx, if vreg was freed. If it was not
* the DP userspace thread is still holding a reference to it,
* and will free alloc ctx eventually.
*/
if (!vregion_put(alloc->vreg))
sof_heap_free(alloc->heap, alloc);
} else {
sof_heap_free(mod_heap, mod->dev);
sof_heap_free(mod_heap, mod);
sof_heap_free(mod_heap, alloc);
sof_heap_free(alloc->heap, alloc);
}
}

Expand Down Expand Up @@ -248,8 +268,19 @@ struct comp_dev *module_adapter_new_ext(const struct comp_driver *drv,
NULL;
#endif

struct processing_module *mod = module_adapter_mem_alloc(drv, config, ext_init);
struct mod_alloc_ctx *ppl_alloc = NULL;
#if CONFIG_IPC_MAJOR_4
struct ipc_comp_dev *ipc_pipe;
struct ipc *ipc = ipc_get();

/* resolve the pipeline pointer early to pass its alloc to mem_alloc */
ipc_pipe = ipc_get_comp_by_ppl_id(ipc, COMP_TYPE_PIPELINE, config->pipeline_id,
IPC_COMP_IGNORE_REMOTE);
if (ipc_pipe && ipc_pipe->pipeline)
ppl_alloc = ipc_pipe->pipeline->alloc;

@lyakh lyakh Sep 4, 2026

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.

is it actually valid to have ipc_pipe == NULL? And why do we ignore remote pipelines? I suppose remote pipelines are only possible with DP, is that the intention here? But DP on the same core we accept. I think you want IPC_COMP_ALL here. And ipc_pipe shouldn't be NULL even for chain DMA, right? So if it's NULL, it's an error? And ipc_pipe->pipeline should be non-NULL too? Also the allocation context must be there too. So in practice ppl_alloc is always non-NULL?

@jsarha jsarha Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That is now indeed true.

@jsarha jsarha Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is only a code block move, to have dev->pipeline resolved early, so that we can use module_adapter_mem_free() in the error branches later in this function. module_adapter_mem_free() currently depends in dev->pipeline being set. IPC_COMP_IGNORE_REMOTE was there before this change.

#endif

struct processing_module *mod = module_adapter_mem_alloc(drv, config, ext_init, ppl_alloc);
if (!mod)
return NULL;

Expand All @@ -273,6 +304,21 @@ struct comp_dev *module_adapter_new_ext(const struct comp_driver *drv,
dst->ext_data = &ext_data;
#endif

#if CONFIG_IPC_MAJOR_4
/*
* Set the pipeline pointer if ipc_pipe is valid. Do this
* early so that we can use module_adapter_mem_free() in error
* handling.
*/
if (ipc_pipe) {
dev->pipeline = ipc_pipe->pipeline;

/* LL modules have the same period as the pipeline */
if (dev->ipc_config.proc_domain == COMP_PROCESSING_DOMAIN_LL)
dev->period = ipc_pipe->pipeline->period;
}
#endif

#if CONFIG_ZEPHYR_DP_SCHEDULER
/* create a task for DP processing */
if (config->proc_domain == COMP_PROCESSING_DOMAIN_DP) {
Expand Down Expand Up @@ -306,22 +352,6 @@ struct comp_dev *module_adapter_new_ext(const struct comp_driver *drv,
else
goto err;

#if CONFIG_IPC_MAJOR_4
struct ipc_comp_dev *ipc_pipe;
struct ipc *ipc = ipc_get();

/* set the pipeline pointer if ipc_pipe is valid */
ipc_pipe = ipc_get_comp_by_ppl_id(ipc, COMP_TYPE_PIPELINE, config->pipeline_id,
IPC_COMP_IGNORE_REMOTE);
if (ipc_pipe) {
dev->pipeline = ipc_pipe->pipeline;

/* LL modules have the same period as the pipeline */
if (dev->ipc_config.proc_domain == COMP_PROCESSING_DOMAIN_LL)
dev->period = ipc_pipe->pipeline->period;
}
#endif

/* Init processing module */
ret = module_init(mod);
if (ret) {
Expand Down
49 changes: 42 additions & 7 deletions src/audio/pipeline/pipeline-graph.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <ipc/stream.h>
#include <ipc/topology.h>
#include <ipc4/module.h>
#include <ipc4/pipeline.h>
#include <errno.h>
#include <stdbool.h>
#include <stddef.h>
Expand Down Expand Up @@ -174,6 +175,7 @@ void pipeline_posn_grant_access(struct k_thread *thread)
struct pipeline *pipeline_new(struct k_heap *heap, uint32_t pipeline_id, uint32_t priority,
uint32_t comp_id, struct create_pipeline_params *pparams)
{
struct mod_alloc_ctx *alloc;
struct sof_ipc_stream_posn posn;
struct pipeline *p;
int ret;
Expand All @@ -184,17 +186,36 @@ struct pipeline *pipeline_new(struct k_heap *heap, uint32_t pipeline_id, uint32_
/* show heap status */
heap_trace_all(0);

alloc = sof_heap_alloc(heap, SOF_MEM_FLAG_USER, sizeof(*alloc), 0);

@lyakh lyakh Sep 4, 2026

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.

I think in the original version alloc was allocated using rmalloc(), i.e. only accessible to the kernel. I think that this version is correct and the previous one would have problems with userspace LL. Could you confirm?

@jsarha jsarha Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, that is why I changed it.

if (!alloc) {
pipe_cl_err("Failed to allocate pipeline alloc context");
return NULL;
}

memset(alloc, 0, sizeof(*alloc));
alloc->heap = heap;

/* Create vregion for pipeline and its modules if size info is available */
if (IS_ENABLED(CONFIG_SOF_VREGIONS) &&
pparams && pparams->mem_data && pparams->mem_data->heap_bytes) {
size_t buf_size = pparams->mem_data->heap_bytes;
uintptr_t vreg_start;

alloc->vreg = vregion_create_map(&vreg_start, &buf_size);
if (!alloc->vreg)
pipe_cl_err("Failed to create pipeline vregion of %u bytes, using heap",
pparams->mem_data->heap_bytes);
}

/* allocate new pipeline */
p = sof_heap_alloc(heap, SOF_MEM_FLAG_USER, sizeof(*p), 0);
p = sof_ctx_zalloc(alloc, SOF_MEM_FLAG_USER, sizeof(*p), 0);
if (!p) {
pipe_cl_err("Out of Memory");
return NULL;
goto free_alloc;
}

memset(p, 0, sizeof(*p));

/* init pipeline */
p->heap = heap;
p->alloc = alloc;
Comment thread
jsarha marked this conversation as resolved.
p->comp_id = comp_id;
p->priority = priority;
p->pipeline_id = pipeline_id;
Expand Down Expand Up @@ -236,7 +257,10 @@ struct pipeline *pipeline_new(struct k_heap *heap, uint32_t pipeline_id, uint32_

return p;
free:
sof_heap_free(heap, p);
sof_ctx_free(alloc, p);
free_alloc:
vregion_put(alloc->vreg);
sof_heap_free(heap, alloc);
return NULL;
}

Expand Down Expand Up @@ -321,6 +345,8 @@ void pipeline_disconnect(struct comp_dev *comp, struct comp_buffer *buffer, int
/* pipelines must be inactive */
int pipeline_free(struct pipeline *p)
{
struct mod_alloc_ctx *alloc = p->alloc;

pipe_dbg(p, "entry");

/*
Expand All @@ -336,7 +362,12 @@ int pipeline_free(struct pipeline *p)
pipeline_posn_offset_put(p->posn_offset);

/* now free the pipeline */
sof_heap_free(p->heap, p);
sof_ctx_free(alloc, p);

/* free alloc context and vregion */
if (vregion_put(alloc->vreg))
pipe_cl_warn("pipeline vregion still in use");
sof_heap_free(alloc->heap, alloc);

/* show heap status */
heap_trace_all(0);
Expand Down Expand Up @@ -413,6 +444,10 @@ int pipeline_complete(struct pipeline *p, struct comp_dev *source,

p->source_comp = source;
p->sink_comp = sink;

if (p->alloc->vreg)
vregion_set_interim(p->alloc->vreg);

p->status = COMP_STATE_READY;

/* show heap status */
Expand Down
14 changes: 7 additions & 7 deletions src/audio/pipeline/pipeline-schedule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ static struct task *ipc4_pipeline_trigger_task_init(struct pipeline *p, uint32_t
{
struct task *task;

task = sof_heap_alloc(p->heap, SOF_MEM_FLAG_USER, sizeof(*task), 0);
task = sof_ctx_alloc(p->alloc, SOF_MEM_FLAG_USER, sizeof(*task), 0);
if (!task)
return NULL;

Expand All @@ -358,7 +358,7 @@ static struct task *ipc4_pipeline_trigger_task_init(struct pipeline *p, uint32_t
/* All trigger tasks use the highest priority, regardless of pipeline priority. */
if (schedule_task_init_ll(task, SOF_UUID(pipe_trigger_task_uuid), type, -1,
ipc4_pipeline_trigger_task, p, p->core, 0) < 0) {
sof_heap_free(p->heap, task);
sof_ctx_free(p->alloc, task);
return NULL;
}

Expand All @@ -370,8 +370,8 @@ static struct task *pipeline_task_init(struct pipeline *p, uint32_t type)
{
struct pipeline_task *task = NULL;

task = sof_heap_alloc(p->heap, SOF_MEM_FLAG_USER,
sizeof(*task), 0);
task = sof_ctx_alloc(p->alloc, SOF_MEM_FLAG_USER,
sizeof(*task), 0);
if (!task)
return NULL;

Expand All @@ -385,7 +385,7 @@ static struct task *pipeline_task_init(struct pipeline *p, uint32_t type)
ipc3_pipeline_task,
#endif
p, p->core, 0) < 0) {
sof_heap_free(p->heap, task);
sof_ctx_free(p->alloc, task);
return NULL;
}

Expand Down Expand Up @@ -562,14 +562,14 @@ void pipeline_comp_ll_task_free(struct pipeline *p)
delayed_trigger_owner[p->core] = NULL;

if (p->trigger_task)
sof_heap_free(p->heap, p->trigger_task);
sof_ctx_free(p->alloc, p->trigger_task);
#endif

if (p->pipe_task) {
#if !CONFIG_LIBRARY || UNIT_TEST
schedule_task_free(p->pipe_task);
#endif
sof_heap_free(p->heap, p->pipe_task);
sof_ctx_free(p->alloc, p->pipe_task);
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/include/sof/audio/pipeline.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ struct comp_dev;
struct ipc;
struct ipc_msg;
struct k_heap;
struct mod_alloc_ctx;

/*
* Pipeline status to stop execution of current path, but to keep the
Expand Down Expand Up @@ -53,7 +54,7 @@ struct k_heap;
* Audio pipeline.
*/
struct pipeline {
struct k_heap *heap; /**< heap used for allocating this pipeline */
struct mod_alloc_ctx *alloc; /**< alloc context used for allocating this pipeline */
uint32_t comp_id; /**< component id for pipeline */
uint32_t pipeline_id; /**< pipeline id */
uint32_t sched_id; /**< Scheduling component id */
Expand Down
3 changes: 3 additions & 0 deletions test/cmocka/src/audio/pipeline/pipeline_connection_mocks.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// Author: Jakub Dabek <jakub.dabek@linux.intel.com>

#include <sof/audio/ipc-config.h>
#include <rtos/alloc.h>
#include "pipeline_connection_mocks.h"

extern struct schedulers *schedulers;
Expand Down Expand Up @@ -32,6 +33,7 @@ struct pipeline_connect_data *get_standard_connect_objects(void)

struct pipeline *pipe = &pipeline_connect_data->p;

pipe->alloc = calloc(sizeof(*pipe->alloc), 1);
pipe->frames_per_sched = 5;
pipe->pipeline_id = PIPELINE_ID_SAME;
pipe->status = COMP_STATE_INIT;
Expand Down Expand Up @@ -91,6 +93,7 @@ struct pipeline_connect_data *get_standard_connect_objects(void)

void free_standard_connect_objects(struct pipeline_connect_data *data)
{
free(data->p.alloc);
free(data->p.pipe_task);
free(data->p.sched_comp);
free(data->second);
Expand Down
2 changes: 1 addition & 1 deletion zephyr/test/userspace/test_ll_task.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ static void pipeline_check(void)
zassert_not_null(p, "pipeline creation failed");

/* Verify heap assignment */
zassert_equal(p->heap, heap, "pipeline heap not equal to user heap");
zassert_equal(p->alloc->heap, heap, "pipeline heap not equal to user heap");

/* Verify pipeline properties */
zassert_equal(p->pipeline_id, pipeline_id, "pipeline id mismatch");
Expand Down
Loading