[4/4] ASoC: tegra: i2s: Add support for more than 2 channels
diff mbox series

Message ID 20180727125931.9794-5-jorge.sanjuan@codethink.co.uk
State New
Headers show
Series
  • ASoC: Tegra30 TDM support
Related show

Commit Message

Jorge Sanjuan July 27, 2018, 12:59 p.m. UTC
From: Edward Cragg <edward.cragg@codethink.co.uk>

The CIF configuration and clock setting is currently hard coded for 2
channels. Since the hardware is capable of supporting 1-8 channels add
support for reading the channel count from the supplied parameters to
allow for better TDM support. It seems the original implementation of this
driver was fixed at 2 channels for simplicity, and not implementing TDM.

Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
Signed-off-by: Jorge Sanjuan <jorge.sanjuan@codethink.co.uk>
---
 sound/soc/tegra/tegra30_i2s.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

Comments

Jon Hunter July 30, 2018, 9:46 a.m. UTC | #1
On 27/07/18 13:59, Jorge Sanjuan wrote:
> From: Edward Cragg <edward.cragg@codethink.co.uk>
> 
> The CIF configuration and clock setting is currently hard coded for 2
> channels. Since the hardware is capable of supporting 1-8 channels add
> support for reading the channel count from the supplied parameters to
> allow for better TDM support. It seems the original implementation of this
> driver was fixed at 2 channels for simplicity, and not implementing TDM.
> 
> Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
> Signed-off-by: Jorge Sanjuan <jorge.sanjuan@codethink.co.uk>
> ---
>  sound/soc/tegra/tegra30_i2s.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
> index e26c19ef7439..0f240d7989d0 100644
> --- a/sound/soc/tegra/tegra30_i2s.c
> +++ b/sound/soc/tegra/tegra30_i2s.c
> @@ -138,16 +138,17 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
>  	struct device *dev = dai->dev;
>  	struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>  	unsigned int mask, val, reg;
> -	int ret, sample_size, srate, i2sclock, bitcnt;
> +	int ret, sample_size, srate, i2sclock, bitcnt, audio_bits, channels;
>  	struct tegra30_ahub_cif_conf cif_conf;
>  
> -	if (params_channels(params) != 2)
> +	if (params_channels(params) > 8)
>  		return -EINVAL;

For normal I2S mode, channels should always be 2 and so it could be worth checking
if we are using TDM mode here or not.

>  
>  	mask = TEGRA30_I2S_CTRL_BIT_SIZE_MASK;
>  	switch (params_format(params)) {
>  	case SNDRV_PCM_FORMAT_S16_LE:
>  		val = TEGRA30_I2S_CTRL_BIT_SIZE_16;
> +		audio_bits = TEGRA30_AUDIOCIF_BITS_16;
>  		sample_size = 16;
>  		break;
>  	case SNDRV_PCM_FORMAT_S24_LE:
> @@ -157,6 +158,7 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
>  		break;
>  	case SNDRV_PCM_FORMAT_S32_LE:
>  		val = TEGRA30_I2S_CTRL_BIT_SIZE_32;
> +		audio_bits = TEGRA30_AUDIOCIF_BITS_32;
>  		sample_size = 32;
>  		break;
>  	default:
> @@ -166,9 +168,10 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
>  	regmap_update_bits(i2s->regmap, TEGRA30_I2S_CTRL, mask, val);
>  
>  	srate = params_rate(params);
> +	channels = params_channels(params);
>  
>  	/* Final "* 2" required by Tegra hardware */
> -	i2sclock = srate * params_channels(params) * sample_size * 2;
> +	i2sclock = srate * channels * sample_size * 2;
>  
>  	bitcnt = (i2sclock / (2 * srate)) - 1;
>  	if (bitcnt < 0 || bitcnt > TEGRA30_I2S_TIMING_CHANNEL_BIT_COUNT_MASK_US)
> @@ -188,10 +191,10 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
>  	regmap_write(i2s->regmap, TEGRA30_I2S_TIMING, val);
>  
>  	cif_conf.threshold = 0;
> -	cif_conf.audio_channels = 2;
> -	cif_conf.client_channels = 2;
> -	cif_conf.audio_bits = TEGRA30_AUDIOCIF_BITS_16;
> -	cif_conf.client_bits = TEGRA30_AUDIOCIF_BITS_16;
> +	cif_conf.audio_channels = channels;
> +	cif_conf.client_channels = channels;
> +	cif_conf.audio_bits = audio_bits;
> +	cif_conf.client_bits = audio_bits;
>  	cif_conf.expand = 0;
>  	cif_conf.stereo_conv = 0;
>  	cif_conf.replicate = 0;
> @@ -329,7 +332,7 @@ static const struct snd_soc_dai_driver tegra30_i2s_dai_template = {
>  	.playback = {
>  		.stream_name = "Playback",
>  		.channels_min = 2,
> -		.channels_max = 2,
> +		.channels_max = 8,
>  		.rates = SNDRV_PCM_RATE_8000_96000,
>  		.formats = SNDRV_PCM_FMTBIT_S32_LE |
>  			   SNDRV_PCM_FMTBIT_S24_LE |
> @@ -338,7 +341,7 @@ static const struct snd_soc_dai_driver tegra30_i2s_dai_template = {
>  	.capture = {
>  		.stream_name = "Capture",
>  		.channels_min = 2,
> -		.channels_max = 2,
> +		.channels_max = 8,
>  		.rates = SNDRV_PCM_RATE_8000_96000,
>  		.formats = SNDRV_PCM_FMTBIT_S32_LE |
>  			   SNDRV_PCM_FMTBIT_S24_LE |
> 

Otherwise, assuming that you fix patch 3/4 and rebase this one, looks good to me.

Cheers
Jon
Mark Brown July 30, 2018, 10:21 a.m. UTC | #2
On Mon, Jul 30, 2018 at 10:46:14AM +0100, Jon Hunter wrote:
> On 27/07/18 13:59, Jorge Sanjuan wrote:

> > -	if (params_channels(params) != 2)
> > +	if (params_channels(params) > 8)
> >  		return -EINVAL;

> For normal I2S mode, channels should always be 2 and so it could be worth checking
> if we are using TDM mode here or not.

Yes, there's some question if a multi-channel I2S setup is going to be
all the left channels then all the right channels, have multiple data
lines in parallel (this especially common for high end applications) or
something else.  Usually it's safer to use a DSP mode for those.

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns.  Doing this makes your messages much
easier to read and reply to.

Patch
diff mbox series

diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
index e26c19ef7439..0f240d7989d0 100644
--- a/sound/soc/tegra/tegra30_i2s.c
+++ b/sound/soc/tegra/tegra30_i2s.c
@@ -138,16 +138,17 @@  static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
 	struct device *dev = dai->dev;
 	struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
 	unsigned int mask, val, reg;
-	int ret, sample_size, srate, i2sclock, bitcnt;
+	int ret, sample_size, srate, i2sclock, bitcnt, audio_bits, channels;
 	struct tegra30_ahub_cif_conf cif_conf;
 
-	if (params_channels(params) != 2)
+	if (params_channels(params) > 8)
 		return -EINVAL;
 
 	mask = TEGRA30_I2S_CTRL_BIT_SIZE_MASK;
 	switch (params_format(params)) {
 	case SNDRV_PCM_FORMAT_S16_LE:
 		val = TEGRA30_I2S_CTRL_BIT_SIZE_16;
+		audio_bits = TEGRA30_AUDIOCIF_BITS_16;
 		sample_size = 16;
 		break;
 	case SNDRV_PCM_FORMAT_S24_LE:
@@ -157,6 +158,7 @@  static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
 		break;
 	case SNDRV_PCM_FORMAT_S32_LE:
 		val = TEGRA30_I2S_CTRL_BIT_SIZE_32;
+		audio_bits = TEGRA30_AUDIOCIF_BITS_32;
 		sample_size = 32;
 		break;
 	default:
@@ -166,9 +168,10 @@  static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
 	regmap_update_bits(i2s->regmap, TEGRA30_I2S_CTRL, mask, val);
 
 	srate = params_rate(params);
+	channels = params_channels(params);
 
 	/* Final "* 2" required by Tegra hardware */
-	i2sclock = srate * params_channels(params) * sample_size * 2;
+	i2sclock = srate * channels * sample_size * 2;
 
 	bitcnt = (i2sclock / (2 * srate)) - 1;
 	if (bitcnt < 0 || bitcnt > TEGRA30_I2S_TIMING_CHANNEL_BIT_COUNT_MASK_US)
@@ -188,10 +191,10 @@  static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
 	regmap_write(i2s->regmap, TEGRA30_I2S_TIMING, val);
 
 	cif_conf.threshold = 0;
-	cif_conf.audio_channels = 2;
-	cif_conf.client_channels = 2;
-	cif_conf.audio_bits = TEGRA30_AUDIOCIF_BITS_16;
-	cif_conf.client_bits = TEGRA30_AUDIOCIF_BITS_16;
+	cif_conf.audio_channels = channels;
+	cif_conf.client_channels = channels;
+	cif_conf.audio_bits = audio_bits;
+	cif_conf.client_bits = audio_bits;
 	cif_conf.expand = 0;
 	cif_conf.stereo_conv = 0;
 	cif_conf.replicate = 0;
@@ -329,7 +332,7 @@  static const struct snd_soc_dai_driver tegra30_i2s_dai_template = {
 	.playback = {
 		.stream_name = "Playback",
 		.channels_min = 2,
-		.channels_max = 2,
+		.channels_max = 8,
 		.rates = SNDRV_PCM_RATE_8000_96000,
 		.formats = SNDRV_PCM_FMTBIT_S32_LE |
 			   SNDRV_PCM_FMTBIT_S24_LE |
@@ -338,7 +341,7 @@  static const struct snd_soc_dai_driver tegra30_i2s_dai_template = {
 	.capture = {
 		.stream_name = "Capture",
 		.channels_min = 2,
-		.channels_max = 2,
+		.channels_max = 8,
 		.rates = SNDRV_PCM_RATE_8000_96000,
 		.formats = SNDRV_PCM_FMTBIT_S32_LE |
 			   SNDRV_PCM_FMTBIT_S24_LE |