[PATCH v2] davinci-mcasp: Add support for multichannel playback

[PATCH v2] davinci-mcasp: Add support for multichannel playback

[Michal Bachraty]

Davinci McASP has support for I2S multichannel playback.
For I2S playback/receive, each serializer is capable to play 2 channels
(L/R) audio data.Serializer function (Playback-receive-none) is configured
in DT, depending on hardware specification. It is possible to play less
channels than configured in DT. For that purpose,only specific number of
active serializers are enabled. McASP FIFO need to have DMA transfer Bcnt
set to number of enabled serializers, otherwise no data are transfered to
McASP and Alsa generates "DMA/IRQ playback write error (DMA or IRQ trouble?)"
error.

Signed-off-by: Michal Bachraty <michal.bachraty@streamunlimited.com>
---
 sound/soc/davinci/davinci-mcasp.c |   56 ++++++++++++++++++++++++++++++++-----
 sound/soc/davinci/davinci-pcm.c   |   16 ++++++-----
 sound/soc/davinci/davinci-pcm.h   |    1 +
 3 files changed, 59 insertions(+), 14 deletions(-)

diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index afef3fb..b84bb73 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -235,6 +235,10 @@
 #define DISMOD		(val)(val<<2)
 #define TXSTATE		BIT(4)
 #define RXSTATE		BIT(5)
+#define SRMOD_MASK	3
+#define SRMOD_INACTIVE	0
+#define SRMOD_TX	1
+#define SRMOD_RX	2
 
 /*
  * DAVINCI_MCASP_LBCTL_REG - Loop Back Control Register Bits
@@ -657,12 +661,15 @@ static int davinci_config_channel_size(struct davinci_audio_dev *dev,
 	return 0;
 }
 
-static void davinci_hw_common_param(struct davinci_audio_dev *dev, int stream)
+static int davinci_hw_common_param(struct davinci_audio_dev *dev, int stream,
+				    int channels)
 {
 	int i;
 	u8 tx_ser = 0;
 	u8 rx_ser = 0;
-
+	u8 ser;
+	u8 slots = dev->tdm_slots;
+	u8 max_active_serializers = (channels + slots - 1) / slots;
 	/* Default configuration */
 	mcasp_set_bits(dev->base + DAVINCI_MCASP_PWREMUMGT_REG, MCASP_SOFT);
 
@@ -680,16 +687,42 @@ static void davinci_hw_common_param(struct davinci_audio_dev *dev, int stream)
 	}
 
 	for (i = 0; i < dev->num_serializer; i++) {
+		if (dev->serial_dir[i] == TX_MODE)
+			tx_ser++;
+		if (dev->serial_dir[i] == RX_MODE)
+			rx_ser++;
+	}
+
+	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
+		ser = tx_ser;
+	else
+		ser = rx_ser;
+
+	if (ser < max_active_serializers) {
+		dev_warn(dev->dev, "stream has more channels (%d) than are "
+			"enabled in mcasp (%d)\n", channels, ser * slots);
+		return -EINVAL;
+	}
+
+	tx_ser = 0;
+	rx_ser = 0;
+
+	for (i = 0; i < dev->num_serializer; i++) {
 		mcasp_set_bits(dev->base + DAVINCI_MCASP_XRSRCTL_REG(i),
 					dev->serial_dir[i]);
-		if (dev->serial_dir[i] == TX_MODE) {
+		if (dev->serial_dir[i] == TX_MODE &&
+					tx_ser < max_active_serializers) {
 			mcasp_set_bits(dev->base + DAVINCI_MCASP_PDIR_REG,
 					AXR(i));
 			tx_ser++;
-		} else if (dev->serial_dir[i] == RX_MODE) {
+		} else if (dev->serial_dir[i] == RX_MODE &&
+					rx_ser < max_active_serializers) {
 			mcasp_clr_bits(dev->base + DAVINCI_MCASP_PDIR_REG,
 					AXR(i));
 			rx_ser++;
+		} else {
+			mcasp_mod_bits(dev->base + DAVINCI_MCASP_XRSRCTL_REG(i),
+					SRMOD_INACTIVE, SRMOD_MASK);
 		}
 	}
 
@@ -729,6 +762,8 @@ static void davinci_hw_common_param(struct davinci_audio_dev *dev, int stream)
 				((dev->rxnumevt * rx_ser) << 8), NUMEVT_MASK);
 		}
 	}
+
+	return 0;
 }
 
 static void davinci_hw_param(struct davinci_audio_dev *dev, int stream)
@@ -812,8 +847,14 @@ static int davinci_mcasp_hw_params(struct snd_pcm_substream *substream,
 					&dev->dma_params[substream->stream];
 	int word_length;
 	u8 fifo_level;
+	u8 slots = dev->tdm_slots;
+	int channels;
+	struct snd_interval *pcm_channels = hw_param_interval(params,
+					SNDRV_PCM_HW_PARAM_CHANNELS);
+	channels = pcm_channels->min;
 
-	davinci_hw_common_param(dev, substream->stream);
+	if (davinci_hw_common_param(dev, substream->stream, channels))
+		return -EINVAL;
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 		fifo_level = dev->txnumevt;
 	else
@@ -862,6 +903,7 @@ static int davinci_mcasp_hw_params(struct snd_pcm_substream *substream,
 		dma_params->acnt = dma_params->data_type;
 
 	dma_params->fifo_level = fifo_level;
+	dma_params->active_serializers = (channels + slots - 1) / slots;
 	davinci_config_channel_size(dev, word_length);
 
 	return 0;
@@ -936,13 +978,13 @@ static struct snd_soc_dai_driver davinci_mcasp_dai[] = {
 		.name		= "davinci-mcasp.0",
 		.playback	= {
 			.channels_min	= 2,
-			.channels_max 	= 2,
+			.channels_max	= 8,
 			.rates 		= DAVINCI_MCASP_RATES,
 			.formats	= DAVINCI_MCASP_PCM_FMTS,
 		},
 		.capture 	= {
 			.channels_min 	= 2,
-			.channels_max 	= 2,
+			.channels_max	= 8,
 			.rates 		= DAVINCI_MCASP_RATES,
 			.formats	= DAVINCI_MCASP_PCM_FMTS,
 		},
diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c
index bb57552..3af8b50 100644
--- a/sound/soc/davinci/davinci-pcm.c
+++ b/sound/soc/davinci/davinci-pcm.c
@@ -182,6 +182,7 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
 	unsigned short acnt;
 	unsigned int count;
 	unsigned int fifo_level;
+	unsigned char serializers = prtd->params->active_serializers;
 
 	period_size = snd_pcm_lib_period_bytes(substream);
 	dma_offset = prtd->period * period_size;
@@ -195,14 +196,14 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
 	data_type = prtd->params->data_type;
 	count = period_size / data_type;
 	if (fifo_level)
-		count /= fifo_level;
+		count /= fifo_level * serializers;
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		src = dma_pos;
 		dst = prtd->params->dma_addr;
 		src_bidx = data_type;
-		dst_bidx = 0;
-		src_cidx = data_type * fifo_level;
+		dst_bidx = 4;
+		src_cidx = data_type * fifo_level * serializers;
 		dst_cidx = 0;
 	} else {
 		src = prtd->params->dma_addr;
@@ -210,7 +211,7 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
 		src_bidx = 0;
 		dst_bidx = data_type;
 		src_cidx = 0;
-		dst_cidx = data_type * fifo_level;
+		dst_cidx = data_type * fifo_level * serializers;
 	}
 
 	acnt = prtd->params->acnt;
@@ -224,9 +225,10 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
 		edma_set_transfer_params(prtd->asp_link[0], acnt, count, 1, 0,
 							ASYNC);
 	else
-		edma_set_transfer_params(prtd->asp_link[0], acnt, fifo_level,
-							count, fifo_level,
-							ABSYNC);
+		edma_set_transfer_params(prtd->asp_link[0], acnt,
+						fifo_level * serializers,
+						count, fifo_level * serializers,
+						ABSYNC);
 }
 
 static void davinci_pcm_dma_irq(unsigned link, u16 ch_status, void *data)
diff --git a/sound/soc/davinci/davinci-pcm.h b/sound/soc/davinci/davinci-pcm.h
index fbb710c..0d84d32 100644
--- a/sound/soc/davinci/davinci-pcm.h
+++ b/sound/soc/davinci/davinci-pcm.h
@@ -27,6 +27,7 @@ struct davinci_pcm_dma_params {
 	unsigned char data_type;	/* xfer data type */
 	unsigned char convert_mono_stereo;
 	unsigned int fifo_level;
+	unsigned char active_serializers; /* num. of active audio serializers */
 };
 
 int davinci_soc_platform_register(struct device *dev);
-- 
1.7.9.5

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Bedia, Vaibhav]

Hi Michal,

On Wed, Feb 27, 2013 at 22:08:45, Michal Bachraty wrote:
> Davinci McASP has support for I2S multichannel playback.
> For I2S playback/receive, each serializer is capable to play 2 channels
> (L/R) audio data.Serializer function (Playback-receive-none) is configured
> in DT, depending on hardware specification. It is possible to play less
> channels than configured in DT. For that purpose,only specific number of
> active serializers are enabled. McASP FIFO need to have DMA transfer Bcnt
> set to number of enabled serializers, otherwise no data are transfered to
> McASP and Alsa generates "DMA/IRQ playback write error (DMA or IRQ trouble?)"
> error.
> 

Thanks for looking into this. Before going into details, a few generic comments.
All serializers configured in Tx (or Rx) work off common clock generators and
hence the serializers will be operating in sync. I assume the setup that you
have matches this requirement. Based on the DMA programming assumed in the
implementation the user needs to ensure that buffer has the data in the right
format. Can you please describe the setup that you have and how you tested this? 

> Signed-off-by: Michal Bachraty <michal.bachraty@streamunlimited.com>
> ---
>  sound/soc/davinci/davinci-mcasp.c |   56 ++++++++++++++++++++++++++++++++-----
>  sound/soc/davinci/davinci-pcm.c   |   16 ++++++-----
>  sound/soc/davinci/davinci-pcm.h   |    1 +
>  3 files changed, 59 insertions(+), 14 deletions(-)
> 
> diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
> index afef3fb..b84bb73 100644
> --- a/sound/soc/davinci/davinci-mcasp.c
> +++ b/sound/soc/davinci/davinci-mcasp.c
> @@ -235,6 +235,10 @@
>  #define DISMOD		(val)(val<<2)
>  #define TXSTATE		BIT(4)
>  #define RXSTATE		BIT(5)
> +#define SRMOD_MASK	3
> +#define SRMOD_INACTIVE	0
> +#define SRMOD_TX	1
> +#define SRMOD_RX	2

I don't see SRMOD_TX/RX being used anywhere.

>  
>  /*
>   * DAVINCI_MCASP_LBCTL_REG - Loop Back Control Register Bits
> @@ -657,12 +661,15 @@ static int davinci_config_channel_size(struct davinci_audio_dev *dev,
>  	return 0;
>  }
>  
> -static void davinci_hw_common_param(struct davinci_audio_dev *dev, int stream)
> +static int davinci_hw_common_param(struct davinci_audio_dev *dev, int stream,
> +				    int channels)
>  {
>  	int i;
>  	u8 tx_ser = 0;
>  	u8 rx_ser = 0;
> -
> +	u8 ser;
> +	u8 slots = dev->tdm_slots;
> +	u8 max_active_serializers = (channels + slots - 1) / slots;
>  	/* Default configuration */
>  	mcasp_set_bits(dev->base + DAVINCI_MCASP_PWREMUMGT_REG, MCASP_SOFT);
>  
> @@ -680,16 +687,42 @@ static void davinci_hw_common_param(struct davinci_audio_dev *dev, int stream)
>  	}
>  
>  	for (i = 0; i < dev->num_serializer; i++) {
> +		if (dev->serial_dir[i] == TX_MODE)
> +			tx_ser++;
> +		if (dev->serial_dir[i] == RX_MODE)
> +			rx_ser++;
> +	}
> +
> +	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
> +		ser = tx_ser;
> +	else
> +		ser = rx_ser;
> +
> +	if (ser < max_active_serializers) {
> +		dev_warn(dev->dev, "stream has more channels (%d) than are "
> +			"enabled in mcasp (%d)\n", channels, ser * slots);
> +		return -EINVAL;
> +	}
> +
> +	tx_ser = 0;
> +	rx_ser = 0;

The number of active serializers is already being calculated below.

> +
> +	for (i = 0; i < dev->num_serializer; i++) {
>  		mcasp_set_bits(dev->base + DAVINCI_MCASP_XRSRCTL_REG(i),
>  					dev->serial_dir[i]);
> -		if (dev->serial_dir[i] == TX_MODE) {
> +		if (dev->serial_dir[i] == TX_MODE &&
> +					tx_ser < max_active_serializers) {
>  			mcasp_set_bits(dev->base + DAVINCI_MCASP_PDIR_REG,
>  					AXR(i));
>  			tx_ser++;
> -		} else if (dev->serial_dir[i] == RX_MODE) {
> +		} else if (dev->serial_dir[i] == RX_MODE &&
> +					rx_ser < max_active_serializers) {
>  			mcasp_clr_bits(dev->base + DAVINCI_MCASP_PDIR_REG,
>  					AXR(i));
>  			rx_ser++;
> +		} else {
> +			mcasp_mod_bits(dev->base + DAVINCI_MCASP_XRSRCTL_REG(i),
> +					SRMOD_INACTIVE, SRMOD_MASK);

Sorry I don't follow what you are trying to do here.

>  		}
>  	}
>  
> @@ -729,6 +762,8 @@ static void davinci_hw_common_param(struct davinci_audio_dev *dev, int stream)
>  				((dev->rxnumevt * rx_ser) << 8), NUMEVT_MASK);
>  		}
>  	}
> +
> +	return 0;
>  }
>  
>  static void davinci_hw_param(struct davinci_audio_dev *dev, int stream)
> @@ -812,8 +847,14 @@ static int davinci_mcasp_hw_params(struct snd_pcm_substream *substream,
>  					&dev->dma_params[substream->stream];
>  	int word_length;
>  	u8 fifo_level;
> +	u8 slots = dev->tdm_slots;
> +	int channels;
> +	struct snd_interval *pcm_channels = hw_param_interval(params,
> +					SNDRV_PCM_HW_PARAM_CHANNELS);
> +	channels = pcm_channels->min;
>  
> -	davinci_hw_common_param(dev, substream->stream);
> +	if (davinci_hw_common_param(dev, substream->stream, channels))
> +		return -EINVAL;
>  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
>  		fifo_level = dev->txnumevt;
>  	else
> @@ -862,6 +903,7 @@ static int davinci_mcasp_hw_params(struct snd_pcm_substream *substream,
>  		dma_params->acnt = dma_params->data_type;
>  
>  	dma_params->fifo_level = fifo_level;
> +	dma_params->active_serializers = (channels + slots - 1) / slots;
>  	davinci_config_channel_size(dev, word_length);
>  
>  	return 0;
> @@ -936,13 +978,13 @@ static struct snd_soc_dai_driver davinci_mcasp_dai[] = {
>  		.name		= "davinci-mcasp.0",
>  		.playback	= {
>  			.channels_min	= 2,
> -			.channels_max 	= 2,
> +			.channels_max	= 8,

Why are you setting this to 8?

>  			.rates 		= DAVINCI_MCASP_RATES,
>  			.formats	= DAVINCI_MCASP_PCM_FMTS,
>  		},
>  		.capture 	= {
>  			.channels_min 	= 2,
> -			.channels_max 	= 2,
> +			.channels_max	= 8,

Same here.
>  			.rates 		= DAVINCI_MCASP_RATES,
>  			.formats	= DAVINCI_MCASP_PCM_FMTS,
>  		},
> diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c
> index bb57552..3af8b50 100644
> --- a/sound/soc/davinci/davinci-pcm.c
> +++ b/sound/soc/davinci/davinci-pcm.c
> @@ -182,6 +182,7 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
>  	unsigned short acnt;
>  	unsigned int count;
>  	unsigned int fifo_level;
> +	unsigned char serializers = prtd->params->active_serializers;

Can you please describe what DMA configuration you want? I think you can
get rid of the active_serializers configuration by making use of tx_num_evt.

>  
>  	period_size = snd_pcm_lib_period_bytes(substream);
>  	dma_offset = prtd->period * period_size;
> @@ -195,14 +196,14 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
>  	data_type = prtd->params->data_type;
>  	count = period_size / data_type;
>  	if (fifo_level)
> -		count /= fifo_level;
> +		count /= fifo_level * serializers;

I think there's a problem is the way in which tx_num_evt is interpreted in the current code.
Instead of setting to fifo_level to tx_num_evt, if you set it to tx_num_evt * num_serializers
you can get rid of the additional code to take care of the number of serializers.

>  
>  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>  		src = dma_pos;
>  		dst = prtd->params->dma_addr;
>  		src_bidx = data_type;
> -		dst_bidx = 0;
> -		src_cidx = data_type * fifo_level;
> +		dst_bidx = 4;

Err.. this will most likely break other audio configurations. You should look at how to
avoid this change by making use of the mask and rotation operations in the McASP code.

> +		src_cidx = data_type * fifo_level * serializers;
>  		dst_cidx = 0;
>  	} else {
>  		src = prtd->params->dma_addr;
> @@ -210,7 +211,7 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
>  		src_bidx = 0;
>  		dst_bidx = data_type;
>  		src_cidx = 0;
> -		dst_cidx = data_type * fifo_level;
> +		dst_cidx = data_type * fifo_level * serializers;

With the change in fifo_level as described above, this won't be necessary.
>  	}
>  
>  	acnt = prtd->params->acnt;
> @@ -224,9 +225,10 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
>  		edma_set_transfer_params(prtd->asp_link[0], acnt, count, 1, 0,
>  							ASYNC);
>  	else
> -		edma_set_transfer_params(prtd->asp_link[0], acnt, fifo_level,
> -							count, fifo_level,
> -							ABSYNC);
> +		edma_set_transfer_params(prtd->asp_link[0], acnt,
> +						fifo_level * serializers,
> +						count, fifo_level * serializers,
> +						ABSYNC);

Same comment applies here.

Regards,
Vaibhav 

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Michal Bachraty]

Hi Vaibhav,

Please look at  [alsa-devel] [PATCH v3] davinci-mcasp: Add support for 
multichannel playback thread. 
There was some changes about computing number of channels. There is also 
described, how this driver works.
Also please look at [alsa-devel] davici-mcasp: "tx-num-evt" confusion with 
number of serializers thread, where are discussed using "tx-num-evt".

I tested driver with 2 - 4 - 6 - 8 channnels I2S (2 slots TDM) with using 1 -2 
-3 -4 serializers. and S16_LE (will do S24_3LE) audio.

Best,
Michal


On Tuesday, March 05, 2013 11:06:16 Bedia, Vaibhav wrote:
> Hi Michal,
> 
> On Wed, Feb 27, 2013 at 22:08:45, Michal Bachraty wrote:
> > Davinci McASP has support for I2S multichannel playback.
> > For I2S playback/receive, each serializer is capable to play 2 channels
> > (L/R) audio data.Serializer function (Playback-receive-none) is configured
> > in DT, depending on hardware specification. It is possible to play less
> > channels than configured in DT. For that purpose,only specific number of
> > active serializers are enabled. McASP FIFO need to have DMA transfer Bcnt
> > set to number of enabled serializers, otherwise no data are transfered to
> > McASP and Alsa generates "DMA/IRQ playback write error (DMA or IRQ
> > trouble?)" error.
> 
> Thanks for looking into this. Before going into details, a few generic
> comments. All serializers configured in Tx (or Rx) work off common clock
> generators and hence the serializers will be operating in sync. I assume
> the setup that you have matches this requirement. Based on the DMA
> programming assumed in the implementation the user needs to ensure that
> buffer has the data in the right format. Can you please describe the setup
> that you have and how you tested this?
> > Signed-off-by: Michal Bachraty <michal.bachraty@streamunlimited.com>
> > ---
> > 
> >  sound/soc/davinci/davinci-mcasp.c |   56
> >  ++++++++++++++++++++++++++++++++----- sound/soc/davinci/davinci-pcm.c  
> >  |   16 ++++++-----
> >  sound/soc/davinci/davinci-pcm.h   |    1 +
> >  3 files changed, 59 insertions(+), 14 deletions(-)
> > 
> > diff --git a/sound/soc/davinci/davinci-mcasp.c
> > b/sound/soc/davinci/davinci-mcasp.c index afef3fb..b84bb73 100644
> > --- a/sound/soc/davinci/davinci-mcasp.c
> > +++ b/sound/soc/davinci/davinci-mcasp.c
> > @@ -235,6 +235,10 @@
> > 
> >  #define DISMOD		(val)(val<<2)
> >  #define TXSTATE		BIT(4)
> >  #define RXSTATE		BIT(5)
> > 
> > +#define SRMOD_MASK	3
> > +#define SRMOD_INACTIVE	0
> > +#define SRMOD_TX	1
> > +#define SRMOD_RX	2
> 
> I don't see SRMOD_TX/RX being used anywhere.
> 
> >  /*
> >  
> >   * DAVINCI_MCASP_LBCTL_REG - Loop Back Control Register Bits
> > 
> > @@ -657,12 +661,15 @@ static int davinci_config_channel_size(struct
> > davinci_audio_dev *dev,> 
> >  	return 0;
> >  
> >  }
> > 
> > -static void davinci_hw_common_param(struct davinci_audio_dev *dev, int
> > stream) +static int davinci_hw_common_param(struct davinci_audio_dev
> > *dev, int stream, +				    int channels)
> > 
> >  {
> >  
> >  	int i;
> >  	u8 tx_ser = 0;
> >  	u8 rx_ser = 0;
> > 
> > -
> > +	u8 ser;
> > +	u8 slots = dev->tdm_slots;
> > +	u8 max_active_serializers = (channels + slots - 1) / slots;
> > 
> >  	/* Default configuration */
> >  	mcasp_set_bits(dev->base + DAVINCI_MCASP_PWREMUMGT_REG, MCASP_SOFT);
> > 
> > @@ -680,16 +687,42 @@ static void davinci_hw_common_param(struct
> > davinci_audio_dev *dev, int stream)> 
> >  	}
> >  	
> >  	for (i = 0; i < dev->num_serializer; i++) {
> > 
> > +		if (dev->serial_dir[i] == TX_MODE)
> > +			tx_ser++;
> > +		if (dev->serial_dir[i] == RX_MODE)
> > +			rx_ser++;
> > +	}
> > +
> > +	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
> > +		ser = tx_ser;
> > +	else
> > +		ser = rx_ser;
> > +
> > +	if (ser < max_active_serializers) {
> > +		dev_warn(dev->dev, "stream has more channels (%d) than are "
> > +			"enabled in mcasp (%d)\n", channels, ser * slots);
> > +		return -EINVAL;
> > +	}
> > +
> > +	tx_ser = 0;
> > +	rx_ser = 0;
> 
> The number of active serializers is already being calculated below.
> 
> > +
> > +	for (i = 0; i < dev->num_serializer; i++) {
> > 
> >  		mcasp_set_bits(dev->base + DAVINCI_MCASP_XRSRCTL_REG(i),
> >  		
> >  					dev->serial_dir[i]);
> > 
> > -		if (dev->serial_dir[i] == TX_MODE) {
> > +		if (dev->serial_dir[i] == TX_MODE &&
> > +					tx_ser < max_active_serializers) {
> > 
> >  			mcasp_set_bits(dev->base + DAVINCI_MCASP_PDIR_REG,
> >  			
> >  					AXR(i));
> >  			
> >  			tx_ser++;
> > 
> > -		} else if (dev->serial_dir[i] == RX_MODE) {
> > +		} else if (dev->serial_dir[i] == RX_MODE &&
> > +					rx_ser < max_active_serializers) {
> > 
> >  			mcasp_clr_bits(dev->base + DAVINCI_MCASP_PDIR_REG,
> >  			
> >  					AXR(i));
> >  			
> >  			rx_ser++;
> > 
> > +		} else {
> > +			mcasp_mod_bits(dev->base + DAVINCI_MCASP_XRSRCTL_REG(i),
> > +					SRMOD_INACTIVE, SRMOD_MASK);
> 
> Sorry I don't follow what you are trying to do here.
> 
> >  		}
> >  	
> >  	}
> > 
> > @@ -729,6 +762,8 @@ static void davinci_hw_common_param(struct
> > davinci_audio_dev *dev, int stream)> 
> >  				((dev->rxnumevt * rx_ser) << 8), NUMEVT_MASK);
> >  		
> >  		}
> >  	
> >  	}
> > 
> > +
> > +	return 0;
> > 
> >  }
> >  
> >  static void davinci_hw_param(struct davinci_audio_dev *dev, int stream)
> > 
> > @@ -812,8 +847,14 @@ static int davinci_mcasp_hw_params(struct
> > snd_pcm_substream *substream,> 
> >  					&dev->dma_params[substream->stream];
> >  	
> >  	int word_length;
> >  	u8 fifo_level;
> > 
> > +	u8 slots = dev->tdm_slots;
> > +	int channels;
> > +	struct snd_interval *pcm_channels = hw_param_interval(params,
> > +					SNDRV_PCM_HW_PARAM_CHANNELS);
> > +	channels = pcm_channels->min;
> > 
> > -	davinci_hw_common_param(dev, substream->stream);
> > +	if (davinci_hw_common_param(dev, substream->stream, channels))
> > +		return -EINVAL;
> > 
> >  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> >  	
> >  		fifo_level = dev->txnumevt;
> >  	
> >  	else
> > 
> > @@ -862,6 +903,7 @@ static int davinci_mcasp_hw_params(struct
> > snd_pcm_substream *substream,> 
> >  		dma_params->acnt = dma_params->data_type;
> >  	
> >  	dma_params->fifo_level = fifo_level;
> > 
> > +	dma_params->active_serializers = (channels + slots - 1) / slots;
> > 
> >  	davinci_config_channel_size(dev, word_length);
> >  	
> >  	return 0;
> > 
> > @@ -936,13 +978,13 @@ static struct snd_soc_dai_driver davinci_mcasp_dai[]
> > = {> 
> >  		.name		= "davinci-mcasp.0",
> >  		.playback	= {
> >  		
> >  			.channels_min	= 2,
> > 
> > -			.channels_max 	= 2,
> > +			.channels_max	= 8,
> 
> Why are you setting this to 8?
> 
> >  			.rates 		= DAVINCI_MCASP_RATES,
> >  			.formats	= DAVINCI_MCASP_PCM_FMTS,
> >  		
> >  		},
> >  		.capture 	= {
> >  		
> >  			.channels_min 	= 2,
> > 
> > -			.channels_max 	= 2,
> > +			.channels_max	= 8,
> 
> Same here.
> 
> >  			.rates 		= DAVINCI_MCASP_RATES,
> >  			.formats	= DAVINCI_MCASP_PCM_FMTS,
> >  		
> >  		},
> > 
> > diff --git a/sound/soc/davinci/davinci-pcm.c
> > b/sound/soc/davinci/davinci-pcm.c index bb57552..3af8b50 100644
> > --- a/sound/soc/davinci/davinci-pcm.c
> > +++ b/sound/soc/davinci/davinci-pcm.c
> > @@ -182,6 +182,7 @@ static void davinci_pcm_enqueue_dma(struct
> > snd_pcm_substream *substream)> 
> >  	unsigned short acnt;
> >  	unsigned int count;
> >  	unsigned int fifo_level;
> > 
> > +	unsigned char serializers = prtd->params->active_serializers;
> 
> Can you please describe what DMA configuration you want? I think you can
> get rid of the active_serializers configuration by making use of tx_num_evt.
> >  	period_size = snd_pcm_lib_period_bytes(substream);
> >  	dma_offset = prtd->period * period_size;
> > 
> > @@ -195,14 +196,14 @@ static void davinci_pcm_enqueue_dma(struct
> > snd_pcm_substream *substream)> 
> >  	data_type = prtd->params->data_type;
> >  	count = period_size / data_type;
> >  	if (fifo_level)
> > 
> > -		count /= fifo_level;
> > +		count /= fifo_level * serializers;
> 
> I think there's a problem is the way in which tx_num_evt is interpreted in
> the current code. Instead of setting to fifo_level to tx_num_evt, if you
> set it to tx_num_evt * num_serializers you can get rid of the additional
> code to take care of the number of serializers.
> >  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> >  	
> >  		src = dma_pos;
> >  		dst = prtd->params->dma_addr;
> >  		src_bidx = data_type;
> > 
> > -		dst_bidx = 0;
> > -		src_cidx = data_type * fifo_level;
> > +		dst_bidx = 4;
> 
> Err.. this will most likely break other audio configurations. You should
> look at how to avoid this change by making use of the mask and rotation
> operations in the McASP code.
> > +		src_cidx = data_type * fifo_level * serializers;
> > 
> >  		dst_cidx = 0;
> >  	
> >  	} else {
> >  	
> >  		src = prtd->params->dma_addr;
> > 
> > @@ -210,7 +211,7 @@ static void davinci_pcm_enqueue_dma(struct
> > snd_pcm_substream *substream)> 
> >  		src_bidx = 0;
> >  		dst_bidx = data_type;
> >  		src_cidx = 0;
> > 
> > -		dst_cidx = data_type * fifo_level;
> > +		dst_cidx = data_type * fifo_level * serializers;
> 
> With the change in fifo_level as described above, this won't be necessary.
> 
> >  	}
> >  	
> >  	acnt = prtd->params->acnt;
> > 
> > @@ -224,9 +225,10 @@ static void davinci_pcm_enqueue_dma(struct
> > snd_pcm_substream *substream)> 
> >  		edma_set_transfer_params(prtd->asp_link[0], acnt, count, 1, 0,
> >  		
> >  							ASYNC);
> >  	
> >  	else
> > 
> > -		edma_set_transfer_params(prtd->asp_link[0], acnt, fifo_level,
> > -							count, fifo_level,
> > -							ABSYNC);
> > +		edma_set_transfer_params(prtd->asp_link[0], acnt,
> > +						fifo_level * serializers,
> > +						count, fifo_level * serializers,
> > +						ABSYNC);
> 
> Same comment applies here.
> 
> Regards,
> Vaibhav

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Bedia, Vaibhav]

On Tue, Mar 05, 2013 at 17:11:20, Michal Bachraty wrote:
> Hi Vaibhav,
> 
> Please look at  [alsa-devel] [PATCH v3] davinci-mcasp: Add support for 
> multichannel playback thread. 
> There was some changes about computing number of channels. There is also 
> described, how this driver works.

Oops. I somehow missed v3 :(

I just had a look at it and most the comments that I have on v2 are still
applicable and IMO need to be addressed before getting merged. You also don't
have a change-log in v3 so I don't understand why you made some changes.

> Also please look at [alsa-devel] davici-mcasp: "tx-num-evt" confusion with 
> number of serializers thread, where are discussed using "tx-num-evt".
> 

Just noticed this one. Will respond to it.

> I tested driver with 2 - 4 - 6 - 8 channnels I2S (2 slots TDM) with using 1 -2 
> -3 -4 serializers. and S16_LE (will do S24_3LE) audio.

Did you use aplay for this? If so, what were the parameters used. If not, can you
describe how the userspace needs to manage the audio buffer? For anyone looking
at leveraging the multi-serializer feature this information is important to
highlight.

Regards,
Vaibhav 

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Michal Bachraty]

On Tuesday, March 05, 2013 12:03:45 Bedia, Vaibhav wrote:
> I just had a look at it and most the comments that I have on v2 are still
> applicable and IMO need to be addressed before getting merged. You also
> don't have a change-log in v3 so I don't understand why you made some
> changes.

I found that, there is need to upgrade for other TDM slots configs.

> > I tested driver with 2 - 4 - 6 - 8 channnels I2S (2 slots TDM) with using
> > 1 -2 -3 -4 serializers. and S16_LE (will do S24_3LE) audio.
> 
> Did you use aplay for this? If so, what were the parameters used. If not,
> can you describe how the userspace needs to manage the audio buffer? For
> anyone looking at leveraging the multi-serializer feature this information
> is important to highlight.

Yes aplay  -D "hw:1,0" -r48000 -fS16_LE -c6 < sine16_6ch.txt or similar. I 
hear 6ch audio on TAS5086. Also I had generated data and I saw data on 
oscilloscope.
This patch only exteds driver for using more serializers. When you run 2ch. or 
one serializer), there is same configuration  as was in previous driver 
version.

Best,
Michal

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Daniel Mack]

Hi Vaibhav,
Hi Michal,

On 05.03.2013 12:06, Bedia, Vaibhav wrote:
> On Wed, Feb 27, 2013 at 22:08:45, Michal Bachraty wrote:
>> Davinci McASP has support for I2S multichannel playback.
>> For I2S playback/receive, each serializer is capable to play 2 channels
>> (L/R) audio data.Serializer function (Playback-receive-none) is configured
>> in DT, depending on hardware specification. It is possible to play less
>> channels than configured in DT. For that purpose,only specific number of
>> active serializers are enabled. McASP FIFO need to have DMA transfer Bcnt
>> set to number of enabled serializers, otherwise no data are transfered to
>> McASP and Alsa generates "DMA/IRQ playback write error (DMA or IRQ trouble?)"
>> error.
> 
> Thanks for looking into this. Before going into details, a few generic comments.

To explain: Michal and me are working on the same hardware - a custom
made AM33xx board which has a version with multichannel output.

> All serializers configured in Tx (or Rx) work off common clock generators and
> hence the serializers will be operating in sync. I assume the setup that you
> have matches this requirement. Based on the DMA programming assumed in the
> implementation the user needs to ensure that buffer has the data in the right
> format. Can you please describe the setup that you have and how you tested this? 

As Michal described, we used a board with a multichannel Codec on it,
which connects 3 of its I2S inputs to the AM33xx's AXR data lines.

Software wise, we tested with 'aplay -cX', and that seems to work just fine.


> 
>> Signed-off-by: Michal Bachraty <michal.bachraty@streamunlimited.com>
>> ---
>>  sound/soc/davinci/davinci-mcasp.c |   56 ++++++++++++++++++++++++++++++++-----
>>  sound/soc/davinci/davinci-pcm.c   |   16 ++++++-----
>>  sound/soc/davinci/davinci-pcm.h   |    1 +
>>  3 files changed, 59 insertions(+), 14 deletions(-)
>>
>> diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
>> index afef3fb..b84bb73 100644
>> --- a/sound/soc/davinci/davinci-mcasp.c
>> +++ b/sound/soc/davinci/davinci-mcasp.c
>> @@ -235,6 +235,10 @@
>>  #define DISMOD		(val)(val<<2)
>>  #define TXSTATE		BIT(4)
>>  #define RXSTATE		BIT(5)
>> +#define SRMOD_MASK	3
>> +#define SRMOD_INACTIVE	0
>> +#define SRMOD_TX	1
>> +#define SRMOD_RX	2
> 
> I don't see SRMOD_TX/RX being used anywhere.

That's true, they can be dropped.

> 
>>  
>>  /*
>>   * DAVINCI_MCASP_LBCTL_REG - Loop Back Control Register Bits
>> @@ -657,12 +661,15 @@ static int davinci_config_channel_size(struct davinci_audio_dev *dev,
>>  	return 0;
>>  }
>>  
>> -static void davinci_hw_common_param(struct davinci_audio_dev *dev, int stream)
>> +static int davinci_hw_common_param(struct davinci_audio_dev *dev, int stream,
>> +				    int channels)
>>  {
>>  	int i;
>>  	u8 tx_ser = 0;
>>  	u8 rx_ser = 0;
>> -
>> +	u8 ser;
>> +	u8 slots = dev->tdm_slots;
>> +	u8 max_active_serializers = (channels + slots - 1) / slots;
>>  	/* Default configuration */
>>  	mcasp_set_bits(dev->base + DAVINCI_MCASP_PWREMUMGT_REG, MCASP_SOFT);
>>  
>> @@ -680,16 +687,42 @@ static void davinci_hw_common_param(struct davinci_audio_dev *dev, int stream)
>>  	}
>>  
>>  	for (i = 0; i < dev->num_serializer; i++) {
>> +		if (dev->serial_dir[i] == TX_MODE)
>> +			tx_ser++;
>> +		if (dev->serial_dir[i] == RX_MODE)
>> +			rx_ser++;
>> +	}
>> +
>> +	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
>> +		ser = tx_ser;
>> +	else
>> +		ser = rx_ser;
>> +
>> +	if (ser < max_active_serializers) {
>> +		dev_warn(dev->dev, "stream has more channels (%d) than are "
>> +			"enabled in mcasp (%d)\n", channels, ser * slots);
>> +		return -EINVAL;
>> +	}
>> +
>> +	tx_ser = 0;
>> +	rx_ser = 0;
> 
> The number of active serializers is already being calculated below.

True. The code can be shifted around so the calculation is only done once.

>> +
>> +	for (i = 0; i < dev->num_serializer; i++) {
>>  		mcasp_set_bits(dev->base + DAVINCI_MCASP_XRSRCTL_REG(i),
>>  					dev->serial_dir[i]);
>> -		if (dev->serial_dir[i] == TX_MODE) {
>> +		if (dev->serial_dir[i] == TX_MODE &&
>> +					tx_ser < max_active_serializers) {
>>  			mcasp_set_bits(dev->base + DAVINCI_MCASP_PDIR_REG,
>>  					AXR(i));
>>  			tx_ser++;
>> -		} else if (dev->serial_dir[i] == RX_MODE) {
>> +		} else if (dev->serial_dir[i] == RX_MODE &&
>> +					rx_ser < max_active_serializers) {
>>  			mcasp_clr_bits(dev->base + DAVINCI_MCASP_PDIR_REG,
>>  					AXR(i));
>>  			rx_ser++;
>> +		} else {
>> +			mcasp_mod_bits(dev->base + DAVINCI_MCASP_XRSRCTL_REG(i),
>> +					SRMOD_INACTIVE, SRMOD_MASK);
> 
> Sorry I don't follow what you are trying to do here.
> 
>>  		}
>>  	}
>>  
>> @@ -729,6 +762,8 @@ static void davinci_hw_common_param(struct davinci_audio_dev *dev, int stream)
>>  				((dev->rxnumevt * rx_ser) << 8), NUMEVT_MASK);
>>  		}
>>  	}
>> +
>> +	return 0;
>>  }
>>  
>>  static void davinci_hw_param(struct davinci_audio_dev *dev, int stream)
>> @@ -812,8 +847,14 @@ static int davinci_mcasp_hw_params(struct snd_pcm_substream *substream,
>>  					&dev->dma_params[substream->stream];
>>  	int word_length;
>>  	u8 fifo_level;
>> +	u8 slots = dev->tdm_slots;
>> +	int channels;
>> +	struct snd_interval *pcm_channels = hw_param_interval(params,
>> +					SNDRV_PCM_HW_PARAM_CHANNELS);
>> +	channels = pcm_channels->min;
>>  
>> -	davinci_hw_common_param(dev, substream->stream);
>> +	if (davinci_hw_common_param(dev, substream->stream, channels))
>> +		return -EINVAL;
>>  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
>>  		fifo_level = dev->txnumevt;
>>  	else
>> @@ -862,6 +903,7 @@ static int davinci_mcasp_hw_params(struct snd_pcm_substream *substream,
>>  		dma_params->acnt = dma_params->data_type;
>>  
>>  	dma_params->fifo_level = fifo_level;
>> +	dma_params->active_serializers = (channels + slots - 1) / slots;
>>  	davinci_config_channel_size(dev, word_length);
>>  
>>  	return 0;
>> @@ -936,13 +978,13 @@ static struct snd_soc_dai_driver davinci_mcasp_dai[] = {
>>  		.name		= "davinci-mcasp.0",
>>  		.playback	= {
>>  			.channels_min	= 2,
>> -			.channels_max 	= 2,
>> +			.channels_max	= 8,
> 
> Why are you setting this to 8?

Well, the ASoC core will look at this field when parsing the dai links,
and will build a sound card that has min(codec_dai->channels_max,
cpu_dai->channels_max) channels. Hence, this number has to reflect the
maximum possible output channels for this DAI. In v3, it's actually set
to 512. Or was that not your question?

I'll let Michal comment on the rest, as it's his work. Note though that
v3 of this patch has already been applied to Mark's tree, but we can
easily fix the details you mentioned with another patch.

Thanks a lot for your thorough review!


Daniel



> 
>>  			.rates 		= DAVINCI_MCASP_RATES,
>>  			.formats	= DAVINCI_MCASP_PCM_FMTS,
>>  		},
>>  		.capture 	= {
>>  			.channels_min 	= 2,
>> -			.channels_max 	= 2,
>> +			.channels_max	= 8,
> 
> Same here.
>>  			.rates 		= DAVINCI_MCASP_RATES,
>>  			.formats	= DAVINCI_MCASP_PCM_FMTS,
>>  		},
>> diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c
>> index bb57552..3af8b50 100644
>> --- a/sound/soc/davinci/davinci-pcm.c
>> +++ b/sound/soc/davinci/davinci-pcm.c
>> @@ -182,6 +182,7 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
>>  	unsigned short acnt;
>>  	unsigned int count;
>>  	unsigned int fifo_level;
>> +	unsigned char serializers = prtd->params->active_serializers;
> 
> Can you please describe what DMA configuration you want? I think you can
> get rid of the active_serializers configuration by making use of tx_num_evt.
> 
>>  
>>  	period_size = snd_pcm_lib_period_bytes(substream);
>>  	dma_offset = prtd->period * period_size;
>> @@ -195,14 +196,14 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
>>  	data_type = prtd->params->data_type;
>>  	count = period_size / data_type;
>>  	if (fifo_level)
>> -		count /= fifo_level;
>> +		count /= fifo_level * serializers;
> 
> I think there's a problem is the way in which tx_num_evt is interpreted in the current code.
> Instead of setting to fifo_level to tx_num_evt, if you set it to tx_num_evt * num_serializers
> you can get rid of the additional code to take care of the number of serializers.
> 
>>  
>>  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>>  		src = dma_pos;
>>  		dst = prtd->params->dma_addr;
>>  		src_bidx = data_type;
>> -		dst_bidx = 0;
>> -		src_cidx = data_type * fifo_level;
>> +		dst_bidx = 4;
> 
> Err.. this will most likely break other audio configurations. You should look at how to
> avoid this change by making use of the mask and rotation operations in the McASP code.
> 
>> +		src_cidx = data_type * fifo_level * serializers;
>>  		dst_cidx = 0;
>>  	} else {
>>  		src = prtd->params->dma_addr;
>> @@ -210,7 +211,7 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
>>  		src_bidx = 0;
>>  		dst_bidx = data_type;
>>  		src_cidx = 0;
>> -		dst_cidx = data_type * fifo_level;
>> +		dst_cidx = data_type * fifo_level * serializers;
> 
> With the change in fifo_level as described above, this won't be necessary.
>>  	}
>>  
>>  	acnt = prtd->params->acnt;
>> @@ -224,9 +225,10 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
>>  		edma_set_transfer_params(prtd->asp_link[0], acnt, count, 1, 0,
>>  							ASYNC);
>>  	else
>> -		edma_set_transfer_params(prtd->asp_link[0], acnt, fifo_level,
>> -							count, fifo_level,
>> -							ABSYNC);
>> +		edma_set_transfer_params(prtd->asp_link[0], acnt,
>> +						fifo_level * serializers,
>> +						count, fifo_level * serializers,
>> +						ABSYNC);
> 
> Same comment applies here.
> 
> Regards,
> Vaibhav 
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Bedia, Vaibhav]

Hi Daniel,

On Tue, Mar 05, 2013 at 21:31:59, Daniel Mack wrote:
[...]
> 
> As Michal described, we used a board with a multichannel Codec on it,
> which connects 3 of its I2S inputs to the AM33xx's AXR data lines.
> 
> Software wise, we tested with 'aplay -cX', and that seems to work just fine.
> 

Since you are using a multichannel codec things are a lot simplified for you :)
Someone might want to hook up multiple codecs to get multi-channel behavior.
There will be only 1 CPU DAI but there can be upto 16 CODEC DAIs operating in
sync. I haven't really followed the recent ASoC changes so I don't know whether
something like this can be handled right now.

Mark, Liam,

Any suggestions?

[...]
> >>  	return 0;
> >> @@ -936,13 +978,13 @@ static struct snd_soc_dai_driver davinci_mcasp_dai[] = {
> >>  		.name		= "davinci-mcasp.0",
> >>  		.playback	= {
> >>  			.channels_min	= 2,
> >> -			.channels_max 	= 2,
> >> +			.channels_max	= 8,
> > 
> > Why are you setting this to 8?
> 
> Well, the ASoC core will look at this field when parsing the dai links,
> and will build a sound card that has min(codec_dai->channels_max,
> cpu_dai->channels_max) channels. Hence, this number has to reflect the
> maximum possible output channels for this DAI. In v3, it's actually set
> to 512. Or was that not your question?

32 slots on a max of 16 serializers gives the max as 512 and hence I wanted
to know why 8 was selected.

Regards,
Vaibhav

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 642 bytes --]

On Wed, Mar 06, 2013 at 10:54:52AM +0000, Bedia, Vaibhav wrote:

> Since you are using a multichannel codec things are a lot simplified for you :)
> Someone might want to hook up multiple codecs to get multi-channel behavior.
> There will be only 1 CPU DAI but there can be upto 16 CODEC DAIs operating in
> sync. I haven't really followed the recent ASoC changes so I don't know whether
> something like this can be handled right now.

> Mark, Liam,

> Any suggestions?

Nobody with any interest in such systems has stepped up to work on the
framework, and of course relatively little CPU hardware supports
multiple streams on a single DAI.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Daniel Mack]

On 06.03.2013 11:54, Bedia, Vaibhav wrote:
> Hi Daniel,
> 
> On Tue, Mar 05, 2013 at 21:31:59, Daniel Mack wrote:
> [...]
>>
>> As Michal described, we used a board with a multichannel Codec on it,
>> which connects 3 of its I2S inputs to the AM33xx's AXR data lines.
>>
>> Software wise, we tested with 'aplay -cX', and that seems to work just fine.
>>
> 
> Since you are using a multichannel codec things are a lot simplified for you :)
> Someone might want to hook up multiple codecs to get multi-channel behavior.
> There will be only 1 CPU DAI but there can be upto 16 CODEC DAIs operating in
> sync. I haven't really followed the recent ASoC changes so I don't know whether
> something like this can be handled right now.

I would rather instanciate multiple McASP cores for such setups, so you
can (in theory) also make them operate on different bit depths and clock
dividers. You think that's possible as well?


Thanks,
Daniel

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Bedia, Vaibhav]

On Wed, Mar 06, 2013 at 16:30:48, Daniel Mack wrote:
> On 06.03.2013 11:54, Bedia, Vaibhav wrote:
> > Hi Daniel,
> > 
> > On Tue, Mar 05, 2013 at 21:31:59, Daniel Mack wrote:
> > [...]
> >>
> >> As Michal described, we used a board with a multichannel Codec on it,
> >> which connects 3 of its I2S inputs to the AM33xx's AXR data lines.
> >>
> >> Software wise, we tested with 'aplay -cX', and that seems to work just fine.
> >>
> > 
> > Since you are using a multichannel codec things are a lot simplified for you :)
> > Someone might want to hook up multiple codecs to get multi-channel behavior.
> > There will be only 1 CPU DAI but there can be upto 16 CODEC DAIs operating in
> > sync. I haven't really followed the recent ASoC changes so I don't know whether
> > something like this can be handled right now.
> 
> I would rather instanciate multiple McASP cores for such setups, so you
> can (in theory) also make them operate on different bit depths and clock
> dividers. You think that's possible as well?
> 

Yeah. Using multiple McASPs would be much simpler to handle both in s/w and h/w.

Regards,
Vaibhav

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Bedia, Vaibhav]

On Wed, Mar 06, 2013 at 16:27:15, Mark Brown wrote:
> On Wed, Mar 06, 2013 at 10:54:52AM +0000, Bedia, Vaibhav wrote:
> 
> > Since you are using a multichannel codec things are a lot simplified for you :)
> > Someone might want to hook up multiple codecs to get multi-channel behavior.
> > There will be only 1 CPU DAI but there can be upto 16 CODEC DAIs operating in
> > sync. I haven't really followed the recent ASoC changes so I don't know whether
> > something like this can be handled right now.
> 
> > Mark, Liam,
> 
> > Any suggestions?
> 
> Nobody with any interest in such systems has stepped up to work on the
> framework, and of course relatively little CPU hardware supports
> multiple streams on a single DAI.
> 

Ok. Thinking more on this I guess the situation I described above will indeed be a rare
case. There would be additional complexity in ensuring that the codecs are always operating
in sync. It would perhaps be simpler to cascade the codecs (if supported) and use only one
interface to transfer the data. Or just use multichannel codec like Michal and Daniel
are doing.

Regards,
Vaibhav

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Michal Bachraty]

Hi Vaibhav,
 
> > >>  	return 0;
> > >> 
> > >> @@ -936,13 +978,13 @@ static struct snd_soc_dai_driver
> > >> davinci_mcasp_dai[] = {> >> 
> > >>  		.name		= "davinci-mcasp.0",
> > >>  		.playback	= {
> > >>  		
> > >>  			.channels_min	= 2,
> > >> 
> > >> -			.channels_max 	= 2,
> > >> +			.channels_max	= 8,
> > > 
> > > Why are you setting this to 8?
> > 
> > Well, the ASoC core will look at this field when parsing the dai links,
> > and will build a sound card that has min(codec_dai->channels_max,
> > cpu_dai->channels_max) channels. Hence, this number has to reflect the
> > maximum possible output channels for this DAI. In v3, it's actually set
> > to 512. Or was that not your question?
> 
> 32 slots on a max of 16 serializers gives the max as 512 and hence I wanted
> to know why 8 was selected.

That number was changed in patch v3 to 512.  8 was selected only for Am335 and 
2TDM slots. That number was not general and therefore it was changed.  It is 
no need for discusion for that number, because it was changed to proper value 
in v3 of this patch. If not, let us know.

Michal.

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Bedia, Vaibhav]

On Wed, Mar 06, 2013 at 19:57:35, Michal Bachraty wrote:
[...] 
> That number was changed in patch v3 to 512.  8 was selected only for Am335 and 
> 2TDM slots. That number was not general and therefore it was changed.  It is 
> no need for discusion for that number, because it was changed to proper value 
> in v3 of this patch. If not, let us know.
> 

Yeah, that number is fine now. You can ignore this comment.

Regards,
Vaibhav

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Michal Bachraty]

 Hi Vaibhav,
 Hi Daniel.

Daniel, thaks for comments, Vaibhav thanks for review!
Please 
On Tuesday, March 05, 2013 17:01:59 Daniel Mack wrote:
> I'll let Michal comment on the rest, as it's his work. Note though that
> v3 of this patch has already been applied to Mark's tree, but we can
> easily fix the details you mentioned with another patch.
> 
This is part for me, so:

> 
> >>  			.rates 		= DAVINCI_MCASP_RATES,
> >>  			.formats	= DAVINCI_MCASP_PCM_FMTS,
> >>  		
> >>  		},
> >>  		.capture 	= {
> >>  		
> >>  			.channels_min 	= 2,
> >> 
> >> -			.channels_max 	= 2,
> >> +			.channels_max	= 8,
> > 
> > Same here.
> > 
> >>  			.rates 		= DAVINCI_MCASP_RATES,
> >>  			.formats	= DAVINCI_MCASP_PCM_FMTS,
> >>  		
> >>  		},
> >> 
> >> diff --git a/sound/soc/davinci/davinci-pcm.c
> >> b/sound/soc/davinci/davinci-pcm.c index bb57552..3af8b50 100644
> >> --- a/sound/soc/davinci/davinci-pcm.c
> >> +++ b/sound/soc/davinci/davinci-pcm.c
> >> @@ -182,6 +182,7 @@ static void davinci_pcm_enqueue_dma(struct
> >> snd_pcm_substream *substream)>> 
> >>  	unsigned short acnt;
> >>  	unsigned int count;
> >>  	unsigned int fifo_level;
> >> 
> >> +	unsigned char serializers = prtd->params->active_serializers;
> > 
> > Can you please describe what DMA configuration you want? I think you can
> > get rid of the active_serializers configuration by making use of
> > tx_num_evt.> 
> >>  	period_size = snd_pcm_lib_period_bytes(substream);
> >>  	dma_offset = prtd->period * period_size;
> >> 
> >> @@ -195,14 +196,14 @@ static void davinci_pcm_enqueue_dma(struct
> >> snd_pcm_substream *substream)>> 
> >>  	data_type = prtd->params->data_type;
> >>  	count = period_size / data_type;
> >>  	if (fifo_level)
> >> 
> >> -		count /= fifo_level;
> >> +		count /= fifo_level * serializers;
> > 
> > I think there's a problem is the way in which tx_num_evt is interpreted in
> > the current code. Instead of setting to fifo_level to tx_num_evt, if you
> > set it to tx_num_evt * num_serializers you can get rid of the additional
> > code to take care of the number of serializers.> 

Yes, threre is misleading with tx_num_evt in DT. This misleading is discussed 
in [alsa-devel] davici-mcasp: "tx-num-evt" confusion with number of 
serializers. thread. 

Also this code misleads fifo_level usage, so maybe that code can be upgraded. 

In davinci-mcasp code, there is fifo_level defined as txnum
	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
		fifo_level = dev->txnumevt;
	else
		fifo_level = dev->rxnumevt;



> >>  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> >>  	
> >>  		src = dma_pos;
> >>  		dst = prtd->params->dma_addr;
> >>  		src_bidx = data_type;
> >> 
> >> -		dst_bidx = 0;
> >> -		src_cidx = data_type * fifo_level;
> >> +		dst_bidx = 4;
> > 
> > Err.. this will most likely break other audio configurations. You should
> > look at how to avoid this change by making use of the mask and rotation
> > operations in the McASP code.> 

McAsp FIFO and McASP data port reg. is 32 bit wide, each new sample is copied 
to McASP after McAsp send DMA event. McASP also do data rotations and masking. 
When more serializers are enabled, each new sample for new serializer should 
be placed to new data room at +4 address. After that new DMA event is 
generated.
I tested it with TDM slots = 2 and also 6 with one serializer and also two 
serializer enabled. 
Why do you think that brokes other audio configurations? 

> >> +		src_cidx = data_type * fifo_level * serializers;
> >> 
> >>  		dst_cidx = 0;
> >>  	
> >>  	} else {
> >>  	
> >>  		src = prtd->params->dma_addr;
> >> 
> >> @@ -210,7 +211,7 @@ static void davinci_pcm_enqueue_dma(struct
> >> snd_pcm_substream *substream)>> 
> >>  		src_bidx = 0;
> >>  		dst_bidx = data_type;
> >>  		src_cidx = 0;
> >> 
> >> -		dst_cidx = data_type * fifo_level;
> >> +		dst_cidx = data_type * fifo_level * serializers;
> > 
> > With the change in fifo_level as described above, this won't be necessary.

Described upper and also in Re: [alsa-devel] davici-mcasp: "tx-num-evt" 
confusion with number of serializers thread

Best 
Michal.

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Bedia, Vaibhav]

On Wed, Mar 06, 2013 at 20:14:59, Michal Bachraty wrote:
[...]
> 
> Yes, threre is misleading with tx_num_evt in DT. This misleading is discussed 
> in [alsa-devel] davici-mcasp: "tx-num-evt" confusion with number of 
> serializers. thread. 
> 

I responded to that thread yesterday. Can you please check that.
 
> > >>  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> > >>  	
> > >>  		src = dma_pos;
> > >>  		dst = prtd->params->dma_addr;
> > >>  		src_bidx = data_type;
> > >> 
> > >> -		dst_bidx = 0;
> > >> -		src_cidx = data_type * fifo_level;
> > >> +		dst_bidx = 4;
> > > 
> > > Err.. this will most likely break other audio configurations. You should
> > > look at how to avoid this change by making use of the mask and rotation
> > > operations in the McASP code.> 
> 
> McAsp FIFO and McASP data port reg. is 32 bit wide, each new sample is copied 
> to McASP after McAsp send DMA event. McASP also do data rotations and masking. 
> When more serializers are enabled, each new sample for new serializer should 
> be placed to new data room at +4 address. After that new DMA event is 
> generated.
> I tested it with TDM slots = 2 and also 6 with one serializer and also two 
> serializer enabled. 
> Why do you think that brokes other audio configurations? 
> 

Refer to section 22.3.10.1.2 of the AM335x TRM.

"To access through the data port, simply have the CPU or DMA access the XRBUF
through its data port location. Through the data port, the DMA/CPU can service
all the serializers through a single address. The McASP automatically cycles
through the appropriate serializers.

For transmit operations through the data port, the DMA/CPU should write to the
same XBUF data port address to service all of the active transmit serializers.
In addition, the DMA/CPU should write to the XBUF for all active transmit
serializers"

The DMA controller is expected to write to the same address. I am a bit
surprised the +4 thing works without issue. Still, you should not rely
on that since that's not supported as per the specs.

Regards,
Vaibhav

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Mike Looijmans]

On 03/06/2013 12:19 PM, Bedia, Vaibhav wrote:
> On Wed, Mar 06, 2013 at 16:27:15, Mark Brown wrote:
>> On Wed, Mar 06, 2013 at 10:54:52AM +0000, Bedia, Vaibhav wrote:
>>
>>> Since you are using a multichannel codec things are a lot simplified for you :)
>>> Someone might want to hook up multiple codecs to get multi-channel behavior.
>>> There will be only 1 CPU DAI but there can be upto 16 CODEC DAIs operating in
>>> sync. I haven't really followed the recent ASoC changes so I don't know whether
>>> something like this can be handled right now.
>>
>>> Mark, Liam,
>>
>>> Any suggestions?
>>
>> Nobody with any interest in such systems has stepped up to work on the
>> framework, and of course relatively little CPU hardware supports
>> multiple streams on a single DAI.
>>
>
> Ok. Thinking more on this I guess the situation I described above will indeed be a rare
> case. There would be additional complexity in ensuring that the codecs are always operating
> in sync. It would perhaps be simpler to cascade the codecs (if supported) and use only one
> interface to transfer the data. Or just use multichannel codec like Michal and Daniel
> are doing.

I have 9 codecs connected, and the main problem to solve was to get the 
system to start ALL of them instead of just one.

I solved that by creating a "multiplex" codec, which in itself doesn't 
do anything. It exports a mixer where you can switch on/off the codecs 
you want to use, and it forwards all commands (_ops) to the codecs that 
are enabled. The serializer settings are sent to the asp driver as if 
they are TDM masks (using the bits to enable serializers instead).

In userland you see 10 codecs, you can use each separately, or use the 
10th codec (the multiplexer) to use any combination of other codecs.

-- 
Mike Looijmans - Topic Automation

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Daniel Mack]

Hi Vaibhav,
Hi Michal,

On 05.03.2013 12:06, Bedia, Vaibhav wrote:
> On Wed, Feb 27, 2013 at 22:08:45, Michal Bachraty wrote:
>> Davinci McASP has support for I2S multichannel playback.
>> For I2S playback/receive, each serializer is capable to play 2 channels
>> (L/R) audio data.Serializer function (Playback-receive-none) is configured
>> in DT, depending on hardware specification. It is possible to play less
>> channels than configured in DT. For that purpose,only specific number of
>> active serializers are enabled. McASP FIFO need to have DMA transfer Bcnt
>> set to number of enabled serializers, otherwise no data are transfered to
>> McASP and Alsa generates "DMA/IRQ playback write error (DMA or IRQ trouble?)"
>> error.
>>
> 
> Thanks for looking into this. Before going into details, a few generic comments.
> All serializers configured in Tx (or Rx) work off common clock generators and
> hence the serializers will be operating in sync. I assume the setup that you
> have matches this requirement. Based on the DMA programming assumed in the
> implementation the user needs to ensure that buffer has the data in the right
> format. Can you please describe the setup that you have and how you tested this? 
> 
>> Signed-off-by: Michal Bachraty <michal.bachraty@streamunlimited.com>
>> ---
>>  sound/soc/davinci/davinci-mcasp.c |   56 ++++++++++++++++++++++++++++++++-----
>>  sound/soc/davinci/davinci-pcm.c   |   16 ++++++-----
>>  sound/soc/davinci/davinci-pcm.h   |    1 +
>>  3 files changed, 59 insertions(+), 14 deletions(-)
>>
>> diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
>> index afef3fb..b84bb73 100644
>> --- a/sound/soc/davinci/davinci-mcasp.c
>> +++ b/sound/soc/davinci/davinci-mcasp.c
>> @@ -235,6 +235,10 @@
>>  #define DISMOD		(val)(val<<2)
>>  #define TXSTATE		BIT(4)
>>  #define RXSTATE		BIT(5)
>> +#define SRMOD_MASK	3
>> +#define SRMOD_INACTIVE	0
>> +#define SRMOD_TX	1
>> +#define SRMOD_RX	2
> 
> I don't see SRMOD_TX/RX being used anywhere.
> 
>>  
>>  /*
>>   * DAVINCI_MCASP_LBCTL_REG - Loop Back Control Register Bits
>> @@ -657,12 +661,15 @@ static int davinci_config_channel_size(struct davinci_audio_dev *dev,
>>  	return 0;
>>  }
>>  
>> -static void davinci_hw_common_param(struct davinci_audio_dev *dev, int stream)
>> +static int davinci_hw_common_param(struct davinci_audio_dev *dev, int stream,
>> +				    int channels)
>>  {
>>  	int i;
>>  	u8 tx_ser = 0;
>>  	u8 rx_ser = 0;
>> -
>> +	u8 ser;
>> +	u8 slots = dev->tdm_slots;
>> +	u8 max_active_serializers = (channels + slots - 1) / slots;
>>  	/* Default configuration */
>>  	mcasp_set_bits(dev->base + DAVINCI_MCASP_PWREMUMGT_REG, MCASP_SOFT);
>>  
>> @@ -680,16 +687,42 @@ static void davinci_hw_common_param(struct davinci_audio_dev *dev, int stream)
>>  	}
>>  
>>  	for (i = 0; i < dev->num_serializer; i++) {
>> +		if (dev->serial_dir[i] == TX_MODE)
>> +			tx_ser++;
>> +		if (dev->serial_dir[i] == RX_MODE)
>> +			rx_ser++;
>> +	}
>> +
>> +	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
>> +		ser = tx_ser;
>> +	else
>> +		ser = rx_ser;
>> +
>> +	if (ser < max_active_serializers) {
>> +		dev_warn(dev->dev, "stream has more channels (%d) than are "
>> +			"enabled in mcasp (%d)\n", channels, ser * slots);
>> +		return -EINVAL;
>> +	}
>> +
>> +	tx_ser = 0;
>> +	rx_ser = 0;
> 
> The number of active serializers is already being calculated below.

FWIW, I'm now sending a cleanup patch for the two details I quoted
above. With those out of the way, the rest (namely the dst_bidx issue)
can be solved separately.

Please have a look and tell me if you're ok with that.


Thanks for your review!

Daniel

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Mike Looijmans]

On 03/06/2013 12:19 PM, Bedia, Vaibhav wrote:
> On Wed, Mar 06, 2013 at 16:30:48, Daniel Mack wrote:
>> On 06.03.2013 11:54, Bedia, Vaibhav wrote:
>>> Hi Daniel,
>>>
>>> On Tue, Mar 05, 2013 at 21:31:59, Daniel Mack wrote:
>>> [...]
>>>>
>>>> As Michal described, we used a board with a multichannel Codec on it,
>>>> which connects 3 of its I2S inputs to the AM33xx's AXR data lines.
>>>>
>>>> Software wise, we tested with 'aplay -cX', and that seems to work just fine.
>>>>
>>>
>>> Since you are using a multichannel codec things are a lot simplified for you :)
>>> Someone might want to hook up multiple codecs to get multi-channel behavior.
>>> There will be only 1 CPU DAI but there can be upto 16 CODEC DAIs operating in
>>> sync. I haven't really followed the recent ASoC changes so I don't know whether
>>> something like this can be handled right now.
>>
>> I would rather instanciate multiple McASP cores for such setups, so you
>> can (in theory) also make them operate on different bit depths and clock
>> dividers. You think that's possible as well?
>
> Yeah. Using multiple McASPs would be much simpler to handle both in s/w and h/w.

Would it?

The McASP has multiple serializers, but they all share the same clocks 
and format settings. The data will come in as "L1 L2 L3 .. R1 R2 R3 .." 
so the code would have keep track of which channels have to go where, 
and rearrange it to the clients. And it must be able to dynamically 
add/remove channels without disturbing the other streams.

So unless you want to physically add more McASP hardware to the system, 
I don't see how this would be "simpler" to handle than other options.

A way to "group" the codecs together is much simpler to implement (which 
is simply proven by the simple fact that I implemented it on my system) 
and it makes it very easy to control on the user side too - the 
selection of codecs is just a mixer control.

Mike.

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Bedia, Vaibhav]

On Fri, Mar 08, 2013 at 12:59:49, Mike Looijmans wrote:
> 
> I have 9 codecs connected, and the main problem to solve was to get the 
> system to start ALL of them instead of just one.
> 
> I solved that by creating a "multiplex" codec, which in itself doesn't 
> do anything. It exports a mixer where you can switch on/off the codecs 
> you want to use, and it forwards all commands (_ops) to the codecs that 
> are enabled. The serializer settings are sent to the asp driver as if 
> they are TDM masks (using the bits to enable serializers instead).
> 
> In userland you see 10 codecs, you can use each separately, or use the 
> 10th codec (the multiplexer) to use any combination of other codecs.
> 

Interesting approach. I was thinking more on the lines of how to expose
the different serializer-codec pairs to the core. Since there's only one
DMA interface for the different serializers doing this in a generic
manner looked complicated, at least to me.

I am aware of couple of multi-channel systems that preferred the approach
that I mentioned. In one, a cascading of codecs was preferred so as to free
up the extra serializer pads for different functions at the SoC level. 
There was a custom app to split the data coming in from different channels
but since it freed up a few pads, this approach was preferred. In another,
sending SPDIF data to an HDMI transmitter was preferred, again to free up
some pads. It finally depends on what the use-case is and which system level
constraints are more important to satisfy.

It would be good if you could post the changes that made so that we can
discuss on how the approach in detail and possibly enhance the core to
handle the multi-codec scenarios that use-cases like your require.

Regards,
Vaibhav

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Bedia, Vaibhav]

On Fri, Mar 08, 2013 at 19:52:09, Mike Looijmans wrote:
> On 03/06/2013 12:19 PM, Bedia, Vaibhav wrote:
> > On Wed, Mar 06, 2013 at 16:30:48, Daniel Mack wrote:
> >> On 06.03.2013 11:54, Bedia, Vaibhav wrote:
> >>> Hi Daniel,
> >>>
> >>> On Tue, Mar 05, 2013 at 21:31:59, Daniel Mack wrote:
> >>> [...]
> >>>>
> >>>> As Michal described, we used a board with a multichannel Codec on it,
> >>>> which connects 3 of its I2S inputs to the AM33xx's AXR data lines.
> >>>>
> >>>> Software wise, we tested with 'aplay -cX', and that seems to work just fine.
> >>>>
> >>>
> >>> Since you are using a multichannel codec things are a lot simplified for you :)
> >>> Someone might want to hook up multiple codecs to get multi-channel behavior.
> >>> There will be only 1 CPU DAI but there can be upto 16 CODEC DAIs operating in
> >>> sync. I haven't really followed the recent ASoC changes so I don't know whether
> >>> something like this can be handled right now.
> >>
> >> I would rather instanciate multiple McASP cores for such setups, so you
> >> can (in theory) also make them operate on different bit depths and clock
> >> dividers. You think that's possible as well?
> >
> > Yeah. Using multiple McASPs would be much simpler to handle both in s/w and h/w.
> 
> Would it?
> 
> The McASP has multiple serializers, but they all share the same clocks 
> and format settings. The data will come in as "L1 L2 L3 .. R1 R2 R3 .." 
> so the code would have keep track of which channels have to go where, 
> and rearrange it to the clients. And it must be able to dynamically 
> add/remove channels without disturbing the other streams.
> 

I don't see how the grouping of codecs like you are mentioning helps
add/remove channels while capture/playback in on-going. By implementing
a dummy codec you can select which serializers are enabled/disabled but
won't the selection have to be be before starting any streaming since you
can't update the DMA controller configuration on the fly. Or maybe that's
what you meant by add/remove channels without disturbing the other streams.

> So unless you want to physically add more McASP hardware to the system, 
> I don't see how this would be "simpler" to handle than other options.
> 
> A way to "group" the codecs together is much simpler to implement (which 
> is simply proven by the simple fact that I implemented it on my system) 
> and it makes it very easy to control on the user side too - the 
> selection of codecs is just a mixer control.
> 

It's good that you have working a implementation. Like I mentioned in the
other thread it would be good if you can post the changes that you did
so that the approach can be discussed in detail, obviously assuming you want
something like to be handled in mainline code.

Regards,
Vaibhav

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Mike Looijmans]

On 03/11/2013 08:01 AM, Bedia, Vaibhav wrote:
> On Fri, Mar 08, 2013 at 12:59:49, Mike Looijmans wrote:
>>
>> I have 9 codecs connected, and the main problem to solve was to get the
>> system to start ALL of them instead of just one.
>>
>> I solved that by creating a "multiplex" codec, which in itself doesn't
>> do anything. It exports a mixer where you can switch on/off the codecs
>> you want to use, and it forwards all commands (_ops) to the codecs that
>> are enabled. The serializer settings are sent to the asp driver as if
>> they are TDM masks (using the bits to enable serializers instead).
>>
>> In userland you see 10 codecs, you can use each separately, or use the
>> 10th codec (the multiplexer) to use any combination of other codecs.
>>
>
> Interesting approach. I was thinking more on the lines of how to expose
> the different serializer-codec pairs to the core. Since there's only one
> DMA interface for the different serializers doing this in a generic
> manner looked complicated, at least to me.
>
> I am aware of couple of multi-channel systems that preferred the approach
> that I mentioned. In one, a cascading of codecs was preferred so as to free
> up the extra serializer pads for different functions at the SoC level.
> There was a custom app to split the data coming in from different channels
> but since it freed up a few pads, this approach was preferred. In another,
> sending SPDIF data to an HDMI transmitter was preferred, again to free up
> some pads. It finally depends on what the use-case is and which system level
> constraints are more important to satisfy.
>
> It would be good if you could post the changes that made so that we can
> discuss on how the approach in detail and possibly enhance the core to
> handle the multi-codec scenarios that use-cases like your require.

I'll post the code, but it's based on a 2.6.37 kernel and it's tailored 
to "my" platform, with numerous changes to clock settings and such. So 
don't expect it to be useful. It's only interesting as an "example" or 
for illustrating the general idea, but entirely useless for inclusion in 
mainstream.

-- 
Mike Looijmans - Topic Automation

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Bedia, Vaibhav]

On Mon, Mar 11, 2013 at 14:03:50, Mike Looijmans wrote:
> 
> I'll post the code, but it's based on a 2.6.37 kernel and it's tailored 
> to "my" platform, with numerous changes to clock settings and such. So 
> don't expect it to be useful. It's only interesting as an "example" or 
> for illustrating the general idea, but entirely useless for inclusion in 
> mainstream.

Thanks. In my opinion any proof of concept will be good to get some
discussion going on this topic.

Regards,
Vaibhav

Can DAPM paths span multiple codecs/drivers?

[Mike Looijmans]

If two codecs need each other for certain operations, is there a way to 
define DAPM links between them?

For example, codec A has an analog input "x". It can route this input 
via some output pin to an input of codec B. (In other words, A could be 
a codec that provides an SPDIF output and B could be a codec that can 
only handle digital inputs like the SPDIF from codec A).

Now codec B can record from input "x", but to do so, it would have to 
activate a path leading through codec A. (hence, codec A will need to be 
a bit active)

I have a suspicion that this is possible by hand-crafting some of the 
dapm structs and linking them together. Before I try, I would like to 
know if there's even a possiblity that this will work from folks more 
knowledgable on this topic.

My current implementation is to tell DAPM that i have an "always 
enabled" ADC path and just do everything myself. Which is of course 
wasteful.

Mike.

Re: [PATCH v2] davinci-mcasp: Add support for multichannel playback

[Mike Looijmans]

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

On 03/11/2013 09:56 AM, Bedia, Vaibhav wrote:
> On Mon, Mar 11, 2013 at 14:03:50, Mike Looijmans wrote:
>>
>> I'll post the code, but it's based on a 2.6.37 kernel and it's tailored
>> to "my" platform, with numerous changes to clock settings and such. So
>> don't expect it to be useful. It's only interesting as an "example" or
>> for illustrating the general idea, but entirely useless for inclusion in
>> mainstream.
>
> Thanks. In my opinion any proof of concept will be good to get some
> discussion going on this topic.

It has been way to long since I last replied, and there's still much to do.

Anyway, I attached the "multiplex" codec source code. The multiplex 
assumes that all its child codecs share the same (cpu) dai and that the 
dai is capable of handling multiple codecs (either using TDM or multiple 
pins, or whatever combination). The machine part must take care of 
forwarding the "mask" setting of the multiplexer and translate it to 
something the CPU DAI understands.
(If the codecs do not share the DAI, you don't need the multiplexer...)

The multiplex only supports capture.

To use it, compile it in (change Kconfig) and have the board file 
register codecs with the multiplexer. The multiplexer creates a mixer 
switch control for each codec. To use a set of codecs, set their switch 
to "on" (which is default) and record from the multiplex codec. The 
multiplex will control the codecs.

Codecs can also be used without the multiplex in between, no change there.

See it as a proof of concept. It works fine for me...

Mike.

[-- Attachment #2: multiplex.c --]
[-- Type: text/x-csrc, Size: 12131 bytes --]

/*
*/

#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/init.h>
#include <linux/delay.h>
#include <linux/pm.h>
#include <linux/platform_device.h>
#include <linux/cdev.h>
#include <linux/slab.h>

#include <sound/core.h>
#include <sound/pcm.h>
#include <sound/pcm_params.h>
#include <sound/soc.h>
#include <sound/soc-dapm.h>
#include <sound/initval.h>
#include <sound/tlv.h>

#include "multiplex.h"

#define MULTIPLEX_INPUT_SELECT	0x10

struct multiplex_priv {
	struct snd_soc_dai **dai_list;
	unsigned int* active_codec_mask;
	struct snd_kcontrol_new dapm_mixer_inputs[MULTIPLEX_MAX_NR_CODECS];
	struct soc_mixer_control dapm_mixer_inputs_private[MULTIPLEX_MAX_NR_CODECS];
	struct snd_soc_dapm_widget dapm_mixer;
};


static int multiplex_write(struct snd_soc_codec *codec, unsigned int reg,
				unsigned int val)
{
	struct multiplex_priv *multiplex = snd_soc_codec_get_drvdata(codec);

	/*printk(KERN_WARNING "%s (reg=0x%x, val=0x%x)\n", __FUNCTION__, reg, val);*/
	switch(reg)
	{
		case MULTIPLEX_INPUT_SELECT:
			*multiplex->active_codec_mask = val;
			return 0;
	}
	return 0;
}

static unsigned int multiplex_read(struct snd_soc_codec *codec, unsigned int reg)
{
	struct multiplex_priv *multiplex = snd_soc_codec_get_drvdata(codec);

	/*printk(KERN_WARNING "%s (reg=0x%x)\n", __FUNCTION__, reg);*/
	switch(reg)
	{
		case MULTIPLEX_INPUT_SELECT:
			return *multiplex->active_codec_mask;
	}
	return 0;
}

static const struct snd_kcontrol_new input_mixer_controls[] = {
	SOC_DAPM_SINGLE("Switch 0", MULTIPLEX_INPUT_SELECT, 0, 1, 0),
	SOC_DAPM_SINGLE("Switch 1", MULTIPLEX_INPUT_SELECT, 1, 1, 0),
	SOC_DAPM_SINGLE("Switch 2", MULTIPLEX_INPUT_SELECT, 2, 1, 0),
	SOC_DAPM_SINGLE("Switch 3", MULTIPLEX_INPUT_SELECT, 3, 1, 0),
	SOC_DAPM_SINGLE("Switch 4", MULTIPLEX_INPUT_SELECT, 4, 1, 0),
	SOC_DAPM_SINGLE("Switch 5", MULTIPLEX_INPUT_SELECT, 5, 1, 0),
	SOC_DAPM_SINGLE("Switch 6", MULTIPLEX_INPUT_SELECT, 6, 1, 0),
};


static const struct snd_soc_dapm_widget multiplex_dapm_widgets[] = {
	SND_SOC_DAPM_ADC("ADC", "Capture", SND_SOC_NOPM, 0, 0),
	SND_SOC_DAPM_INPUT("IN"),
};

static const struct snd_soc_dapm_route multiplex_dapm_routes[] = {
/*	{"Input Mixer", "Switch 0", "IN"},
	{"Input Mixer", "Switch 1", "IN"},
	{"Input Mixer", "Switch 2", "IN"},
	{"Input Mixer", "Switch 3", "IN"},
	{"Input Mixer", "Switch 4", "IN"},
	{"Input Mixer", "Switch 5", "IN"},
	{"Input Mixer", "Switch 6", "IN"},*/
	{"ADC", NULL, "Input Mixer"},
};

static int multiplex_set_dai_sysclk(struct snd_soc_dai *dai,
                                int clk_id, unsigned int freq, int dir)
{
	struct snd_soc_codec *codec = dai->codec;
	struct multiplex_priv *multiplex = snd_soc_codec_get_drvdata(codec);
	unsigned int active_codec_mask = *multiplex->active_codec_mask;
	unsigned int i;
	int ret;

	printk(KERN_DEBUG "%s(%d, %u)", __func__, clk_id, freq);

	for (i = 0; multiplex->dai_list[i] != NULL; ++i)
	{
		if (active_codec_mask & BIT(i))
		{
			struct snd_soc_dai *codec_dai =  multiplex->dai_list[i];
			if (codec_dai->driver->ops->set_sysclk) {
				ret = codec_dai->driver->ops->set_sysclk(codec_dai, clk_id, freq, dir);
				if (ret < 0) {
					printk(KERN_ERR "multiplex: can't set codec %s sysclk\n",
						codec_dai->name);
					return ret;
				}
			}
		}
	}

	return 0; /* We don't care */
}

static int multiplex_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
{
	/* We only support I2S, just check if that's set and do nothing */
	if ((fmt & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBM_CFM)
		return -EINVAL;
	if ((fmt & SND_SOC_DAIFMT_FORMAT_MASK) != SND_SOC_DAIFMT_I2S)
		return -EINVAL;

	return 0;
}

static int multiplex_hw_params(struct snd_pcm_substream *substream,
			     struct snd_pcm_hw_params *params,
			     struct snd_soc_dai *dai)
{
	struct snd_soc_codec *codec = dai->codec;
	struct multiplex_priv *multiplex = snd_soc_codec_get_drvdata(codec);
	unsigned int active_codec_mask = *multiplex->active_codec_mask;
	unsigned int i;
	int ret;

	for (i = 0; multiplex->dai_list[i] != NULL; ++i)
	{
		if (active_codec_mask & BIT(i))
		{
			struct snd_soc_dai *codec_dai =  multiplex->dai_list[i];
			if (codec_dai->driver->ops->hw_params) {
				ret = codec_dai->driver->ops->hw_params(substream, params, codec_dai);
				if (ret < 0) {
					printk(KERN_ERR "multiplex: can't set codec %s hw params\n",
						codec_dai->name);
					return ret;
				}
			}
		}
	}

	return 0;
}


static void multiplex_shutdown(struct snd_pcm_substream *substream,
				 struct snd_soc_dai *dai)
{
	struct snd_soc_pcm_runtime *rtd = substream->private_data;
	struct snd_soc_codec *codec = rtd->codec;
	struct multiplex_priv *multiplex = snd_soc_codec_get_drvdata(codec);
	unsigned int active_codec_mask = *multiplex->active_codec_mask;
	unsigned int i;
	struct snd_soc_pcm_runtime rtd_fake;
	const char* stream_name = rtd->codec_dai->driver->capture.stream_name;

	printk(KERN_DEBUG "%s (%s)\n", __func__, stream_name);

	/* snd_soc_dapm_stream_event only uses the "codec" field of the
	 * rtd struct, that's why this trick works */
	rtd_fake.pcm = rtd->pcm;
	rtd_fake.platform = rtd->platform;
	rtd_fake.codec_dai = rtd->codec_dai;
	rtd_fake.cpu_dai = rtd->cpu_dai;


	for (i = 0; multiplex->dai_list[i] != NULL; ++i)
	{
		if (active_codec_mask & BIT(i))
		{
			struct snd_soc_dai *codec_dai =  multiplex->dai_list[i];
			rtd_fake.codec = codec_dai->codec;
			/* Send shutdown event first (just like core) */
			if (codec_dai->driver->ops->shutdown) {
				codec_dai->driver->ops->shutdown(substream, codec_dai);
			}
			/* Tell DAPM we don't need the path any longer */
			snd_soc_dapm_stream_event(&rtd_fake, stream_name, SND_SOC_DAPM_STREAM_STOP);
		}
	}
}

static int multiplex_mute(struct snd_soc_dai *dai, int mute)
{
	struct snd_soc_codec *codec = dai->codec;
	struct multiplex_priv *multiplex = snd_soc_codec_get_drvdata(codec);
	unsigned int active_codec_mask = *multiplex->active_codec_mask;
	unsigned int i;
	int ret = 0;

	for (i = 0; multiplex->dai_list[i] != NULL; ++i)
	{
		if (active_codec_mask & BIT(i))
		{
			struct snd_soc_dai *codec_dai =  multiplex->dai_list[i];
			if (codec_dai->driver->ops->digital_mute) {
				int r =codec_dai->driver->ops->digital_mute(codec_dai, mute);
				if (r != 0)
					ret = r;
			}
		}
	}

	return ret;
}


static int multiplex_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
{
	struct snd_soc_pcm_runtime *rtd = substream->private_data;
	struct multiplex_priv *multiplex = snd_soc_codec_get_drvdata(rtd->codec);
	unsigned int active_codec_mask = *multiplex->active_codec_mask;
	unsigned int i;
	const char* stream_name = rtd->codec_dai->driver->capture.stream_name;
	struct snd_soc_pcm_runtime rtd_fake;

	printk(KERN_DEBUG "%s (%s)\n", __func__, stream_name);

	/* snd_soc_dapm_stream_event only uses the "codec" field of the
	 * rtd struct, that's why this trick works */
	rtd_fake.pcm = rtd->pcm;
	rtd_fake.platform = rtd->platform;
	rtd_fake.codec_dai = rtd->codec_dai;
	rtd_fake.cpu_dai = rtd->cpu_dai;

	for (i = 0; multiplex->dai_list[i] != NULL; ++i)
	{
		if (active_codec_mask & BIT(i))
		{
			struct snd_soc_codec *client =  multiplex->dai_list[i]->codec;
			if (!client)
			{
				printk(KERN_WARNING "multiplex: No CODEC for %d\n", i);
				continue;
			}
			rtd_fake.codec = client;
			snd_soc_dapm_stream_event(&rtd_fake, stream_name, SND_SOC_DAPM_STREAM_START);
		}
	}

	return 0;
}


#define MULTIPLEX_RATES	(SNDRV_PCM_RATE_KNOT | SNDRV_PCM_RATE_8000_96000)
#define MULTIPLEX_FORMATS	(SNDRV_PCM_FMTBIT_S16_LE \
			 | SNDRV_PCM_FMTBIT_S20_3LE \
			 | SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S24_LE \
			 | SNDRV_PCM_FMTBIT_S32_LE \
			 )

static struct snd_soc_dai_ops multiplex_ops = {
	.set_sysclk = multiplex_set_dai_sysclk,
	.hw_params = multiplex_hw_params,
	.shutdown = multiplex_shutdown,
	.digital_mute = multiplex_mute,
	.set_fmt = multiplex_set_dai_fmt,
	.prepare = multiplex_prepare,
};

static struct snd_soc_dai_driver multiplex_dais[] = {
	{
	.name = "multiplex-adc",
	.capture = {
		    .stream_name = "Capture",
		    .channels_min = 2,
		    .channels_max = 2 * MULTIPLEX_MAX_NR_CODECS,
		    .rates = MULTIPLEX_RATES,
		    .rate_min = 8000,
		    .rate_max = 51200,
		    .formats = MULTIPLEX_FORMATS,},
	.ops = &multiplex_ops,
	.symmetric_rates = 1,
	},
};

static int multiplex_probe(struct snd_soc_codec *codec)
{
	struct multiplex_priv *multiplex = snd_soc_codec_get_drvdata(codec);
	unsigned int i;
	unsigned int num_children;

	codec->idle_bias_off = 1;

	/* Add some default routes for DAPM so that it knows we're ready to go */
	snd_soc_dapm_new_controls(codec, multiplex_dapm_widgets,
				ARRAY_SIZE(multiplex_dapm_widgets));
	for (i = 0; multiplex->dai_list[i] != NULL; ++i)
	{
		printk(KERN_DEBUG "multiplex %d: dai=%s codec=%s\n", i, multiplex->dai_list[i]->name, multiplex->dai_list[i]->codec->name);
		multiplex->dapm_mixer_inputs_private[i].reg = MULTIPLEX_INPUT_SELECT;
		multiplex->dapm_mixer_inputs_private[i].shift = i;
		multiplex->dapm_mixer_inputs_private[i].rshift = i;
		multiplex->dapm_mixer_inputs_private[i].max = 1;
		multiplex->dapm_mixer_inputs_private[i].platform_max = 1;
		multiplex->dapm_mixer_inputs_private[i].invert = 0;
		multiplex->dapm_mixer_inputs[i].iface = SNDRV_CTL_ELEM_IFACE_MIXER;
		multiplex->dapm_mixer_inputs[i].name = (char*)multiplex->dai_list[i]->codec->name;
		multiplex->dapm_mixer_inputs[i].info = snd_soc_info_volsw;
		multiplex->dapm_mixer_inputs[i].get = snd_soc_dapm_get_volsw;
		multiplex->dapm_mixer_inputs[i].put = snd_soc_dapm_put_volsw;
		multiplex->dapm_mixer_inputs[i].private_value = (unsigned long)&multiplex->dapm_mixer_inputs_private[i];
	}
	num_children = i;
	multiplex->dapm_mixer.id = snd_soc_dapm_mixer_named_ctl; /* or 'snd_soc_dapm_mixer' for a prefix*/
	multiplex->dapm_mixer.name = "Input Mixer";
	multiplex->dapm_mixer.reg = SND_SOC_NOPM;
	multiplex->dapm_mixer.shift = 0;
	multiplex->dapm_mixer.invert = 0;
	multiplex->dapm_mixer.kcontrols = multiplex->dapm_mixer_inputs;
	multiplex->dapm_mixer.num_kcontrols = num_children;
	snd_soc_dapm_new_controls(codec, &multiplex->dapm_mixer, 1);
	/* Set up route for switch (Input Mixer must exist when we do this) */
	for (i = 0; multiplex->dai_list[i] != NULL; ++i)
	{
		struct snd_soc_dapm_route this_route =
			{"Input Mixer", multiplex->dai_list[i]->codec->name, "IN"};
		snd_soc_dapm_add_routes(codec, &this_route, 1);
	}
	snd_soc_dapm_add_routes(codec, multiplex_dapm_routes,
				ARRAY_SIZE(multiplex_dapm_routes));
	snd_soc_dapm_enable_pin(codec, "IN");
	snd_soc_dapm_new_widgets(codec);
	return 0;
}


static struct snd_soc_codec_driver soc_codec_dev_multiplex = {
	.read = multiplex_read,
	.write = multiplex_write,
	.probe = multiplex_probe,
};

static __devinit int multiplex_platform_probe(struct platform_device *pdev)
{
	struct multiplex_priv *multiplex;
	int ret;
	struct multiplex_platform_data *pdata = pdev->dev.platform_data;

	multiplex = kzalloc(sizeof(struct multiplex_priv), GFP_KERNEL);
	if (multiplex == NULL)
		return -ENOMEM;

	multiplex->dai_list = pdata->dai_list;
	multiplex->active_codec_mask = pdata->active_mask;

	platform_set_drvdata(pdev, multiplex);

	ret = snd_soc_register_codec(&pdev->dev,
			&soc_codec_dev_multiplex, multiplex_dais, ARRAY_SIZE(multiplex_dais));
	if (ret < 0)
		kfree(multiplex);
	return ret;
}

static __devexit int multiplex_platform_remove(struct platform_device *pdev)
{
	snd_soc_unregister_codec(&pdev->dev);
	kfree(platform_get_drvdata(pdev));
	return 0;
}

static struct platform_driver multiplex_codec_driver = {
	.driver = {
			.name = "multiplex-codec",
			.owner = THIS_MODULE,
	},
	.probe = multiplex_platform_probe,
	.remove = __devexit_p(multiplex_platform_remove),
};

static int __init multiplex_modinit(void)
{
	int ret;
	ret = platform_driver_register(&multiplex_codec_driver);
	return ret;
}
module_init(multiplex_modinit);

static void __exit multiplex_exit(void)
{
	platform_driver_unregister(&multiplex_codec_driver);
}
module_exit(multiplex_exit);

MODULE_DESCRIPTION("ASoC multiplex codec driver");
MODULE_AUTHOR("Mike Looijmans");
MODULE_LICENSE("GPL");
MODULE_ALIAS("platform:multiplex-codec");

[-- Attachment #3: multiplex.h --]
[-- Type: text/x-chdr, Size: 132 bytes --]


#define MULTIPLEX_MAX_NR_CODECS	8

struct multiplex_platform_data {
	struct snd_soc_dai **dai_list;
	unsigned int* active_mask;
};

[-- Attachment #4: Type: text/plain, Size: 0 bytes --]