[alsa-devel] [PATCH v2 0/4] ASoC: davinci: edma dmaengine PCM and mcasp preparation
Hi,
This is a cutdown version of the previous [1] series containing only the ASoC part of the edma dmaengine conversion, but without the actual switch to this new edma-pcm platform for the McASP since the edma arch and dmaengine core changes are needed for it to be usable.
Changes since v1: - arch and dmaengine patches has been dropped from the ASoC series - Patches has been reordered so only one patch is going to be needed to switch AM335x/AM447x to use the edma-pcm platform driver - all comments from Lars-Peter Clausen has been addressed for the edma-pcm driver - no changes to Kconfig/Makefile - do not yet build the edma-pcm driver. It will be enabled and integrated when we can do the real switch.
[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2014-March/074269.html
Regards, Peter --- Peter Ujfalusi (4): ASoC: davinci: Add edma dmaengine platform driver ASoC: davinci-mcasp: Provide correct filter_data for dmaengine for non-DT boot ASoC: davinci-mcasp: Constraint on the period and buffer size based on FIFO usage ASoC: davinci-mcasp: Assign the dma_data earlier in dai_probe callback
sound/soc/davinci/davinci-mcasp.c | 59 +++++++++++++++++++++++++++++++++------ sound/soc/davinci/edma-pcm.c | 57 +++++++++++++++++++++++++++++++++++++ sound/soc/davinci/edma-pcm.h | 25 +++++++++++++++++ 3 files changed, 132 insertions(+), 9 deletions(-) create mode 100644 sound/soc/davinci/edma-pcm.c create mode 100644 sound/soc/davinci/edma-pcm.h
Platform driver glue for SoC using eDMA3 to use dmaengine PCM. The maximum number of periods need to be limited to 19 since the edma dmaengine driver limits the paRAM slot use for audio at in cyclic mode.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/davinci/edma-pcm.c | 57 ++++++++++++++++++++++++++++++++++++++++++++ sound/soc/davinci/edma-pcm.h | 25 +++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 sound/soc/davinci/edma-pcm.c create mode 100644 sound/soc/davinci/edma-pcm.h
diff --git a/sound/soc/davinci/edma-pcm.c b/sound/soc/davinci/edma-pcm.c new file mode 100644 index 000000000000..d38afb1c61ae --- /dev/null +++ b/sound/soc/davinci/edma-pcm.c @@ -0,0 +1,57 @@ +/* + * edma-pcm.c - eDMA PCM driver using dmaengine for AM3xxx, AM4xxx + * + * Copyright (C) 2014 Texas Instruments, Inc. + * + * Author: Peter Ujfalusi peter.ujfalusi@ti.com + * + * Based on: sound/soc/tegra/tegra_pcm.c + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + */ + +#include <linux/module.h> +#include <sound/core.h> +#include <sound/pcm.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> +#include <sound/dmaengine_pcm.h> +#include <linux/edma.h> + +static const struct snd_pcm_hardware edma_pcm_hardware = { + .info = SNDRV_PCM_INFO_MMAP | + SNDRV_PCM_INFO_MMAP_VALID | + SNDRV_PCM_INFO_BATCH | + SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME | + SNDRV_PCM_INFO_INTERLEAVED, + .buffer_bytes_max = 128 * 1024, + .period_bytes_min = 32, + .period_bytes_max = 64 * 1024, + .periods_min = 2, + .periods_max = 19, /* Limit by edma dmaengine driver */ +}; + +static const struct snd_dmaengine_pcm_config edma_dmaengine_pcm_config = { + .pcm_hardware = &edma_pcm_hardware, + .prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config, + .compat_filter_fn = edma_filter_fn, + .prealloc_buffer_size = 128 * 1024, +}; + +int edma_pcm_platform_register(struct device *dev) +{ + return devm_snd_dmaengine_pcm_register(dev, &edma_dmaengine_pcm_config, + SND_DMAENGINE_PCM_FLAG_COMPAT); +} +EXPORT_SYMBOL_GPL(edma_pcm_platform_register); + +MODULE_AUTHOR("Peter Ujfalusi peter.ujfalusi@ti.com"); +MODULE_DESCRIPTION("eDMA PCM ASoC platform driver"); +MODULE_LICENSE("GPL"); diff --git a/sound/soc/davinci/edma-pcm.h b/sound/soc/davinci/edma-pcm.h new file mode 100644 index 000000000000..894c378c0f74 --- /dev/null +++ b/sound/soc/davinci/edma-pcm.h @@ -0,0 +1,25 @@ +/* + * edma-pcm.h - eDMA PCM driver using dmaengine for AM3xxx, AM4xxx + * + * Copyright (C) 2014 Texas Instruments, Inc. + * + * Author: Peter Ujfalusi peter.ujfalusi@ti.com + * + * Based on: sound/soc/tegra/tegra_pcm.h + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + */ + +#ifndef __EDMA_PCM_H__ +#define __EDMA_PCM_H__ + +int edma_pcm_platform_register(struct device *dev); + +#endif /* __EDMA_PCM_H__ */
On 03/14/2014 03:42 PM, Peter Ujfalusi wrote:
Platform driver glue for SoC using eDMA3 to use dmaengine PCM. The maximum number of periods need to be limited to 19 since the edma dmaengine driver limits the paRAM slot use for audio at in cyclic mode.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
Looks good.
Reviewed-by: Lars-Peter Clausen lars@metafoo.de
sound/soc/davinci/edma-pcm.c | 57 ++++++++++++++++++++++++++++++++++++++++++++ sound/soc/davinci/edma-pcm.h | 25 +++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 sound/soc/davinci/edma-pcm.c create mode 100644 sound/soc/davinci/edma-pcm.h
diff --git a/sound/soc/davinci/edma-pcm.c b/sound/soc/davinci/edma-pcm.c new file mode 100644 index 000000000000..d38afb1c61ae --- /dev/null +++ b/sound/soc/davinci/edma-pcm.c @@ -0,0 +1,57 @@ +/*
- edma-pcm.c - eDMA PCM driver using dmaengine for AM3xxx, AM4xxx
- Copyright (C) 2014 Texas Instruments, Inc.
- Author: Peter Ujfalusi peter.ujfalusi@ti.com
- Based on: sound/soc/tegra/tegra_pcm.c
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License
- version 2 as published by the Free Software Foundation.
- This program is distributed in the hope that it will be useful, but
- WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- General Public License for more details.
- */
+#include <linux/module.h> +#include <sound/core.h> +#include <sound/pcm.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> +#include <sound/dmaengine_pcm.h> +#include <linux/edma.h>
+static const struct snd_pcm_hardware edma_pcm_hardware = {
- .info = SNDRV_PCM_INFO_MMAP |
SNDRV_PCM_INFO_MMAP_VALID |SNDRV_PCM_INFO_BATCH |SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME |SNDRV_PCM_INFO_INTERLEAVED,- .buffer_bytes_max = 128 * 1024,
- .period_bytes_min = 32,
- .period_bytes_max = 64 * 1024,
- .periods_min = 2,
- .periods_max = 19, /* Limit by edma dmaengine driver */
+};
+static const struct snd_dmaengine_pcm_config edma_dmaengine_pcm_config = {
- .pcm_hardware = &edma_pcm_hardware,
- .prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config,
- .compat_filter_fn = edma_filter_fn,
- .prealloc_buffer_size = 128 * 1024,
+};
+int edma_pcm_platform_register(struct device *dev) +{
- return devm_snd_dmaengine_pcm_register(dev, &edma_dmaengine_pcm_config,
SND_DMAENGINE_PCM_FLAG_COMPAT);+} +EXPORT_SYMBOL_GPL(edma_pcm_platform_register);
+MODULE_AUTHOR("Peter Ujfalusi peter.ujfalusi@ti.com"); +MODULE_DESCRIPTION("eDMA PCM ASoC platform driver"); +MODULE_LICENSE("GPL"); diff --git a/sound/soc/davinci/edma-pcm.h b/sound/soc/davinci/edma-pcm.h new file mode 100644 index 000000000000..894c378c0f74 --- /dev/null +++ b/sound/soc/davinci/edma-pcm.h @@ -0,0 +1,25 @@ +/*
- edma-pcm.h - eDMA PCM driver using dmaengine for AM3xxx, AM4xxx
- Copyright (C) 2014 Texas Instruments, Inc.
- Author: Peter Ujfalusi peter.ujfalusi@ti.com
- Based on: sound/soc/tegra/tegra_pcm.h
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License
- version 2 as published by the Free Software Foundation.
- This program is distributed in the hope that it will be useful, but
- WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- General Public License for more details.
- */
+#ifndef __EDMA_PCM_H__ +#define __EDMA_PCM_H__
+int edma_pcm_platform_register(struct device *dev);
+#endif /* __EDMA_PCM_H__ */
On Fri, Mar 14, 2014 at 04:42:45PM +0200, Peter Ujfalusi wrote:
Platform driver glue for SoC using eDMA3 to use dmaengine PCM. The maximum number of periods need to be limited to 19 since the edma dmaengine driver limits the paRAM slot use for audio at in cyclic mode.
Applied, thanks.
When we boot with non-DT mode the damengine will need the channel number and a filter function in order to get the channel. The filter_data is filled in the DAI driver while the filter_function will be provided by the edma-pcm driver.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/davinci/davinci-mcasp.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index b0ae0677f023..a01ae97c90aa 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -1026,6 +1026,7 @@ nodata: static int davinci_mcasp_probe(struct platform_device *pdev) { struct davinci_pcm_dma_params *dma_params; + struct snd_dmaengine_dai_dma_data *dma_data; struct resource *mem, *ioarea, *res, *dat; struct davinci_mcasp_pdata *pdata; struct davinci_mcasp *mcasp; @@ -1095,6 +1096,7 @@ static int davinci_mcasp_probe(struct platform_device *pdev) mcasp->dat_port = true;
dma_params = &mcasp->dma_params[SNDRV_PCM_STREAM_PLAYBACK]; + dma_data = &mcasp->dma_data[SNDRV_PCM_STREAM_PLAYBACK]; dma_params->asp_chan_q = pdata->asp_chan_q; dma_params->ram_chan_q = pdata->ram_chan_q; dma_params->sram_pool = pdata->sram_pool; @@ -1105,7 +1107,7 @@ static int davinci_mcasp_probe(struct platform_device *pdev) dma_params->dma_addr = mem->start + pdata->tx_dma_offset;
/* Unconditional dmaengine stuff */ - mcasp->dma_data[SNDRV_PCM_STREAM_PLAYBACK].addr = dma_params->dma_addr; + dma_data->addr = dma_params->dma_addr;
res = platform_get_resource(pdev, IORESOURCE_DMA, 0); if (res) @@ -1113,7 +1115,14 @@ static int davinci_mcasp_probe(struct platform_device *pdev) else dma_params->channel = pdata->tx_dma_channel;
+ /* dmaengine filter data for DT and non-DT boot */ + if (pdev->dev.of_node) + dma_data->filter_data = "tx"; + else + dma_data->filter_data = &dma_params->channel; + dma_params = &mcasp->dma_params[SNDRV_PCM_STREAM_CAPTURE]; + dma_data = &mcasp->dma_data[SNDRV_PCM_STREAM_CAPTURE]; dma_params->asp_chan_q = pdata->asp_chan_q; dma_params->ram_chan_q = pdata->ram_chan_q; dma_params->sram_pool = pdata->sram_pool; @@ -1124,7 +1133,7 @@ static int davinci_mcasp_probe(struct platform_device *pdev) dma_params->dma_addr = mem->start + pdata->rx_dma_offset;
/* Unconditional dmaengine stuff */ - mcasp->dma_data[SNDRV_PCM_STREAM_CAPTURE].addr = dma_params->dma_addr; + dma_data->addr = dma_params->dma_addr;
if (mcasp->version < MCASP_VERSION_3) { mcasp->fifo_base = DAVINCI_MCASP_V2_AFIFO_BASE; @@ -1140,9 +1149,11 @@ static int davinci_mcasp_probe(struct platform_device *pdev) else dma_params->channel = pdata->rx_dma_channel;
- /* Unconditional dmaengine stuff */ - mcasp->dma_data[SNDRV_PCM_STREAM_PLAYBACK].filter_data = "tx"; - mcasp->dma_data[SNDRV_PCM_STREAM_CAPTURE].filter_data = "rx"; + /* dmaengine filter data for DT and non-DT boot */ + if (pdev->dev.of_node) + dma_data->filter_data = "rx"; + else + dma_data->filter_data = &dma_params->channel;
dev_set_drvdata(&pdev->dev, mcasp);
On Fri, Mar 14, 2014 at 04:42:46PM +0200, Peter Ujfalusi wrote:
When we boot with non-DT mode the damengine will need the channel number and a filter function in order to get the channel. The filter_data is filled in the DAI driver while the filter_function will be provided by the edma-pcm driver.
Applied, thanks.
We need to place constraint on the period and buffer size if the read or write AFIFO is enabled and it is configured for more than one word otherwise the DMA will fail in buffer configuration where the sizes are not aligned with the requested FIFO configuration.
Some application (like mplayer) needs the constraint placed on the buffer size as well. If only period size is constrained they might fail to figure out the allowed buffer configuration.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/davinci/davinci-mcasp.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index a01ae97c90aa..3ca6e8c4568a 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -720,6 +720,7 @@ 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); + int afifo_numevt;
if (mcasp->version == MCASP_VERSION_4) snd_soc_dai_set_dma_data(dai, substream, @@ -727,6 +728,21 @@ static int davinci_mcasp_startup(struct snd_pcm_substream *substream, else snd_soc_dai_set_dma_data(dai, substream, mcasp->dma_params);
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) + afifo_numevt = mcasp->txnumevt; + else + afifo_numevt = mcasp->rxnumevt; + + if (afifo_numevt > 1) { + snd_pcm_hw_constraint_step(substream->runtime, 0, + SNDRV_PCM_HW_PARAM_PERIOD_SIZE, + afifo_numevt); + + snd_pcm_hw_constraint_step(substream->runtime, 0, + SNDRV_PCM_HW_PARAM_BUFFER_SIZE, + afifo_numevt); + } + return 0; }
On 03/14/2014 03:42 PM, Peter Ujfalusi wrote:
We need to place constraint on the period and buffer size if the read or write AFIFO is enabled and it is configured for more than one word otherwise the DMA will fail in buffer configuration where the sizes are not aligned with the requested FIFO configuration.
Some application (like mplayer) needs the constraint placed on the buffer size as well. If only period size is constrained they might fail to figure out the allowed buffer configuration.
Hm... the sound like there is still a bug somewhere. We constrain the buffer size to be a multiple of the period size. If the period size is constraint to be a multiple of a constant then the buffer size should automatically be constrained to be a multiple of constant * period count. And just constraining the buffer size to be a multiple of the burst size still allows buffer sizes that are not a multiple of burst size * period count.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
sound/soc/davinci/davinci-mcasp.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index a01ae97c90aa..3ca6e8c4568a 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -720,6 +720,7 @@ 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);
int afifo_numevt;
if (mcasp->version == MCASP_VERSION_4) snd_soc_dai_set_dma_data(dai, substream,
@@ -727,6 +728,21 @@ static int davinci_mcasp_startup(struct snd_pcm_substream *substream, else snd_soc_dai_set_dma_data(dai, substream, mcasp->dma_params);
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
afifo_numevt = mcasp->txnumevt;- else
afifo_numevt = mcasp->rxnumevt;
Shouldn't this use the same calculation that's used to set dma_data->maxburst? Also we should add this to the dmaengine PCM driver, since there will most likely be more DMA controllers with this restriction, doesn't necessarily need to be done in this patch series though.
- if (afifo_numevt > 1) {
snd_pcm_hw_constraint_step(substream->runtime, 0,SNDRV_PCM_HW_PARAM_PERIOD_SIZE,afifo_numevt);snd_pcm_hw_constraint_step(substream->runtime, 0,SNDRV_PCM_HW_PARAM_BUFFER_SIZE,afifo_numevt);- }
- return 0; }
On 03/16/2014 01:18 PM, Lars-Peter Clausen wrote:
On 03/14/2014 03:42 PM, Peter Ujfalusi wrote:
We need to place constraint on the period and buffer size if the read or write AFIFO is enabled and it is configured for more than one word otherwise the DMA will fail in buffer configuration where the sizes are not aligned with the requested FIFO configuration.
Some application (like mplayer) needs the constraint placed on the buffer size as well. If only period size is constrained they might fail to figure out the allowed buffer configuration.
Hm... the sound like there is still a bug somewhere. We constrain the buffer size to be a multiple of the period size. If the period size is constraint to be a multiple of a constant then the buffer size should automatically be constrained to be a multiple of constant * period count. And just constraining the buffer size to be a multiple of the burst size still allows buffer sizes that are not a multiple of burst size * period count.
Yeah, aplay, PA is fine. mplayer however does things in a different manner. I use mplayer mainly to test PA (with -ao pulse) but sometimes I use it through ALSA and this is where I noticed the issue.
If I constrain only period_size mplayer will fail to set hw_params. If I only constrain the buffer_size than we can have period_size which does not allign with the FIFO settings.
I also tried to have snd_pcm_hw_rule_add() and using the snd_interval_step() - which need to be exported but mplayer fails with that also. It fails even if I add the rule for period and buffer size.
Also the constraint as it is currently is not optimal since it does not take into account the number of channels as you pointed out.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
sound/soc/davinci/davinci-mcasp.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index a01ae97c90aa..3ca6e8c4568a 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -720,6 +720,7 @@ 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);
int afifo_numevt;
if (mcasp->version == MCASP_VERSION_4) snd_soc_dai_set_dma_data(dai, substream,
@@ -727,6 +728,21 @@ static int davinci_mcasp_startup(struct snd_pcm_substream *substream, else snd_soc_dai_set_dma_data(dai, substream, mcasp->dma_params);
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
afifo_numevt = mcasp->txnumevt;- else
afifo_numevt = mcasp->rxnumevt;Shouldn't this use the same calculation that's used to set dma_data->maxburst? Also we should add this to the dmaengine PCM driver, since there will most likely be more DMA controllers with this restriction, doesn't necessarily need to be done in this patch series though.
At startup time we do not know the number of channels going to be used. I'm planning to improve this constraint handling in a coming series. Using the afifo_numevt as step is not optimal since in case of stereo stream the step should be (afifo_numevt / 2). afifo_numevt is the sample count and not the frame count.
- if (afifo_numevt > 1) {
snd_pcm_hw_constraint_step(substream->runtime, 0,SNDRV_PCM_HW_PARAM_PERIOD_SIZE,afifo_numevt);snd_pcm_hw_constraint_step(substream->runtime, 0,SNDRV_PCM_HW_PARAM_BUFFER_SIZE,afifo_numevt);- }
return 0;
On Mon, Mar 17, 2014 at 03:28:49PM +0200, Peter Ujfalusi wrote:
On 03/16/2014 01:18 PM, Lars-Peter Clausen wrote:
Hm... the sound like there is still a bug somewhere. We constrain the buffer size to be a multiple of the period size. If the period size is constraint to be a multiple of a constant then the buffer size should automatically be constrained to be a multiple of constant * period count. And just constraining the buffer size to be a multiple of the burst size still allows buffer sizes that are not a multiple of burst size * period count.
Yeah, aplay, PA is fine. mplayer however does things in a different manner. I use mplayer mainly to test PA (with -ao pulse) but sometimes I use it through ALSA and this is where I noticed the issue.
If I constrain only period_size mplayer will fail to set hw_params. If I only constrain the buffer_size than we can have period_size which does not allign with the FIFO settings.
I also tried to have snd_pcm_hw_rule_add() and using the snd_interval_step() - which need to be exported but mplayer fails with that also. It fails even if I add the rule for period and buffer size.
Also the constraint as it is currently is not optimal since it does not take into account the number of channels as you pointed out.
This is all sounding like the thing that needs to be looked at here is mplayer so we understand what's going wrong with regard to the buffer sizes. It's sounding like if we should be doing it this is a general thing which we should be constraining presumably it'd apply to all drivers, not just this one, and so shouldn't be being fixed in the driver but it's not obvious to me why the period constraint isn't sufficient.
On 03/17/2014 06:52 PM, Mark Brown wrote:
This is all sounding like the thing that needs to be looked at here is mplayer so we understand what's going wrong with regard to the buffer sizes.
The error which was printed by mplayer was the snd_pcm_hw_params() failure. Prior to this call it calls number of snd_pcm_hw_params_set_* including: snd_pcm_hw_params_set_buffer_time_near() snd_pcm_hw_params_set_periods_near()
all without error.
So I recompiled alsa-lib with debug and compiled mplayer on the board as well. I only kept the constraint for the period size from this patch (no constraint on buffer size).
The compiled and original mplayer binary is working fine, no errors :o Then I recompiled alsa-lib without debug (as it was before): same thing, moth mplayer binary works fine and it picks correct buffer configuration.
I'll resend the last two patch and place the constraint only to the period size.
It's sounding like if we should be doing it this is a general thing which we should be constraining presumably it'd apply to all drivers, not just this one, and so shouldn't be being fixed in the driver but it's not obvious to me why the period constraint isn't sufficient.
It can only be done if we have fixed FIFO for the dai. Right now this is kind of true for McASP. In HW we actually have 64 32bit word FIFO and currently you can set the desired FIFO depth via DT/pdata. I'm not really happy about this since it creates this constraint on the buffer allocation when the SoC is using eDMA (when sDMA is in used with McASP we do not have such issue). McBSP also have FIFO on OMAPs, but there I do not lock the FIFO depth, it is adaptive based on the period/buffer size.
If we have more device with fixed FIFO depth then it make sense to handle the constraint in the core.
However in case of McASP it is still not that straight forward. The DMA burst size depends on the number of channels, number of used serializers and the AFIFO's depth configuration. At the end the period size need to be constraint to be steps of DMA burst size for eDMA.
Not sure if this is applicable for other SoCs.
On Tue, Mar 18, 2014 at 02:35:48PM +0200, Peter Ujfalusi wrote:
On 03/17/2014 06:52 PM, Mark Brown wrote:
It's sounding like if we should be doing it this is a general thing which we should be constraining presumably it'd apply to all drivers, not just this one, and so shouldn't be being fixed in the driver but it's not obvious to me why the period constraint isn't sufficient.
It can only be done if we have fixed FIFO for the dai. Right now this is kind of true for McASP. In HW we actually have 64 32bit word FIFO and currently you can set the desired FIFO depth via DT/pdata. I'm not really happy about this
Sorry, what I meant was more the bit where you were setting the constraint on both the buffer and the period than the constraint itself - that seemed somehow redundant.
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 | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c index 3ca6e8c4568a..a846288371bf 100644 --- a/sound/soc/davinci/davinci-mcasp.c +++ b/sound/soc/davinci/davinci-mcasp.c @@ -722,12 +722,6 @@ static int davinci_mcasp_startup(struct snd_pcm_substream *substream, struct davinci_mcasp *mcasp = snd_soc_dai_get_drvdata(dai); int afifo_numevt;
- 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); - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) afifo_numevt = mcasp->txnumevt; else @@ -755,6 +749,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) { @@ -808,6 +821,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 = {
On Fri, Mar 14, 2014 at 04:42:48PM +0200, Peter Ujfalusi wrote:
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.
This is OK but depends on patch 3.
participants (3)
-
Lars-Peter Clausen -
Mark Brown -
Peter Ujfalusi