[alsa-devel] [PATCH] davinci-mcasp: Add support for multichannel playback
Daniel Mack
zonque at gmail.com
Tue Feb 26 23:49:37 CET 2013
Hi Michal,
(+ Mark and Liam - the ASoC maintainers)
many thanks for the patch. I couldn't try it yet on real hardware, but
will do so soon. Meanwhile, below are some comments on locations I
stumbled over ...
Best regards,
Daniel
On 26.02.2013 17:20, 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.
>
> Signed-off-by: Michal Bachraty <michal.bachraty at streamunlimited.com>
> ---
> sound/soc/davinci/davinci-mcasp.c | 49 +++++++++++++++++++++++++++++++------
> sound/soc/davinci/davinci-pcm.c | 15 ++++++------
> sound/soc/davinci/davinci-pcm.h | 1 +
> 3 files changed, 51 insertions(+), 14 deletions(-)
>
> diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
> index d2ca682..ebd6ee0 100644
> --- a/sound/soc/davinci/davinci-mcasp.c
> +++ b/sound/soc/davinci/davinci-mcasp.c
> @@ -657,12 +657,14 @@ 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)
Stray change of of the return value here? At least, you don't return
anything, which gives a build warning. But you don't care for the return
value either, so this is probably just unintentional.
> {
> int i;
> u8 tx_ser = 0;
> u8 rx_ser = 0;
> -
> + u8 ser;
> + u8 stream_serializers = channels/2 + channels%2;
I think you can write that as '(channels + 1) / 2', which is more
obvious. Nit: I'd rather call the variable num_serializers or something
like that ...
> /* Default configuration */
> mcasp_set_bits(dev->base + DAVINCI_MCASP_PWREMUMGT_REG, MCASP_SOFT);
>
> @@ -680,16 +682,44 @@ 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;
> + }
No curly brackets needed here.
> +
> + if (ser < stream_serializers) {
> + printk(KERN_WARNING "davinci-mcasp: stream has more channels "
> + "(%d) than are enabled in mcasp (%d)",
> + channels, ser * 2);
Use dev_warn(), and check the indentation.
> + 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 < stream_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 < stream_serializers) {
> mcasp_clr_bits(dev->base + DAVINCI_MCASP_PDIR_REG,
> AXR(i));
> rx_ser++;
> + } else {
> + mcasp_clr_bits(dev->base + DAVINCI_MCASP_XRSRCTL_REG(i),
> + 3);
Can the magic '3' go into a macro definition?
> }
> }
>
> @@ -812,8 +842,12 @@ static int davinci_mcasp_hw_params(struct snd_pcm_substream *substream,
> &dev->dma_params[substream->stream];
> int word_length;
> u8 fifo_level;
> + 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);
> + davinci_hw_common_param(dev, substream->stream, channels);
> if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> fifo_level = dev->txnumevt;
> else
> @@ -862,6 +896,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/2 + channels%2;
See above.
> davinci_config_channel_size(dev, word_length);
>
> return 0;
> @@ -936,13 +971,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..9951973 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;
> @@ -193,7 +194,7 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
> period_size);
>
> data_type = prtd->params->data_type;
> - count = period_size / data_type;
> + count = period_size / (data_type * serializers);
> if (fifo_level)
> count /= fifo_level;
>
> @@ -201,8 +202,8 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
> 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,9 @@ 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, 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);
>
More information about the Alsa-devel
mailing list