[PATCH][2.6.31-rc5] ARM: OMAP: McBSP: Fix ASoC on OMAP1510 by fixing API of omap_mcbsp_start/stop

2009-08-07 Thread Jarkko Nikula 
Simultaneous audio playback and capture on OMAP1510 can cause that second
stream is stalled if there is enough delay between startup of the audio
streams.

Current implementation of the omap_mcbsp_start is starting both transmitter
and receiver at the same time and it is called only for firstly started
audio stream from the OMAP McBSP based ASoC DAI driver.

Since DMA request lines on OMAP1510 are edge sensitive, the DMA request is
missed if there is no DMA transfer set up at that time when the first word
after McBSP startup is transmitted. The problem hasn't noted before since
later OMAPs are using level sensitive DMA request lines.

Fix the problem by changing API of omap_mcbsp_start and omap_mcbsp_stop by
allowing to start and stop individually McBSP transmitter and receiver
logics. Then call those functions individually for both audio playback
and capture streams. This ensures that DMA transfer is setup before
transmitter or receiver is started.

Thanks to Janusz Krzysztofik jkrzy...@tis.icnet.pl for detailed problem
analysis and Peter Ujfalusi peter.ujfal...@nokia.com for info about DMA
request line behavior differences between the OMAP generations.

Reported-and-tested-by: Janusz Krzysztofik jkrzy...@tis.icnet.pl
Signed-off-by: Jarkko Nikula jhnik...@gmail.com
Cc: Mark Brown broo...@opensource.wolfsonmicro.com
Cc: Tony Lindgren t...@atomide.com
---
 arch/arm/plat-omap/include/mach/mcbsp.h |4 +-
 arch/arm/plat-omap/mcbsp.c  |   50 +++---
 sound/soc/omap/omap-mcbsp.c |   10 +++---
 3 files changed, 39 insertions(+), 25 deletions(-)

diff --git a/arch/arm/plat-omap/include/mach/mcbsp.h 
b/arch/arm/plat-omap/include/mach/mcbsp.h
index bb154ea..57249bb 100644
--- a/arch/arm/plat-omap/include/mach/mcbsp.h
+++ b/arch/arm/plat-omap/include/mach/mcbsp.h
@@ -387,8 +387,8 @@ void omap_mcbsp_register_board_cfg(struct 
omap_mcbsp_platform_data *config,
 void omap_mcbsp_config(unsigned int id, const struct omap_mcbsp_reg_cfg * 
config);
 int omap_mcbsp_request(unsigned int id);
 void omap_mcbsp_free(unsigned int id);
-void omap_mcbsp_start(unsigned int id);
-void omap_mcbsp_stop(unsigned int id);
+void omap_mcbsp_start(unsigned int id, int tx, int rx);
+void omap_mcbsp_stop(unsigned int id, int tx, int rx);
 void omap_mcbsp_xmit_word(unsigned int id, u32 word);
 u32 omap_mcbsp_recv_word(unsigned int id);
 
diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
index efa0e01..a3d2313 100644
--- a/arch/arm/plat-omap/mcbsp.c
+++ b/arch/arm/plat-omap/mcbsp.c
@@ -328,14 +328,15 @@ void omap_mcbsp_free(unsigned int id)
 EXPORT_SYMBOL(omap_mcbsp_free);
 
 /*
- * Here we start the McBSP, by enabling the sample
- * generator, both transmitter and receivers,
- * and the frame sync.
+ * Here we start the McBSP, by enabling transmitter, receiver or both.
+ * If no transmitter or receiver is active prior calling, then sample-rate
+ * generator and frame sync are started.
  */
-void omap_mcbsp_start(unsigned int id)
+void omap_mcbsp_start(unsigned int id, int tx, int rx)
 {
struct omap_mcbsp *mcbsp;
void __iomem *io_base;
+   int idle;
u16 w;
 
if (!omap_mcbsp_check_valid_id(id)) {
@@ -348,32 +349,40 @@ void omap_mcbsp_start(unsigned int id)
mcbsp-rx_word_length = (OMAP_MCBSP_READ(io_base, RCR1)  5)  0x7;
mcbsp-tx_word_length = (OMAP_MCBSP_READ(io_base, XCR1)  5)  0x7;
 
-   /* Start the sample generator */
-   w = OMAP_MCBSP_READ(io_base, SPCR2);
-   OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1  6));
+   idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
+ OMAP_MCBSP_READ(io_base, SPCR1))  1);
+
+   if (idle) {
+   /* Start the sample generator */
+   w = OMAP_MCBSP_READ(io_base, SPCR2);
+   OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1  6));
+   }
 
/* Enable transmitter and receiver */
w = OMAP_MCBSP_READ(io_base, SPCR2);
-   OMAP_MCBSP_WRITE(io_base, SPCR2, w | 1);
+   OMAP_MCBSP_WRITE(io_base, SPCR2, w | (tx  1));
 
w = OMAP_MCBSP_READ(io_base, SPCR1);
-   OMAP_MCBSP_WRITE(io_base, SPCR1, w | 1);
+   OMAP_MCBSP_WRITE(io_base, SPCR1, w | (rx  1));
 
udelay(100);
 
-   /* Start frame sync */
-   w = OMAP_MCBSP_READ(io_base, SPCR2);
-   OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1  7));
+   if (idle) {
+   /* Start frame sync */
+   w = OMAP_MCBSP_READ(io_base, SPCR2);
+   OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1  7));
+   }
 
/* Dump McBSP Regs */
omap_mcbsp_dump_reg(id);
 }
 EXPORT_SYMBOL(omap_mcbsp_start);
 
-void omap_mcbsp_stop(unsigned int id)
+void omap_mcbsp_stop(unsigned int id, int tx, int rx)
 {
struct omap_mcbsp *mcbsp;
void __iomem *io_base;
+   int idle;
u16 w;
 
if (!omap_mcbsp_check_valid_id(id)) {
@@ -386,15 +395,20 @@ void omap_mcbsp_stop(unsigned int id)
 
/* Reset transmitter

Re: [PATCH][2.6.31-rc5] ARM: OMAP: McBSP: Fix ASoC on OMAP1510 by fixing API of omap_mcbsp_start/stop

2009-08-07 Thread Tony Lindgren 
* Jarkko Nikula jhnik...@gmail.com [090807 09:59]:
 Simultaneous audio playback and capture on OMAP1510 can cause that second
 stream is stalled if there is enough delay between startup of the audio
 streams.
 
 Current implementation of the omap_mcbsp_start is starting both transmitter
 and receiver at the same time and it is called only for firstly started
 audio stream from the OMAP McBSP based ASoC DAI driver.
 
 Since DMA request lines on OMAP1510 are edge sensitive, the DMA request is
 missed if there is no DMA transfer set up at that time when the first word
 after McBSP startup is transmitted. The problem hasn't noted before since
 later OMAPs are using level sensitive DMA request lines.
 
 Fix the problem by changing API of omap_mcbsp_start and omap_mcbsp_stop by
 allowing to start and stop individually McBSP transmitter and receiver
 logics. Then call those functions individually for both audio playback
 and capture streams. This ensures that DMA transfer is setup before
 transmitter or receiver is started.
 
 Thanks to Janusz Krzysztofik jkrzy...@tis.icnet.pl for detailed problem
 analysis and Peter Ujfalusi peter.ujfal...@nokia.com for info about DMA
 request line behavior differences between the OMAP generations.
 
 Reported-and-tested-by: Janusz Krzysztofik jkrzy...@tis.icnet.pl
 Signed-off-by: Jarkko Nikula jhnik...@gmail.com
 Cc: Mark Brown broo...@opensource.wolfsonmicro.com
 Cc: Tony Lindgren t...@atomide.com

This seems OK to me queue via the alsa list as it's mostly audio related.

Acked-by: Tony Lindgren t...@atomide.com




 ---
  arch/arm/plat-omap/include/mach/mcbsp.h |4 +-
  arch/arm/plat-omap/mcbsp.c  |   50 +++---
  sound/soc/omap/omap-mcbsp.c |   10 +++---
  3 files changed, 39 insertions(+), 25 deletions(-)
 
 diff --git a/arch/arm/plat-omap/include/mach/mcbsp.h 
 b/arch/arm/plat-omap/include/mach/mcbsp.h
 index bb154ea..57249bb 100644
 --- a/arch/arm/plat-omap/include/mach/mcbsp.h
 +++ b/arch/arm/plat-omap/include/mach/mcbsp.h
 @@ -387,8 +387,8 @@ void omap_mcbsp_register_board_cfg(struct 
 omap_mcbsp_platform_data *config,
  void omap_mcbsp_config(unsigned int id, const struct omap_mcbsp_reg_cfg * 
 config);
  int omap_mcbsp_request(unsigned int id);
  void omap_mcbsp_free(unsigned int id);
 -void omap_mcbsp_start(unsigned int id);
 -void omap_mcbsp_stop(unsigned int id);
 +void omap_mcbsp_start(unsigned int id, int tx, int rx);
 +void omap_mcbsp_stop(unsigned int id, int tx, int rx);
  void omap_mcbsp_xmit_word(unsigned int id, u32 word);
  u32 omap_mcbsp_recv_word(unsigned int id);
  
 diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
 index efa0e01..a3d2313 100644
 --- a/arch/arm/plat-omap/mcbsp.c
 +++ b/arch/arm/plat-omap/mcbsp.c
 @@ -328,14 +328,15 @@ void omap_mcbsp_free(unsigned int id)
  EXPORT_SYMBOL(omap_mcbsp_free);
  
  /*
 - * Here we start the McBSP, by enabling the sample
 - * generator, both transmitter and receivers,
 - * and the frame sync.
 + * Here we start the McBSP, by enabling transmitter, receiver or both.
 + * If no transmitter or receiver is active prior calling, then sample-rate
 + * generator and frame sync are started.
   */
 -void omap_mcbsp_start(unsigned int id)
 +void omap_mcbsp_start(unsigned int id, int tx, int rx)
  {
   struct omap_mcbsp *mcbsp;
   void __iomem *io_base;
 + int idle;
   u16 w;
  
   if (!omap_mcbsp_check_valid_id(id)) {
 @@ -348,32 +349,40 @@ void omap_mcbsp_start(unsigned int id)
   mcbsp-rx_word_length = (OMAP_MCBSP_READ(io_base, RCR1)  5)  0x7;
   mcbsp-tx_word_length = (OMAP_MCBSP_READ(io_base, XCR1)  5)  0x7;
  
 - /* Start the sample generator */
 - w = OMAP_MCBSP_READ(io_base, SPCR2);
 - OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1  6));
 + idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
 +   OMAP_MCBSP_READ(io_base, SPCR1))  1);
 +
 + if (idle) {
 + /* Start the sample generator */
 + w = OMAP_MCBSP_READ(io_base, SPCR2);
 + OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1  6));
 + }
  
   /* Enable transmitter and receiver */
   w = OMAP_MCBSP_READ(io_base, SPCR2);
 - OMAP_MCBSP_WRITE(io_base, SPCR2, w | 1);
 + OMAP_MCBSP_WRITE(io_base, SPCR2, w | (tx  1));
  
   w = OMAP_MCBSP_READ(io_base, SPCR1);
 - OMAP_MCBSP_WRITE(io_base, SPCR1, w | 1);
 + OMAP_MCBSP_WRITE(io_base, SPCR1, w | (rx  1));
  
   udelay(100);
  
 - /* Start frame sync */
 - w = OMAP_MCBSP_READ(io_base, SPCR2);
 - OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1  7));
 + if (idle) {
 + /* Start frame sync */
 + w = OMAP_MCBSP_READ(io_base, SPCR2);
 + OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1  7));
 + }
  
   /* Dump McBSP Regs */
   omap_mcbsp_dump_reg(id);
  }
  EXPORT_SYMBOL(omap_mcbsp_start);
  
 -void omap_mcbsp_stop(unsigned int id)
 +void omap_mcbsp_stop(unsigned int id, int tx, int rx)
  {
   struct

Re: [alsa-devel] [PATCH][2.6.31-rc5] ARM: OMAP: McBSP: Fix ASoC on OMAP1510 by fixing API of omap_mcbsp_start/stop

2009-08-07 Thread Jarkko Nikula 
On Fri, 7 Aug 2009 11:30:54 +0300
Peter Ujfalusi peter.ujfal...@nokia.com wrote:

 Looks nice, but I would have done it a bit differently...
 
...
  -void omap_mcbsp_start(unsigned int id);
  -void omap_mcbsp_stop(unsigned int id);
  +void omap_mcbsp_start(unsigned int id, int tx, int rx);
  +void omap_mcbsp_stop(unsigned int id, int tx, int rx);
 
 void omap_mcbsp_start(unsigned int id, int dir);
 void omap_mcbsp_stop(unsigned int id, int dir);
 
Valid point and cleans up a bit the ALSA SoC part of the patch.
However I didn't want to limit this generic arch/arm/plat-omap/mcbsp.c
into audio use case only. My change still allows to start transmitter
and receiver exactly at the same time. Might be worth if the
generic McBSP driver is used for something else than audio.


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

Re: [alsa-devel] [PATCH][2.6.31-rc5] ARM: OMAP: McBSP: Fix ASoC on OMAP1510 by fixing API of omap_mcbsp_start/stop

2009-08-07 Thread Peter Ujfalusi 
On Friday 07 August 2009 11:47:57 ext Jarkko Nikula wrote:
 On Fri, 7 Aug 2009 11:30:54 +0300

 Peter Ujfalusi peter.ujfal...@nokia.com wrote:
  Looks nice, but I would have done it a bit differently...

 ...

   -void omap_mcbsp_start(unsigned int id);
   -void omap_mcbsp_stop(unsigned int id);
   +void omap_mcbsp_start(unsigned int id, int tx, int rx);
   +void omap_mcbsp_stop(unsigned int id, int tx, int rx);
 
  void omap_mcbsp_start(unsigned int id, int dir);
  void omap_mcbsp_stop(unsigned int id, int dir);

 Valid point and cleans up a bit the ALSA SoC part of the patch.
 However I didn't want to limit this generic arch/arm/plat-omap/mcbsp.c
 into audio use case only. My change still allows to start transmitter
 and receiver exactly at the same time. Might be worth if the
 generic McBSP driver is used for something else than audio.

Good point.

-- 
Péter
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [PATCH][2.6.31-rc5] ARM: OMAP: McBSP: Fix ASoC on OMAP1510 by fixing API of omap_mcbsp_start/stop

2009-08-07 Thread Peter Ujfalusi 
On Friday 07 August 2009 09:59:47 ext Jarkko Nikula wrote:
 Simultaneous audio playback and capture on OMAP1510 can cause that second
 stream is stalled if there is enough delay between startup of the audio
 streams.

 Current implementation of the omap_mcbsp_start is starting both transmitter
 and receiver at the same time and it is called only for firstly started
 audio stream from the OMAP McBSP based ASoC DAI driver.

 Since DMA request lines on OMAP1510 are edge sensitive, the DMA request is
 missed if there is no DMA transfer set up at that time when the first word
 after McBSP startup is transmitted. The problem hasn't noted before since
 later OMAPs are using level sensitive DMA request lines.

 Fix the problem by changing API of omap_mcbsp_start and omap_mcbsp_stop by
 allowing to start and stop individually McBSP transmitter and receiver
 logics. Then call those functions individually for both audio playback
 and capture streams. This ensures that DMA transfer is setup before
 transmitter or receiver is started.

 Thanks to Janusz Krzysztofik jkrzy...@tis.icnet.pl for detailed problem
 analysis and Peter Ujfalusi peter.ujfal...@nokia.com for info about DMA
 request line behavior differences between the OMAP generations.

 Reported-and-tested-by: Janusz Krzysztofik jkrzy...@tis.icnet.pl
 Signed-off-by: Jarkko Nikula jhnik...@gmail.com
 Cc: Mark Brown broo...@opensource.wolfsonmicro.com
 Cc: Tony Lindgren t...@atomide.com
 ---
  arch/arm/plat-omap/include/mach/mcbsp.h |4 +-
  arch/arm/plat-omap/mcbsp.c  |   50
 +++--- sound/soc/omap/omap-mcbsp.c |  
 10 +++---
  3 files changed, 39 insertions(+), 25 deletions(-)

Acked-by: Peter Ujfalusi peter.ujfal...@nokia.com

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

Re: [alsa-devel] [PATCH][2.6.31-rc5] ARM: OMAP: McBSP: Fix ASoC on OMAP1510 by fixing API of omap_mcbsp_start/stop

2009-08-07 Thread Peter Ujfalusi 
On Friday 07 August 2009 09:59:47 ext Jarkko Nikula wrote:
 Simultaneous audio playback and capture on OMAP1510 can cause that second
 stream is stalled if there is enough delay between startup of the audio
 streams.

 Current implementation of the omap_mcbsp_start is starting both transmitter
 and receiver at the same time and it is called only for firstly started
 audio stream from the OMAP McBSP based ASoC DAI driver.

 Since DMA request lines on OMAP1510 are edge sensitive, the DMA request is
 missed if there is no DMA transfer set up at that time when the first word
 after McBSP startup is transmitted. The problem hasn't noted before since
 later OMAPs are using level sensitive DMA request lines.

 Fix the problem by changing API of omap_mcbsp_start and omap_mcbsp_stop by
 allowing to start and stop individually McBSP transmitter and receiver
 logics. Then call those functions individually for both audio playback
 and capture streams. This ensures that DMA transfer is setup before
 transmitter or receiver is started.

Looks nice, but I would have done it a bit differently...


 Thanks to Janusz Krzysztofik jkrzy...@tis.icnet.pl for detailed problem
 analysis and Peter Ujfalusi peter.ujfal...@nokia.com for info about DMA
 request line behavior differences between the OMAP generations.

 Reported-and-tested-by: Janusz Krzysztofik jkrzy...@tis.icnet.pl
 Signed-off-by: Jarkko Nikula jhnik...@gmail.com
 Cc: Mark Brown broo...@opensource.wolfsonmicro.com
 Cc: Tony Lindgren t...@atomide.com
 ---
  arch/arm/plat-omap/include/mach/mcbsp.h |4 +-
  arch/arm/plat-omap/mcbsp.c  |   50
 +++--- sound/soc/omap/omap-mcbsp.c |  
 10 +++---
  3 files changed, 39 insertions(+), 25 deletions(-)

 diff --git a/arch/arm/plat-omap/include/mach/mcbsp.h
 b/arch/arm/plat-omap/include/mach/mcbsp.h index bb154ea..57249bb 100644
 --- a/arch/arm/plat-omap/include/mach/mcbsp.h
 +++ b/arch/arm/plat-omap/include/mach/mcbsp.h
 @@ -387,8 +387,8 @@ void omap_mcbsp_register_board_cfg(struct
 omap_mcbsp_platform_data *config, void omap_mcbsp_config(unsigned int id,
 const struct omap_mcbsp_reg_cfg * config); int omap_mcbsp_request(unsigned
 int id);
  void omap_mcbsp_free(unsigned int id);
 -void omap_mcbsp_start(unsigned int id);
 -void omap_mcbsp_stop(unsigned int id);
 +void omap_mcbsp_start(unsigned int id, int tx, int rx);
 +void omap_mcbsp_stop(unsigned int id, int tx, int rx);

void omap_mcbsp_start(unsigned int id, int dir);
void omap_mcbsp_stop(unsigned int id, int dir);

  void omap_mcbsp_xmit_word(unsigned int id, u32 word);
  u32 omap_mcbsp_recv_word(unsigned int id);

 diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
 index efa0e01..a3d2313 100644
 --- a/arch/arm/plat-omap/mcbsp.c
 +++ b/arch/arm/plat-omap/mcbsp.c
 @@ -328,14 +328,15 @@ void omap_mcbsp_free(unsigned int id)
  EXPORT_SYMBOL(omap_mcbsp_free);

  /*
 - * Here we start the McBSP, by enabling the sample
 - * generator, both transmitter and receivers,
 - * and the frame sync.
 + * Here we start the McBSP, by enabling transmitter, receiver or both.
 + * If no transmitter or receiver is active prior calling, then sample-rate
 + * generator and frame sync are started.

 * dir == 0: tx
 * dir == 1: rx

   */
 -void omap_mcbsp_start(unsigned int id)
 +void omap_mcbsp_start(unsigned int id, int tx, int rx)

void omap_mcbsp_start(unsigned int id, int dir)

  {
   struct omap_mcbsp *mcbsp;
   void __iomem *io_base;
 + int idle;
   u16 w;

...

   /* Enable transmitter and receiver */
   w = OMAP_MCBSP_READ(io_base, SPCR2);
 - OMAP_MCBSP_WRITE(io_base, SPCR2, w | 1);
 + OMAP_MCBSP_WRITE(io_base, SPCR2, w | (tx  1));

   w = OMAP_MCBSP_READ(io_base, SPCR1);
 - OMAP_MCBSP_WRITE(io_base, SPCR1, w | 1);
 + OMAP_MCBSP_WRITE(io_base, SPCR1, w | (rx  1));

if (dir) {
/* RX: Enable receiver */
w = OMAP_MCBSP_READ(io_base, SPCR1);
OMAP_MCBSP_WRITE(io_base, SPCR1, w | 1);
} else {
/* TX: Enable transmitter */
w = OMAP_MCBSP_READ(io_base, SPCR2);
OMAP_MCBSP_WRITE(io_base, SPCR2, w | 1);
}

   udelay(100);

...

 -void omap_mcbsp_stop(unsigned int id)
 +void omap_mcbsp_stop(unsigned int id, int tx, int rx)

void omap_mcbsp_stop(unsigned int id, int dir)

  {
   struct omap_mcbsp *mcbsp;
   void __iomem *io_base;
 + int idle;
   u16 w;

   if (!omap_mcbsp_check_valid_id(id)) {
 @@ -386,15 +395,20 @@ void omap_mcbsp_stop(unsigned int id)

   /* Reset transmitter */
   w = OMAP_MCBSP_READ(io_base, SPCR2);
 - OMAP_MCBSP_WRITE(io_base, SPCR2, w  ~(1));
 + OMAP_MCBSP_WRITE(io_base, SPCR2, w  ~(tx  1));

   /* Reset receiver */
   w = OMAP_MCBSP_READ(io_base, SPCR1);
 - OMAP_MCBSP_WRITE(io_base, SPCR1, w  ~(1));
 + OMAP_MCBSP_WRITE(io_base, SPCR1, w  ~(rx  1));

if (dir) {
/* RX: