+#define SDW_PLAYBACK_MIN_NUM_PERIODS 2 +#define SDW_PLAYBACK_MAX_NUM_PERIODS 8 +#define SDW_PLAYBACK_MAX_PERIOD_SIZE 8192
that's a fairly small max period. That's 21ms for 2ch S32_LE 48kHz.
Does this come from specific limitations or is this an arbitrary number?
A comment on this wouldn't hurt.
+static u32 sdw0_dma_enable_reg[ACP63_SDW0_DMA_MAX_STREAMS] = {
- ACP_SW0_AUDIO0_TX_EN,
- ACP_SW0_AUDIO1_TX_EN,
- ACP_SW0_AUDIO2_TX_EN,
- ACP_SW0_AUDIO0_RX_EN,
- ACP_SW0_AUDIO1_RX_EN,
- ACP_SW0_AUDIO2_RX_EN,
+};
+static u32 sdw1_dma_enable_reg[ACP63_SDW1_DMA_MAX_STREAMS] = {
- ACP_SW1_AUDIO1_TX_EN,
- ACP_SW1_AUDIO1_RX_EN,
+};
Still no explanation as to why SDW0 indices start at zero and SDW1 indices start at one?
+static const struct snd_pcm_hardware acp63_sdw_hardware_playback = {
- .info = SNDRV_PCM_INFO_INTERLEAVED |
SNDRV_PCM_INFO_BLOCK_TRANSFER |SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME,- .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S8 |
SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE,- .channels_min = 2,
- .channels_max = 2,
- .rates = SNDRV_PCM_RATE_48000,
- .rate_min = 48000,
- .rate_max = 48000,
- .buffer_bytes_max = SDW_PLAYBACK_MAX_NUM_PERIODS * SDW_PLAYBACK_MAX_PERIOD_SIZE,
- .period_bytes_min = SDW_PLAYBACK_MIN_PERIOD_SIZE,
- .period_bytes_max = SDW_PLAYBACK_MAX_PERIOD_SIZE,
- .periods_min = SDW_PLAYBACK_MIN_NUM_PERIODS,
- .periods_max = SDW_PLAYBACK_MAX_NUM_PERIODS,
+};
+static const struct snd_pcm_hardware acp63_sdw_hardware_capture = {
- .info = SNDRV_PCM_INFO_INTERLEAVED |
SNDRV_PCM_INFO_BLOCK_TRANSFER |SNDRV_PCM_INFO_MMAP |SNDRV_PCM_INFO_MMAP_VALID |SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME,- .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S8 |
SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE,- .channels_min = 2,
- .channels_max = 2,
- .rates = SNDRV_PCM_RATE_48000,
- .rate_min = 48000,
- .rate_max = 48000,
- .buffer_bytes_max = SDW_CAPTURE_MAX_NUM_PERIODS * SDW_CAPTURE_MAX_PERIOD_SIZE,
- .period_bytes_min = SDW_CAPTURE_MIN_PERIOD_SIZE,
- .period_bytes_max = SDW_CAPTURE_MAX_PERIOD_SIZE,
- .periods_min = SDW_CAPTURE_MIN_NUM_PERIODS,
- .periods_max = SDW_CAPTURE_MAX_NUM_PERIODS,
+};
+static int acp63_sdw_dma_open(struct snd_soc_component *component,
struct snd_pcm_substream *substream)+{
- struct snd_pcm_runtime *runtime;
- struct acp_sdw_dma_stream *stream;
- struct snd_soc_dai *cpu_dai;
- struct amd_sdw_manager *amd_manager;
- struct snd_soc_pcm_runtime *prtd = substream->private_data;
- int ret;
- runtime = substream->runtime;
- cpu_dai = asoc_rtd_to_cpu(prtd, 0);
- amd_manager = snd_soc_dai_get_drvdata(cpu_dai);
- stream = kzalloc(sizeof(*stream), GFP_KERNEL);
- if (!stream)
return -ENOMEM;- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
runtime->hw = acp63_sdw_hardware_playback;- else
runtime->hw = acp63_sdw_hardware_capture;- ret = snd_pcm_hw_constraint_integer(runtime,
SNDRV_PCM_HW_PARAM_PERIODS);- if (ret < 0) {
dev_err(component->dev, "set integer constraint failed\n");kfree(stream);return ret;- }
it's not clear to me why you have to add this specific constraint, isn't this checked already with the sdw_hardware_playback information.
+static u64 acp63_sdw_get_byte_count(struct acp_sdw_dma_stream *stream, void __iomem *acp_base) +{
- union acp_sdw_dma_count byte_count;
- u32 pos_low_reg, pos_high_reg;
- byte_count.bytescount = 0;
- switch (stream->instance) {
- case ACP_SDW0:
pos_low_reg = sdw0_dma_ring_buf_reg[stream->stream_id].pos_low_reg;pos_high_reg = sdw0_dma_ring_buf_reg[stream->stream_id].pos_high_reg;break;- case ACP_SDW1:
pos_low_reg = sdw1_dma_ring_buf_reg[stream->stream_id].pos_low_reg;pos_high_reg = sdw1_dma_ring_buf_reg[stream->stream_id].pos_high_reg;break;- default:
return -EINVAL;
returning -EINVAL as a u64 doesn't seem quite right to me?
- }
- if (pos_low_reg) {
byte_count.bcount.high = readl(acp_base + pos_high_reg);byte_count.bcount.low = readl(acp_base + pos_low_reg);- }
- return byte_count.bytescount;
+}