From c4c635183047fcf38faa912c7757cd642970a130 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Mon, 7 Sep 2026 13:54:27 +0300 Subject: [PATCH] ASoC: SOF: Intel: hda-dai-ops: Correct aggregate DAI pipeline trigger sequencing For an aggregate DAI link (dai_link->num_cpus > 1, e.g. a multi-link SoundWire speaker configuration), only one CPU DAI actually owns a firmware pipeline object: the topology marks the other member(s) as aggregated-only (is_aggregated_dai() in sof-audio.c) and never sets up a firmware pipeline for them, since their audio is carried by the same shared DSP pipeline that the owning DAI drives -- the extra widgets exist purely to represent the aggregation in the topology graph. A single pipeline state IPC affects every physical link belonging to the aggregate at once, so both directions of the trigger need to be sequenced against the host-side state of every member, not just the DAI currently being triggered: - On start, the RUNNING IPC makes firmware begin driving every link of the aggregate immediately, so it must not be sent until every member's own host-side link DMA has actually been armed. - On stop, the PAUSED IPC quiesces every link of the aggregate at once, so it must be sent before any member's host-side link DMA is stopped, so that all of them can be safely stopped afterwards. hda_ipc4_pre_trigger()/hda_ipc4_post_trigger() previously resolved the pipeline purely from the currently triggering CPU DAI's own widget, which meant: - Only the owning DAI could ever reach the code that sends the pipeline IPC; every other member bailed out immediately because its own pipe_widget->instance_id is never assigned. On stop, this happened to still work only because the owning DAI is conventionally the first CPU DAI triggered, so PAUSED was sent before any host-side DMA had stopped purely by coincidence of ordering. - On start, there was no gating against the state of sibling DAIs at all, so the pipeline could be told to go RUNNING as soon as the owning DAI's own link DMA was armed, regardless of whether the other aggregate members had started theirs. Since ASoC triggers each CPU DAI of the aggregate sequentially, this leaves a window where firmware is actively driving a link whose host-side DMA has not started yet, producing an audible black-out at stream start. Fix this with two changes: - hda_ipc4_all_link_dmas_running() gates the RUNNING IPC on every CPU DAI of the aggregate having its link DMA armed, checked directly from the AZX_PPLCCTL_RUN hardware bit rather than from hdac_ext_stream.hstream.running, which is a host-DMA-only field that link streams never set. - hda_ipc4_find_owning_pipe_widget() lets pre_trigger/post_trigger resolve the pipeline that actually needs the IPC regardless of which CPU DAI is currently being triggered. On start, this lets whichever DAI ASoC happens to trigger last complete the gated transition -- previously, once the owning DAI's own attempt deferred, no other DAI's callback could ever pick it back up. On stop, this makes the PAUSED-before-any-host-stop sequencing hold regardless of which CPU DAI happens to be triggered first, rather than relying on the owning DAI conventionally being first. Signed-off-by: Peter Ujfalusi --- sound/soc/sof/intel/hda-dai-ops.c | 107 ++++++++++++++++++++++++++++-- 1 file changed, 100 insertions(+), 7 deletions(-) diff --git a/sound/soc/sof/intel/hda-dai-ops.c b/sound/soc/sof/intel/hda-dai-ops.c index f0be42048db33c..c0f843d0062b67 100644 --- a/sound/soc/sof/intel/hda-dai-ops.c +++ b/sound/soc/sof/intel/hda-dai-ops.c @@ -356,6 +356,64 @@ static struct hdac_ext_link *sdw_get_hlink(struct snd_sof_dev *sdev, return hdac_bus_eml_sdw_get_hlink(bus); } +/* + * For aggregate DAIs (num_cpus > 1), each CPU DAI owns its own independently + * allocated link DMA (its own hdac_ext_stream / PPLC register block, see + * hda_link_stream_assign()). The firmware pipeline's RUNNING IPC starts all + * of the aggregate's physical links on the DSP side in one shot, so it must + * not be sent until every one of those link DMAs has actually been armed on + * the host side. Only one CPU DAI in the aggregate typically owns a real + * firmware pipeline object (see is_aggregated_dai() in sof-audio.c), so this + * check is intentionally independent of any per-DAI pipeline/spipe + * association -- it only cares about the host-side link DMA state. + */ +static bool hda_ipc4_all_link_dmas_running(struct snd_pcm_substream *substream) +{ + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); + struct snd_soc_dai *dai; + int i; + + for_each_rtd_cpu_dais(rtd, i, dai) { + struct hdac_ext_stream *hext_stream; + + hext_stream = snd_soc_dai_get_dma_data(dai, substream); + if (!hext_stream || + !(readl(hext_stream->pplc_addr + AZX_REG_PPLCCTL) & AZX_PPLCCTL_RUN)) + return false; + } + + return true; +} + +/* + * For aggregate DAIs (num_cpus > 1), only one CPU DAI actually owns a + * firmware pipeline object -- see is_aggregated_dai() in sof-audio.c. The + * other member(s) have their own widget, but their pipe_widget->instance_id + * stays unset since their pipeline is never set up in firmware. Since ASoC + * triggers each CPU DAI of the aggregate sequentially and the owning DAI is + * not guaranteed to be triggered last, search the aggregate's sibling CPU + * DAIs for the one that does own a valid pipeline, so its state transition + * can be completed regardless of which DAI is currently being triggered. + */ +static struct snd_sof_widget * +hda_ipc4_find_owning_pipe_widget(struct snd_pcm_substream *substream) +{ + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); + struct snd_soc_dai *dai; + int i; + + for_each_rtd_cpu_dais(rtd, i, dai) { + struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(dai, + substream->stream); + struct snd_sof_widget *sw = w ? w->dobj.private : NULL; + + if (sw && sw->spipe->pipe_widget->instance_id >= 0) + return sw; + } + + return NULL; +} + static int hda_ipc4_pre_trigger(struct snd_sof_dev *sdev, struct snd_soc_dai *cpu_dai, struct snd_pcm_substream *substream, int cmd) { @@ -369,10 +427,16 @@ static int hda_ipc4_pre_trigger(struct snd_sof_dev *sdev, struct snd_soc_dai *cp w = snd_soc_dai_get_widget(cpu_dai, substream->stream); swidget = w->dobj.private; pipe_widget = swidget->spipe->pipe_widget; - pipeline = pipe_widget->private; - if (pipe_widget->instance_id < 0) - return 0; + if (pipe_widget->instance_id < 0) { + swidget = hda_ipc4_find_owning_pipe_widget(substream); + if (!swidget) + return 0; + + pipe_widget = swidget->spipe->pipe_widget; + } + + pipeline = pipe_widget->private; guard(mutex)(&ipc4_data->pipeline_state_mutex); @@ -383,13 +447,22 @@ static int hda_ipc4_pre_trigger(struct snd_sof_dev *sdev, struct snd_soc_dai *cp case SNDRV_PCM_TRIGGER_PAUSE_PUSH: case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_STOP: + /* + * For aggregate DAIs with shared pipelines, the state check + * deduplicates: the first DAI sends the IPC, subsequent DAIs + * sharing the same pipeline see it already paused and skip. + * For aggregate DAIs with different pipelines, each DAI pauses + * its own pipeline independently. + */ + if (pipeline->state == SOF_IPC4_PIPE_PAUSED) + break; + ret = sof_ipc4_set_pipeline_state(sdev, pipe_widget->instance_id, SOF_IPC4_PIPE_PAUSED); if (ret < 0) return ret; pipeline->state = SOF_IPC4_PIPE_PAUSED; - break; default: dev_err(sdev->dev, "unknown trigger command %d\n", cmd); @@ -436,25 +509,42 @@ static int hda_trigger(struct snd_sof_dev *sdev, struct snd_soc_dai *cpu_dai, static int hda_ipc4_post_trigger(struct snd_sof_dev *sdev, struct snd_soc_dai *cpu_dai, struct snd_pcm_substream *substream, int cmd) { + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); struct sof_ipc4_fw_data *ipc4_data = sdev->private; struct snd_sof_widget *pipe_widget; struct sof_ipc4_pipeline *pipeline; struct snd_sof_widget *swidget; struct snd_soc_dapm_widget *w; + int num_cpus = rtd->dai_link->num_cpus; int ret = 0; w = snd_soc_dai_get_widget(cpu_dai, substream->stream); swidget = w->dobj.private; pipe_widget = swidget->spipe->pipe_widget; - pipeline = pipe_widget->private; - if (pipe_widget->instance_id < 0) - return 0; + if (pipe_widget->instance_id < 0) { + swidget = hda_ipc4_find_owning_pipe_widget(substream); + if (!swidget) + return 0; + + pipe_widget = swidget->spipe->pipe_widget; + } + + pipeline = pipe_widget->private; guard(mutex)(&ipc4_data->pipeline_state_mutex); switch (cmd) { case SNDRV_PCM_TRIGGER_START: + /* + * For aggregated DAIs (num_cpus > 1), defer the RUNNING IPC + * until every CPU DAI's link DMA has been armed via + * hda_trigger(): the firmware starts all of the aggregate's + * physical links as soon as it sees this one IPC. + */ + if (num_cpus > 1 && !hda_ipc4_all_link_dmas_running(substream)) + break; + if (pipeline->state != SOF_IPC4_PIPE_PAUSED) { ret = sof_ipc4_set_pipeline_state(sdev, pipe_widget->instance_id, SOF_IPC4_PIPE_PAUSED); @@ -473,6 +563,9 @@ static int hda_ipc4_post_trigger(struct snd_sof_dev *sdev, struct snd_soc_dai *c swidget->spipe->started_count++; break; case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: + if (num_cpus > 1 && !hda_ipc4_all_link_dmas_running(substream)) + break; + ret = sof_ipc4_set_pipeline_state(sdev, pipe_widget->instance_id, SOF_IPC4_PIPE_RUNNING); if (ret < 0)