ASoC: SOF: ipc4-topology: Enable deep buffer capture #5605
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -619,6 +619,7 @@ static int sof_ipc4_widget_setup_pcm(struct snd_sof_widget *swidget) | |
| struct sof_ipc4_available_audio_format *available_fmt; | ||
| struct snd_soc_component *scomp = swidget->scomp; | ||
| struct sof_ipc4_copier *ipc4_copier; | ||
| struct snd_sof_pcm_stream *sps; | ||
| struct snd_sof_pcm *spcm; | ||
| int node_type = 0; | ||
| int ret, dir; | ||
|
|
@@ -663,24 +664,23 @@ static int sof_ipc4_widget_setup_pcm(struct snd_sof_widget *swidget) | |
| if (ret) | ||
| goto free_available_fmt; | ||
|
|
||
| sps = &spcm->stream[dir]; | ||
| 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); | ||
|
|
||
| /* Set default DMA buffer size if it is not specified in topology */ | ||
| if (!sps->dsp_max_burst_size_in_ms) { | ||
| struct snd_sof_widget *pipe_widget = swidget->spipe->pipe_widget; | ||
| struct sof_ipc4_pipeline *pipeline = pipe_widget->private; | ||
|
|
||
| if (dir == SNDRV_PCM_STREAM_PLAYBACK) | ||
| sps->dsp_max_burst_size_in_ms = pipeline->use_chain_dma ? | ||
| SOF_IPC4_CHAIN_DMA_BUFFER_SIZE : SOF_IPC4_MIN_DMA_BUFFER_SIZE; | ||
| else | ||
| /* Capture data is copied from DSP to host in 1ms bursts */ | ||
| sps->dsp_max_burst_size_in_ms = 1; | ||
| } | ||
|
|
||
| skip_gtw_cfg: | ||
|
|
@@ -2421,10 +2421,19 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget, | |
| copier_data->gtw_cfg.dma_buffer_size); | ||
| break; | ||
| case snd_soc_dapm_dai_out: | ||
| copier_data->gtw_cfg.dma_buffer_size = | ||
| SOF_IPC4_MIN_DMA_BUFFER_SIZE * copier_data->base_config.obs; | ||
| break; | ||
| case snd_soc_dapm_aif_out: | ||
| copier_data->gtw_cfg.dma_buffer_size = | ||
| max((u32)SOF_IPC4_MIN_DMA_BUFFER_SIZE, deep_buffer_dma_ms) * | ||
|
Collaborator
There was a problem hiding this comment. Are you sure about this? case snd_soc_dapm_dai_out:
copier_data->gtw_cfg.dma_buffer_size =
SOF_IPC4_MIN_DMA_BUFFER_SIZE * copier_data->base_config.obs;
break;
case snd_soc_dapm_aif_out:
copier_data->gtw_cfg.dma_buffer_size =
SOF_IPC4_MIN_DMA_BUFFER_SIZE * copier_data->base_config.obs;
max((u32)SOF_IPC4_MIN_DMA_BUFFER_SIZE, deep_buffer_dma_ms) *
copier_data->base_config.obs;
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);
break;
Collaborator
There was a problem hiding this comment. yeah the DAI doesnt operate with deep buffer dma sizes and it will be ignored but best to seprate the DAI and aif widgets here.
Collaborator
There was a problem hiding this comment. good catch @ujfalusi , this does work, but will add to the latency. add not caught by our latency test as that doesn't cover the deepbuffer PCMs (at least yet).
Collaborator
There was a problem hiding this comment. Or on second look, I don't think this has material impact as the deep-buffer token is not set unless type is aif_in/out. Test confirms this: ... but agreed, probably for maintainability, better to keep code aligned with capture+playback here if they should be behaving the same (and not have two different looking pieces of code that in the end do the same thing). |
||
| copier_data->base_config.obs; | ||
| 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); | ||
|
Collaborator
There was a problem hiding this comment. This is a helpful new dev_dbg()! |
||
| break; | ||
| default: | ||
| break; | ||
| } | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we still need this? Won't this negate the benefits of deep buffering for capture?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought this was only for the "!sps->dsp_max_burst_size_in_ms" case where a larger value is not specific in topoligy. Then we default to 1ms (as we did before). Right @singalsu ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's correct. If there is no value from topology, the dsp_max_burst_size_in_ms is set to 1 (millisecond) just like it was always for capture before this patch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we call this out via dev_warn() ? We should.