All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] ASoC: amd: acp dma driver code cleanup
@ 2018-04-24  3:04 ` Vijendar Mukunda
  0 siblings, 0 replies; 10+ messages in thread
From: Vijendar Mukunda @ 2018-04-24  3:04 UTC (permalink / raw)
  Cc: Vijendar Mukunda, Liam Girdwood, Mark Brown, Jaroslav Kysela,
	Takashi Iwai, Alex Deucher, Akshu Agrawal, Jason Clinton,
	Guenter Roeck, Kuninori Morimoto, Greg Kroah-Hartman,
	moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	open list

Added dma configuration parameters in audio_substream_data
structure. Moved dma configuration parameters initialization
to dma hw params callback.
Removed separate byte count variables for playback and capture.
Added variables to store ACP register offsets in audio_substream_data
structure.

Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
---
 sound/soc/amd/acp-pcm-dma.c | 241 ++++++++++++++++++--------------------------
 sound/soc/amd/acp.h         |  35 +++++--
 2 files changed, 126 insertions(+), 150 deletions(-)

diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index 5ffe2ef..4a4bbdf 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -317,54 +317,21 @@ static void acp_pte_config(void __iomem *acp_mmio, struct page *pg,
 }
 
 static void config_acp_dma(void __iomem *acp_mmio,
-			   struct audio_substream_data *audio_config,
+			   struct audio_substream_data *rtd,
 			   u32 asic_type)
 {
-	u32 pte_offset, sram_bank;
-	u16 ch1, ch2, destination, dma_dscr_idx;
-
-	if (audio_config->direction == SNDRV_PCM_STREAM_PLAYBACK) {
-		pte_offset = ACP_PLAYBACK_PTE_OFFSET;
-		ch1 = SYSRAM_TO_ACP_CH_NUM;
-		ch2 = ACP_TO_I2S_DMA_CH_NUM;
-		sram_bank = ACP_SHARED_RAM_BANK_1_ADDRESS;
-		destination = TO_ACP_I2S_1;
-
-	} else {
-		pte_offset = ACP_CAPTURE_PTE_OFFSET;
-		ch1 = SYSRAM_TO_ACP_CH_NUM;
-		ch2 = ACP_TO_I2S_DMA_CH_NUM;
-		switch (asic_type) {
-		case CHIP_STONEY:
-			sram_bank = ACP_SHARED_RAM_BANK_3_ADDRESS;
-			break;
-		default:
-			sram_bank = ACP_SHARED_RAM_BANK_5_ADDRESS;
-		}
-		destination = FROM_ACP_I2S_1;
-	}
-
-	acp_pte_config(acp_mmio, audio_config->pg, audio_config->num_of_pages,
-		       pte_offset);
-	if (audio_config->direction == SNDRV_PCM_STREAM_PLAYBACK)
-		dma_dscr_idx = PLAYBACK_START_DMA_DESCR_CH12;
-	else
-		dma_dscr_idx = CAPTURE_START_DMA_DESCR_CH14;
-
+	acp_pte_config(acp_mmio, rtd->pg, rtd->num_of_pages,
+		       rtd->pte_offset);
 	/* Configure System memory <-> ACP SRAM DMA descriptors */
-	set_acp_sysmem_dma_descriptors(acp_mmio, audio_config->size,
-				       audio_config->direction, pte_offset, ch1,
-				       sram_bank, dma_dscr_idx, asic_type);
-
-	if (audio_config->direction == SNDRV_PCM_STREAM_PLAYBACK)
-		dma_dscr_idx = PLAYBACK_START_DMA_DESCR_CH13;
-	else
-		dma_dscr_idx = CAPTURE_START_DMA_DESCR_CH15;
+	set_acp_sysmem_dma_descriptors(acp_mmio, rtd->size,
+				       rtd->direction, rtd->pte_offset,
+				       rtd->ch1, rtd->sram_bank,
+				       rtd->dma_dscr_idx_1, asic_type);
 	/* Configure ACP SRAM <-> I2S DMA descriptors */
-	set_acp_to_i2s_dma_descriptors(acp_mmio, audio_config->size,
-				       audio_config->direction, sram_bank,
-				       destination, ch2, dma_dscr_idx,
-				       asic_type);
+	set_acp_to_i2s_dma_descriptors(acp_mmio, rtd->size,
+				       rtd->direction, rtd->sram_bank,
+				       rtd->destination, rtd->ch2,
+				       rtd->dma_dscr_idx_2, asic_type);
 }
 
 /* Start a given DMA channel transfer */
@@ -700,7 +667,6 @@ static irqreturn_t dma_irq_handler(int irq, void *arg)
 
 static int acp_dma_open(struct snd_pcm_substream *substream)
 {
-	u16 bank;
 	int ret = 0;
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_soc_pcm_runtime *prtd = substream->private_data;
@@ -720,6 +686,7 @@ static int acp_dma_open(struct snd_pcm_substream *substream)
 		default:
 			runtime->hw = acp_pcm_hardware_playback;
 		}
+		adata->bytescount = 0;
 	} else {
 		switch (intr_data->asic_type) {
 		case CHIP_STONEY:
@@ -728,6 +695,7 @@ static int acp_dma_open(struct snd_pcm_substream *substream)
 		default:
 			runtime->hw = acp_pcm_hardware_capture;
 		}
+		adata->bytescount = 0;
 	}
 
 	ret = snd_pcm_hw_constraint_integer(runtime,
@@ -749,28 +717,6 @@ static int acp_dma_open(struct snd_pcm_substream *substream)
 	 */
 	if (!intr_data->play_i2ssp_stream && !intr_data->capture_i2ssp_stream)
 		acp_reg_write(1, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB);
-
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		intr_data->play_i2ssp_stream = substream;
-		/*
-		 * For Stoney, Memory gating is disabled,i.e SRAM Banks
-		 * won't be turned off. The default state for SRAM banks is ON.
-		 * Setting SRAM bank state code skipped for STONEY platform.
-		 */
-		if (intr_data->asic_type != CHIP_STONEY) {
-			for (bank = 1; bank <= 4; bank++)
-				acp_set_sram_bank_state(intr_data->acp_mmio,
-							bank, true);
-		}
-	} else {
-		intr_data->capture_i2ssp_stream = substream;
-		if (intr_data->asic_type != CHIP_STONEY) {
-			for (bank = 5; bank <= 8; bank++)
-				acp_set_sram_bank_state(intr_data->acp_mmio,
-							bank, true);
-		}
-	}
-
 	return 0;
 }
 
@@ -779,6 +725,7 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
 {
 	int status;
 	uint64_t size;
+	u16 bank;
 	u32 val = 0;
 	struct page *pg;
 	struct snd_pcm_runtime *runtime;
@@ -804,6 +751,60 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
 		acp_reg_write(val, adata->acp_mmio,
 			      mmACP_I2S_16BIT_RESOLUTION_EN);
 	}
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		switch (adata->asic_type) {
+		case CHIP_STONEY:
+			rtd->pte_offset = ACP_ST_PLAYBACK_PTE_OFFSET;
+			break;
+		default:
+			rtd->pte_offset = ACP_PLAYBACK_PTE_OFFSET;
+		}
+		rtd->ch1 = SYSRAM_TO_ACP_CH_NUM;
+		rtd->ch2 = ACP_TO_I2S_DMA_CH_NUM;
+		rtd->sram_bank = ACP_SRAM_BANK_1_ADDRESS;
+		rtd->destination = TO_ACP_I2S_1;
+		rtd->dma_dscr_idx_1 = PLAYBACK_START_DMA_DESCR_CH12;
+		rtd->dma_dscr_idx_2 = PLAYBACK_START_DMA_DESCR_CH13;
+		rtd->byte_cnt_high_reg_offset =
+		mmACP_I2S_TRANSMIT_BYTE_CNT_HIGH;
+		rtd->byte_cnt_low_reg_offset = mmACP_I2S_TRANSMIT_BYTE_CNT_LOW;
+		adata->play_i2ssp_stream = substream;
+		/*
+		 * For Stoney, Memory gating is disabled,i.e SRAM Banks
+		 * won't be turned off. The default state for SRAM banks is ON.
+		 * Setting SRAM bank state code skipped for STONEY platform.
+		 */
+		if (adata->asic_type != CHIP_STONEY) {
+			for (bank = 1; bank <= 4; bank++)
+				acp_set_sram_bank_state(adata->acp_mmio,
+							bank, true);
+		}
+	} else {
+		rtd->pte_offset = ACP_CAPTURE_PTE_OFFSET;
+		rtd->ch1 = SYSRAM_TO_ACP_CH_NUM;
+		rtd->ch2 = ACP_TO_I2S_DMA_CH_NUM;
+		switch (adata->asic_type) {
+		case CHIP_STONEY:
+			rtd->sram_bank = ACP_SRAM_BANK_2_ADDRESS;
+			break;
+		default:
+			rtd->sram_bank = ACP_SRAM_BANK_5_ADDRESS;
+		}
+		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;
+		adata->capture_i2ssp_stream = substream;
+		if (adata->asic_type != CHIP_STONEY) {
+			for (bank = 5; bank <= 8; bank++)
+				acp_set_sram_bank_state(adata->acp_mmio,
+							bank, true);
+		}
+	}
+
 	size = params_buffer_bytes(params);
 	status = snd_pcm_lib_malloc_pages(substream, size);
 	if (status < 0)
@@ -837,26 +838,15 @@ 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)
+static u64 acp_get_byte_count(struct audio_substream_data *rtd)
 {
-	union acp_dma_count playback_dma_count;
-	union acp_dma_count capture_dma_count;
-	u64 bytescount = 0;
+	union acp_dma_count byte_count;
 
-	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;
+	byte_count.bcount.high = acp_reg_read(rtd->acp_mmio,
+					      rtd->byte_cnt_high_reg_offset);
+	byte_count.bcount.low  = acp_reg_read(rtd->acp_mmio,
+					      rtd->byte_cnt_low_reg_offset);
+	return byte_count.bytescount;
 }
 
 static snd_pcm_uframes_t acp_dma_pointer(struct snd_pcm_substream *substream)
@@ -872,15 +862,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->acp_mmio, substream->stream);
+	bytescount = acp_get_byte_count(rtd);
 
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		if (bytescount > rtd->i2ssp_renderbytescount)
-			bytescount = bytescount - rtd->i2ssp_renderbytescount;
-	} else {
-		if (bytescount > rtd->i2ssp_capturebytescount)
-			bytescount = bytescount - rtd->i2ssp_capturebytescount;
-	}
+	if (bytescount > rtd->bytescount)
+		bytescount = bytescount - rtd->bytescount;
 	pos = do_div(bytescount, buffersize);
 	return bytes_to_frames(runtime, pos);
 }
@@ -898,21 +883,15 @@ static int acp_dma_prepare(struct snd_pcm_substream *substream)
 
 	if (!rtd)
 		return -EINVAL;
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		config_acp_dma_channel(rtd->acp_mmio, SYSRAM_TO_ACP_CH_NUM,
-				       PLAYBACK_START_DMA_DESCR_CH12,
-				       NUM_DSCRS_PER_CHANNEL, 0);
-		config_acp_dma_channel(rtd->acp_mmio, ACP_TO_I2S_DMA_CH_NUM,
-				       PLAYBACK_START_DMA_DESCR_CH13,
-				       NUM_DSCRS_PER_CHANNEL, 0);
-	} else {
-		config_acp_dma_channel(rtd->acp_mmio, ACP_TO_SYSRAM_CH_NUM,
-				       CAPTURE_START_DMA_DESCR_CH14,
-				       NUM_DSCRS_PER_CHANNEL, 0);
-		config_acp_dma_channel(rtd->acp_mmio, I2S_TO_ACP_DMA_CH_NUM,
-				       CAPTURE_START_DMA_DESCR_CH15,
-				       NUM_DSCRS_PER_CHANNEL, 0);
-	}
+
+	config_acp_dma_channel(rtd->acp_mmio,
+			       rtd->ch1,
+			       rtd->dma_dscr_idx_1,
+			       NUM_DSCRS_PER_CHANNEL, 0);
+	config_acp_dma_channel(rtd->acp_mmio,
+			       rtd->ch2,
+			       rtd->dma_dscr_idx_2,
+			       NUM_DSCRS_PER_CHANNEL, 0);
 	return 0;
 }
 
@@ -934,15 +913,13 @@ 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);
+		bytescount = acp_get_byte_count(rtd);
+		if (rtd->bytescount == 0)
+			rtd->bytescount = bytescount;
 		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-			if (rtd->i2ssp_renderbytescount == 0)
-				rtd->i2ssp_renderbytescount = bytescount;
-			acp_dma_start(rtd->acp_mmio,
-				      SYSRAM_TO_ACP_CH_NUM, false);
+			acp_dma_start(rtd->acp_mmio, rtd->ch1, false);
 			while (acp_reg_read(rtd->acp_mmio, mmACP_DMA_CH_STS) &
-						BIT(SYSRAM_TO_ACP_CH_NUM)) {
+				BIT(rtd->ch1)) {
 				if (!loops--) {
 					dev_err(component->dev,
 						"acp dma start timeout\n");
@@ -950,40 +927,21 @@ static int acp_dma_trigger(struct snd_pcm_substream *substream, int cmd)
 				}
 				cpu_relax();
 			}
-
-			acp_dma_start(rtd->acp_mmio,
-				      ACP_TO_I2S_DMA_CH_NUM, true);
-
-		} else {
-			if (rtd->i2ssp_capturebytescount == 0)
-				rtd->i2ssp_capturebytescount = bytescount;
-			acp_dma_start(rtd->acp_mmio,
-				      I2S_TO_ACP_DMA_CH_NUM, true);
 		}
+		acp_dma_start(rtd->acp_mmio, rtd->ch2, true);
 		ret = 0;
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
-		/*
-		 * Need to stop only circular DMA channels :
-		 * ACP_TO_I2S_DMA_CH_NUM / I2S_TO_ACP_DMA_CH_NUM. Non-circular
-		 * 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) {
-			ret = acp_dma_stop(rtd->acp_mmio,
-					   SYSRAM_TO_ACP_CH_NUM);
-			ret = acp_dma_stop(rtd->acp_mmio,
-					   ACP_TO_I2S_DMA_CH_NUM);
-			rtd->i2ssp_renderbytescount = 0;
+			acp_dma_stop(rtd->acp_mmio, rtd->ch1);
+			ret =  acp_dma_stop(rtd->acp_mmio, rtd->ch2);
 		} else {
-			ret = acp_dma_stop(rtd->acp_mmio,
-					   I2S_TO_ACP_DMA_CH_NUM);
-			ret = acp_dma_stop(rtd->acp_mmio,
-					   ACP_TO_SYSRAM_CH_NUM);
-			rtd->i2ssp_capturebytescount = 0;
+			acp_dma_stop(rtd->acp_mmio, rtd->ch2);
+			ret = acp_dma_stop(rtd->acp_mmio, rtd->ch1);
 		}
+		rtd->bytescount = 0;
 		break;
 	default:
 		ret = -EINVAL;
@@ -1028,8 +986,6 @@ static int acp_dma_close(struct snd_pcm_substream *substream)
 								    DRV_NAME);
 	struct audio_drv_data *adata = dev_get_drvdata(component->dev);
 
-	kfree(rtd);
-
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		adata->play_i2ssp_stream = NULL;
 		/*
@@ -1059,6 +1015,7 @@ static int acp_dma_close(struct snd_pcm_substream *substream)
 	if (!adata->play_i2ssp_stream && !adata->capture_i2ssp_stream)
 		acp_reg_write(0, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB);
 
+	kfree(rtd);
 	return 0;
 }
 
diff --git a/sound/soc/amd/acp.h b/sound/soc/amd/acp.h
index 0e6089b..62695ed 100644
--- a/sound/soc/amd/acp.h
+++ b/sound/soc/amd/acp.h
@@ -10,17 +10,28 @@
 #define ACP_PLAYBACK_PTE_OFFSET			10
 #define ACP_CAPTURE_PTE_OFFSET			0
 
+/* Playback and Capture Offset for Stoney */
+#define ACP_ST_PLAYBACK_PTE_OFFSET	0x04
+#define ACP_ST_CAPTURE_PTE_OFFSET	0x00
+
 #define ACP_GARLIC_CNTL_DEFAULT			0x00000FB4
 #define ACP_ONION_CNTL_DEFAULT			0x00000FB4
 
 #define ACP_PHYSICAL_BASE			0x14000
 
-/* Playback SRAM address (as a destination in dma descriptor) */
-#define ACP_SHARED_RAM_BANK_1_ADDRESS		0x4002000
-
-/* Capture SRAM address (as a source in dma descriptor) */
-#define ACP_SHARED_RAM_BANK_5_ADDRESS		0x400A000
-#define ACP_SHARED_RAM_BANK_3_ADDRESS		0x4006000
+/*
+ * In case of I2S SP controller instance, Stoney uses SRAM bank 1 for
+ * playback and SRAM Bank 2 for capture where as in case of BT I2S
+ * Instance, Stoney uses SRAM Bank 3 for playback & SRAM Bank 4 will
+ * be used for capture. Carrizo uses I2S SP controller instance. SRAM Banks
+ * 1, 2, 3, 4 will be used for playback & SRAM Banks 5, 6, 7, 8 will be used
+ * for capture scenario.
+ */
+#define ACP_SRAM_BANK_1_ADDRESS		0x4002000
+#define ACP_SRAM_BANK_2_ADDRESS		0x4004000
+#define ACP_SRAM_BANK_3_ADDRESS		0x4006000
+#define ACP_SRAM_BANK_4_ADDRESS		0x4008000
+#define ACP_SRAM_BANK_5_ADDRESS		0x400A000
 
 #define ACP_DMA_RESET_TIME			10000
 #define ACP_CLOCK_EN_TIME_OUT_VALUE		0x000000FF
@@ -85,9 +96,17 @@ struct audio_substream_data {
 	unsigned int order;
 	u16 num_of_pages;
 	u16 direction;
+	u16 ch1;
+	u16 ch2;
+	u16 destination;
+	u16 dma_dscr_idx_1;
+	u16 dma_dscr_idx_2;
+	u32 pte_offset;
+	u32 sram_bank;
+	u32 byte_cnt_high_reg_offset;
+	u32 byte_cnt_low_reg_offset;
 	uint64_t size;
-	u64 i2ssp_renderbytescount;
-	u64 i2ssp_capturebytescount;
+	u64 bytescount;
 	void __iomem *acp_mmio;
 };
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 1/3] ASoC: amd: acp dma driver code cleanup
@ 2018-04-24  3:04 ` Vijendar Mukunda
  0 siblings, 0 replies; 10+ messages in thread
From: Vijendar Mukunda @ 2018-04-24  3:04 UTC (permalink / raw)
  Cc: Vijendar Mukunda, Liam Girdwood, Mark Brown, Jaroslav Kysela,
	Takashi Iwai, Alex Deucher, Akshu Agrawal, Jason Clinton,
	Guenter Roeck, Kuninori Morimoto, Greg Kroah-Hartman,
	moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	open list

Added dma configuration parameters in audio_substream_data
structure. Moved dma configuration parameters initialization
to dma hw params callback.
Removed separate byte count variables for playback and capture.
Added variables to store ACP register offsets in audio_substream_data
structure.

Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
---
 sound/soc/amd/acp-pcm-dma.c | 241 ++++++++++++++++++--------------------------
 sound/soc/amd/acp.h         |  35 +++++--
 2 files changed, 126 insertions(+), 150 deletions(-)

diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index 5ffe2ef..4a4bbdf 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -317,54 +317,21 @@ static void acp_pte_config(void __iomem *acp_mmio, struct page *pg,
 }
 
 static void config_acp_dma(void __iomem *acp_mmio,
-			   struct audio_substream_data *audio_config,
+			   struct audio_substream_data *rtd,
 			   u32 asic_type)
 {
-	u32 pte_offset, sram_bank;
-	u16 ch1, ch2, destination, dma_dscr_idx;
-
-	if (audio_config->direction == SNDRV_PCM_STREAM_PLAYBACK) {
-		pte_offset = ACP_PLAYBACK_PTE_OFFSET;
-		ch1 = SYSRAM_TO_ACP_CH_NUM;
-		ch2 = ACP_TO_I2S_DMA_CH_NUM;
-		sram_bank = ACP_SHARED_RAM_BANK_1_ADDRESS;
-		destination = TO_ACP_I2S_1;
-
-	} else {
-		pte_offset = ACP_CAPTURE_PTE_OFFSET;
-		ch1 = SYSRAM_TO_ACP_CH_NUM;
-		ch2 = ACP_TO_I2S_DMA_CH_NUM;
-		switch (asic_type) {
-		case CHIP_STONEY:
-			sram_bank = ACP_SHARED_RAM_BANK_3_ADDRESS;
-			break;
-		default:
-			sram_bank = ACP_SHARED_RAM_BANK_5_ADDRESS;
-		}
-		destination = FROM_ACP_I2S_1;
-	}
-
-	acp_pte_config(acp_mmio, audio_config->pg, audio_config->num_of_pages,
-		       pte_offset);
-	if (audio_config->direction == SNDRV_PCM_STREAM_PLAYBACK)
-		dma_dscr_idx = PLAYBACK_START_DMA_DESCR_CH12;
-	else
-		dma_dscr_idx = CAPTURE_START_DMA_DESCR_CH14;
-
+	acp_pte_config(acp_mmio, rtd->pg, rtd->num_of_pages,
+		       rtd->pte_offset);
 	/* Configure System memory <-> ACP SRAM DMA descriptors */
-	set_acp_sysmem_dma_descriptors(acp_mmio, audio_config->size,
-				       audio_config->direction, pte_offset, ch1,
-				       sram_bank, dma_dscr_idx, asic_type);
-
-	if (audio_config->direction == SNDRV_PCM_STREAM_PLAYBACK)
-		dma_dscr_idx = PLAYBACK_START_DMA_DESCR_CH13;
-	else
-		dma_dscr_idx = CAPTURE_START_DMA_DESCR_CH15;
+	set_acp_sysmem_dma_descriptors(acp_mmio, rtd->size,
+				       rtd->direction, rtd->pte_offset,
+				       rtd->ch1, rtd->sram_bank,
+				       rtd->dma_dscr_idx_1, asic_type);
 	/* Configure ACP SRAM <-> I2S DMA descriptors */
-	set_acp_to_i2s_dma_descriptors(acp_mmio, audio_config->size,
-				       audio_config->direction, sram_bank,
-				       destination, ch2, dma_dscr_idx,
-				       asic_type);
+	set_acp_to_i2s_dma_descriptors(acp_mmio, rtd->size,
+				       rtd->direction, rtd->sram_bank,
+				       rtd->destination, rtd->ch2,
+				       rtd->dma_dscr_idx_2, asic_type);
 }
 
 /* Start a given DMA channel transfer */
@@ -700,7 +667,6 @@ static irqreturn_t dma_irq_handler(int irq, void *arg)
 
 static int acp_dma_open(struct snd_pcm_substream *substream)
 {
-	u16 bank;
 	int ret = 0;
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_soc_pcm_runtime *prtd = substream->private_data;
@@ -720,6 +686,7 @@ static int acp_dma_open(struct snd_pcm_substream *substream)
 		default:
 			runtime->hw = acp_pcm_hardware_playback;
 		}
+		adata->bytescount = 0;
 	} else {
 		switch (intr_data->asic_type) {
 		case CHIP_STONEY:
@@ -728,6 +695,7 @@ static int acp_dma_open(struct snd_pcm_substream *substream)
 		default:
 			runtime->hw = acp_pcm_hardware_capture;
 		}
+		adata->bytescount = 0;
 	}
 
 	ret = snd_pcm_hw_constraint_integer(runtime,
@@ -749,28 +717,6 @@ static int acp_dma_open(struct snd_pcm_substream *substream)
 	 */
 	if (!intr_data->play_i2ssp_stream && !intr_data->capture_i2ssp_stream)
 		acp_reg_write(1, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB);
-
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		intr_data->play_i2ssp_stream = substream;
-		/*
-		 * For Stoney, Memory gating is disabled,i.e SRAM Banks
-		 * won't be turned off. The default state for SRAM banks is ON.
-		 * Setting SRAM bank state code skipped for STONEY platform.
-		 */
-		if (intr_data->asic_type != CHIP_STONEY) {
-			for (bank = 1; bank <= 4; bank++)
-				acp_set_sram_bank_state(intr_data->acp_mmio,
-							bank, true);
-		}
-	} else {
-		intr_data->capture_i2ssp_stream = substream;
-		if (intr_data->asic_type != CHIP_STONEY) {
-			for (bank = 5; bank <= 8; bank++)
-				acp_set_sram_bank_state(intr_data->acp_mmio,
-							bank, true);
-		}
-	}
-
 	return 0;
 }
 
@@ -779,6 +725,7 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
 {
 	int status;
 	uint64_t size;
+	u16 bank;
 	u32 val = 0;
 	struct page *pg;
 	struct snd_pcm_runtime *runtime;
@@ -804,6 +751,60 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
 		acp_reg_write(val, adata->acp_mmio,
 			      mmACP_I2S_16BIT_RESOLUTION_EN);
 	}
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		switch (adata->asic_type) {
+		case CHIP_STONEY:
+			rtd->pte_offset = ACP_ST_PLAYBACK_PTE_OFFSET;
+			break;
+		default:
+			rtd->pte_offset = ACP_PLAYBACK_PTE_OFFSET;
+		}
+		rtd->ch1 = SYSRAM_TO_ACP_CH_NUM;
+		rtd->ch2 = ACP_TO_I2S_DMA_CH_NUM;
+		rtd->sram_bank = ACP_SRAM_BANK_1_ADDRESS;
+		rtd->destination = TO_ACP_I2S_1;
+		rtd->dma_dscr_idx_1 = PLAYBACK_START_DMA_DESCR_CH12;
+		rtd->dma_dscr_idx_2 = PLAYBACK_START_DMA_DESCR_CH13;
+		rtd->byte_cnt_high_reg_offset =
+		mmACP_I2S_TRANSMIT_BYTE_CNT_HIGH;
+		rtd->byte_cnt_low_reg_offset = mmACP_I2S_TRANSMIT_BYTE_CNT_LOW;
+		adata->play_i2ssp_stream = substream;
+		/*
+		 * For Stoney, Memory gating is disabled,i.e SRAM Banks
+		 * won't be turned off. The default state for SRAM banks is ON.
+		 * Setting SRAM bank state code skipped for STONEY platform.
+		 */
+		if (adata->asic_type != CHIP_STONEY) {
+			for (bank = 1; bank <= 4; bank++)
+				acp_set_sram_bank_state(adata->acp_mmio,
+							bank, true);
+		}
+	} else {
+		rtd->pte_offset = ACP_CAPTURE_PTE_OFFSET;
+		rtd->ch1 = SYSRAM_TO_ACP_CH_NUM;
+		rtd->ch2 = ACP_TO_I2S_DMA_CH_NUM;
+		switch (adata->asic_type) {
+		case CHIP_STONEY:
+			rtd->sram_bank = ACP_SRAM_BANK_2_ADDRESS;
+			break;
+		default:
+			rtd->sram_bank = ACP_SRAM_BANK_5_ADDRESS;
+		}
+		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;
+		adata->capture_i2ssp_stream = substream;
+		if (adata->asic_type != CHIP_STONEY) {
+			for (bank = 5; bank <= 8; bank++)
+				acp_set_sram_bank_state(adata->acp_mmio,
+							bank, true);
+		}
+	}
+
 	size = params_buffer_bytes(params);
 	status = snd_pcm_lib_malloc_pages(substream, size);
 	if (status < 0)
@@ -837,26 +838,15 @@ 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)
+static u64 acp_get_byte_count(struct audio_substream_data *rtd)
 {
-	union acp_dma_count playback_dma_count;
-	union acp_dma_count capture_dma_count;
-	u64 bytescount = 0;
+	union acp_dma_count byte_count;
 
-	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;
+	byte_count.bcount.high = acp_reg_read(rtd->acp_mmio,
+					      rtd->byte_cnt_high_reg_offset);
+	byte_count.bcount.low  = acp_reg_read(rtd->acp_mmio,
+					      rtd->byte_cnt_low_reg_offset);
+	return byte_count.bytescount;
 }
 
 static snd_pcm_uframes_t acp_dma_pointer(struct snd_pcm_substream *substream)
@@ -872,15 +862,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->acp_mmio, substream->stream);
+	bytescount = acp_get_byte_count(rtd);
 
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		if (bytescount > rtd->i2ssp_renderbytescount)
-			bytescount = bytescount - rtd->i2ssp_renderbytescount;
-	} else {
-		if (bytescount > rtd->i2ssp_capturebytescount)
-			bytescount = bytescount - rtd->i2ssp_capturebytescount;
-	}
+	if (bytescount > rtd->bytescount)
+		bytescount = bytescount - rtd->bytescount;
 	pos = do_div(bytescount, buffersize);
 	return bytes_to_frames(runtime, pos);
 }
@@ -898,21 +883,15 @@ static int acp_dma_prepare(struct snd_pcm_substream *substream)
 
 	if (!rtd)
 		return -EINVAL;
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		config_acp_dma_channel(rtd->acp_mmio, SYSRAM_TO_ACP_CH_NUM,
-				       PLAYBACK_START_DMA_DESCR_CH12,
-				       NUM_DSCRS_PER_CHANNEL, 0);
-		config_acp_dma_channel(rtd->acp_mmio, ACP_TO_I2S_DMA_CH_NUM,
-				       PLAYBACK_START_DMA_DESCR_CH13,
-				       NUM_DSCRS_PER_CHANNEL, 0);
-	} else {
-		config_acp_dma_channel(rtd->acp_mmio, ACP_TO_SYSRAM_CH_NUM,
-				       CAPTURE_START_DMA_DESCR_CH14,
-				       NUM_DSCRS_PER_CHANNEL, 0);
-		config_acp_dma_channel(rtd->acp_mmio, I2S_TO_ACP_DMA_CH_NUM,
-				       CAPTURE_START_DMA_DESCR_CH15,
-				       NUM_DSCRS_PER_CHANNEL, 0);
-	}
+
+	config_acp_dma_channel(rtd->acp_mmio,
+			       rtd->ch1,
+			       rtd->dma_dscr_idx_1,
+			       NUM_DSCRS_PER_CHANNEL, 0);
+	config_acp_dma_channel(rtd->acp_mmio,
+			       rtd->ch2,
+			       rtd->dma_dscr_idx_2,
+			       NUM_DSCRS_PER_CHANNEL, 0);
 	return 0;
 }
 
@@ -934,15 +913,13 @@ 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);
+		bytescount = acp_get_byte_count(rtd);
+		if (rtd->bytescount == 0)
+			rtd->bytescount = bytescount;
 		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-			if (rtd->i2ssp_renderbytescount == 0)
-				rtd->i2ssp_renderbytescount = bytescount;
-			acp_dma_start(rtd->acp_mmio,
-				      SYSRAM_TO_ACP_CH_NUM, false);
+			acp_dma_start(rtd->acp_mmio, rtd->ch1, false);
 			while (acp_reg_read(rtd->acp_mmio, mmACP_DMA_CH_STS) &
-						BIT(SYSRAM_TO_ACP_CH_NUM)) {
+				BIT(rtd->ch1)) {
 				if (!loops--) {
 					dev_err(component->dev,
 						"acp dma start timeout\n");
@@ -950,40 +927,21 @@ static int acp_dma_trigger(struct snd_pcm_substream *substream, int cmd)
 				}
 				cpu_relax();
 			}
-
-			acp_dma_start(rtd->acp_mmio,
-				      ACP_TO_I2S_DMA_CH_NUM, true);
-
-		} else {
-			if (rtd->i2ssp_capturebytescount == 0)
-				rtd->i2ssp_capturebytescount = bytescount;
-			acp_dma_start(rtd->acp_mmio,
-				      I2S_TO_ACP_DMA_CH_NUM, true);
 		}
+		acp_dma_start(rtd->acp_mmio, rtd->ch2, true);
 		ret = 0;
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
-		/*
-		 * Need to stop only circular DMA channels :
-		 * ACP_TO_I2S_DMA_CH_NUM / I2S_TO_ACP_DMA_CH_NUM. Non-circular
-		 * 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) {
-			ret = acp_dma_stop(rtd->acp_mmio,
-					   SYSRAM_TO_ACP_CH_NUM);
-			ret = acp_dma_stop(rtd->acp_mmio,
-					   ACP_TO_I2S_DMA_CH_NUM);
-			rtd->i2ssp_renderbytescount = 0;
+			acp_dma_stop(rtd->acp_mmio, rtd->ch1);
+			ret =  acp_dma_stop(rtd->acp_mmio, rtd->ch2);
 		} else {
-			ret = acp_dma_stop(rtd->acp_mmio,
-					   I2S_TO_ACP_DMA_CH_NUM);
-			ret = acp_dma_stop(rtd->acp_mmio,
-					   ACP_TO_SYSRAM_CH_NUM);
-			rtd->i2ssp_capturebytescount = 0;
+			acp_dma_stop(rtd->acp_mmio, rtd->ch2);
+			ret = acp_dma_stop(rtd->acp_mmio, rtd->ch1);
 		}
+		rtd->bytescount = 0;
 		break;
 	default:
 		ret = -EINVAL;
@@ -1028,8 +986,6 @@ static int acp_dma_close(struct snd_pcm_substream *substream)
 								    DRV_NAME);
 	struct audio_drv_data *adata = dev_get_drvdata(component->dev);
 
-	kfree(rtd);
-
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		adata->play_i2ssp_stream = NULL;
 		/*
@@ -1059,6 +1015,7 @@ static int acp_dma_close(struct snd_pcm_substream *substream)
 	if (!adata->play_i2ssp_stream && !adata->capture_i2ssp_stream)
 		acp_reg_write(0, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB);
 
+	kfree(rtd);
 	return 0;
 }
 
diff --git a/sound/soc/amd/acp.h b/sound/soc/amd/acp.h
index 0e6089b..62695ed 100644
--- a/sound/soc/amd/acp.h
+++ b/sound/soc/amd/acp.h
@@ -10,17 +10,28 @@
 #define ACP_PLAYBACK_PTE_OFFSET			10
 #define ACP_CAPTURE_PTE_OFFSET			0
 
+/* Playback and Capture Offset for Stoney */
+#define ACP_ST_PLAYBACK_PTE_OFFSET	0x04
+#define ACP_ST_CAPTURE_PTE_OFFSET	0x00
+
 #define ACP_GARLIC_CNTL_DEFAULT			0x00000FB4
 #define ACP_ONION_CNTL_DEFAULT			0x00000FB4
 
 #define ACP_PHYSICAL_BASE			0x14000
 
-/* Playback SRAM address (as a destination in dma descriptor) */
-#define ACP_SHARED_RAM_BANK_1_ADDRESS		0x4002000
-
-/* Capture SRAM address (as a source in dma descriptor) */
-#define ACP_SHARED_RAM_BANK_5_ADDRESS		0x400A000
-#define ACP_SHARED_RAM_BANK_3_ADDRESS		0x4006000
+/*
+ * In case of I2S SP controller instance, Stoney uses SRAM bank 1 for
+ * playback and SRAM Bank 2 for capture where as in case of BT I2S
+ * Instance, Stoney uses SRAM Bank 3 for playback & SRAM Bank 4 will
+ * be used for capture. Carrizo uses I2S SP controller instance. SRAM Banks
+ * 1, 2, 3, 4 will be used for playback & SRAM Banks 5, 6, 7, 8 will be used
+ * for capture scenario.
+ */
+#define ACP_SRAM_BANK_1_ADDRESS		0x4002000
+#define ACP_SRAM_BANK_2_ADDRESS		0x4004000
+#define ACP_SRAM_BANK_3_ADDRESS		0x4006000
+#define ACP_SRAM_BANK_4_ADDRESS		0x4008000
+#define ACP_SRAM_BANK_5_ADDRESS		0x400A000
 
 #define ACP_DMA_RESET_TIME			10000
 #define ACP_CLOCK_EN_TIME_OUT_VALUE		0x000000FF
@@ -85,9 +96,17 @@ struct audio_substream_data {
 	unsigned int order;
 	u16 num_of_pages;
 	u16 direction;
+	u16 ch1;
+	u16 ch2;
+	u16 destination;
+	u16 dma_dscr_idx_1;
+	u16 dma_dscr_idx_2;
+	u32 pte_offset;
+	u32 sram_bank;
+	u32 byte_cnt_high_reg_offset;
+	u32 byte_cnt_low_reg_offset;
 	uint64_t size;
-	u64 i2ssp_renderbytescount;
-	u64 i2ssp_capturebytescount;
+	u64 bytescount;
 	void __iomem *acp_mmio;
 };
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] ASoC: AMD: Move clk enable from hw_params/free to startup/shutdown
  2018-04-24  3:04 ` Vijendar Mukunda
@ 2018-04-24  3:04   ` Vijendar Mukunda
  -1 siblings, 0 replies; 10+ messages in thread
From: Vijendar Mukunda @ 2018-04-24  3:04 UTC (permalink / raw)
  Cc: Akshu Agrawal, Vijendar Mukunda, Liam Girdwood, Mark Brown,
	Jaroslav Kysela, Takashi Iwai, Alex Deucher, Kuninori Morimoto,
	moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	open list

From: Akshu Agrawal <akshu.agrawal@amd.com>

hw_param can be called multiple times and thus we can have
more clk enable. The clk may not get diabled due to refcounting.
startup/shutdown ensures single clk enable/disable call.

Signed-off-by: Akshu Agrawal <akshu.agrawal@amd.com>
Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
---
 sound/soc/amd/acp-da7219-max98357a.c | 54 ++++++++++++++++++++++++------------
 1 file changed, 37 insertions(+), 17 deletions(-)

diff --git a/sound/soc/amd/acp-da7219-max98357a.c b/sound/soc/amd/acp-da7219-max98357a.c
index b205c78..0f16f6d 100644
--- a/sound/soc/amd/acp-da7219-max98357a.c
+++ b/sound/soc/amd/acp-da7219-max98357a.c
@@ -38,8 +38,7 @@
 #include "../codecs/da7219.h"
 #include "../codecs/da7219-aad.h"
 
-#define CZ_PLAT_CLK 24000000
-#define MCLK_RATE 24576000
+#define CZ_PLAT_CLK 25000000
 #define DUAL_CHANNEL		2
 
 static struct snd_soc_jack cz_jack;
@@ -62,7 +61,7 @@ static int cz_da7219_init(struct snd_soc_pcm_runtime *rtd)
 	}
 
 	ret = snd_soc_dai_set_pll(codec_dai, 0, DA7219_SYSCLK_PLL,
-				  CZ_PLAT_CLK, MCLK_RATE);
+				  CZ_PLAT_CLK, DA7219_PLL_FREQ_OUT_98304);
 	if (ret < 0) {
 		dev_err(rtd->dev, "can't set codec pll: %d\n", ret);
 		return ret;
@@ -85,8 +84,7 @@ static int cz_da7219_init(struct snd_soc_pcm_runtime *rtd)
 	return 0;
 }
 
-static int cz_da7219_hw_params(struct snd_pcm_substream *substream,
-			     struct snd_pcm_hw_params *params)
+static int da7219_clk_enable(struct snd_pcm_substream *substream)
 {
 	int ret = 0;
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
@@ -100,11 +98,9 @@ static int cz_da7219_hw_params(struct snd_pcm_substream *substream,
 	return ret;
 }
 
-static int cz_da7219_hw_free(struct snd_pcm_substream *substream)
+static void da7219_clk_disable(void)
 {
 	clk_disable_unprepare(da7219_dai_clk);
-
-	return 0;
 }
 
 static const unsigned int channels[] = {
@@ -127,7 +123,7 @@ static const struct snd_pcm_hw_constraint_list constraints_channels = {
 	.mask = 0,
 };
 
-static int cz_fe_startup(struct snd_pcm_substream *substream)
+static int cz_da7219_startup(struct snd_pcm_substream *substream)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 
@@ -141,23 +137,47 @@ static int cz_fe_startup(struct snd_pcm_substream *substream)
 	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
 				   &constraints_rates);
 
-	return 0;
+	return da7219_clk_enable(substream);
+}
+
+static void cz_da7219_shutdown(struct snd_pcm_substream *substream)
+{
+	da7219_clk_disable();
+}
+
+static int cz_max_startup(struct snd_pcm_substream *substream)
+{
+	return da7219_clk_enable(substream);
+}
+
+static void cz_max_shutdown(struct snd_pcm_substream *substream)
+{
+	da7219_clk_disable();
+}
+
+static int cz_dmic_startup(struct snd_pcm_substream *substream)
+{
+	return da7219_clk_enable(substream);
+}
+
+static void cz_dmic_shutdown(struct snd_pcm_substream *substream)
+{
+	da7219_clk_disable();
 }
 
 static struct snd_soc_ops cz_da7219_cap_ops = {
-	.hw_params = cz_da7219_hw_params,
-	.hw_free = cz_da7219_hw_free,
-	.startup = cz_fe_startup,
+	.startup = cz_da7219_startup,
+	.shutdown = cz_da7219_shutdown,
 };
 
 static struct snd_soc_ops cz_max_play_ops = {
-	.hw_params = cz_da7219_hw_params,
-	.hw_free = cz_da7219_hw_free,
+	.startup = cz_max_startup,
+	.shutdown = cz_max_shutdown,
 };
 
 static struct snd_soc_ops cz_dmic_cap_ops = {
-	.hw_params = cz_da7219_hw_params,
-	.hw_free = cz_da7219_hw_free,
+	.startup = cz_dmic_startup,
+	.shutdown = cz_dmic_shutdown,
 };
 
 static struct snd_soc_dai_link cz_dai_7219_98357[] = {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] ASoC: AMD: Move clk enable from hw_params/free to startup/shutdown
@ 2018-04-24  3:04   ` Vijendar Mukunda
  0 siblings, 0 replies; 10+ messages in thread
From: Vijendar Mukunda @ 2018-04-24  3:04 UTC (permalink / raw)
  Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	Kuninori Morimoto, Liam Girdwood, open list, Takashi Iwai,
	Mark Brown, Vijendar Mukunda, Alex Deucher, Akshu Agrawal

From: Akshu Agrawal <akshu.agrawal@amd.com>

hw_param can be called multiple times and thus we can have
more clk enable. The clk may not get diabled due to refcounting.
startup/shutdown ensures single clk enable/disable call.

Signed-off-by: Akshu Agrawal <akshu.agrawal@amd.com>
Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
---
 sound/soc/amd/acp-da7219-max98357a.c | 54 ++++++++++++++++++++++++------------
 1 file changed, 37 insertions(+), 17 deletions(-)

diff --git a/sound/soc/amd/acp-da7219-max98357a.c b/sound/soc/amd/acp-da7219-max98357a.c
index b205c78..0f16f6d 100644
--- a/sound/soc/amd/acp-da7219-max98357a.c
+++ b/sound/soc/amd/acp-da7219-max98357a.c
@@ -38,8 +38,7 @@
 #include "../codecs/da7219.h"
 #include "../codecs/da7219-aad.h"
 
-#define CZ_PLAT_CLK 24000000
-#define MCLK_RATE 24576000
+#define CZ_PLAT_CLK 25000000
 #define DUAL_CHANNEL		2
 
 static struct snd_soc_jack cz_jack;
@@ -62,7 +61,7 @@ static int cz_da7219_init(struct snd_soc_pcm_runtime *rtd)
 	}
 
 	ret = snd_soc_dai_set_pll(codec_dai, 0, DA7219_SYSCLK_PLL,
-				  CZ_PLAT_CLK, MCLK_RATE);
+				  CZ_PLAT_CLK, DA7219_PLL_FREQ_OUT_98304);
 	if (ret < 0) {
 		dev_err(rtd->dev, "can't set codec pll: %d\n", ret);
 		return ret;
@@ -85,8 +84,7 @@ static int cz_da7219_init(struct snd_soc_pcm_runtime *rtd)
 	return 0;
 }
 
-static int cz_da7219_hw_params(struct snd_pcm_substream *substream,
-			     struct snd_pcm_hw_params *params)
+static int da7219_clk_enable(struct snd_pcm_substream *substream)
 {
 	int ret = 0;
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
@@ -100,11 +98,9 @@ static int cz_da7219_hw_params(struct snd_pcm_substream *substream,
 	return ret;
 }
 
-static int cz_da7219_hw_free(struct snd_pcm_substream *substream)
+static void da7219_clk_disable(void)
 {
 	clk_disable_unprepare(da7219_dai_clk);
-
-	return 0;
 }
 
 static const unsigned int channels[] = {
@@ -127,7 +123,7 @@ static const struct snd_pcm_hw_constraint_list constraints_channels = {
 	.mask = 0,
 };
 
-static int cz_fe_startup(struct snd_pcm_substream *substream)
+static int cz_da7219_startup(struct snd_pcm_substream *substream)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 
@@ -141,23 +137,47 @@ static int cz_fe_startup(struct snd_pcm_substream *substream)
 	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
 				   &constraints_rates);
 
-	return 0;
+	return da7219_clk_enable(substream);
+}
+
+static void cz_da7219_shutdown(struct snd_pcm_substream *substream)
+{
+	da7219_clk_disable();
+}
+
+static int cz_max_startup(struct snd_pcm_substream *substream)
+{
+	return da7219_clk_enable(substream);
+}
+
+static void cz_max_shutdown(struct snd_pcm_substream *substream)
+{
+	da7219_clk_disable();
+}
+
+static int cz_dmic_startup(struct snd_pcm_substream *substream)
+{
+	return da7219_clk_enable(substream);
+}
+
+static void cz_dmic_shutdown(struct snd_pcm_substream *substream)
+{
+	da7219_clk_disable();
 }
 
 static struct snd_soc_ops cz_da7219_cap_ops = {
-	.hw_params = cz_da7219_hw_params,
-	.hw_free = cz_da7219_hw_free,
-	.startup = cz_fe_startup,
+	.startup = cz_da7219_startup,
+	.shutdown = cz_da7219_shutdown,
 };
 
 static struct snd_soc_ops cz_max_play_ops = {
-	.hw_params = cz_da7219_hw_params,
-	.hw_free = cz_da7219_hw_free,
+	.startup = cz_max_startup,
+	.shutdown = cz_max_shutdown,
 };
 
 static struct snd_soc_ops cz_dmic_cap_ops = {
-	.hw_params = cz_da7219_hw_params,
-	.hw_free = cz_da7219_hw_free,
+	.startup = cz_dmic_startup,
+	.shutdown = cz_dmic_shutdown,
 };
 
 static struct snd_soc_dai_link cz_dai_7219_98357[] = {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 3/3] ASoC: amd: dma driver changes for bt i2s instance
  2018-04-24  3:04 ` Vijendar Mukunda
@ 2018-04-24  3:04   ` Vijendar Mukunda
  -1 siblings, 0 replies; 10+ messages in thread
From: Vijendar Mukunda @ 2018-04-24  3:04 UTC (permalink / raw)
  Cc: Vijendar Mukunda, Akshu Agrawal, Liam Girdwood, Mark Brown,
	Jaroslav Kysela, Takashi Iwai, Alex Deucher, Kuninori Morimoto,
	Jason Clinton, Guenter Roeck, Greg Kroah-Hartman,
	moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	open list

With in ACP, There are three I2S controllers can be
configured/enabled ( I2S SP, I2S MICSP, I2S BT).
Default enabled I2S controller instance is I2S SP.
This patch provides required changes to support I2S BT
controller Instance.

Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
Signed-off-by: Akshu Agrawal <akshu.agrawal@amd.com>
---
v1->v2: defined i2s instance macros in acp header file
v2->v3: sqaushed previous patch series and spilt changes
	into two patches ( acp dma driver code cleanup patch
	and bt i2s specific changes patch)
 sound/soc/amd/acp-da7219-max98357a.c |  23 ++++
 sound/soc/amd/acp-pcm-dma.c          | 255 +++++++++++++++++++++++++++--------
 sound/soc/amd/acp.h                  |  40 ++++++
 3 files changed, 261 insertions(+), 57 deletions(-)

diff --git a/sound/soc/amd/acp-da7219-max98357a.c b/sound/soc/amd/acp-da7219-max98357a.c
index 0f16f6d..4a0653f 100644
--- a/sound/soc/amd/acp-da7219-max98357a.c
+++ b/sound/soc/amd/acp-da7219-max98357a.c
@@ -35,6 +35,7 @@
 #include <linux/i2c.h>
 #include <linux/acpi.h>
 
+#include "acp.h"
 #include "../codecs/da7219.h"
 #include "../codecs/da7219-aad.h"
 
@@ -43,6 +44,7 @@
 
 static struct snd_soc_jack cz_jack;
 struct clk *da7219_dai_clk;
+extern int bt_pad_enable;
 
 static int cz_da7219_init(struct snd_soc_pcm_runtime *rtd)
 {
@@ -126,6 +128,9 @@ static const struct snd_pcm_hw_constraint_list constraints_channels = {
 static int cz_da7219_startup(struct snd_pcm_substream *substream)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_card *card = rtd->card;
+	struct acp_platform_info *machine = snd_soc_card_get_drvdata(card);
 
 	/*
 	 * On this platform for PCM device we support stereo
@@ -137,6 +142,7 @@ static int cz_da7219_startup(struct snd_pcm_substream *substream)
 	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
 				   &constraints_rates);
 
+	machine->i2s_instance = I2S_BT_INSTANCE;
 	return da7219_clk_enable(substream);
 }
 
@@ -147,6 +153,11 @@ static void cz_da7219_shutdown(struct snd_pcm_substream *substream)
 
 static int cz_max_startup(struct snd_pcm_substream *substream)
 {
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_card *card = rtd->card;
+	struct acp_platform_info *machine = snd_soc_card_get_drvdata(card);
+
+	machine->i2s_instance = I2S_SP_INSTANCE;
 	return da7219_clk_enable(substream);
 }
 
@@ -157,6 +168,11 @@ static void cz_max_shutdown(struct snd_pcm_substream *substream)
 
 static int cz_dmic_startup(struct snd_pcm_substream *substream)
 {
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_card *card = rtd->card;
+	struct acp_platform_info *machine = snd_soc_card_get_drvdata(card);
+
+	machine->i2s_instance = I2S_SP_INSTANCE;
 	return da7219_clk_enable(substream);
 }
 
@@ -260,10 +276,16 @@ static int cz_probe(struct platform_device *pdev)
 {
 	int ret;
 	struct snd_soc_card *card;
+	struct acp_platform_info *machine;
 
+	machine = devm_kzalloc(&pdev->dev, sizeof(struct acp_platform_info),
+			       GFP_KERNEL);
+	if (!machine)
+		return -ENOMEM;
 	card = &cz_card;
 	cz_card.dev = &pdev->dev;
 	platform_set_drvdata(pdev, card);
+	snd_soc_card_set_drvdata(card, machine);
 	ret = devm_snd_soc_register_card(&pdev->dev, &cz_card);
 	if (ret) {
 		dev_err(&pdev->dev,
@@ -271,6 +293,7 @@ static int cz_probe(struct platform_device *pdev)
 				cz_card.name, ret);
 		return ret;
 	}
+	bt_pad_enable = device_property_read_bool(&pdev->dev, "bt-pad-enable");
 	return 0;
 }
 
diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index 4a4bbdf..864bd6a 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -37,12 +37,14 @@
 #define MAX_BUFFER (PLAYBACK_MAX_PERIOD_SIZE * PLAYBACK_MAX_NUM_PERIODS)
 #define MIN_BUFFER MAX_BUFFER
 
-#define ST_PLAYBACK_MAX_PERIOD_SIZE 8192
+#define ST_PLAYBACK_MAX_PERIOD_SIZE 4096
 #define ST_CAPTURE_MAX_PERIOD_SIZE  ST_PLAYBACK_MAX_PERIOD_SIZE
 #define ST_MAX_BUFFER (ST_PLAYBACK_MAX_PERIOD_SIZE * PLAYBACK_MAX_NUM_PERIODS)
 #define ST_MIN_BUFFER ST_MAX_BUFFER
 
 #define DRV_NAME "acp_audio_dma"
+bool bt_pad_enable = false;
+EXPORT_SYMBOL(bt_pad_enable);
 
 static const struct snd_pcm_hardware acp_pcm_hardware_playback = {
 	.info = SNDRV_PCM_INFO_INTERLEAVED |
@@ -357,6 +359,9 @@ static void acp_dma_start(void __iomem *acp_mmio,
 	case ACP_TO_I2S_DMA_CH_NUM:
 	case ACP_TO_SYSRAM_CH_NUM:
 	case I2S_TO_ACP_DMA_CH_NUM:
+	case ACP_TO_I2S_DMA_BT_INSTANCE_CH_NUM:
+	case ACP_TO_SYSRAM_BT_INSTANCE_CH_NUM:
+	case I2S_TO_ACP_DMA_BT_INSTANCE_CH_NUM:
 		dma_ctrl |= ACP_DMA_CNTL_0__DMAChIOCEn_MASK;
 		break;
 	default:
@@ -519,6 +524,13 @@ static int acp_init(void __iomem *acp_mmio, u32 asic_type)
 	val &= ~ACP_SOFT_RESET__SoftResetAud_MASK;
 	acp_reg_write(val, acp_mmio, mmACP_SOFT_RESET);
 
+	/* For BT instance change pins from UART to BT */
+	if (bt_pad_enable) {
+		val = acp_reg_read(acp_mmio, mmACP_BT_UART_PAD_SEL);
+		val |= ACP_BT_UART_PAD_SELECT_MASK;
+		acp_reg_write(val, acp_mmio, mmACP_BT_UART_PAD_SEL);
+	}
+
 	/* initiailize Onion control DAGB register */
 	acp_reg_write(ACP_ONION_CNTL_DEFAULT, acp_mmio,
 		      mmACP_AXI2DAGB_ONION_CNTL);
@@ -637,6 +649,24 @@ static irqreturn_t dma_irq_handler(int irq, void *arg)
 			      acp_mmio, mmACP_EXTERNAL_INTR_STAT);
 	}
 
+	if ((intr_flag & BIT(ACP_TO_I2S_DMA_BT_INSTANCE_CH_NUM)) != 0) {
+		valid_irq = true;
+		if (acp_reg_read(acp_mmio, mmACP_DMA_CUR_DSCR_9) ==
+			PLAYBACK_START_DMA_DESCR_CH9)
+			dscr_idx = PLAYBACK_END_DMA_DESCR_CH8;
+		else
+			dscr_idx = PLAYBACK_START_DMA_DESCR_CH8;
+		config_acp_dma_channel(acp_mmio,
+				       SYSRAM_TO_ACP_BT_INSTANCE_CH_NUM,
+				       dscr_idx, 1, 0);
+		acp_dma_start(acp_mmio, SYSRAM_TO_ACP_BT_INSTANCE_CH_NUM,
+			      false);
+		snd_pcm_period_elapsed(irq_data->play_i2sbt_stream);
+		acp_reg_write((intr_flag &
+			      BIT(ACP_TO_I2S_DMA_BT_INSTANCE_CH_NUM)) << 16,
+			      acp_mmio, mmACP_EXTERNAL_INTR_STAT);
+	}
+
 	if ((intr_flag & BIT(I2S_TO_ACP_DMA_CH_NUM)) != 0) {
 		valid_irq = true;
 		if (acp_reg_read(acp_mmio, mmACP_DMA_CUR_DSCR_15) ==
@@ -659,6 +689,31 @@ static irqreturn_t dma_irq_handler(int irq, void *arg)
 			      acp_mmio, mmACP_EXTERNAL_INTR_STAT);
 	}
 
+	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_11) ==
+			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);
+		acp_reg_write((intr_flag &
+			      BIT(I2S_TO_ACP_DMA_BT_INSTANCE_CH_NUM)) << 16,
+			      acp_mmio, mmACP_EXTERNAL_INTR_STAT);
+	}
+
+	if ((intr_flag & BIT(ACP_TO_SYSRAM_BT_INSTANCE_CH_NUM)) != 0) {
+		valid_irq = true;
+		snd_pcm_period_elapsed(irq_data->capture_i2sbt_stream);
+		acp_reg_write((intr_flag &
+			      BIT(ACP_TO_SYSRAM_BT_INSTANCE_CH_NUM)) << 16,
+			      acp_mmio, mmACP_EXTERNAL_INTR_STAT);
+	}
+
 	if (valid_irq)
 		return IRQ_HANDLED;
 	else
@@ -715,7 +770,8 @@ static int acp_dma_open(struct snd_pcm_substream *substream)
 	 * This enablement is not required for another stream, if current
 	 * stream is not closed
 	 */
-	if (!intr_data->play_i2ssp_stream && !intr_data->capture_i2ssp_stream)
+	if (!intr_data->play_i2ssp_stream && !intr_data->capture_i2ssp_stream &&
+	    !intr_data->play_i2sbt_stream && !intr_data->capture_i2sbt_stream)
 		acp_reg_write(1, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB);
 	return 0;
 }
@@ -734,6 +790,8 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
 	struct snd_soc_component *component = snd_soc_rtdcom_lookup(prtd,
 								    DRV_NAME);
 	struct audio_drv_data *adata = dev_get_drvdata(component->dev);
+	struct snd_soc_card *card = prtd->card;
+	struct acp_platform_info *pinfo = snd_soc_card_get_drvdata(card);
 
 	runtime = substream->runtime;
 	rtd = runtime->private_data;
@@ -741,35 +799,70 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
 	if (WARN_ON(!rtd))
 		return -EINVAL;
 
+	rtd->i2s_instance = pinfo->i2s_instance;
 	if (adata->asic_type == CHIP_STONEY) {
 		val = acp_reg_read(adata->acp_mmio,
 				   mmACP_I2S_16BIT_RESOLUTION_EN);
-		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-			val |= ACP_I2S_SP_16BIT_RESOLUTION_EN;
-		else
-			val |= ACP_I2S_MIC_16BIT_RESOLUTION_EN;
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+			switch (rtd->i2s_instance) {
+			case I2S_BT_INSTANCE:
+				val |= ACP_I2S_BT_16BIT_RESOLUTION_EN;
+				break;
+			case I2S_SP_INSTANCE:
+			default:
+				val |= ACP_I2S_SP_16BIT_RESOLUTION_EN;
+			}
+		} else {
+			switch (rtd->i2s_instance) {
+			case I2S_BT_INSTANCE:
+				val |= ACP_I2S_BT_16BIT_RESOLUTION_EN;
+				break;
+			case I2S_SP_INSTANCE:
+			default:
+				val |= ACP_I2S_MIC_16BIT_RESOLUTION_EN;
+			}
+		}
 		acp_reg_write(val, adata->acp_mmio,
 			      mmACP_I2S_16BIT_RESOLUTION_EN);
 	}
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		switch (adata->asic_type) {
-		case CHIP_STONEY:
-			rtd->pte_offset = ACP_ST_PLAYBACK_PTE_OFFSET;
+		switch (rtd->i2s_instance) {
+		case I2S_BT_INSTANCE:
+			rtd->pte_offset = ACP_ST_BT_PLAYBACK_PTE_OFFSET;
+			rtd->ch1 = SYSRAM_TO_ACP_BT_INSTANCE_CH_NUM;
+			rtd->ch2 = ACP_TO_I2S_DMA_BT_INSTANCE_CH_NUM;
+			rtd->sram_bank = ACP_SRAM_BANK_3_ADDRESS;
+			rtd->destination = TO_BLUETOOTH;
+			rtd->dma_dscr_idx_1 = PLAYBACK_START_DMA_DESCR_CH8;
+			rtd->dma_dscr_idx_2 = PLAYBACK_START_DMA_DESCR_CH9;
+			rtd->byte_cnt_high_reg_offset =
+			mmACP_I2S_BT_TRANSMIT_BYTE_CNT_HIGH;
+			rtd->byte_cnt_low_reg_offset =
+			mmACP_I2S_BT_TRANSMIT_BYTE_CNT_LOW;
+			adata->play_i2sbt_stream = substream;
 			break;
+		case I2S_SP_INSTANCE:
 		default:
-			rtd->pte_offset = ACP_PLAYBACK_PTE_OFFSET;
+			switch (adata->asic_type) {
+			case CHIP_STONEY:
+				rtd->pte_offset = ACP_ST_PLAYBACK_PTE_OFFSET;
+				break;
+			default:
+				rtd->pte_offset = ACP_PLAYBACK_PTE_OFFSET;
+			}
+			rtd->ch1 = SYSRAM_TO_ACP_CH_NUM;
+			rtd->ch2 = ACP_TO_I2S_DMA_CH_NUM;
+			rtd->sram_bank = ACP_SRAM_BANK_1_ADDRESS;
+			rtd->destination = TO_ACP_I2S_1;
+			rtd->dma_dscr_idx_1 = PLAYBACK_START_DMA_DESCR_CH12;
+			rtd->dma_dscr_idx_2 = PLAYBACK_START_DMA_DESCR_CH13;
+			rtd->byte_cnt_high_reg_offset =
+			mmACP_I2S_TRANSMIT_BYTE_CNT_HIGH;
+			rtd->byte_cnt_low_reg_offset =
+			mmACP_I2S_TRANSMIT_BYTE_CNT_LOW;
+			adata->play_i2ssp_stream = substream;
 		}
-		rtd->ch1 = SYSRAM_TO_ACP_CH_NUM;
-		rtd->ch2 = ACP_TO_I2S_DMA_CH_NUM;
-		rtd->sram_bank = ACP_SRAM_BANK_1_ADDRESS;
-		rtd->destination = TO_ACP_I2S_1;
-		rtd->dma_dscr_idx_1 = PLAYBACK_START_DMA_DESCR_CH12;
-		rtd->dma_dscr_idx_2 = PLAYBACK_START_DMA_DESCR_CH13;
-		rtd->byte_cnt_high_reg_offset =
-		mmACP_I2S_TRANSMIT_BYTE_CNT_HIGH;
-		rtd->byte_cnt_low_reg_offset = mmACP_I2S_TRANSMIT_BYTE_CNT_LOW;
-		adata->play_i2ssp_stream = substream;
 		/*
 		 * For Stoney, Memory gating is disabled,i.e SRAM Banks
 		 * won't be turned off. The default state for SRAM banks is ON.
@@ -781,23 +874,42 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
 							bank, true);
 		}
 	} else {
-		rtd->pte_offset = ACP_CAPTURE_PTE_OFFSET;
-		rtd->ch1 = SYSRAM_TO_ACP_CH_NUM;
-		rtd->ch2 = ACP_TO_I2S_DMA_CH_NUM;
-		switch (adata->asic_type) {
-		case CHIP_STONEY:
-			rtd->sram_bank = ACP_SRAM_BANK_2_ADDRESS;
+		switch (rtd->i2s_instance) {
+		case I2S_BT_INSTANCE:
+			rtd->pte_offset = ACP_ST_BT_CAPTURE_PTE_OFFSET;
+			rtd->ch1 = ACP_TO_SYSRAM_BT_INSTANCE_CH_NUM;
+			rtd->ch2 = I2S_TO_ACP_DMA_BT_INSTANCE_CH_NUM;
+			rtd->sram_bank = ACP_SRAM_BANK_4_ADDRESS;
+			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;
+			adata->capture_i2sbt_stream = substream;
 			break;
+		case I2S_SP_INSTANCE:
 		default:
-			rtd->sram_bank = ACP_SRAM_BANK_5_ADDRESS;
+			rtd->pte_offset = ACP_CAPTURE_PTE_OFFSET;
+			rtd->ch1 = SYSRAM_TO_ACP_CH_NUM;
+			rtd->ch2 = ACP_TO_I2S_DMA_CH_NUM;
+			switch (adata->asic_type) {
+			case CHIP_STONEY:
+				rtd->sram_bank = ACP_SRAM_BANK_2_ADDRESS;
+				break;
+			default:
+				rtd->sram_bank = ACP_SRAM_BANK_5_ADDRESS;
+			}
+			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;
+			adata->capture_i2ssp_stream = 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;
-		adata->capture_i2ssp_stream = substream;
 		if (adata->asic_type != CHIP_STONEY) {
 			for (bank = 5; bank <= 8; bank++)
 				acp_set_sram_bank_state(adata->acp_mmio,
@@ -987,24 +1099,39 @@ static int acp_dma_close(struct snd_pcm_substream *substream)
 	struct audio_drv_data *adata = dev_get_drvdata(component->dev);
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		adata->play_i2ssp_stream = NULL;
-		/*
-		 * For Stoney, Memory gating is disabled,i.e SRAM Banks
-		 * won't be turned off. The default state for SRAM banks is ON.
-		 * Setting SRAM bank state code skipped for STONEY platform.
-		 * added condition checks for Carrizo platform only
-		 */
-		if (adata->asic_type != CHIP_STONEY) {
-			for (bank = 1; bank <= 4; bank++)
-				acp_set_sram_bank_state(adata->acp_mmio, bank,
-							false);
+		switch (rtd->i2s_instance) {
+		case I2S_BT_INSTANCE:
+			adata->play_i2sbt_stream = NULL;
+			break;
+		case I2S_SP_INSTANCE:
+		default:
+			adata->play_i2ssp_stream = NULL;
+			/*
+			 * For Stoney, Memory gating is disabled,i.e SRAM Banks
+			 * won't be turned off. The default state for SRAM banks
+			 * is ON.Setting SRAM bank state code skipped for STONEY
+			 * platform. Added condition checks for Carrizo platform
+			 * only.
+			 */
+			if (adata->asic_type != CHIP_STONEY) {
+				for (bank = 1; bank <= 4; bank++)
+					acp_set_sram_bank_state(adata->acp_mmio,
+								bank, false);
+			}
 		}
 	} else  {
-		adata->capture_i2ssp_stream = NULL;
-		if (adata->asic_type != CHIP_STONEY) {
-			for (bank = 5; bank <= 8; bank++)
-				acp_set_sram_bank_state(adata->acp_mmio, bank,
-							false);
+		switch (rtd->i2s_instance) {
+		case I2S_BT_INSTANCE:
+			adata->capture_i2sbt_stream = NULL;
+			break;
+		case I2S_SP_INSTANCE:
+		default:
+			adata->capture_i2ssp_stream = NULL;
+			if (adata->asic_type != CHIP_STONEY) {
+				for (bank = 5; bank <= 8; bank++)
+					acp_set_sram_bank_state(adata->acp_mmio,
+								bank, false);
+			}
 		}
 	}
 
@@ -1012,7 +1139,8 @@ static int acp_dma_close(struct snd_pcm_substream *substream)
 	 * Disable ACP irq, when the current stream is being closed and
 	 * another stream is also not active.
 	 */
-	if (!adata->play_i2ssp_stream && !adata->capture_i2ssp_stream)
+	if (!adata->play_i2ssp_stream && !adata->capture_i2ssp_stream &&
+	    !adata->play_i2sbt_stream && !adata->capture_i2sbt_stream)
 		acp_reg_write(0, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB);
 
 	kfree(rtd);
@@ -1067,6 +1195,8 @@ static int acp_audio_probe(struct platform_device *pdev)
 
 	audio_drv_data->play_i2ssp_stream = NULL;
 	audio_drv_data->capture_i2ssp_stream = NULL;
+	audio_drv_data->play_i2sbt_stream = NULL;
+	audio_drv_data->capture_i2sbt_stream = NULL;
 
 	audio_drv_data->asic_type =  *pdata;
 
@@ -1123,6 +1253,7 @@ static int acp_pcm_resume(struct device *dev)
 {
 	u16 bank;
 	int status;
+	struct audio_substream_data *rtd;
 	struct audio_drv_data *adata = dev_get_drvdata(dev);
 
 	status = acp_init(adata->acp_mmio, adata->asic_type);
@@ -1142,9 +1273,8 @@ static int acp_pcm_resume(struct device *dev)
 				acp_set_sram_bank_state(adata->acp_mmio, bank,
 							true);
 		}
-		config_acp_dma(adata->acp_mmio,
-			       adata->play_i2ssp_stream->runtime->private_data,
-			       adata->asic_type);
+		rtd = adata->play_i2ssp_stream->runtime->private_data;
+		config_acp_dma(adata->acp_mmio, rtd, adata->asic_type);
 	}
 	if (adata->capture_i2ssp_stream &&
 	    adata->capture_i2ssp_stream->runtime) {
@@ -1153,9 +1283,20 @@ static int acp_pcm_resume(struct device *dev)
 				acp_set_sram_bank_state(adata->acp_mmio, bank,
 							true);
 		}
-		config_acp_dma(adata->acp_mmio,
-			       adata->capture_i2ssp_stream->runtime->private_data,
-			       adata->asic_type);
+		rtd =  adata->capture_i2ssp_stream->runtime->private_data;
+		config_acp_dma(adata->acp_mmio, rtd, adata->asic_type);
+	}
+	if (adata->asic_type != CHIP_CARRIZO) {
+		if (adata->play_i2sbt_stream &&
+		    adata->play_i2sbt_stream->runtime) {
+			rtd = adata->play_i2sbt_stream->runtime->private_data;
+			config_acp_dma(adata->acp_mmio, rtd, adata->asic_type);
+		}
+		if (adata->capture_i2sbt_stream &&
+		    adata->capture_i2sbt_stream->runtime) {
+			rtd = adata->capture_i2sbt_stream->runtime->private_data;
+			config_acp_dma(adata->acp_mmio, rtd, adata->asic_type);
+		}
 	}
 	acp_reg_write(1, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB);
 	return 0;
diff --git a/sound/soc/amd/acp.h b/sound/soc/amd/acp.h
index 62695ed..9cd3e96 100644
--- a/sound/soc/amd/acp.h
+++ b/sound/soc/amd/acp.h
@@ -13,6 +13,8 @@
 /* Playback and Capture Offset for Stoney */
 #define ACP_ST_PLAYBACK_PTE_OFFSET	0x04
 #define ACP_ST_CAPTURE_PTE_OFFSET	0x00
+#define ACP_ST_BT_PLAYBACK_PTE_OFFSET	0x08
+#define ACP_ST_BT_CAPTURE_PTE_OFFSET	0x0c
 
 #define ACP_GARLIC_CNTL_DEFAULT			0x00000FB4
 #define ACP_ONION_CNTL_DEFAULT			0x00000FB4
@@ -46,8 +48,13 @@
 
 #define TO_ACP_I2S_1   0x2
 #define TO_ACP_I2S_2   0x4
+#define TO_BLUETOOTH   0x3
 #define FROM_ACP_I2S_1 0xa
 #define FROM_ACP_I2S_2 0xb
+#define FROM_BLUETOOTH 0xb
+
+#define I2S_SP_INSTANCE                 0x01
+#define I2S_BT_INSTANCE                 0x02
 
 #define ACP_TILE_ON_MASK                0x03
 #define ACP_TILE_OFF_MASK               0x02
@@ -68,6 +75,14 @@
 #define ACP_TO_SYSRAM_CH_NUM 14
 #define I2S_TO_ACP_DMA_CH_NUM 15
 
+/* Playback DMA Channels for I2S BT instance */
+#define SYSRAM_TO_ACP_BT_INSTANCE_CH_NUM  8
+#define ACP_TO_I2S_DMA_BT_INSTANCE_CH_NUM 9
+
+/* Capture DMA Channels for I2S BT Instance */
+#define ACP_TO_SYSRAM_BT_INSTANCE_CH_NUM 10
+#define I2S_TO_ACP_DMA_BT_INSTANCE_CH_NUM 11
+
 #define NUM_DSCRS_PER_CHANNEL 2
 
 #define PLAYBACK_START_DMA_DESCR_CH12 0
@@ -80,9 +95,23 @@
 #define CAPTURE_START_DMA_DESCR_CH15 6
 #define CAPTURE_END_DMA_DESCR_CH15 7
 
+/* I2S BT Instance DMA Descriptors */
+#define PLAYBACK_START_DMA_DESCR_CH8 8
+#define PLAYBACK_END_DMA_DESCR_CH8 9
+#define PLAYBACK_START_DMA_DESCR_CH9 10
+#define PLAYBACK_END_DMA_DESCR_CH9 11
+
+#define CAPTURE_START_DMA_DESCR_CH10 12
+#define CAPTURE_END_DMA_DESCR_CH10 13
+#define CAPTURE_START_DMA_DESCR_CH11 14
+#define CAPTURE_END_DMA_DESCR_CH11 15
+
 #define mmACP_I2S_16BIT_RESOLUTION_EN       0x5209
 #define ACP_I2S_MIC_16BIT_RESOLUTION_EN 0x01
 #define ACP_I2S_SP_16BIT_RESOLUTION_EN	0x02
+#define ACP_I2S_BT_16BIT_RESOLUTION_EN	0x04
+#define ACP_BT_UART_PAD_SELECT_MASK	0x1
+
 enum acp_dma_priority_level {
 	/* 0x0 Specifies the DMA channel is given normal priority */
 	ACP_DMA_PRIORITY_LEVEL_NORMAL = 0x0,
@@ -95,6 +124,7 @@ struct audio_substream_data {
 	struct page *pg;
 	unsigned int order;
 	u16 num_of_pages;
+	u16 i2s_instance;
 	u16 direction;
 	u16 ch1;
 	u16 ch2;
@@ -113,10 +143,20 @@ struct audio_substream_data {
 struct audio_drv_data {
 	struct snd_pcm_substream *play_i2ssp_stream;
 	struct snd_pcm_substream *capture_i2ssp_stream;
+	struct snd_pcm_substream *play_i2sbt_stream;
+	struct snd_pcm_substream *capture_i2sbt_stream;
 	void __iomem *acp_mmio;
 	u32 asic_type;
 };
 
+/*
+ * this structure used for platform data transfer between machine driver
+ * and dma driver
+ */
+struct acp_platform_info {
+	u16 i2s_instance;
+};
+
 union acp_dma_count {
 	struct {
 	u32 low;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 3/3] ASoC: amd: dma driver changes for bt i2s instance
@ 2018-04-24  3:04   ` Vijendar Mukunda
  0 siblings, 0 replies; 10+ messages in thread
From: Vijendar Mukunda @ 2018-04-24  3:04 UTC (permalink / raw)
  Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	open list, Kuninori Morimoto, Liam Girdwood, Greg Kroah-Hartman,
	Takashi Iwai, Jason Clinton, Mark Brown, Vijendar Mukunda,
	Alex Deucher, Akshu Agrawal, Guenter Roeck

With in ACP, There are three I2S controllers can be
configured/enabled ( I2S SP, I2S MICSP, I2S BT).
Default enabled I2S controller instance is I2S SP.
This patch provides required changes to support I2S BT
controller Instance.

Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
Signed-off-by: Akshu Agrawal <akshu.agrawal@amd.com>
---
v1->v2: defined i2s instance macros in acp header file
v2->v3: sqaushed previous patch series and spilt changes
	into two patches ( acp dma driver code cleanup patch
	and bt i2s specific changes patch)
 sound/soc/amd/acp-da7219-max98357a.c |  23 ++++
 sound/soc/amd/acp-pcm-dma.c          | 255 +++++++++++++++++++++++++++--------
 sound/soc/amd/acp.h                  |  40 ++++++
 3 files changed, 261 insertions(+), 57 deletions(-)

diff --git a/sound/soc/amd/acp-da7219-max98357a.c b/sound/soc/amd/acp-da7219-max98357a.c
index 0f16f6d..4a0653f 100644
--- a/sound/soc/amd/acp-da7219-max98357a.c
+++ b/sound/soc/amd/acp-da7219-max98357a.c
@@ -35,6 +35,7 @@
 #include <linux/i2c.h>
 #include <linux/acpi.h>
 
+#include "acp.h"
 #include "../codecs/da7219.h"
 #include "../codecs/da7219-aad.h"
 
@@ -43,6 +44,7 @@
 
 static struct snd_soc_jack cz_jack;
 struct clk *da7219_dai_clk;
+extern int bt_pad_enable;
 
 static int cz_da7219_init(struct snd_soc_pcm_runtime *rtd)
 {
@@ -126,6 +128,9 @@ static const struct snd_pcm_hw_constraint_list constraints_channels = {
 static int cz_da7219_startup(struct snd_pcm_substream *substream)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_card *card = rtd->card;
+	struct acp_platform_info *machine = snd_soc_card_get_drvdata(card);
 
 	/*
 	 * On this platform for PCM device we support stereo
@@ -137,6 +142,7 @@ static int cz_da7219_startup(struct snd_pcm_substream *substream)
 	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
 				   &constraints_rates);
 
+	machine->i2s_instance = I2S_BT_INSTANCE;
 	return da7219_clk_enable(substream);
 }
 
@@ -147,6 +153,11 @@ static void cz_da7219_shutdown(struct snd_pcm_substream *substream)
 
 static int cz_max_startup(struct snd_pcm_substream *substream)
 {
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_card *card = rtd->card;
+	struct acp_platform_info *machine = snd_soc_card_get_drvdata(card);
+
+	machine->i2s_instance = I2S_SP_INSTANCE;
 	return da7219_clk_enable(substream);
 }
 
@@ -157,6 +168,11 @@ static void cz_max_shutdown(struct snd_pcm_substream *substream)
 
 static int cz_dmic_startup(struct snd_pcm_substream *substream)
 {
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_card *card = rtd->card;
+	struct acp_platform_info *machine = snd_soc_card_get_drvdata(card);
+
+	machine->i2s_instance = I2S_SP_INSTANCE;
 	return da7219_clk_enable(substream);
 }
 
@@ -260,10 +276,16 @@ static int cz_probe(struct platform_device *pdev)
 {
 	int ret;
 	struct snd_soc_card *card;
+	struct acp_platform_info *machine;
 
+	machine = devm_kzalloc(&pdev->dev, sizeof(struct acp_platform_info),
+			       GFP_KERNEL);
+	if (!machine)
+		return -ENOMEM;
 	card = &cz_card;
 	cz_card.dev = &pdev->dev;
 	platform_set_drvdata(pdev, card);
+	snd_soc_card_set_drvdata(card, machine);
 	ret = devm_snd_soc_register_card(&pdev->dev, &cz_card);
 	if (ret) {
 		dev_err(&pdev->dev,
@@ -271,6 +293,7 @@ static int cz_probe(struct platform_device *pdev)
 				cz_card.name, ret);
 		return ret;
 	}
+	bt_pad_enable = device_property_read_bool(&pdev->dev, "bt-pad-enable");
 	return 0;
 }
 
diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index 4a4bbdf..864bd6a 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -37,12 +37,14 @@
 #define MAX_BUFFER (PLAYBACK_MAX_PERIOD_SIZE * PLAYBACK_MAX_NUM_PERIODS)
 #define MIN_BUFFER MAX_BUFFER
 
-#define ST_PLAYBACK_MAX_PERIOD_SIZE 8192
+#define ST_PLAYBACK_MAX_PERIOD_SIZE 4096
 #define ST_CAPTURE_MAX_PERIOD_SIZE  ST_PLAYBACK_MAX_PERIOD_SIZE
 #define ST_MAX_BUFFER (ST_PLAYBACK_MAX_PERIOD_SIZE * PLAYBACK_MAX_NUM_PERIODS)
 #define ST_MIN_BUFFER ST_MAX_BUFFER
 
 #define DRV_NAME "acp_audio_dma"
+bool bt_pad_enable = false;
+EXPORT_SYMBOL(bt_pad_enable);
 
 static const struct snd_pcm_hardware acp_pcm_hardware_playback = {
 	.info = SNDRV_PCM_INFO_INTERLEAVED |
@@ -357,6 +359,9 @@ static void acp_dma_start(void __iomem *acp_mmio,
 	case ACP_TO_I2S_DMA_CH_NUM:
 	case ACP_TO_SYSRAM_CH_NUM:
 	case I2S_TO_ACP_DMA_CH_NUM:
+	case ACP_TO_I2S_DMA_BT_INSTANCE_CH_NUM:
+	case ACP_TO_SYSRAM_BT_INSTANCE_CH_NUM:
+	case I2S_TO_ACP_DMA_BT_INSTANCE_CH_NUM:
 		dma_ctrl |= ACP_DMA_CNTL_0__DMAChIOCEn_MASK;
 		break;
 	default:
@@ -519,6 +524,13 @@ static int acp_init(void __iomem *acp_mmio, u32 asic_type)
 	val &= ~ACP_SOFT_RESET__SoftResetAud_MASK;
 	acp_reg_write(val, acp_mmio, mmACP_SOFT_RESET);
 
+	/* For BT instance change pins from UART to BT */
+	if (bt_pad_enable) {
+		val = acp_reg_read(acp_mmio, mmACP_BT_UART_PAD_SEL);
+		val |= ACP_BT_UART_PAD_SELECT_MASK;
+		acp_reg_write(val, acp_mmio, mmACP_BT_UART_PAD_SEL);
+	}
+
 	/* initiailize Onion control DAGB register */
 	acp_reg_write(ACP_ONION_CNTL_DEFAULT, acp_mmio,
 		      mmACP_AXI2DAGB_ONION_CNTL);
@@ -637,6 +649,24 @@ static irqreturn_t dma_irq_handler(int irq, void *arg)
 			      acp_mmio, mmACP_EXTERNAL_INTR_STAT);
 	}
 
+	if ((intr_flag & BIT(ACP_TO_I2S_DMA_BT_INSTANCE_CH_NUM)) != 0) {
+		valid_irq = true;
+		if (acp_reg_read(acp_mmio, mmACP_DMA_CUR_DSCR_9) ==
+			PLAYBACK_START_DMA_DESCR_CH9)
+			dscr_idx = PLAYBACK_END_DMA_DESCR_CH8;
+		else
+			dscr_idx = PLAYBACK_START_DMA_DESCR_CH8;
+		config_acp_dma_channel(acp_mmio,
+				       SYSRAM_TO_ACP_BT_INSTANCE_CH_NUM,
+				       dscr_idx, 1, 0);
+		acp_dma_start(acp_mmio, SYSRAM_TO_ACP_BT_INSTANCE_CH_NUM,
+			      false);
+		snd_pcm_period_elapsed(irq_data->play_i2sbt_stream);
+		acp_reg_write((intr_flag &
+			      BIT(ACP_TO_I2S_DMA_BT_INSTANCE_CH_NUM)) << 16,
+			      acp_mmio, mmACP_EXTERNAL_INTR_STAT);
+	}
+
 	if ((intr_flag & BIT(I2S_TO_ACP_DMA_CH_NUM)) != 0) {
 		valid_irq = true;
 		if (acp_reg_read(acp_mmio, mmACP_DMA_CUR_DSCR_15) ==
@@ -659,6 +689,31 @@ static irqreturn_t dma_irq_handler(int irq, void *arg)
 			      acp_mmio, mmACP_EXTERNAL_INTR_STAT);
 	}
 
+	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_11) ==
+			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);
+		acp_reg_write((intr_flag &
+			      BIT(I2S_TO_ACP_DMA_BT_INSTANCE_CH_NUM)) << 16,
+			      acp_mmio, mmACP_EXTERNAL_INTR_STAT);
+	}
+
+	if ((intr_flag & BIT(ACP_TO_SYSRAM_BT_INSTANCE_CH_NUM)) != 0) {
+		valid_irq = true;
+		snd_pcm_period_elapsed(irq_data->capture_i2sbt_stream);
+		acp_reg_write((intr_flag &
+			      BIT(ACP_TO_SYSRAM_BT_INSTANCE_CH_NUM)) << 16,
+			      acp_mmio, mmACP_EXTERNAL_INTR_STAT);
+	}
+
 	if (valid_irq)
 		return IRQ_HANDLED;
 	else
@@ -715,7 +770,8 @@ static int acp_dma_open(struct snd_pcm_substream *substream)
 	 * This enablement is not required for another stream, if current
 	 * stream is not closed
 	 */
-	if (!intr_data->play_i2ssp_stream && !intr_data->capture_i2ssp_stream)
+	if (!intr_data->play_i2ssp_stream && !intr_data->capture_i2ssp_stream &&
+	    !intr_data->play_i2sbt_stream && !intr_data->capture_i2sbt_stream)
 		acp_reg_write(1, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB);
 	return 0;
 }
@@ -734,6 +790,8 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
 	struct snd_soc_component *component = snd_soc_rtdcom_lookup(prtd,
 								    DRV_NAME);
 	struct audio_drv_data *adata = dev_get_drvdata(component->dev);
+	struct snd_soc_card *card = prtd->card;
+	struct acp_platform_info *pinfo = snd_soc_card_get_drvdata(card);
 
 	runtime = substream->runtime;
 	rtd = runtime->private_data;
@@ -741,35 +799,70 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
 	if (WARN_ON(!rtd))
 		return -EINVAL;
 
+	rtd->i2s_instance = pinfo->i2s_instance;
 	if (adata->asic_type == CHIP_STONEY) {
 		val = acp_reg_read(adata->acp_mmio,
 				   mmACP_I2S_16BIT_RESOLUTION_EN);
-		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-			val |= ACP_I2S_SP_16BIT_RESOLUTION_EN;
-		else
-			val |= ACP_I2S_MIC_16BIT_RESOLUTION_EN;
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+			switch (rtd->i2s_instance) {
+			case I2S_BT_INSTANCE:
+				val |= ACP_I2S_BT_16BIT_RESOLUTION_EN;
+				break;
+			case I2S_SP_INSTANCE:
+			default:
+				val |= ACP_I2S_SP_16BIT_RESOLUTION_EN;
+			}
+		} else {
+			switch (rtd->i2s_instance) {
+			case I2S_BT_INSTANCE:
+				val |= ACP_I2S_BT_16BIT_RESOLUTION_EN;
+				break;
+			case I2S_SP_INSTANCE:
+			default:
+				val |= ACP_I2S_MIC_16BIT_RESOLUTION_EN;
+			}
+		}
 		acp_reg_write(val, adata->acp_mmio,
 			      mmACP_I2S_16BIT_RESOLUTION_EN);
 	}
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		switch (adata->asic_type) {
-		case CHIP_STONEY:
-			rtd->pte_offset = ACP_ST_PLAYBACK_PTE_OFFSET;
+		switch (rtd->i2s_instance) {
+		case I2S_BT_INSTANCE:
+			rtd->pte_offset = ACP_ST_BT_PLAYBACK_PTE_OFFSET;
+			rtd->ch1 = SYSRAM_TO_ACP_BT_INSTANCE_CH_NUM;
+			rtd->ch2 = ACP_TO_I2S_DMA_BT_INSTANCE_CH_NUM;
+			rtd->sram_bank = ACP_SRAM_BANK_3_ADDRESS;
+			rtd->destination = TO_BLUETOOTH;
+			rtd->dma_dscr_idx_1 = PLAYBACK_START_DMA_DESCR_CH8;
+			rtd->dma_dscr_idx_2 = PLAYBACK_START_DMA_DESCR_CH9;
+			rtd->byte_cnt_high_reg_offset =
+			mmACP_I2S_BT_TRANSMIT_BYTE_CNT_HIGH;
+			rtd->byte_cnt_low_reg_offset =
+			mmACP_I2S_BT_TRANSMIT_BYTE_CNT_LOW;
+			adata->play_i2sbt_stream = substream;
 			break;
+		case I2S_SP_INSTANCE:
 		default:
-			rtd->pte_offset = ACP_PLAYBACK_PTE_OFFSET;
+			switch (adata->asic_type) {
+			case CHIP_STONEY:
+				rtd->pte_offset = ACP_ST_PLAYBACK_PTE_OFFSET;
+				break;
+			default:
+				rtd->pte_offset = ACP_PLAYBACK_PTE_OFFSET;
+			}
+			rtd->ch1 = SYSRAM_TO_ACP_CH_NUM;
+			rtd->ch2 = ACP_TO_I2S_DMA_CH_NUM;
+			rtd->sram_bank = ACP_SRAM_BANK_1_ADDRESS;
+			rtd->destination = TO_ACP_I2S_1;
+			rtd->dma_dscr_idx_1 = PLAYBACK_START_DMA_DESCR_CH12;
+			rtd->dma_dscr_idx_2 = PLAYBACK_START_DMA_DESCR_CH13;
+			rtd->byte_cnt_high_reg_offset =
+			mmACP_I2S_TRANSMIT_BYTE_CNT_HIGH;
+			rtd->byte_cnt_low_reg_offset =
+			mmACP_I2S_TRANSMIT_BYTE_CNT_LOW;
+			adata->play_i2ssp_stream = substream;
 		}
-		rtd->ch1 = SYSRAM_TO_ACP_CH_NUM;
-		rtd->ch2 = ACP_TO_I2S_DMA_CH_NUM;
-		rtd->sram_bank = ACP_SRAM_BANK_1_ADDRESS;
-		rtd->destination = TO_ACP_I2S_1;
-		rtd->dma_dscr_idx_1 = PLAYBACK_START_DMA_DESCR_CH12;
-		rtd->dma_dscr_idx_2 = PLAYBACK_START_DMA_DESCR_CH13;
-		rtd->byte_cnt_high_reg_offset =
-		mmACP_I2S_TRANSMIT_BYTE_CNT_HIGH;
-		rtd->byte_cnt_low_reg_offset = mmACP_I2S_TRANSMIT_BYTE_CNT_LOW;
-		adata->play_i2ssp_stream = substream;
 		/*
 		 * For Stoney, Memory gating is disabled,i.e SRAM Banks
 		 * won't be turned off. The default state for SRAM banks is ON.
@@ -781,23 +874,42 @@ static int acp_dma_hw_params(struct snd_pcm_substream *substream,
 							bank, true);
 		}
 	} else {
-		rtd->pte_offset = ACP_CAPTURE_PTE_OFFSET;
-		rtd->ch1 = SYSRAM_TO_ACP_CH_NUM;
-		rtd->ch2 = ACP_TO_I2S_DMA_CH_NUM;
-		switch (adata->asic_type) {
-		case CHIP_STONEY:
-			rtd->sram_bank = ACP_SRAM_BANK_2_ADDRESS;
+		switch (rtd->i2s_instance) {
+		case I2S_BT_INSTANCE:
+			rtd->pte_offset = ACP_ST_BT_CAPTURE_PTE_OFFSET;
+			rtd->ch1 = ACP_TO_SYSRAM_BT_INSTANCE_CH_NUM;
+			rtd->ch2 = I2S_TO_ACP_DMA_BT_INSTANCE_CH_NUM;
+			rtd->sram_bank = ACP_SRAM_BANK_4_ADDRESS;
+			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;
+			adata->capture_i2sbt_stream = substream;
 			break;
+		case I2S_SP_INSTANCE:
 		default:
-			rtd->sram_bank = ACP_SRAM_BANK_5_ADDRESS;
+			rtd->pte_offset = ACP_CAPTURE_PTE_OFFSET;
+			rtd->ch1 = SYSRAM_TO_ACP_CH_NUM;
+			rtd->ch2 = ACP_TO_I2S_DMA_CH_NUM;
+			switch (adata->asic_type) {
+			case CHIP_STONEY:
+				rtd->sram_bank = ACP_SRAM_BANK_2_ADDRESS;
+				break;
+			default:
+				rtd->sram_bank = ACP_SRAM_BANK_5_ADDRESS;
+			}
+			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;
+			adata->capture_i2ssp_stream = 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;
-		adata->capture_i2ssp_stream = substream;
 		if (adata->asic_type != CHIP_STONEY) {
 			for (bank = 5; bank <= 8; bank++)
 				acp_set_sram_bank_state(adata->acp_mmio,
@@ -987,24 +1099,39 @@ static int acp_dma_close(struct snd_pcm_substream *substream)
 	struct audio_drv_data *adata = dev_get_drvdata(component->dev);
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		adata->play_i2ssp_stream = NULL;
-		/*
-		 * For Stoney, Memory gating is disabled,i.e SRAM Banks
-		 * won't be turned off. The default state for SRAM banks is ON.
-		 * Setting SRAM bank state code skipped for STONEY platform.
-		 * added condition checks for Carrizo platform only
-		 */
-		if (adata->asic_type != CHIP_STONEY) {
-			for (bank = 1; bank <= 4; bank++)
-				acp_set_sram_bank_state(adata->acp_mmio, bank,
-							false);
+		switch (rtd->i2s_instance) {
+		case I2S_BT_INSTANCE:
+			adata->play_i2sbt_stream = NULL;
+			break;
+		case I2S_SP_INSTANCE:
+		default:
+			adata->play_i2ssp_stream = NULL;
+			/*
+			 * For Stoney, Memory gating is disabled,i.e SRAM Banks
+			 * won't be turned off. The default state for SRAM banks
+			 * is ON.Setting SRAM bank state code skipped for STONEY
+			 * platform. Added condition checks for Carrizo platform
+			 * only.
+			 */
+			if (adata->asic_type != CHIP_STONEY) {
+				for (bank = 1; bank <= 4; bank++)
+					acp_set_sram_bank_state(adata->acp_mmio,
+								bank, false);
+			}
 		}
 	} else  {
-		adata->capture_i2ssp_stream = NULL;
-		if (adata->asic_type != CHIP_STONEY) {
-			for (bank = 5; bank <= 8; bank++)
-				acp_set_sram_bank_state(adata->acp_mmio, bank,
-							false);
+		switch (rtd->i2s_instance) {
+		case I2S_BT_INSTANCE:
+			adata->capture_i2sbt_stream = NULL;
+			break;
+		case I2S_SP_INSTANCE:
+		default:
+			adata->capture_i2ssp_stream = NULL;
+			if (adata->asic_type != CHIP_STONEY) {
+				for (bank = 5; bank <= 8; bank++)
+					acp_set_sram_bank_state(adata->acp_mmio,
+								bank, false);
+			}
 		}
 	}
 
@@ -1012,7 +1139,8 @@ static int acp_dma_close(struct snd_pcm_substream *substream)
 	 * Disable ACP irq, when the current stream is being closed and
 	 * another stream is also not active.
 	 */
-	if (!adata->play_i2ssp_stream && !adata->capture_i2ssp_stream)
+	if (!adata->play_i2ssp_stream && !adata->capture_i2ssp_stream &&
+	    !adata->play_i2sbt_stream && !adata->capture_i2sbt_stream)
 		acp_reg_write(0, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB);
 
 	kfree(rtd);
@@ -1067,6 +1195,8 @@ static int acp_audio_probe(struct platform_device *pdev)
 
 	audio_drv_data->play_i2ssp_stream = NULL;
 	audio_drv_data->capture_i2ssp_stream = NULL;
+	audio_drv_data->play_i2sbt_stream = NULL;
+	audio_drv_data->capture_i2sbt_stream = NULL;
 
 	audio_drv_data->asic_type =  *pdata;
 
@@ -1123,6 +1253,7 @@ static int acp_pcm_resume(struct device *dev)
 {
 	u16 bank;
 	int status;
+	struct audio_substream_data *rtd;
 	struct audio_drv_data *adata = dev_get_drvdata(dev);
 
 	status = acp_init(adata->acp_mmio, adata->asic_type);
@@ -1142,9 +1273,8 @@ static int acp_pcm_resume(struct device *dev)
 				acp_set_sram_bank_state(adata->acp_mmio, bank,
 							true);
 		}
-		config_acp_dma(adata->acp_mmio,
-			       adata->play_i2ssp_stream->runtime->private_data,
-			       adata->asic_type);
+		rtd = adata->play_i2ssp_stream->runtime->private_data;
+		config_acp_dma(adata->acp_mmio, rtd, adata->asic_type);
 	}
 	if (adata->capture_i2ssp_stream &&
 	    adata->capture_i2ssp_stream->runtime) {
@@ -1153,9 +1283,20 @@ static int acp_pcm_resume(struct device *dev)
 				acp_set_sram_bank_state(adata->acp_mmio, bank,
 							true);
 		}
-		config_acp_dma(adata->acp_mmio,
-			       adata->capture_i2ssp_stream->runtime->private_data,
-			       adata->asic_type);
+		rtd =  adata->capture_i2ssp_stream->runtime->private_data;
+		config_acp_dma(adata->acp_mmio, rtd, adata->asic_type);
+	}
+	if (adata->asic_type != CHIP_CARRIZO) {
+		if (adata->play_i2sbt_stream &&
+		    adata->play_i2sbt_stream->runtime) {
+			rtd = adata->play_i2sbt_stream->runtime->private_data;
+			config_acp_dma(adata->acp_mmio, rtd, adata->asic_type);
+		}
+		if (adata->capture_i2sbt_stream &&
+		    adata->capture_i2sbt_stream->runtime) {
+			rtd = adata->capture_i2sbt_stream->runtime->private_data;
+			config_acp_dma(adata->acp_mmio, rtd, adata->asic_type);
+		}
 	}
 	acp_reg_write(1, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB);
 	return 0;
diff --git a/sound/soc/amd/acp.h b/sound/soc/amd/acp.h
index 62695ed..9cd3e96 100644
--- a/sound/soc/amd/acp.h
+++ b/sound/soc/amd/acp.h
@@ -13,6 +13,8 @@
 /* Playback and Capture Offset for Stoney */
 #define ACP_ST_PLAYBACK_PTE_OFFSET	0x04
 #define ACP_ST_CAPTURE_PTE_OFFSET	0x00
+#define ACP_ST_BT_PLAYBACK_PTE_OFFSET	0x08
+#define ACP_ST_BT_CAPTURE_PTE_OFFSET	0x0c
 
 #define ACP_GARLIC_CNTL_DEFAULT			0x00000FB4
 #define ACP_ONION_CNTL_DEFAULT			0x00000FB4
@@ -46,8 +48,13 @@
 
 #define TO_ACP_I2S_1   0x2
 #define TO_ACP_I2S_2   0x4
+#define TO_BLUETOOTH   0x3
 #define FROM_ACP_I2S_1 0xa
 #define FROM_ACP_I2S_2 0xb
+#define FROM_BLUETOOTH 0xb
+
+#define I2S_SP_INSTANCE                 0x01
+#define I2S_BT_INSTANCE                 0x02
 
 #define ACP_TILE_ON_MASK                0x03
 #define ACP_TILE_OFF_MASK               0x02
@@ -68,6 +75,14 @@
 #define ACP_TO_SYSRAM_CH_NUM 14
 #define I2S_TO_ACP_DMA_CH_NUM 15
 
+/* Playback DMA Channels for I2S BT instance */
+#define SYSRAM_TO_ACP_BT_INSTANCE_CH_NUM  8
+#define ACP_TO_I2S_DMA_BT_INSTANCE_CH_NUM 9
+
+/* Capture DMA Channels for I2S BT Instance */
+#define ACP_TO_SYSRAM_BT_INSTANCE_CH_NUM 10
+#define I2S_TO_ACP_DMA_BT_INSTANCE_CH_NUM 11
+
 #define NUM_DSCRS_PER_CHANNEL 2
 
 #define PLAYBACK_START_DMA_DESCR_CH12 0
@@ -80,9 +95,23 @@
 #define CAPTURE_START_DMA_DESCR_CH15 6
 #define CAPTURE_END_DMA_DESCR_CH15 7
 
+/* I2S BT Instance DMA Descriptors */
+#define PLAYBACK_START_DMA_DESCR_CH8 8
+#define PLAYBACK_END_DMA_DESCR_CH8 9
+#define PLAYBACK_START_DMA_DESCR_CH9 10
+#define PLAYBACK_END_DMA_DESCR_CH9 11
+
+#define CAPTURE_START_DMA_DESCR_CH10 12
+#define CAPTURE_END_DMA_DESCR_CH10 13
+#define CAPTURE_START_DMA_DESCR_CH11 14
+#define CAPTURE_END_DMA_DESCR_CH11 15
+
 #define mmACP_I2S_16BIT_RESOLUTION_EN       0x5209
 #define ACP_I2S_MIC_16BIT_RESOLUTION_EN 0x01
 #define ACP_I2S_SP_16BIT_RESOLUTION_EN	0x02
+#define ACP_I2S_BT_16BIT_RESOLUTION_EN	0x04
+#define ACP_BT_UART_PAD_SELECT_MASK	0x1
+
 enum acp_dma_priority_level {
 	/* 0x0 Specifies the DMA channel is given normal priority */
 	ACP_DMA_PRIORITY_LEVEL_NORMAL = 0x0,
@@ -95,6 +124,7 @@ struct audio_substream_data {
 	struct page *pg;
 	unsigned int order;
 	u16 num_of_pages;
+	u16 i2s_instance;
 	u16 direction;
 	u16 ch1;
 	u16 ch2;
@@ -113,10 +143,20 @@ struct audio_substream_data {
 struct audio_drv_data {
 	struct snd_pcm_substream *play_i2ssp_stream;
 	struct snd_pcm_substream *capture_i2ssp_stream;
+	struct snd_pcm_substream *play_i2sbt_stream;
+	struct snd_pcm_substream *capture_i2sbt_stream;
 	void __iomem *acp_mmio;
 	u32 asic_type;
 };
 
+/*
+ * this structure used for platform data transfer between machine driver
+ * and dma driver
+ */
+struct acp_platform_info {
+	u16 i2s_instance;
+};
+
 union acp_dma_count {
 	struct {
 	u32 low;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] ASoC: amd: acp dma driver code cleanup
  2018-04-24  3:04 ` Vijendar Mukunda
                   ` (2 preceding siblings ...)
  (?)
@ 2018-04-24  6:05 ` Daniel Kurtz
  2018-04-24  7:06   ` Mukunda,Vijendar
  -1 siblings, 1 reply; 10+ messages in thread
From: Daniel Kurtz @ 2018-04-24  6:05 UTC (permalink / raw)
  To: Vijendar.Mukunda
  Cc: Liam Girdwood, Mark Brown, perex, tiwai, alexander.deucher,
	Akshu Agrawal, jclinton, Guenter Roeck, Kuninori Morimoto,
	Greg Kroah-Hartman, alsa-devel, linux-kernel

Hi Vijendar,


On Mon, Apr 23, 2018 at 9:02 PM Vijendar Mukunda <Vijendar.Mukunda@amd.com>
wrote:

> Added dma configuration parameters in audio_substream_data
> structure. Moved dma configuration parameters initialization
> to dma hw params callback.
> Removed separate byte count variables for playback and capture.
> Added variables to store ACP register offsets in audio_substream_data
> structure.

Thanks for splitting the patch, this is moving in the right direction, but
still very difficult to review since it is mixing different changes
together.
Just try to make each patch a single logical cleanup.
For example, perhaps create a set of patches that does:
   (1) Variable renames (eg audio_config -> rtd) & white space cleanup
   (2) Add dma configuration parameters to audio_substream_data structure
and initialize them in hw_params.
   (3) Remove separate byte count variables for playback and capture
   (4) Update the PTE offsets
   (5) Update the SRAM_BANKs

Note that (1) doesn't change functionality at all, (2) refactors but
doesn't change behavior or logic, (3) simplifies behavior but doesn't
change logic, and (4) & (5) build on the others but start making real
logical changes.

-Dan


> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
> ---
>   sound/soc/amd/acp-pcm-dma.c | 241
++++++++++++++++++--------------------------
>   sound/soc/amd/acp.h         |  35 +++++--
>   2 files changed, 126 insertions(+), 150 deletions(-)

> diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
> index 5ffe2ef..4a4bbdf 100644
> --- a/sound/soc/amd/acp-pcm-dma.c
> +++ b/sound/soc/amd/acp-pcm-dma.c
> @@ -317,54 +317,21 @@ static void acp_pte_config(void __iomem *acp_mmio,
struct page *pg,
>   }

>   static void config_acp_dma(void __iomem *acp_mmio,
> -                          struct audio_substream_data *audio_config,
> +                          struct audio_substream_data *rtd,
>                             u32 asic_type)
>   {
> -       u32 pte_offset, sram_bank;
> -       u16 ch1, ch2, destination, dma_dscr_idx;
> -
> -       if (audio_config->direction == SNDRV_PCM_STREAM_PLAYBACK) {
> -               pte_offset = ACP_PLAYBACK_PTE_OFFSET;
> -               ch1 = SYSRAM_TO_ACP_CH_NUM;
> -               ch2 = ACP_TO_I2S_DMA_CH_NUM;
> -               sram_bank = ACP_SHARED_RAM_BANK_1_ADDRESS;
> -               destination = TO_ACP_I2S_1;
> -
> -       } else {
> -               pte_offset = ACP_CAPTURE_PTE_OFFSET;
> -               ch1 = SYSRAM_TO_ACP_CH_NUM;
> -               ch2 = ACP_TO_I2S_DMA_CH_NUM;
> -               switch (asic_type) {
> -               case CHIP_STONEY:
> -                       sram_bank = ACP_SHARED_RAM_BANK_3_ADDRESS;
> -                       break;
> -               default:
> -                       sram_bank = ACP_SHARED_RAM_BANK_5_ADDRESS;
> -               }
> -               destination = FROM_ACP_I2S_1;
> -       }
> -
> -       acp_pte_config(acp_mmio, audio_config->pg,
audio_config->num_of_pages,
> -                      pte_offset);
> -       if (audio_config->direction == SNDRV_PCM_STREAM_PLAYBACK)
> -               dma_dscr_idx = PLAYBACK_START_DMA_DESCR_CH12;
> -       else
> -               dma_dscr_idx = CAPTURE_START_DMA_DESCR_CH14;
> -
> +       acp_pte_config(acp_mmio, rtd->pg, rtd->num_of_pages,
> +                      rtd->pte_offset);
>          /* Configure System memory <-> ACP SRAM DMA descriptors */
> -       set_acp_sysmem_dma_descriptors(acp_mmio, audio_config->size,
> -                                      audio_config->direction,
pte_offset, ch1,
> -                                      sram_bank, dma_dscr_idx,
asic_type);
> -
> -       if (audio_config->direction == SNDRV_PCM_STREAM_PLAYBACK)
> -               dma_dscr_idx = PLAYBACK_START_DMA_DESCR_CH13;
> -       else
> -               dma_dscr_idx = CAPTURE_START_DMA_DESCR_CH15;
> +       set_acp_sysmem_dma_descriptors(acp_mmio, rtd->size,
> +                                      rtd->direction, rtd->pte_offset,
> +                                      rtd->ch1, rtd->sram_bank,
> +                                      rtd->dma_dscr_idx_1, asic_type);
>          /* Configure ACP SRAM <-> I2S DMA descriptors */
> -       set_acp_to_i2s_dma_descriptors(acp_mmio, audio_config->size,
> -                                      audio_config->direction, sram_bank,
> -                                      destination, ch2, dma_dscr_idx,
> -                                      asic_type);
> +       set_acp_to_i2s_dma_descriptors(acp_mmio, rtd->size,
> +                                      rtd->direction, rtd->sram_bank,
> +                                      rtd->destination, rtd->ch2,
> +                                      rtd->dma_dscr_idx_2, asic_type);
>   }

>   /* Start a given DMA channel transfer */
> @@ -700,7 +667,6 @@ static irqreturn_t dma_irq_handler(int irq, void *arg)

>   static int acp_dma_open(struct snd_pcm_substream *substream)
>   {
> -       u16 bank;
>          int ret = 0;
>          struct snd_pcm_runtime *runtime = substream->runtime;
>          struct snd_soc_pcm_runtime *prtd = substream->private_data;
> @@ -720,6 +686,7 @@ static int acp_dma_open(struct snd_pcm_substream
*substream)
>                  default:
>                          runtime->hw = acp_pcm_hardware_playback;
>                  }
> +               adata->bytescount = 0;
>          } else {
>                  switch (intr_data->asic_type) {
>                  case CHIP_STONEY:
> @@ -728,6 +695,7 @@ static int acp_dma_open(struct snd_pcm_substream
*substream)
>                  default:
>                          runtime->hw = acp_pcm_hardware_capture;
>                  }
> +               adata->bytescount = 0;
>          }

>          ret = snd_pcm_hw_constraint_integer(runtime,
> @@ -749,28 +717,6 @@ static int acp_dma_open(struct snd_pcm_substream
*substream)
>           */
>          if (!intr_data->play_i2ssp_stream &&
!intr_data->capture_i2ssp_stream)
>                  acp_reg_write(1, adata->acp_mmio,
mmACP_EXTERNAL_INTR_ENB);
> -
> -       if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> -               intr_data->play_i2ssp_stream = substream;
> -               /*
> -                * For Stoney, Memory gating is disabled,i.e SRAM Banks
> -                * won't be turned off. The default state for SRAM banks
is ON.
> -                * Setting SRAM bank state code skipped for STONEY
platform.
> -                */
> -               if (intr_data->asic_type != CHIP_STONEY) {
> -                       for (bank = 1; bank <= 4; bank++)
> -
acp_set_sram_bank_state(intr_data->acp_mmio,
> -                                                       bank, true);
> -               }
> -       } else {
> -               intr_data->capture_i2ssp_stream = substream;
> -               if (intr_data->asic_type != CHIP_STONEY) {
> -                       for (bank = 5; bank <= 8; bank++)
> -
acp_set_sram_bank_state(intr_data->acp_mmio,
> -                                                       bank, true);
> -               }
> -       }
> -
>          return 0;
>   }

> @@ -779,6 +725,7 @@ static int acp_dma_hw_params(struct snd_pcm_substream
*substream,
>   {
>          int status;
>          uint64_t size;
> +       u16 bank;
>          u32 val = 0;
>          struct page *pg;
>          struct snd_pcm_runtime *runtime;
> @@ -804,6 +751,60 @@ static int acp_dma_hw_params(struct
snd_pcm_substream *substream,
>                  acp_reg_write(val, adata->acp_mmio,
>                                mmACP_I2S_16BIT_RESOLUTION_EN);
>          }
> +
> +       if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +               switch (adata->asic_type) {
> +               case CHIP_STONEY:
> +                       rtd->pte_offset = ACP_ST_PLAYBACK_PTE_OFFSET;
> +                       break;
> +               default:
> +                       rtd->pte_offset = ACP_PLAYBACK_PTE_OFFSET;
> +               }
> +               rtd->ch1 = SYSRAM_TO_ACP_CH_NUM;
> +               rtd->ch2 = ACP_TO_I2S_DMA_CH_NUM;
> +               rtd->sram_bank = ACP_SRAM_BANK_1_ADDRESS;
> +               rtd->destination = TO_ACP_I2S_1;
> +               rtd->dma_dscr_idx_1 = PLAYBACK_START_DMA_DESCR_CH12;
> +               rtd->dma_dscr_idx_2 = PLAYBACK_START_DMA_DESCR_CH13;
> +               rtd->byte_cnt_high_reg_offset =
> +               mmACP_I2S_TRANSMIT_BYTE_CNT_HIGH;
> +               rtd->byte_cnt_low_reg_offset =
mmACP_I2S_TRANSMIT_BYTE_CNT_LOW;
> +               adata->play_i2ssp_stream = substream;
> +               /*
> +                * For Stoney, Memory gating is disabled,i.e SRAM Banks
> +                * won't be turned off. The default state for SRAM banks
is ON.
> +                * Setting SRAM bank state code skipped for STONEY
platform.
> +                */
> +               if (adata->asic_type != CHIP_STONEY) {
> +                       for (bank = 1; bank <= 4; bank++)
> +                               acp_set_sram_bank_state(adata->acp_mmio,
> +                                                       bank, true);
> +               }
> +       } else {
> +               rtd->pte_offset = ACP_CAPTURE_PTE_OFFSET;
> +               rtd->ch1 = SYSRAM_TO_ACP_CH_NUM;
> +               rtd->ch2 = ACP_TO_I2S_DMA_CH_NUM;
> +               switch (adata->asic_type) {
> +               case CHIP_STONEY:
> +                       rtd->sram_bank = ACP_SRAM_BANK_2_ADDRESS;
> +                       break;
> +               default:
> +                       rtd->sram_bank = ACP_SRAM_BANK_5_ADDRESS;
> +               }
> +               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;
> +               adata->capture_i2ssp_stream = substream;
> +               if (adata->asic_type != CHIP_STONEY) {
> +                       for (bank = 5; bank <= 8; bank++)
> +                               acp_set_sram_bank_state(adata->acp_mmio,
> +                                                       bank, true);
> +               }
> +       }
> +
>          size = params_buffer_bytes(params);
>          status = snd_pcm_lib_malloc_pages(substream, size);
>          if (status < 0)
> @@ -837,26 +838,15 @@ 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)
> +static u64 acp_get_byte_count(struct audio_substream_data *rtd)
>   {
> -       union acp_dma_count playback_dma_count;
> -       union acp_dma_count capture_dma_count;
> -       u64 bytescount = 0;
> +       union acp_dma_count byte_count;

> -       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;
> +       byte_count.bcount.high = acp_reg_read(rtd->acp_mmio,
> +
rtd->byte_cnt_high_reg_offset);
> +       byte_count.bcount.low  = acp_reg_read(rtd->acp_mmio,
> +
rtd->byte_cnt_low_reg_offset);
> +       return byte_count.bytescount;
>   }

>   static snd_pcm_uframes_t acp_dma_pointer(struct snd_pcm_substream
*substream)
> @@ -872,15 +862,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->acp_mmio, substream->stream);
> +       bytescount = acp_get_byte_count(rtd);

> -       if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> -               if (bytescount > rtd->i2ssp_renderbytescount)
> -                       bytescount = bytescount -
rtd->i2ssp_renderbytescount;
> -       } else {
> -               if (bytescount > rtd->i2ssp_capturebytescount)
> -                       bytescount = bytescount -
rtd->i2ssp_capturebytescount;
> -       }
> +       if (bytescount > rtd->bytescount)
> +               bytescount = bytescount - rtd->bytescount;
>          pos = do_div(bytescount, buffersize);
>          return bytes_to_frames(runtime, pos);
>   }
> @@ -898,21 +883,15 @@ static int acp_dma_prepare(struct snd_pcm_substream
*substream)

>          if (!rtd)
>                  return -EINVAL;
> -       if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> -               config_acp_dma_channel(rtd->acp_mmio,
SYSRAM_TO_ACP_CH_NUM,
> -                                      PLAYBACK_START_DMA_DESCR_CH12,
> -                                      NUM_DSCRS_PER_CHANNEL, 0);
> -               config_acp_dma_channel(rtd->acp_mmio,
ACP_TO_I2S_DMA_CH_NUM,
> -                                      PLAYBACK_START_DMA_DESCR_CH13,
> -                                      NUM_DSCRS_PER_CHANNEL, 0);
> -       } else {
> -               config_acp_dma_channel(rtd->acp_mmio,
ACP_TO_SYSRAM_CH_NUM,
> -                                      CAPTURE_START_DMA_DESCR_CH14,
> -                                      NUM_DSCRS_PER_CHANNEL, 0);
> -               config_acp_dma_channel(rtd->acp_mmio,
I2S_TO_ACP_DMA_CH_NUM,
> -                                      CAPTURE_START_DMA_DESCR_CH15,
> -                                      NUM_DSCRS_PER_CHANNEL, 0);
> -       }
> +
> +       config_acp_dma_channel(rtd->acp_mmio,
> +                              rtd->ch1,
> +                              rtd->dma_dscr_idx_1,
> +                              NUM_DSCRS_PER_CHANNEL, 0);
> +       config_acp_dma_channel(rtd->acp_mmio,
> +                              rtd->ch2,
> +                              rtd->dma_dscr_idx_2,
> +                              NUM_DSCRS_PER_CHANNEL, 0);
>          return 0;
>   }

> @@ -934,15 +913,13 @@ 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);
> +               bytescount = acp_get_byte_count(rtd);
> +               if (rtd->bytescount == 0)
> +                       rtd->bytescount = bytescount;
>                  if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> -                       if (rtd->i2ssp_renderbytescount == 0)
> -                               rtd->i2ssp_renderbytescount = bytescount;
> -                       acp_dma_start(rtd->acp_mmio,
> -                                     SYSRAM_TO_ACP_CH_NUM, false);
> +                       acp_dma_start(rtd->acp_mmio, rtd->ch1, false);
>                          while (acp_reg_read(rtd->acp_mmio,
mmACP_DMA_CH_STS) &
> -
BIT(SYSRAM_TO_ACP_CH_NUM)) {
> +                               BIT(rtd->ch1)) {
>                                  if (!loops--) {
>                                          dev_err(component->dev,
>                                                  "acp dma start
timeout\n");
> @@ -950,40 +927,21 @@ static int acp_dma_trigger(struct snd_pcm_substream
*substream, int cmd)
>                                  }
>                                  cpu_relax();
>                          }
> -
> -                       acp_dma_start(rtd->acp_mmio,
> -                                     ACP_TO_I2S_DMA_CH_NUM, true);
> -
> -               } else {
> -                       if (rtd->i2ssp_capturebytescount == 0)
> -                               rtd->i2ssp_capturebytescount = bytescount;
> -                       acp_dma_start(rtd->acp_mmio,
> -                                     I2S_TO_ACP_DMA_CH_NUM, true);
>                  }
> +               acp_dma_start(rtd->acp_mmio, rtd->ch2, true);
>                  ret = 0;
>                  break;
>          case SNDRV_PCM_TRIGGER_STOP:
>          case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
>          case SNDRV_PCM_TRIGGER_SUSPEND:
> -               /*
> -                * Need to stop only circular DMA channels :
> -                * ACP_TO_I2S_DMA_CH_NUM / I2S_TO_ACP_DMA_CH_NUM.
Non-circular
> -                * 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) {
> -                       ret = acp_dma_stop(rtd->acp_mmio,
> -                                          SYSRAM_TO_ACP_CH_NUM);
> -                       ret = acp_dma_stop(rtd->acp_mmio,
> -                                          ACP_TO_I2S_DMA_CH_NUM);
> -                       rtd->i2ssp_renderbytescount = 0;
> +                       acp_dma_stop(rtd->acp_mmio, rtd->ch1);
> +                       ret =  acp_dma_stop(rtd->acp_mmio, rtd->ch2);
>                  } else {
> -                       ret = acp_dma_stop(rtd->acp_mmio,
> -                                          I2S_TO_ACP_DMA_CH_NUM);
> -                       ret = acp_dma_stop(rtd->acp_mmio,
> -                                          ACP_TO_SYSRAM_CH_NUM);
> -                       rtd->i2ssp_capturebytescount = 0;
> +                       acp_dma_stop(rtd->acp_mmio, rtd->ch2);
> +                       ret = acp_dma_stop(rtd->acp_mmio, rtd->ch1);
>                  }
> +               rtd->bytescount = 0;
>                  break;
>          default:
>                  ret = -EINVAL;
> @@ -1028,8 +986,6 @@ static int acp_dma_close(struct snd_pcm_substream
*substream)

DRV_NAME);
>          struct audio_drv_data *adata = dev_get_drvdata(component->dev);

> -       kfree(rtd);
> -
>          if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>                  adata->play_i2ssp_stream = NULL;
>                  /*
> @@ -1059,6 +1015,7 @@ static int acp_dma_close(struct snd_pcm_substream
*substream)
>          if (!adata->play_i2ssp_stream && !adata->capture_i2ssp_stream)
>                  acp_reg_write(0, adata->acp_mmio,
mmACP_EXTERNAL_INTR_ENB);

> +       kfree(rtd);
>          return 0;
>   }

> diff --git a/sound/soc/amd/acp.h b/sound/soc/amd/acp.h
> index 0e6089b..62695ed 100644
> --- a/sound/soc/amd/acp.h
> +++ b/sound/soc/amd/acp.h
> @@ -10,17 +10,28 @@
>   #define ACP_PLAYBACK_PTE_OFFSET                        10
>   #define ACP_CAPTURE_PTE_OFFSET                 0

> +/* Playback and Capture Offset for Stoney */
> +#define ACP_ST_PLAYBACK_PTE_OFFSET     0x04
> +#define ACP_ST_CAPTURE_PTE_OFFSET      0x00
> +
>   #define ACP_GARLIC_CNTL_DEFAULT                        0x00000FB4
>   #define ACP_ONION_CNTL_DEFAULT                 0x00000FB4

>   #define ACP_PHYSICAL_BASE                      0x14000

> -/* Playback SRAM address (as a destination in dma descriptor) */
> -#define ACP_SHARED_RAM_BANK_1_ADDRESS          0x4002000
> -
> -/* Capture SRAM address (as a source in dma descriptor) */
> -#define ACP_SHARED_RAM_BANK_5_ADDRESS          0x400A000
> -#define ACP_SHARED_RAM_BANK_3_ADDRESS          0x4006000
> +/*
> + * In case of I2S SP controller instance, Stoney uses SRAM bank 1 for
> + * playback and SRAM Bank 2 for capture where as in case of BT I2S
> + * Instance, Stoney uses SRAM Bank 3 for playback & SRAM Bank 4 will
> + * be used for capture. Carrizo uses I2S SP controller instance. SRAM
Banks
> + * 1, 2, 3, 4 will be used for playback & SRAM Banks 5, 6, 7, 8 will be
used
> + * for capture scenario.
> + */
> +#define ACP_SRAM_BANK_1_ADDRESS                0x4002000
> +#define ACP_SRAM_BANK_2_ADDRESS                0x4004000
> +#define ACP_SRAM_BANK_3_ADDRESS                0x4006000
> +#define ACP_SRAM_BANK_4_ADDRESS                0x4008000
> +#define ACP_SRAM_BANK_5_ADDRESS                0x400A000

>   #define ACP_DMA_RESET_TIME                     10000
>   #define ACP_CLOCK_EN_TIME_OUT_VALUE            0x000000FF
> @@ -85,9 +96,17 @@ struct audio_substream_data {
>          unsigned int order;
>          u16 num_of_pages;
>          u16 direction;
> +       u16 ch1;
> +       u16 ch2;
> +       u16 destination;
> +       u16 dma_dscr_idx_1;
> +       u16 dma_dscr_idx_2;
> +       u32 pte_offset;
> +       u32 sram_bank;
> +       u32 byte_cnt_high_reg_offset;
> +       u32 byte_cnt_low_reg_offset;
>          uint64_t size;
> -       u64 i2ssp_renderbytescount;
> -       u64 i2ssp_capturebytescount;
> +       u64 bytescount;
>          void __iomem *acp_mmio;
>   };

> --
> 2.7.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] ASoC: amd: acp dma driver code cleanup
  2018-04-24  6:05 ` [PATCH 1/3] ASoC: amd: acp dma driver code cleanup Daniel Kurtz
@ 2018-04-24  7:06   ` Mukunda,Vijendar
  0 siblings, 0 replies; 10+ messages in thread
From: Mukunda,Vijendar @ 2018-04-24  7:06 UTC (permalink / raw)
  To: Daniel Kurtz
  Cc: Liam Girdwood, Mark Brown, perex, tiwai, alexander.deucher,
	Akshu Agrawal, jclinton, Guenter Roeck, Kuninori Morimoto,
	Greg Kroah-Hartman, alsa-devel, linux-kernel



On Tuesday 24 April 2018 11:35 AM, Daniel Kurtz wrote:
> Hi Vijendar,
> 
> 
> On Mon, Apr 23, 2018 at 9:02 PM Vijendar Mukunda <Vijendar.Mukunda@amd.com>
> wrote:
> 
>> Added dma configuration parameters in audio_substream_data
>> structure. Moved dma configuration parameters initialization
>> to dma hw params callback.
>> Removed separate byte count variables for playback and capture.
>> Added variables to store ACP register offsets in audio_substream_data
>> structure.
> 
> Thanks for splitting the patch, this is moving in the right direction, but
> still very difficult to review since it is mixing different changes
> together.
> Just try to make each patch a single logical cleanup.
> For example, perhaps create a set of patches that does:
>     (1) Variable renames (eg audio_config -> rtd) & white space cleanup
>     (2) Add dma configuration parameters to audio_substream_data structure
> and initialize them in hw_params.
>     (3) Remove separate byte count variables for playback and capture
>     (4) Update the PTE offsets
>     (5) Update the SRAM_BANKs
> 
> Note that (1) doesn't change functionality at all, (2) refactors but
> doesn't change behavior or logic, (3) simplifies behavior but doesn't
> change logic, and (4) & (5) build on the others but start making real
> logical changes.
> 
> -Dan

Hi Dan,

I will split the patch and re spin the patch set.

Thanks,
Vijendar
> 
> 
>> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
>> ---
>>    sound/soc/amd/acp-pcm-dma.c | 241
> ++++++++++++++++++--------------------------
>>    sound/soc/amd/acp.h         |  35 +++++--
>>    2 files changed, 126 insertions(+), 150 deletions(-)
> 
>> diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
>> index 5ffe2ef..4a4bbdf 100644
>> --- a/sound/soc/amd/acp-pcm-dma.c
>> +++ b/sound/soc/amd/acp-pcm-dma.c
>> @@ -317,54 +317,21 @@ static void acp_pte_config(void __iomem *acp_mmio,
> struct page *pg,
>>    }
> 
>>    static void config_acp_dma(void __iomem *acp_mmio,
>> -                          struct audio_substream_data *audio_config,
>> +                          struct audio_substream_data *rtd,
>>                              u32 asic_type)
>>    {
>> -       u32 pte_offset, sram_bank;
>> -       u16 ch1, ch2, destination, dma_dscr_idx;
>> -
>> -       if (audio_config->direction == SNDRV_PCM_STREAM_PLAYBACK) {
>> -               pte_offset = ACP_PLAYBACK_PTE_OFFSET;
>> -               ch1 = SYSRAM_TO_ACP_CH_NUM;
>> -               ch2 = ACP_TO_I2S_DMA_CH_NUM;
>> -               sram_bank = ACP_SHARED_RAM_BANK_1_ADDRESS;
>> -               destination = TO_ACP_I2S_1;
>> -
>> -       } else {
>> -               pte_offset = ACP_CAPTURE_PTE_OFFSET;
>> -               ch1 = SYSRAM_TO_ACP_CH_NUM;
>> -               ch2 = ACP_TO_I2S_DMA_CH_NUM;
>> -               switch (asic_type) {
>> -               case CHIP_STONEY:
>> -                       sram_bank = ACP_SHARED_RAM_BANK_3_ADDRESS;
>> -                       break;
>> -               default:
>> -                       sram_bank = ACP_SHARED_RAM_BANK_5_ADDRESS;
>> -               }
>> -               destination = FROM_ACP_I2S_1;
>> -       }
>> -
>> -       acp_pte_config(acp_mmio, audio_config->pg,
> audio_config->num_of_pages,
>> -                      pte_offset);
>> -       if (audio_config->direction == SNDRV_PCM_STREAM_PLAYBACK)
>> -               dma_dscr_idx = PLAYBACK_START_DMA_DESCR_CH12;
>> -       else
>> -               dma_dscr_idx = CAPTURE_START_DMA_DESCR_CH14;
>> -
>> +       acp_pte_config(acp_mmio, rtd->pg, rtd->num_of_pages,
>> +                      rtd->pte_offset);
>>           /* Configure System memory <-> ACP SRAM DMA descriptors */
>> -       set_acp_sysmem_dma_descriptors(acp_mmio, audio_config->size,
>> -                                      audio_config->direction,
> pte_offset, ch1,
>> -                                      sram_bank, dma_dscr_idx,
> asic_type);
>> -
>> -       if (audio_config->direction == SNDRV_PCM_STREAM_PLAYBACK)
>> -               dma_dscr_idx = PLAYBACK_START_DMA_DESCR_CH13;
>> -       else
>> -               dma_dscr_idx = CAPTURE_START_DMA_DESCR_CH15;
>> +       set_acp_sysmem_dma_descriptors(acp_mmio, rtd->size,
>> +                                      rtd->direction, rtd->pte_offset,
>> +                                      rtd->ch1, rtd->sram_bank,
>> +                                      rtd->dma_dscr_idx_1, asic_type);
>>           /* Configure ACP SRAM <-> I2S DMA descriptors */
>> -       set_acp_to_i2s_dma_descriptors(acp_mmio, audio_config->size,
>> -                                      audio_config->direction, sram_bank,
>> -                                      destination, ch2, dma_dscr_idx,
>> -                                      asic_type);
>> +       set_acp_to_i2s_dma_descriptors(acp_mmio, rtd->size,
>> +                                      rtd->direction, rtd->sram_bank,
>> +                                      rtd->destination, rtd->ch2,
>> +                                      rtd->dma_dscr_idx_2, asic_type);
>>    }
> 
>>    /* Start a given DMA channel transfer */
>> @@ -700,7 +667,6 @@ static irqreturn_t dma_irq_handler(int irq, void *arg)
> 
>>    static int acp_dma_open(struct snd_pcm_substream *substream)
>>    {
>> -       u16 bank;
>>           int ret = 0;
>>           struct snd_pcm_runtime *runtime = substream->runtime;
>>           struct snd_soc_pcm_runtime *prtd = substream->private_data;
>> @@ -720,6 +686,7 @@ static int acp_dma_open(struct snd_pcm_substream
> *substream)
>>                   default:
>>                           runtime->hw = acp_pcm_hardware_playback;
>>                   }
>> +               adata->bytescount = 0;
>>           } else {
>>                   switch (intr_data->asic_type) {
>>                   case CHIP_STONEY:
>> @@ -728,6 +695,7 @@ static int acp_dma_open(struct snd_pcm_substream
> *substream)
>>                   default:
>>                           runtime->hw = acp_pcm_hardware_capture;
>>                   }
>> +               adata->bytescount = 0;
>>           }
> 
>>           ret = snd_pcm_hw_constraint_integer(runtime,
>> @@ -749,28 +717,6 @@ static int acp_dma_open(struct snd_pcm_substream
> *substream)
>>            */
>>           if (!intr_data->play_i2ssp_stream &&
> !intr_data->capture_i2ssp_stream)
>>                   acp_reg_write(1, adata->acp_mmio,
> mmACP_EXTERNAL_INTR_ENB);
>> -
>> -       if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>> -               intr_data->play_i2ssp_stream = substream;
>> -               /*
>> -                * For Stoney, Memory gating is disabled,i.e SRAM Banks
>> -                * won't be turned off. The default state for SRAM banks
> is ON.
>> -                * Setting SRAM bank state code skipped for STONEY
> platform.
>> -                */
>> -               if (intr_data->asic_type != CHIP_STONEY) {
>> -                       for (bank = 1; bank <= 4; bank++)
>> -
> acp_set_sram_bank_state(intr_data->acp_mmio,
>> -                                                       bank, true);
>> -               }
>> -       } else {
>> -               intr_data->capture_i2ssp_stream = substream;
>> -               if (intr_data->asic_type != CHIP_STONEY) {
>> -                       for (bank = 5; bank <= 8; bank++)
>> -
> acp_set_sram_bank_state(intr_data->acp_mmio,
>> -                                                       bank, true);
>> -               }
>> -       }
>> -
>>           return 0;
>>    }
> 
>> @@ -779,6 +725,7 @@ static int acp_dma_hw_params(struct snd_pcm_substream
> *substream,
>>    {
>>           int status;
>>           uint64_t size;
>> +       u16 bank;
>>           u32 val = 0;
>>           struct page *pg;
>>           struct snd_pcm_runtime *runtime;
>> @@ -804,6 +751,60 @@ static int acp_dma_hw_params(struct
> snd_pcm_substream *substream,
>>                   acp_reg_write(val, adata->acp_mmio,
>>                                 mmACP_I2S_16BIT_RESOLUTION_EN);
>>           }
>> +
>> +       if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>> +               switch (adata->asic_type) {
>> +               case CHIP_STONEY:
>> +                       rtd->pte_offset = ACP_ST_PLAYBACK_PTE_OFFSET;
>> +                       break;
>> +               default:
>> +                       rtd->pte_offset = ACP_PLAYBACK_PTE_OFFSET;
>> +               }
>> +               rtd->ch1 = SYSRAM_TO_ACP_CH_NUM;
>> +               rtd->ch2 = ACP_TO_I2S_DMA_CH_NUM;
>> +               rtd->sram_bank = ACP_SRAM_BANK_1_ADDRESS;
>> +               rtd->destination = TO_ACP_I2S_1;
>> +               rtd->dma_dscr_idx_1 = PLAYBACK_START_DMA_DESCR_CH12;
>> +               rtd->dma_dscr_idx_2 = PLAYBACK_START_DMA_DESCR_CH13;
>> +               rtd->byte_cnt_high_reg_offset =
>> +               mmACP_I2S_TRANSMIT_BYTE_CNT_HIGH;
>> +               rtd->byte_cnt_low_reg_offset =
> mmACP_I2S_TRANSMIT_BYTE_CNT_LOW;
>> +               adata->play_i2ssp_stream = substream;
>> +               /*
>> +                * For Stoney, Memory gating is disabled,i.e SRAM Banks
>> +                * won't be turned off. The default state for SRAM banks
> is ON.
>> +                * Setting SRAM bank state code skipped for STONEY
> platform.
>> +                */
>> +               if (adata->asic_type != CHIP_STONEY) {
>> +                       for (bank = 1; bank <= 4; bank++)
>> +                               acp_set_sram_bank_state(adata->acp_mmio,
>> +                                                       bank, true);
>> +               }
>> +       } else {
>> +               rtd->pte_offset = ACP_CAPTURE_PTE_OFFSET;
>> +               rtd->ch1 = SYSRAM_TO_ACP_CH_NUM;
>> +               rtd->ch2 = ACP_TO_I2S_DMA_CH_NUM;
>> +               switch (adata->asic_type) {
>> +               case CHIP_STONEY:
>> +                       rtd->sram_bank = ACP_SRAM_BANK_2_ADDRESS;
>> +                       break;
>> +               default:
>> +                       rtd->sram_bank = ACP_SRAM_BANK_5_ADDRESS;
>> +               }
>> +               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;
>> +               adata->capture_i2ssp_stream = substream;
>> +               if (adata->asic_type != CHIP_STONEY) {
>> +                       for (bank = 5; bank <= 8; bank++)
>> +                               acp_set_sram_bank_state(adata->acp_mmio,
>> +                                                       bank, true);
>> +               }
>> +       }
>> +
>>           size = params_buffer_bytes(params);
>>           status = snd_pcm_lib_malloc_pages(substream, size);
>>           if (status < 0)
>> @@ -837,26 +838,15 @@ 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)
>> +static u64 acp_get_byte_count(struct audio_substream_data *rtd)
>>    {
>> -       union acp_dma_count playback_dma_count;
>> -       union acp_dma_count capture_dma_count;
>> -       u64 bytescount = 0;
>> +       union acp_dma_count byte_count;
> 
>> -       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;
>> +       byte_count.bcount.high = acp_reg_read(rtd->acp_mmio,
>> +
> rtd->byte_cnt_high_reg_offset);
>> +       byte_count.bcount.low  = acp_reg_read(rtd->acp_mmio,
>> +
> rtd->byte_cnt_low_reg_offset);
>> +       return byte_count.bytescount;
>>    }
> 
>>    static snd_pcm_uframes_t acp_dma_pointer(struct snd_pcm_substream
> *substream)
>> @@ -872,15 +862,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->acp_mmio, substream->stream);
>> +       bytescount = acp_get_byte_count(rtd);
> 
>> -       if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>> -               if (bytescount > rtd->i2ssp_renderbytescount)
>> -                       bytescount = bytescount -
> rtd->i2ssp_renderbytescount;
>> -       } else {
>> -               if (bytescount > rtd->i2ssp_capturebytescount)
>> -                       bytescount = bytescount -
> rtd->i2ssp_capturebytescount;
>> -       }
>> +       if (bytescount > rtd->bytescount)
>> +               bytescount = bytescount - rtd->bytescount;
>>           pos = do_div(bytescount, buffersize);
>>           return bytes_to_frames(runtime, pos);
>>    }
>> @@ -898,21 +883,15 @@ static int acp_dma_prepare(struct snd_pcm_substream
> *substream)
> 
>>           if (!rtd)
>>                   return -EINVAL;
>> -       if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>> -               config_acp_dma_channel(rtd->acp_mmio,
> SYSRAM_TO_ACP_CH_NUM,
>> -                                      PLAYBACK_START_DMA_DESCR_CH12,
>> -                                      NUM_DSCRS_PER_CHANNEL, 0);
>> -               config_acp_dma_channel(rtd->acp_mmio,
> ACP_TO_I2S_DMA_CH_NUM,
>> -                                      PLAYBACK_START_DMA_DESCR_CH13,
>> -                                      NUM_DSCRS_PER_CHANNEL, 0);
>> -       } else {
>> -               config_acp_dma_channel(rtd->acp_mmio,
> ACP_TO_SYSRAM_CH_NUM,
>> -                                      CAPTURE_START_DMA_DESCR_CH14,
>> -                                      NUM_DSCRS_PER_CHANNEL, 0);
>> -               config_acp_dma_channel(rtd->acp_mmio,
> I2S_TO_ACP_DMA_CH_NUM,
>> -                                      CAPTURE_START_DMA_DESCR_CH15,
>> -                                      NUM_DSCRS_PER_CHANNEL, 0);
>> -       }
>> +
>> +       config_acp_dma_channel(rtd->acp_mmio,
>> +                              rtd->ch1,
>> +                              rtd->dma_dscr_idx_1,
>> +                              NUM_DSCRS_PER_CHANNEL, 0);
>> +       config_acp_dma_channel(rtd->acp_mmio,
>> +                              rtd->ch2,
>> +                              rtd->dma_dscr_idx_2,
>> +                              NUM_DSCRS_PER_CHANNEL, 0);
>>           return 0;
>>    }
> 
>> @@ -934,15 +913,13 @@ 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);
>> +               bytescount = acp_get_byte_count(rtd);
>> +               if (rtd->bytescount == 0)
>> +                       rtd->bytescount = bytescount;
>>                   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>> -                       if (rtd->i2ssp_renderbytescount == 0)
>> -                               rtd->i2ssp_renderbytescount = bytescount;
>> -                       acp_dma_start(rtd->acp_mmio,
>> -                                     SYSRAM_TO_ACP_CH_NUM, false);
>> +                       acp_dma_start(rtd->acp_mmio, rtd->ch1, false);
>>                           while (acp_reg_read(rtd->acp_mmio,
> mmACP_DMA_CH_STS) &
>> -
> BIT(SYSRAM_TO_ACP_CH_NUM)) {
>> +                               BIT(rtd->ch1)) {
>>                                   if (!loops--) {
>>                                           dev_err(component->dev,
>>                                                   "acp dma start
> timeout\n");
>> @@ -950,40 +927,21 @@ static int acp_dma_trigger(struct snd_pcm_substream
> *substream, int cmd)
>>                                   }
>>                                   cpu_relax();
>>                           }
>> -
>> -                       acp_dma_start(rtd->acp_mmio,
>> -                                     ACP_TO_I2S_DMA_CH_NUM, true);
>> -
>> -               } else {
>> -                       if (rtd->i2ssp_capturebytescount == 0)
>> -                               rtd->i2ssp_capturebytescount = bytescount;
>> -                       acp_dma_start(rtd->acp_mmio,
>> -                                     I2S_TO_ACP_DMA_CH_NUM, true);
>>                   }
>> +               acp_dma_start(rtd->acp_mmio, rtd->ch2, true);
>>                   ret = 0;
>>                   break;
>>           case SNDRV_PCM_TRIGGER_STOP:
>>           case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
>>           case SNDRV_PCM_TRIGGER_SUSPEND:
>> -               /*
>> -                * Need to stop only circular DMA channels :
>> -                * ACP_TO_I2S_DMA_CH_NUM / I2S_TO_ACP_DMA_CH_NUM.
> Non-circular
>> -                * 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) {
>> -                       ret = acp_dma_stop(rtd->acp_mmio,
>> -                                          SYSRAM_TO_ACP_CH_NUM);
>> -                       ret = acp_dma_stop(rtd->acp_mmio,
>> -                                          ACP_TO_I2S_DMA_CH_NUM);
>> -                       rtd->i2ssp_renderbytescount = 0;
>> +                       acp_dma_stop(rtd->acp_mmio, rtd->ch1);
>> +                       ret =  acp_dma_stop(rtd->acp_mmio, rtd->ch2);
>>                   } else {
>> -                       ret = acp_dma_stop(rtd->acp_mmio,
>> -                                          I2S_TO_ACP_DMA_CH_NUM);
>> -                       ret = acp_dma_stop(rtd->acp_mmio,
>> -                                          ACP_TO_SYSRAM_CH_NUM);
>> -                       rtd->i2ssp_capturebytescount = 0;
>> +                       acp_dma_stop(rtd->acp_mmio, rtd->ch2);
>> +                       ret = acp_dma_stop(rtd->acp_mmio, rtd->ch1);
>>                   }
>> +               rtd->bytescount = 0;
>>                   break;
>>           default:
>>                   ret = -EINVAL;
>> @@ -1028,8 +986,6 @@ static int acp_dma_close(struct snd_pcm_substream
> *substream)
> 
> DRV_NAME);
>>           struct audio_drv_data *adata = dev_get_drvdata(component->dev);
> 
>> -       kfree(rtd);
>> -
>>           if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>>                   adata->play_i2ssp_stream = NULL;
>>                   /*
>> @@ -1059,6 +1015,7 @@ static int acp_dma_close(struct snd_pcm_substream
> *substream)
>>           if (!adata->play_i2ssp_stream && !adata->capture_i2ssp_stream)
>>                   acp_reg_write(0, adata->acp_mmio,
> mmACP_EXTERNAL_INTR_ENB);
> 
>> +       kfree(rtd);
>>           return 0;
>>    }
> 
>> diff --git a/sound/soc/amd/acp.h b/sound/soc/amd/acp.h
>> index 0e6089b..62695ed 100644
>> --- a/sound/soc/amd/acp.h
>> +++ b/sound/soc/amd/acp.h
>> @@ -10,17 +10,28 @@
>>    #define ACP_PLAYBACK_PTE_OFFSET                        10
>>    #define ACP_CAPTURE_PTE_OFFSET                 0
> 
>> +/* Playback and Capture Offset for Stoney */
>> +#define ACP_ST_PLAYBACK_PTE_OFFSET     0x04
>> +#define ACP_ST_CAPTURE_PTE_OFFSET      0x00
>> +
>>    #define ACP_GARLIC_CNTL_DEFAULT                        0x00000FB4
>>    #define ACP_ONION_CNTL_DEFAULT                 0x00000FB4
> 
>>    #define ACP_PHYSICAL_BASE                      0x14000
> 
>> -/* Playback SRAM address (as a destination in dma descriptor) */
>> -#define ACP_SHARED_RAM_BANK_1_ADDRESS          0x4002000
>> -
>> -/* Capture SRAM address (as a source in dma descriptor) */
>> -#define ACP_SHARED_RAM_BANK_5_ADDRESS          0x400A000
>> -#define ACP_SHARED_RAM_BANK_3_ADDRESS          0x4006000
>> +/*
>> + * In case of I2S SP controller instance, Stoney uses SRAM bank 1 for
>> + * playback and SRAM Bank 2 for capture where as in case of BT I2S
>> + * Instance, Stoney uses SRAM Bank 3 for playback & SRAM Bank 4 will
>> + * be used for capture. Carrizo uses I2S SP controller instance. SRAM
> Banks
>> + * 1, 2, 3, 4 will be used for playback & SRAM Banks 5, 6, 7, 8 will be
> used
>> + * for capture scenario.
>> + */
>> +#define ACP_SRAM_BANK_1_ADDRESS                0x4002000
>> +#define ACP_SRAM_BANK_2_ADDRESS                0x4004000
>> +#define ACP_SRAM_BANK_3_ADDRESS                0x4006000
>> +#define ACP_SRAM_BANK_4_ADDRESS                0x4008000
>> +#define ACP_SRAM_BANK_5_ADDRESS                0x400A000
> 
>>    #define ACP_DMA_RESET_TIME                     10000
>>    #define ACP_CLOCK_EN_TIME_OUT_VALUE            0x000000FF
>> @@ -85,9 +96,17 @@ struct audio_substream_data {
>>           unsigned int order;
>>           u16 num_of_pages;
>>           u16 direction;
>> +       u16 ch1;
>> +       u16 ch2;
>> +       u16 destination;
>> +       u16 dma_dscr_idx_1;
>> +       u16 dma_dscr_idx_2;
>> +       u32 pte_offset;
>> +       u32 sram_bank;
>> +       u32 byte_cnt_high_reg_offset;
>> +       u32 byte_cnt_low_reg_offset;
>>           uint64_t size;
>> -       u64 i2ssp_renderbytescount;
>> -       u64 i2ssp_capturebytescount;
>> +       u64 bytescount;
>>           void __iomem *acp_mmio;
>>    };
> 
>> --
>> 2.7.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] ASoC: AMD: Move clk enable from hw_params/free to startup/shutdown
  2018-04-24  3:04   ` Vijendar Mukunda
  (?)
@ 2018-04-24 16:36   ` Daniel Kurtz
  2018-04-25  3:36     ` Agrawal, Akshu
  -1 siblings, 1 reply; 10+ messages in thread
From: Daniel Kurtz @ 2018-04-24 16:36 UTC (permalink / raw)
  To: Vijendar.Mukunda
  Cc: Akshu Agrawal, Liam Girdwood, Mark Brown, perex, tiwai,
	alexander.deucher, Kuninori Morimoto, alsa-devel, linux-kernel

On Mon, Apr 23, 2018 at 9:03 PM Vijendar Mukunda <Vijendar.Mukunda@amd.com>
wrote:

> From: Akshu Agrawal <akshu.agrawal@amd.com>

> hw_param can be called multiple times and thus we can have
> more clk enable. The clk may not get diabled due to refcounting.
> startup/shutdown ensures single clk enable/disable call.

> Signed-off-by: Akshu Agrawal <akshu.agrawal@amd.com>
> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
> ---
>   sound/soc/amd/acp-da7219-max98357a.c | 54
++++++++++++++++++++++++------------
>   1 file changed, 37 insertions(+), 17 deletions(-)

> diff --git a/sound/soc/amd/acp-da7219-max98357a.c
b/sound/soc/amd/acp-da7219-max98357a.c
> index b205c78..0f16f6d 100644
> --- a/sound/soc/amd/acp-da7219-max98357a.c
> +++ b/sound/soc/amd/acp-da7219-max98357a.c
> @@ -38,8 +38,7 @@
>   #include "../codecs/da7219.h"
>   #include "../codecs/da7219-aad.h"

> -#define CZ_PLAT_CLK 24000000
> -#define MCLK_RATE 24576000
> +#define CZ_PLAT_CLK 25000000
>   #define DUAL_CHANNEL           2

>   static struct snd_soc_jack cz_jack;
> @@ -62,7 +61,7 @@ static int cz_da7219_init(struct snd_soc_pcm_runtime
*rtd)
>          }

>          ret = snd_soc_dai_set_pll(codec_dai, 0, DA7219_SYSCLK_PLL,
> -                                 CZ_PLAT_CLK, MCLK_RATE);
> +                                 CZ_PLAT_CLK, DA7219_PLL_FREQ_OUT_98304);

These are unrelated fixes that should be in their own patch.

>          if (ret < 0) {
>                  dev_err(rtd->dev, "can't set codec pll: %d\n", ret);
>                  return ret;
> @@ -85,8 +84,7 @@ static int cz_da7219_init(struct snd_soc_pcm_runtime
*rtd)
>          return 0;
>   }

> -static int cz_da7219_hw_params(struct snd_pcm_substream *substream,
> -                            struct snd_pcm_hw_params *params)
> +static int da7219_clk_enable(struct snd_pcm_substream *substream)
>   {
>          int ret = 0;
>          struct snd_soc_pcm_runtime *rtd = substream->private_data;
> @@ -100,11 +98,9 @@ static int cz_da7219_hw_params(struct
snd_pcm_substream *substream,
>          return ret;
>   }

> -static int cz_da7219_hw_free(struct snd_pcm_substream *substream)
> +static void da7219_clk_disable(void)
>   {
>          clk_disable_unprepare(da7219_dai_clk);
> -
> -       return 0;
>   }

>   static const unsigned int channels[] = {
> @@ -127,7 +123,7 @@ static const struct snd_pcm_hw_constraint_list
constraints_channels = {
>          .mask = 0,
>   };

> -static int cz_fe_startup(struct snd_pcm_substream *substream)
> +static int cz_da7219_startup(struct snd_pcm_substream *substream)
>   {
>          struct snd_pcm_runtime *runtime = substream->runtime;

> @@ -141,23 +137,47 @@ static int cz_fe_startup(struct snd_pcm_substream
*substream)
>          snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
>                                     &constraints_rates);

> -       return 0;
> +       return da7219_clk_enable(substream);
> +}
> +
> +static void cz_da7219_shutdown(struct snd_pcm_substream *substream)
> +{
> +       da7219_clk_disable();
> +}
> +
> +static int cz_max_startup(struct snd_pcm_substream *substream)
> +{
> +       return da7219_clk_enable(substream);
> +}
> +
> +static void cz_max_shutdown(struct snd_pcm_substream *substream)
> +{
> +       da7219_clk_disable();
> +}
> +
> +static int cz_dmic_startup(struct snd_pcm_substream *substream)
> +{
> +       return da7219_clk_enable(substream);
> +}
> +
> +static void cz_dmic_shutdown(struct snd_pcm_substream *substream)
> +{
> +       da7219_clk_disable();
>   }

This is ok, or you could combine the common cz_max_* & cz_dmic_*.


>   static struct snd_soc_ops cz_da7219_cap_ops = {

I think these should all be "static const struct snd_soc_ops"  (please fix
in a separate patch).

> -       .hw_params = cz_da7219_hw_params,
> -       .hw_free = cz_da7219_hw_free,
> -       .startup = cz_fe_startup,
> +       .startup = cz_da7219_startup,
> +       .shutdown = cz_da7219_shutdown,
>   };

>   static struct snd_soc_ops cz_max_play_ops = {
> -       .hw_params = cz_da7219_hw_params,
> -       .hw_free = cz_da7219_hw_free,
> +       .startup = cz_max_startup,
> +       .shutdown = cz_max_shutdown,
>   };

>   static struct snd_soc_ops cz_dmic_cap_ops = {
> -       .hw_params = cz_da7219_hw_params,
> -       .hw_free = cz_da7219_hw_free,
> +       .startup = cz_dmic_startup,
> +       .shutdown = cz_dmic_shutdown,
>   };

>   static struct snd_soc_dai_link cz_dai_7219_98357[] = {
> --
> 2.7.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] ASoC: AMD: Move clk enable from hw_params/free to startup/shutdown
  2018-04-24 16:36   ` Daniel Kurtz
@ 2018-04-25  3:36     ` Agrawal, Akshu
  0 siblings, 0 replies; 10+ messages in thread
From: Agrawal, Akshu @ 2018-04-25  3:36 UTC (permalink / raw)
  To: Daniel Kurtz, Vijendar.Mukunda
  Cc: Liam Girdwood, Mark Brown, perex, tiwai, alexander.deucher,
	Kuninori Morimoto, alsa-devel, linux-kernel



On 4/24/2018 10:06 PM, Daniel Kurtz wrote:
> On Mon, Apr 23, 2018 at 9:03 PM Vijendar Mukunda <Vijendar.Mukunda@amd.com>
> wrote:
> 
>> From: Akshu Agrawal <akshu.agrawal@amd.com>
> 
>> hw_param can be called multiple times and thus we can have
>> more clk enable. The clk may not get diabled due to refcounting.
>> startup/shutdown ensures single clk enable/disable call.
> 
>> Signed-off-by: Akshu Agrawal <akshu.agrawal@amd.com>
>> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
>> ---
>>    sound/soc/amd/acp-da7219-max98357a.c | 54
> ++++++++++++++++++++++++------------
>>    1 file changed, 37 insertions(+), 17 deletions(-)
> 
>> diff --git a/sound/soc/amd/acp-da7219-max98357a.c
> b/sound/soc/amd/acp-da7219-max98357a.c
>> index b205c78..0f16f6d 100644
>> --- a/sound/soc/amd/acp-da7219-max98357a.c
>> +++ b/sound/soc/amd/acp-da7219-max98357a.c
>> @@ -38,8 +38,7 @@
>>    #include "../codecs/da7219.h"
>>    #include "../codecs/da7219-aad.h"
> 
>> -#define CZ_PLAT_CLK 24000000
>> -#define MCLK_RATE 24576000
>> +#define CZ_PLAT_CLK 25000000
>>    #define DUAL_CHANNEL           2
> 
>>    static struct snd_soc_jack cz_jack;
>> @@ -62,7 +61,7 @@ static int cz_da7219_init(struct snd_soc_pcm_runtime
> *rtd)
>>           }
> 
>>           ret = snd_soc_dai_set_pll(codec_dai, 0, DA7219_SYSCLK_PLL,
>> -                                 CZ_PLAT_CLK, MCLK_RATE);
>> +                                 CZ_PLAT_CLK, DA7219_PLL_FREQ_OUT_98304);
> 
> These are unrelated fixes that should be in their own patch.
> 
Accepted. Will split it.

>>           if (ret < 0) {
>>                   dev_err(rtd->dev, "can't set codec pll: %d\n", ret);
>>                   return ret;
>> @@ -85,8 +84,7 @@ static int cz_da7219_init(struct snd_soc_pcm_runtime
> *rtd)
>>           return 0;
>>    }
> 
>> -static int cz_da7219_hw_params(struct snd_pcm_substream *substream,
>> -                            struct snd_pcm_hw_params *params)
>> +static int da7219_clk_enable(struct snd_pcm_substream *substream)
>>    {
>>           int ret = 0;
>>           struct snd_soc_pcm_runtime *rtd = substream->private_data;
>> @@ -100,11 +98,9 @@ static int cz_da7219_hw_params(struct
> snd_pcm_substream *substream,
>>           return ret;
>>    }
> 
>> -static int cz_da7219_hw_free(struct snd_pcm_substream *substream)
>> +static void da7219_clk_disable(void)
>>    {
>>           clk_disable_unprepare(da7219_dai_clk);
>> -
>> -       return 0;
>>    }
> 
>>    static const unsigned int channels[] = {
>> @@ -127,7 +123,7 @@ static const struct snd_pcm_hw_constraint_list
> constraints_channels = {
>>           .mask = 0,
>>    };
> 
>> -static int cz_fe_startup(struct snd_pcm_substream *substream)
>> +static int cz_da7219_startup(struct snd_pcm_substream *substream)
>>    {
>>           struct snd_pcm_runtime *runtime = substream->runtime;
> 
>> @@ -141,23 +137,47 @@ static int cz_fe_startup(struct snd_pcm_substream
> *substream)
>>           snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
>>                                      &constraints_rates);
> 
>> -       return 0;
>> +       return da7219_clk_enable(substream);
>> +}
>> +
>> +static void cz_da7219_shutdown(struct snd_pcm_substream *substream)
>> +{
>> +       da7219_clk_disable();
>> +}
>> +
>> +static int cz_max_startup(struct snd_pcm_substream *substream)
>> +{
>> +       return da7219_clk_enable(substream);
>> +}
>> +
>> +static void cz_max_shutdown(struct snd_pcm_substream *substream)
>> +{
>> +       da7219_clk_disable();
>> +}
>> +
>> +static int cz_dmic_startup(struct snd_pcm_substream *substream)
>> +{
>> +       return da7219_clk_enable(substream);
>> +}
>> +
>> +static void cz_dmic_shutdown(struct snd_pcm_substream *substream)
>> +{
>> +       da7219_clk_disable();
>>    }
> 
> This is ok, or you could combine the common cz_max_* & cz_dmic_*.
> 
> 
Need to be separate as they have different cpu dai.

>>    static struct snd_soc_ops cz_da7219_cap_ops = {
> 
> I think these should all be "static const struct snd_soc_ops"  (please fix
> in a separate patch).
> 

Accepted. Will post another patch for same.

>> -       .hw_params = cz_da7219_hw_params,
>> -       .hw_free = cz_da7219_hw_free,
>> -       .startup = cz_fe_startup,
>> +       .startup = cz_da7219_startup,
>> +       .shutdown = cz_da7219_shutdown,
>>    };
> 
>>    static struct snd_soc_ops cz_max_play_ops = {
>> -       .hw_params = cz_da7219_hw_params,
>> -       .hw_free = cz_da7219_hw_free,
>> +       .startup = cz_max_startup,
>> +       .shutdown = cz_max_shutdown,
>>    };
> 
>>    static struct snd_soc_ops cz_dmic_cap_ops = {
>> -       .hw_params = cz_da7219_hw_params,
>> -       .hw_free = cz_da7219_hw_free,
>> +       .startup = cz_dmic_startup,
>> +       .shutdown = cz_dmic_shutdown,
>>    };
> 
>>    static struct snd_soc_dai_link cz_dai_7219_98357[] = {
>> --
>> 2.7.4

Thanks,
Akshu

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-04-25  3:37 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-24  3:04 [PATCH 1/3] ASoC: amd: acp dma driver code cleanup Vijendar Mukunda
2018-04-24  3:04 ` Vijendar Mukunda
2018-04-24  3:04 ` [PATCH 2/3] ASoC: AMD: Move clk enable from hw_params/free to startup/shutdown Vijendar Mukunda
2018-04-24  3:04   ` Vijendar Mukunda
2018-04-24 16:36   ` Daniel Kurtz
2018-04-25  3:36     ` Agrawal, Akshu
2018-04-24  3:04 ` [PATCH v3 3/3] ASoC: amd: dma driver changes for bt i2s instance Vijendar Mukunda
2018-04-24  3:04   ` Vijendar Mukunda
2018-04-24  6:05 ` [PATCH 1/3] ASoC: amd: acp dma driver code cleanup Daniel Kurtz
2018-04-24  7:06   ` Mukunda,Vijendar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.