[1/3] ASoC: amd: Report accurate hw_ptr during dma

Commit Message

Alex Deucher Nov. 3, 2017, 8:35 p.m. UTC

From: Vijendar Mukunda <[e-mail]>

Using hw register to read transmitted byte count and report
accordingly the hw pointer.

BUG=b:63121716
TEST=
modprobe snd-soc-acp-pcm.ko
modprobe snd-soc-acp-rt5645.ko
aplay <file>

Signed-off-by: Vijendar Mukunda <[e-mail]>
Signed-off-by: Akshu Agrawal <[e-mail]>
Reviewed-on: https://chromium-review.googlesource.com/659699
Commit-Ready: Akshu Agrawal <[e-mail]>
Tested-by: Akshu Agrawal <[e-mail]>
Reviewed-by: Jason Clinton <[e-mail]>
Reviewed-on: https://chromium-review.googlesource.com/676627
Signed-off-by: Alex Deucher <[e-mail]>
---
 sound/soc/amd/acp-pcm-dma.c | 71 ++++++++++++++++++++++++++++-----------------
 sound/soc/amd/acp.h         | 10 +++++++
 2 files changed, 55 insertions(+), 26 deletions(-)

Comments

Mark Brown Nov. 6, 2017, 3:48 p.m. UTC | #1

On Fri, Nov 03, 2017 at 04:35:43PM -0400, Alex Deucher wrote:

> Signed-off-by: Vijendar Mukunda <[e-mail]>
> Signed-off-by: Akshu Agrawal <[e-mail]>
> Reviewed-on: https://chromium-review.googlesource.com/659699
> Commit-Ready: Akshu Agrawal <[e-mail]>
> Tested-by: Akshu Agrawal <[e-mail]>
> Reviewed-by: Jason Clinton <[e-mail]>
> Reviewed-on: https://chromium-review.googlesource.com/676627

These two URLs are different, what was being reviewed here?  What is
Commit-Ready supposed to mean?

Mark Brown Nov. 7, 2017, 11:37 a.m. UTC | #2

On Tue, Nov 07, 2017 at 07:26:03PM +0530, Mukunda,Vijendar wrote:
> Removing URL links and commit-ready description in v2.

This doesn't really answer my question:

> > These two URLs are different, what was being reviewed here?  What is
> > Commit-Ready supposed to mean?

Please don't top post, reply in line with needed context.  This allows
readers to readily follow the flow of conversation and understand what
you are talking about and also helps ensure that everything in the
discussion is being addressed.

Mukunda,Vijendar Nov. 7, 2017, 1:56 p.m. UTC | #3

Removing URL links and commit-ready description in v2.


On Monday 06 November 2017 09:18 PM, Mark Brown wrote:
> On Fri, Nov 03, 2017 at 04:35:43PM -0400, Alex Deucher wrote:
>
>> Signed-off-by: Vijendar Mukunda <[e-mail]>
>> Signed-off-by: Akshu Agrawal <[e-mail]>
>> Reviewed-on: https://chromium-review.googlesource.com/659699
>> Commit-Ready: Akshu Agrawal <[e-mail]>
>> Tested-by: Akshu Agrawal <[e-mail]>
>> Reviewed-by: Jason Clinton <[e-mail]>
>> Reviewed-on: https://chromium-review.googlesource.com/676627
> These two URLs are different, what was being reviewed here?  What is
> Commit-Ready supposed to mean?

Akshu Agrawal Nov. 7, 2017, 4:04 p.m. UTC | #4

On 11/7/2017 5:07 PM, Mark Brown wrote:
> On Tue, Nov 07, 2017 at 07:26:03PM +0530, Mukunda,Vijendar wrote:
>> Removing URL links and commit-ready description in v2.
> 
> This doesn't really answer my question:
> 
>>> These two URLs are different, what was being reviewed here?  What is
>>> Commit-Ready supposed to mean?

Same patch is reviewed, once on 4.4 kernel (659699) and then on 4.12 
kernel (672267).
Commit-ready is to get it merged on tree after receiving a +2.

> 
> Please don't top post, reply in line with needed context.  This allows
> readers to readily follow the flow of conversation and understand what
> you are talking about and also helps ensure that everything in the
> discussion is being addressed.
>

Mark Brown Nov. 8, 2017, 4:05 p.m. UTC | #5

On Tue, Nov 07, 2017 at 09:34:35PM +0530, Agrawal, Akshu wrote:
> On 11/7/2017 5:07 PM, Mark Brown wrote:

> > > > These two URLs are different, what was being reviewed here?  What is
> > > > Commit-Ready supposed to mean?

> Same patch is reviewed, once on 4.4 kernel (659699) and then on 4.12 kernel
> (672267).
> Commit-ready is to get it merged on tree after receiving a +2.

OK, that's not unreasonable but probably best to mention if the review
was in the context of a very old kernel, sometimes upstream changes can
invalidate the review.

Patch

diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index 73b58ee..e19f281 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -817,39 +817,48 @@  static int acp_dma_hw_free(struct snd_pcm_substream *substream)
 	return snd_pcm_lib_free_pages(substream);
 }
 
+static u64 acp_get_byte_count(void __iomem *acp_mmio, int stream)
+{
+	union acp_dma_count playback_dma_count;
+	union acp_dma_count capture_dma_count;
+	u64 bytescount = 0;
+
+	if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		playback_dma_count.bcount.high = acp_reg_read(acp_mmio,
+					mmACP_I2S_TRANSMIT_BYTE_CNT_HIGH);
+		playback_dma_count.bcount.low  = acp_reg_read(acp_mmio,
+					mmACP_I2S_TRANSMIT_BYTE_CNT_LOW);
+		bytescount = playback_dma_count.bytescount;
+	} else {
+		capture_dma_count.bcount.high = acp_reg_read(acp_mmio,
+					mmACP_I2S_RECEIVED_BYTE_CNT_HIGH);
+		capture_dma_count.bcount.low  = acp_reg_read(acp_mmio,
+					mmACP_I2S_RECEIVED_BYTE_CNT_LOW);
+		bytescount = capture_dma_count.bytescount;
+	}
+	return bytescount;
+}
+
 static snd_pcm_uframes_t acp_dma_pointer(struct snd_pcm_substream *substream)
 {
-	u16 dscr;
-	u32 mul, dma_config, period_bytes;
+	u32 buffersize;
 	u32 pos = 0;
+	u64 bytescount = 0;
 
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct audio_substream_data *rtd = runtime->private_data;
 
-	period_bytes = frames_to_bytes(runtime, runtime->period_size);
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		dscr = acp_reg_read(rtd->acp_mmio, mmACP_DMA_CUR_DSCR_13);
+	buffersize = frames_to_bytes(runtime, runtime->buffer_size);
+	bytescount = acp_get_byte_count(rtd->acp_mmio, substream->stream);
 
-		if (dscr == PLAYBACK_START_DMA_DESCR_CH13)
-			mul = 0;
-		else
-			mul = 1;
-		pos =  (mul * period_bytes);
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		if (bytescount > rtd->renderbytescount)
+			bytescount = bytescount - rtd->renderbytescount;
+		pos =  bytescount % buffersize;
 	} else {
-		dma_config = acp_reg_read(rtd->acp_mmio, mmACP_DMA_CNTL_14);
-		if (dma_config != 0) {
-			dscr = acp_reg_read(rtd->acp_mmio,
-						mmACP_DMA_CUR_DSCR_14);
-			if (dscr == CAPTURE_START_DMA_DESCR_CH14)
-				mul = 1;
-			else
-				mul = 2;
-			pos = (mul * period_bytes);
-		}
-
-		if (pos >= (2 * period_bytes))
-			pos = 0;
-
+		if (bytescount > rtd->capturebytescount)
+			bytescount = bytescount - rtd->capturebytescount;
+		pos = bytescount % buffersize;
 	}
 	return bytes_to_frames(runtime, pos);
 }
@@ -904,6 +913,7 @@  static int acp_dma_trigger(struct snd_pcm_substream *substream, int cmd)
 {
 	int ret;
 	u32 loops = 1000;
+	u64 bytescount = 0;
 
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_soc_pcm_runtime *prtd = substream->private_data;
@@ -915,7 +925,11 @@  static int acp_dma_trigger(struct snd_pcm_substream *substream, int cmd)
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 	case SNDRV_PCM_TRIGGER_RESUME:
+		bytescount = acp_get_byte_count(rtd->acp_mmio,
+						substream->stream);
 		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+			if (rtd->renderbytescount == 0)
+				rtd->renderbytescount = bytescount;
 			acp_dma_start(rtd->acp_mmio,
 						SYSRAM_TO_ACP_CH_NUM, false);
 			while (acp_reg_read(rtd->acp_mmio, mmACP_DMA_CH_STS) &
@@ -932,6 +946,8 @@  static int acp_dma_trigger(struct snd_pcm_substream *substream, int cmd)
 					ACP_TO_I2S_DMA_CH_NUM, true);
 
 		} else {
+			if (rtd->capturebytescount == 0)
+				rtd->capturebytescount = bytescount;
 			acp_dma_start(rtd->acp_mmio,
 					    I2S_TO_ACP_DMA_CH_NUM, true);
 		}
@@ -945,12 +961,15 @@  static int acp_dma_trigger(struct snd_pcm_substream *substream, int cmd)
 		 * channels will stopped automatically after its transfer
 		 * completes : SYSRAM_TO_ACP_CH_NUM / ACP_TO_SYSRAM_CH_NUM
 		 */
-		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 			ret = acp_dma_stop(rtd->acp_mmio,
 					ACP_TO_I2S_DMA_CH_NUM);
-		else
+			rtd->renderbytescount = 0;
+		} else {
 			ret = acp_dma_stop(rtd->acp_mmio,
 					I2S_TO_ACP_DMA_CH_NUM);
+			rtd->capturebytescount = 0;
+		}
 		break;
 	default:
 		ret = -EINVAL;
diff --git a/sound/soc/amd/acp.h b/sound/soc/amd/acp.h
index a330a99..de08ff0 100644
--- a/sound/soc/amd/acp.h
+++ b/sound/soc/amd/acp.h
@@ -83,6 +83,8 @@  struct audio_substream_data {
 	u16 num_of_pages;
 	u16 direction;
 	uint64_t size;
+	u64 renderbytescount;
+	u64 capturebytescount;
 	void __iomem *acp_mmio;
 };
 
@@ -93,6 +95,14 @@  struct audio_drv_data {
 	u32 asic_type;
 };
 
+union acp_dma_count {
+	struct {
+	u32 low;
+	u32 high;
+	} bcount;
+	u64 bytescount;
+};
+
 enum {
 	ACP_TILE_P1 = 0,
 	ACP_TILE_P2,