From 6c6a1867278a3f47b1e21e8af6a407d73c83dcab Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Thu, 23 Nov 2023 16:56:30 +0200 Subject: [PATCH] audio: pipeline: do not propagate pipeline triggers for IPC4 One major difference in IPC4 versus IPC3 is that each pipeline will get a separate SET_PIPELINE_STATE command. This is in stark contrast to IPC3 where host sends STREAM_TRIG_START and firmware is expected to propagate the state change to all connected pipelines in the graph. Change the code such that when pipeline component trigger is executed in IPC4 build, propagation is stopped whenever we reach another pipeline. This prevents bugs stemming from IPC3 related logic to propagate pipeline triggers, interfering with IPC4 usages like in bug 8481. To avoid false -ENODATA errors, drop IPC4 support for this capability. The check added in commit b154132b46ea ("ipc4: check pipeline priority for error report") covers most cases, but this relies on host to send the SET_PIPELINE_STATE calls in particular order. This unfortunately is not a mandatory order to follow, so logic to check upstream pipeline status to conclude downstream is in permanent -ENODATA state, is not as solid as it is for IPC3. As this may lead to hard to debug errors (e.g. when changing pipeline priorities in topologies which again affects the order of the SET_PIPELINE_STATE), better to simply disable this feature for IPC4. Link: https://github.com/thesofproject/sof/issues/8481 Signed-off-by: Kai Vehmanen --- src/audio/pipeline/pipeline-stream.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/audio/pipeline/pipeline-stream.c b/src/audio/pipeline/pipeline-stream.c index d4e24d36451e..af696810d5c5 100644 --- a/src/audio/pipeline/pipeline-stream.c +++ b/src/audio/pipeline/pipeline-stream.c @@ -47,6 +47,17 @@ pipeline_should_report_enodata_on_trigger(struct comp_dev *rsrc, struct pipeline_data *ppl_data = ctx->comp_data; struct comp_dev *pipe_source = ppl_data->start->pipeline->source_comp; + /* In IPC3, FW propagates triggers to connected pipelines, so + * it can have determistic logic to conclude no data is + * available. + * In IPC4, host controls state of each pipeline separately, + * so FW cannot reliably detect case of no data based on + * observing state of src->pipeline here. + */ +#if CONFIG_IPC_MAJOR_4 + return false; +#endif + /* only applies to capture pipelines */ if (dir != SOF_IPC_STREAM_CAPTURE) return false; @@ -388,8 +399,13 @@ static int pipeline_comp_trigger(struct comp_dev *current, /* trigger should propagate to the connected pipelines, * which need to be scheduled together + * + * IPC4 has a SET_PIPELINE_STATE for each pipeline, so FW + * should not propagate triggers on its own. + * IPC3 has commands only for graph edges, so propagation is + * needed in many cases. */ - if (!is_single_ppl && !is_same_sched) { + if (!is_single_ppl && (!is_same_sched || IS_ENABLED(CONFIG_IPC_MAJOR_4))) { pipe_dbg(current->pipeline, "pipeline_comp_trigger(), current is from another pipeline");