[alsa-devel] [RFC][PATCH 1/3] ARM: OMAP: DMA: Add support for DMA channel self linking on OMAP1510
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);
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
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;
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.
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
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?
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=6... [2]http://git.kernel.org/?p=linux/kernel/git/broonie/sound-2.6.git;a=commit;h=4... [3]http://git.kernel.org/?p=linux/kernel/git/broonie/sound-2.6.git;a=commit;h=4... [4]http://git.kernel.org/?p=linux/kernel/git/tiwai/sound-2.6.git;a=commit;h=aec...
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.
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 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
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.
participants (4)
-
Janusz Krzysztofik -
Jarkko Nikula -
Mark Brown -
Tony Lindgren