diff mbox

[1/7] ASoC: amd: dma driver changes for BT I2S controller instance

Message ID 1520836460-21809-2-git-send-email-Vijendar.Mukunda@amd.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vijendar Mukunda March 12, 2018, 6:34 a.m. UTC 
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.

BT I2S is bi-directional dai and same is used for playback and capture.
Added condition checks to differentiate I2S instance based on cpu dai name.

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

Comments

Mark Brown March 12, 2018, 6:25 p.m. UTC | #1 
On Mon, Mar 12, 2018 at 12:04:14PM +0530, Vijendar Mukunda wrote:

> +		 * 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/capture device.

I'm still not thrilled about this.  If we're hard coding names they
should be explicitly assigned names so that there's less chance that
some other change in the subsystem will change things.

Please make some effort to keep the code within 80 columns too.
Vijendar Mukunda March 14, 2018, 10:36 a.m. UTC | #2 
On Monday 12 March 2018 11:55 PM, Mark Brown wrote:
> On Mon, Mar 12, 2018 at 12:04:14PM +0530, Vijendar Mukunda wrote:
>
>> +		 * 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/capture device.
> I'm still not thrilled about this.  If we're hard coding names they
> should be explicitly assigned names so that there's less chance that
> some other change in the subsystem will change things.
AMD Gpu driver creates devices for Playback and capture devices for both the I2S Controller instances using MFD framework.
Designware driver probe call gets invoked for every device creation with resource information and platform data provided by GPU driver.
In runtime , it seems exporting the device resource name from dwc driver to acp dma driver is not possible.
We understand your concern, but we have made the all necessary checks to avoid any fall-backs due to this.
Our design makes sure that the same naming convention is followed in machine driver too and there also we mention cpu dai name in the same way.
We explored other options but this seems to be the best which makes acp dma driver compatible with multiple platforms.
>
> Please make some effort to keep the code within 80 columns too.
We will fix it and share updated patch.
Mark Brown March 14, 2018, 4:24 p.m. UTC | #3 
On Wed, Mar 14, 2018 at 04:06:15PM +0530, Mukunda,Vijendar wrote:

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns.  Doing this makes your messages much
easier to read and reply to.

> On Monday 12 March 2018 11:55 PM, Mark Brown wrote:

> > I'm still not thrilled about this.  If we're hard coding names they
> > should be explicitly assigned names so that there's less chance that
> > some other change in the subsystem will change things.

> AMD Gpu driver creates devices for Playback and capture devices for both the I2S Controller instances using MFD framework.
> Designware driver probe call gets invoked for every device creation with resource information and platform data provided by GPU driver.
> In runtime , it seems exporting the device resource name from dwc driver to acp dma driver is not possible.

This is not the case, if you are instantiating a platform device you can
pass platform data to it when you do so by initializing the dev.platform_data 
field.
Vijendar Mukunda March 16, 2018, 1:16 p.m. UTC | #4 
On Wednesday 14 March 2018 09:54 PM, Mark Brown wrote:
> On Wed, Mar 14, 2018 at 04:06:15PM +0530, Mukunda,Vijendar wrote:
>
> Please fix your mail client to word wrap within paragraphs at something
> substantially less than 80 columns.  Doing this makes your messages much
> easier to read and reply to.
>
>> On Monday 12 March 2018 11:55 PM, Mark Brown wrote:
>>> I'm still not thrilled about this.  If we're hard coding names they
>>> should be explicitly assigned names so that there's less chance that
>>> some other change in the subsystem will change things.
>> AMD Gpu driver creates devices for Playback and capture devices for both the I2S Controller instances using MFD framework.
>> Designware driver probe call gets invoked for every device creation with resource information and platform data provided by GPU driver.
>> In runtime , it seems exporting the device resource name from dwc driver to acp dma driver is not possible.
> This is not the case, if you are instantiating a platform device you can
> pass platform data to it when you do so by initializing the dev.platform_data
> field.
We understood your point.
We will implement required code changes and post updated patch.
diff mbox

Patch

diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index 540088d..4611706 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -710,7 +710,32 @@  static int acp_dma_open(struct snd_pcm_substream *substream)
 		default:
 			runtime->hw = acp_pcm_hardware_playback;
 		}
+		/* 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/capture device.
+		 */
+		if (strcmp(prtd->cpu_dai->name, "designware-i2s.1.auto") == 0) {
+			adata->i2s_play_instance = I2S_SP_INSTANCE;
+			adata->i2ssp_renderbytescount = 0;
+		} else if (strcmp(prtd->cpu_dai->name, "designware-i2s.3.auto") == 0) {
+			adata->i2s_play_instance = I2S_BT_INSTANCE;
+			adata->i2sbt_renderbytescount = 0;
+		} else
+			return -EINVAL;
 	} else {
+		if (strcmp(prtd->cpu_dai->name, "designware-i2s.2.auto") == 0) {
+			adata->i2s_capture_instance = I2S_SP_INSTANCE;
+			adata->i2ssp_capturebytescount = 0;
+		} else if (strcmp(prtd->cpu_dai->name, "designware-i2s.3.auto") == 0) {
+			adata->i2s_capture_instance = I2S_BT_INSTANCE;
+			adata->i2sbt_capturebytescount = 0;
+		} else
+			return -EINVAL;
 		switch (intr_data->asic_type) {
 		case CHIP_STONEY:
 			runtime->hw = acp_st_pcm_hardware_capture;
@@ -736,11 +761,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.
@@ -751,7 +784,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,
@@ -1010,34 +1050,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;
 }
@@ -1089,6 +1143,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;
 
@@ -1177,6 +1233,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 ba01510..e8406b9 100644
--- a/sound/soc/amd/acp.h
+++ b/sound/soc/amd/acp.h
@@ -72,6 +72,8 @@ 
 #define mmACP_I2S_16BIT_RESOLUTION_EN       0x5209
 #define ACP_I2S_MIC_16BIT_RESOLUTION_EN 0x01
 #define ACP_I2S_SP_16BIT_RESOLUTION_EN	0x02
+#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,
@@ -88,12 +90,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;
 };