ASoC: SOF: Revert dynamic Deep Buffer support for now#5748
ASoC: SOF: Revert dynamic Deep Buffer support for now#5748ujfalusi wants to merge 2 commits intothesofproject:topic/sof-devfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Reverts the previously introduced “dynamic Deep Buffer” behavior for IPC4 host copiers, returning Deep Buffer handling to a static buffer sizing model while adjusting related ALSA constraints.
Changes:
- Renames the PCM stream field from minimum-burst semantics to maximum-burst semantics and updates usage accordingly.
- Removes dynamic host DMA buffer sizing logic and restores static sizing based on Deep Buffer token (or minimum defaults).
- Updates HDA period-time constraints to account for an initial maximum-burst transfer plus a small headroom.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| sound/soc/sof/sof-audio.h | Renames stream burst-size field to reflect “max burst” semantics. |
| sound/soc/sof/ipc4-topology.h | Removes constant used only by dynamic Deep Buffer sizing logic. |
| sound/soc/sof/ipc4-topology.c | Drops dynamic DMA buffer sizing and restores static sizing; updates PCM stream burst-size initialization. |
| sound/soc/sof/intel/hda-pcm.c | Adjusts ALSA PERIOD_TIME constraint based on max burst + headroom; updates comments accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sof_update_ipc_object(scomp, &sps->dsp_max_burst_size_in_ms, | ||
| SOF_COPIER_DEEP_BUFFER_TOKENS, | ||
| swidget->tuples, | ||
| swidget->num_tuples, sizeof(u32), 1); |
There was a problem hiding this comment.
The return value from sof_update_ipc_object() is ignored here. If token parsing fails due to an invalid token list (e.g., -EINVAL), this will silently fall back to defaults instead of aborting setup like other parsing failures in this function. Capture the return value and handle errors consistently (propagate or log and return).
| sof_update_ipc_object(scomp, &sps->dsp_max_burst_size_in_ms, | |
| SOF_COPIER_DEEP_BUFFER_TOKENS, | |
| swidget->tuples, | |
| swidget->num_tuples, sizeof(u32), 1); | |
| ret = sof_update_ipc_object(scomp, &sps->dsp_max_burst_size_in_ms, | |
| SOF_COPIER_DEEP_BUFFER_TOKENS, | |
| swidget->tuples, | |
| swidget->num_tuples, sizeof(u32), 1); | |
| if (ret) { | |
| dev_err(scomp->dev, "parse host copier deep buffer token failed %d\n", | |
| ret); | |
| goto free_available_fmt; | |
| } |
| dev_dbg(sdev->dev, "copier %s, dma buffer%s: %u ms (%u bytes)", | ||
| swidget->widget->name, | ||
| deep_buffer_dma_ms ? " (using Deep Buffer)" : "", | ||
| max((u32)SOF_IPC4_MIN_DMA_BUFFER_SIZE, deep_buffer_dma_ms), | ||
| copier_data->gtw_cfg.dma_buffer_size); |
There was a problem hiding this comment.
This dev_dbg() format string lacks a trailing newline, while most other log messages in this file include one. Add "\n" at the end to keep log formatting consistent and avoid messages being concatenated.
| dev_dbg(sdev->dev, "copier %s, dma buffer%s: %u ms (%u bytes)", | ||
| swidget->widget->name, | ||
| deep_buffer_dma_ms ? " (using Deep Buffer)" : "", | ||
| max((u32)SOF_IPC4_MIN_DMA_BUFFER_SIZE, deep_buffer_dma_ms), | ||
| copier_data->gtw_cfg.dma_buffer_size); |
There was a problem hiding this comment.
This dev_dbg() format string lacks a trailing newline, while most other log messages in this file include one. Add "\n" at the end to keep log formatting consistent and avoid messages being concatenated.
| * Consequent DMA burst sizes are shorter and their length can vary. | ||
| * To avoid immediate xrun by the initial burst we need to place | ||
| * constraint on the period size (via PERIOD_TIME) to cover the size of | ||
| * the host buffer. | ||
| * We need to add headroom of max 10ms as the firmware needs time to | ||
| * settle to the 1ms pacing and initially it can run faster for few |
There was a problem hiding this comment.
Minor wording in this new comment: "Consequent DMA burst sizes" should be "Subsequent DMA burst sizes" (and consider "for a few internal periods" instead of "for few").
| * Consequent DMA burst sizes are shorter and their length can vary. | |
| * To avoid immediate xrun by the initial burst we need to place | |
| * constraint on the period size (via PERIOD_TIME) to cover the size of | |
| * the host buffer. | |
| * We need to add headroom of max 10ms as the firmware needs time to | |
| * settle to the 1ms pacing and initially it can run faster for few | |
| * Subsequent DMA burst sizes are shorter and their length can vary. | |
| * To avoid immediate xrun by the initial burst we need to place | |
| * constraint on the period size (via PERIOD_TIME) to cover the size of | |
| * the host buffer. | |
| * We need to add headroom of max 10ms as the firmware needs time to | |
| * settle to the 1ms pacing and initially it can run faster for a few |
|
@copilot: this is a revert series, it reverts the changes and it must do 1:1 |
This PR reverts the changes done by #5673
The reason is that the discussion on user space ABI to help applications to understand the jumpyDMA is dragging out and it is likely going to go in a different direction and the SOF implementation will need changes
https://lore.kernel.org/linux-sound/a03350b3-5828-4ffe-ade7-b94bb6ec7d64@linux.intel.com/