[RFC][PATCH 1/3] ARM: OMAP: DMA: Add support for DMA channel self linking on OMAP1510

[RFC][PATCH 1/3] ARM: OMAP: DMA: Add support for DMA channel self linking on OMAP1510

[Janusz Krzysztofik]

Implement DMA channel self linking on OMAP1510 using AUTO_INIT and REPEAT
flags of the DMA CCR register.

Created against linux-2.6.31-rc5.

Tested on Amstrad Delta.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>

---
Note:
In its current form, enable_lnk() seems not very consistent (ie. result of a
conditional assignment is then overwritten unconditionally). As rather
different from OMAP1510 that I am familiar with, I am not quite sure how
support for other OMAP models should really look like. So I decided to leave
it as is and put my bit in a separate block, adding an additional FIXME
comment.

--- linux-2.6.31-rc5/arch/arm/plat-omap/dma.c.orig	2009-08-07 13:38:02.000000000 +0200
+++ linux-2.6.31-rc5/arch/arm/plat-omap/dma.c	2009-08-10 09:01:41.000000000 +0200
@@ -653,8 +653,21 @@ static inline void enable_lnk(int lch)
 {
 	u32 l;
 
+	if (omap_dma_in_1510_mode()) {
+		/* only self linking is supported */
+		if (dma_chan[lch].next_lch == lch) {
+			/* set AUTO_INIT | REPEAT bits in CCR */
+			l = dma_read(CCR(lch));
+			l |= 3 << 8;
+			dma_write(l, CCR(lch));
+		}
+		return;
+	}
+
 	l = dma_read(CLNK_CTRL(lch));
 
+	/* FIXME:
+	 * this conditional assignment is then overwritten unconditionally */
 	if (cpu_class_is_omap1())
 		l &= ~(1 << 14);
 
@@ -675,13 +688,21 @@ static inline void disable_lnk(int lch)
 {
 	u32 l;
 
-	l = dma_read(CLNK_CTRL(lch));
+	if (omap_dma_in_1510_mode())
+		l = dma_read(CCR(lch));
+	else
+		l = dma_read(CLNK_CTRL(lch));
 
 	/* Disable interrupts */
 	if (cpu_class_is_omap1()) {
 		dma_write(0, CICR(lch));
-		/* Set the STOP_LNK bit */
-		l |= 1 << 14;
+		if (omap_dma_in_1510_mode()) {
+			/* Unset AUTO_INIT & REPEAT bits */
+			l &= ~(3 << 8);
+		} else {
+			/* Set the STOP_LNK bit */
+			l |= 1 << 14;
+		}
 	}
 
 	if (cpu_class_is_omap2()) {
@@ -690,7 +711,10 @@ static inline void disable_lnk(int lch)
 		l &= ~(1 << 15);
 	}
 
-	dma_write(l, CLNK_CTRL(lch));
+	if (omap_dma_in_1510_mode())
+		dma_write(l, CCR(lch));
+	else
+		dma_write(l, CLNK_CTRL(lch));
 	dma_chan[lch].flags &= ~OMAP_DMA_ACTIVE;
 }
 
@@ -928,7 +952,7 @@ void omap_start_dma(int lch)
 {
 	u32 l;
 
-	if (!omap_dma_in_1510_mode() && dma_chan[lch].next_lch != -1) {
+	if (dma_chan[lch].next_lch != -1) {
 		int next_lch, cur_lch;
 		char dma_chan_link_map[OMAP_DMA4_LOGICAL_DMA_CH_COUNT];
 
@@ -979,7 +1003,7 @@ void omap_stop_dma(int lch)
 {
 	u32 l;
 
-	if (!omap_dma_in_1510_mode() && dma_chan[lch].next_lch != -1) {
+	if (dma_chan[lch].next_lch != -1) {
 		int next_lch, cur_lch = lch;
 		char dma_chan_link_map[OMAP_DMA4_LOGICAL_DMA_CH_COUNT];
 
@@ -1130,7 +1154,7 @@ int omap_dma_running(void)
  */
 void omap_dma_link_lch(int lch_head, int lch_queue)
 {
-	if (omap_dma_in_1510_mode()) {
+	if (omap_dma_in_1510_mode() && (lch_head != lch_queue)) {
 		printk(KERN_ERR "DMA linking is not supported in 1510 mode\n");
 		BUG();
 		return;
@@ -1152,7 +1176,7 @@ EXPORT_SYMBOL(omap_dma_link_lch);
  */
 void omap_dma_unlink_lch(int lch_head, int lch_queue)
 {
-	if (omap_dma_in_1510_mode()) {
+	if (omap_dma_in_1510_mode() && (lch_head != lch_queue)) {
 		printk(KERN_ERR "DMA linking is not supported in 1510 mode\n");
 		BUG();
 		return;
@@ -1822,7 +1846,7 @@ static int omap1_dma_handle_ch(int ch)
 	if (unlikely(csr & OMAP_DMA_DROP_IRQ))
 		printk(KERN_WARNING "DMA synchronization event drop occurred "
 		       "with device %d\n", dma_chan[ch].dev_id);
-	if (likely(csr & OMAP_DMA_BLOCK_IRQ))
+	if (likely(csr & OMAP_DMA_BLOCK_IRQ) && (dma_chan[ch].next_lch != ch))
 		dma_chan[ch].flags &= ~OMAP_DMA_ACTIVE;
 	if (likely(dma_chan[ch].callback != NULL))
 		dma_chan[ch].callback(ch, csr, dma_chan[ch].data);

Re: [RFC][PATCH 1/3] ARM: OMAP: DMA: Add support for DMA channel self linking on OMAP1510

[Jarkko Nikula]

On Mon, 10 Aug 2009 10:50:00 +0200
Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:

> Implement DMA channel self linking on OMAP1510 using AUTO_INIT and REPEAT
> flags of the DMA CCR register.
> 
> Created against linux-2.6.31-rc5.
> 
> Tested on Amstrad Delta.
> 
> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> 
Tony, I'm fine with this patch. Although the autoinitialization !=
channel linking, in SW point of view autoinitialization and channel
linking with itself looks the same so it's better to re-utilize
existing omap_dma_link_lch also for 1510 DMA autoinitialization than
inventing own API for it.

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

[RFC][PATCH 1/3 v2] ARM: OMAP: DMA: Add support for DMA channel self linking on OMAP1510

[Janusz Krzysztofik]

Implement DMA channel self linking on OMAP1510 using AUTO_INIT and REPEAT
flags of the DMA CCR register.

Created against linux-2.6.31-rc5.

Tested on Amstrad Delta.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>

---
I'm sorry, but the original patch appears to have an issue. To be honest, I
started with the one I submit here as v2, got pretty good results, then
modified it in order to better fit into existing framework, and did not test
it well enough after the change.

The problem reveals itself when an application keeps a stream open while
cycling through playing sounds. I don't know if that particular application
may just behave wrong, however, I know that setting AUTO_INIT and REPEAT
flags once after opening a stream and unsetting them once before closing it
gives stable, correct results, while redoing it for every single piece of
sound without closing the stream in between, like the current patch does,
results in inaccurate transfer startups and random application hangups.

I know it's not very fair to change the patch after others that depend on it
have already been applied, but I hope there will be no problems with accepting
the way I have reimplemented it. In any case, I'll appreciate any comments or
suggestions.

--- linux-2.6.31-rc5/arch/arm/plat-omap/dma.c.orig	2009-08-23 02:24:46.000000000 +0200
+++ linux-2.6.31-rc5/arch/arm/plat-omap/dma.c	2009-08-23 14:53:48.000000000 +0200
@@ -1131,6 +1131,11 @@ int omap_dma_running(void)
 void omap_dma_link_lch(int lch_head, int lch_queue)
 {
 	if (omap_dma_in_1510_mode()) {
+		if (lch_head == lch_queue) {
+			dma_write(dma_read(CCR(lch_head)) | (3 << 8),
+								CCR(lch_head));
+			return;
+		}
 		printk(KERN_ERR "DMA linking is not supported in 1510 mode\n");
 		BUG();
 		return;
@@ -1153,6 +1158,11 @@ EXPORT_SYMBOL(omap_dma_link_lch);
 void omap_dma_unlink_lch(int lch_head, int lch_queue)
 {
 	if (omap_dma_in_1510_mode()) {
+		if (lch_head == lch_queue) {
+			dma_write(dma_read(CCR(lch_head)) & ~(3 << 8),
+								CCR(lch_head));
+			return;
+		}
 		printk(KERN_ERR "DMA linking is not supported in 1510 mode\n");
 		BUG();
 		return;

Re: [alsa-devel] [RFC][PATCH 1/3 v2] ARM: OMAP: DMA: Add support for DMA channel self linking on OMAP1510

[Mark Brown]

On Sun, Aug 23, 2009 at 05:56:12PM +0200, Janusz Krzysztofik wrote:

> I know it's not very fair to change the patch after others that depend on it
> have already been applied, but I hope there will be no problems with accepting
> the way I have reimplemented it. In any case, I'll appreciate any comments or
> suggestions.

It's not that it's not fair, at this point it's not possible - please
make any changes you feel are required against current ASoC.  The patch
has been applied for a little while now and is buried quite deep in the
already published history.

Re: [alsa-devel] [RFC][PATCH 1/3 v2] ARM: OMAP: DMA: Add support for DMA channel self linking on OMAP1510

[Janusz Krzysztofik]

Sunday 23 August 2009 19:03:59 Mark Brown wrote:
> It's not that it's not fair, at this point it's not possible - please
> make any changes you feel are required against current ASoC.  The patch
> has been applied for a little while now and is buried quite deep in the
> already published history.

Mark,

I see your point. It also seemed to me that you had already applied my 
original patch, however while preparing that v2 I found no single evidence of 
it being applied, so took into consideration it was not my patch I had on my 
mind but Eduardo's patches on OMAP McBSP that you had agreeed with Tony to 
push via ASoC tree. The last message regarding this original patch I am able 
to locate was from Jarkko[1], where he said it would be better to leave this 
one in Tony's hands.

If I missed something and my original patch has really been applied, I'll be 
happy to prepare a new version that reverts previos changes and makes those 
proposed in patch v2, if those are acceptable.

Thanks,
Janusz

[1] http://www.spinics.net/lists/linux-omap/msg17172.html

Re: [alsa-devel] [RFC][PATCH 1/3 v2] ARM: OMAP: DMA: Add support for DMA channel self linking on OMAP1510

[Mark Brown]

On Sun, Aug 23, 2009 at 07:45:39PM +0200, Janusz Krzysztofik wrote:

> If I missed something and my original patch has really been applied, I'll be 
> happy to prepare a new version that reverts previos changes and makes those 
> proposed in patch v2, if those are acceptable.

If none of your patches have been applied then what are the other
patches that depend on this?  Nobody reported any issues in testing with
the other OMAP patches.

I don't really mind which tree these go via but I rather suspect that
ALSA will be better given all the other changes and I also suspect that
you'll need to regenerate your changes against a current tree due to
those changes.  Tony, are you OK with that?

Re: [alsa-devel] [RFC][PATCH 1/3 v2] ARM: OMAP: DMA: Add support for DMA channel self linking on OMAP1510

[Janusz Krzysztofik]

Sunday 23 August 2009 20:05:43 Mark Brown wrote:
> If none of your patches have been applied

Mark,

I did not say that. You have applied patches 2/3 [1] and 3/3 [2] from the 
series.

> then what are the other 
> patches that depend on this? Nobody reported any issues in testing with 
> the other OMAP patches.

Dependency on patch 1/3 is not compile time, but run time, and OMAP1510 
specific, that's probably why nobody has reported any issues.

> I don't really mind which tree these go via but I rather suspect that
> ALSA will be better given all the other changes and I also suspect that
> you'll need to regenerate your changes against a current tree due to
> those changes.

arch/arm/plat-omap/dma.c has not been touched since 2009-05-28 in yours [3] 
and since 2009-06-23 in Takashi's tree, so my patch, whether original or v2, 
should still be up to date. That may change if it is to be pushed via OMAP 
tree.

Sorry for all that noise.

Thanks,
Janusz

[1]http://git.kernel.org/?p=linux/kernel/git/broonie/sound-2.6.git;a=commit;h=64844a6ac8ddd586cb832fea7cf2e93e5e7e03f4
[2]http://git.kernel.org/?p=linux/kernel/git/broonie/sound-2.6.git;a=commit;h=471e3dec3abe2d41e8c742046353fcb01bc2459e
[3]http://git.kernel.org/?p=linux/kernel/git/broonie/sound-2.6.git;a=commit;h=44169075e6eaa87bab6a296209d8d0610879b394
[4]http://git.kernel.org/?p=linux/kernel/git/tiwai/sound-2.6.git;a=commit;h=aecedb94b366d6fb5e2a17ca18a5dc78e593198e

Re: [alsa-devel] [RFC][PATCH 1/3 v2] ARM: OMAP: DMA: Add support for DMA channel self linking on OMAP1510

[Mark Brown]

On Sun, Aug 23, 2009 at 08:38:05PM +0200, Janusz Krzysztofik wrote:
> Sunday 23 August 2009 20:05:43 Mark Brown wrote:

> > then what are the other 
> > patches that depend on this? Nobody reported any issues in testing with 
> > the other OMAP patches.

> Dependency on patch 1/3 is not compile time, but run time, and OMAP1510 
> specific, that's probably why nobody has reported any issues.

OK, given that and the fact that you're reporting that none of these
files are afected by any of the other patches it sounds like it'd be
better to keep it in the OMAP tree unless Tony says otherwse.

Re: [RFC][PATCH 1/3 v2] ARM: OMAP: DMA: Add support for DMA channel self linking on OMAP1510

[Jarkko Nikula]

On Sun, 23 Aug 2009 19:49:43 +0100
Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> On Sun, Aug 23, 2009 at 08:38:05PM +0200, Janusz Krzysztofik wrote:
> > Sunday 23 August 2009 20:05:43 Mark Brown wrote:
> 
> > > then what are the other 
> > > patches that depend on this? Nobody reported any issues in testing with 
> > > the other OMAP patches.
> 
> > Dependency on patch 1/3 is not compile time, but run time, and OMAP1510 
> > specific, that's probably why nobody has reported any issues.
> 
> OK, given that and the fact that you're reporting that none of these
> files are afected by any of the other patches it sounds like it'd be
> better to keep it in the OMAP tree unless Tony says otherwse.

Yep, Janusz's commits 64844a6ac8ddd586cb832fea7cf2e93e5e7e03f4 and
471e3dec3abe2d41e8c742046353fcb01bc2459e do not cause problems on other
OMAPs but will cause that ASoC on 1510 is broken until his DMA
patch(es) are applied. I'm sure you Janusz will ping Tony until he'll
apply them :-)

We should remember that arch/arm/plat-omap/mcbsp.c changes done via
ALSA tree has been an exception and that's only for simpler procedure.
McBSP is not so critical if sources goes out-of-sync but DMA is.


-- 
Jarkko

Re: [RFC][PATCH 1/3 v2] ARM: OMAP: DMA: Add support for DMA channel self linking on OMAP1510

[Tony Lindgren]

* Jarkko Nikula <jhnikula@gmail.com> [090824 08:47]:
> On Sun, 23 Aug 2009 19:49:43 +0100
> Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> 
> > On Sun, Aug 23, 2009 at 08:38:05PM +0200, Janusz Krzysztofik wrote:
> > > Sunday 23 August 2009 20:05:43 Mark Brown wrote:
> > 
> > > > then what are the other 
> > > > patches that depend on this? Nobody reported any issues in testing with 
> > > > the other OMAP patches.
> > 
> > > Dependency on patch 1/3 is not compile time, but run time, and OMAP1510 
> > > specific, that's probably why nobody has reported any issues.
> > 
> > OK, given that and the fact that you're reporting that none of these
> > files are afected by any of the other patches it sounds like it'd be
> > better to keep it in the OMAP tree unless Tony says otherwse.
> 
> Yep, Janusz's commits 64844a6ac8ddd586cb832fea7cf2e93e5e7e03f4 and
> 471e3dec3abe2d41e8c742046353fcb01bc2459e do not cause problems on other
> OMAPs but will cause that ASoC on 1510 is broken until his DMA
> patch(es) are applied. I'm sure you Janusz will ping Tony until he'll
> apply them :-)
> 
> We should remember that arch/arm/plat-omap/mcbsp.c changes done via
> ALSA tree has been an exception and that's only for simpler procedure.
> McBSP is not so critical if sources goes out-of-sync but DMA is.

Sorry for the delay in replying. I'm OK for these to go in via the alsa
tree as there are not currently other patches pending in the mcbsp or
dma area for omap. So I think it's better merge all the audio related
patches together in this case.

Acked-by: Tony Lindgren <tony@atomide.com>

Re: [RFC][PATCH 1/3 v2] ARM: OMAP: DMA: Add support for DMA channel self linking on OMAP1510

[Mark Brown]

On Mon, Aug 24, 2009 at 03:49:03PM +0300, Tony Lindgren wrote:

> > We should remember that arch/arm/plat-omap/mcbsp.c changes done via
> > ALSA tree has been an exception and that's only for simpler procedure.
> > McBSP is not so critical if sources goes out-of-sync but DMA is.

> Sorry for the delay in replying. I'm OK for these to go in via the alsa
> tree as there are not currently other patches pending in the mcbsp or
> dma area for omap. So I think it's better merge all the audio related
> patches together in this case.

Applied, thanks.