[alsa-devel] [PATCH 0/6] ASoC: davinci-mcasp: Dynamic AFIFO and DMA configuration
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
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
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
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
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
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
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
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.
participants (2)
-
Mark Brown -
Peter Ujfalusi