From c4f093b189758bcbb5d8e20aed85167f128c2788 Mon Sep 17 00:00:00 2001 From: Serhiy Katsyuba Date: Wed, 10 Jun 2026 16:02:00 +0200 Subject: [PATCH 1/3] ipc4: harden IPC4 pipeline triggering logic Using IPC4_MOD_ID() is not the best way to check if IPC4 is enabled. For module ID == 0, IPC4_MOD_ID() returns 0 for both IPC3 and IPC4. Module ID 0 is a valid IPC4 BASEFW ID. Since BASEFW is never added to a pipeline, this change doesn't fix any real problem. However, it's just more appropriate and safer to use IS_ENABLED(CONFIG_IPC_MAJOR_4): if module ID data becomes corrupted (zeroed) at runtime, this shouldn't make debugging even harder by causing unexpected pipeline behavior. Signed-off-by: Serhiy Katsyuba --- src/audio/pipeline/pipeline-graph.c | 2 +- src/audio/pipeline/pipeline-params.c | 2 +- src/audio/pipeline/pipeline-schedule.c | 2 +- src/audio/pipeline/pipeline-stream.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/audio/pipeline/pipeline-graph.c b/src/audio/pipeline/pipeline-graph.c index 2a679c2b57ef..8494f17998bd 100644 --- a/src/audio/pipeline/pipeline-graph.c +++ b/src/audio/pipeline/pipeline-graph.c @@ -369,7 +369,7 @@ static int pipeline_comp_reset(struct comp_dev *current, * scheduled together, except for IPC4, where each pipeline receives * commands from the host separately */ - if (!is_single_ppl && IPC4_MOD_ID(current->ipc_config.id)) + if (!is_single_ppl && IS_ENABLED(CONFIG_IPC_MAJOR_4)) return 0; /* Propagate reset across pipelines only in the same direction diff --git a/src/audio/pipeline/pipeline-params.c b/src/audio/pipeline/pipeline-params.c index 16273e0897d4..ed32fc9e3b02 100644 --- a/src/audio/pipeline/pipeline-params.c +++ b/src/audio/pipeline/pipeline-params.c @@ -267,7 +267,7 @@ static int pipeline_comp_prepare(struct comp_dev *current, if (!comp_is_single_pipeline(current, ppl_data->start)) { /* ipc4 module is only prepared in its parent pipeline */ - if (IPC4_MOD_ID(current->ipc_config.id)) + if (IS_ENABLED(CONFIG_IPC_MAJOR_4)) return 0; /* Propagate prepare only to pipelines in the same direction */ diff --git a/src/audio/pipeline/pipeline-schedule.c b/src/audio/pipeline/pipeline-schedule.c index 45fd1eed639c..4d0566aeff7e 100644 --- a/src/audio/pipeline/pipeline-schedule.c +++ b/src/audio/pipeline/pipeline-schedule.c @@ -140,7 +140,7 @@ static enum task_state pipeline_task_cmd(struct pipeline *p, err = SOF_TASK_STATE_RESCHEDULE; } else if (p->status == COMP_STATE_PAUSED) { /* reset the pipeline components for IPC4 after the STOP trigger */ - if (cmd == COMP_TRIGGER_STOP && IPC4_MOD_ID(host->ipc_config.id)) { + if (cmd == COMP_TRIGGER_STOP && IS_ENABLED(CONFIG_IPC_MAJOR_4)) { err = pipeline_reset(host->pipeline, host); if (err < 0) reply->error = err; diff --git a/src/audio/pipeline/pipeline-stream.c b/src/audio/pipeline/pipeline-stream.c index 8bfdfc182912..e4a166554fa2 100644 --- a/src/audio/pipeline/pipeline-stream.c +++ b/src/audio/pipeline/pipeline-stream.c @@ -229,7 +229,7 @@ static int pipeline_comp_list(struct comp_dev *current, * component and we aren't using IPC4. With IPC4 each pipeline receives * commands separately so we don't need to trigger them together */ - if (!is_single_ppl && (!is_same_sched || IPC4_MOD_ID(current->ipc_config.id))) { + if (!is_single_ppl && (!is_same_sched || IS_ENABLED(CONFIG_IPC_MAJOR_4))) { pipe_dbg(current->pipeline, "current is from another pipeline"); return 0; From c291489caf1e7ef5edb6da86930d0494bca24949 Mon Sep 17 00:00:00 2001 From: Serhiy Katsyuba Date: Wed, 10 Jun 2026 16:44:32 +0200 Subject: [PATCH 2/3] ipc4: assert IPC4 pipeline triggering logic With IPC4, each pipeline is triggered separately. Exactly 1 pipeline is expected in the pipelines list in pipeline_schedule_triggered(). Unfortunately, we still have considerable complex IPC3 pipeline triggering code being used with IPC4. This assertion ensures that the code works correctly for the IPC4 case. Signed-off-by: Serhiy Katsyuba --- src/audio/pipeline/pipeline-schedule.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/audio/pipeline/pipeline-schedule.c b/src/audio/pipeline/pipeline-schedule.c index 4d0566aeff7e..f8d5c09f2c1b 100644 --- a/src/audio/pipeline/pipeline-schedule.c +++ b/src/audio/pipeline/pipeline-schedule.c @@ -284,6 +284,16 @@ void pipeline_schedule_triggered(struct pipeline_walk_context *ctx, struct pipeline *p; uint32_t flags; +#ifdef CONFIG_IPC_MAJOR_4 + /* + * With IPC4, each pipeline is triggered separately. Exactly 1 pipeline + * is expected in the pipelines list (it's unclear whether an empty list + * should be tolerated). + */ + assert(list_is_empty(&ctx->pipelines) || + list_item_is_last(ctx->pipelines.next, &ctx->pipelines)); +#endif + /* * Interrupts have to be disabled while adding tasks to or removing them * from the scheduler list. Without that scheduling can begin From e4ccefad418d08d578ec501b72ec1aab01693304 Mon Sep 17 00:00:00 2001 From: Serhiy Katsyuba Date: Wed, 10 Jun 2026 10:44:44 +0200 Subject: [PATCH 3/3] Revert "ipc4: handler: maintain IPC set_pipeline_state order" This reverts commit 05bffd7e306e5febba1866724e2f7f28c56cd471. The LL scheduler already guarantees that pipelines run in the order they were triggered by the host (while respecting priorities). The original workaround (https://github.com/thesofproject/sof/pull/8504), which is now being reverted, has few problems: adds 1 ms startup latency per pipeline, ignores pipeline priorities and, hopefully, is unnecessary. The original workaround was added as a result of investigating issue https://github.com/thesofproject/sof/issues/8481. I believe there were two different problems that both caused the same issue. The first is a problem in the topology. The driver sent a multi-pipeline start for pipelines 0 and 1, meaning the driver expects pipeline 1 to start after pipeline 0. However, pipeline 0 was created with priority 1 and pipeline 1 with priority 0, so the firmware started pipeline 1 first. However, fixing the priority in the topology did not change the pipeline start order. This was probably caused by a bug in the firmware: complex IPC3 pipeline triggering logic was used instead of straightforward IPC4 logic. This appears to have been fixed by https://github.com/thesofproject/sof/pull/8506. I'm not exactly sure why the original workaround with wait_for_compound_msg() was added. Perhaps having two problems causing the same issue simultaneously led to some confusion during verification. Let's try removing it, and if we encounter any problems with pipeline triggering order (hopefully not), we'll fix them elsewhere. --- src/ipc/ipc4/handler-user.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/ipc/ipc4/handler-user.c b/src/ipc/ipc4/handler-user.c index 5a5f3d6d6f32..1de694c04e28 100644 --- a/src/ipc/ipc4/handler-user.c +++ b/src/ipc/ipc4/handler-user.c @@ -543,17 +543,6 @@ static int ipc4_set_pipeline_state(struct ipc4_message_request *ipc4) ipc_compound_pre_start(state.primary.r.type); ret = ipc4_pipeline_trigger(ppl_icd, cmd, &delayed); ipc_compound_post_start(state.primary.r.type, ret, delayed); - if (delayed) { - /* To maintain pipeline order for triggers, we must - * do a blocking wait until trigger is processed. - * This will add a max delay of 'ppl_count' LL ticks - * to process the full trigger list. - */ - if (ipc_wait_for_compound_msg() != 0) { - ipc_cmd_err(&ipc_tr, "ipc4: fail with delayed trigger"); - return IPC4_FAILURE; - } - } } if (ret != 0)