Skip to content

ASoC: SOF: Intel: hda-dai-ops: Correct aggregate DAI pipeline trigger… - #5924

Open
ujfalusi wants to merge 1 commit into
thesofproject:topic/sof-devfrom
ujfalusi:peter/sof/pr/aggregated-sdw-trigger-2
Open

ASoC: SOF: Intel: hda-dai-ops: Correct aggregate DAI pipeline trigger…#5924
ujfalusi wants to merge 1 commit into
thesofproject:topic/sof-devfrom
ujfalusi:peter/sof/pr/aggregated-sdw-trigger-2

Conversation

@ujfalusi

@ujfalusi ujfalusi commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

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

… 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 <peter.ujfalusi@linux.intel.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

A new helper dereferences sw->spipe->pipe_widget without NULL checks, which can crash trigger paths if the pipeline linkage is absent or transient.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR fixes trigger sequencing for aggregate CPU DAIs (e.g., multi-link SoundWire) on Intel SOF IPC4 by ensuring firmware pipeline state transitions are coordinated across all member link DMAs, avoiding premature DSP-side start/stop relative to host-side link DMA arming.

Changes:

  • Add gating so the pipeline RUNNING IPC is deferred until all aggregate members’ link DMAs are actually armed (checked via AZX_PPLCCTL_RUN).
  • Resolve the owning pipeline widget across sibling CPU DAIs so the required pipeline IPC can be issued regardless of which CPU DAI is currently being triggered.
  • Deduplicate PAUSED IPC transitions when multiple DAIs share a pipeline.
File summaries
File Description
sound/soc/sof/intel/hda-dai-ops.c Adds aggregate-aware pipeline ownership resolution and RUNNING IPC gating based on link DMA RUN state.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +406 to +411
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;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

it is granted to be never NULL, no need for the check, not even in case of DSPless mode, when this path is not even reached.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants