Skip to content

src: audio: module_adapter: Do the params config right after init #10094

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

Merged
kv2019i merged 1 commit into from
Jul 29, 2025
Merged

src: audio: module_adapter: Do the params config right after init #10094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 23 additions & 0 deletions src/audio/kpb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,8 @@ static void kpb_set_params(struct comp_dev *dev,
{}
#endif /* CONFIG_IPC_MAJOR_4 */

static int kpb_params(struct comp_dev *dev, struct sof_ipc_stream_params *params);

/*
* \brief Create a key phrase buffer component.
* \param[in] config - generic ipc component pointer.
Expand Down Expand Up @@ -534,6 +536,17 @@ static struct comp_dev *kpb_new(const struct comp_driver *drv,
dev->state = COMP_STATE_READY;
kpb_change_state(kpb, KPB_STATE_CREATED);

#if CONFIG_IPC_MAJOR_4
struct sof_ipc_stream_params params;

/* retrieve params from the base config for IPC4 */
ret = kpb_params(dev, &params);
if (ret < 0) {
rfree(dev);
return NULL;
}
#endif

return dev;
}

Expand Down Expand Up @@ -784,12 +797,22 @@ static int kpb_params(struct comp_dev *dev,
static int kpb_prepare(struct comp_dev *dev)
{
struct comp_data *kpb = comp_get_drvdata(dev);
struct sof_ipc_stream_params params;
int ret = 0;
int i;
size_t hb_size_req = KPB_MAX_BUFFER_SIZE(kpb->config.sampling_width, kpb->config.channels);

comp_dbg(dev, "kpb_prepare()");

/* retrieve the params from the base_cfg and update the source/sink buffer params */
kpb_set_params(dev, &params);

ret = kpb_verify_params(dev, &params);
if (ret < 0) {
comp_err(dev, "pcm params verification failed");
return -EINVAL;
}

if (kpb->state == KPB_STATE_RESETTING ||
kpb->state == KPB_STATE_RESET_FINISHING) {
comp_cl_err(&comp_kpb, "can not prepare KPB due to ongoing reset, state log %x",
Expand Down
35 changes: 35 additions & 0 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 @@ -123,6 +123,20 @@ struct comp_dev *module_adapter_new(const struct comp_driver *drv,

dev->state = COMP_STATE_READY;

#if CONFIG_IPC_MAJOR_4
struct sof_ipc_stream_params params;

/*
* retrieve the stream params based on the module base cfg. There's no need to initialize
* params here because it will be filled in based on the module base_cfg.
*/
ret = module_adapter_params(dev, &params);
if (ret) {
comp_err(dev, "module_adapter_new() %d: module params failed", ret);
goto err;
}
#endif

comp_dbg(dev, "module_adapter_new() done");
return dev;
err:
Expand Down Expand Up @@ -179,7 +193,28 @@ int module_adapter_prepare(struct comp_dev *dev)
int i = 0;

comp_dbg(dev, "module_adapter_prepare() start");
#if CONFIG_IPC_MAJOR_4
/*
* if the stream_params are valid, just update the sink/source buffer params. If not,
* retrieve the params from the basecfg, allocate stream_params and then update the
* sink/source buffer params.
*/
if (!mod->stream_params) {
Copy link
Collaborator

@lyakh lyakh Jul 22, 2025

Choose a reason for hiding this comment

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

this doesn't seem possible any more? It's now allocated in .new()

Copy link
Collaborator Author

@ranj063 ranj063 Jul 24, 2025

Choose a reason for hiding this comment

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

No it is possible when you restart a stream after a pipeline reset without freeing it

Copy link
Collaborator

@lyakh lyakh Jul 24, 2025

Choose a reason for hiding this comment

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

@ranj063 interesting. Could you explain a bit more just for my understanding - "restart" a stream - you don't mean stop -> start, just a reset, right? But when do such "resets" happen apart from when stopping streams?

Copy link
Collaborator Author

@ranj063 ranj063 Jul 24, 2025

Choose a reason for hiding this comment

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

@lyakh it never happens in our tests. But there's a QB test that does this sequence (it called randomDMAtest or something like that). I added this specifically to make that test pass:
set up pipelines -> start pipelines -> stop pipelines - >reset pipelines -> Put the DSP in D3 -> exit D3 -> prepare & restart pipelines -> stop pipelines -> reset pipelines -> free pipelines

struct sof_ipc_stream_params params;

ret = module_adapter_params(dev, &params);
if (ret) {
comp_err(dev, "module_adapter_new() %d: module params failed", ret);
return ret;
}
} else {
ret = comp_verify_params(dev, mod->verify_params_flags, mod->stream_params);
if (ret < 0) {
comp_err(dev, "module_adapter_params(): comp_verify_params() failed.");
return ret;
}
}
#endif
/* Prepare module */
if (IS_PROCESSING_MODE_SINK_SOURCE(mod))
ret = module_adapter_sink_src_prepare(dev);
Expand Down
49 changes: 0 additions & 49 deletions src/ipc/ipc4/handler.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,46 +155,6 @@ __cold static int ipc4_delete_pipeline(struct ipc4_message_request *ipc4)
return ipc_pipeline_free(ipc, pipe->primary.r.instance_id);
}

static int ipc4_comp_params(struct comp_dev *current,
struct comp_buffer *calling_buf,
struct pipeline_walk_context *ctx, int dir)
{
struct pipeline_data *ppl_data = ctx->comp_data;
int err;

/* don't do any params if current is running */
if (current->state == COMP_STATE_ACTIVE)
return 0;

/* Stay on the current pipeline */
if (current->pipeline != ((struct pipeline_data *)ctx->comp_data)->p)
return 0;

err = comp_params(current, &ppl_data->params->params);
if (err < 0 || err == PPL_STATUS_PATH_STOP)
return err;

return pipeline_for_each_comp(current, ctx, dir);
}

static int ipc4_pipeline_params(struct pipeline *p, struct comp_dev *host)
{
struct sof_ipc_pcm_params hw_params = {{ 0 }};
struct pipeline_data data = {
.start = host,
.params = &hw_params,
.p = p,
};

struct pipeline_walk_context param_ctx = {
.comp_func = ipc4_comp_params,
.comp_data = &data,
.skip_incomplete = true,
};

return param_ctx.comp_func(host, NULL, &param_ctx, host->direction);
}

static int ipc4_pcm_params(struct ipc_comp_dev *pcm_dev)
{
int err, reset_err;
Expand All @@ -205,15 +165,6 @@ static int ipc4_pcm_params(struct ipc_comp_dev *pcm_dev)
return -EINVAL;
}

/* configure pipeline audio params */
err = ipc4_pipeline_params(pcm_dev->cd->pipeline, pcm_dev->cd);
if (err < 0) {
ipc_cmd_err(&ipc_tr, "ipc: pipe %d comp %d params failed %d",
pcm_dev->cd->pipeline->pipeline_id,
pcm_dev->cd->pipeline->comp_id, err);
goto error;
}

/* prepare pipeline audio params */
err = pipeline_prepare(pcm_dev->cd->pipeline, pcm_dev->cd);
if (err < 0) {
Expand Down
68 changes: 44 additions & 24 deletions src/samples/audio/detect_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,7 @@ static int test_keyword_cmd(struct comp_dev *dev, int cmd, void *data,
}
}
#endif /* CONFIG_IPC_MAJOR_4 */
static int test_keyword_params(struct comp_dev *dev, struct sof_ipc_stream_params *params);

static struct comp_dev *test_keyword_new(const struct comp_driver *drv,
const struct comp_ipc_config *config,
Expand Down Expand Up @@ -749,6 +750,16 @@ static struct comp_dev *test_keyword_new(const struct comp_driver *drv,
dev->direction = SOF_IPC_STREAM_CAPTURE;
dev->direction_set = true;
dev->state = COMP_STATE_READY;

#if CONFIG_IPC_MAJOR_4
struct sof_ipc_stream_params params;

/* retrieve params based on base config for IPC4 */
ret = test_keyword_params(dev, &params);
if (ret < 0)
goto cd_fail;
#endif

return dev;

cd_fail:
Expand Down Expand Up @@ -816,30 +827,6 @@ static int test_keyword_params(struct comp_dev *dev,

cd->sample_valid_bytes = params->sample_valid_bytes;

/* keyword components will only ever have 1 source */
sourceb = comp_dev_get_first_data_producer(dev);
channels = audio_stream_get_channels(&sourceb->stream);
frame_fmt = audio_stream_get_frm_fmt(&sourceb->stream);
rate = audio_stream_get_rate(&sourceb->stream);

if (channels != 1) {
comp_err(dev, "test_keyword_params(): only single-channel supported");
return -EINVAL;
}

if (!detector_is_sample_width_supported(frame_fmt)) {
comp_err(dev, "test_keyword_params(): only 16-bit format supported");
return -EINVAL;
}

/* calculate the length of the preamble */
if (cd->config.preamble_time) {
cd->keyphrase_samples = cd->config.preamble_time *
(rate / 1000);
} else {
cd->keyphrase_samples = KEYPHRASE_DEFAULT_PREAMBLE_LENGTH;
}

/*
* Threshold might be already set via IPC4_DETECT_TEST_SET_CONFIG,
* otherwise apply default value.
Expand All @@ -855,6 +842,32 @@ static int test_keyword_params(struct comp_dev *dev,
cd->config.activation_threshold = err;
}

/* keyword components will only ever have 1 source */
sourceb = comp_dev_get_first_data_producer(dev);
if (sourceb) {
channels = audio_stream_get_channels(&sourceb->stream);
frame_fmt = audio_stream_get_frm_fmt(&sourceb->stream);
rate = audio_stream_get_rate(&sourceb->stream);

if (channels != 1) {
comp_err(dev, "test_keyword_params(): only single-channel supported");
return -EINVAL;
}

if (!detector_is_sample_width_supported(frame_fmt)) {
comp_err(dev, "test_keyword_params(): only 16-bit format supported");
return -EINVAL;
}

/* calculate the length of the preamble */
if (cd->config.preamble_time) {
cd->keyphrase_samples = cd->config.preamble_time *
(rate / 1000);
} else {
cd->keyphrase_samples = KEYPHRASE_DEFAULT_PREAMBLE_LENGTH;
}
}

#if CONFIG_AMS
cd->kpd_uuid_id = AMS_INVALID_MSG_TYPE;
#endif /* CONFIG_AMS */
Expand Down Expand Up @@ -926,6 +939,7 @@ static int test_keyword_reset(struct comp_dev *dev)
static int test_keyword_prepare(struct comp_dev *dev)
{
struct comp_data *cd = comp_get_drvdata(dev);
struct sof_ipc_stream_params params;
uint16_t valid_bits = cd->sample_valid_bytes * 8;
uint16_t sample_width;
int ret;
Expand All @@ -938,6 +952,12 @@ static int test_keyword_prepare(struct comp_dev *dev)

comp_info(dev, "test_keyword_prepare()");

ret = test_keyword_params(dev, &params);
if (ret < 0) {
comp_err(dev, "test_keyword_prepare(): params config failed.");
return ret;
}

/*
* FIXME: this condition is always "false" for IPC4 as audio format cannot be changed
* without component re-creation. Does this "if" makes sense for IPC3?
Expand Down
Loading