mmp audio support

mmp-pcm handle audio dma based on dmaengine
Support mmp and pxa910

Signed-off-by Zhangfei Gao <[email protected]>

Signed-off-by Leo Yan <[email protected]>

Signed-off-by Qiao Zhou <[email protected]>

---
 include/linux/platform_data/mmp_audio.h |   22 ++
 sound/soc/pxa/Kconfig                   |    8 +
 sound/soc/pxa/Makefile                  |    2 +
 sound/soc/pxa/mmp-pcm.c                 |  448 +++++++++++++++++++++++++++++++
 4 files changed, 480 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/platform_data/mmp_audio.h
 create mode 100644 sound/soc/pxa/mmp-pcm.c

include/linux/platform_data/mmp_audio.h new file +22 -0

new file mode 100644
index 0000000..0f25d16
@@ -0,0 +1,22 @@
+/*
+ *  MMP Platform AUDIO Management
+ *
+ *  Copyright (c) 2011 Marvell Semiconductors Inc.
+ *
+ *  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.
+ *
+ */
+
+#ifndef MMP_AUDIO_H
+#define MMP_AUDIO_H
+
+struct mmp_audio_platdata {
+	u32 period_max_capture;
+	u32 buffer_max_capture;
+	u32 period_max_playback;
+	u32 buffer_max_playback;
+};
+
+#endif /* MMP_AUDIO_H */

sound/soc/pxa/Kconfig +8 -0

index a0f7d3c..a516068 100644
@@ -8,6 +8,14 @@ config SND_PXA2XX_SOC
 	  the PXA2xx AC97, I2S or SSP interface. You will also need
 	  to select the audio interfaces to support below.
 
+config SND_MMP_SOC
+	bool "Soc Audio for Marvell MMP chips"
+	depends on ARCH_MMP
+	select SND_ARM
+	help
+	  Say Y if you want to add support for codecs attached to
+	  the MMP SSPA interface.
+
 config SND_PXA2XX_AC97
 	tristate
 	select SND_AC97_CODEC

sound/soc/pxa/Makefile +2 -0

index af35762..f913e9b 100644
@@ -3,11 +3,13 @@ snd-soc-pxa2xx-objs := pxa2xx-pcm.o
 snd-soc-pxa2xx-ac97-objs := pxa2xx-ac97.o
 snd-soc-pxa2xx-i2s-objs := pxa2xx-i2s.o
 snd-soc-pxa-ssp-objs := pxa-ssp.o
+snd-soc-mmp-objs := mmp-pcm.o
 
 obj-$(CONFIG_SND_PXA2XX_SOC) += snd-soc-pxa2xx.o
 obj-$(CONFIG_SND_PXA2XX_SOC_AC97) += snd-soc-pxa2xx-ac97.o
 obj-$(CONFIG_SND_PXA2XX_SOC_I2S) += snd-soc-pxa2xx-i2s.o
 obj-$(CONFIG_SND_PXA_SOC_SSP) += snd-soc-pxa-ssp.o
+obj-$(CONFIG_SND_MMP_SOC) += snd-soc-mmp.o
 
 # PXA Machine Support
 snd-soc-corgi-objs := corgi.o

sound/soc/pxa/mmp-pcm.c new file +448 -0

new file mode 100644
index 0000000..abafbb9
@@ -0,0 +1,448 @@
+/*
+ * linux/sound/soc/pxa/mmp-pcm.c
+ *
+ * Copyright (C) 2011 Marvell International Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/platform_data/mmp_dma.h>
+#include <linux/platform_data/mmp_audio.h>
+#include <sound/pxa2xx-lib.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <mach/sram.h>
+
+struct mmp_runtime_data {
+	int ssp_id;
+	u32 period_size;
+	u32 pointer;
+	struct resource *dma_res;
+	struct mmp_tdma_data tdma_data;
+	struct gen_pool *gpool;
+	struct snd_pcm_substream *substream;
+	struct dma_chan *dma_chan;
+	struct dma_async_tx_descriptor *desc;
+};
+
+#define MMP_PCM_INFO (SNDRV_PCM_INFO_MMAP |	\
+		SNDRV_PCM_INFO_MMAP_VALID |	\
+		SNDRV_PCM_INFO_INTERLEAVED |	\
+		SNDRV_PCM_INFO_PAUSE |		\
+		SNDRV_PCM_INFO_RESUME)
+
+#define MMP_PCM_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \
+			 SNDRV_PCM_FMTBIT_S24_LE | \
+			 SNDRV_PCM_FMTBIT_S32_LE)
+
+static struct snd_pcm_hardware mmp_pcm_hardware[] = {
+	{
+		.info			= MMP_PCM_INFO,
+		.formats		= MMP_PCM_FORMATS,
+		.period_bytes_min	= 1024,
+		.period_bytes_max	= 2048,
+		.periods_min		= 2,
+		.periods_max		= 32,
+		.buffer_bytes_max	= 4096,
+		.fifo_size		= 32,
+	},
+	{
+		.info			= MMP_PCM_INFO,
+		.formats		= MMP_PCM_FORMATS,
+		.period_bytes_min	= 1024,
+		.period_bytes_max	= 2048,
+		.periods_min		= 2,
+		.periods_max		= 32,
+		.buffer_bytes_max	= 4096,
+		.fifo_size		= 32,
+	},
+};
+
+static void mmp_pcm_adma_irq(void *data)
+{
+	struct snd_pcm_substream *substream = data;
+	struct mmp_runtime_data *prtd = substream->runtime->private_data;
+	size_t dma_bytes = substream->runtime->dma_bytes;
+
+	prtd->pointer = (prtd->pointer + prtd->period_size) % dma_bytes;
+	snd_pcm_period_elapsed(substream);
+	return;
+}
+
+static bool filter(struct dma_chan *chan, void *param)
+{
+	struct mmp_runtime_data *prtd = param;
+	bool found = false;
+	char *devname;
+
+	devname = kasprintf(GFP_KERNEL, "%s.%d", prtd->dma_res->name,
+		prtd->ssp_id);
+	if ((strcmp(dev_name(chan->device->dev), devname) == 0) &&
+		(chan->chan_id == prtd->dma_res->start)) {
+		chan->private = &prtd->tdma_data;
+		found = true;
+	}
+
+	kfree(devname);
+	return found;
+}
+
+static int mmp_pcm_hw_params(struct snd_pcm_substream *substream,
+			      struct snd_pcm_hw_params *params)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct mmp_runtime_data *prtd = runtime->private_data;
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct pxa2xx_pcm_dma_params *dma_params;
+	size_t totsize = params_buffer_bytes(params);
+	size_t period = params_period_bytes(params);
+	int ret;
+	struct dma_slave_config slave_config;
+	dma_cap_mask_t mask;
+
+	dma_params = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
+	if (!dma_params)
+		return 0;
+
+	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_S8:
+		prtd->tdma_data.bus_size = 8;
+		break;
+	case SNDRV_PCM_FORMAT_S16_LE:
+		prtd->tdma_data.bus_size = 16;
+		break;
+	case SNDRV_PCM_FORMAT_S24_LE:
+		prtd->tdma_data.bus_size = 24;
+		break;
+	case SNDRV_PCM_FORMAT_S32_LE:
+		prtd->tdma_data.bus_size = 32;
+		break;
+	default:
+		return -EINVAL;
+	}
+	prtd->tdma_data.pack_mod = true;
+
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_CYCLIC, mask);
+	prtd->dma_chan = dma_request_channel(mask, filter, prtd);
+	if (!prtd->dma_chan)
+		return -EINVAL;
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		slave_config.direction    = DMA_TO_DEVICE;
+		slave_config.dst_addr     = dma_params->dev_addr;
+		slave_config.dst_maxburst = 4;
+	} else {
+		slave_config.direction	  = DMA_FROM_DEVICE;
+		slave_config.src_addr	  = dma_params->dev_addr;
+		slave_config.src_maxburst = 4;
+	}
+
+	ret = dmaengine_slave_config(prtd->dma_chan, &slave_config);
+	if (ret)
+		return ret;
+
+	snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer);
+
+	prtd->period_size = period;
+	runtime->dma_bytes = totsize;
+
+	prtd->desc = prtd->dma_chan->device->device_prep_dma_cyclic(
+			prtd->dma_chan, runtime->dma_addr, totsize, period,
+			substream->stream == SNDRV_PCM_STREAM_PLAYBACK ?
+			DMA_TO_DEVICE : DMA_FROM_DEVICE, NULL);
+	if (!prtd->desc) {
+		dev_err(&prtd->dma_chan->dev->device, "cannot prepare slave dma\n");
+		return -EINVAL;
+	}
+
+	prtd->desc->callback = mmp_pcm_adma_irq;
+	prtd->desc->callback_param = substream;
+	dmaengine_submit(prtd->desc);
+	return 0;
+}
+
+static int mmp_pcm_hw_free(struct snd_pcm_substream *substream)
+{
+	struct mmp_runtime_data *prtd = substream->runtime->private_data;
+
+	if (prtd->dma_chan) {
+		dma_release_channel(prtd->dma_chan);
+		prtd->dma_chan = NULL;
+	}
+
+	return 0;
+}
+
+static int mmp_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	struct mmp_runtime_data *prtd = substream->runtime->private_data;
+	int ret = 0;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+		prtd->pointer = 0;
+		dma_async_issue_pending(prtd->dma_chan);
+		break;
+
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		dmaengine_terminate_all(prtd->dma_chan);
+		break;
+
+	case SNDRV_PCM_TRIGGER_RESUME:
+		dma_async_issue_pending(prtd->dma_chan);
+		break;
+
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		dma_async_issue_pending(prtd->dma_chan);
+		break;
+
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+static snd_pcm_uframes_t mmp_pcm_pointer(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct mmp_runtime_data *prtd = runtime->private_data;
+	snd_pcm_uframes_t x;
+
+	x = bytes_to_frames(runtime, prtd->pointer);
+	if (x == runtime->buffer_size)
+		x = 0;
+
+	return x;
+}
+
+static int mmp_pcm_open(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct platform_device *pdev = to_platform_device(rtd->platform->dev);
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	struct mmp_runtime_data *prtd;
+	struct resource *r;
+	int ret;
+
+	r = platform_get_resource(pdev, IORESOURCE_DMA, substream->stream);
+	if (!r)
+		return -EBUSY;
+
+	snd_soc_set_runtime_hwparams(substream,
+				&mmp_pcm_hardware[substream->stream]);
+
+	/*
+	 * For mysterious reasons (and despite what the manual says)
+	 * playback samples are lost if the DMA count is not a multiple
+	 * of the DMA burst size.  Let's add a rule to enforce that.
+	 */
+	ret = snd_pcm_hw_constraint_step(runtime, 0,
+		SNDRV_PCM_HW_PARAM_PERIOD_BYTES, 32);
+	if (ret)
+		goto out;
+
+	ret = snd_pcm_hw_constraint_step(runtime, 0,
+		SNDRV_PCM_HW_PARAM_BUFFER_BYTES, 32);
+	if (ret)
+		goto out;
+
+	ret = snd_pcm_hw_constraint_integer(runtime,
+					SNDRV_PCM_HW_PARAM_PERIODS);
+	if (ret < 0)
+		goto out;
+
+	prtd = devm_kzalloc(&pdev->dev,
+			sizeof(struct mmp_runtime_data), GFP_KERNEL);
+	if (prtd == NULL) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	prtd->substream = substream;
+	runtime->private_data = prtd;
+	prtd->dma_res = r;
+	prtd->ssp_id = cpu_dai->id;
+	return 0;
+
+out:
+	return ret;
+}
+
+static int mmp_pcm_close(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct platform_device *pdev = to_platform_device(rtd->platform->dev);
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct mmp_runtime_data *prtd = runtime->private_data;
+
+	devm_kfree(&pdev->dev, prtd);
+	runtime->private_data = NULL;
+	return 0;
+}
+
+static int mmp_pcm_mmap(struct snd_pcm_substream *substream,
+			 struct vm_area_struct *vma)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	unsigned long off = vma->vm_pgoff;
+
+	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+	return remap_pfn_range(vma, vma->vm_start,
+		__phys_to_pfn(runtime->dma_addr) + off,
+		vma->vm_end - vma->vm_start, vma->vm_page_prot);
+}
+
+struct snd_pcm_ops mmp_pcm_ops = {
+	.open		= mmp_pcm_open,
+	.close		= mmp_pcm_close,
+	.ioctl		= snd_pcm_lib_ioctl,
+	.hw_params	= mmp_pcm_hw_params,
+	.hw_free	= mmp_pcm_hw_free,
+	.trigger	= mmp_pcm_trigger,
+	.pointer	= mmp_pcm_pointer,
+	.mmap		= mmp_pcm_mmap,
+};
+
+static int mmp_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int stream)
+{
+	struct snd_pcm_substream *substream = pcm->streams[stream].substream;
+	struct snd_dma_buffer *buf = &substream->dma_buffer;
+	size_t size = mmp_pcm_hardware[stream].buffer_bytes_max;
+	struct gen_pool *gpool;
+
+	buf->dev.type = SNDRV_DMA_TYPE_DEV;
+	buf->dev.dev = pcm->card->dev;
+	buf->private_data = NULL;
+
+	gpool = sram_get_gpool("asram");
+	if (!gpool)
+		return -ENOMEM;
+
+	buf->area = (unsigned char *)gen_pool_alloc(gpool, size);
+	if (!buf->area)
+		return -ENOMEM;
+	buf->addr = gen_pool_virt_to_phys(gpool, (unsigned long)buf->area);
+	buf->bytes = size;
+	return 0;
+}
+
+static void mmp_pcm_free_dma_buffers(struct snd_pcm *pcm)
+{
+	struct snd_pcm_substream *substream;
+	struct snd_dma_buffer *buf;
+	int stream;
+	struct gen_pool *gpool;
+
+	gpool = sram_get_gpool("asram");
+	if (!gpool)
+		return;
+
+	for (stream = 0; stream < 2; stream++) {
+		size_t size = mmp_pcm_hardware[stream].buffer_bytes_max;
+
+		substream = pcm->streams[stream].substream;
+		if (!substream)
+			continue;
+
+		buf = &substream->dma_buffer;
+		if (!buf->area)
+			continue;
+		gen_pool_free(gpool, (unsigned long)buf->area, size);
+		buf->area = NULL;
+	}
+
+	return;
+}
+
+static u64 mmp_pcm_dmamask = DMA_BIT_MASK(64);
+
+int mmp_pcm_new(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_card *card = rtd->card->snd_card;
+	struct snd_pcm *pcm = rtd->pcm;
+	int ret = 0;
+
+	if (!card->dev->dma_mask)
+		card->dev->dma_mask = &mmp_pcm_dmamask;
+
+	if (!card->dev->coherent_dma_mask)
+		card->dev->coherent_dma_mask = DMA_BIT_MASK(64);
+
+	if (pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream) {
+		ret = mmp_pcm_preallocate_dma_buffer(pcm,
+			SNDRV_PCM_STREAM_PLAYBACK);
+		if (ret)
+			goto out;
+	}
+
+	if (pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream) {
+		ret = mmp_pcm_preallocate_dma_buffer(pcm,
+			SNDRV_PCM_STREAM_CAPTURE);
+		if (ret)
+			goto out;
+	}
+ out:
+	return ret;
+}
+
+struct snd_soc_platform_driver mmp_soc_platform = {
+	.ops		= &mmp_pcm_ops,
+	.pcm_new	= mmp_pcm_new,
+	.pcm_free	= mmp_pcm_free_dma_buffers,
+};
+
+static __devinit int mmp_pcm_probe(struct platform_device *pdev)
+{
+	struct mmp_audio_platdata *pdata = pdev->dev.platform_data;
+
+	if (pdata) {
+		mmp_pcm_hardware[SNDRV_PCM_STREAM_PLAYBACK].buffer_bytes_max =
+						pdata->buffer_max_playback;
+		mmp_pcm_hardware[SNDRV_PCM_STREAM_PLAYBACK].period_bytes_max =
+						pdata->period_max_playback;
+		mmp_pcm_hardware[SNDRV_PCM_STREAM_CAPTURE].buffer_bytes_max =
+						pdata->buffer_max_capture;
+		mmp_pcm_hardware[SNDRV_PCM_STREAM_CAPTURE].period_bytes_max =
+						pdata->period_max_capture;
+	}
+	return snd_soc_register_platform(&pdev->dev, &mmp_soc_platform);
+}
+
+static int __devexit mmp_pcm_remove(struct platform_device *pdev)
+{
+	snd_soc_unregister_platform(&pdev->dev);
+	return 0;
+}
+
+static struct platform_driver mmp_pcm_driver = {
+	.driver = {
+		.name = "mmp-pcm-audio",
+		.owner = THIS_MODULE,
+	},
+
+	.probe = mmp_pcm_probe,
+	.remove = __devexit_p(mmp_pcm_remove),
+};
+
+module_platform_driver(mmp_pcm_driver);
+
+MODULE_AUTHOR("Leo Yan <[email protected]>");
+MODULE_DESCRIPTION("MMP Soc Audio DMA module");
+MODULE_LICENSE("GPL");

Quoted 2 lines · Zhangfei Gao

mmp-pcm handle audio dma based on dmaengine
Support mmp and pxa910

Looks like this is *not* using soc-dmaengine library, why?

sound/soc/pxa/mmp-pcm.c line 140

Quoted 3 lines

sound/soc/pxa/mmp-pcm.c new file +3 -0

+	prtd->dma_chan = dma_request_channel(mask, filter, prtd);
+	if (!prtd->dma_chan)
+		return -EINVAL;

sound/soc/pxa/mmp-pcm.c line 331

Quoted 9 lines

sound/soc/pxa/mmp-pcm.c new file +9 -0

+static int mmp_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int stream)
+{
+	struct snd_pcm_substream *substream = pcm->streams[stream].substream;
+	struct snd_dma_buffer *buf = &substream->dma_buffer;
+	size_t size = mmp_pcm_hardware[stream].buffer_bytes_max;
+	struct gen_pool *gpool;
+
+	buf->dev.type = SNDRV_DMA_TYPE_DEV;
+	buf->dev.dev = pcm->card->dev;

Quoted 5 lines

sound/soc/pxa/mmp-pcm.c new file +5 -0

+static u64 mmp_pcm_dmamask = DMA_BIT_MASK(64);
+
+int mmp_pcm_new(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_card *card = rtd->card->snd_card;

sound/soc/pxa/mmp-pcm.c line 386

Quoted 8 lines

sound/soc/pxa/mmp-pcm.c new file +8 -0

+	struct snd_pcm *pcm = rtd->pcm;
+	int ret = 0;
+
+	if (!card->dev->dma_mask)
+		card->dev->dma_mask = &mmp_pcm_dmamask;
+
+	if (!card->dev->coherent_dma_mask)
+		card->dev->coherent_dma_mask = DMA_BIT_MASK(64);

Quoted 4 lines · Vinod Koul

Quoted 2 lines · Zhangfei Gao

mmp-pcm handle audio dma based on dmaengine
Support mmp and pxa910

Looks like this is *not* using soc-dmaengine library, why?

Note also...

Quoted 4 lines

sound/soc/pxa/mmp-pcm.c line 140

Quoted 4 lines

sound/soc/pxa/mmp-pcm.c new file +3 -0

+   prtd->dma_chan = dma_request_channel(mask, filter, prtd);
+   if (!prtd->dma_chan)
+           return -EINVAL;

This should be done at probe time, so we know the struct device, so that...

Quoted 1 line

Quoted 1 line

+static int mmp_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int

stream)

Quoted 2 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+{
+   struct snd_pcm_substream *substream =

pcm->streams[stream].substream;

Quoted 6 lines

sound/soc/pxa/mmp-pcm.c line 331

Quoted 6 lines

sound/soc/pxa/mmp-pcm.c new file +6 -0

+   struct snd_dma_buffer *buf = &substream->dma_buffer;
+   size_t size = mmp_pcm_hardware[stream].buffer_bytes_max;
+   struct gen_pool *gpool;
+
+   buf->dev.type = SNDRV_DMA_TYPE_DEV;
+   buf->dev.dev = pcm->card->dev;

... this uses the right device, and...

Quoted 5 lines

Quoted 5 lines

sound/soc/pxa/mmp-pcm.c new file +5 -0

+static u64 mmp_pcm_dmamask = DMA_BIT_MASK(64);
+
+int mmp_pcm_new(struct snd_soc_pcm_runtime *rtd)
+{
+   struct snd_card *card = rtd->card->snd_card;

Quoted 8 lines

sound/soc/pxa/mmp-pcm.c line 386

Quoted 8 lines

sound/soc/pxa/mmp-pcm.c new file +8 -0

+   struct snd_pcm *pcm = rtd->pcm;
+   int ret = 0;
+
+   if (!card->dev->dma_mask)
+           card->dev->dma_mask = &mmp_pcm_dmamask;
+
+   if (!card->dev->coherent_dma_mask)
+           card->dev->coherent_dma_mask = DMA_BIT_MASK(64);

... we don't need crap like this.

Because then we'll be allocating buffers against the _right_ struct device which is the DMA engine struct device.
--
To unsubscribe from this list: send the line "unsubscribe alsa-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Do you mean at open time, like snd_dmaengine_pcm_open. The channel resource is limited and better get dynamically. As a result the pcm_new and preallocate already called before.

Quoted 3 lines · zhangfei gao

Do you mean at open time, like snd_dmaengine_pcm_open. The channel resource is limited and better get dynamically. As a  result the pcm_new and preallocate already called before.

This is where dealing with slave DMA channels in a virtualized setup becomes a far better solution than trying to assign a particular physical channel at request time.

What we may wish to think about is having a way for slave drivers to assert to DMA engine the priority of a channel, which they can change dynamically according to what they're doing.  Eg, an ALSA driver would leave the channel low priority while it's not expecting to be used, but as soon as we see the prepare call, set it to high priority.

The DMA engine driver could use that to decide to assign a physical channel to the virtual channel, so that DMA can start as soon as possible even with other activity on the DMA engine.

However, I've yet to see any setup where the number of physical DMA channels available exceeds the number of actual _simultaneous_ users. Even with the five channels on SA11x0 shared between 12? peripherals, with my DMA engine driver I've only seen one or two physical channels being used simultaneously.

When debugging with putting snd_dmaengine_pcm_open to probe or pcm_new, there is issue.
snd_dmaengine_pcm_open -> snd_pcm_hw_constraint_integer(substream->runtime,

SNDRV_PCM_HW_PARAM_PERIODS);
The runtime is only exist at runtime.
So if using soc-dmaengine, I am afraid the snd_dmaengine_pcm_open has to be put in open instead of probe.

Quoted 6 lines · zhangfei gao

When debugging with putting snd_dmaengine_pcm_open to probe or pcm_new, there is issue.
snd_dmaengine_pcm_open -> snd_pcm_hw_constraint_integer(substream->runtime,

Quoted 4 lines

SNDRV_PCM_HW_PARAM_PERIODS);
The runtime is only exist at runtime.
So if using soc-dmaengine, I am afraid the snd_dmaengine_pcm_open has to be put in open instead of probe.

I think you got it wrong, the snd_dmaengine_pcm_open needs to be called from your CPU pcm_open. See other examples how this is used in other drivers.

Quoted 11 lines · Vinod Koul

Quoted 10 lines · zhangfei gao

When debugging with putting snd_dmaengine_pcm_open to probe or pcm_new, there is issue.
snd_dmaengine_pcm_open -> snd_pcm_hw_constraint_integer(substream->runtime,

SNDRV_PCM_HW_PARAM_PERIODS);
The runtime is only exist at runtime.
So if using soc-dmaengine, I am afraid the snd_dmaengine_pcm_open has to be put in open instead of probe.

Quoted 3 lines

I think you got it wrong, the snd_dmaengine_pcm_open needs to be called from your CPU pcm_open. See other examples how this is used in other drivers.

But, as I pointed out earlier, there's the issue of using the wrong struct device to allocate memory for the DMA engine.  That's something that my soc-dmaengine.c got _right_, and soc-dmaengine-pcm.c gets _wrong_.

Quoted 12 lines · Russell King - ARM Linux

Quoted 11 lines · Vinod Koul

Quoted 10 lines · zhangfei gao

When debugging with putting snd_dmaengine_pcm_open to probe or pcm_new, there is issue.
snd_dmaengine_pcm_open -> snd_pcm_hw_constraint_integer(substream->runtime,

SNDRV_PCM_HW_PARAM_PERIODS);
The runtime is only exist at runtime.
So if using soc-dmaengine, I am afraid the snd_dmaengine_pcm_open has to be put in open instead of probe.

Quoted 7 lines

Quoted 3 lines

I think you got it wrong, the snd_dmaengine_pcm_open needs to be called from your CPU pcm_open. See other examples how this is used in other drivers.

But, as I pointed out earlier, there's the issue of using the wrong struct device to allocate memory for the DMA engine.  That's something that my soc-dmaengine.c got _right_, and soc-dmaengine-pcm.c gets _wrong_.

Could you help clarify what's the struct device should be used? Any example, not find soc-dmaengine.c.
When debug,
rtd->card->snd_card->dev is same as substream->pcm->card->dev, which we using now.

But, as I pointed out earlier, there's the issue of using the wrong struct device to allocate memory for the DMA engine. That's something that my soc-dmaengine.c got _right_, and soc-dmaengine-pcm.c gets _wrong_.

Quoted 3 lines · Russell King - ARM Linux

But, as I pointed out earlier, there's the issue of using the wrong struct device to allocate memory for the DMA engine. That's something that my soc-dmaengine.c got _right_, and soc-dmaengine-pcm.c gets _wrong_.

What is the issue with the current code - it *looks* like you want to use the component device for the DMA as the struct device with dmaengine but I don't understand the issue that's created if we don't (and why things appear to be working for people as they are).

[...]
What I have as my own version (now with cyclic DMA support) is IMHO a far superior and more correct implementation.

Quoted 4 lines · Mark Brown

What is the issue with the current code - it *looks* like you want to use the component device for the DMA as the struct device with dmaengine but I don't understand the issue that's created if we don't (and why things appear to be working for people as they are).

Look. It's very very very very simple.

What does the DMA API take? A struct device. What struct device? Some random struct device for something in the system, or what? No, it takes the struct device for the _device_ in the system which is _performing_ the DMA.

If you persist in not wanting to care about this, then I'm afraid I'll ignore soc-dmaengine-pcm.c entirely as to me its totally bolloxed code. What I have as my own version (now with cyclic DMA support) is IMHO a far superior and more correct implementation.

Quoted 8 lines · Russell King - ARM Linux

Quoted 4 lines · Mark Brown

What is the issue with the current code - it *looks* like you want to use the component device for the DMA as the struct device with dmaengine but I don't understand the issue that's created if we don't (and why things appear to be working for people as they are).

Look. It's very very very very simple.

Quoted 4 lines

What does the DMA API take? A struct device. What struct device? Some random struct device for something in the system, or what? No, it takes the struct device for the _device_ in the system which is _performing_ the DMA.

I'm assuming that you mean the client rather than the DMA controller itself (which we must already have found)? That makes sense,

Currently we seem to have nobody actually working on this code in mainline, as far as I can tell you and Vinod are the only people with any active interest at a framework level (with Vinod's mostly being at the dmaengine rather than ASoC level; I don't have any hardware with any sort of dmaengine support) and because you have this out of tree implementation you're working on you're mostly just offering feedback in these driver review threads where apparently chunks of it are getting missed. Nobody else seems to be showing much inclination to get involved so it seems unlikely to collide with other work.

Quoted 2 lines · Mark Brown

I'm assuming that you mean the client rather than the DMA controller itself (which we must already have found)? That makes sense,

No, I mean the DMA controller *itself*.

That's partly because you took in soc-dmaengine.c, after you ridiculed my version - mainly because it didn't support cyclic DMA at the time.

And so I assumed that you'd made up your mind, and just plain weren't interested.

You have been very resistive towards discussing issues
coming up in this area, and in other areas I've raised with ASoC such as the struct device lifetime issues - even when I've created patches for you.

Moreover, because I don't see any hope for the SA11x0 PCM support going in (it certainly won't support capture mode due to the need to run the playback side to receive capture samples) I'm really not pushing it; it's more my testbed for testing out my DMA engine work than serious ASoC work.

The SSPA is a configurable multi-channel audio serial (TDM) interface. It's configurable at runtime to support up to 128 channels and the number of bits per sample: 8, 12, 16, 20, 24 and 32 bits. It also support stereo format: I2S, left-justified or right-justified.

sound/soc/pxa/mmp-sspa.c new file +4 -0

+	res = request_mem_region(res->start, resource_size(res),
+			pdev->name);
+	if (res == NULL) {
+		dev_err(&pdev->dev, "failed to request memory resource\n");

Quoted 4 lines · Zhangfei Gao

The SSPA is a configurable multi-channel audio serial (TDM) interface. It's configurable at runtime to support up to 128 channels and the number of bits per sample: 8, 12, 16, 20, 24 and 32 bits. It also support stereo format: I2S, left-justified or right-justified.

Mostly looks good. ?A few fairly minor things...

Quoted 5 lines

+
+ ? ? if (!cpu_dai->active) {
+ ? ? ? ? ? ? clk_enable(sysclk);
+ ? ? ? ? ? ? clk_enable(sspa->clk);
+ ? ? }

The clock API is refcounted so you shouldn't need to worry about multiple enables, you should just be able to unconditionally enable and disable. ?If this is needed we probably have a problem we should fix.

Quoted 8 lines

+ ? ? switch (pll_id) {
+ ? ? case MMP_SYSCLK:
+ ? ? ? ? ? ? clk_set_rate(sysclk, freq_out);
+ ? ? ? ? ? ? break;
+ ? ? case MMP_SSPA_CLK:
+ ? ? ? ? ? ? clk_set_rate(sspa->clk, freq_out);
+ ? ? ? ? ? ? break;

You're ignoring the return values here.

Quoted 6 lines

+ ? ? priv->sspa->clk = clk_get(&pdev->dev, NULL);
+ ? ? if (IS_ERR(priv->sspa->clk)) {
+ ? ? ? ? ? ? ret = PTR_ERR(priv->sspa->clk);
+ ? ? ? ? ? ? goto err_free_clk;
+ ? ? }

devm_clk_get().

Quoted 15 lines

+ ? ? res = request_mem_region(res->start, resource_size(res),
+ ? ? ? ? ? ? ? ? ? ? pdev->name);
+ ? ? if (res == NULL) {
+ ? ? ? ? ? ? dev_err(&pdev->dev, "failed to request memory resource\n");
+ ? ? ? ? ? ? ret = -EBUSY;
+ ? ? ? ? ? ? goto err_get_res;
+ ? ? }
+
+ ? ? priv->sspa->mmio_base = ioremap(res->start, resource_size(res));
+ ? ? if (priv->sspa->mmio_base == NULL) {
+ ? ? ? ? ? ? dev_err(&pdev->dev, "failed to ioremap() registers\n");
+ ? ? ? ? ? ? ret = -ENODEV;
+ ? ? ? ? ? ? goto err_free_mem;
+ ? ? }

devm_request_and_ioremap().

sound/soc/pxa/mmp-sspa.c

Quoted 7 lines

sound/soc/pxa/mmp-sspa.c new file +6 -0

+err_free_clk:
+ ? ? devm_kfree(&pdev->dev, priv->dma_params);
+err_priv_dma_params:
+ ? ? devm_kfree(&pdev->dev, priv->sspa);
+err_priv_sspa:
+ ? ? devm_kfree(&pdev->dev, priv);

The whole point of the devm_ stuff is that you don't need to do stuff like this.

_______________________________________________
Alsa-devel mailing list
Alsa-devel at alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

sound/soc/pxa/Kconfig config SND_PXA2XX_SOC_IMOTE2 · line 213

Quoted 5 lines · Zhangfei Gao

sound/soc/pxa/Kconfig +5 -0

+config SND_MMP_SOC_BROWNSTONE
+	tristate "SoC Audio support for Marvell Brownstone"
+	depends on SND_MMP_SOC&&  MACH_BROWNSTONE
+	select SND_MMP_SOC_SSPA
+	select SND_SOC_WM8994

Should depend on MFD_WM8994.

Review

Quoted 2 lines

(quoted diff) +2 -0

+static void brownstone_ext_control(struct snd_soc_dapm_context *dapm)
+{

This stuff is really unexpected in a modern machine driver, it was used on things like spitz due to the odd wiring but for modern stuff I'd not expect to see it.

sound/soc/pxa/brownstone.c line 56

Quoted 8 lines

sound/soc/pxa/brownstone.c new file +7 -0

+	if (brownstone_spk_func == BROWNSTONE_SPK_ON) {
+		snd_soc_dapm_enable_pin(dapm, "Ext Left Spk");
+		snd_soc_dapm_enable_pin(dapm, "Ext Right Spk");
+	} else {
+		snd_soc_dapm_disable_pin(dapm, "Ext Left Spk");
+		snd_soc_dapm_disable_pin(dapm, "Ext Right Spk");
+	}

Just define a single widget for the speakers and use a SND_SOC_DAPM_PIN_SWITCH().

sound/soc/pxa/brownstone.c line 64

Quoted 7 lines

sound/soc/pxa/brownstone.c new file +7 -0

+	/* set up jack connection */
+	switch (brownstone_jack_func) {
+	case BROWNSTONE_HP:
+		snd_soc_dapm_disable_pin(dapm, "Headset Mic");
+		snd_soc_dapm_enable_pin(dapm, "Main Mic");
+		snd_soc_dapm_enable_pin(dapm, "Headset Stereophone");
+		break;

This should all be autodetectable, users having to manually select it is *very* unusual. Is there really no accessory detection hardware on the board?

sound/soc/pxa/brownstone.c line 175

Quoted 6 lines

sound/soc/pxa/brownstone.c new file +5 -0

+	snd_soc_dapm_enable_pin(dapm, "Ext Left Spk");
+	snd_soc_dapm_enable_pin(dapm, "Ext Right Spk");
+	snd_soc_dapm_enable_pin(dapm, "Headset Stereophone");
+	snd_soc_dapm_enable_pin(dapm, "Headset Mic");
+	snd_soc_dapm_enable_pin(dapm, "Main Mic");

Everything is enable dby default.

sound/soc/pxa/brownstone.c line 199

Quoted 6 lines

sound/soc/pxa/brownstone.c new file +6 -0

+	/* turn on micbias 1/2 always */
+	snd_soc_update_bits(codec, WM8994_POWER_MANAGEMENT_1,
+			    WM8994_MICB1_ENA_MASK |
+			    WM8994_MICB2_ENA_MASK,
+			    WM8994_MICB1_ENA |
+			    WM8994_MICB2_ENA);

If you need to do this force enable them with DAPM, this won't work anyway as the widgets will be powered off as soon as DAPM notices they're on.

sound/soc/pxa/brownstone.c line 235

Quoted 5 lines

sound/soc/pxa/brownstone.c new file +5 -0

+	snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_I2S |
+		SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS);
+
+	snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_I2S |
+		SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS);

Set this in the dai_link.

sound/soc/pxa/brownstone.c line 292

Quoted 3 lines

sound/soc/pxa/brownstone.c new file +3 -0

+static struct platform_driver mmp_driver = {
+	.driver		= {
+		.name	= "mmp-audio",

Should probably be something like "brownstone-audio".