[PATCH] [RFC] ASoC: OMAP: fix OMAP1510 broken PCM pointer callback

[PATCH] [RFC] ASoC: OMAP: fix OMAP1510 broken PCM pointer callback

[Janusz Krzysztofik]

This patch tries to work around the problem of broken OMAP1510 PCM playback 
pointer calculation by replacing DMA function call that incorrectly tries to 
read the value form DMA hardware with a value computed locally from an 
already maintained variable omap_runtime_data.period_index.

Tested on OMAP5910 based Amstrad Delta (E3) using work in progress ASoC 
driver.

Based on linux-2.6-asoc.git v2.6.31-rc1.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
---
It seems that on OMAP1510, DMA Channel Progress Counter registers 
(DMA_CPC_CH[0-8]) always contain values derived from DMA channels destination 
port address, even if constant, and there are no DMA registers available that 
would follow DMA channels source port address. Because of this limitation, 
current implementation of omap_get_dma_src_pos() for OMAP1510 is broken and 
doesn't seem to be easy correctable.

--- linux-2.6.31-rc1/sound/soc/omap/omap-pcm.c.orig	2009-06-27 
20:20:16.000000000 +0200
+++ linux-2.6.31-rc1/sound/soc/omap/omap-pcm.c	2009-06-27 23:21:42.000000000 
+0200
@@ -216,12 +216,15 @@ static snd_pcm_uframes_t omap_pcm_pointe
 	dma_addr_t ptr;
 	snd_pcm_uframes_t offset;
 
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-		ptr = omap_get_dma_src_pos(prtd->dma_ch);
-	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 if (!(cpu_is_omap1510())) {
+		ptr = omap_get_dma_src_pos(prtd->dma_ch);
+		offset = bytes_to_frames(runtime, ptr - runtime->dma_addr);
+	} else
+		offset = prtd->period_index * runtime->period_size;
 
-	offset = bytes_to_frames(runtime, ptr - runtime->dma_addr);
 	if (offset >= runtime->buffer_size)
 		offset = 0;
 

Re: [PATCH] [RFC] ASoC: OMAP: fix OMAP1510 broken PCM pointer callback

[Jarkko Nikula]

On Sun, 28 Jun 2009 00:21:05 +0200
Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:

> This patch tries to work around the problem of broken OMAP1510 PCM
> playback pointer calculation by replacing DMA function call that
> incorrectly tries to read the value form DMA hardware with a value
> computed locally from an already maintained variable
> omap_runtime_data.period_index.
> 
> Tested on OMAP5910 based Amstrad Delta (E3) using work in progress
> ASoC driver.
> 
> Based on linux-2.6-asoc.git v2.6.31-rc1.
> 
> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> ---
> It seems that on OMAP1510, DMA Channel Progress Counter registers 
> (DMA_CPC_CH[0-8]) always contain values derived from DMA channels
> destination port address, even if constant, and there are no DMA
> registers available that would follow DMA channels source port
> address. Because of this limitation, current implementation of
> omap_get_dma_src_pos() for OMAP1510 is broken and doesn't seem to be
> easy correctable.
> 
Hi

Before going into workaround, did you try to change function
omap_get_dma_src_pos to read CSSA_L instead of CPC in
arch/arm/plat-omap/dma.c that I was speculating in mail below?

http://mailman.alsa-project.org/pipermail/alsa-devel/2009-June/018569.html

While CPC seems to be correct [1], you mentioned before that patch below
broke the older ALSA driver so it's worth to find out if the problem
can be corrected by simple one line fix (or two if omap_get_dma_dst_pos
need to be fixed as well).

http://marc.info/?l=linux-omap&m=121280267705523


-- 
Jarkko

1. http://www.google.com/search?q=spru674

Re: [PATCH] [RFC] ASoC: OMAP: fix OMAP1510 broken PCM pointer callback

[Janusz Krzysztofik]

On Sunday 28 June 2009 21:37:32 Jarkko Nikula wrote:
> On Sun, 28 Jun 2009 00:21:05 +0200
> Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:
> > This patch tries to work around the problem of broken OMAP1510 PCM
> > playback pointer calculation by replacing DMA function call that
> > incorrectly tries to read the value form DMA hardware with a value
> > computed locally from an already maintained variable
> > omap_runtime_data.period_index.
> >
> > Tested on OMAP5910 based Amstrad Delta (E3) using work in progress
> > ASoC driver.
> >
> > Based on linux-2.6-asoc.git v2.6.31-rc1.
> >
> > Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> > ---
> > It seems that on OMAP1510, DMA Channel Progress Counter registers
> > (DMA_CPC_CH[0-8]) always contain values derived from DMA channels
> > destination port address, even if constant, and there are no DMA
> > registers available that would follow DMA channels source port
> > address. Because of this limitation, current implementation of
> > omap_get_dma_src_pos() for OMAP1510 is broken and doesn't seem to be
> > easy correctable.
>
> Hi
>
> Before going into workaround, did you try to change function
> omap_get_dma_src_pos to read CSSA_L instead of CPC in
> arch/arm/plat-omap/dma.c that I was speculating in mail below?
>
> http://mailman.alsa-project.org/pipermail/alsa-devel/2009-June/018569.html
>
> While CPC seems to be correct [1], you mentioned before that patch below
> broke the older ALSA driver so it's worth to find out if the problem
> can be corrected by simple one line fix (or two if omap_get_dma_dst_pos
> need to be fixed as well).
>
> http://marc.info/?l=linux-omap&m=121280267705523

Jarkko,

AFAIK, both CSSA_L and CDSA_L DMA registers are static. Loaded by CPU with 16 
LSB of initial source and destination port addresses respectively, they are 
never updated by the DMA engine itself. That's why they can't be used for 
transfer progress indication unless updated by CPU.

The old omap-alsa driver was just updating them, intentionally or not, by 
reprogramming and restarting DMA every PCM period. That's why calculating PCM 
pointers from CSSA_L/CDSA_L worked.

ASoC OMAP driver transfers whole PCM buffer with single DMA transfer, so it 
doesn't need to update DMA source/destination port address after initial 
playback/capture setup, even if restarting DMA, and actually never does this. 
Calculating PCM pointers from CSSA_L/CDSA_L registers without updating them 
every period would then be wrong.

For capture, reading CPC, that follows destination port address progress, just 
works fine (for both old and new driver). For playback, similar hardware 
functionality seems to be missing, so it has to be emulated in software if 
required.

Thanks,
Janusz

Re: [PATCH] [RFC] ASoC: OMAP: fix OMAP1510 broken PCM pointer callback

[Jarkko Nikula]

On Mon, 29 Jun 2009 00:08:59 +0200
Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:

> AFAIK, both CSSA_L and CDSA_L DMA registers are static. Loaded by CPU
> with 16 LSB of initial source and destination port addresses
> respectively, they are never updated by the DMA engine itself. That's
> why they can't be used for transfer progress indication unless
> updated by CPU.
> 
> The old omap-alsa driver was just updating them, intentionally or
> not, by reprogramming and restarting DMA every PCM period. That's why
> calculating PCM pointers from CSSA_L/CDSA_L worked.
> 
Thanks for finding out this. Good to know that OMAP low-level DMA code
now in this respect is as good as HW allows it.

> ASoC OMAP driver transfers whole PCM buffer with single DMA transfer,
> so it doesn't need to update DMA source/destination port address
> after initial playback/capture setup, even if restarting DMA, and
> actually never does this. Calculating PCM pointers from CSSA_L/CDSA_L
> registers without updating them every period would then be wrong.
> 
> For capture, reading CPC, that follows destination port address
> progress, just works fine (for both old and new driver). For
> playback, similar hardware functionality seems to be missing, so it
> has to be emulated in software if required.
> 
Kind an odd HW behaviour but that can happen. Probably it would be good
to explain this also in function omap_get_dma_src_pos.

Mark, I think this is fair to queue a fix for 2.6.31.

Acked-by: Jarkko Nikula <jhnikula@gmail.com>

Re: [PATCH] [RFC] ASoC: OMAP: fix OMAP1510 broken PCM pointer callback

[Peter Ujfalusi]

On Monday 29 June 2009 01:08:59 ext Janusz Krzysztofik wrote:
>
> AFAIK, both CSSA_L and CDSA_L DMA registers are static. Loaded by CPU with
> 16 LSB of initial source and destination port addresses respectively, they
> are never updated by the DMA engine itself. That's why they can't be used
> for transfer progress indication unless updated by CPU.
>
> The old omap-alsa driver was just updating them, intentionally or not, by
> reprogramming and restarting DMA every PCM period. That's why calculating
> PCM pointers from CSSA_L/CDSA_L worked.
>
> ASoC OMAP driver transfers whole PCM buffer with single DMA transfer, so it
> doesn't need to update DMA source/destination port address after initial
> playback/capture setup, even if restarting DMA, and actually never does
> this. Calculating PCM pointers from CSSA_L/CDSA_L registers without
> updating them every period would then be wrong.
>
> For capture, reading CPC, that follows destination port address progress,
> just works fine (for both old and new driver). For playback, similar
> hardware functionality seems to be missing, so it has to be emulated in
> software if required.

Hmmm, I had taken a look at the 2.4.21 kernel sources, which I have laying 
around in my disk from an old project which used OMAP1510. The OSS audio code 
does use the CPC register for determining the DMA progress both for playback 
and recording. I know that the audio was working OK on that board, since we 
had doom running there.
The difference that I can see is that the OSS code also configured the 
CCR:SYNC(4:0) bits as well.
Looking at the DMA_CPC register description in the OMAP1510 TRM: it list two 
cases on how it behaves and both require the DMA_CCR:SYNC != 0...

The current DMA code for OMAP1510 just plain ignores the DMA_CCR:SYNC for some 
reason.
Can you try the following patch:

iff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
index 7fc8c04..38874e4 100644
--- a/arch/arm/plat-omap/dma.c
+++ b/arch/arm/plat-omap/dma.c
@@ -266,6 +266,8 @@ void omap_set_dma_transfer_params(int lch, int data_type, 
int elem_count,
                ccr &= ~(1 << 5);
                if (sync_mode == OMAP_DMA_SYNC_FRAME)
                        ccr |= 1 << 5;
+               if (dma_trigger)
+                       ccr |= dma_trigger & 0x1f;
                dma_write(ccr, CCR(lch));

                ccr = dma_read(CCR2(lch));

Than can you print out in case of playback both the destination and source 
addresses supplied to the DMA, than in the pointer callback also print out the 
value returned by the omap_get_dma_src_pos function to see if this actually 
helps?

-- 
Péter

Re: [PATCH] [RFC] ASoC: OMAP: fix OMAP1510 broken PCM pointer callback

[Jarkko Nikula]

On Mon, 29 Jun 2009 09:37:58 +0300
Peter Ujfalusi <peter.ujfalusi@nokia.com> wrote:

> Hmmm, I had taken a look at the 2.4.21 kernel sources, which I have
> laying around in my disk from an old project which used OMAP1510. The
> OSS audio code does use the CPC register for determining the DMA
> progress both for playback and recording. I know that the audio was
> working OK on that board, since we had doom running there.
> The difference that I can see is that the OSS code also configured
> the CCR:SYNC(4:0) bits as well.
> Looking at the DMA_CPC register description in the OMAP1510 TRM: it
> list two cases on how it behaves and both require the DMA_CCR:SYNC !=
> 0...
> 
> The current DMA code for OMAP1510 just plain ignores the DMA_CCR:SYNC
> for some reason.
> Can you try the following patch:
> 
> iff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
> index 7fc8c04..38874e4 100644
> --- a/arch/arm/plat-omap/dma.c
> +++ b/arch/arm/plat-omap/dma.c
> @@ -266,6 +266,8 @@ void omap_set_dma_transfer_params(int lch, int
> data_type, int elem_count,
>                 ccr &= ~(1 << 5);
>                 if (sync_mode == OMAP_DMA_SYNC_FRAME)
>                         ccr |= 1 << 5;
> +               if (dma_trigger)
> +                       ccr |= dma_trigger & 0x1f;
>                 dma_write(ccr, CCR(lch));
> 
>                 ccr = dma_read(CCR2(lch));
> 
> Than can you print out in case of playback both the destination and
> source addresses supplied to the DMA, than in the pointer callback
> also print out the value returned by the omap_get_dma_src_pos
> function to see if this actually helps?
> 
Thanks for info Peter. So, Mark, put the workaround patch and my
acked-by on hold until this is also tried.


-- 
Jarkko

Re: [PATCH] [RFC] ASoC: OMAP: fix OMAP1510 broken PCM pointer callback

[Janusz Krzysztofik]

On Monday 29 June 2009 08:37:58 Peter Ujfalusi wrote:
> On Monday 29 June 2009 01:08:59 ext Janusz Krzysztofik wrote:
> > For capture, reading CPC, that follows destination port address progress,
> > just works fine (for both old and new driver). For playback, similar
> > hardware functionality seems to be missing, so it has to be emulated in
> > software if required.
>
> Hmmm, I had taken a look at the 2.4.21 kernel sources, which I have laying
> around in my disk from an old project which used OMAP1510. The OSS audio
> code does use the CPC register for determining the DMA progress both for
> playback and recording. I know that the audio was working OK on that board,
> since we had doom running there.

I am not able to find any information on how DMA can be configured for CPC to 
reflect either source or destination port address progress. Official 
documentation, even if not very clear for me, seems to suggest that it always 
follows destination port address, even if constatnt (ie 
DMA_CCR:DST_AMODE(15:14) == 0).

> The difference that I can see is that the OSS code also configured the
> CCR:SYNC(4:0) bits as well.
> Looking at the DMA_CPC register description in the OMAP1510 TRM: it list
> two cases on how it behaves and both require the DMA_CCR:SYNC != 0...

Not exactly. According to my copy of 
http://focus.ti.com/lit/ug/spru674/spru674.pdf, the secnod case is:

	If the channel transfer is synchronized on frames
	(DMA_CCR SYNC ≠ 0 and DMA_CCR FS = 1) or not
	synchronized (DMA_CCR SYNC = 0), the register is
	updated with 16 LSB of the address each time the
	destination port issues the last request for a frame.

Anyway, that seems to be irrelevant in our case.

> The current DMA code for OMAP1510 just plain ignores the DMA_CCR:SYNC for
> some reason.

Not exactly. Even if not touched inside any of omap_set_dma_transfer_params(), 
omap_set_dma_src_params() nor omap_set_dma_dest_params(), these bits are 
already set by arch/arm/plat-omap/dma.c:omap_request_dma(int dev_id, ...):

 765         } else if (cpu_is_omap7xx() || cpu_is_omap15xx()) {
 766                 dma_write(dev_id, CCR(free_ch));
 
> Can you try the following patch:

Sure. I remember we already talk about this before. That time, I had only 
verified that the change you proposed did not help in solving a different 
problem I used to have before. Then I found the above explanation, but never 
got an idea to let you know.

> iff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
> index 7fc8c04..38874e4 100644
> --- a/arch/arm/plat-omap/dma.c
> +++ b/arch/arm/plat-omap/dma.c
> @@ -266,6 +266,8 @@ void omap_set_dma_transfer_params(int lch, int
> data_type, int elem_count,
>                 ccr &= ~(1 << 5);
>                 if (sync_mode == OMAP_DMA_SYNC_FRAME)
>                         ccr |= 1 << 5;
> +               if (dma_trigger)
> +                       ccr |= dma_trigger & 0x1f;
>                 dma_write(ccr, CCR(lch));
>
>                 ccr = dma_read(CCR2(lch));

BTW, here the code tries to read (and than write) an address (CCR2) that is 
not supported on OMAP1510. I have not noticed any side effects, neither 
negative nor positive, but can provide a patch if OMAP guys ever find 
patching this as desired.

> Than can you print out in case of playback both the destination and source
> addresses supplied to the DMA, than in the pointer callback also print out
> the value returned by the omap_get_dma_src_pos function to see if this
> actually helps?

Here you are. Same figures wheather with or without your patch. I also printed 
a value that CCR is written with, so you can verify those SYNC(4:0) bits are 
set correctly.

root@amsdelta:~# aplay -f S16_LE -t raw -d 1 </dev/urandom
Playing raw data 'stdin' : Signed 16 bit Little Endian, Rate 8000 Hz, Mono
[  116.780000] omap_pcm_prepare(): dma_params.src_start = 0x11d80000
[  116.780000] omap_pcm_prepare(): dma_params.dst_start = 0xe1011806
[  116.790000] omap_set_dma_src_params(): dma_write(0x11d8, CSSA_U(0x0)
[  116.800000] omap_set_dma_src_params(): dma_write(0x0000, CSSA_L(0x0)
[  116.810000] omap_set_dma_dest_params(): dma_write(0x1008, CCR(0x0))
[  116.820000] omap_set_dma_dest_params(): dma_write(0xe101, CDSA_U(0x0)
[  116.830000] omap_set_dma_dest_params(): dma_write(0x1806, CDSA_L(0x0)
[  116.900000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
[  116.900000] omap_pcm_pointer(): offset = 0x0c03
[  116.920000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
[  116.920000] omap_pcm_pointer(): offset = 0x0c03
[  116.950000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
[  116.950000] omap_pcm_pointer(): offset = 0x0c03
[  116.980000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
[  116.980000] omap_pcm_pointer(): offset = 0x0c03
[  116.990000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
[  116.990000] omap_pcm_pointer(): offset = 0x0c03
[  117.010000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
[  117.010000] omap_pcm_pointer(): offset = 0x0c03
[  117.140000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
[  117.140000] omap_pcm_pointer(): offset = 0x0c03
underrun!!! (at least 1799126512.681 ms long)
[  117.150000] omap_pcm_prepare(): dma_params.src_start = 0x11d80000
[  117.170000] omap_pcm_prepare(): dma_params.dst_start = 0xe1011806
[  117.170000] omap_set_dma_src_params(): dma_write(0x11d8, CSSA_U(0x0)
[  117.180000] omap_set_dma_src_params(): dma_write(0x0000, CSSA_L(0x0)
[  117.190000] omap_set_dma_dest_params(): dma_write(0x1008, CCR(0x0))
[  117.200000] omap_set_dma_dest_params(): dma_write(0xe101, CDSA_U(0x0)
[  117.210000] omap_set_dma_dest_params(): dma_write(0x1806, CDSA_L(0x0)
[  117.340000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
[  117.340000] omap_pcm_pointer(): offset = 0x0c03
root@amsdelta:~#

And here, the same with capture, just for reference:

root@amsdelta:~# arecord -f S16_LE -t raw -d 1 >/dev/null
Recording raw data 'stdin' : Signed 16 bit Little Endian, Rate 8000 Hz, Mono
[  316.530000] omap_pcm_prepare(): dma_params.src_start = 0xe1011802
[  316.540000] omap_pcm_prepare(): dma_params.dst_start = 0x11da0000
[  316.550000] omap_set_dma_src_params(): dma_write(0xe101, CSSA_U(0x0)
[  316.550000] omap_set_dma_src_params(): dma_write(0x1802, CSSA_L(0x0)
[  316.560000] omap_set_dma_dest_params(): dma_write(0x4009, CCR(0x0))
[  316.570000] omap_set_dma_dest_params(): dma_write(0x11da, CDSA_U(0x0)
[  316.580000] omap_set_dma_dest_params(): dma_write(0x0000, CDSA_L(0x0)
[  316.590000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x01c2
[  316.590000] omap_pcm_pointer(): offset = 0x00e1
[  316.610000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x00dc
[  316.610000] omap_pcm_pointer(): offset = 0x006e
[  316.620000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x01b8
[  316.620000] omap_pcm_pointer(): offset = 0x00dc
[  316.630000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x0294
[  316.630000] omap_pcm_pointer(): offset = 0x014a
[  316.650000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x037e
[  316.650000] omap_pcm_pointer(): offset = 0x01bf
...

Now, playback with my workaround applied, slightly modified for always 
reporting dma_read(CPC(0x0)) result, even if not used for offset calculation:

root@amsdelta:~# aplay -f S16_LE -t raw -d 1 </dev/urandom
Playing raw data 'stdin' : Signed 16 bit Little Endian, Rate 8000 Hz, Mono
[  182.990000] omap_pcm_prepare(): dma_params.src_start = 0x11d80000
[  183.000000] omap_pcm_prepare(): dma_params.dst_start = 0xe1011806
[  183.010000] omap_set_dma_src_params(): dma_write(0x11d8, CSSA_U(0x0)
[  183.020000] omap_set_dma_src_params(): dma_write(0x0000, CSSA_L(0x0)
[  183.030000] omap_set_dma_dest_params(): dma_write(0x1008, CCR(0x0))
[  183.040000] omap_set_dma_dest_params(): dma_write(0xe101, CDSA_U(0x0)
[  183.050000] omap_set_dma_dest_params(): dma_write(0x1806, CDSA_L(0x0)
[  183.120000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
[  183.120000] omap_pcm_pointer(): offset = 0x0000
[  183.230000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
[  183.230000] omap_pcm_pointer(): offset = 0x03e8
[  183.260000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
[  183.260000] omap_pcm_pointer(): offset = 0x03e8
[  183.360000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
[  183.360000] omap_pcm_pointer(): offset = 0x07d0
[  183.380000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
[  183.380000] omap_pcm_pointer(): offset = 0x07d0
[  183.480000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
[  183.480000] omap_pcm_pointer(): offset = 0x0bb8
[  183.510000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
[  183.510000] omap_pcm_pointer(): offset = 0x0bb8
[  183.610000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
[  183.610000] omap_pcm_pointer(): offset = 0x0000
[  183.730000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
[  183.730000] omap_pcm_pointer(): offset = 0x03e8
[  183.860000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
[  183.860000] omap_pcm_pointer(): offset = 0x07d0
[  183.980000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
[  183.980000] omap_pcm_pointer(): offset = 0x0bb8
[  184.110000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
[  184.110000] omap_pcm_pointer(): offset = 0x0000
root@amsdelta:~#

Peter,
I have really no idea how the old 2.4 code could do the job with just reading 
CPC for both capture and playback, but if you think that with my ams-delta 
machine on hand I can still help you in finding that out, please let me know.

Cheers,
Janusz

Re: [alsa-devel] [PATCH] [RFC] ASoC: OMAP: fix OMAP1510 broken PCM pointer callback

[Peter Ujfalusi]

On Monday 29 June 2009 16:51:07 ext Janusz Krzysztofik wrote:
> I am not able to find any information on how DMA can be configured for CPC
> to reflect either source or destination port address progress. Official
> documentation, even if not very clear for me, seems to suggest that it
> always follows destination port address, even if constatnt (ie
> DMA_CCR:DST_AMODE(15:14) == 0).

Yeah, the CPC register only mentioned in the register description. I think the 
description suggests that it should have the LSB of the source (in contrast 
what it actually contains), since it states that the content will be updated 
when the destination port issued a request for an element/frame. To my 
knowledge only - static - ports can issue DMA requests.

>
> > The difference that I can see is that the OSS code also configured the
> > CCR:SYNC(4:0) bits as well.
> > Looking at the DMA_CPC register description in the OMAP1510 TRM: it list
> > two cases on how it behaves and both require the DMA_CCR:SYNC != 0...
>
> Not exactly. According to my copy of
> http://focus.ti.com/lit/ug/spru674/spru674.pdf, the secnod case is:
>
> 	If the channel transfer is synchronized on frames
> 	(DMA_CCR SYNC ≠ 0 and DMA_CCR FS = 1) or not
> 	synchronized (DMA_CCR SYNC = 0), the register is
> 	updated with 16 LSB of the address each time the
> 	destination port issues the last request for a frame.
>
> Anyway, that seems to be irrelevant in our case.

Yes, that's true, but we are using element mode.

> Not exactly. Even if not touched inside any of
> omap_set_dma_transfer_params(), omap_set_dma_src_params() nor
> omap_set_dma_dest_params(), these bits are already set by
> arch/arm/plat-omap/dma.c:omap_request_dma(int dev_id, ...):
>
>  765         } else if (cpu_is_omap7xx() || cpu_is_omap15xx()) {
>  766                 dma_write(dev_id, CCR(free_ch));

Correct, I have missed this one.

> Here you are. Same figures wheather with or without your patch. I also
> printed a value that CCR is written with, so you can verify those SYNC(4:0)
> bits are set correctly.
>
> root@amsdelta:~# aplay -f S16_LE -t raw -d 1 </dev/urandom
> Playing raw data 'stdin' : Signed 16 bit Little Endian, Rate 8000 Hz, Mono
> [  116.780000] omap_pcm_prepare(): dma_params.src_start = 0x11d80000
> [  116.780000] omap_pcm_prepare(): dma_params.dst_start = 0xe1011806
> [  116.790000] omap_set_dma_src_params(): dma_write(0x11d8, CSSA_U(0x0)
> [  116.800000] omap_set_dma_src_params(): dma_write(0x0000, CSSA_L(0x0)
> [  116.810000] omap_set_dma_dest_params(): dma_write(0x1008, CCR(0x0))
> [  116.820000] omap_set_dma_dest_params(): dma_write(0xe101, CDSA_U(0x0)
> [  116.830000] omap_set_dma_dest_params(): dma_write(0x1806, CDSA_L(0x0)
> [  116.900000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
> [  116.900000] omap_pcm_pointer(): offset = 0x0c03
> [  116.920000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
> [  116.920000] omap_pcm_pointer(): offset = 0x0c03
> [  116.950000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
> [  116.950000] omap_pcm_pointer(): offset = 0x0c03
> [  116.980000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
> [  116.980000] omap_pcm_pointer(): offset = 0x0c03
> [  116.990000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
> [  116.990000] omap_pcm_pointer(): offset = 0x0c03
> [  117.010000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
> [  117.010000] omap_pcm_pointer(): offset = 0x0c03
> [  117.140000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
> [  117.140000] omap_pcm_pointer(): offset = 0x0c03
> underrun!!! (at least 1799126512.681 ms long)
> [  117.150000] omap_pcm_prepare(): dma_params.src_start = 0x11d80000
> [  117.170000] omap_pcm_prepare(): dma_params.dst_start = 0xe1011806
> [  117.170000] omap_set_dma_src_params(): dma_write(0x11d8, CSSA_U(0x0)
> [  117.180000] omap_set_dma_src_params(): dma_write(0x0000, CSSA_L(0x0)
> [  117.190000] omap_set_dma_dest_params(): dma_write(0x1008, CCR(0x0))
> [  117.200000] omap_set_dma_dest_params(): dma_write(0xe101, CDSA_U(0x0)
> [  117.210000] omap_set_dma_dest_params(): dma_write(0x1806, CDSA_L(0x0)
> [  117.340000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x1806
> [  117.340000] omap_pcm_pointer(): offset = 0x0c03
> root@amsdelta:~#
>
> And here, the same with capture, just for reference:
>
> root@amsdelta:~# arecord -f S16_LE -t raw -d 1 >/dev/null
> Recording raw data 'stdin' : Signed 16 bit Little Endian, Rate 8000 Hz,
> Mono [  316.530000] omap_pcm_prepare(): dma_params.src_start = 0xe1011802 [
>  316.540000] omap_pcm_prepare(): dma_params.dst_start = 0x11da0000 [ 
> 316.550000] omap_set_dma_src_params(): dma_write(0xe101, CSSA_U(0x0) [ 
> 316.550000] omap_set_dma_src_params(): dma_write(0x1802, CSSA_L(0x0) [ 
> 316.560000] omap_set_dma_dest_params(): dma_write(0x4009, CCR(0x0)) [ 
> 316.570000] omap_set_dma_dest_params(): dma_write(0x11da, CDSA_U(0x0) [ 
> 316.580000] omap_set_dma_dest_params(): dma_write(0x0000, CDSA_L(0x0) [ 
> 316.590000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x01c2 [ 
> 316.590000] omap_pcm_pointer(): offset = 0x00e1
> [  316.610000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x00dc
> [  316.610000] omap_pcm_pointer(): offset = 0x006e
> [  316.620000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x01b8
> [  316.620000] omap_pcm_pointer(): offset = 0x00dc
> [  316.630000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x0294
> [  316.630000] omap_pcm_pointer(): offset = 0x014a
> [  316.650000] omap_get_dma_src_pos(): dma_read(CPC(0x0)) == 0x037e
> [  316.650000] omap_pcm_pointer(): offset = 0x01bf
> ...

I find this whole thin amusing... It seams that the TRM is not quite written 
for the same chip. In contrast what it states for the CPC, it looks in a way, 
that the CPC is updated with the LSB of the destination at the last request 
issued by the source port (well you only need to negate the whole description 
to get to this conclusion).

> Peter,
> I have really no idea how the old 2.4 code could do the job with just
> reading CPC for both capture and playback, but if you think that with my
> ams-delta machine on hand I can still help you in finding that out, please
> let me know.

Me either now. The OSS driver uses this call to get the offset for capture and 
playback:
dma_addr_t omap_get_dma_pos(dma_regs_t * regs)
{
	return (dma_addr_t) (regs->cpc | (regs->cssa_u << 16));
}

Now this is bogus for either in case of playback or capture, but it should be 
not right for both...

It would be nice to understand how this is working, but for now I think your 
workaround is good for handling the situation.

Mark: I agree with Jarkko, this can be queued for 2.6.31.

Thanks Janusz for the debugging and figuring this out!

Acked-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>

Re: [alsa-devel] [PATCH] [RFC] ASoC: OMAP: fix OMAP1510 broken PCM pointer callback

[Mark Brown]

On Sun, Jun 28, 2009 at 12:21:05AM +0200, Janusz Krzysztofik wrote:
> This patch tries to work around the problem of broken OMAP1510 PCM playback 
> pointer calculation by replacing DMA function call that incorrectly tries to 
> read the value form DMA hardware with a value computed locally from an 
> already maintained variable omap_runtime_data.period_index.
> 
> Tested on OMAP5910 based Amstrad Delta (E3) using work in progress ASoC 
> driver.
> 
> Based on linux-2.6-asoc.git v2.6.31-rc1.
> 
> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>

Applied, thanks.  Please CC me on ASoC patches.