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
+153 −84
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
deac1c8
pipeline: replace heap member with alloc context
Sep 3, 2026
42a09eb
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
169 changes: 101 additions & 68 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 @@ -75,26 +75,17 @@ 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)
struct mod_alloc_ctx *module_adapter_dp_alloc_ctx_new(const struct comp_driver *drv,
const struct comp_ipc_config *config,
const struct module_ext_init_data *ext_init,
uint32_t flags)
{
struct k_heap *mod_heap;
struct vregion *mod_vreg;
struct processing_module *mod;
struct comp_dev *dev;
/*
* For DP shared modules the struct processing_module object must be
* accessible from all cores. Unfortunately at this point there's no
* information of components the module will be bound to. So we need to
* allocate shared memory for each DP module.
* To be removed when pipeline 2.0 is ready.
*/
uint32_t flags = config->proc_domain == COMP_PROCESSING_DOMAIN_DP ?
SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT : SOF_MEM_FLAG_USER;
struct mod_alloc_ctx *alloc;

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)) {
if (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) {
comp_cl_err(drv, "Failed to allocate DP module heap / vregion");
Expand All @@ -114,27 +105,59 @@ struct processing_module *module_adapter_mem_alloc(const struct comp_driver *drv
#endif
mod_vreg = NULL;
}
alloc = sof_heap_alloc(mod_heap, flags, sizeof(*alloc), 0);
if (!alloc) {
comp_cl_err(drv, "sof_alloc_ctx allocation failed");
vregion_put(mod_vreg);
return NULL;
}

if (!mod_vreg)
mod = sof_heap_alloc(mod_heap, flags, sizeof(*mod), 0);
else if (flags & SOF_MEM_FLAG_COHERENT)
mod = vregion_alloc_coherent(mod_vreg, sizeof(*mod));
else
mod = vregion_alloc(mod_vreg, sizeof(*mod));
memset(alloc, 0, sizeof(*alloc));
alloc->heap = mod_heap;
alloc->vreg = mod_vreg;

return alloc;
}

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,
struct mod_alloc_ctx *ppl_alloc)
{
struct processing_module *mod;
struct mod_alloc_ctx *alloc;
struct comp_dev *dev;
/*
* For DP shared modules the struct processing_module object must be
* accessible from all cores. Unfortunately at this point there's no
* information of components the module will be bound to. So we need to
* allocate shared memory for each DP module.
* To be removed when pipeline 2.0 is ready.
*/
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_LL) {
/* LL modules share the pipeline's alloc context */
alloc = ppl_alloc;
vregion_get(alloc->vreg);
} else if (config->proc_domain == COMP_PROCESSING_DOMAIN_DP) {
alloc = module_adapter_dp_alloc_ctx_new(drv, config, ext_init, flags);
if (!alloc)
return NULL;

@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.

use goto edev instead, optionally add an error message

@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.

mod_vreg is not defined outside the else branch, but I'll add an error message.

@lyakh lyakh Sep 7, 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.

you could restore mod_vreg - that would reduce the diff too

} else {
comp_cl_err(drv, "bad proc_domain %d", config->proc_domain);
return NULL;
}

mod = sof_ctx_alloc(alloc, flags, sizeof(*mod), 0);
if (!mod) {
comp_cl_err(drv, "failed to allocate memory for module");
goto emod;
}

struct mod_alloc_ctx *alloc = sof_heap_alloc(mod_heap, flags, sizeof(*alloc), 0);

if (!alloc)
goto ealloc;

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

Expand All @@ -144,11 +167,7 @@ struct processing_module *module_adapter_mem_alloc(const struct comp_driver *drv
* then it can be cached. Effectively it can be only cached in
* single-core configurations.
*/
if (mod_vreg)
dev = vregion_alloc_coherent(mod_vreg, sizeof(*dev));
else
dev = sof_heap_alloc(mod_heap, SOF_MEM_FLAG_COHERENT, sizeof(*dev), 0);

dev = sof_ctx_alloc(alloc, SOF_MEM_FLAG_COHERENT, sizeof(*dev), 0);
if (!dev) {
comp_cl_err(drv, "failed to allocate memory for comp_dev");
goto edev;
Expand All @@ -163,41 +182,45 @@ struct processing_module *module_adapter_mem_alloc(const struct comp_driver *drv
return mod;

edev:
sof_heap_free(mod_heap, alloc);
ealloc:
if (mod_vreg)
vregion_free(mod_vreg, mod);
else
sof_heap_free(mod_heap, mod);
sof_ctx_free(alloc, mod);
emod:
vregion_put(mod_vreg);
vregion_put(alloc->vreg);
if (config->proc_domain == COMP_PROCESSING_DOMAIN_DP)
sof_heap_free(alloc->heap, alloc);

return NULL;
}

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 +271,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 +307,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 +355,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 %zu 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
Loading
Loading