ASoC: SOF: Intel: hda-dai-ops: Correct aggregate DAI pipeline trigger… - #5924
ASoC: SOF: Intel: hda-dai-ops: Correct aggregate DAI pipeline trigger…#5924ujfalusi wants to merge 1 commit into
Conversation
… 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>
There was a problem hiding this comment.
🟡 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.
| 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; |
There was a problem hiding this comment.
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.
Dell XPS 16 DA16260: positive balance results with aggregate DAI sequencing patchLocal test report for SOF Linux PR #5924, commit On a 2026 Dell XPS 16 DA16260, persistent right-heavy speaker centering changed to balanced after applying the PR's aggregate DAI pipeline sequencing change. It improved on two initialized patched boots and returned on an intervening baseline boot. This is a positive subjective result with the qualifications below, not a measured explanation of the physical imbalance. Hardware: Dell subsystem The test used two optional boot images with the same kernel and original initrd base, with matching preload additions and an identical early module-loading arrangement. The unpatched rebuild's executable code matched the installed driver. Only the intended
The other three profiles match across the four initialized boots. Both coefficient variants occur with both outcomes, following patch presence. However, no baseline/patched pair simultaneously matches coefficient selection and volume snapshots; exact in-song source, position, route and gain histories were not recorded. Volume/profile interactions and variable boot state remain possible confounds. This supports the patch as a strong causal lead, not a fully isolated efficacy claim. The final user-controlled reference session used native S32LE/48 kHz stereo playback directly to the physical speakers, with steady Both and one-second side cycles. Four plays completed cleanup; all 144 active routing observations passed. Source channels remained at unity; endpoint levels remained equal and unmuted. The user reported balanced centering on the replacement XPS 16 but residual left “eee” / right “ooo” timbre; they heard the same timbre difference on both physical XPS 16 units. That tone difference is tracked separately and has no established software cause. Generated/queued frame counts are not physical rendered timestamps or acoustic measurements. One attempted patched confirmation boot failed right-tweeter probe ( Exact adapted patch SHA-256: Attribution: Codex using Astra Ultra performed the programmatic investigation and prepared this post on the account owner’s behalf. Listening observations were supplied by the laptop owner; automated digital checks are not presented as acoustic measurements. The immediate downstream request is to consider this patch for the Panther Lake kernel package. A separately named local kernel is being built; no long-term reliability claim is made. |
|
@mapleroyal , thank you for your feedback! |
… 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:
hda_ipc4_pre_trigger()/hda_ipc4_post_trigger() previously resolved the pipeline purely from the currently triggering CPU DAI's own widget, which meant:
Fix this with two changes: