All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot
@ 2009-08-13  4:25 Barry Song
  2009-08-17 17:50 ` Mark Brown
  0 siblings, 1 reply; 10+ messages in thread
From: Barry Song @ 2009-08-13  4:25 UTC (permalink / raw)
  To: broonie; +Cc: uclinux-dist-devel, alsa-devel, Barry Song

For ad1938, the audio channel n just uses slot n, but for ad1836, it's
different, channel 0 uses slot 0, channel 1 uses slot 4, channels 2 uses
slot1, ... So add set_tdm_slot entry and use the mask field to define
the relationship between audio channel and slot number.

Signed-off-by: Barry Song <21cnbao@gmail.com>
---
 sound/soc/blackfin/bf5xx-ad1938.c  |    7 ++++++-
 sound/soc/blackfin/bf5xx-tdm-pcm.c |   12 +++++++++---
 sound/soc/blackfin/bf5xx-tdm.c     |   29 +++++++++++++++++++++--------
 sound/soc/blackfin/bf5xx-tdm.h     |   10 ++++++++++
 4 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/sound/soc/blackfin/bf5xx-ad1938.c b/sound/soc/blackfin/bf5xx-ad1938.c
index 08269e9..05163ea 100644
--- a/sound/soc/blackfin/bf5xx-ad1938.c
+++ b/sound/soc/blackfin/bf5xx-ad1938.c
@@ -74,8 +74,13 @@ static int bf5xx_ad1938_hw_params(struct snd_pcm_substream *substream,
 	if (ret < 0)
 		return ret;
 
+	/* set cpu DAI slots, 8 channels, mask is defined as slot seq */
+	ret = snd_soc_dai_set_tdm_slot(codec_dai, 0x76543210, 8, 32);
+	if (ret < 0)
+		return ret;
+
 	/* set codec DAI slots, 8 channels, all channels are enabled */
-	ret = snd_soc_dai_set_tdm_slot(codec_dai, 0xFF, 8);
+	ret = snd_soc_dai_set_tdm_slot(codec_dai, 0xFF, 8, 32);
 	if (ret < 0)
 		return ret;
 
diff --git a/sound/soc/blackfin/bf5xx-tdm-pcm.c b/sound/soc/blackfin/bf5xx-tdm-pcm.c
index ccb5e82..044ef45 100644
--- a/sound/soc/blackfin/bf5xx-tdm-pcm.c
+++ b/sound/soc/blackfin/bf5xx-tdm-pcm.c
@@ -43,7 +43,7 @@
 #include "bf5xx-tdm.h"
 #include "bf5xx-sport.h"
 
-#define PCM_BUFFER_MAX  0x10000
+#define PCM_BUFFER_MAX  0x8000
 #define FRAGMENT_SIZE_MIN  (4*1024)
 #define FRAGMENTS_MIN  2
 #define FRAGMENTS_MAX  32
@@ -177,6 +177,9 @@ out:
 static int bf5xx_pcm_copy(struct snd_pcm_substream *substream, int channel,
 	snd_pcm_uframes_t pos, void *buf, snd_pcm_uframes_t count)
 {
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct sport_device *sport = runtime->private_data;
+	struct bf5xx_tdm_port *tdm_port = sport->private_data;
 	unsigned int *src;
 	unsigned int *dst;
 	int i;
@@ -186,9 +189,11 @@ static int bf5xx_pcm_copy(struct snd_pcm_substream *substream, int channel,
 		dst = (unsigned int *)substream->runtime->dma_area;
 
 		dst += pos * 8;
+
 		while (count--) {
 			for (i = 0; i < substream->runtime->channels; i++)
-				*(dst + i) = *src++;
+				*(dst + (((tdm_port->slot_seq >>
+					(4 * i)) & 0xF))) = *src++;
 			dst += 8;
 		}
 	} else {
@@ -198,7 +203,8 @@ static int bf5xx_pcm_copy(struct snd_pcm_substream *substream, int channel,
 		src += pos * 8;
 		while (count--) {
 			for (i = 0; i < substream->runtime->channels; i++)
-				*dst++ = *(src+i);
+				*dst++ = *(src + (((tdm_port->slot_seq >>
+					(4 * i)) & 0xF)));
 			src += 8;
 		}
 	}
diff --git a/sound/soc/blackfin/bf5xx-tdm.c b/sound/soc/blackfin/bf5xx-tdm.c
index 3096bad..0f4ee15 100644
--- a/sound/soc/blackfin/bf5xx-tdm.c
+++ b/sound/soc/blackfin/bf5xx-tdm.c
@@ -46,14 +46,6 @@
 #include "bf5xx-sport.h"
 #include "bf5xx-tdm.h"
 
-struct bf5xx_tdm_port {
-	u16 tcr1;
-	u16 rcr1;
-	u16 tcr2;
-	u16 rcr2;
-	int configured;
-};
-
 static struct bf5xx_tdm_port bf5xx_tdm;
 static int sport_num = CONFIG_SND_BF5XX_SPORT_NUM;
 
@@ -181,6 +173,24 @@ static void bf5xx_tdm_shutdown(struct snd_pcm_substream *substream,
 		bf5xx_tdm.configured = 0;
 }
 
+static int bf5xx_tdm_set_tdm_slot(struct snd_soc_dai *dai,
+		unsigned int mask, int slots, int width)
+{
+	/*
+	 * For ad1938, the audio channel n just uses slot n, but for
+	 * ad1836, it's different, channel 0 uses slot 0, channel 1
+	 * uses slot 4, channels 2 uses slot1, ...  So add set_tdm_slot
+	 * entry and use the mask field to define the relationship
+	 * between audio channel and slot number.
+	 *
+	 * bit[0..3] means the slot channel 0 will use
+	 * bit[4..7] means the slot channel 1 will use
+	 * ...
+	 */
+	bf5xx_tdm.slot_seq = mask;
+	return 0;
+}
+
 #ifdef CONFIG_PM
 static int bf5xx_tdm_suspend(struct snd_soc_dai *dai)
 {
@@ -235,6 +245,7 @@ static struct snd_soc_dai_ops bf5xx_tdm_dai_ops = {
 	.hw_params      = bf5xx_tdm_hw_params,
 	.set_fmt        = bf5xx_tdm_set_dai_fmt,
 	.shutdown       = bf5xx_tdm_shutdown,
+	.set_tdm_slot   = bf5xx_tdm_set_tdm_slot,
 };
 
 struct snd_soc_dai bf5xx_tdm_dai = {
@@ -300,6 +311,8 @@ static int __devinit bfin_tdm_probe(struct platform_device *pdev)
 		pr_err("Failed to register DAI: %d\n", ret);
 		goto sport_config_err;
 	}
+
+	sport_handle->private_data = &bf5xx_tdm;
 	return 0;
 
 sport_config_err:
diff --git a/sound/soc/blackfin/bf5xx-tdm.h b/sound/soc/blackfin/bf5xx-tdm.h
index 618ec3d..701c769 100644
--- a/sound/soc/blackfin/bf5xx-tdm.h
+++ b/sound/soc/blackfin/bf5xx-tdm.h
@@ -9,6 +9,16 @@
 #ifndef _BF5XX_TDM_H
 #define _BF5XX_TDM_H
 
+struct bf5xx_tdm_port {
+	u16 tcr1;
+	u16 rcr1;
+	u16 tcr2;
+	u16 rcr2;
+	/* which slot used for the corresponding audio channel? */
+	int slot_seq;
+	int configured;
+};
+
 extern struct snd_soc_dai bf5xx_tdm_dai;
 
 #endif
-- 
1.5.6.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot
  2009-08-13  4:25 [PATCH V2 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot Barry Song
@ 2009-08-17 17:50 ` Mark Brown
  2009-08-18  2:53   ` Barry Song
  0 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2009-08-17 17:50 UTC (permalink / raw)
  To: Barry Song; +Cc: uclinux-dist-devel, alsa-devel

On Thu, Aug 13, 2009 at 12:25:11PM +0800, Barry Song wrote:

> For ad1938, the audio channel n just uses slot n, but for ad1836, it's
> different, channel 0 uses slot 0, channel 1 uses slot 4, channels 2 uses
> slot1, ... So add set_tdm_slot entry and use the mask field to define
> the relationship between audio channel and slot number.

I think what you're trying to describe here is that the device is
expecting to see all the left channels on the bus followed by all the
right channels?  This is the normal case for I2S TDM so no unusual
configuration is required in order to implement it.

I really don't think we should have Blackfin implementing a different
API here - it makes the TDM slot API much harder to use.  Instead we
should have a consistent API between all devices.  This may mean that we
need to extend the APIs here.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot
  2009-08-17 17:50 ` Mark Brown
@ 2009-08-18  2:53   ` Barry Song
  2009-08-18 13:11     ` Mark Brown
  0 siblings, 1 reply; 10+ messages in thread
From: Barry Song @ 2009-08-18  2:53 UTC (permalink / raw)
  To: Mark Brown; +Cc: uclinux-dist-devel, alsa-devel

On Tue, Aug 18, 2009 at 1:50 AM, Mark
Brown<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Aug 13, 2009 at 12:25:11PM +0800, Barry Song wrote:
>
>> For ad1938, the audio channel n just uses slot n, but for ad1836, it's
>> different, channel 0 uses slot 0, channel 1 uses slot 4, channels 2 uses
>> slot1, ... So add set_tdm_slot entry and use the mask field to define
>> the relationship between audio channel and slot number.
>
> I think what you're trying to describe here is that the device is
> expecting to see all the left channels on the bus followed by all the
> right channels?  This is the normal case for I2S TDM so no unusual
> configuration is required in order to implement it.
>
> I really don't think we should have Blackfin implementing a different
> API here - it makes the TDM slot API much harder to use.  Instead we
> should have a consistent API between all devices.  This may mean that we
> need to extend the APIs here.
>
Do you mean you will extend TDM API to handle this? I can follow your
on-going changes.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot
  2009-08-18  2:53   ` Barry Song
@ 2009-08-18 13:11     ` Mark Brown
  2009-08-19  1:34       ` Barry Song
  0 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2009-08-18 13:11 UTC (permalink / raw)
  To: Barry Song; +Cc: uclinux-dist-devel, alsa-devel

On Tue, Aug 18, 2009 at 10:53:39AM +0800, Barry Song wrote:
> On Tue, Aug 18, 2009 at 1:50 AM, Mark
> Brown<broonie@opensource.wolfsonmicro.com> wrote:

> > I think what you're trying to describe here is that the device is
> > expecting to see all the left channels on the bus followed by all the
> > right channels?  This is the normal case for I2S TDM so no unusual
> > configuration is required in order to implement it.

> Do you mean you will extend TDM API to handle this? I can follow your
> on-going changes.

It's possible, though I'd not hold my breath since I don't have any
systems which can use it.  Could you confirm which mode your device is
operating in for this configuration - if it's I2S then there should be
nothing needed.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot
  2009-08-18 13:11     ` Mark Brown
@ 2009-08-19  1:34       ` Barry Song
  2009-08-19 11:34         ` Mark Brown
  0 siblings, 1 reply; 10+ messages in thread
From: Barry Song @ 2009-08-19  1:34 UTC (permalink / raw)
  To: Mark Brown; +Cc: uclinux-dist-devel, alsa-devel

[-- Attachment #1: Type: text/plain, Size: 953 bytes --]

On Tue, Aug 18, 2009 at 9:11 PM, Mark
Brown<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, Aug 18, 2009 at 10:53:39AM +0800, Barry Song wrote:
>> On Tue, Aug 18, 2009 at 1:50 AM, Mark
>> Brown<broonie@opensource.wolfsonmicro.com> wrote:
>
>> > I think what you're trying to describe here is that the device is
>> > expecting to see all the left channels on the bus followed by all the
>> > right channels?  This is the normal case for I2S TDM so no unusual
>> > configuration is required in order to implement it.
>
>> Do you mean you will extend TDM API to handle this? I can follow your
>> on-going changes.
>
> It's possible, though I'd not hold my breath since I don't have any
> systems which can use it.  Could you confirm which mode your device is
> operating in for this configuration - if it's I2S then there should be
> nothing needed.
>
The timing is like the attached diagram, I think it's not I2S but like DSP.

[-- Attachment #2: 1836.GIF --]
[-- Type: image/gif, Size: 19139 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot
  2009-08-19  1:34       ` Barry Song
@ 2009-08-19 11:34         ` Mark Brown
  2009-08-20  2:37           ` Barry Song
  0 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2009-08-19 11:34 UTC (permalink / raw)
  To: Barry Song; +Cc: uclinux-dist-devel, alsa-devel

On Wed, Aug 19, 2009 at 09:34:16AM +0800, Barry Song wrote:

> The timing is like the attached diagram, I think it's not I2S but like DSP.

Yup, that DSP mode with I2S like channel layout (does it also do TDM in
I2S mode by any chance?).  Could you submit a version of your patch
which uses the default TDM layout - an API to rewrite things is likely
to be a separate API?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot
  2009-08-19 11:34         ` Mark Brown
@ 2009-08-20  2:37           ` Barry Song
  2009-08-28 12:42             ` Mark Brown
  0 siblings, 1 reply; 10+ messages in thread
From: Barry Song @ 2009-08-20  2:37 UTC (permalink / raw)
  To: Mark Brown; +Cc: uclinux-dist-devel, alsa-devel

On Wed, Aug 19, 2009 at 7:34 PM, Mark
Brown<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Aug 19, 2009 at 09:34:16AM +0800, Barry Song wrote:
>
>> The timing is like the attached diagram, I think it's not I2S but like DSP.
>
> Yup, that DSP mode with I2S like channel layout (does it also do TDM in
> I2S mode by any chance?).  Could you submit a version of your patch
> which uses the default TDM layout - an API to rewrite things is likely
> to be a separate API?
>
I don't know what is your exact meaning.  Do you mean I send a patch
whose tdm slot is not changed, then send a patch which change the tdm
slot?
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot
  2009-08-20  2:37           ` Barry Song
@ 2009-08-28 12:42             ` Mark Brown
  2009-08-31  3:14               ` Barry Song
  0 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2009-08-28 12:42 UTC (permalink / raw)
  To: Barry Song; +Cc: uclinux-dist-devel, alsa-devel

On Thu, 2009-08-20 at 10:37 +0800, Barry Song wrote:
> On Wed, Aug 19, 2009 at 7:34 PM, Mark
> Brown<broonie@opensource.wolfsonmicro.com> wrote:

> > Yup, that DSP mode with I2S like channel layout (does it also do TDM in
> > I2S mode by any chance?).  Could you submit a version of your patch
> > which uses the default TDM layout - an API to rewrite things is likely
> > to be a separate API?

> I don't know what is your exact meaning.  Do you mean I send a patch
> whose tdm slot is not changed, then send a patch which change the tdm
> slot?

Send a patch that allows selection of the active slots without rewriting

Although thinking about it I'm not sure if the driver is actually
implementing TDM or not - does the CPU actually get configured to stop
driving the data line during an inactive slot (so something else can
drive it) or is it simply sending zeros on those slots?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot
  2009-08-28 12:42             ` Mark Brown
@ 2009-08-31  3:14               ` Barry Song
  2009-08-31 12:01                 ` Mark Brown
  0 siblings, 1 reply; 10+ messages in thread
From: Barry Song @ 2009-08-31  3:14 UTC (permalink / raw)
  To: Mark Brown; +Cc: uclinux-dist-devel, alsa-devel

On Fri, Aug 28, 2009 at 8:42 PM, Mark
Brown<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, 2009-08-20 at 10:37 +0800, Barry Song wrote:
>> On Wed, Aug 19, 2009 at 7:34 PM, Mark
>> Brown<broonie@opensource.wolfsonmicro.com> wrote:
>
>> > Yup, that DSP mode with I2S like channel layout (does it also do TDM in
>> > I2S mode by any chance?).  Could you submit a version of your patch
>> > which uses the default TDM layout - an API to rewrite things is likely
>> > to be a separate API?
>
>> I don't know what is your exact meaning.  Do you mean I send a patch
>> whose tdm slot is not changed, then send a patch which change the tdm
>> slot?
>
> Send a patch that allows selection of the active slots without rewriting
>
Sorry. I am not sure about your meaning yet. The problem is the slot
order, the match between slot number and channel number, not only
which slot is active, and which is not active.

> Although thinking about it I'm not sure if the driver is actually
> implementing TDM or not - does the CPU actually get configured to stop
> driving the data line during an inactive slot (so something else can
> drive it) or is it simply sending zeros on those slots?

It's the later. The hardware signal is still active but no valid data
in inactive slot.
>
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot
  2009-08-31  3:14               ` Barry Song
@ 2009-08-31 12:01                 ` Mark Brown
  0 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2009-08-31 12:01 UTC (permalink / raw)
  To: Barry Song; +Cc: uclinux-dist-devel, alsa-devel

On Mon, 2009-08-31 at 11:14 +0800, Barry Song wrote:
> On Fri, Aug 28, 2009 at 8:42 PM, Mark
> Brown<broonie@opensource.wolfsonmicro.com> wrote:

> > Send a patch that allows selection of the active slots without rewriting

> Sorry. I am not sure about your meaning yet. The problem is the slot
> order, the match between slot number and channel number, not only
> which slot is active, and which is not active.

As I previously said I think we should add another API for that rather
than try to add the feature into the TDM slot API.

> > Although thinking about it I'm not sure if the driver is actually
> > implementing TDM or not - does the CPU actually get configured to stop
> > driving the data line during an inactive slot (so something else can
> > drive it) or is it simply sending zeros on those slots?

> It's the later. The hardware signal is still active but no valid data
> in inactive slot.

This is part of the reason - data slot reordering needn't mean actual
support for TDM (where the data lines are left undriven during idle
timeslots so that other devices can transmit in those slots). Indeed, as
your implementation shows it should be possible to provide a soft
implementation above the driver layer. Since Blackfin needs to copy the
data anyway there's an advantage to implementing in the Blackfin driver
but for most other devices there's no particular advantage in driver
support.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2009-08-31 12:01 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-08-13  4:25 [PATCH V2 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot Barry Song
2009-08-17 17:50 ` Mark Brown
2009-08-18  2:53   ` Barry Song
2009-08-18 13:11     ` Mark Brown
2009-08-19  1:34       ` Barry Song
2009-08-19 11:34         ` Mark Brown
2009-08-20  2:37           ` Barry Song
2009-08-28 12:42             ` Mark Brown
2009-08-31  3:14               ` Barry Song
2009-08-31 12:01                 ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.