[PATCH] ASoC: mmp-pcm: config pcm slave via generic dmaengine

[PATCH] ASoC: mmp-pcm: config pcm slave via generic dmaengine

[Qiao Zhou]

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;
-- 
1.7.0.4

Re: [PATCH] ASoC: mmp-pcm: config pcm slave via generic dmaengine

[Mark Brown]

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

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?

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

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

Re: [PATCH] ASoC: mmp-pcm: config pcm slave via generic dmaengine

[Lars-Peter Clausen]

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

Re: [PATCH] ASoC: mmp-pcm: config pcm slave via generic dmaengine

[Mark Brown]

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

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.

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

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

Re: [PATCH] ASoC: mmp-pcm: config pcm slave via generic dmaengine

[Qiao Zhou]

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.

-- 

Best Regards
Qiao

Re: [PATCH] ASoC: mmp-pcm: config pcm slave via generic dmaengine

[Lars-Peter Clausen]

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;

Re: [PATCH] ASoC: mmp-pcm: config pcm slave via generic dmaengine

[Lars-Peter Clausen]

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

Re: [PATCH] ASoC: mmp-pcm: config pcm slave via generic dmaengine

[Takashi Iwai]

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;
> 

Re: [PATCH] ASoC: mmp-pcm: config pcm slave via generic dmaengine

[Mark Brown]

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

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...

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

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

Re: [PATCH] ASoC: mmp-pcm: config pcm slave via generic dmaengine

[Mark Brown]

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

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.

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

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