[alsa-devel] [PATCH V2 2/3] add set_tdm_slot in tdm dai to define the relationship between audio channel and tdm slot
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
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.
On Tue, Aug 18, 2009 at 1:50 AM, Mark Brownbroonie@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.
On Tue, Aug 18, 2009 at 10:53:39AM +0800, Barry Song wrote:
On Tue, Aug 18, 2009 at 1:50 AM, Mark Brownbroonie@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.
On Tue, Aug 18, 2009 at 9:11 PM, Mark Brownbroonie@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 Brownbroonie@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.
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?
On Wed, Aug 19, 2009 at 7:34 PM, Mark Brownbroonie@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?
On Thu, 2009-08-20 at 10:37 +0800, Barry Song wrote:
On Wed, Aug 19, 2009 at 7:34 PM, Mark Brownbroonie@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?
On Fri, Aug 28, 2009 at 8:42 PM, Mark Brownbroonie@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 Brownbroonie@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.
On Mon, 2009-08-31 at 11:14 +0800, Barry Song wrote:
On Fri, Aug 28, 2009 at 8:42 PM, Mark Brownbroonie@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.
participants (2)
-
Barry Song -
Mark Brown