diff mbox

[V2] ASoC: amd: dma driver changes for BT I2S controller instance

Message ID 1522355994-32039-1-git-send-email-Vijendar.Mukunda@amd.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vijendar Mukunda March 29, 2018, 8:39 p.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.

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>
v1->v2: fixed kbuild errors
---
 sound/soc/amd/acp-pcm-dma.c | 116 +++++++++++++++++++++++++++++++++++---------
 sound/soc/amd/acp.h         |  17 +++++++
 2 files changed, 111 insertions(+), 22 deletions(-)

Comments

Mark Brown April 16, 2018, 5:22 p.m. UTC | #1 
On Fri, Mar 30, 2018 at 02:09:54AM +0530, Vijendar Mukunda wrote:

> 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>
> v1->v2: fixed kbuild errors
> ---

Please put inter-version changelogs after the --- as covered in
SubmittingPatches.

> +#ifdef CONFIG_SND_DESIGNWARE_PCM
> +		adata->i2s_capture_instance = dev->i2s_instance;
> +#else
> +		adata->i2s_capture_instance = I2S_SP_INSTANCE;
> +#endif

These ifdefs aren't great - we should be able to build a single kernel
image that works on all systems so if we've got configuration options
based on the presence of other drivers then presumably existing systems
are going to be disrupted here.  On the other hand if all systems are
able to use the Designware driver then we should just do that
unconditionally.

This will also break in cases where the Designware driver gets built as
a module.
Vijendar Mukunda April 17, 2018, 6:08 a.m. UTC | #2 
On Monday 16 April 2018 10:52 PM, Mark Brown wrote:
> On Fri, Mar 30, 2018 at 02:09:54AM +0530, Vijendar Mukunda wrote:
> 
>> 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>
>> v1->v2: fixed kbuild errors
>> ---
> 
> Please put inter-version changelogs after the --- as covered in
> SubmittingPatches.
> 
>> +#ifdef CONFIG_SND_DESIGNWARE_PCM
>> +		adata->i2s_capture_instance = dev->i2s_instance;
>> +#else
>> +		adata->i2s_capture_instance = I2S_SP_INSTANCE;
>> +#endif
> 
> These ifdefs aren't great - we should be able to build a single kernel
> image that works on all systems so if we've got configuration options
> based on the presence of other drivers then presumably existing systems
> are going to be disrupted here.  On the other hand if all systems are
> able to use the Designware driver then we should just do that
> unconditionally.
> 
> This will also break in cases where the Designware driver gets built as
> a module.
> 
I removed Ifdefs and further simplified DMA Driver design.
I had posted fresh series.
diff mbox

Patch

diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index 540088d..cad995e 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -697,6 +697,9 @@  static int acp_dma_open(struct snd_pcm_substream *substream)
 	struct snd_soc_pcm_runtime *prtd = substream->private_data;
 	struct snd_soc_component *component = snd_soc_rtdcom_lookup(prtd, DRV_NAME);
 	struct audio_drv_data *intr_data = dev_get_drvdata(component->dev);
+#ifdef CONFIG_SND_DESIGNWARE_PCM
+	struct dw_i2s_dev *dev = snd_soc_dai_get_drvdata(prtd->cpu_dai);
+#endif
 	struct audio_substream_data *adata =
 		kzalloc(sizeof(struct audio_substream_data), GFP_KERNEL);
 	if (adata == NULL)
@@ -710,7 +713,29 @@  static int acp_dma_open(struct snd_pcm_substream *substream)
 		default:
 			runtime->hw = acp_pcm_hardware_playback;
 		}
+#ifdef CONFIG_SND_DESIGNWARE_PCM
+		adata->i2s_play_instance = dev->i2s_instance;
+#else
+		adata->i2s_play_instance = I2S_SP_INSTANCE;
+#endif
+		if (adata->i2s_play_instance == I2S_SP_INSTANCE)
+			adata->i2ssp_renderbytescount = 0;
+		else if (adata->i2s_play_instance == I2S_BT_INSTANCE)
+			adata->i2sbt_renderbytescount = 0;
+		else
+			return -EINVAL;
 	} else {
+#ifdef CONFIG_SND_DESIGNWARE_PCM
+		adata->i2s_capture_instance = dev->i2s_instance;
+#else
+		adata->i2s_capture_instance = I2S_SP_INSTANCE;
+#endif
+		if (adata->i2s_capture_instance == I2S_SP_INSTANCE)
+			adata->i2ssp_capturebytescount = 0;
+		else if (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,20 @@  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 +785,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 +1051,49 @@  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 +1145,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 +1235,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..7ad165b 100644
--- a/sound/soc/amd/acp.h
+++ b/sound/soc/amd/acp.h
@@ -4,6 +4,17 @@ 
 
 #include "include/acp_2_2_d.h"
 #include "include/acp_2_2_sh_mask.h"
+#ifdef CONFIG_SND_DESIGNWARE_PCM
+#include "../dwc/local.h"
+#endif
+
+#ifndef I2S_SP_INSTANCE
+#define I2S_SP_INSTANCE 0x01
+#endif
+
+#ifndef I2S_BT_INSTANCE
+#define I2S_BT_INSTANCE 0x02
+#endif
 
 #define ACP_PAGE_SIZE_4K_ENABLE			0x02
 
@@ -88,12 +99,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;
 };