[alsa-devel] [PATCH 00/11] ASoC: OMAP: Convert to use dmaengine

newer
[alsa-devel] [PATCH] ASoC: ad193x:...

older
[alsa-devel] Invitation to connect...

Peter Ujfalusi

12 Sep 2012 12 Sep '12

1:46 p.m.

Hello,

This series will switch the OMAP audio to use dmaengine. The final patch which does the switch was based on Russell King's earlier patch.

The first 10 patch is to prepare the OMAP audio drivers for a smooth change to dmaengine: - sDMA FRAME sync mode is removed and replaced with PACKET mode - dai drivers no longer need to configure sDMA sync mode - dai drivers does not need to specify the DMA word length - with the exception of the omap-hdmi driver which requires 32bit word length regardless of the audio format in use - the McPDM driver used (to my surprise) hackish way of getting the DMA channel and address - via defines from some header files

I have tested the series on: - BeagleBoard (audio via McBSP) with aplay/arecord. In element mode and in threshold mode with different period sizes - OMAP4 Blaze (audio via McPDM and DMIC)

With this conversion the NO_PERIOD_WAKEUP is not supported at the moment. I'll be looking at the dmaengine core and omap parts to add this feature back.

The patches has been generated against: git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-3.7

Janusz: Can you retest this series on OMAP1 to be sure I have not broken it? Ricardo: Can you test the omap-hmdi if it is still working?

Regards, Peter --- Peter Ujfalusi (11): dmaengine: omap: Support for element mode in cyclic DMA ASoC: omap-mcbsp: Use sDMA packet mode instead of frame mode ASoC: omap-pcm: Select sDMA synchronization based on packet_size ASoC: OMAP: Remove sync_mode from omap_pcm_dma_data struct ASoC: omap-pcm: Prepare to configure the DMA data_type based on stream properties ARM: OMAP4: hwmod_data: Add resource names to McPDM memory ranges ASoC: omap-mcpdm: Use platform_get_resource_* to get resources ASoC: OMAP: mcbsp, mcpdm, dmic: Let omap-pcm to pick the dma_type ASoC: omap-pcm, omap-hdmi: Change the use of omap_pcm_dma_data->data_type ASoC: OMAP: mcbsp, mcpdm, dmic, hdmi: Set dma_data at startup time ASoC: omap-pcm: Convert to use dmaengine

-- 1.7.12

Show replies by date

Peter Ujfalusi

12 Sep 12 Sep

1:46 p.m.

New subject: [alsa-devel] [PATCH 01/11] dmaengine: omap: Support for element mode in cyclic DMA

When src_maxburst/dst_maxburst is set to 0 by the users of cyclic DMA (mostly audio) indicates that we should configure the omap DMA to element sync mode instead of packet mode.

Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- drivers/dma/omap-dma.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c index ae05618..b77a40d 100644 --- a/drivers/dma/omap-dma.c +++ b/drivers/dma/omap-dma.c @@ -413,7 +413,10 @@ static struct dma_async_tx_descriptor *omap_dma_prep_dma_cyclic( d->dev_addr = dev_addr; d->fi = burst; d->es = es; - d->sync_mode = OMAP_DMA_SYNC_PACKET; + if (burst) + d->sync_mode = OMAP_DMA_SYNC_PACKET; + else + d->sync_mode = OMAP_DMA_SYNC_ELEMENT; d->sync_type = sync_type; d->periph_port = OMAP_DMA_PORT_MPUI; d->sg[0].addr = buf_addr;

-- 1.7.12

Peter Ujfalusi

1:46 p.m.

New subject: [alsa-devel] [PATCH 02/11] ASoC: omap-mcbsp: Use sDMA packet mode instead of frame mode

When McBSP is configured in threshold mode we can use sDMA packet mode in all cases.

Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/omap/omap-mcbsp.c | 47 ++++++++++++++++----------------------------- 1 file changed, 17 insertions(+), 30 deletions(-)

diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index 2e91a86..fe3debc 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -81,9 +81,6 @@ static void omap_mcbsp_set_threshold(struct snd_pcm_substream *substream) */ if (dma_data->packet_size) words = dma_data->packet_size; - else if (mcbsp->dma_op_mode == MCBSP_DMA_MODE_THRESHOLD) - words = snd_pcm_lib_period_bytes(substream) / - (mcbsp->wlen / 8); else words = 1;

@@ -251,6 +248,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, dma_data->set_threshold = omap_mcbsp_set_threshold; if (mcbsp->dma_op_mode == MCBSP_DMA_MODE_THRESHOLD) { int period_words, max_thrsh; + int divider = 0;

period_words = params_period_bytes(params) / (wlen / 8); if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) @@ -258,34 +256,23 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, else max_thrsh = mcbsp->max_rx_thres; /* - * If the period contains less or equal number of words, - * we are using the original threshold mode setup: - * McBSP threshold = sDMA frame size = period_size - * Otherwise we switch to sDMA packet mode: - * McBSP threshold = sDMA packet size - * sDMA frame size = period size + * Use sDMA packet mode if McBSP is in threshold mode: + * If period words less than the FIFO size the packet + * size is set to the number of period words, otherwise + * Look for the biggest threshold value which divides + * the period size evenly. */ - if (period_words > max_thrsh) { - int divider = 0; - - /* - * Look for the biggest threshold value, which - * divides the period size evenly. - */ - divider = period_words / max_thrsh; - if (period_words % max_thrsh) - divider++; - while (period_words % divider && - divider < period_words) - divider++; - if (divider == period_words) - return -EINVAL; - - pkt_size = period_words / divider; - sync_mode = OMAP_DMA_SYNC_PACKET; - } else { - sync_mode = OMAP_DMA_SYNC_FRAME; - } + divider = period_words / max_thrsh; + if (period_words % max_thrsh) + divider++; + while (period_words % divider && + divider < period_words) + divider++; + if (divider == period_words) + return -EINVAL; + + pkt_size = period_words / divider; + sync_mode = OMAP_DMA_SYNC_PACKET; } else if (channels > 1) { /* Use packet mode for non mono streams */ pkt_size = channels;

-- 1.7.12

Peter Ujfalusi

1:46 p.m.

New subject: [alsa-devel] [PATCH 03/11] ASoC: omap-pcm: Select sDMA synchronization based on packet_size

Since we only have element or packet synchronization we can use the dma_data->packet_size to select the desired mode: if packet_size is 0 we use ELEMENT mode if packet_size is not 0 we use PACKET mode for sDMA synchronization.

Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/omap/omap-pcm.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c index f0feb06..02eeb2e 100644 --- a/sound/soc/omap/omap-pcm.c +++ b/sound/soc/omap/omap-pcm.c @@ -165,7 +165,12 @@ static int omap_pcm_prepare(struct snd_pcm_substream *substream) memset(&dma_params, 0, sizeof(dma_params)); dma_params.data_type = dma_data->data_type; dma_params.trigger = dma_data->dma_req; - dma_params.sync_mode = dma_data->sync_mode; + + if (dma_data->packet_size) + dma_params.sync_mode = OMAP_DMA_SYNC_PACKET; + else + dma_params.sync_mode = OMAP_DMA_SYNC_ELEMENT; + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { dma_params.src_amode = OMAP_DMA_AMODE_POST_INC; dma_params.dst_amode = OMAP_DMA_AMODE_CONSTANT;

-- 1.7.12

Peter Ujfalusi

1:47 p.m.

New subject: [alsa-devel] [PATCH 04/11] ASoC: OMAP: Remove sync_mode from omap_pcm_dma_data struct

The omap-pcm platform driver no longer needs this parameter to select between ELEMENT and PACKET mode. The selection is based on the configured packet_size.

Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/omap/omap-dmic.c | 1 - sound/soc/omap/omap-hdmi.c | 1 - sound/soc/omap/omap-mcbsp.c | 5 +---- sound/soc/omap/omap-mcpdm.c | 2 -- sound/soc/omap/omap-pcm.h | 1 - 5 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/sound/soc/omap/omap-dmic.c b/sound/soc/omap/omap-dmic.c index 75f5dca..60b7b8c 100644 --- a/sound/soc/omap/omap-dmic.c +++ b/sound/soc/omap/omap-dmic.c @@ -64,7 +64,6 @@ struct omap_dmic { static struct omap_pcm_dma_data omap_dmic_dai_dma_params = { .name = "DMIC capture", .data_type = OMAP_DMA_DATA_TYPE_S32, - .sync_mode = OMAP_DMA_SYNC_PACKET, };

static inline void omap_dmic_write(struct omap_dmic *dmic, u16 reg, u32 val) diff --git a/sound/soc/omap/omap-hdmi.c b/sound/soc/omap/omap-hdmi.c index a08245d..b194646 100644 --- a/sound/soc/omap/omap-hdmi.c +++ b/sound/soc/omap/omap-hdmi.c @@ -290,7 +290,6 @@ static __devinit int omap_hdmi_probe(struct platform_device *pdev)

hdmi_data->dma_params.dma_req = hdmi_rsrc->start; hdmi_data->dma_params.name = "HDMI playback"; - hdmi_data->dma_params.sync_mode = OMAP_DMA_SYNC_PACKET;

/* * TODO: We assume that there is only one DSS HDMI device. Future diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index fe3debc..5b3bacc 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -225,7 +225,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, struct omap_mcbsp *mcbsp = snd_soc_dai_get_drvdata(cpu_dai); struct omap_mcbsp_reg_cfg *regs = &mcbsp->cfg_regs; struct omap_pcm_dma_data *dma_data; - int wlen, channels, wpf, sync_mode = OMAP_DMA_SYNC_ELEMENT; + int wlen, channels, wpf; int pkt_size = 0; unsigned int format, div, framesize, master;

@@ -272,15 +272,12 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, return -EINVAL;

pkt_size = period_words / divider; - sync_mode = OMAP_DMA_SYNC_PACKET; } else if (channels > 1) { /* Use packet mode for non mono streams */ pkt_size = channels; - sync_mode = OMAP_DMA_SYNC_PACKET; } }

- dma_data->sync_mode = sync_mode; dma_data->packet_size = pkt_size;

snd_soc_dai_set_dma_data(cpu_dai, substream, dma_data); diff --git a/sound/soc/omap/omap-mcpdm.c b/sound/soc/omap/omap-mcpdm.c index f7babb3..baf92da 100644 --- a/sound/soc/omap/omap-mcpdm.c +++ b/sound/soc/omap/omap-mcpdm.c @@ -73,14 +73,12 @@ static struct omap_pcm_dma_data omap_mcpdm_dai_dma_params[] = { .name = "Audio playback", .dma_req = OMAP44XX_DMA_MCPDM_DL, .data_type = OMAP_DMA_DATA_TYPE_S32, - .sync_mode = OMAP_DMA_SYNC_PACKET, .port_addr = OMAP44XX_MCPDM_L3_BASE + MCPDM_REG_DN_DATA, }, { .name = "Audio capture", .dma_req = OMAP44XX_DMA_MCPDM_UP, .data_type = OMAP_DMA_DATA_TYPE_S32, - .sync_mode = OMAP_DMA_SYNC_PACKET, .port_addr = OMAP44XX_MCPDM_L3_BASE + MCPDM_REG_UP_DATA, }, }; diff --git a/sound/soc/omap/omap-pcm.h b/sound/soc/omap/omap-pcm.h index b92248c..1bf47e4 100644 --- a/sound/soc/omap/omap-pcm.h +++ b/sound/soc/omap/omap-pcm.h @@ -33,7 +33,6 @@ struct omap_pcm_dma_data { unsigned long port_addr; /* transmit/receive register */ void (*set_threshold)(struct snd_pcm_substream *substream); int data_type; /* data type 8,16,32 */ - int sync_mode; /* DMA sync mode */ int packet_size; /* packet size only in PACKET mode */ };

-- 1.7.12

Peter Ujfalusi

1:47 p.m.

New subject: [alsa-devel] [PATCH 05/11] ASoC: omap-pcm: Prepare to configure the DMA data_type based on stream properties

Based on the format of the stream the omap-pcm can decide alone what data type should be used with by the sDMA. Keep the possibility for OMAP dai drivers to tell omap-pcm if they want to use different data type. This is needed for the omap-hdmi for example which needs 32bit data type even if the stream format is S16_LE.

The check if (dma_data->data_type) is safe at the moment since omap-pcm does not support 8bit samples (OMAP_DMA_DATA_TYPE_S8 == 0x00).

The next step is to redefine the meaning of dma_data->data_type to unblock this limitation.

Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/omap/omap-pcm.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c index 02eeb2e..4c13a5f 100644 --- a/sound/soc/omap/omap-pcm.c +++ b/sound/soc/omap/omap-pcm.c @@ -149,6 +149,24 @@ static int omap_pcm_hw_free(struct snd_pcm_substream *substream) return 0; }

+static int omap_pcm_get_dma_type(int num_bits) +{ + int data_type; + + switch (num_bits) { + case 16: + data_type = OMAP_DMA_DATA_TYPE_S16; + break; + case 32: + data_type = OMAP_DMA_DATA_TYPE_S32; + break; + default: + data_type = -EINVAL; + break; + } + return data_type; +} + static int omap_pcm_prepare(struct snd_pcm_substream *substream) { struct snd_pcm_runtime *runtime = substream->runtime; @@ -163,7 +181,16 @@ static int omap_pcm_prepare(struct snd_pcm_substream *substream) return 0;

memset(&dma_params, 0, sizeof(dma_params)); - dma_params.data_type = dma_data->data_type; + + if (dma_data->data_type) + dma_params.data_type = dma_data->data_type; + else + dma_params.data_type = omap_pcm_get_dma_type( + snd_pcm_format_physical_width(runtime->format)); + + if (dma_params.data_type < 0) + return dma_params.data_type; + dma_params.trigger = dma_data->dma_req;

if (dma_data->packet_size) @@ -195,7 +222,7 @@ static int omap_pcm_prepare(struct snd_pcm_substream *substream) * still can get an interrupt at each period bounary */ bytes = snd_pcm_lib_period_bytes(substream); - dma_params.elem_count = bytes >> dma_data->data_type; + dma_params.elem_count = bytes >> dma_params.data_type; dma_params.frame_count = runtime->periods; omap_set_dma_params(prtd->dma_ch, &dma_params);

-- 1.7.12

Peter Ujfalusi

1:47 p.m.

New subject: [alsa-devel] [PATCH 06/11] ARM: OMAP4: hwmod_data: Add resource names to McPDM memory ranges

To help the driver to get the correct memory range to access McPDM registers.

Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c index 242aee4..f65251e 100644 --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c @@ -5059,6 +5059,7 @@ static struct omap_hwmod_ocp_if omap44xx_l4_per__mcbsp4 = {

static struct omap_hwmod_addr_space omap44xx_mcpdm_addrs[] = { { + .name = "mpu", .pa_start = 0x40132000, .pa_end = 0x4013207f, .flags = ADDR_TYPE_RT @@ -5077,6 +5078,7 @@ static struct omap_hwmod_ocp_if omap44xx_l4_abe__mcpdm = {

static struct omap_hwmod_addr_space omap44xx_mcpdm_dma_addrs[] = { { + .name = "dma", .pa_start = 0x49032000, .pa_end = 0x4903207f, .flags = ADDR_TYPE_RT

-- 1.7.12

Peter Ujfalusi

1:47 p.m.

New subject: [alsa-devel] [PATCH 07/11] ASoC: omap-mcpdm: Use platform_get_resource_* to get resources

Get the needed resources in a correct way and avoid using defines for them.

Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/omap/omap-mcpdm.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/sound/soc/omap/omap-mcpdm.c b/sound/soc/omap/omap-mcpdm.c index baf92da..36648e0 100644 --- a/sound/soc/omap/omap-mcpdm.c +++ b/sound/soc/omap/omap-mcpdm.c @@ -41,7 +41,6 @@ #include <sound/soc.h>

#include <plat/dma.h> -#include <plat/omap_hwmod.h> #include "omap-mcpdm.h" #include "omap-pcm.h"

@@ -71,15 +70,11 @@ struct omap_mcpdm { static struct omap_pcm_dma_data omap_mcpdm_dai_dma_params[] = { { .name = "Audio playback", - .dma_req = OMAP44XX_DMA_MCPDM_DL, .data_type = OMAP_DMA_DATA_TYPE_S32, - .port_addr = OMAP44XX_MCPDM_L3_BASE + MCPDM_REG_DN_DATA, }, { .name = "Audio capture", - .dma_req = OMAP44XX_DMA_MCPDM_UP, .data_type = OMAP_DMA_DATA_TYPE_S32, - .port_addr = OMAP44XX_MCPDM_L3_BASE + MCPDM_REG_UP_DATA, }, };

@@ -452,10 +447,33 @@ static __devinit int asoc_mcpdm_probe(struct platform_device *pdev)

mutex_init(&mcpdm->mutex);

+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dma"); + if (res == NULL) + return -ENOMEM; + + omap_mcpdm_dai_dma_params[0].port_addr = res->start + MCPDM_REG_DN_DATA; + omap_mcpdm_dai_dma_params[1].port_addr = res->start + MCPDM_REG_UP_DATA; + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (res == NULL) return -ENOMEM;

+ res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "dn_link"); + if (!res) + return -ENODEV; + + omap_mcpdm_dai_dma_params[0].dma_req = res->start; + + res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "up_link"); + if (!res) + return -ENODEV; + + omap_mcpdm_dai_dma_params[1].dma_req = res->start; + + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mpu"); + if (res == NULL) + return -ENOMEM; + if (!devm_request_mem_region(&pdev->dev, res->start, resource_size(res), "McPDM")) return -EBUSY;

-- 1.7.12

Peter Ujfalusi

1:47 p.m.

New subject: [alsa-devel] [PATCH 08/11] ASoC: OMAP: mcbsp, mcpdm, dmic: Let omap-pcm to pick the dma_type

omap-pcm can figure out the correct dma_type based on the stream's format. In this way we can get rid of the plat/dma.h include from these drivers.

Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/omap/omap-dmic.c | 2 -- sound/soc/omap/omap-mcbsp.c | 3 --- sound/soc/omap/omap-mcpdm.c | 3 --- 3 files changed, 8 deletions(-)

diff --git a/sound/soc/omap/omap-dmic.c b/sound/soc/omap/omap-dmic.c index 60b7b8c..df0ff24 100644 --- a/sound/soc/omap/omap-dmic.c +++ b/sound/soc/omap/omap-dmic.c @@ -33,7 +33,6 @@ #include <linux/slab.h> #include <linux/pm_runtime.h> #include <linux/of_device.h> -#include <plat/dma.h>

#include <sound/core.h> #include <sound/pcm.h> @@ -63,7 +62,6 @@ struct omap_dmic { */ static struct omap_pcm_dma_data omap_dmic_dai_dma_params = { .name = "DMIC capture", - .data_type = OMAP_DMA_DATA_TYPE_S32, };

static inline void omap_dmic_write(struct omap_dmic *dmic, u16 reg, u32 val) diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index 5b3bacc..a230646 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -34,7 +34,6 @@ #include <sound/initval.h> #include <sound/soc.h>

-#include <plat/dma.h> #include <plat/mcbsp.h> #include "mcbsp.h" #include "omap-mcbsp.h" @@ -234,11 +233,9 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream,

switch (params_format(params)) { case SNDRV_PCM_FORMAT_S16_LE: - dma_data->data_type = OMAP_DMA_DATA_TYPE_S16; wlen = 16; break; case SNDRV_PCM_FORMAT_S32_LE: - dma_data->data_type = OMAP_DMA_DATA_TYPE_S32; wlen = 32; break; default: diff --git a/sound/soc/omap/omap-mcpdm.c b/sound/soc/omap/omap-mcpdm.c index 36648e0..c1b4935 100644 --- a/sound/soc/omap/omap-mcpdm.c +++ b/sound/soc/omap/omap-mcpdm.c @@ -40,7 +40,6 @@ #include <sound/pcm_params.h> #include <sound/soc.h>

-#include <plat/dma.h> #include "omap-mcpdm.h" #include "omap-pcm.h"

@@ -70,11 +69,9 @@ struct omap_mcpdm { static struct omap_pcm_dma_data omap_mcpdm_dai_dma_params[] = { { .name = "Audio playback", - .data_type = OMAP_DMA_DATA_TYPE_S32, }, { .name = "Audio capture", - .data_type = OMAP_DMA_DATA_TYPE_S32, }, };

-- 1.7.12

Peter Ujfalusi

1:47 p.m.

New subject: [alsa-devel] [PATCH 09/11] ASoC: omap-pcm, omap-hdmi: Change the use of omap_pcm_dma_data->data_type

Instead of the OMAP DMA data type definition the data_type will be used to specify the number of bits the DMA word should be configured or 0 in case when based on the stream's format the omap-pcm can decide the needed DMA word size. This feature is needed for the omap-hdmi where the sDMA need to be configured for 32bit word type regardless of the audio format used.

Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/omap/omap-hdmi.c | 3 +-- sound/soc/omap/omap-pcm.c | 3 ++- sound/soc/omap/omap-pcm.h | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/sound/soc/omap/omap-hdmi.c b/sound/soc/omap/omap-hdmi.c index b194646..0951767 100644 --- a/sound/soc/omap/omap-hdmi.c +++ b/sound/soc/omap/omap-hdmi.c @@ -34,7 +34,6 @@ #include <sound/asoundef.h> #include <video/omapdss.h>

-#include <plat/dma.h> #include "omap-pcm.h" #include "omap-hdmi.h"

@@ -100,7 +99,7 @@ static int omap_hdmi_dai_hw_params(struct snd_pcm_substream *substream, return -EINVAL; }

- priv->dma_params.data_type = OMAP_DMA_DATA_TYPE_S32; + priv->dma_params.data_type = 32;

snd_soc_dai_set_dma_data(dai, substream, &priv->dma_params); diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c index 4c13a5f..74da4b7 100644 --- a/sound/soc/omap/omap-pcm.c +++ b/sound/soc/omap/omap-pcm.c @@ -183,7 +183,8 @@ static int omap_pcm_prepare(struct snd_pcm_substream *substream) memset(&dma_params, 0, sizeof(dma_params));

if (dma_data->data_type) - dma_params.data_type = dma_data->data_type; + dma_params.data_type = omap_pcm_get_dma_type( + dma_data->data_type); else dma_params.data_type = omap_pcm_get_dma_type( snd_pcm_format_physical_width(runtime->format)); diff --git a/sound/soc/omap/omap-pcm.h b/sound/soc/omap/omap-pcm.h index 1bf47e4..cabe74c 100644 --- a/sound/soc/omap/omap-pcm.h +++ b/sound/soc/omap/omap-pcm.h @@ -32,7 +32,8 @@ struct omap_pcm_dma_data { int dma_req; /* DMA request line */ unsigned long port_addr; /* transmit/receive register */ void (*set_threshold)(struct snd_pcm_substream *substream); - int data_type; /* data type 8,16,32 */ + int data_type; /* 8, 16, 32 (bits) or 0 to let omap-pcm + * to decide the sDMA data type */ int packet_size; /* packet size only in PACKET mode */ };

-- 1.7.12

Peter Ujfalusi

1:47 p.m.

New subject: [alsa-devel] [PATCH 10/11] ASoC: OMAP: mcbsp, mcpdm, dmic, hdmi: Set dma_data at startup time

Set the dma_data for the stream (snd_soc_dai_set_dma_data) at dai_startup time so omap-pcm will have access to the needed information regarding to the DMA channel earlier. This is needed for the clean dmaengine support.

Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/omap/omap-dmic.c | 6 ++++-- sound/soc/omap/omap-hdmi.c | 15 +++++++++------ sound/soc/omap/omap-mcbsp.c | 7 ++++--- sound/soc/omap/omap-mcpdm.c | 8 ++++---- 4 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/sound/soc/omap/omap-dmic.c b/sound/soc/omap/omap-dmic.c index df0ff24..68f2cd1 100644 --- a/sound/soc/omap/omap-dmic.c +++ b/sound/soc/omap/omap-dmic.c @@ -118,6 +118,7 @@ static int omap_dmic_dai_startup(struct snd_pcm_substream *substream,

mutex_unlock(&dmic->mutex);

+ snd_soc_dai_set_dma_data(dai, substream, &omap_dmic_dai_dma_params); return ret; }

@@ -202,6 +203,7 @@ static int omap_dmic_dai_hw_params(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai); + struct omap_pcm_dma_data *dma_data; int channels;

dmic->clk_div = omap_dmic_select_divider(dmic, params_rate(params)); @@ -227,8 +229,8 @@ static int omap_dmic_dai_hw_params(struct snd_pcm_substream *substream, }

/* packet size is threshold * channels */ - omap_dmic_dai_dma_params.packet_size = dmic->threshold * channels; - snd_soc_dai_set_dma_data(dai, substream, &omap_dmic_dai_dma_params); + dma_data = snd_soc_dai_get_dma_data(dai, substream); + dma_data->packet_size = dmic->threshold * channels;

return 0; } diff --git a/sound/soc/omap/omap-hdmi.c b/sound/soc/omap/omap-hdmi.c index 0951767..f59c69f 100644 --- a/sound/soc/omap/omap-hdmi.c +++ b/sound/soc/omap/omap-hdmi.c @@ -67,6 +67,9 @@ static int omap_hdmi_dai_startup(struct snd_pcm_substream *substream, dev_err(dai->dev, "audio not supported\n"); return -ENODEV; } + + snd_soc_dai_set_dma_data(dai, substream, &priv->dma_params); + return 0; }

@@ -85,24 +88,24 @@ static int omap_hdmi_dai_hw_params(struct snd_pcm_substream *substream, struct hdmi_priv *priv = snd_soc_dai_get_drvdata(dai); struct snd_aes_iec958 *iec = &priv->iec; struct snd_cea_861_aud_if *cea = &priv->cea; + struct omap_pcm_dma_data *dma_data; int err = 0;

+ dma_data = snd_soc_dai_get_dma_data(dai, substream); + switch (params_format(params)) { case SNDRV_PCM_FORMAT_S16_LE: - priv->dma_params.packet_size = 16; + dma_data->packet_size = 16; break; case SNDRV_PCM_FORMAT_S24_LE: - priv->dma_params.packet_size = 32; + dma_data->packet_size = 32; break; default: dev_err(dai->dev, "format not supported!\n"); return -EINVAL; }

- priv->dma_params.data_type = 32; - - snd_soc_dai_set_dma_data(dai, substream, - &priv->dma_params); + dma_data->data_type = 32;

/* * fill the IEC-60958 channel status word diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index a230646..fef2f59 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -151,6 +151,9 @@ static int omap_mcbsp_dai_startup(struct snd_pcm_substream *substream, SNDRV_PCM_HW_PARAM_PERIOD_SIZE, 2); }

+ snd_soc_dai_set_dma_data(cpu_dai, substream, + &mcbsp->dma_data[substream->stream]); + return err; }

@@ -228,7 +231,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, int pkt_size = 0; unsigned int format, div, framesize, master;

- dma_data = &mcbsp->dma_data[substream->stream]; + dma_data = snd_soc_dai_get_dma_data(cpu_dai, substream); channels = params_channels(params);

switch (params_format(params)) { @@ -277,8 +280,6 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream,

dma_data->packet_size = pkt_size;

- snd_soc_dai_set_dma_data(cpu_dai, substream, dma_data); - if (mcbsp->configured) { /* McBSP already configured by another stream */ return 0; diff --git a/sound/soc/omap/omap-mcpdm.c b/sound/soc/omap/omap-mcpdm.c index c1b4935..060cd16 100644 --- a/sound/soc/omap/omap-mcpdm.c +++ b/sound/soc/omap/omap-mcpdm.c @@ -266,9 +266,11 @@ static int omap_mcpdm_dai_startup(struct snd_pcm_substream *substream, } omap_mcpdm_open_streams(mcpdm); } - mutex_unlock(&mcpdm->mutex);

+ snd_soc_dai_set_dma_data(dai, substream, + &omap_mcpdm_dai_dma_params[substream->stream]); + return 0; }

@@ -323,7 +325,7 @@ static int omap_mcpdm_dai_hw_params(struct snd_pcm_substream *substream, return -EINVAL; }

- dma_data = &omap_mcpdm_dai_dma_params[stream]; + dma_data = snd_soc_dai_get_dma_data(dai, substream);

/* Configure McPDM channels, and DMA packet size */ if (stream == SNDRV_PCM_STREAM_PLAYBACK) { @@ -335,8 +337,6 @@ static int omap_mcpdm_dai_hw_params(struct snd_pcm_substream *substream, dma_data->packet_size = mcpdm->up_threshold * channels; }

- snd_soc_dai_set_dma_data(dai, substream, dma_data); - return 0; }

-- 1.7.12

Peter Ujfalusi

1:47 p.m.

New subject: [alsa-devel] [PATCH 11/11] ASoC: omap-pcm: Convert to use dmaengine

Original author: Russell King rmk+kernel@arm.linux.org.uk

Switch the omap-pcm to use dmaengine. Certain features are not supported by after dmaengine conversion: 1. No period wakeup mode DMA engine has no way to communicate this information through standard channels.

2. Pause/Resume OMAP DMA engine backend does not support pausing and resuming an in-progress transfer. It is unclear from the specs what effect clearing the enable bit has on the DMA position of a destination synchronized transfer, and whether the transfer can be restarted from the exact point that it was paused (or whether the data in the FIFO read from memory is simply discarded.)

Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com CC: Russell King rmk+kernel@arm.linux.org.uk --- sound/soc/omap/Kconfig | 3 +- sound/soc/omap/omap-pcm.c | 279 +++++++++++----------------------------------- 2 files changed, 67 insertions(+), 215 deletions(-)

diff --git a/sound/soc/omap/Kconfig b/sound/soc/omap/Kconfig index 2c484a5..7048137 100644 --- a/sound/soc/omap/Kconfig +++ b/sound/soc/omap/Kconfig @@ -1,6 +1,7 @@ config SND_OMAP_SOC tristate "SoC Audio for the Texas Instruments OMAP chips" - depends on ARCH_OMAP + depends on ARCH_OMAP && DMA_OMAP + select SND_SOC_DMAENGINE_PCM

config SND_OMAP_SOC_DMIC tristate diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c index 74da4b7..eb68c9e 100644 --- a/sound/soc/omap/omap-pcm.c +++ b/sound/soc/omap/omap-pcm.c @@ -25,21 +25,24 @@ #include <linux/dma-mapping.h> #include <linux/slab.h> #include <linux/module.h> +#include <linux/omap-dma.h> #include <sound/core.h> #include <sound/pcm.h> #include <sound/pcm_params.h> +#include <sound/dmaengine_pcm.h> #include <sound/soc.h>

-#include <plat/dma.h> #include "omap-pcm.h"

static const struct snd_pcm_hardware omap_pcm_hardware = { .info = SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID | - SNDRV_PCM_INFO_INTERLEAVED | - SNDRV_PCM_INFO_PAUSE | - SNDRV_PCM_INFO_RESUME | - SNDRV_PCM_INFO_NO_PERIOD_WAKEUP, + SNDRV_PCM_INFO_INTERLEAVED, + /* + * TODO: support for + * SNDRV_PCM_INFO_NO_PERIOD_WAKEUP + * via dmaengine. + */ .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S32_LE, .period_bytes_min = 32, @@ -49,61 +52,34 @@ static const struct snd_pcm_hardware omap_pcm_hardware = { .buffer_bytes_max = 128 * 1024, };

-struct omap_runtime_data { - spinlock_t lock; - struct omap_pcm_dma_data *dma_data; - int dma_ch; - int period_index; -}; - -static void omap_pcm_dma_irq(int ch, u16 stat, void *data) +static int omap_pcm_get_dma_buswidth(int num_bits) { - struct snd_pcm_substream *substream = data; - struct snd_pcm_runtime *runtime = substream->runtime; - struct omap_runtime_data *prtd = runtime->private_data; - unsigned long flags; - - if ((cpu_is_omap1510())) { - /* - * OMAP1510 doesn't fully support DMA progress counter - * and there is no software emulation implemented yet, - * so have to maintain our own progress counters - * that can be used by omap_pcm_pointer() instead. - */ - spin_lock_irqsave(&prtd->lock, flags); - if ((stat == OMAP_DMA_LAST_IRQ) && - (prtd->period_index == runtime->periods - 1)) { - /* we are in sync, do nothing */ - spin_unlock_irqrestore(&prtd->lock, flags); - return; - } - if (prtd->period_index >= 0) { - if (stat & OMAP_DMA_BLOCK_IRQ) { - /* end of buffer reached, loop back */ - prtd->period_index = 0; - } else if (stat & OMAP_DMA_LAST_IRQ) { - /* update the counter for the last period */ - prtd->period_index = runtime->periods - 1; - } else if (++prtd->period_index >= runtime->periods) { - /* end of buffer missed? loop back */ - prtd->period_index = 0; - } - } - spin_unlock_irqrestore(&prtd->lock, flags); - } + int buswidth;

- snd_pcm_period_elapsed(substream); + switch (num_bits) { + case 16: + buswidth = DMA_SLAVE_BUSWIDTH_2_BYTES; + break; + case 32: + buswidth = DMA_SLAVE_BUSWIDTH_4_BYTES; + break; + default: + buswidth = -EINVAL; + break; + } + return buswidth; }

+ /* this may get called several times by oss emulation */ static int omap_pcm_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { struct snd_pcm_runtime *runtime = substream->runtime; struct snd_soc_pcm_runtime *rtd = substream->private_data; - struct omap_runtime_data *prtd = runtime->private_data; struct omap_pcm_dma_data *dma_data; - + struct dma_slave_config config; + struct dma_chan *chan; int err = 0;

dma_data = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream); @@ -116,195 +92,78 @@ static int omap_pcm_hw_params(struct snd_pcm_substream *substream, snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer); runtime->dma_bytes = params_buffer_bytes(params);

- if (prtd->dma_data) - return 0; - prtd->dma_data = dma_data; - err = omap_request_dma(dma_data->dma_req, dma_data->name, - omap_pcm_dma_irq, substream, &prtd->dma_ch); - if (!err) { - /* - * Link channel with itself so DMA doesn't need any - * reprogramming while looping the buffer - */ - omap_dma_link_lch(prtd->dma_ch, prtd->dma_ch); - } - - return err; -} - -static int omap_pcm_hw_free(struct snd_pcm_substream *substream) -{ - struct snd_pcm_runtime *runtime = substream->runtime; - struct omap_runtime_data *prtd = runtime->private_data; - - if (prtd->dma_data == NULL) - return 0; + chan = snd_dmaengine_pcm_get_chan(substream); + if (!chan) + return -EINVAL;

- omap_dma_unlink_lch(prtd->dma_ch, prtd->dma_ch); - omap_free_dma(prtd->dma_ch); - prtd->dma_data = NULL; + /* fills in addr_width and direction */ + err = snd_hwparams_to_dma_slave_config(substream, params, &config); + if (err) + return err;

- snd_pcm_set_runtime_buffer(substream, NULL); + /* Override the *_dma addr_width if requested by the DAI driver */ + if (dma_data->data_type) { + int buswidth = omap_pcm_get_dma_buswidth(dma_data->data_type);

- return 0; -} + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) + config.dst_addr_width = buswidth; + else + config.src_addr_width = buswidth; + }

-static int omap_pcm_get_dma_type(int num_bits) -{ - int data_type; + config.src_addr = dma_data->port_addr; + config.dst_addr = dma_data->port_addr; + config.src_maxburst = dma_data->packet_size; + config.dst_maxburst = dma_data->packet_size;

- switch (num_bits) { - case 16: - data_type = OMAP_DMA_DATA_TYPE_S16; - break; - case 32: - data_type = OMAP_DMA_DATA_TYPE_S32; - break; - default: - data_type = -EINVAL; - break; - } - return data_type; + return dmaengine_slave_config(chan, &config); }

-static int omap_pcm_prepare(struct snd_pcm_substream *substream) +static int omap_pcm_hw_free(struct snd_pcm_substream *substream) { - struct snd_pcm_runtime *runtime = substream->runtime; - struct omap_runtime_data *prtd = runtime->private_data; - struct omap_pcm_dma_data *dma_data = prtd->dma_data; - struct omap_dma_channel_params dma_params; - int bytes; - - /* return if this is a bufferless transfer e.g. - * codec <--> BT codec or GSM modem -- lg FIXME */ - if (!prtd->dma_data) - return 0; - - memset(&dma_params, 0, sizeof(dma_params)); - - if (dma_data->data_type) - dma_params.data_type = omap_pcm_get_dma_type( - dma_data->data_type); - else - dma_params.data_type = omap_pcm_get_dma_type( - snd_pcm_format_physical_width(runtime->format)); - - if (dma_params.data_type < 0) - return dma_params.data_type; - - dma_params.trigger = dma_data->dma_req; - - if (dma_data->packet_size) - dma_params.sync_mode = OMAP_DMA_SYNC_PACKET; - else - dma_params.sync_mode = OMAP_DMA_SYNC_ELEMENT; - - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { - dma_params.src_amode = OMAP_DMA_AMODE_POST_INC; - dma_params.dst_amode = OMAP_DMA_AMODE_CONSTANT; - dma_params.src_or_dst_synch = OMAP_DMA_DST_SYNC; - dma_params.src_start = runtime->dma_addr; - dma_params.dst_start = dma_data->port_addr; - dma_params.dst_port = OMAP_DMA_PORT_MPUI; - dma_params.dst_fi = dma_data->packet_size; - } else { - dma_params.src_amode = OMAP_DMA_AMODE_CONSTANT; - dma_params.dst_amode = OMAP_DMA_AMODE_POST_INC; - dma_params.src_or_dst_synch = OMAP_DMA_SRC_SYNC; - dma_params.src_start = dma_data->port_addr; - dma_params.dst_start = runtime->dma_addr; - dma_params.src_port = OMAP_DMA_PORT_MPUI; - dma_params.src_fi = dma_data->packet_size; - } - /* - * Set DMA transfer frame size equal to ALSA period size and frame - * count as no. of ALSA periods. Then with DMA frame interrupt enabled, - * we can transfer the whole ALSA buffer with single DMA transfer but - * still can get an interrupt at each period bounary - */ - bytes = snd_pcm_lib_period_bytes(substream); - dma_params.elem_count = bytes >> dma_params.data_type; - dma_params.frame_count = runtime->periods; - omap_set_dma_params(prtd->dma_ch, &dma_params); - - if ((cpu_is_omap1510())) - omap_enable_dma_irq(prtd->dma_ch, OMAP_DMA_FRAME_IRQ | - OMAP_DMA_LAST_IRQ | OMAP_DMA_BLOCK_IRQ); - else if (!substream->runtime->no_period_wakeup) - omap_enable_dma_irq(prtd->dma_ch, OMAP_DMA_FRAME_IRQ); - else { - /* - * No period wakeup: - * we need to disable BLOCK_IRQ, which is enabled by the omap - * dma core at request dma time. - */ - omap_disable_dma_irq(prtd->dma_ch, OMAP_DMA_BLOCK_IRQ); - } - - if (!(cpu_class_is_omap1())) { - omap_set_dma_src_burst_mode(prtd->dma_ch, - OMAP_DMA_DATA_BURST_16); - omap_set_dma_dest_burst_mode(prtd->dma_ch, - OMAP_DMA_DATA_BURST_16); - } - + snd_pcm_set_runtime_buffer(substream, NULL); return 0; }

static int omap_pcm_trigger(struct snd_pcm_substream *substream, int cmd) { - struct snd_pcm_runtime *runtime = substream->runtime; - struct omap_runtime_data *prtd = runtime->private_data; - struct omap_pcm_dma_data *dma_data = prtd->dma_data; - unsigned long flags; + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct omap_pcm_dma_data *dma_data; int ret = 0;

- spin_lock_irqsave(&prtd->lock, flags); + dma_data = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream); + switch (cmd) { case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: - prtd->period_index = 0; /* Configure McBSP internal buffer usage */ if (dma_data->set_threshold) dma_data->set_threshold(substream); - - omap_start_dma(prtd->dma_ch); break;

case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_PAUSE_PUSH: - prtd->period_index = -1; - omap_stop_dma(prtd->dma_ch); break; default: ret = -EINVAL; } - spin_unlock_irqrestore(&prtd->lock, flags); + + if (ret == 0) + ret = snd_dmaengine_pcm_trigger(substream, cmd);

return ret; }

static snd_pcm_uframes_t omap_pcm_pointer(struct snd_pcm_substream *substream) { - struct snd_pcm_runtime *runtime = substream->runtime; - struct omap_runtime_data *prtd = runtime->private_data; - dma_addr_t ptr; snd_pcm_uframes_t offset;

- if (cpu_is_omap1510()) { - offset = prtd->period_index * runtime->period_size; - } else if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) { - ptr = omap_get_dma_dst_pos(prtd->dma_ch); - offset = bytes_to_frames(runtime, ptr - runtime->dma_addr); - } else { - ptr = omap_get_dma_src_pos(prtd->dma_ch); - offset = bytes_to_frames(runtime, ptr - runtime->dma_addr); - } - - if (offset >= runtime->buffer_size) - offset = 0; + if (cpu_is_omap1510()) + offset = snd_dmaengine_pcm_pointer_no_residue(substream); + else + offset = snd_dmaengine_pcm_pointer(substream);

return offset; } @@ -312,7 +171,8 @@ static snd_pcm_uframes_t omap_pcm_pointer(struct snd_pcm_substream *substream) static int omap_pcm_open(struct snd_pcm_substream *substream) { struct snd_pcm_runtime *runtime = substream->runtime; - struct omap_runtime_data *prtd; + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct omap_pcm_dma_data *dma_data; int ret;

snd_soc_set_runtime_hwparams(substream, &omap_pcm_hardware); @@ -321,25 +181,17 @@ static int omap_pcm_open(struct snd_pcm_substream *substream) ret = snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS); if (ret < 0) - goto out; - - prtd = kzalloc(sizeof(*prtd), GFP_KERNEL); - if (prtd == NULL) { - ret = -ENOMEM; - goto out; - } - spin_lock_init(&prtd->lock); - runtime->private_data = prtd; + return ret;

-out: + dma_data = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream); + ret = snd_dmaengine_pcm_open(substream, omap_dma_filter_fn, + &dma_data->dma_req); return ret; }

static int omap_pcm_close(struct snd_pcm_substream *substream) { - struct snd_pcm_runtime *runtime = substream->runtime; - - kfree(runtime->private_data); + snd_dmaengine_pcm_close(substream); return 0; }

@@ -360,7 +212,6 @@ static struct snd_pcm_ops omap_pcm_ops = { .ioctl = snd_pcm_lib_ioctl, .hw_params = omap_pcm_hw_params, .hw_free = omap_pcm_hw_free, - .prepare = omap_pcm_prepare, .trigger = omap_pcm_trigger, .pointer = omap_pcm_pointer, .mmap = omap_pcm_mmap,

-- 1.7.12

Russell King - ARM Linux

2 p.m.

New subject: [alsa-devel] [PATCH 11/11] ASoC: omap-pcm: Convert to use dmaengine

On Wed, Sep 12, 2012 at 02:47:07PM +0300, Peter Ujfalusi wrote:

...

Pause/Resume OMAP DMA engine backend does not support pausing and resuming an in-progress transfer. It is unclear from the specs what effect clearing the enable bit has on the DMA position of a destination synchronized transfer, and whether the transfer can be restarted from the exact point that it was paused (or whether the data in the FIFO read from memory is simply discarded.)

It's worth noting that this comment (which was in my original patch) is there to spark _comment_ and _discussion_ and should not make its way into the final version of these patches.

Given that suspend/resume is important on OMAP platforms, it's something that needs to be resolved - in a way that complies with what ALSA expects. I do not believe that the way the existing drivers do this is compliant as the manuals imply that stopping memory->peripheral transfers results in data being discarded from the DMA's FIFOs. As I understand it, ALSA requires no data to be discarded.

As we have no way to know how much data may be discarded from the DMA FIFO...

Peter Ujfalusi

2:53 p.m.

New subject: [alsa-devel] [PATCH 11/11] ASoC: omap-pcm: Convert to use dmaengine

On 09/12/2012 03:00 PM, Russell King - ARM Linux wrote:

...

On Wed, Sep 12, 2012 at 02:47:07PM +0300, Peter Ujfalusi wrote:

...

Pause/Resume OMAP DMA engine backend does not support pausing and resuming an in-progress transfer. It is unclear from the specs what effect clearing the enable bit has on the DMA position of a destination synchronized transfer, and whether the transfer can be restarted from the exact point that it was paused (or whether the data in the FIFO read from memory is simply discarded.)

It's worth noting that this comment (which was in my original patch) is there to spark _comment_ and _discussion_ and should not make its way into the final version of these patches.

Given that suspend/resume is important on OMAP platforms, it's something that needs to be resolved - in a way that complies with what ALSA expects. I do not believe that the way the existing drivers do this is compliant as the manuals imply that stopping memory->peripheral transfers results in data being discarded from the DMA's FIFOs. As I understand it, ALSA requires no data to be discarded.

As we have no way to know how much data may be discarded from the DMA FIFO...

I need to look at this, but at first look we do wait for the drain in omap_stop_dma(). We used to use omap_stop_dma/omap_start_dma for pause/resume operations. But sDMA also have a bit: CDPi: PAUSE_LINK_LIST which should do what we are looking for. Need to read the relevant parts of the TRM, but AFAIK we are using normal mode with linked list (self linking). I already have the patch for this, I just need to test it on HW.

-- Péter

Peter Ujfalusi

4:35 p.m.

New subject: [alsa-devel] [PATCH 11/11] ASoC: omap-pcm: Convert to use dmaengine

On 09/12/2012 03:53 PM, Peter Ujfalusi wrote:

...

I need to look at this, but at first look we do wait for the drain in omap_stop_dma(). We used to use omap_stop_dma/omap_start_dma for pause/resume operations. But sDMA also have a bit: CDPi: PAUSE_LINK_LIST which should do what we are looking for. Need to read the relevant parts of the TRM, but AFAIK we are using normal mode with linked list (self linking). I already have the patch for this, I just need to test it on HW.

We can get the PAUSE/RESUME work either: 1. drivers/dma/omap-dma.c static int omap_dma_pause(struct omap_chan *c) { - /* FIXME: not supported by platform private API */ - return -EINVAL; + /* Only in cyclic mode */ + if (!c->cyclic) + return -EINVAL; + + if (!c->paused) { + omap_stop_dma(c->dma_ch); + c->paused = true; + } + + return 0; }

static int omap_dma_resume(struct omap_chan *c) { - /* FIXME: not supported by platform private API */ - return -EINVAL; + /* Only in cyclic mode */ + if (!c->cyclic) + return -EINVAL; + + if (c->paused) { + omap_start_dma(c->dma_ch); + c->paused = false; + } + + return 0; }

2. arch/arm/plat-omap/dma.c +void omap_pause_linked_dma(int lch) +{ + u32 l; + unsigned long flags; + + local_irq_save(flags); + l = p->dma_read(CDP, lch); + l |= 0x80; /* PAUSE_LINK_LIST */ + p->dma_write(l, CDP, lch); + local_irq_restore(flags); +} +EXPORT_SYMBOL(omap_pause_linked_dma); + +void omap_resume_linked_dma(int lch) +{ + u32 l; + unsigned long flags; + + local_irq_save(flags); + l = p->dma_read(CDP, lch); + l &= (~0x80); /* PAUSE_LINK_LIST */ + p->dma_write(l, CDP, lch); + local_irq_restore(flags); +} +EXPORT_SYMBOL(omap_resume_linked_dma);

drivers/dma/omap-dma.c static int omap_dma_pause(struct omap_chan *c) { - /* FIXME: not supported by platform private API */ - return -EINVAL; + /* Only in cyclic mode */ + if (!c->cyclic) + return -EINVAL; + + if (!c->paused) { + omap_pause_linked_dma(c->dma_ch); + c->paused = true; + } + + return 0; }

static int omap_dma_resume(struct omap_chan *c) { - /* FIXME: not supported by platform private API */ - return -EINVAL; + /* Only in cyclic mode */ + if (!c->cyclic) + return -EINVAL; + + if (c->paused) { + omap_resume_linked_dma(c->dma_ch); + c->paused = false; + } + + return 0; }

Both works fine (have tested it on BeagleBoard with mplayer). With [1] we have identical way of dealing with the PAUSE/RESUME as we had previously, so it is know to work fine on OMAP1/2/3/4/5

We have never used [2] in the past, but I think that is not going to work on OMAP1 and OMAP2420. Also we need to add code to arch/arm/plat-omap/dma.c for this...

I can resend the series with either of these to fix this regression caused by the dmaengine conversion (after cleanup off course).

-- Péter

Takashi Iwai

4:05 p.m.

New subject: [alsa-devel] [PATCH 11/11] ASoC: omap-pcm: Convert to use dmaengine

At Wed, 12 Sep 2012 13:00:28 +0100, Russell King - ARM Linux wrote:

...

On Wed, Sep 12, 2012 at 02:47:07PM +0300, Peter Ujfalusi wrote:

...

Pause/Resume OMAP DMA engine backend does not support pausing and resuming an in-progress transfer. It is unclear from the specs what effect clearing the enable bit has on the DMA position of a destination synchronized transfer, and whether the transfer can be restarted from the exact point that it was paused (or whether the data in the FIFO read from memory is simply discarded.)

It's worth noting that this comment (which was in my original patch) is there to spark _comment_ and _discussion_ and should not make its way into the final version of these patches.

I agree this can be regarded as a sort of regression...

...

Given that suspend/resume is important on OMAP platforms, it's something that needs to be resolved - in a way that complies with what ALSA expects. I do not believe that the way the existing drivers do this is compliant as the manuals imply that stopping memory->peripheral transfers results in data being discarded from the DMA's FIFOs. As I understand it, ALSA requires no data to be discarded.

... but in general you don't need to implement the full PAUSE and RESUME operations for PCM. For example, when SNDRV_PCM_INFO_RESUME bit isn't set, user-space is supposed to do a clean restart of the stream at wake up. So, it shouldn't break the apps severely in theory (but in practice, who knows :)

...

As we have no way to know how much data may be discarded from the DMA FIFO...

Yeah, the FIFO and the amount of other on flight data can be better managed.

Takashi

Mark Brown

13 Sep 13 Sep

10:11 a.m.

On Wed, Sep 12, 2012 at 02:46:56PM +0300, Peter Ujfalusi wrote:

...

Hello,

This series will switch the OMAP audio to use dmaengine. The final patch which does the switch was based on Russell King's earlier patch.

I'm fine with this from the ASoC side but it sounds like you're going to respin anyway. Are the earlier bits of the series safe to apply without the last bit, it seems like that's the only bit that really needs a respin so we may as well go ahead and apply the earlier bits now?

Peter Ujfalusi

11:20 a.m.

On 09/13/2012 11:11 AM, Mark Brown wrote:

...

On Wed, Sep 12, 2012 at 02:46:56PM +0300, Peter Ujfalusi wrote:

...
Hello,

This series will switch the OMAP audio to use dmaengine. The final patch which does the switch was based on Russell King's earlier patch.

I'm fine with this from the ASoC side but it sounds like you're going to respin anyway. Are the earlier bits of the series safe to apply without the last bit, it seems like that's the only bit that really needs a respin so we may as well go ahead and apply the earlier bits now?

Yes, I'm preparing the second series (adding the pause/resume support). Patch 2-10 is to prepare the OMAP audio drivers for the dmaengine conversion they can be applied earlier IMHO. I can in turn can send only dmaengine related patches in v2 (patch 1 and 10 from this series and additional ones). Anyways I need some time to figure out how to add back the support for SNDRV_PCM_INFO_NO_PERIOD_WAKEUP. Either way is good for me.

-- Péter

4484

Age (days ago)

4485

Last active (days ago)

List overview

Download

17 comments

4 participants

tags (0)

participants (4)

Mark Brown
Peter Ujfalusi
Russell King - ARM Linux
Takashi Iwai