[PATCH 0/6] ASoC: davinci-mcasp: Dynamic AFIFO and DMA configuration

[PATCH 0/6] ASoC: davinci-mcasp: Dynamic AFIFO and DMA configuration

[Peter Ujfalusi]

Hi,

Apart from the first patch which is a rebased resend of a previous patch this
series will change the way McASP driver will configure the AFIFO and thus the
DMA burst.

The AFIFO and DMA configuration will be in one function so it is going to be
easier to see the code and debug if needed.

The main change is that instead of static tx/rx numevt (which is provided via DT
or pdata) we are going to switch to dynamic configuration.
With this setup we do not need to place any constraint on the period or buffer
size since the AFIFO numevt and the corresponding DMA burst size will be picked
according to period size, active serializers and the preferred numevt level.
The only case when the code can fail if the period size in words can not be
divided evenly by the active serializers, which is unlikely to happen.

Regards,
Peter
---
Peter Ujfalusi (6):
  ASoC: davinci-mcasp: Assign the dma_data earlier in dai_probe callback
  ASoC: davinci-mcasp: Fix debug typo in davinci_mcasp_hw_params()
  ASoC: davinci-mcasp: Simplify and clean up the AFIFO configuration
    code
  ASoC: davinci-mcasp: Configure the AFIFO and DMA burst size at the
    same place
  ASoC: davinic-mcasp: Adopt the AFIFO/DMA configuration to the stream
    (dynamic depth)
  ASoC: davinci-mcasp: Fine tune and correct the DMA burst configuration

 sound/soc/davinci/davinci-mcasp.c | 145 +++++++++++++++++++++++---------------
 sound/soc/davinci/davinci-mcasp.h |   1 +
 2 files changed, 89 insertions(+), 57 deletions(-)

-- 
1.9.1

[PATCH 1/6] ASoC: davinci-mcasp: Assign the dma_data earlier in dai_probe callback

[Peter Ujfalusi]

Set up the playback_dma_data/capture_dma_data for the dai at probe
time since the generic dmaengine PCM stack needs to have access to
this information early.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/davinci/davinci-mcasp.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index 01d224c4992b..5a83fac15089 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -752,22 +752,7 @@ static int davinci_mcasp_trigger(struct snd_pcm_substream *substream,
 	return ret;
 }
 
-static int davinci_mcasp_startup(struct snd_pcm_substream *substream,
-				 struct snd_soc_dai *dai)
-{
-	struct davinci_mcasp *mcasp = snd_soc_dai_get_drvdata(dai);
-
-	if (mcasp->version == MCASP_VERSION_4)
-		snd_soc_dai_set_dma_data(dai, substream,
-					&mcasp->dma_data[substream->stream]);
-	else
-		snd_soc_dai_set_dma_data(dai, substream, mcasp->dma_params);
-
-	return 0;
-}
-
 static const struct snd_soc_dai_ops davinci_mcasp_dai_ops = {
-	.startup	= davinci_mcasp_startup,
 	.trigger	= davinci_mcasp_trigger,
 	.hw_params	= davinci_mcasp_hw_params,
 	.set_fmt	= davinci_mcasp_set_dai_fmt,
@@ -775,6 +760,25 @@ static const struct snd_soc_dai_ops davinci_mcasp_dai_ops = {
 	.set_sysclk	= davinci_mcasp_set_sysclk,
 };
 
+static int davinci_mcasp_dai_probe(struct snd_soc_dai *dai)
+{
+	struct davinci_mcasp *mcasp = snd_soc_dai_get_drvdata(dai);
+
+	if (mcasp->version == MCASP_VERSION_4) {
+		/* Using dmaengine PCM */
+		dai->playback_dma_data =
+				&mcasp->dma_data[SNDRV_PCM_STREAM_PLAYBACK];
+		dai->capture_dma_data =
+				&mcasp->dma_data[SNDRV_PCM_STREAM_CAPTURE];
+	} else {
+		/* Using davinci-pcm */
+		dai->playback_dma_data = mcasp->dma_params;
+		dai->capture_dma_data = mcasp->dma_params;
+	}
+
+	return 0;
+}
+
 #ifdef CONFIG_PM_SLEEP
 static int davinci_mcasp_suspend(struct snd_soc_dai *dai)
 {
@@ -828,6 +832,7 @@ static int davinci_mcasp_resume(struct snd_soc_dai *dai)
 static struct snd_soc_dai_driver davinci_mcasp_dai[] = {
 	{
 		.name		= "davinci-mcasp.0",
+		.probe		= davinci_mcasp_dai_probe,
 		.suspend	= davinci_mcasp_suspend,
 		.resume		= davinci_mcasp_resume,
 		.playback	= {
@@ -847,6 +852,7 @@ static struct snd_soc_dai_driver davinci_mcasp_dai[] = {
 	},
 	{
 		.name		= "davinci-mcasp.1",
+		.probe		= davinci_mcasp_dai_probe,
 		.playback 	= {
 			.channels_min	= 1,
 			.channels_max	= 384,
-- 
1.9.1

[PATCH 2/6] ASoC: davinci-mcasp: Fix debug typo in davinci_mcasp_hw_params()

[Peter Ujfalusi]

requred -> required

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/davinci/davinci-mcasp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index 5a83fac15089..4007b1f3141b 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -656,7 +656,7 @@ static int davinci_mcasp_hw_params(struct snd_pcm_substream *substream,
 	if (mcasp->bclk_master) {
 		unsigned int bclk_freq = snd_soc_params_to_bclk(params);
 		if (mcasp->sysclk_freq % bclk_freq != 0) {
-			dev_err(mcasp->dev, "Can't produce requred BCLK\n");
+			dev_err(mcasp->dev, "Can't produce required BCLK\n");
 			return -EINVAL;
 		}
 		davinci_mcasp_set_clkdiv(
-- 
1.9.1

[PATCH 3/6] ASoC: davinci-mcasp: Simplify and clean up the AFIFO configuration code

[Peter Ujfalusi]

We can have more linear code flow by using variables in
mcasp_common_hw_param() related to the AFIFO configuration.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/davinci/davinci-mcasp.c | 48 +++++++++++++++++++--------------------
 sound/soc/davinci/davinci-mcasp.h |  1 +
 2 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index 4007b1f3141b..9620d0165133 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -38,6 +38,8 @@
 #include "davinci-pcm.h"
 #include "davinci-mcasp.h"
 
+#define MCASP_MAX_AFIFO_DEPTH	64
+
 struct davinci_mcasp_context {
 	u32	txfmtctl;
 	u32	rxfmtctl;
@@ -461,9 +463,9 @@ static int mcasp_common_hw_param(struct davinci_mcasp *mcasp, int stream,
 	int i;
 	u8 tx_ser = 0;
 	u8 rx_ser = 0;
-	u8 ser;
 	u8 slots = mcasp->tdm_slots;
 	u8 max_active_serializers = (channels + slots - 1) / slots;
+	u8 active_serializers, numevt;
 	u32 reg;
 	/* Default configuration */
 	if (mcasp->version != MCASP_VERSION_4)
@@ -497,36 +499,34 @@ static int mcasp_common_hw_param(struct davinci_mcasp *mcasp, int stream,
 		}
 	}
 
-	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
-		ser = tx_ser;
-	else
-		ser = rx_ser;
+	if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		active_serializers = tx_ser;
+		numevt = mcasp->txnumevt;
+		reg = mcasp->fifo_base + MCASP_WFIFOCTL_OFFSET;
+	} else {
+		active_serializers = rx_ser;
+		numevt = mcasp->rxnumevt;
+		reg = mcasp->fifo_base + MCASP_RFIFOCTL_OFFSET;
+	}
 
-	if (ser < max_active_serializers) {
+	if (active_serializers < max_active_serializers) {
 		dev_warn(mcasp->dev, "stream has more channels (%d) than are "
-			"enabled in mcasp (%d)\n", channels, ser * slots);
+			 "enabled in mcasp (%d)\n", channels,
+			 active_serializers * slots);
 		return -EINVAL;
 	}
 
-	if (mcasp->txnumevt && stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		if (mcasp->txnumevt * tx_ser > 64)
-			mcasp->txnumevt = 1;
-
-		reg = mcasp->fifo_base + MCASP_WFIFOCTL_OFFSET;
-		mcasp_mod_bits(mcasp, reg, tx_ser, NUMDMA_MASK);
-		mcasp_mod_bits(mcasp, reg, ((mcasp->txnumevt * tx_ser) << 8),
-			       NUMEVT_MASK);
-	}
+	/* AFIFO is not in use */
+	if (!numevt)
+		return 0;
 
-	if (mcasp->rxnumevt && stream == SNDRV_PCM_STREAM_CAPTURE) {
-		if (mcasp->rxnumevt * rx_ser > 64)
-			mcasp->rxnumevt = 1;
+	if (numevt * active_serializers > MCASP_MAX_AFIFO_DEPTH)
+		numevt = active_serializers;
 
-		reg = mcasp->fifo_base + MCASP_RFIFOCTL_OFFSET;
-		mcasp_mod_bits(mcasp, reg, rx_ser, NUMDMA_MASK);
-		mcasp_mod_bits(mcasp, reg, ((mcasp->rxnumevt * rx_ser) << 8),
-			       NUMEVT_MASK);
-	}
+	/* Configure the AFIFO */
+	numevt *= active_serializers;
+	mcasp_mod_bits(mcasp, reg, active_serializers, NUMDMA_MASK);
+	mcasp_mod_bits(mcasp, reg, NUMEVT(numevt), NUMEVT_MASK);
 
 	return 0;
 }
diff --git a/sound/soc/davinci/davinci-mcasp.h b/sound/soc/davinci/davinci-mcasp.h
index 53243c22818d..9737108f0305 100644
--- a/sound/soc/davinci/davinci-mcasp.h
+++ b/sound/soc/davinci/davinci-mcasp.h
@@ -289,6 +289,7 @@
  */
 #define FIFO_ENABLE	BIT(16)
 #define NUMEVT_MASK	(0xFF << 8)
+#define NUMEVT(x)	(((x) & 0xFF) << 8)
 #define NUMDMA_MASK	(0xFF)
 
 #endif	/* DAVINCI_MCASP_H */
-- 
1.9.1

[PATCH 4/6] ASoC: davinci-mcasp: Configure the AFIFO and DMA burst size at the same place

[Peter Ujfalusi]

Move the dma_params->fifo_level and dma_data->maxburst configuration to the
mcasp_common_hw_param() function where we configure the AFIFO registers.
It makes the code regarding to AFIFO and DMA configuration more easy to
follow since it is now clear how the AFIFO and how the DMA is going to be
configured.
Previously this has been done in two functions using a bit different
calculation form - which ended up with the same result in both case at the
end, but it was confusing.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/davinci/davinci-mcasp.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index 9620d0165133..dd6910b289dd 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -460,6 +460,8 @@ static int davinci_config_channel_size(struct davinci_mcasp *mcasp,
 static int mcasp_common_hw_param(struct davinci_mcasp *mcasp, int stream,
 				    int channels)
 {
+	struct davinci_pcm_dma_params *dma_params = &mcasp->dma_params[stream];
+	struct snd_dmaengine_dai_dma_data *dma_data = &mcasp->dma_data[stream];
 	int i;
 	u8 tx_ser = 0;
 	u8 rx_ser = 0;
@@ -516,9 +518,14 @@ static int mcasp_common_hw_param(struct davinci_mcasp *mcasp, int stream,
 		return -EINVAL;
 	}
 
+
 	/* AFIFO is not in use */
-	if (!numevt)
+	if (!numevt) {
+		/* Configure the burst size for platform drivers */
+		dma_params->fifo_level = 0;
+		dma_data->maxburst = 0;
 		return 0;
+	}
 
 	if (numevt * active_serializers > MCASP_MAX_AFIFO_DEPTH)
 		numevt = active_serializers;
@@ -528,6 +535,10 @@ static int mcasp_common_hw_param(struct davinci_mcasp *mcasp, int stream,
 	mcasp_mod_bits(mcasp, reg, active_serializers, NUMDMA_MASK);
 	mcasp_mod_bits(mcasp, reg, NUMEVT(numevt), NUMEVT_MASK);
 
+	/* Configure the burst size for platform drivers */
+	dma_params->fifo_level = numevt;
+	dma_data->maxburst = numevt;
+
 	return 0;
 }
 
@@ -643,12 +654,7 @@ static int davinci_mcasp_hw_params(struct snd_pcm_substream *substream,
 	struct davinci_mcasp *mcasp = snd_soc_dai_get_drvdata(cpu_dai);
 	struct davinci_pcm_dma_params *dma_params =
 					&mcasp->dma_params[substream->stream];
-	struct snd_dmaengine_dai_dma_data *dma_data =
-					&mcasp->dma_data[substream->stream];
 	int word_length;
-	u8 fifo_level;
-	u8 slots = mcasp->tdm_slots;
-	u8 active_serializers;
 	int channels = params_channels(params);
 	int ret;
 
@@ -707,21 +713,11 @@ static int davinci_mcasp_hw_params(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
-	/* Calculate FIFO level */
-	active_serializers = (channels + slots - 1) / slots;
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-		fifo_level = mcasp->txnumevt * active_serializers;
-	else
-		fifo_level = mcasp->rxnumevt * active_serializers;
-
-	if (mcasp->version == MCASP_VERSION_2 && !fifo_level)
+	if (mcasp->version == MCASP_VERSION_2 && !dma_params->fifo_level)
 		dma_params->acnt = 4;
 	else
 		dma_params->acnt = dma_params->data_type;
 
-	dma_params->fifo_level = fifo_level;
-	dma_data->maxburst = fifo_level;
-
 	davinci_config_channel_size(mcasp, word_length);
 
 	return 0;
-- 
1.9.1

[PATCH 5/6] ASoC: davinic-mcasp: Adopt the AFIFO/DMA configuration to the stream (dynamic depth)

[Peter Ujfalusi]

Configure the AFIFO numevt parameter based on the requested tx/rx_numevt,
active serializers and period size in words.
In this way McASP can adopt it's (and the DMA) configuration runtime and
can pick the most optimal setup which satisfy the parameters.
This way we do not need to place any constraint on the stream itself,
allowing application greater freedom on how they want to set up ALSA.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/davinci/davinci-mcasp.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index dd6910b289dd..9e877044a00a 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -458,7 +458,7 @@ static int davinci_config_channel_size(struct davinci_mcasp *mcasp,
 }
 
 static int mcasp_common_hw_param(struct davinci_mcasp *mcasp, int stream,
-				    int channels)
+				 int period_words, int channels)
 {
 	struct davinci_pcm_dma_params *dma_params = &mcasp->dma_params[stream];
 	struct snd_dmaengine_dai_dma_data *dma_data = &mcasp->dma_data[stream];
@@ -467,7 +467,7 @@ static int mcasp_common_hw_param(struct davinci_mcasp *mcasp, int stream,
 	u8 rx_ser = 0;
 	u8 slots = mcasp->tdm_slots;
 	u8 max_active_serializers = (channels + slots - 1) / slots;
-	u8 active_serializers, numevt;
+	int active_serializers, numevt, n;
 	u32 reg;
 	/* Default configuration */
 	if (mcasp->version != MCASP_VERSION_4)
@@ -518,7 +518,6 @@ static int mcasp_common_hw_param(struct davinci_mcasp *mcasp, int stream,
 		return -EINVAL;
 	}
 
-
 	/* AFIFO is not in use */
 	if (!numevt) {
 		/* Configure the burst size for platform drivers */
@@ -527,11 +526,26 @@ static int mcasp_common_hw_param(struct davinci_mcasp *mcasp, int stream,
 		return 0;
 	}
 
-	if (numevt * active_serializers > MCASP_MAX_AFIFO_DEPTH)
+	if (period_words % active_serializers) {
+		dev_err(mcasp->dev, "Invalid combination of period words and "
+			"active serializers: %d, %d\n", period_words,
+			active_serializers);
+		return -EINVAL;
+	}
+
+	/*
+	 * Calculate the optimal AFIFO depth for platform side:
+	 * The number of words for numevt need to be in steps of active
+	 * serializers.
+	 */
+	n = numevt % active_serializers;
+	if (n)
+		numevt += (active_serializers - n);
+	while (period_words % numevt && numevt > 0)
+		numevt -= active_serializers;
+	if (numevt <= 0)
 		numevt = active_serializers;
 
-	/* Configure the AFIFO */
-	numevt *= active_serializers;
 	mcasp_mod_bits(mcasp, reg, active_serializers, NUMDMA_MASK);
 	mcasp_mod_bits(mcasp, reg, NUMEVT(numevt), NUMEVT_MASK);
 
@@ -656,6 +670,7 @@ static int davinci_mcasp_hw_params(struct snd_pcm_substream *substream,
 					&mcasp->dma_params[substream->stream];
 	int word_length;
 	int channels = params_channels(params);
+	int period_size = params_period_size(params);
 	int ret;
 
 	/* If mcasp is BCLK master we need to set BCLK divider */
@@ -669,7 +684,8 @@ static int davinci_mcasp_hw_params(struct snd_pcm_substream *substream,
 			cpu_dai, 1, mcasp->sysclk_freq / bclk_freq);
 	}
 
-	ret = mcasp_common_hw_param(mcasp, substream->stream, channels);
+	ret = mcasp_common_hw_param(mcasp, substream->stream,
+				    period_size * channels, channels);
 	if (ret)
 		return ret;
 
-- 
1.9.1

[PATCH 6/6] ASoC: davinci-mcasp: Fine tune and correct the DMA burst configuration

[Peter Ujfalusi]

When the AFIFO is not enabled but more than one serializers are used the DMA
need to transfer number of words equal to active serializers when a DMA
request is generated.
When configuring the burst for the DMA avoid using value '1' for the burst
since it is going to enable additional logic in the DMA drivers. Burst '1'
means that the DMA should send/receive one word per DMA requests.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/davinci/davinci-mcasp.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index 9e877044a00a..52fc6bbd56bd 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -521,8 +521,19 @@ static int mcasp_common_hw_param(struct davinci_mcasp *mcasp, int stream,
 	/* AFIFO is not in use */
 	if (!numevt) {
 		/* Configure the burst size for platform drivers */
-		dma_params->fifo_level = 0;
-		dma_data->maxburst = 0;
+		if (active_serializers > 1) {
+			/*
+			 * If more than one serializers are in use we have one
+			 * DMA request to provide data for all serializers.
+			 * For example if three serializers are enabled the DMA
+			 * need to transfer three words per DMA request.
+			 */
+			dma_params->fifo_level = active_serializers;
+			dma_data->maxburst = active_serializers;
+		} else {
+			dma_params->fifo_level = 0;
+			dma_data->maxburst = 0;
+		}
 		return 0;
 	}
 
@@ -550,6 +561,8 @@ static int mcasp_common_hw_param(struct davinci_mcasp *mcasp, int stream,
 	mcasp_mod_bits(mcasp, reg, NUMEVT(numevt), NUMEVT_MASK);
 
 	/* Configure the burst size for platform drivers */
+	if (numevt == 1)
+		numevt = 0;
 	dma_params->fifo_level = numevt;
 	dma_data->maxburst = numevt;
 
-- 
1.9.1

Re: [PATCH 0/6] ASoC: davinci-mcasp: Dynamic AFIFO and DMA configuration

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 276 bytes --]

On Tue, Apr 01, 2014 at 03:55:06PM +0300, Peter Ujfalusi wrote:
> Hi,
> 
> Apart from the first patch which is a rebased resend of a previous patch this
> series will change the way McASP driver will configure the AFIFO and thus the
> DMA burst.

Applied all, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]