[alsa-devel] [PATCH] ASoC: mmp-pcm: config pcm slave via generic dmaengine
use snd_dmaengine_pcm_prepare_slave_config to set slave config, and remove the max_burst_size = 4 hard code.
select SND_SOC_GENERIC_DMAENGINE_PCM for mmp-pcm.
Signed-off-by: Qiao Zhou zhouqiao@marvell.com --- sound/soc/pxa/Kconfig | 2 +- sound/soc/pxa/mmp-pcm.c | 18 +++--------------- 2 files changed, 4 insertions(+), 16 deletions(-)
diff --git a/sound/soc/pxa/Kconfig b/sound/soc/pxa/Kconfig index 4db74a0..6473052 100644 --- a/sound/soc/pxa/Kconfig +++ b/sound/soc/pxa/Kconfig @@ -11,7 +11,7 @@ config SND_PXA2XX_SOC config SND_MMP_SOC bool "Soc Audio for Marvell MMP chips" depends on ARCH_MMP - select SND_DMAENGINE_PCM + select SND_SOC_GENERIC_DMAENGINE_PCM select SND_ARM help Say Y if you want to add support for codecs attached to diff --git a/sound/soc/pxa/mmp-pcm.c b/sound/soc/pxa/mmp-pcm.c index 7929e19..682ee52 100644 --- a/sound/soc/pxa/mmp-pcm.c +++ b/sound/soc/pxa/mmp-pcm.c @@ -67,27 +67,15 @@ static int mmp_pcm_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { struct dma_chan *chan = snd_dmaengine_pcm_get_chan(substream); - struct snd_soc_pcm_runtime *rtd = substream->private_data; - struct snd_dmaengine_dai_dma_data *dma_params; struct dma_slave_config slave_config; int ret;
- dma_params = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream); - if (!dma_params) - return 0; - - ret = snd_hwparams_to_dma_slave_config(substream, params, &slave_config); + ret = + snd_dmaengine_pcm_prepare_slave_config(substream, params, + &slave_config); if (ret) return ret;
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { - slave_config.dst_addr = dma_params->addr; - slave_config.dst_maxburst = 4; - } else { - slave_config.src_addr = dma_params->addr; - slave_config.src_maxburst = 4; - } - ret = dmaengine_slave_config(chan, &slave_config); if (ret) return ret;
On Tue, Dec 17, 2013 at 04:22:24PM +0800, Qiao Zhou wrote:
use snd_dmaengine_pcm_prepare_slave_config to set slave config, and remove the max_burst_size = 4 hard code.
select SND_SOC_GENERIC_DMAENGINE_PCM for mmp-pcm.
Applied, thanks. Can you also convert to use snd_dmaengine_pcm_register() and remove this file completely?
On 12/17/2013 01:49 PM, Mark Brown wrote:
On Tue, Dec 17, 2013 at 04:22:24PM +0800, Qiao Zhou wrote:
use snd_dmaengine_pcm_prepare_slave_config to set slave config, and remove the max_burst_size = 4 hard code.
select SND_SOC_GENERIC_DMAENGINE_PCM for mmp-pcm.
Applied, thanks. Can you also convert to use snd_dmaengine_pcm_register() and remove this file completely?
The problem here is that the driver uses sram for its audio memory and the platform has a custom function for looking up the sram pool. What we really need is a generic version of that which we can use in memalloc.c in the core.
- Lars
On Tue, Dec 17, 2013 at 03:42:52PM +0100, Lars-Peter Clausen wrote:
On 12/17/2013 01:49 PM, Mark Brown wrote:
Applied, thanks. Can you also convert to use snd_dmaengine_pcm_register() and remove this file completely?
The problem here is that the driver uses sram for its audio memory and the platform has a custom function for looking up the sram pool. What we really need is a generic version of that which we can use in memalloc.c in the core.
Ah, yes - now I remember.
On 12/17/2013 10:42 PM, Lars-Peter Clausen wrote:
On 12/17/2013 01:49 PM, Mark Brown wrote:
On Tue, Dec 17, 2013 at 04:22:24PM +0800, Qiao Zhou wrote:
use snd_dmaengine_pcm_prepare_slave_config to set slave config, and remove the max_burst_size = 4 hard code.
select SND_SOC_GENERIC_DMAENGINE_PCM for mmp-pcm.
Applied, thanks. Can you also convert to use snd_dmaengine_pcm_register() and remove this file completely?
The problem here is that the driver uses sram for its audio memory and the platform has a custom function for looking up the sram pool. What we really need is a generic version of that which we can use in memalloc.c in the core.
- Lars
I'll check the general memalloc implementation. Thanks.
On 12/18/2013 06:55 AM, Qiao Zhou wrote:
On 12/17/2013 10:42 PM, Lars-Peter Clausen wrote:
On 12/17/2013 01:49 PM, Mark Brown wrote:
On Tue, Dec 17, 2013 at 04:22:24PM +0800, Qiao Zhou wrote:
use snd_dmaengine_pcm_prepare_slave_config to set slave config, and remove the max_burst_size = 4 hard code.
select SND_SOC_GENERIC_DMAENGINE_PCM for mmp-pcm.
Applied, thanks. Can you also convert to use snd_dmaengine_pcm_register() and remove this file completely?
The problem here is that the driver uses sram for its audio memory and the platform has a custom function for looking up the sram pool. What we really need is a generic version of that which we can use in memalloc.c in the core.
- Lars
I'll check the general memalloc implementation. Thanks.
It's a bit dirty but if nobody has any strong objections we could add the following temporary hack until there is a generic method for looking up genpools. This would allow us to cut down mmp-pcm.c quite a bit by switching it over to the generic dmaengine PCM driver.
--- a/sound/core/memalloc.c +++ b/sound/core/memalloc.c @@ -178,6 +178,15 @@ static void snd_malloc_dev_iram(struct snd_dma_buffer *dmab, size_t size) if (dev->of_node) pool = of_get_named_gen_pool(dev->of_node, "iram", 0);
+#ifdef CONFIG_CPU_MMP2 + /* + * Temporary hack until we have a generic gen_pool lookup + * infrastructure. + */ + if (!pool) + pool = sram_get_gpool("asram"); +#endif + if (!pool) return;
At Wed, 18 Dec 2013 10:16:36 +0100, Lars-Peter Clausen wrote:
On 12/18/2013 06:55 AM, Qiao Zhou wrote:
On 12/17/2013 10:42 PM, Lars-Peter Clausen wrote:
On 12/17/2013 01:49 PM, Mark Brown wrote:
On Tue, Dec 17, 2013 at 04:22:24PM +0800, Qiao Zhou wrote:
use snd_dmaengine_pcm_prepare_slave_config to set slave config, and remove the max_burst_size = 4 hard code.
select SND_SOC_GENERIC_DMAENGINE_PCM for mmp-pcm.
Applied, thanks. Can you also convert to use snd_dmaengine_pcm_register() and remove this file completely?
The problem here is that the driver uses sram for its audio memory and the platform has a custom function for looking up the sram pool. What we really need is a generic version of that which we can use in memalloc.c in the core.
- Lars
I'll check the general memalloc implementation. Thanks.
It's a bit dirty but if nobody has any strong objections we could add the following temporary hack until there is a generic method for looking up genpools. This would allow us to cut down mmp-pcm.c quite a bit by switching it over to the generic dmaengine PCM driver.
SNDRV_DMA_TYPE_DEV_IRAM is specific to such machines, so I have no big objection (as long as we can remember for the later revisit :)
thanks,
Takashi
--- a/sound/core/memalloc.c +++ b/sound/core/memalloc.c @@ -178,6 +178,15 @@ static void snd_malloc_dev_iram(struct snd_dma_buffer *dmab, size_t size) if (dev->of_node) pool = of_get_named_gen_pool(dev->of_node, "iram", 0);
+#ifdef CONFIG_CPU_MMP2
- /*
* Temporary hack until we have a generic gen_pool lookup* infrastructure.*/- if (!pool)
pool = sram_get_gpool("asram");+#endif
- if (!pool) return;
On Wed, Dec 18, 2013 at 10:16:36AM +0100, Lars-Peter Clausen wrote:
+#ifdef CONFIG_CPU_MMP2
- /*
* Temporary hack until we have a generic gen_pool lookup* infrastructure.*/- if (!pool)
pool = sram_get_gpool("asram");+#endif
I worry about what this'd do on a multiplatform kernel - it seems unlikely that someone has a pool with the same name but that sounds like famous last words. Nothing in mainline does though so...
On 12/18/2013 12:03 PM, Mark Brown wrote:
On Wed, Dec 18, 2013 at 10:16:36AM +0100, Lars-Peter Clausen wrote:
+#ifdef CONFIG_CPU_MMP2
- /*
* Temporary hack until we have a generic gen_pool lookup* infrastructure.*/- if (!pool)
pool = sram_get_gpool("asram");+#endif
I worry about what this'd do on a multiplatform kernel - it seems unlikely that someone has a pool with the same name but that sounds like famous last words. Nothing in mainline does though so...
The sram_get_gpool() function is mmp2 specific. It basically iterates a list of available pools. Those pools are registered by mmp2 platform code. So on a multiplatform kernel not booting mmp2 it will always return NULL since no pools have been registered.
- Lars
On Wed, Dec 18, 2013 at 11:14:36AM +0100, Lars-Peter Clausen wrote:
On 12/18/2013 12:03 PM, Mark Brown wrote:
I worry about what this'd do on a multiplatform kernel - it seems unlikely that someone has a pool with the same name but that sounds like famous last words. Nothing in mainline does though so...
The sram_get_gpool() function is mmp2 specific. It basically iterates a list
It is? That's an unfortunate name then.
of available pools. Those pools are registered by mmp2 platform code. So on a multiplatform kernel not booting mmp2 it will always return NULL since no pools have been registered.
Yes, and like I say there's no non-MMP references to the string asram either. It seems viable to do things this way, though we need to get the generic interface sorted before too many other platforms get the same idea.
participants (4)
-
Lars-Peter Clausen -
Mark Brown -
Qiao Zhou -
Takashi Iwai