diff mbox series

ASoC: amd: Buffer Size instead of MAX Buffer

Message ID 1581426768-8937-1-git-send-email-Vishnuvardhanrao.Ravulapati@amd.com (mailing list archive)
State Accepted
Commit 1880b1f1d686b17387b5bf45654eb1d087ead918
Headers show
Series ASoC: amd: Buffer Size instead of MAX Buffer | expand

Commit Message

RAVULAPATI, VISHNU VARDHAN RAO Feb. 11, 2020, 1:12 p.m. UTC 
Because of MAX BUFFER size in register,when user/app give small
buffer size produces noise of old data in buffer.
This patch rectifies this noise when using different
buffer sizes less than MAX BUFFER.

Signed-off-by: Ravulapati Vishnu vardhan rao <Vishnuvardhanrao.Ravulapati@amd.com>
---
 sound/soc/amd/raven/acp3x-i2s.c     | 8 ++++++++
 sound/soc/amd/raven/acp3x-pcm-dma.c | 7 +------
 2 files changed, 9 insertions(+), 6 deletions(-)

Comments

Mark Brown Feb. 11, 2020, 3:38 p.m. UTC | #1 
On Tue, Feb 11, 2020 at 06:42:28PM +0530, Ravulapati Vishnu vardhan rao wrote:

> Because of MAX BUFFER size in register,when user/app give small
> buffer size produces noise of old data in buffer.
> This patch rectifies this noise when using different
> buffer sizes less than MAX BUFFER.

In what way does this patch fix the issue?  I looks like it's moving a
buffer size setting from DMA to I2S but it's not clear why or how this
fixes the issue, or indeed what the actual issue that's causing what are
presumably underruns is?
vishnu Feb. 12, 2020, 9:06 a.m. UTC | #2 
On 11/02/20 9:08 PM, Mark Brown wrote:
> On Tue, Feb 11, 2020 at 06:42:28PM +0530, Ravulapati Vishnu vardhan rao wrote:
> 
>> Because of MAX BUFFER size in register,when user/app give small
>> buffer size produces noise of old data in buffer.
>> This patch rectifies this noise when using different
>> buffer sizes less than MAX BUFFER.
> 
> In what way does this patch fix the issue?  I looks like it's moving a
> buffer size setting from DMA to I2S but it's not clear why or how this
> fixes the issue, or indeed what the actual issue that's causing what are
> presumably underruns is?
> 
prior to this fix the value in Tx/Rx Ring Buffer Size register 
ACP_BT_TX_RINGBUFSIZE,ACP_BT_RX_RINGBUFSIZE and same in I2S RINGBUFSIZE 
registers was statically set to maximum which is wrong.
Buffer size must be equal to actual allocated.

Due to which When I play any audio:aplay -Dhw:2,0 test.wav and then stop 
and play  aplay -Dhw:2,0 -c2 -fS16_LE -r48000 /dev/zero 
--buffer-size=2048 I hear some part of old audio.


So after adding above fix the issue is not reproduced.
Mark Brown Feb. 12, 2020, noon UTC | #3 
On Wed, Feb 12, 2020 at 02:36:32PM +0530, vishnu wrote:

> prior to this fix the value in Tx/Rx Ring Buffer Size register
> ACP_BT_TX_RINGBUFSIZE,ACP_BT_RX_RINGBUFSIZE and same in I2S RINGBUFSIZE
> registers was statically set to maximum which is wrong.
> Buffer size must be equal to actual allocated.

OK, makes sense - this is the sort of thing which should have been in
the commit message.
diff mbox series

Patch

diff --git a/sound/soc/amd/raven/acp3x-i2s.c b/sound/soc/amd/raven/acp3x-i2s.c
index 31cd400..91a3881 100644
--- a/sound/soc/amd/raven/acp3x-i2s.c
+++ b/sound/soc/amd/raven/acp3x-i2s.c
@@ -170,6 +170,7 @@  static int acp3x_i2s_trigger(struct snd_pcm_substream *substream,
 	struct snd_soc_card *card;
 	struct acp3x_platform_info *pinfo;
 	u32 ret, val, period_bytes, reg_val, ier_val, water_val;
+	u32 buf_size, buf_reg;
 
 	prtd = substream->private_data;
 	rtd = substream->runtime->private_data;
@@ -183,6 +184,8 @@  static int acp3x_i2s_trigger(struct snd_pcm_substream *substream,
 	}
 	period_bytes = frames_to_bytes(substream->runtime,
 			substream->runtime->period_size);
+	buf_size = frames_to_bytes(substream->runtime,
+			substream->runtime->buffer_size);
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
@@ -196,6 +199,7 @@  static int acp3x_i2s_trigger(struct snd_pcm_substream *substream,
 					mmACP_BT_TX_INTR_WATERMARK_SIZE;
 				reg_val = mmACP_BTTDM_ITER;
 				ier_val = mmACP_BTTDM_IER;
+				buf_reg = mmACP_BT_TX_RINGBUFSIZE;
 				break;
 			case I2S_SP_INSTANCE:
 			default:
@@ -203,6 +207,7 @@  static int acp3x_i2s_trigger(struct snd_pcm_substream *substream,
 					mmACP_I2S_TX_INTR_WATERMARK_SIZE;
 				reg_val = mmACP_I2STDM_ITER;
 				ier_val = mmACP_I2STDM_IER;
+				buf_reg = mmACP_I2S_TX_RINGBUFSIZE;
 			}
 		} else {
 			switch (rtd->i2s_instance) {
@@ -211,6 +216,7 @@  static int acp3x_i2s_trigger(struct snd_pcm_substream *substream,
 					mmACP_BT_RX_INTR_WATERMARK_SIZE;
 				reg_val = mmACP_BTTDM_IRER;
 				ier_val = mmACP_BTTDM_IER;
+				buf_reg = mmACP_BT_RX_RINGBUFSIZE;
 				break;
 			case I2S_SP_INSTANCE:
 			default:
@@ -218,9 +224,11 @@  static int acp3x_i2s_trigger(struct snd_pcm_substream *substream,
 					mmACP_I2S_RX_INTR_WATERMARK_SIZE;
 				reg_val = mmACP_I2STDM_IRER;
 				ier_val = mmACP_I2STDM_IER;
+				buf_reg = mmACP_I2S_RX_RINGBUFSIZE;
 			}
 		}
 		rv_writel(period_bytes, rtd->acp3x_base + water_val);
+		rv_writel(buf_size, rtd->acp3x_base + buf_reg);
 		val = rv_readl(rtd->acp3x_base + reg_val);
 		val = val | BIT(0);
 		rv_writel(val, rtd->acp3x_base + reg_val);
diff --git a/sound/soc/amd/raven/acp3x-pcm-dma.c b/sound/soc/amd/raven/acp3x-pcm-dma.c
index aecc3c0..d62c0d9 100644
--- a/sound/soc/amd/raven/acp3x-pcm-dma.c
+++ b/sound/soc/amd/raven/acp3x-pcm-dma.c
@@ -110,7 +110,7 @@  static void config_acp3x_dma(struct i2s_stream_instance *rtd, int direction)
 {
 	u16 page_idx;
 	u32 low, high, val, acp_fifo_addr, reg_fifo_addr;
-	u32 reg_ringbuf_size, reg_dma_size, reg_fifo_size;
+	u32 reg_dma_size, reg_fifo_size;
 	dma_addr_t addr;
 
 	addr = rtd->dma_addr;
@@ -157,7 +157,6 @@  static void config_acp3x_dma(struct i2s_stream_instance *rtd, int direction)
 	if (direction == SNDRV_PCM_STREAM_PLAYBACK) {
 		switch (rtd->i2s_instance) {
 		case I2S_BT_INSTANCE:
-			reg_ringbuf_size = mmACP_BT_TX_RINGBUFSIZE;
 			reg_dma_size = mmACP_BT_TX_DMA_SIZE;
 			acp_fifo_addr = ACP_SRAM_PTE_OFFSET +
 						BT_PB_FIFO_ADDR_OFFSET;
@@ -169,7 +168,6 @@  static void config_acp3x_dma(struct i2s_stream_instance *rtd, int direction)
 
 		case I2S_SP_INSTANCE:
 		default:
-			reg_ringbuf_size = mmACP_I2S_TX_RINGBUFSIZE;
 			reg_dma_size = mmACP_I2S_TX_DMA_SIZE;
 			acp_fifo_addr = ACP_SRAM_PTE_OFFSET +
 						SP_PB_FIFO_ADDR_OFFSET;
@@ -181,7 +179,6 @@  static void config_acp3x_dma(struct i2s_stream_instance *rtd, int direction)
 	} else {
 		switch (rtd->i2s_instance) {
 		case I2S_BT_INSTANCE:
-			reg_ringbuf_size = mmACP_BT_RX_RINGBUFSIZE;
 			reg_dma_size = mmACP_BT_RX_DMA_SIZE;
 			acp_fifo_addr = ACP_SRAM_PTE_OFFSET +
 						BT_CAPT_FIFO_ADDR_OFFSET;
@@ -193,7 +190,6 @@  static void config_acp3x_dma(struct i2s_stream_instance *rtd, int direction)
 
 		case I2S_SP_INSTANCE:
 		default:
-			reg_ringbuf_size = mmACP_I2S_RX_RINGBUFSIZE;
 			reg_dma_size = mmACP_I2S_RX_DMA_SIZE;
 			acp_fifo_addr = ACP_SRAM_PTE_OFFSET +
 						SP_CAPT_FIFO_ADDR_OFFSET;
@@ -203,7 +199,6 @@  static void config_acp3x_dma(struct i2s_stream_instance *rtd, int direction)
 				rtd->acp3x_base + mmACP_I2S_RX_RINGBUFADDR);
 		}
 	}
-	rv_writel(MAX_BUFFER, rtd->acp3x_base + reg_ringbuf_size);
 	rv_writel(DMA_SIZE, rtd->acp3x_base + reg_dma_size);
 	rv_writel(acp_fifo_addr, rtd->acp3x_base + reg_fifo_addr);
 	rv_writel(FIFO_SIZE, rtd->acp3x_base + reg_fifo_size);