diff mbox

[3/9] ASoC: amd: dma driver changes for BT I2S controller instance

Message ID 1518766434-7911-4-git-send-email-Vijendar.Mukunda@amd.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vijendar Mukunda Feb. 16, 2018, 7:33 a.m. UTC 
Implemented dma driver changes to support BT I2S controller
instance.

Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
Signed-off-by: Akshu Agrawal <akshu.agrawal@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 sound/soc/amd/acp-pcm-dma.c | 103 +++++++++++++++++++++++++++++++++++---------
 sound/soc/amd/acp.h         |   8 ++++
 2 files changed, 90 insertions(+), 21 deletions(-)

Comments

Mark Brown Feb. 19, 2018, 4:45 p.m. UTC | #1 
On Fri, Feb 16, 2018 at 01:03:48PM +0530, Vijendar Mukunda wrote:

> Implemented dma driver changes to support BT I2S controller
> instance.

Some sort of description of the changes would make this a lot easier to
review.

> +		if (strcmp(prtd->cpu_dai->name, "designware-i2s.1.auto") == 0) {
> +			adata->i2s_play_instance = I2S_SP_INSTANCE;
> +			adata->i2ssp_renderbytescount = 0;
> +		}
> +		if (strcmp(prtd->cpu_dai->name, "designware-i2s.3.auto") == 0) {
> +			adata->i2s_play_instance = I2S_BT_INSTANCE;
> +			adata->i2sbt_renderbytescount = 0;
> +		}

This strcmp on what looks like an autogenerated DAI name seems a bit
fragile, especially given that we just silently accept cases where we
fail to match anything.  Why are we doing things this way rather than
at least using explicitly set names?

> +	if (adata->asic_type != CHIP_CARRIZO) {
> +		if (adata->play_i2sbt_stream &&
> +			adata->play_i2sbt_stream->runtime) {

As ever please use switch statements for quirking, it makes life easier
when more variants appear.
Vijendar Mukunda Feb. 23, 2018, 7:01 a.m. UTC | #2 
On Monday 19 February 2018 10:15 PM, Mark Brown wrote:
> On Fri, Feb 16, 2018 at 01:03:48PM +0530, Vijendar Mukunda wrote:
>
>> Implemented dma driver changes to support BT I2S controller
>> instance.
> Some sort of description of the changes would make this a lot easier to
> review.

We will update the description as mentioned below.
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.


>
>> +		if (strcmp(prtd->cpu_dai->name, "designware-i2s.1.auto") == 0) {
>> +			adata->i2s_play_instance = I2S_SP_INSTANCE;
>> +			adata->i2ssp_renderbytescount = 0;
>> +		}
>> +		if (strcmp(prtd->cpu_dai->name, "designware-i2s.3.auto") == 0) {
>> +			adata->i2s_play_instance = I2S_BT_INSTANCE;
>> +			adata->i2sbt_renderbytescount = 0;
>> +		}
> This strcmp on what looks like an autogenerated DAI name seems a bit
> fragile, especially given that we just silently accept cases where we
> fail to match anything.  Why are we doing things this way rather than
> at least using explicitly set names?

ACP DMA Driver implemented based on ACP 2.x stack .It uses Designware I2S controller.

AMD Gpu Driver creates device instances for playback & capture scenarios for both the I2S controller instances.( I2S SP & I2S BT).

It has fixed mapping as mentioned below.

designware-i2s.1.auto cpu dai will be used for I2S SP Instance playback device.
designware-i2s.2.auto cpu dai will be used for I2S SP Instance capture device.
designware-i2s.3.auto cpu dai will be used for I2S BT Instance playback device.
designware-i2s.4.auto cpu dai will be used for I2S BT Instance capture device.

We will add error checks for failure case.

>
>> +	if (adata->asic_type != CHIP_CARRIZO) {
>> +		if (adata->play_i2sbt_stream &&
>> +			adata->play_i2sbt_stream->runtime) {
> As ever please use switch statements for quirking, it makes life easier
> when more variants appear.

There are only two variants (Carrizo & Stoney) based on ACP 2.x stack.
Switch statement is not required here as this condition is specific to Carrizo only.
diff mbox

Patch

diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index 91c4775..963ffa7 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -717,7 +717,23 @@  static int acp_dma_open(struct snd_pcm_substream *substream)
 		default:
 			runtime->hw = acp_pcm_hardware_playback;
 		}
+		if (strcmp(prtd->cpu_dai->name, "designware-i2s.1.auto") == 0) {
+			adata->i2s_play_instance = I2S_SP_INSTANCE;
+			adata->i2ssp_renderbytescount = 0;
+		}
+		if (strcmp(prtd->cpu_dai->name, "designware-i2s.3.auto") == 0) {
+			adata->i2s_play_instance = I2S_BT_INSTANCE;
+			adata->i2sbt_renderbytescount = 0;
+		}
 	} else {
+		if (strcmp(prtd->cpu_dai->name, "designware-i2s.2.auto") == 0) {
+			adata->i2s_capture_instance = I2S_SP_INSTANCE;
+			adata->i2ssp_capturebytescount = 0;
+		}
+		if (strcmp(prtd->cpu_dai->name, "designware-i2s.4.auto") == 0) {
+			adata->i2s_capture_instance = I2S_BT_INSTANCE;
+			adata->i2sbt_capturebytescount = 0;
+		}
 		switch (intr_data->asic_type) {
 		case CHIP_STONEY:
 			runtime->hw = acp_st_pcm_hardware_capture;
@@ -743,11 +759,19 @@  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);
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		intr_data->play_i2ssp_stream = substream;
+		switch (adata->i2s_play_instance) {
+		case I2S_BT_INSTANCE:
+			intr_data->play_i2sbt_stream = substream;
+			break;
+		case I2S_SP_INSTANCE:
+		default:
+			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.
@@ -758,7 +782,14 @@  static int acp_dma_open(struct snd_pcm_substream *substream)
 							bank, true);
 		}
 	} else {
-		intr_data->capture_i2ssp_stream = substream;
+		switch (adata->i2s_capture_instance) {
+		case I2S_BT_INSTANCE:
+			intr_data->capture_i2sbt_stream = substream;
+			break;
+		case I2S_SP_INSTANCE:
+		default:
+			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,
@@ -1004,34 +1035,48 @@  static int acp_dma_close(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);
 
-	kfree(rtd);
-
 	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_play_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_capture_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);
+			}
 		}
 	}
 
 	/* 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);
 
 	return 0;
 }
@@ -1083,6 +1128,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;
 
@@ -1171,6 +1218,20 @@  static int acp_pcm_resume(struct device *dev)
 			adata->capture_i2ssp_stream->runtime->private_data,
 			adata->asic_type);
 	}
+	if (adata->asic_type != CHIP_CARRIZO) {
+		if (adata->play_i2sbt_stream &&
+			adata->play_i2sbt_stream->runtime) {
+			config_acp_dma(adata->acp_mmio,
+				adata->play_i2sbt_stream->runtime->private_data,
+				adata->asic_type);
+		}
+		if (adata->capture_i2sbt_stream &&
+			adata->capture_i2sbt_stream->runtime) {
+			config_acp_dma(adata->acp_mmio,
+				adata->capture_i2sbt_stream->runtime->private_data,
+				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 27803b2..366474f 100644
--- a/sound/soc/amd/acp.h
+++ b/sound/soc/amd/acp.h
@@ -70,6 +70,8 @@ 
 #define CAPTURE_END_DMA_DESCR_CH15 7
 
 #define mmACP_I2S_16BIT_RESOLUTION_EN       0x5209
+#define I2S_SP_INSTANCE 1
+#define I2S_BT_INSTANCE 3
 enum acp_dma_priority_level {
 	/* 0x0 Specifies the DMA channel is given normal priority */
 	ACP_DMA_PRIORITY_LEVEL_NORMAL = 0x0,
@@ -86,12 +88,18 @@  struct audio_substream_data {
 	uint64_t size;
 	u64 i2ssp_renderbytescount;
 	u64 i2ssp_capturebytescount;
+	u64 i2sbt_renderbytescount;
+	u64 i2sbt_capturebytescount;
 	void __iomem *acp_mmio;
+	u16 i2s_play_instance;
+	u16 i2s_capture_instance;
 };
 
 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;
 };