ASoC: SOF: ipc4-topology: Enable deep buffer capture #5605
Conversation
|
This needs thesofproject/sof#10393 . Still it looks like DMA transfers are 48 frames / 1 ms, so more work needed. Also seems playback DMA transfers are the same, so I'm not sure I'm looking the right thing. |
singalsu
force-pushed
the
capture_deep_buffer_enable
branch
from
7526ff1 to
e3a9b28
Compare
The update patch works, in FW the DMA reload now happens at about every 10th host copier copy. But there are in captured audio small glitches every few tens of ms, that is likely a FW issue. |
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
I think @singalsu you can push this for review and merge. This works for me (I don't get the same problem on a SDW setup as you did with one HDA), and this does not depend on the FW part. The kernel part can go in whenever and the deepbuffer capability is just not enabled if topology doesn't support.
| 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 is a helpful new dev_dbg()!
singalsu
requested review from
bardliao,
dbaluta,
lgirdwood,
plbossart,
ranj063 and
ujfalusi
as code owners
| 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) * |
There was a problem hiding this comment.
Are you sure about this?
should not we have similar configuration as with playback that the DAI has different buffer size?
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;
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.
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).
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:
[ 509.183219] snd_sof:sof_ipc4_prepare_copier_module: sof-audio-pci-intel-mtl 0000:00:1f.3: copier dai-copier.DMIC.dmic01.capture, dma buffer: 4 ms (3072 bytes)
[ 509.183276] snd_sof:sof_ipc4_prepare_copier_module: sof-audio-pci-intel-mtl 0000:00:1f.3: copier host-copier.46.capture, dma buffer (using Deep Buffer): 10 ms (7680 bytes)
... 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).
| spcm->stream[dir].dsp_max_burst_size_in_ms = 1; | ||
| else | ||
| /* Capture data is copied from DSP to host in 1ms bursts */ | ||
| sps->dsp_max_burst_size_in_ms = 1; |
There was a problem hiding this comment.
why do we still need this? Won't this negate the benefits of deep buffering for capture?
There was a problem hiding this comment.
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.
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.
Do we call this out via dev_warn() ? We should.
This patch lets a capture PCM to operate with deep buffer by taking dsp_max_burst_size_in_ms from topology if it is defined. Earlier the value from topology was omitted for capture. If not defined, the maximum burst size for capture is set similarly as before to one millisecond. The dma_buffer_size is set similarly as for playback from largest of deep_buffer_dma_ms or SOF_IPC4_MIN_DMA_BUFFER_SIZE times OBS. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
singalsu
force-pushed
the
capture_deep_buffer_enable
branch
from
e3a9b28 to
c365ecd
Compare
6 participants
This patch lets a capture PCM to operate with deep buffer by taking dsp_max_burst_size_in_ms from topology if it is defined. Earlier the value from topology was omitted for capture.
If not defined, the maximum burst size for capture is set similarly as before to one millisecond.