linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH] ASoC: fsl_sai: Enable data lines based on input channels
@ 2019-08-11 19:55 Daniel Baluta
  2019-08-14  1:39 ` Nicolin Chen
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Baluta @ 2019-08-11 19:55 UTC (permalink / raw)
  To: nicoleotsuka, broonie
  Cc: linux-kernel, linux-imx, tiwai, alsa-devel, festevam, Daniel Baluta

An audio data frame consists of a number of slots one for each
channel. In the case of I2S there are 2 data slots / frame.

The maximum number of SAI slots / frame is configurable at
IP integration time. This affects the width of Mask Register.
SAI supports up to 32 slots per frame.

The number of datalines is also configurable (up to 8 datalines)
and affects TCE/RCE and the number of data/FIFO registers.

The number of needed data lines (pins) is computed as follows:

* pins = channels / slots.

This can be computed in hw_params function so lets move TRCE bits
seting from startup to hw_params.

Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 sound/soc/fsl/fsl_sai.c | 34 +++++++++++++---------------------
 sound/soc/fsl/fsl_sai.h |  2 +-
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 69cf3678c859..b70032c82fe2 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -414,6 +414,7 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
 	u32 val_cr4 = 0, val_cr5 = 0;
 	u32 slots = (channels == 1) ? 2 : channels;
 	u32 slot_width = word_width;
+	u32 pins;
 	int ret;
 
 	if (sai->slots)
@@ -422,6 +423,8 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
 	if (sai->slot_width)
 		slot_width = sai->slot_width;
 
+	pins = DIV_ROUND_UP(channels, slots);
+
 	if (!sai->is_slave_mode[tx]) {
 		ret = fsl_sai_set_bclk(cpu_dai, tx,
 				slots * slot_width * params_rate(params));
@@ -480,13 +483,17 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
 		}
 	}
 
+	regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx, ofs),
+			   FSL_SAI_CR3_TRCE_MASK,
+			   FSL_SAI_CR3_TRCE((1 << pins) - 1));
+
 	regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx, ofs),
 			   FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
 			   val_cr4);
 	regmap_update_bits(sai->regmap, FSL_SAI_xCR5(tx, ofs),
 			   FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
 			   FSL_SAI_CR5_FBT_MASK, val_cr5);
-	regmap_write(sai->regmap, FSL_SAI_xMR(tx), ~0UL - ((1 << channels) - 1));
+	regmap_write(sai->regmap, FSL_SAI_xMR(tx), ~0UL - ((1 << slots) - 1));
 
 	return 0;
 }
@@ -496,6 +503,10 @@ static int fsl_sai_hw_free(struct snd_pcm_substream *substream,
 {
 	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
 	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
+	unsigned int ofs = sai->soc_data->reg_offset;
+
+	regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx, ofs),
+			   FSL_SAI_CR3_TRCE_MASK, 0);
 
 	if (!sai->is_slave_mode[tx] &&
 			sai->mclk_streams & BIT(substream->stream)) {
@@ -603,32 +614,13 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
 static int fsl_sai_startup(struct snd_pcm_substream *substream,
 		struct snd_soc_dai *cpu_dai)
 {
-	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
-	unsigned int ofs = sai->soc_data->reg_offset;
-	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
 	int ret;
 
-	regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx, ofs),
-			   FSL_SAI_CR3_TRCE_MASK,
-			   FSL_SAI_CR3_TRCE);
-
 	ret = snd_pcm_hw_constraint_list(substream->runtime, 0,
 			SNDRV_PCM_HW_PARAM_RATE, &fsl_sai_rate_constraints);
-
 	return ret;
 }
 
-static void fsl_sai_shutdown(struct snd_pcm_substream *substream,
-		struct snd_soc_dai *cpu_dai)
-{
-	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
-	unsigned int ofs = sai->soc_data->reg_offset;
-	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
-
-	regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx, ofs),
-			   FSL_SAI_CR3_TRCE_MASK, 0);
-}
-
 static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = {
 	.set_sysclk	= fsl_sai_set_dai_sysclk,
 	.set_fmt	= fsl_sai_set_dai_fmt,
@@ -637,7 +629,6 @@ static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = {
 	.hw_free	= fsl_sai_hw_free,
 	.trigger	= fsl_sai_trigger,
 	.startup	= fsl_sai_startup,
-	.shutdown	= fsl_sai_shutdown,
 };
 
 static int fsl_sai_dai_probe(struct snd_soc_dai *cpu_dai)
@@ -881,6 +872,7 @@ static int fsl_sai_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	sai->pdev = pdev;
+
 	sai->soc_data = of_device_get_match_data(&pdev->dev);
 
 	sai->is_lsb_first = of_property_read_bool(np, "lsb-first");
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index c2c43a7d9ba1..3c5383139dc2 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -109,7 +109,7 @@
 #define FSL_SAI_CR2_DIV_MASK	0xff
 
 /* SAI Transmit and Receive Configuration 3 Register */
-#define FSL_SAI_CR3_TRCE	BIT(16)
+#define FSL_SAI_CR3_TRCE(x)	((x) << 16)
 #define FSL_SAI_CR3_TRCE_MASK	GENMASK(23, 16)
 #define FSL_SAI_CR3_WDFL(x)	(x)
 #define FSL_SAI_CR3_WDFL_MASK	0x1f
-- 
2.17.1


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

* Re: [RFC PATCH] ASoC: fsl_sai: Enable data lines based on input channels
  2019-08-11 19:55 [RFC PATCH] ASoC: fsl_sai: Enable data lines based on input channels Daniel Baluta
@ 2019-08-14  1:39 ` Nicolin Chen
  2019-08-14 14:02   ` Daniel Baluta
  0 siblings, 1 reply; 3+ messages in thread
From: Nicolin Chen @ 2019-08-14  1:39 UTC (permalink / raw)
  To: Daniel Baluta
  Cc: broonie, linux-kernel, linux-imx, tiwai, alsa-devel, festevam

On Sun, Aug 11, 2019 at 10:55:45PM +0300, Daniel Baluta wrote:
> An audio data frame consists of a number of slots one for each
> channel. In the case of I2S there are 2 data slots / frame.
> 
> The maximum number of SAI slots / frame is configurable at
> IP integration time. This affects the width of Mask Register.
> SAI supports up to 32 slots per frame.
> 
> The number of datalines is also configurable (up to 8 datalines)
> and affects TCE/RCE and the number of data/FIFO registers.
> 
> The number of needed data lines (pins) is computed as follows:
> 
> * pins = channels / slots.
> 
> This can be computed in hw_params function so lets move TRCE bits
> seting from startup to hw_params.
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> ---
>  sound/soc/fsl/fsl_sai.c | 34 +++++++++++++---------------------
>  sound/soc/fsl/fsl_sai.h |  2 +-
>  2 files changed, 14 insertions(+), 22 deletions(-)
> 
> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> index 69cf3678c859..b70032c82fe2 100644
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c

> @@ -480,13 +483,17 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,

> -	regmap_write(sai->regmap, FSL_SAI_xMR(tx), ~0UL - ((1 << channels) - 1));
> +	regmap_write(sai->regmap, FSL_SAI_xMR(tx), ~0UL - ((1 << slots) - 1));

Would this break mono channel audio?

>  static int fsl_sai_dai_probe(struct snd_soc_dai *cpu_dai)

> @@ -881,6 +872,7 @@ static int fsl_sai_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	sai->pdev = pdev;
> +

Seemly unnecessary

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

* Re: [RFC PATCH] ASoC: fsl_sai: Enable data lines based on input channels
  2019-08-14  1:39 ` Nicolin Chen
@ 2019-08-14 14:02   ` Daniel Baluta
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Baluta @ 2019-08-14 14:02 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: Daniel Baluta, Mark Brown, Linux Kernel Mailing List,
	dl-linux-imx, Takashi Iwai, Linux-ALSA, Fabio Estevam

On Wed, Aug 14, 2019 at 4:39 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> On Sun, Aug 11, 2019 at 10:55:45PM +0300, Daniel Baluta wrote:
> > An audio data frame consists of a number of slots one for each
> > channel. In the case of I2S there are 2 data slots / frame.
> >
> > The maximum number of SAI slots / frame is configurable at
> > IP integration time. This affects the width of Mask Register.
> > SAI supports up to 32 slots per frame.
> >
> > The number of datalines is also configurable (up to 8 datalines)
> > and affects TCE/RCE and the number of data/FIFO registers.
> >
> > The number of needed data lines (pins) is computed as follows:
> >
> > * pins = channels / slots.
> >
> > This can be computed in hw_params function so lets move TRCE bits
> > seting from startup to hw_params.
> >
> > Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> > ---
> >  sound/soc/fsl/fsl_sai.c | 34 +++++++++++++---------------------
> >  sound/soc/fsl/fsl_sai.h |  2 +-
> >  2 files changed, 14 insertions(+), 22 deletions(-)
> >
> > diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> > index 69cf3678c859..b70032c82fe2 100644
> > --- a/sound/soc/fsl/fsl_sai.c
> > +++ b/sound/soc/fsl/fsl_sai.c
>
> > @@ -480,13 +483,17 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
>
> > -     regmap_write(sai->regmap, FSL_SAI_xMR(tx), ~0UL - ((1 << channels) - 1));
> > +     regmap_write(sai->regmap, FSL_SAI_xMR(tx), ~0UL - ((1 << slots) - 1));
>
> Would this break mono channel audio?

Indeed, this isn't good for mono. I see in our internal tree that we
have min(channels, slots).
This would fix mono, let me think if this is really the right solution.

>
> >  static int fsl_sai_dai_probe(struct snd_soc_dai *cpu_dai)
>
> > @@ -881,6 +872,7 @@ static int fsl_sai_probe(struct platform_device *pdev)
> >               return -ENOMEM;
> >
> >       sai->pdev = pdev;
> > +
>
> Seemly unnecessary

Oh, ok. Will fix in next version.

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

end of thread, other threads:[~2019-08-14 14:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-11 19:55 [RFC PATCH] ASoC: fsl_sai: Enable data lines based on input channels Daniel Baluta
2019-08-14  1:39 ` Nicolin Chen
2019-08-14 14:02   ` Daniel Baluta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).