diff mbox

ASoC: tegra: use dmaengine based dma driver

Message ID 1340969673-7776-2-git-send-email-ldewangan@nvidia.com
State Not Applicable, archived
Headers show

Commit Message

Laxman Dewangan June 29, 2012, 11:34 a.m. UTC
Use the dmaengine based Tegra APB DMA driver for
data transfer between SPI fifo and memory in
place of legacy Tegra APB DMA.

Because generic soc-dmaengine-pcm uses the DMAs API
based on dmaengine, using the exported APIs provided
by this generic driver.

The new driver is selected if legacy driver is not
selected and new dma driver is enabled through config
file.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 sound/soc/tegra/Kconfig     |    3 +-
 sound/soc/tegra/tegra_pcm.c |  115 +++++++++++++++++++++++++++++++++++++++++++
 sound/soc/tegra/tegra_pcm.h |    2 +
 3 files changed, 119 insertions(+), 1 deletions(-)

Comments

Stephen Warren June 29, 2012, 5:02 p.m. UTC | #1
On 06/29/2012 05:34 AM, Laxman Dewangan wrote:
> Use the dmaengine based Tegra APB DMA driver for
> data transfer between SPI fifo and memory in
> place of legacy Tegra APB DMA.
> 
> Because generic soc-dmaengine-pcm uses the DMAs API
> based on dmaengine, using the exported APIs provided
> by this generic driver.
> 
> The new driver is selected if legacy driver is not
> selected and new dma driver is enabled through config
> file.

This works just fine with the existing non-dmaengine DMA driver enabled.

However, I can't get it to work with dmaengine:

> # aplay ~/abba-dq-48000-stereo.wav
> [  151.613476] tegra20-i2s tegra20-i2s.0: dmaengine pcm open failed with err -6
> [  151.620557] tegra20-i2s tegra20-i2s.0: can't open platform tegra20-i2s.0: -6
> aplay: main:654: audio open error: No such device or address

I do have the following in my local tree:
68a67b8 ARM: tegra: add device tree AUXDATA for APBDMA
0db7a96 ARM: tegra: dma: rename driver name for clock to "tegra-apbdma"

I also fixed the compatible values in drivers/dma/tegra20-apb-dma.c so
the driver would get instantiated, which it does;
/sys/devices/tegra-apbdma/dma has a bunch of dmaengine channels in it.

(Note: This is on Ventana, although I doubt that makes much difference)

Is there something else I need to do to test this?
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laxman Dewangan June 30, 2012, 3:08 p.m. UTC | #2
On Friday 29 June 2012 10:32 PM, Stephen Warren wrote:
> On 06/29/2012 05:34 AM, Laxman Dewangan wrote:
>> Use the dmaengine based Tegra APB DMA driver for
>> data transfer between SPI fifo and memory in
>> place of legacy Tegra APB DMA.
>>
>> Because generic soc-dmaengine-pcm uses the DMAs API
>> based on dmaengine, using the exported APIs provided
>> by this generic driver.
>>
>> The new driver is selected if legacy driver is not
>> selected and new dma driver is enabled through config
>> file.
> This works just fine with the existing non-dmaengine DMA driver enabled.
>
> However, I can't get it to work with dmaengine:
>
>> # aplay ~/abba-dq-48000-stereo.wav
>> [  151.613476] tegra20-i2s tegra20-i2s.0: dmaengine pcm open failed with err -6
>> [  151.620557] tegra20-i2s tegra20-i2s.0: can't open platform tegra20-i2s.0: -6
>> aplay: main:654: audio open error: No such device or address

With the error, it seems that dmachannel is not getting allocated.

> I do have the following in my local tree:
> 68a67b8 ARM: tegra: add device tree AUXDATA for APBDMA
> 0db7a96 ARM: tegra: dma: rename driver name for clock to "tegra-apbdma"
>

Some dma changes which I sent and already on linux-next are also require 
with the above change.
Alsong with that I have local change like to rename the dma driver to 
compatible with dts files (remove -new from driver).

I think I should send that patches so that you can test it by:
- Taking already applied dma driver change in linux-next.
- Apply my new patches which I am going to send.

And do local change in the tegra_defconfig to disable SYSTEM_DMA and 
enable dmaengine based dma driver.


> I also fixed the compatible values in drivers/dma/tegra20-apb-dma.c so
> the driver would get instantiated, which it does;
> /sys/devices/tegra-apbdma/dma has a bunch of dmaengine channels in it.
>

Possibly some patches in dma driver  which is already applied in next is 
not available in your tree. CYCLIC_DMA patch is important.

> (Note: This is on Ventana, although I doubt that makes much difference)
>

I tested on cardhu only but it should not matter.

> Is there something else I need to do to test this?

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laxman Dewangan July 2, 2012, 8:25 a.m. UTC | #3
On Saturday 30 June 2012 08:38 PM, Laxman Dewangan wrote:
> On Friday 29 June 2012 10:32 PM, Stephen Warren wrote:
>> On 06/29/2012 05:34 AM, Laxman Dewangan wrote:
>>> Use the dmaengine based Tegra APB DMA driver for
>>> data transfer between SPI fifo and memory in
>>> place of legacy Tegra APB DMA.
>>>
>>> Because generic soc-dmaengine-pcm uses the DMAs API
>>> based on dmaengine, using the exported APIs provided
>>> by this generic driver.
>>>
>>> The new driver is selected if legacy driver is not
>>> selected and new dma driver is enabled through config
>>> file.
>> This works just fine with the existing non-dmaengine DMA driver enabled.
>>
>> However, I can't get it to work with dmaengine:
>>
>>> # aplay ~/abba-dq-48000-stereo.wav
>>> [  151.613476] tegra20-i2s tegra20-i2s.0: dmaengine pcm open failed with err -6
>>> [  151.620557] tegra20-i2s tegra20-i2s.0: can't open platform tegra20-i2s.0: -6
>>> aplay: main:654: audio open error: No such device or address
> With the error, it seems that dmachannel is not getting allocated.
>
>> I do have the following in my local tree:
>> 68a67b8 ARM: tegra: add device tree AUXDATA for APBDMA
>> 0db7a96 ARM: tegra: dma: rename driver name for clock to "tegra-apbdma"
>>
> Some dma changes which I sent and already on linux-next are also require
> with the above change.
> Alsong with that I have local change like to rename the dma driver to
> compatible with dts files (remove -new from driver).
>
> I think I should send that patches so that you can test it by:
> - Taking already applied dma driver change in linux-next.
> - Apply my new patches which I am going to send.
>
> And do local change in the tegra_defconfig to disable SYSTEM_DMA and
> enable dmaengine based dma driver.
>
>
The new dma patches are:

  dma: tegra: fix residual calculation for cyclic case
  dma: tegra: rename driver and compatible to match with dts

You require dma changes which is already in linux-next:

  dma: tegra: set DMA_CYCLIC capability
  dma: tegra: do not set transfer desc flag to DMA_CTRL_ACK in cyclic mode
  dma: tegra: add clk_prepare/clk_unprepare
  dma: tegra: use sg_dma_address() for getting dma buffer address
  dma: tegra: add dmaengine based dma driver



--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren July 2, 2012, 4:45 p.m. UTC | #4
On 07/02/2012 02:25 AM, Laxman Dewangan wrote:
> On Saturday 30 June 2012 08:38 PM, Laxman Dewangan wrote:
>> On Friday 29 June 2012 10:32 PM, Stephen Warren wrote:
>>> On 06/29/2012 05:34 AM, Laxman Dewangan wrote:
>>>> Use the dmaengine based Tegra APB DMA driver for
>>>> data transfer between SPI fifo and memory in
>>>> place of legacy Tegra APB DMA.
>>>>
>>>> Because generic soc-dmaengine-pcm uses the DMAs API
>>>> based on dmaengine, using the exported APIs provided
>>>> by this generic driver.
>>>>
>>>> The new driver is selected if legacy driver is not
>>>> selected and new dma driver is enabled through config
>>>> file.
>>>
>>> This works just fine with the existing non-dmaengine DMA driver enabled.
>>>
>>> However, I can't get it to work with dmaengine:
...
>> I think I should send that patches so that you can test it by:
>> - Taking already applied dma driver change in linux-next.
>> - Apply my new patches which I am going to send.
>>
>> And do local change in the tegra_defconfig to disable SYSTEM_DMA and
>> enable dmaengine based dma driver.
>
> The new dma patches are:
> 
>  dma: tegra: fix residual calculation for cyclic case
>  dma: tegra: rename driver and compatible to match with dts
> ...

OK, with those two patches plus this one, everything works on
Cardhu/Tegra30 using dmaengine. However, on Tegra20 DMA doesn't appear
to work. No audio is heard, and aplay eventually exits with the
following error message:

aplay: pcm_write:1603: write error: Input/output error

I believe this means that the DMA simply isn't operating.

I'll assume this is a Tegra20-specific issue in the dmaengine driver
itself, and hence not a problem with this patch, so I'll ack this patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren July 2, 2012, 4:46 p.m. UTC | #5
On 06/29/2012 05:34 AM, Laxman Dewangan wrote:
> Use the dmaengine based Tegra APB DMA driver for
> data transfer between SPI fifo and memory in
> place of legacy Tegra APB DMA.
> 
> Because generic soc-dmaengine-pcm uses the DMAs API
> based on dmaengine, using the exported APIs provided
> by this generic driver.
> 
> The new driver is selected if legacy driver is not
> selected and new dma driver is enabled through config
> file.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

Acked-by: Stephen Warren <swarren@wwwdotorg.org>
Tested-by: Stephen Warren <swarren@wwwdotorg.org>

(Tested on Cardhu/Tegra30. It fails on Whistler/Tegra20, but I assume
that's due to a bug in the dmaengine driver not this patch. There won't
be a regression when applying this patch since we haven't switched to
dmaengine as the default yet).
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown July 2, 2012, 4:47 p.m. UTC | #6
On Mon, Jul 02, 2012 at 10:45:01AM -0600, Stephen Warren wrote:

> to work. No audio is heard, and aplay eventually exits with the
> following error message:

For values of eventually that will be about 10s by default.

> aplay: pcm_write:1603: write error: Input/output error

> I believe this means that the DMA simply isn't operating.

Yes, normally.
Laxman Dewangan July 3, 2012, 6:26 a.m. UTC | #7
On Monday 02 July 2012 10:17 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Mon, Jul 02, 2012 at 10:45:01AM -0600, Stephen Warren wrote:
>
>> to work. No audio is heard, and aplay eventually exits with the
>> following error message:
> For values of eventually that will be about 10s by default.
>
>> aplay: pcm_write:1603: write error: Input/output error
>> I believe this means that the DMA simply isn't operating.
> Yes, normally.

Let me debug this specific to T20 but it will not stop to apply the patch.

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laxman Dewangan July 3, 2012, 6:29 a.m. UTC | #8
Mark,
On Monday 02 July 2012 10:16 PM, Stephen Warren wrote:
> On 06/29/2012 05:34 AM, Laxman Dewangan wrote:
>> Use the dmaengine based Tegra APB DMA driver for
>> data transfer between SPI fifo and memory in
>> place of legacy Tegra APB DMA.
>>
>> Because generic soc-dmaengine-pcm uses the DMAs API
>> based on dmaengine, using the exported APIs provided
>> by this generic driver.
>>
>> The new driver is selected if legacy driver is not
>> selected and new dma driver is enabled through config
>> file.
>>
>> Signed-off-by: Laxman Dewangan<ldewangan@nvidia.com>
> Acked-by: Stephen Warren<swarren@wwwdotorg.org>
> Tested-by: Stephen Warren<swarren@wwwdotorg.org>
>
> (Tested on Cardhu/Tegra30. It fails on Whistler/Tegra20, but I assume
> that's due to a bug in the dmaengine driver not this patch. There won't
> be a regression when applying this patch since we haven't switched to
> dmaengine as the default yet).

Are you taking this patch?
I have other independent patch for the writecombine dma support in pcm 
memory library and wanted to avoid any merge conflicts.
So if it goes in your tree then the other patch also need to go in your 
tree.

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown July 3, 2012, 10:09 a.m. UTC | #9
On Tue, Jul 03, 2012 at 11:59:31AM +0530, Laxman Dewangan wrote:

> Are you taking this patch?
> I have other independent patch for the writecombine dma support in
> pcm memory library and wanted to avoid any merge conflicts.
> So if it goes in your tree then the other patch also need to go in
> your tree.

You've not even left 24 hours for me to respond!
Mark Brown July 3, 2012, 7:07 p.m. UTC | #10
On Fri, Jun 29, 2012 at 05:04:33PM +0530, Laxman Dewangan wrote:
> Use the dmaengine based Tegra APB DMA driver for
> data transfer between SPI fifo and memory in
> place of legacy Tegra APB DMA.

Applied, thanks.
Laxman Dewangan July 18, 2012, 8:59 a.m. UTC | #11
On Monday 02 July 2012 10:17 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Mon, Jul 02, 2012 at 10:45:01AM -0600, Stephen Warren wrote:
>
>> to work. No audio is heard, and aplay eventually exits with the
>> following error message:
> For values of eventually that will be about 10s by default.
>
>> aplay: pcm_write:1603: write error: Input/output error
>> I believe this means that the DMA simply isn't operating.
> Yes, normally.

Just for record and update on this mail, the issue was in dma driver 
where it was not enabling the dma clock.
By luck, it was working on Tegra30 because it was getting enabled in 
somewhere may be in uboot or default power-on ON.
Fixed the issue with patch
[PATCH] dma: tegra: enable/disable dma clock

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
index 76dc230..02bcd30 100644
--- a/sound/soc/tegra/Kconfig
+++ b/sound/soc/tegra/Kconfig
@@ -1,7 +1,8 @@ 
 config SND_SOC_TEGRA
 	tristate "SoC Audio for the Tegra System-on-Chip"
-	depends on ARCH_TEGRA && TEGRA_SYSTEM_DMA
+	depends on ARCH_TEGRA && (TEGRA_SYSTEM_DMA || TEGRA20_APB_DMA)
 	select REGMAP_MMIO
+	select SND_SOC_DMAENGINE_PCM if TEGRA20_APB_DMA
 	help
 	  Say Y or M here if you want support for SoC audio on Tegra.
 
diff --git a/sound/soc/tegra/tegra_pcm.c b/sound/soc/tegra/tegra_pcm.c
index 127348d..5658bce 100644
--- a/sound/soc/tegra/tegra_pcm.c
+++ b/sound/soc/tegra/tegra_pcm.c
@@ -36,6 +36,7 @@ 
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
+#include <sound/dmaengine_pcm.h>
 
 #include "tegra_pcm.h"
 
@@ -56,6 +57,7 @@  static const struct snd_pcm_hardware tegra_pcm_hardware = {
 	.fifo_size		= 4,
 };
 
+#if defined(CONFIG_TEGRA_SYSTEM_DMA)
 static void tegra_pcm_queue_dma(struct tegra_runtime_data *prtd)
 {
 	struct snd_pcm_substream *substream = prtd->substream;
@@ -285,6 +287,119 @@  static struct snd_pcm_ops tegra_pcm_ops = {
 	.pointer	= tegra_pcm_pointer,
 	.mmap		= tegra_pcm_mmap,
 };
+#else
+static int tegra_pcm_open(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct device *dev = rtd->platform->dev;
+	int ret;
+
+	/* Set HW params now that initialization is complete */
+	snd_soc_set_runtime_hwparams(substream, &tegra_pcm_hardware);
+
+	ret = snd_dmaengine_pcm_open(substream, NULL, NULL);
+	if (ret) {
+		dev_err(dev, "dmaengine pcm open failed with err %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int tegra_pcm_close(struct snd_pcm_substream *substream)
+{
+	snd_dmaengine_pcm_close(substream);
+	return 0;
+}
+
+static int tegra_pcm_hw_params(struct snd_pcm_substream *substream,
+				struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct device *dev = rtd->platform->dev;
+	struct dma_chan *chan = snd_dmaengine_pcm_get_chan(substream);
+	struct tegra_pcm_dma_params *dmap;
+	struct dma_slave_config slave_config;
+	int ret;
+
+	dmap = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
+
+	ret = snd_hwparams_to_dma_slave_config(substream, params,
+						&slave_config);
+	if (ret) {
+		dev_err(dev, "hw params config failed with err %d\n", ret);
+		return ret;
+	}
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		slave_config.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		slave_config.dst_addr = dmap->addr;
+		slave_config.src_maxburst = 0;
+	} else {
+		slave_config.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		slave_config.src_addr = dmap->addr;
+		slave_config.dst_maxburst = 0;
+	}
+	slave_config.slave_id = dmap->req_sel;
+
+	ret = dmaengine_slave_config(chan, &slave_config);
+	if (ret < 0) {
+		dev_err(dev, "dma slave config failed with err %d\n", ret);
+		return ret;
+	}
+
+	snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer);
+	return 0;
+}
+
+static int tegra_pcm_hw_free(struct snd_pcm_substream *substream)
+{
+	snd_pcm_set_runtime_buffer(substream, NULL);
+	return 0;
+}
+
+static int tegra_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		return snd_dmaengine_pcm_trigger(substream,
+					SNDRV_PCM_TRIGGER_START);
+
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		return snd_dmaengine_pcm_trigger(substream,
+					SNDRV_PCM_TRIGGER_STOP);
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int tegra_pcm_mmap(struct snd_pcm_substream *substream,
+				struct vm_area_struct *vma)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+
+	return dma_mmap_writecombine(substream->pcm->card->dev, vma,
+					runtime->dma_area,
+					runtime->dma_addr,
+					runtime->dma_bytes);
+}
+
+static struct snd_pcm_ops tegra_pcm_ops = {
+	.open		= tegra_pcm_open,
+	.close		= tegra_pcm_close,
+	.ioctl		= snd_pcm_lib_ioctl,
+	.hw_params	= tegra_pcm_hw_params,
+	.hw_free	= tegra_pcm_hw_free,
+	.trigger	= tegra_pcm_trigger,
+	.pointer	= snd_dmaengine_pcm_pointer,
+	.mmap		= tegra_pcm_mmap,
+};
+#endif
 
 static int tegra_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int stream)
 {
diff --git a/sound/soc/tegra/tegra_pcm.h b/sound/soc/tegra/tegra_pcm.h
index 985d418..a3a4503 100644
--- a/sound/soc/tegra/tegra_pcm.h
+++ b/sound/soc/tegra/tegra_pcm.h
@@ -40,6 +40,7 @@  struct tegra_pcm_dma_params {
 	unsigned long req_sel;
 };
 
+#if defined(CONFIG_TEGRA_SYSTEM_DMA)
 struct tegra_runtime_data {
 	struct snd_pcm_substream *substream;
 	spinlock_t lock;
@@ -51,6 +52,7 @@  struct tegra_runtime_data {
 	struct tegra_dma_req dma_req[2];
 	struct tegra_dma_channel *dma_chan;
 };
+#endif
 
 int tegra_pcm_platform_register(struct device *dev);
 void tegra_pcm_platform_unregister(struct device *dev);