[alsa-devel] [PATCH 1/3] ASoC: AMD: Make ACP->SYSMEM DMA non circular
In capture case we don't want ACP to SYSMEM dma to be circular. This is because if an in place DSP filter is applied to captured output then circular DMA can overwrite the filter value with stale data.
Signed-off-by: Akshu Agrawal akshu.agrawal@amd.com --- sound/soc/amd/acp-pcm-dma.c | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-)
diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c index 94bcf69..816abd6 100644 --- a/sound/soc/amd/acp-pcm-dma.c +++ b/sound/soc/amd/acp-pcm-dma.c @@ -400,7 +400,7 @@ static void acp_dma_cap_channel_disable(void __iomem *acp_mmio, }
/* Start a given DMA channel transfer */ -static void acp_dma_start(void __iomem *acp_mmio, u16 ch_num) +static void acp_dma_start(void __iomem *acp_mmio, u16 ch_num, bool is_circular) { u32 dma_ctrl;
@@ -429,8 +429,11 @@ static void acp_dma_start(void __iomem *acp_mmio, u16 ch_num) break; }
- /* circular for both DMA channel */ - dma_ctrl |= ACP_DMA_CNTL_0__Circular_DMA_En_MASK; + /* enable for ACP to SRAM DMA channel */ + if (is_circular == true) + dma_ctrl |= ACP_DMA_CNTL_0__Circular_DMA_En_MASK; + else + dma_ctrl &= ~ACP_DMA_CNTL_0__Circular_DMA_En_MASK;
acp_reg_write(dma_ctrl, acp_mmio, mmACP_DMA_CNTL_0 + ch_num); } @@ -674,6 +677,7 @@ static int acp_deinit(void __iomem *acp_mmio) /* ACP DMA irq handler routine for playback, capture usecases */ static irqreturn_t dma_irq_handler(int irq, void *arg) { + u16 dscr_idx; u32 intr_flag, ext_intr_status; struct audio_drv_data *irq_data; void __iomem *acp_mmio; @@ -705,6 +709,15 @@ static irqreturn_t dma_irq_handler(int irq, void *arg)
if ((intr_flag & BIT(I2S_TO_ACP_DMA_CH_NUM)) != 0) { valid_irq = true; + if (acp_reg_read(acp_mmio, mmACP_DMA_CUR_DSCR_14) == + CAPTURE_START_DMA_DESCR_CH15) + dscr_idx = CAPTURE_END_DMA_DESCR_CH14; + else + dscr_idx = CAPTURE_START_DMA_DESCR_CH14; + config_acp_dma_channel(acp_mmio, ACP_TO_SYSRAM_CH_NUM, dscr_idx, + 1, 0); + acp_dma_start(acp_mmio, ACP_TO_SYSRAM_CH_NUM, false); + snd_pcm_period_elapsed(irq_data->capture_i2ssp_stream); acp_reg_write((intr_flag & BIT(I2S_TO_ACP_DMA_CH_NUM)) << 16, acp_mmio, mmACP_EXTERNAL_INTR_STAT); @@ -712,6 +725,17 @@ static irqreturn_t dma_irq_handler(int irq, void *arg)
if ((intr_flag & BIT(I2S_TO_ACP_DMA_BT_INSTANCE_CH_NUM)) != 0) { valid_irq = true; + if (acp_reg_read(acp_mmio, mmACP_DMA_CUR_DSCR_10) == + CAPTURE_START_DMA_DESCR_CH11) + dscr_idx = CAPTURE_END_DMA_DESCR_CH10; + else + dscr_idx = CAPTURE_START_DMA_DESCR_CH10; + config_acp_dma_channel(acp_mmio, + ACP_TO_SYSRAM_BT_INSTANCE_CH_NUM, + dscr_idx, 1, 0); + acp_dma_start(acp_mmio, ACP_TO_SYSRAM_BT_INSTANCE_CH_NUM, + false); + snd_pcm_period_elapsed(irq_data->capture_i2sbt_stream); acp_reg_write((intr_flag & BIT(I2S_TO_ACP_DMA_BT_INSTANCE_CH_NUM)) << 16, @@ -1053,9 +1077,11 @@ static int acp_dma_trigger(struct snd_pcm_substream *substream, int cmd) acp_dma_cap_channel_enable(rtd->acp_mmio, CAP_CHANNEL1); } + acp_dma_start(rtd->acp_mmio, rtd->ch1, true); + } else { + acp_dma_start(rtd->acp_mmio, rtd->ch1, true); + acp_dma_start(rtd->acp_mmio, rtd->ch2, true); } - acp_dma_start(rtd->acp_mmio, rtd->ch1); - acp_dma_start(rtd->acp_mmio, rtd->ch2); ret = 0; break; case SNDRV_PCM_TRIGGER_STOP:
From: "Mukunda, Vijendar" Vijendar.Mukunda@amd.com
Give position on ACP->SYSMEM DMA channel for the number of bytes that have been transferred on the current descriptor under service.
Signed-off-by: Vijendar Mukunda Vijendar.Mukunda@amd.com Signed-off-by: Akshu Agrawal akshu.agrawal@amd.com --- sound/soc/amd/acp-pcm-dma.c | 34 ++++++++++++++++++++++------------ sound/soc/amd/acp.h | 3 +++ 2 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c index 816abd6..6240a77 100644 --- a/sound/soc/amd/acp-pcm-dma.c +++ b/sound/soc/amd/acp-pcm-dma.c @@ -922,10 +922,9 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream, rtd->destination = FROM_BLUETOOTH; rtd->dma_dscr_idx_1 = CAPTURE_START_DMA_DESCR_CH10; rtd->dma_dscr_idx_2 = CAPTURE_START_DMA_DESCR_CH11; - rtd->byte_cnt_high_reg_offset = - mmACP_I2S_BT_RECEIVE_BYTE_CNT_HIGH; - rtd->byte_cnt_low_reg_offset = - mmACP_I2S_BT_RECEIVE_BYTE_CNT_LOW; + rtd->dma_curr_dscr = mmACP_DMA_CUR_DSCR_11; + rtd->cur_trans_cnt = mmACP_DMA_CUR_TRANS_CNT_11; + rtd->end_dma_dscr = CAPTURE_END_DMA_DESCR_CH10; adata->capture_i2sbt_stream = substream; break; case I2S_SP_INSTANCE: @@ -945,10 +944,9 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream, rtd->destination = FROM_ACP_I2S_1; rtd->dma_dscr_idx_1 = CAPTURE_START_DMA_DESCR_CH14; rtd->dma_dscr_idx_2 = CAPTURE_START_DMA_DESCR_CH15; - rtd->byte_cnt_high_reg_offset = - mmACP_I2S_RECEIVED_BYTE_CNT_HIGH; - rtd->byte_cnt_low_reg_offset = - mmACP_I2S_RECEIVED_BYTE_CNT_LOW; + rtd->dma_curr_dscr = mmACP_DMA_CUR_DSCR_15; + rtd->cur_trans_cnt = mmACP_DMA_CUR_TRANS_CNT_15; + rtd->end_dma_dscr = CAPTURE_END_DMA_DESCR_CH14; adata->capture_i2ssp_stream = substream; } } @@ -1002,6 +1000,9 @@ static snd_pcm_uframes_t acp_dma_pointer(struct snd_pcm_substream *substream) u32 buffersize; u32 pos = 0; u64 bytescount = 0; + u32 dma_count = 0; + u16 dscr; + u32 period_bytes;
struct snd_pcm_runtime *runtime = substream->runtime; struct audio_substream_data *rtd = runtime->private_data; @@ -1010,10 +1011,19 @@ static snd_pcm_uframes_t acp_dma_pointer(struct snd_pcm_substream *substream) return -EINVAL;
buffersize = frames_to_bytes(runtime, runtime->buffer_size); - bytescount = acp_get_byte_count(rtd); - - bytescount -= rtd->bytescount; - pos = do_div(bytescount, buffersize); + if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) { + period_bytes = frames_to_bytes(runtime, runtime->period_size); + dscr = acp_reg_read(rtd->acp_mmio, rtd->dma_curr_dscr); + dma_count = acp_reg_read(rtd->acp_mmio, rtd->cur_trans_cnt); + if (dscr == rtd->end_dma_dscr) + dma_count += period_bytes; + pos = dma_count % buffersize; + } else { + bytescount = acp_get_byte_count(rtd); + if (bytescount > rtd->bytescount) + bytescount -= rtd->bytescount; + pos = do_div(bytescount, buffersize); + } return bytes_to_frames(runtime, pos); }
diff --git a/sound/soc/amd/acp.h b/sound/soc/amd/acp.h index 0a2240b..cf7c0b9 100644 --- a/sound/soc/amd/acp.h +++ b/sound/soc/amd/acp.h @@ -134,10 +134,13 @@ struct audio_substream_data { u16 destination; u16 dma_dscr_idx_1; u16 dma_dscr_idx_2; + u16 end_dma_dscr; u32 pte_offset; u32 sram_bank; u32 byte_cnt_high_reg_offset; u32 byte_cnt_low_reg_offset; + u32 dma_curr_dscr; + u32 cur_trans_cnt; uint64_t size; u64 bytescount; void __iomem *acp_mmio;
ACP->SYSMEM DMA happens at every I2S->SYSMEM period completion. Thus, there is delay of x frames till I2S->SYSMEM reaches a period length. This delay is communicated to user space.
Signed-off-by: Akshu Agrawal akshu.agrawal@amd.com --- sound/soc/amd/acp-pcm-dma.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c index 6240a77..c24611c 100644 --- a/sound/soc/amd/acp-pcm-dma.c +++ b/sound/soc/amd/acp-pcm-dma.c @@ -922,6 +922,10 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream, rtd->destination = FROM_BLUETOOTH; rtd->dma_dscr_idx_1 = CAPTURE_START_DMA_DESCR_CH10; rtd->dma_dscr_idx_2 = CAPTURE_START_DMA_DESCR_CH11; + rtd->byte_cnt_high_reg_offset = + mmACP_I2S_BT_RECEIVE_BYTE_CNT_HIGH; + rtd->byte_cnt_low_reg_offset = + mmACP_I2S_BT_RECEIVE_BYTE_CNT_LOW; rtd->dma_curr_dscr = mmACP_DMA_CUR_DSCR_11; rtd->cur_trans_cnt = mmACP_DMA_CUR_TRANS_CNT_11; rtd->end_dma_dscr = CAPTURE_END_DMA_DESCR_CH10; @@ -944,6 +948,10 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream, rtd->destination = FROM_ACP_I2S_1; rtd->dma_dscr_idx_1 = CAPTURE_START_DMA_DESCR_CH14; rtd->dma_dscr_idx_2 = CAPTURE_START_DMA_DESCR_CH15; + rtd->byte_cnt_high_reg_offset = + mmACP_I2S_RECEIVED_BYTE_CNT_HIGH; + rtd->byte_cnt_low_reg_offset = + mmACP_I2S_RECEIVED_BYTE_CNT_LOW; rtd->dma_curr_dscr = mmACP_DMA_CUR_DSCR_15; rtd->cur_trans_cnt = mmACP_DMA_CUR_TRANS_CNT_15; rtd->end_dma_dscr = CAPTURE_END_DMA_DESCR_CH14; @@ -1002,7 +1010,7 @@ static snd_pcm_uframes_t acp_dma_pointer(struct snd_pcm_substream *substream) u64 bytescount = 0; u32 dma_count = 0; u16 dscr; - u32 period_bytes; + u32 period_bytes, delay;
struct snd_pcm_runtime *runtime = substream->runtime; struct audio_substream_data *rtd = runtime->private_data; @@ -1011,6 +1019,10 @@ static snd_pcm_uframes_t acp_dma_pointer(struct snd_pcm_substream *substream) return -EINVAL;
buffersize = frames_to_bytes(runtime, runtime->buffer_size); + bytescount = acp_get_byte_count(rtd); + if (bytescount > rtd->bytescount) + bytescount -= rtd->bytescount; + if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) { period_bytes = frames_to_bytes(runtime, runtime->period_size); dscr = acp_reg_read(rtd->acp_mmio, rtd->dma_curr_dscr); @@ -1018,10 +1030,9 @@ static snd_pcm_uframes_t acp_dma_pointer(struct snd_pcm_substream *substream) if (dscr == rtd->end_dma_dscr) dma_count += period_bytes; pos = dma_count % buffersize; + delay = do_div(bytescount, period_bytes); + runtime->delay = bytes_to_frames(runtime, delay); } else { - bytescount = acp_get_byte_count(rtd); - if (bytescount > rtd->bytescount) - bytescount -= rtd->bytescount; pos = do_div(bytescount, buffersize); } return bytes_to_frames(runtime, pos);
On Thu, Aug 02, 2018 at 12:11:54PM +0530, Akshu Agrawal wrote:
In capture case we don't want ACP to SYSMEM dma to be circular. This is because if an in place DSP filter is applied to captured output then circular DMA can overwrite the filter value with stale data.
Isn't that just a problem with userspace not giving itself enough buffer to get the in place processing done before we loop round again. This will simply change the type of error, it won't actually fix anything AFAICT but perhaps I'm missing something.
On 8/2/2018 3:26 PM, Mark Brown wrote:
On Thu, Aug 02, 2018 at 12:11:54PM +0530, Akshu Agrawal wrote:
In capture case we don't want ACP to SYSMEM dma to be circular. This is because if an in place DSP filter is applied to captured output then circular DMA can overwrite the filter value with stale data.
Isn't that just a problem with userspace not giving itself enough buffer to get the in place processing done before we loop round again. This will simply change the type of error, it won't actually fix anything AFAICT but perhaps I'm missing something.
Sorry for the delayed reply. Will try and explain more on whats happening: We have 2 DMAs, one being I2S->ACP(Audio Co-Processor memory) and the second being ACP->SYSMEM. When ACP->SYSMEM is circular then this DMA is faster and keeps updating from ACP memory. This leaves no time for user space to do any kind of in place post processing. This design works fine if the user reads the data from SYSMEM at capture rate (that is the rate at which I2S->ACP transfer happens). But, when an in place post processing happens on SYSMEM then the data gets overwritten.
Sending and updated series with changes in 2/3 and 3/3 patch.
Thanks, Akshu
participants (3)
-
Agrawal, Akshu -
Akshu Agrawal
-
Mark Brown