Skip to contentSkip to conversation

tegra30 tdm audio support

alsa-devel Superseded Ben Dooks 3 participants 20 messages Sep 30, 2019 – Oct 7, 2019

Subsystems (3)
Sound ASoC Tegra architecture support

v3 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Ben Dooks

author
#0 lore raw

This adds support for tegra30 tdm audio and fixes a bug in the fifo handling on rx when non-32 bit aligned data is used.

v2:
- moved edge-control and data-offset to the set_fmt callbacks
- fixed dev_dbg in set_tdm callback
- fixed dev_dbg message contents in set_tdm callback
- changed fsync width to be permanently 1 clock

v3:
- cleanup fsync patch
- fix rebase issue with tdm patch

author
#1 lore raw

PATCH v3 1/7 ASoC: tegra: add a TDM configuration callback

From: Edward Cragg <[email protected]>

Add a callback to configure TDM settings for the Tegra30 I2S ASoC 'platform' driver.

Signed-off-by Edward Cragg <[email protected]>
[[email protected]: merge fix for power management]
[[email protected]: add review change for fsync of 1 clock]
Signed-off-by Ben Dooks <[email protected]>
---
 sound/soc/tegra/tegra30_i2s.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
sound/soc/tegra/tegra30_i2s.c +33 -0
index ac6983c6bd72..7f9ef6abeae2 100644
@@ -254,6 +254,38 @@ static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
 	return 0;
 }
 
+/*
+ * Set up TDM
+ */
+static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
+			       unsigned int tx_mask, unsigned int rx_mask,
+			       int slots, int slot_width)
+{
+	struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+	unsigned int mask, val;
+
+	dev_dbg(dai->dev, "%s: txmask=0x%08x rxmask=0x%08x slots=%d width=%d\n",
+		 __func__, tx_mask, rx_mask, slots, slot_width);
+
+	/* Set up slots and tx/rx masks */
+	mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |
+	       TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
+	       TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_MASK;
+
+	val = (tx_mask << TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_SHIFT) |
+	      (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
+	      ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);
+
+	pm_runtime_get_sync(dai->dev);
+	regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
+	// set the fsync width to minimum of 1 clock width
+	regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL,
+			   TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_MASK, 0x0);
+	pm_runtime_put(dai->dev);
+
+	return 0;
+}
+
 static int tegra30_i2s_probe(struct snd_soc_dai *dai)
 {
 	struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);

@@ -268,6 +300,7 @@ static const struct snd_soc_dai_ops tegra30_i2s_dai_ops = {
 	.set_fmt	= tegra30_i2s_set_fmt,
 	.hw_params	= tegra30_i2s_hw_params,
 	.trigger	= tegra30_i2s_trigger,
+	.set_tdm_slot	= tegra30_i2s_set_tdm,
 };
 
 static const struct snd_soc_dai_driver tegra30_i2s_dai_template = {
Signature · 1 line
2.23.0
maintainer
#2 lore raw

Re: [alsa-devel] [PATCH v3 1/7] ASoC: tegra: add a TDM configuration callback

Quoted 32 lines · Ben Dooks

From: Edward Cragg <[email protected]>

Add a callback to configure TDM settings for the Tegra30 I2S ASoC 'platform' driver.

Signed-off-by Edward Cragg <[email protected]>
[[email protected]: merge fix for power management]
[[email protected]: add review change for fsync of 1 clock]
Signed-off-by Ben Dooks <[email protected]>
---
 sound/soc/tegra/tegra30_i2s.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
sound/soc/tegra/tegra30_i2s.c +11 -0
index ac6983c6bd72..7f9ef6abeae2 100644
@@ -254,6 +254,38 @@ static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
 	return 0;
 }
 
+/*
+ * Set up TDM
+ */
+static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
+			       unsigned int tx_mask, unsigned int rx_mask,
+			       int slots, int slot_width)
+{
+	struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+	unsigned int mask, val;
+
+	dev_dbg(dai->dev, "%s: txmask=0x%08x rxmask=0x%08x slots=%d width=%d\n",

sound/soc/tegra/tegra30_i2s.c static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd, · line 277

Quoted 10 lines
sound/soc/tegra/tegra30_i2s.c +10 -0
+		 __func__, tx_mask, rx_mask, slots, slot_width);
+
+	/* Set up slots and tx/rx masks */
+	mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |
+	       TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
+	       TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_MASK;
+
+	val = (tx_mask << TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_SHIFT) |
+	      (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
+	      ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);

I am not sure why this keeps changing. Why not set to 1 always?

Jon

Signature · 1 line
nvpublic
author
#3 lore raw

Re: [alsa-devel] [PATCH v3 1/7] ASoC: tegra: add a TDM configuration callback

Quoted 38 lines · Jon Hunter
Quoted 36 lines · Ben Dooks

From: Edward Cragg <[email protected]>

Add a callback to configure TDM settings for the Tegra30 I2S ASoC 'platform' driver.

Signed-off-by Edward Cragg <[email protected]>
[[email protected]: merge fix for power management]
[[email protected]: add review change for fsync of 1 clock]
Signed-off-by Ben Dooks <[email protected]>
---
  sound/soc/tegra/tegra30_i2s.c | 33 +++++++++++++++++++++++++++++++++
  1 file changed, 33 insertions(+)
sound/soc/tegra/tegra30_i2s.c +15 -0
index ac6983c6bd72..7f9ef6abeae2 100644
@@ -254,6 +254,38 @@ static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
  	return 0;
  }
 
+/*
+ * Set up TDM
+ */
+static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
+			       unsigned int tx_mask, unsigned int rx_mask,
+			       int slots, int slot_width)
+{
+	struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+	unsigned int mask, val;
+
+	dev_dbg(dai->dev, "%s: txmask=0x%08x rxmask=0x%08x slots=%d width=%d\n",
+		 __func__, tx_mask, rx_mask, slots, slot_width);
+
+	/* Set up slots and tx/rx masks */
+	mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |
Quoted 8 lines

sound/soc/tegra/tegra30_i2s.c static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd, · line 277

Quoted 6 lines
sound/soc/tegra/tegra30_i2s.c +6 -0
+	       TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
+	       TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_MASK;
+
+	val = (tx_mask << TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_SHIFT) |
+	      (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
+	      ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);

I am not sure why this keeps changing. Why not set to 1 always?

This is the total number of slots, not the width of the fsync which has been changed to 1 below this.

Signature · 4 lines
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html
maintainer
#4 lore raw

Re: [alsa-devel] [PATCH v3 1/7] ASoC: tegra: add a TDM configuration callback

Quoted 51 lines · Ben Dooks
Quoted 41 lines · Jon Hunter
Quoted 39 lines · Ben Dooks

From: Edward Cragg <[email protected]>

Add a callback to configure TDM settings for the Tegra30 I2S ASoC 'platform'
driver.

Signed-off-by Edward Cragg <[email protected]>
[[email protected]: merge fix for power management]
[[email protected]: add review change for fsync of 1 clock]
Signed-off-by Ben Dooks <[email protected]>

---
  sound/soc/tegra/tegra30_i2s.c | 33 +++++++++++++++++++++++++++++++++

  1 file changed, 33 insertions(+)
sound/soc/tegra/tegra30_i2s.c +0 -0
index ac6983c6bd72..7f9ef6abeae2 100644
@@ -254,6 +254,38 @@ static int tegra30_i2s_trigger(struct
 snd_pcm_substream *substream, int cmd,
       return 0;
   }

  +/*
+ * Set up TDM
+ */

sound/soc/tegra/tegra30_i2s.c +8 -0
+static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
+                   unsigned int tx_mask, unsigned int rx_mask,
+                   int slots, int slot_width)
+{
+    struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+    unsigned int mask, val;
+
+    dev_dbg(dai->dev, "%s: txmask=0x%08x rxmask=0x%08x slots=%d

width=%d\n",
+         __func__, tx_mask, rx_mask, slots, slot_width); +
+    /* Set up slots and tx/rx masks */
+    mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |

Quoted 8 lines
Quoted 6 lines

+           TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
+           TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_MASK;
+
+    val = (tx_mask << TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_SHIFT) | +          (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) | +          ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);

I am not sure why this keeps changing. Why not set to 1 always?

Quoted 2 lines

This is the total number of slots, not the width of the fsync which has been changed to 1 below this.

Ah yes indeed. Sorry, should have waited until this morning to look at this! Jon

Signature · 5 lines
nvpublic
_______________________________________________
Alsa-devel mailing list
[email protected]
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
maintainer
#5 lore raw

Re: [alsa-devel] [PATCH v3 1/7] ASoC: tegra: add a TDM configuration callback

In reply to Ben Dooks
Quoted 36 lines · Ben Dooks

From: Edward Cragg <[email protected]>

Add a callback to configure TDM settings for the Tegra30 I2S ASoC 'platform' driver.

Signed-off-by Edward Cragg <[email protected]>
[[email protected]: merge fix for power management]
[[email protected]: add review change for fsync of 1 clock]
Signed-off-by Ben Dooks <[email protected]>
---
 sound/soc/tegra/tegra30_i2s.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
sound/soc/tegra/tegra30_i2s.c +15 -0
index ac6983c6bd72..7f9ef6abeae2 100644
@@ -254,6 +254,38 @@ static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
 	return 0;
 }
 
+/*
+ * Set up TDM
+ */
+static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
+			       unsigned int tx_mask, unsigned int rx_mask,
+			       int slots, int slot_width)
+{
+	struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+	unsigned int mask, val;
+
+	dev_dbg(dai->dev, "%s: txmask=0x%08x rxmask=0x%08x slots=%d width=%d\n",
+		 __func__, tx_mask, rx_mask, slots, slot_width);
+
+	/* Set up slots and tx/rx masks */
+	mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |

sound/soc/tegra/tegra30_i2s.c static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd, · line 281

Quoted 10 lines
sound/soc/tegra/tegra30_i2s.c +10 -0
+	       TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
+	       TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_MASK;
+
+	val = (tx_mask << TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_SHIFT) |
+	      (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
+	      ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);
+
+	pm_runtime_get_sync(dai->dev);
+	regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
+	// set the fsync width to minimum of 1 clock width

Please make sure you are consistent with your commenting style and you adhere to the kernel coding style.

Also, I see a lot of ...

ERROR: trailing whitespace
#197: FILE: sound/soc/tegra/tegra30_i2s.c:258:
+ * Set up TDM^M$

ERROR: DOS line endings
#198: FILE: sound/soc/tegra/tegra30_i2s.c:259:
+ */^M$

Jon

Signature · 1 line
nvpublic
author
#6 lore raw

Re: [alsa-devel] [PATCH v3 1/7] ASoC: tegra: add a TDM configuration callback

Quoted 54 lines · Jon Hunter
Quoted 36 lines · Ben Dooks

From: Edward Cragg <[email protected]>

Add a callback to configure TDM settings for the Tegra30 I2S ASoC 'platform' driver.

Signed-off-by Edward Cragg <[email protected]>
[[email protected]: merge fix for power management]
[[email protected]: add review change for fsync of 1 clock]
Signed-off-by Ben Dooks <[email protected]>
---
  sound/soc/tegra/tegra30_i2s.c | 33 +++++++++++++++++++++++++++++++++
  1 file changed, 33 insertions(+)
sound/soc/tegra/tegra30_i2s.c +15 -0
index ac6983c6bd72..7f9ef6abeae2 100644
@@ -254,6 +254,38 @@ static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
  	return 0;
  }
 
+/*
+ * Set up TDM
+ */
+static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
+			       unsigned int tx_mask, unsigned int rx_mask,
+			       int slots, int slot_width)
+{
+	struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+	unsigned int mask, val;
+
+	dev_dbg(dai->dev, "%s: txmask=0x%08x rxmask=0x%08x slots=%d width=%d\n",
+		 __func__, tx_mask, rx_mask, slots, slot_width);
+
+	/* Set up slots and tx/rx masks */
+	mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |

sound/soc/tegra/tegra30_i2s.c static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd, · line 281

Quoted 10 lines
sound/soc/tegra/tegra30_i2s.c +10 -0
+	       TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
+	       TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_MASK;
+
+	val = (tx_mask << TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_SHIFT) |
+	      (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
+	      ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);
+
+	pm_runtime_get_sync(dai->dev);
+	regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
+	// set the fsync width to minimum of 1 clock width

Please make sure you are consistent with your commenting style and you adhere to the kernel coding style.

Also, I see a lot of ...

Quoted 7 lines

ERROR: trailing whitespace
#197: FILE: sound/soc/tegra/tegra30_i2s.c:258:
+ * Set up TDM^M$

ERROR: DOS line endings
#198: FILE: sound/soc/tegra/tegra30_i2s.c:259:
+ */^M$

I'll go and check that later.
I did assume my colleagues had done the relevant checks themselves...

Signature · 4 lines
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html
author
#7 lore raw

Re: [alsa-devel] [PATCH v3 1/7] ASoC: tegra: add a TDM configuration callback

In reply to Jon Hunter
Quoted 54 lines · Jon Hunter
Quoted 36 lines · Ben Dooks

From: Edward Cragg <[email protected]>

Add a callback to configure TDM settings for the Tegra30 I2S ASoC 'platform' driver.

Signed-off-by Edward Cragg <[email protected]>
[[email protected]: merge fix for power management]
[[email protected]: add review change for fsync of 1 clock]
Signed-off-by Ben Dooks <[email protected]>
---
  sound/soc/tegra/tegra30_i2s.c | 33 +++++++++++++++++++++++++++++++++
  1 file changed, 33 insertions(+)
sound/soc/tegra/tegra30_i2s.c +15 -0
index ac6983c6bd72..7f9ef6abeae2 100644
@@ -254,6 +254,38 @@ static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
  	return 0;
  }
 
+/*
+ * Set up TDM
+ */
+static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
+			       unsigned int tx_mask, unsigned int rx_mask,
+			       int slots, int slot_width)
+{
+	struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+	unsigned int mask, val;
+
+	dev_dbg(dai->dev, "%s: txmask=0x%08x rxmask=0x%08x slots=%d width=%d\n",
+		 __func__, tx_mask, rx_mask, slots, slot_width);
+
+	/* Set up slots and tx/rx masks */
+	mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |

sound/soc/tegra/tegra30_i2s.c static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd, · line 281

Quoted 10 lines
sound/soc/tegra/tegra30_i2s.c +10 -0
+	       TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
+	       TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_MASK;
+
+	val = (tx_mask << TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_SHIFT) |
+	      (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
+	      ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);
+
+	pm_runtime_get_sync(dai->dev);
+	regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
+	// set the fsync width to minimum of 1 clock width

Please make sure you are consistent with your commenting style and you adhere to the kernel coding style.

Also, I see a lot of ...

Quoted 7 lines

ERROR: trailing whitespace
#197: FILE: sound/soc/tegra/tegra30_i2s.c:258:
+ * Set up TDM^M$

ERROR: DOS line endings
#198: FILE: sound/soc/tegra/tegra30_i2s.c:259:
+ */^M$

ok, for me I am getting:

Quoted 7 lines
$ ./scripts/checkpatch.pl ./patches/0001-ASoC-tegra-add-a-TDM-configuration-callback.patch WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#6: Add a callback to configure TDM settings for the Tegra30 I2S ASoC 'platform'

total: 0 errors, 1 warnings, 45 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

I don't see any warnings about the line-endings

I will remove the comment about the function, it is fairly self explanatory.

Signature · 4 lines
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html
author
#8 lore raw

PATCH v3 2/7 ASoC: tegra: Allow 24bit and 32bit samples

In reply to Ben Dooks

From: Edward Cragg <[email protected]>

The tegra3 audio can support 24 and 32 bit sample sizes so add the option to the tegra30_i2s_hw_params to configure the S24_LE or S32_LE formats when requested.

Signed-off-by Edward Cragg <[email protected]>
[[email protected]: fixup merge of 24 and 32bit]
[[email protected]: add pm calls around ytdm config]
[[email protected]: drop debug printing to dev_dbg]
Signed-off-by Ben Dooks <[email protected]>

---
squash 5aeca5a055fd ASoC: tegra: i2s: pm_runtime_get_sync() is needed in tdm code

ASoC: tegra: i2s: pm_runtime_get_sync() is needed in tdm code

---
 sound/soc/tegra/tegra30_i2s.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)
sound/soc/tegra/tegra30_i2s.c +20 -5
index 7f9ef6abeae2..b948b010c057 100644
@@ -127,7 +127,7 @@ 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;
 	struct tegra30_ahub_cif_conf cif_conf;
 
 	if (params_channels(params) != 2)

@@ -137,8 +137,19 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
 	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:
+		val = TEGRA30_I2S_CTRL_BIT_SIZE_24;
+		audio_bits = TEGRA30_AUDIOCIF_BITS_24;
+		sample_size = 24;
+		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:
 		return -EINVAL;
 	}

@@ -170,8 +181,8 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
 	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_bits = audio_bits;
+	cif_conf.client_bits = audio_bits;
 	cif_conf.expand = 0;
 	cif_conf.stereo_conv = 0;
 	cif_conf.replicate = 0;

@@ -310,14 +321,18 @@ static const struct snd_soc_dai_driver tegra30_i2s_dai_template = {
 		.channels_min = 2,
 		.channels_max = 2,
 		.rates = SNDRV_PCM_RATE_8000_96000,
-		.formats = SNDRV_PCM_FMTBIT_S16_LE,
+		.formats = SNDRV_PCM_FMTBIT_S32_LE |
+			   SNDRV_PCM_FMTBIT_S24_LE |
+			   SNDRV_PCM_FMTBIT_S16_LE,
 	},
 	.capture = {
 		.stream_name = "Capture",
 		.channels_min = 2,
 		.channels_max = 2,
 		.rates = SNDRV_PCM_RATE_8000_96000,
-		.formats = SNDRV_PCM_FMTBIT_S16_LE,
+		.formats = SNDRV_PCM_FMTBIT_S32_LE |
+			   SNDRV_PCM_FMTBIT_S24_LE |
+			   SNDRV_PCM_FMTBIT_S16_LE,
 	},
 	.ops = &tegra30_i2s_dai_ops,
 	.symmetric_rates = 1,
Signature · 1 line
2.23.0
author
#9 lore raw

PATCH v3 3/7 ASoC: tegra: i2s: Add support for more than 2 channels

In reply to Ben Dooks

From: Edward Cragg <[email protected]>

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 <[email protected]>
[[email protected]: added is_tdm and channel nr check]
[[email protected]: merge edge control into set-format]
[[email protected]: removed is_tdm and moved edge to hw_params]
Signed-off-by Ben Dooks <[email protected]>
---
 sound/soc/tegra/tegra30_i2s.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)
sound/soc/tegra/tegra30_i2s.c +13 -8
index b948b010c057..0fee59171d3b 100644
@@ -67,6 +67,7 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
 {
 	struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
 	unsigned int mask = 0, val = 0;
+	unsigned int ch_mask, ch_val = 0;
 
 	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
 	case SND_SOC_DAIFMT_NB_NF:

@@ -75,6 +76,7 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
 		return -EINVAL;
 	}
 
+	ch_mask = TEGRA30_I2S_CH_CTRL_EGDE_CTRL_MASK;
 	mask |= TEGRA30_I2S_CTRL_MASTER_ENABLE;
 	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
 	case SND_SOC_DAIFMT_CBS_CFS:

@@ -90,10 +92,12 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
 		TEGRA30_I2S_CTRL_LRCK_MASK;
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_DSP_A:
+		ch_val = TEGRA30_I2S_CH_CTRL_EGDE_CTRL_NEG_EDGE;
 		val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
 		val |= TEGRA30_I2S_CTRL_LRCK_L_LOW;
 		break;
 	case SND_SOC_DAIFMT_DSP_B:
+		ch_val = TEGRA30_I2S_CH_CTRL_EGDE_CTRL_NEG_EDGE;
 		val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
 		val |= TEGRA30_I2S_CTRL_LRCK_R_LOW;
 		break;

@@ -115,6 +119,7 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
 
 	pm_runtime_get_sync(dai->dev);
 	regmap_update_bits(i2s->regmap, TEGRA30_I2S_CTRL, mask, val);
+	regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL, ch_mask, ch_val);
 	pm_runtime_put(dai->dev);
 
 	return 0;

@@ -127,10 +132,11 @@ 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, audio_bits;
+	int ret, sample_size, srate, i2sclock, bitcnt, audio_bits, channels;
 	struct tegra30_ahub_cif_conf cif_conf;
 
-	if (params_channels(params) != 2)
+	channels = params_channels(params);
+	if (channels > 8)
 		return -EINVAL;
 
 	mask = TEGRA30_I2S_CTRL_BIT_SIZE_MASK;

@@ -157,9 +163,8 @@ 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);
-
 	/* 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)

@@ -179,8 +184,8 @@ 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_channels = channels;
+	cif_conf.client_channels = channels;
 	cif_conf.audio_bits = audio_bits;
 	cif_conf.client_bits = audio_bits;
 	cif_conf.expand = 0;

@@ -319,7 +324,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 |

@@ -328,7 +333,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 |
Signature · 1 line
2.23.0
author
#10 lore raw

PATCH v3 4/7 ASoC: tegra: disable rx_fifo after disable stream

In reply to Ben Dooks

We see odd FIFO overruns with this, we assume the best thing to do is to disable the RX I2S frontend first, and then disable the FIFO that is using it.

This also fixes an issue where using multi-word frames (TDM) have partial samples stuck in the FIFO which then get read out when the next capture is started.

Signed-off-by Ben Dooks <[email protected]>
---
 sound/soc/tegra/tegra30_i2s.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/tegra/tegra30_i2s.c +1 -1
index 0fee59171d3b..c573151da341 100644
@@ -236,9 +236,9 @@ static void tegra30_i2s_start_capture(struct tegra30_i2s *i2s)
 
 static void tegra30_i2s_stop_capture(struct tegra30_i2s *i2s)
 {
-	tegra30_ahub_disable_rx_fifo(i2s->capture_fifo_cif);
 	regmap_update_bits(i2s->regmap, TEGRA30_I2S_CTRL,
 			   TEGRA30_I2S_CTRL_XFER_EN_RX, 0);
+	tegra30_ahub_disable_rx_fifo(i2s->capture_fifo_cif);
 }
 
 static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
Signature · 1 line
2.23.0
author
#11 lore raw

PATCH v3 5/7 ASoC: tegra: set i2s_offset to 0 for tdm

In reply to Ben Dooks

Set the offset to 0 for TDM mode, as per the current setup. Note we also move the data offset programming to the i2s hw_parameters call as per the suggestion from Jon Hunter.

Signed-off-by Ben Dooks <[email protected]>

---
v2:
- fix the review comments and move the i2s offset setting

---
 sound/soc/tegra/tegra30_i2s.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
sound/soc/tegra/tegra30_i2s.c +7 -6
index c573151da341..a03692b0afc3 100644
@@ -66,7 +66,7 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
 				unsigned int fmt)
 {
 	struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
-	unsigned int mask = 0, val = 0;
+	unsigned int mask = 0, val = 0, data_offset = 1;
 	unsigned int ch_mask, ch_val = 0;
 
 	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {

@@ -95,11 +95,13 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
 		ch_val = TEGRA30_I2S_CH_CTRL_EGDE_CTRL_NEG_EDGE;
 		val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
 		val |= TEGRA30_I2S_CTRL_LRCK_L_LOW;
+		data_offset = 0;
 		break;
 	case SND_SOC_DAIFMT_DSP_B:
 		ch_val = TEGRA30_I2S_CH_CTRL_EGDE_CTRL_NEG_EDGE;
 		val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
 		val |= TEGRA30_I2S_CTRL_LRCK_R_LOW;
+		data_offset = 0;
 		break;
 	case SND_SOC_DAIFMT_I2S:
 		val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_LRCK;

@@ -120,6 +122,10 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
 	pm_runtime_get_sync(dai->dev);
 	regmap_update_bits(i2s->regmap, TEGRA30_I2S_CTRL, mask, val);
 	regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL, ch_mask, ch_val);
+	val = (data_offset << TEGRA30_I2S_OFFSET_RX_DATA_OFFSET_SHIFT) |
+		(data_offset << TEGRA30_I2S_OFFSET_TX_DATA_OFFSET_SHIFT);
+	regmap_write(i2s->regmap, TEGRA30_I2S_OFFSET, val);
+
 	pm_runtime_put(dai->dev);
 
 	return 0;

@@ -203,11 +209,6 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
 	}
 
 	i2s->soc_data->set_audio_cif(i2s->regmap, reg, &cif_conf);
-
-	val = (1 << TEGRA30_I2S_OFFSET_RX_DATA_OFFSET_SHIFT) |
-	      (1 << TEGRA30_I2S_OFFSET_TX_DATA_OFFSET_SHIFT);
-	regmap_write(i2s->regmap, TEGRA30_I2S_OFFSET, val);
-
 	return 0;
 }
Signature · 1 line
2.23.0
maintainer
#12 lore raw

Re: [alsa-devel] [PATCH v3 5/7] ASoC: tegra: set i2s_offset to 0 for tdm

Quoted 26 lines · Ben Dooks

Set the offset to 0 for TDM mode, as per the current setup. Note we also move the data offset programming to the i2s hw_parameters call as per the suggestion from Jon Hunter.

Signed-off-by Ben Dooks <[email protected]>

---
v2:
- fix the review comments and move the i2s offset setting

---
 sound/soc/tegra/tegra30_i2s.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
sound/soc/tegra/tegra30_i2s.c +1 -1
index c573151da341..a03692b0afc3 100644
@@ -66,7 +66,7 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
 				unsigned int fmt)
 {
 	struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
-	unsigned int mask = 0, val = 0;
+	unsigned int mask = 0, val = 0, data_offset = 1;
 	unsigned int ch_mask, ch_val = 0;
 
 	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {

sound/soc/tegra/tegra30_i2s.c static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai, · line 104

Quoted 10 lines
sound/soc/tegra/tegra30_i2s.c +2 -0
@@ -95,11 +95,13 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
 		ch_val = TEGRA30_I2S_CH_CTRL_EGDE_CTRL_NEG_EDGE;
 		val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
 		val |= TEGRA30_I2S_CTRL_LRCK_L_LOW;
+		data_offset = 0;
 		break;
 	case SND_SOC_DAIFMT_DSP_B:
 		ch_val = TEGRA30_I2S_CH_CTRL_EGDE_CTRL_NEG_EDGE;
 		val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
 		val |= TEGRA30_I2S_CTRL_LRCK_R_LOW;
+		data_offset = 0;

My understanding is that the difference between dsp-a and dsp-b is that dsp-a has an offset of 1 and dsp-b has an offset of 0.

Cheers
Jon

Signature · 1 line
nvpublic
author
#13 lore raw

Re: [alsa-devel] [PATCH v3 5/7] ASoC: tegra: set i2s_offset to 0 for tdm

Quoted 27 lines · Jon Hunter
Quoted 24 lines · Ben Dooks

Set the offset to 0 for TDM mode, as per the current setup. Note we also move the data offset programming to the i2s hw_parameters call as per the suggestion from Jon Hunter.

Signed-off-by Ben Dooks <[email protected]>

---
v2:
- fix the review comments and move the i2s offset setting

---
  sound/soc/tegra/tegra30_i2s.c | 13 +++++++------
  1 file changed, 7 insertions(+), 6 deletions(-)
sound/soc/tegra/tegra30_i2s.c +1 -1
index c573151da341..a03692b0afc3 100644
@@ -66,7 +66,7 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
  				unsigned int fmt)
  {
  	struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
-	unsigned int mask = 0, val = 0;
+	unsigned int mask = 0, val = 0, data_offset = 1;
  	unsigned int ch_mask, ch_val = 0;
Quoted 15 lines

sound/soc/tegra/tegra30_i2s.c static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai, · line 104

Quoted 12 lines
sound/soc/tegra/tegra30_i2s.c +2 -0
  	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {

@@ -95,11 +95,13 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
  		ch_val = TEGRA30_I2S_CH_CTRL_EGDE_CTRL_NEG_EDGE;
  		val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
  		val |= TEGRA30_I2S_CTRL_LRCK_L_LOW;
+		data_offset = 0;
  		break;
  	case SND_SOC_DAIFMT_DSP_B:
  		ch_val = TEGRA30_I2S_CH_CTRL_EGDE_CTRL_NEG_EDGE;
  		val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
  		val |= TEGRA30_I2S_CTRL_LRCK_R_LOW;
+		data_offset = 0;

My understanding is that the difference between dsp-a and dsp-b is that dsp-a has an offset of 1 and dsp-b has an offset of 0.

Ok, can anyone else check this before I make the change for DSP_B ?

Signature · 4 lines
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html
maintainer
#14 lore raw

Re: [alsa-devel] [PATCH v3 5/7] ASoC: tegra: set i2s_offset to 0 for tdm

In reply to Jon Hunter
Quoted 2 lines · Jon Hunter

My understanding is that the difference between dsp-a and dsp-b is that dsp-a has an offset of 1 and dsp-b has an offset of 0.

Yes.

author
#15 lore raw

PATCH v3 6/7 ASoC: tegra: config fifos on hw_param changes

In reply to Ben Dooks

If the hw_params uses a different bit or channel count, then we need to change both the I2S unit's CIF configuration as well as the APBIF one.

To allow changing the APBIF, add a call to reconfigure the RX or TX FIFO without changing the DMA or allocation, and get the I2S driver to call it once the hw params have been calculate.

Signed-off-by Ben Dooks <[email protected]>
---
 sound/soc/tegra/tegra30_ahub.c | 114 ++++++++++++++++++---------------
 sound/soc/tegra/tegra30_ahub.h |   5 ++
 sound/soc/tegra/tegra30_i2s.c  |   2 +
 3 files changed, 69 insertions(+), 52 deletions(-)
sound/soc/tegra/tegra30_ahub.c +62 -52
index 952381260dc3..58e05ceb86da 100644
@@ -84,12 +84,40 @@ static int tegra30_ahub_runtime_resume(struct device *dev)
 	return 0;
 }
 
+int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif rxcif,
+			       struct tegra30_ahub_cif_conf *cif_conf)
+{
+	int channel = rxcif - TEGRA30_AHUB_RXCIF_APBIF_RX0;
+	u32 reg, val;
+
+	pm_runtime_get_sync(ahub->dev);
+
+	reg = TEGRA30_AHUB_CHANNEL_CTRL +
+	      (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
+	val = tegra30_apbif_read(reg);
+	val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
+		 TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
+	val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
+	       TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
+	       TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
+	tegra30_apbif_write(reg, val);
+
+	cif_conf->direction = TEGRA30_AUDIOCIF_DIRECTION_RX;
+
+	reg = TEGRA30_AHUB_CIF_RX_CTRL +
+	      (channel * TEGRA30_AHUB_CIF_RX_CTRL_STRIDE);
+	ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, cif_conf);
+
+	pm_runtime_put(ahub->dev);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tegra30_ahub_setup_rx_fifo);
+
 int tegra30_ahub_allocate_rx_fifo(enum tegra30_ahub_rxcif *rxcif,
 				  char *dmachan, int dmachan_len,
 				  dma_addr_t *fiforeg)
 {
 	int channel;
-	u32 reg, val;
 	struct tegra30_ahub_cif_conf cif_conf;
 
 	channel = find_first_zero_bit(ahub->rx_usage,

@@ -104,37 +132,14 @@ int tegra30_ahub_allocate_rx_fifo(enum tegra30_ahub_rxcif *rxcif,
 	*fiforeg = ahub->apbif_addr + TEGRA30_AHUB_CHANNEL_RXFIFO +
 		   (channel * TEGRA30_AHUB_CHANNEL_RXFIFO_STRIDE);
 
-	pm_runtime_get_sync(ahub->dev);
+	memset(&cif_conf, 0, sizeof(cif_conf));
 
-	reg = TEGRA30_AHUB_CHANNEL_CTRL +
-	      (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
-	val = tegra30_apbif_read(reg);
-	val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
-		 TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
-	val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
-	       TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
-	       TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
-	tegra30_apbif_write(reg, 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.expand = 0;
-	cif_conf.stereo_conv = 0;
-	cif_conf.replicate = 0;
-	cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_RX;
-	cif_conf.truncate = 0;
-	cif_conf.mono_conv = 0;
-
-	reg = TEGRA30_AHUB_CIF_RX_CTRL +
-	      (channel * TEGRA30_AHUB_CIF_RX_CTRL_STRIDE);
-	ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, &cif_conf);
-
-	pm_runtime_put(ahub->dev);
 
-	return 0;
+	return tegra30_ahub_setup_rx_fifo(*rxcif, &cif_conf);
 }
 EXPORT_SYMBOL_GPL(tegra30_ahub_allocate_rx_fifo);
 

@@ -186,12 +191,40 @@ int tegra30_ahub_free_rx_fifo(enum tegra30_ahub_rxcif rxcif)
 }
 EXPORT_SYMBOL_GPL(tegra30_ahub_free_rx_fifo);
 
+int tegra30_ahub_setup_tx_fifo(enum tegra30_ahub_txcif txcif,
+			       struct tegra30_ahub_cif_conf *cif_conf)
+{
+	int channel = txcif - TEGRA30_AHUB_TXCIF_APBIF_TX0;
+	u32 reg, val;
+
+	pm_runtime_get_sync(ahub->dev);
+
+	reg = TEGRA30_AHUB_CHANNEL_CTRL +
+	      (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
+	val = tegra30_apbif_read(reg);
+	val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_MASK |
+		 TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_MASK);
+	val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_SHIFT) |
+	       TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_EN |
+	       TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_16;
+	tegra30_apbif_write(reg, val);
+
+	cif_conf->direction = TEGRA30_AUDIOCIF_DIRECTION_TX;
+
+	reg = TEGRA30_AHUB_CIF_TX_CTRL +
+	      (channel * TEGRA30_AHUB_CIF_TX_CTRL_STRIDE);
+	ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, cif_conf);
+
+	pm_runtime_put(ahub->dev);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tegra30_ahub_setup_tx_fifo);
+
 int tegra30_ahub_allocate_tx_fifo(enum tegra30_ahub_txcif *txcif,
 				  char *dmachan, int dmachan_len,
 				  dma_addr_t *fiforeg)
 {
 	int channel;
-	u32 reg, val;
 	struct tegra30_ahub_cif_conf cif_conf;
 
 	channel = find_first_zero_bit(ahub->tx_usage,

@@ -206,37 +239,14 @@ int tegra30_ahub_allocate_tx_fifo(enum tegra30_ahub_txcif *txcif,
 	*fiforeg = ahub->apbif_addr + TEGRA30_AHUB_CHANNEL_TXFIFO +
 		   (channel * TEGRA30_AHUB_CHANNEL_TXFIFO_STRIDE);
 
-	pm_runtime_get_sync(ahub->dev);
-
-	reg = TEGRA30_AHUB_CHANNEL_CTRL +
-	      (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
-	val = tegra30_apbif_read(reg);
-	val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_MASK |
-		 TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_MASK);
-	val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_SHIFT) |
-	       TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_EN |
-	       TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_16;
-	tegra30_apbif_write(reg, val);
-
-	cif_conf.threshold = 0;
+	memset(&cif_conf, 0, sizeof(cif_conf));
 	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.expand = 0;
-	cif_conf.stereo_conv = 0;
-	cif_conf.replicate = 0;
 	cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_TX;
-	cif_conf.truncate = 0;
-	cif_conf.mono_conv = 0;
-
-	reg = TEGRA30_AHUB_CIF_TX_CTRL +
-	      (channel * TEGRA30_AHUB_CIF_TX_CTRL_STRIDE);
-	ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, &cif_conf);
 
-	pm_runtime_put(ahub->dev);
-
-	return 0;
+	return tegra30_ahub_setup_tx_fifo(*txcif, &cif_conf);
 }
 EXPORT_SYMBOL_GPL(tegra30_ahub_allocate_tx_fifo);
sound/soc/tegra/tegra30_ahub.h +5 -0
index 6889c5f23d02..26120aee64b3 100644
@@ -490,6 +490,11 @@ void tegra30_ahub_set_cif(struct regmap *regmap, unsigned int reg,
 void tegra124_ahub_set_cif(struct regmap *regmap, unsigned int reg,
 			   struct tegra30_ahub_cif_conf *conf);
 
+extern int tegra30_ahub_setup_tx_fifo(enum tegra30_ahub_txcif txcif,
+				      struct tegra30_ahub_cif_conf *cif_conf);
+extern int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif,
+				      struct tegra30_ahub_cif_conf *cif_conf);
+
 struct tegra30_ahub_soc_data {
 	u32 mod_list_mask;
 	void (*set_audio_cif)(struct regmap *regmap,
sound/soc/tegra/tegra30_i2s.c +2 -0
index a03692b0afc3..19ac49df6cc8 100644
@@ -202,9 +202,11 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_RX;
+		tegra30_ahub_setup_tx_fifo(i2s->playback_fifo_cif, &cif_conf);
 		reg = TEGRA30_I2S_CIF_RX_CTRL;
 	} else {
 		cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_TX;
+		tegra30_ahub_setup_rx_fifo(i2s->capture_fifo_cif, &cif_conf);
 		reg = TEGRA30_I2S_CIF_TX_CTRL;
 	}
Signature · 1 line
2.23.0
maintainer
#16 lore raw

Re: [alsa-devel] [PATCH v3 6/7] ASoC: tegra: config fifos on hw_param changes

Quoted 88 lines · Ben Dooks

If the hw_params uses a different bit or channel count, then we need to change both the I2S unit's CIF configuration as well as the APBIF one.

To allow changing the APBIF, add a call to reconfigure the RX or TX FIFO without changing the DMA or allocation, and get the I2S driver to call it once the hw params have been calculate.

Signed-off-by Ben Dooks <[email protected]>
---
 sound/soc/tegra/tegra30_ahub.c | 114 ++++++++++++++++++---------------
 sound/soc/tegra/tegra30_ahub.h |   5 ++
 sound/soc/tegra/tegra30_i2s.c  |   2 +
 3 files changed, 69 insertions(+), 52 deletions(-)
sound/soc/tegra/tegra30_ahub.c +30 -18
index 952381260dc3..58e05ceb86da 100644
@@ -84,12 +84,40 @@ static int tegra30_ahub_runtime_resume(struct device *dev)
 	return 0;
 }
 
+int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif rxcif,
+			       struct tegra30_ahub_cif_conf *cif_conf)
+{
+	int channel = rxcif - TEGRA30_AHUB_RXCIF_APBIF_RX0;
+	u32 reg, val;
+
+	pm_runtime_get_sync(ahub->dev);
+
+	reg = TEGRA30_AHUB_CHANNEL_CTRL +
+	      (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
+	val = tegra30_apbif_read(reg);
+	val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
+		 TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
+	val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
+	       TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
+	       TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
+	tegra30_apbif_write(reg, val);
+
+	cif_conf->direction = TEGRA30_AUDIOCIF_DIRECTION_RX;
+
+	reg = TEGRA30_AHUB_CIF_RX_CTRL +
+	      (channel * TEGRA30_AHUB_CIF_RX_CTRL_STRIDE);
+	ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, cif_conf);
+
+	pm_runtime_put(ahub->dev);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tegra30_ahub_setup_rx_fifo);
+
 int tegra30_ahub_allocate_rx_fifo(enum tegra30_ahub_rxcif *rxcif,
 				  char *dmachan, int dmachan_len,
 				  dma_addr_t *fiforeg)
 {
 	int channel;
-	u32 reg, val;
 	struct tegra30_ahub_cif_conf cif_conf;
 
 	channel = find_first_zero_bit(ahub->rx_usage,

@@ -104,37 +132,14 @@ int tegra30_ahub_allocate_rx_fifo(enum tegra30_ahub_rxcif *rxcif,
 	*fiforeg = ahub->apbif_addr + TEGRA30_AHUB_CHANNEL_RXFIFO +
 		   (channel * TEGRA30_AHUB_CHANNEL_RXFIFO_STRIDE);
 
-	pm_runtime_get_sync(ahub->dev);
+	memset(&cif_conf, 0, sizeof(cif_conf));
 
-	reg = TEGRA30_AHUB_CHANNEL_CTRL +
-	      (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
-	val = tegra30_apbif_read(reg);
-	val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
-		 TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
-	val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
-	       TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
-	       TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
-	tegra30_apbif_write(reg, 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.expand = 0;
-	cif_conf.stereo_conv = 0;
-	cif_conf.replicate = 0;
-	cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_RX;
-	cif_conf.truncate = 0;

sound/soc/tegra/tegra30_ahub.c int tegra30_ahub_allocate_rx_fifo(enum tegra30_ahub_rxcif *rxcif, · line 142

Quoted 10 lines
sound/soc/tegra/tegra30_ahub.c +1 -8
-	cif_conf.mono_conv = 0;
-
-	reg = TEGRA30_AHUB_CIF_RX_CTRL +
-	      (channel * TEGRA30_AHUB_CIF_RX_CTRL_STRIDE);
-	ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, &cif_conf);
-
-	pm_runtime_put(ahub->dev);
 
-	return 0;
+	return tegra30_ahub_setup_rx_fifo(*rxcif, &cif_conf);

It seems a bit odd, that you still configure some of the cif_conf members and then call tegra30_ahub_setup_rx_fifo() here. Why not just allocate it and then move all the programming to tegra30_ahub_setup_rx_fifo()?

Quoted 106 lines
sound/soc/tegra/tegra30_ahub.c +31 -26
 }
 EXPORT_SYMBOL_GPL(tegra30_ahub_allocate_rx_fifo);
 

@@ -186,12 +191,40 @@ int tegra30_ahub_free_rx_fifo(enum tegra30_ahub_rxcif rxcif)
 }
 EXPORT_SYMBOL_GPL(tegra30_ahub_free_rx_fifo);
 
+int tegra30_ahub_setup_tx_fifo(enum tegra30_ahub_txcif txcif,
+			       struct tegra30_ahub_cif_conf *cif_conf)
+{
+	int channel = txcif - TEGRA30_AHUB_TXCIF_APBIF_TX0;
+	u32 reg, val;
+
+	pm_runtime_get_sync(ahub->dev);
+
+	reg = TEGRA30_AHUB_CHANNEL_CTRL +
+	      (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
+	val = tegra30_apbif_read(reg);
+	val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_MASK |
+		 TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_MASK);
+	val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_SHIFT) |
+	       TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_EN |
+	       TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_16;
+	tegra30_apbif_write(reg, val);
+
+	cif_conf->direction = TEGRA30_AUDIOCIF_DIRECTION_TX;
+
+	reg = TEGRA30_AHUB_CIF_TX_CTRL +
+	      (channel * TEGRA30_AHUB_CIF_TX_CTRL_STRIDE);
+	ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, cif_conf);
+
+	pm_runtime_put(ahub->dev);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tegra30_ahub_setup_tx_fifo);
+
 int tegra30_ahub_allocate_tx_fifo(enum tegra30_ahub_txcif *txcif,
 				  char *dmachan, int dmachan_len,
 				  dma_addr_t *fiforeg)
 {
 	int channel;
-	u32 reg, val;
 	struct tegra30_ahub_cif_conf cif_conf;
 
 	channel = find_first_zero_bit(ahub->tx_usage,

@@ -206,37 +239,14 @@ int tegra30_ahub_allocate_tx_fifo(enum tegra30_ahub_txcif *txcif,
 	*fiforeg = ahub->apbif_addr + TEGRA30_AHUB_CHANNEL_TXFIFO +
 		   (channel * TEGRA30_AHUB_CHANNEL_TXFIFO_STRIDE);
 
-	pm_runtime_get_sync(ahub->dev);
-
-	reg = TEGRA30_AHUB_CHANNEL_CTRL +
-	      (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
-	val = tegra30_apbif_read(reg);
-	val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_MASK |
-		 TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_MASK);
-	val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_SHIFT) |
-	       TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_EN |
-	       TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_16;
-	tegra30_apbif_write(reg, val);
-
-	cif_conf.threshold = 0;
+	memset(&cif_conf, 0, sizeof(cif_conf));
 	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.expand = 0;
-	cif_conf.stereo_conv = 0;
-	cif_conf.replicate = 0;
 	cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_TX;
-	cif_conf.truncate = 0;
-	cif_conf.mono_conv = 0;
-
-	reg = TEGRA30_AHUB_CIF_TX_CTRL +
-	      (channel * TEGRA30_AHUB_CIF_TX_CTRL_STRIDE);
-	ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, &cif_conf);
 
-	pm_runtime_put(ahub->dev);
-
-	return 0;
+	return tegra30_ahub_setup_tx_fifo(*txcif, &cif_conf);
 }
 EXPORT_SYMBOL_GPL(tegra30_ahub_allocate_tx_fifo);
sound/soc/tegra/tegra30_ahub.h +5 -0
index 6889c5f23d02..26120aee64b3 100644
@@ -490,6 +490,11 @@ void tegra30_ahub_set_cif(struct regmap *regmap, unsigned int reg,
 void tegra124_ahub_set_cif(struct regmap *regmap, unsigned int reg,
 			   struct tegra30_ahub_cif_conf *conf);
 
+extern int tegra30_ahub_setup_tx_fifo(enum tegra30_ahub_txcif txcif,
+				      struct tegra30_ahub_cif_conf *cif_conf);
+extern int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif,
+				      struct tegra30_ahub_cif_conf *cif_conf);
+
 struct tegra30_ahub_soc_data {
 	u32 mod_list_mask;
 	void (*set_audio_cif)(struct regmap *regmap,
sound/soc/tegra/tegra30_i2s.c +0 -0
index a03692b0afc3..19ac49df6cc8 100644

sound/soc/tegra/tegra30_i2s.c static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream, · line 204

Quoted 3 lines
sound/soc/tegra/tegra30_i2s.c +0 -0
@@ -202,9 +202,11 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_RX;

You set the direction here and then set it again in tegra30_ahub_setup_tx_fifo(). It only needs to be done once.

sound/soc/tegra/tegra30_i2s.c static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream, · line 212

Quoted 9 lines
sound/soc/tegra/tegra30_i2s.c +2 -0
+		tegra30_ahub_setup_tx_fifo(i2s->playback_fifo_cif, &cif_conf);
 		reg = TEGRA30_I2S_CIF_RX_CTRL;
 	} else {
 		cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_TX;
+		tegra30_ahub_setup_rx_fifo(i2s->capture_fifo_cif, &cif_conf);
 		reg = TEGRA30_I2S_CIF_TX_CTRL;
 	}

Cheers
Jon

Signature · 1 line
nvpublic
author
#17 lore raw

Re: [alsa-devel] [PATCH v3 6/7] ASoC: tegra: config fifos on hw_param changes

Quoted 98 lines · Jon Hunter
Quoted 96 lines · Ben Dooks

If the hw_params uses a different bit or channel count, then we need to change both the I2S unit's CIF configuration as well as the APBIF one.

To allow changing the APBIF, add a call to reconfigure the RX or TX FIFO without changing the DMA or allocation, and get the I2S driver to call it once the hw params have been calculate.

Signed-off-by Ben Dooks <[email protected]>
---
  sound/soc/tegra/tegra30_ahub.c | 114 ++++++++++++++++++---------------
  sound/soc/tegra/tegra30_ahub.h |   5 ++
  sound/soc/tegra/tegra30_i2s.c  |   2 +
  3 files changed, 69 insertions(+), 52 deletions(-)
sound/soc/tegra/tegra30_ahub.c +30 -25
index 952381260dc3..58e05ceb86da 100644
@@ -84,12 +84,40 @@ static int tegra30_ahub_runtime_resume(struct device *dev)
  	return 0;
  }
 
+int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif rxcif,
+			       struct tegra30_ahub_cif_conf *cif_conf)
+{
+	int channel = rxcif - TEGRA30_AHUB_RXCIF_APBIF_RX0;
+	u32 reg, val;
+
+	pm_runtime_get_sync(ahub->dev);
+
+	reg = TEGRA30_AHUB_CHANNEL_CTRL +
+	      (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
+	val = tegra30_apbif_read(reg);
+	val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
+		 TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
+	val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
+	       TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
+	       TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
+	tegra30_apbif_write(reg, val);
+
+	cif_conf->direction = TEGRA30_AUDIOCIF_DIRECTION_RX;
+
+	reg = TEGRA30_AHUB_CIF_RX_CTRL +
+	      (channel * TEGRA30_AHUB_CIF_RX_CTRL_STRIDE);
+	ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, cif_conf);
+
+	pm_runtime_put(ahub->dev);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tegra30_ahub_setup_rx_fifo);
+
  int tegra30_ahub_allocate_rx_fifo(enum tegra30_ahub_rxcif *rxcif,
  				  char *dmachan, int dmachan_len,
  				  dma_addr_t *fiforeg)
  {
  	int channel;
-	u32 reg, val;
  	struct tegra30_ahub_cif_conf cif_conf;
 
  	channel = find_first_zero_bit(ahub->rx_usage,

@@ -104,37 +132,14 @@ int tegra30_ahub_allocate_rx_fifo(enum tegra30_ahub_rxcif *rxcif,
  	*fiforeg = ahub->apbif_addr + TEGRA30_AHUB_CHANNEL_RXFIFO +
  		   (channel * TEGRA30_AHUB_CHANNEL_RXFIFO_STRIDE);
 
-	pm_runtime_get_sync(ahub->dev);
+	memset(&cif_conf, 0, sizeof(cif_conf));
 
-	reg = TEGRA30_AHUB_CHANNEL_CTRL +
-	      (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
-	val = tegra30_apbif_read(reg);
-	val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
-		 TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
-	val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
-	       TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
-	       TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
-	tegra30_apbif_write(reg, 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.expand = 0;
-	cif_conf.stereo_conv = 0;
-	cif_conf.replicate = 0;
-	cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_RX;
-	cif_conf.truncate = 0;
-	cif_conf.mono_conv = 0;
-
-	reg = TEGRA30_AHUB_CIF_RX_CTRL +
-	      (channel * TEGRA30_AHUB_CIF_RX_CTRL_STRIDE);
-	ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, &cif_conf);
-
-	pm_runtime_put(ahub->dev);
Quoted 7 lines

sound/soc/tegra/tegra30_ahub.c int tegra30_ahub_allocate_rx_fifo(enum tegra30_ahub_rxcif *rxcif, · line 142

Quoted 2 lines
sound/soc/tegra/tegra30_ahub.c +1 -1
-	return 0;
+	return tegra30_ahub_setup_rx_fifo(*rxcif, &cif_conf);

It seems a bit odd, that you still configure some of the cif_conf members and then call tegra30_ahub_setup_rx_fifo() here. Why not just allocate it and then move all the programming to tegra30_ahub_setup_rx_fifo()?

I was trying to keep the behaviour the same, IIRC the original is first called before the format information is known.

Quoted 107 lines
Quoted 107 lines
sound/soc/tegra/tegra30_ahub.c +31 -26
  }
  EXPORT_SYMBOL_GPL(tegra30_ahub_allocate_rx_fifo);
 

@@ -186,12 +191,40 @@ int tegra30_ahub_free_rx_fifo(enum tegra30_ahub_rxcif rxcif)
  }
  EXPORT_SYMBOL_GPL(tegra30_ahub_free_rx_fifo);
 
+int tegra30_ahub_setup_tx_fifo(enum tegra30_ahub_txcif txcif,
+			       struct tegra30_ahub_cif_conf *cif_conf)
+{
+	int channel = txcif - TEGRA30_AHUB_TXCIF_APBIF_TX0;
+	u32 reg, val;
+
+	pm_runtime_get_sync(ahub->dev);
+
+	reg = TEGRA30_AHUB_CHANNEL_CTRL +
+	      (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
+	val = tegra30_apbif_read(reg);
+	val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_MASK |
+		 TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_MASK);
+	val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_SHIFT) |
+	       TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_EN |
+	       TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_16;
+	tegra30_apbif_write(reg, val);
+
+	cif_conf->direction = TEGRA30_AUDIOCIF_DIRECTION_TX;
+
+	reg = TEGRA30_AHUB_CIF_TX_CTRL +
+	      (channel * TEGRA30_AHUB_CIF_TX_CTRL_STRIDE);
+	ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, cif_conf);
+
+	pm_runtime_put(ahub->dev);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tegra30_ahub_setup_tx_fifo);
+
  int tegra30_ahub_allocate_tx_fifo(enum tegra30_ahub_txcif *txcif,
  				  char *dmachan, int dmachan_len,
  				  dma_addr_t *fiforeg)
  {
  	int channel;
-	u32 reg, val;
  	struct tegra30_ahub_cif_conf cif_conf;
 
  	channel = find_first_zero_bit(ahub->tx_usage,

@@ -206,37 +239,14 @@ int tegra30_ahub_allocate_tx_fifo(enum tegra30_ahub_txcif *txcif,
  	*fiforeg = ahub->apbif_addr + TEGRA30_AHUB_CHANNEL_TXFIFO +
  		   (channel * TEGRA30_AHUB_CHANNEL_TXFIFO_STRIDE);
 
-	pm_runtime_get_sync(ahub->dev);
-
-	reg = TEGRA30_AHUB_CHANNEL_CTRL +
-	      (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
-	val = tegra30_apbif_read(reg);
-	val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_MASK |
-		 TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_MASK);
-	val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_SHIFT) |
-	       TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_EN |
-	       TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_16;
-	tegra30_apbif_write(reg, val);
-
-	cif_conf.threshold = 0;
+	memset(&cif_conf, 0, sizeof(cif_conf));
  	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.expand = 0;
-	cif_conf.stereo_conv = 0;
-	cif_conf.replicate = 0;
  	cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_TX;
-	cif_conf.truncate = 0;
-	cif_conf.mono_conv = 0;
-
-	reg = TEGRA30_AHUB_CIF_TX_CTRL +
-	      (channel * TEGRA30_AHUB_CIF_TX_CTRL_STRIDE);
-	ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, &cif_conf);
 
-	pm_runtime_put(ahub->dev);
-
-	return 0;
+	return tegra30_ahub_setup_tx_fifo(*txcif, &cif_conf);
  }
  EXPORT_SYMBOL_GPL(tegra30_ahub_allocate_tx_fifo);
sound/soc/tegra/tegra30_ahub.h +5 -0
index 6889c5f23d02..26120aee64b3 100644
@@ -490,6 +490,11 @@ void tegra30_ahub_set_cif(struct regmap *regmap, unsigned int reg,
  void tegra124_ahub_set_cif(struct regmap *regmap, unsigned int reg,
  			   struct tegra30_ahub_cif_conf *conf);
 
+extern int tegra30_ahub_setup_tx_fifo(enum tegra30_ahub_txcif txcif,
+				      struct tegra30_ahub_cif_conf *cif_conf);
+extern int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif,
+				      struct tegra30_ahub_cif_conf *cif_conf);
+
  struct tegra30_ahub_soc_data {
  	u32 mod_list_mask;
  	void (*set_audio_cif)(struct regmap *regmap,
sound/soc/tegra/tegra30_i2s.c +0 -0
index a03692b0afc3..19ac49df6cc8 100644
@@ -202,9 +202,11 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
Quoted 5 lines

sound/soc/tegra/tegra30_i2s.c static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream, · line 204

Quoted 2 lines
sound/soc/tegra/tegra30_i2s.c +0 -0
  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
  		cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_RX;

You set the direction here and then set it again in tegra30_ahub_setup_tx_fifo(). It only needs to be done once.

ok, will fix that.

Quoted 13 lines

sound/soc/tegra/tegra30_i2s.c static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream, · line 212

Quoted 9 lines
sound/soc/tegra/tegra30_i2s.c +2 -0
+		tegra30_ahub_setup_tx_fifo(i2s->playback_fifo_cif, &cif_conf);
  		reg = TEGRA30_I2S_CIF_RX_CTRL;
  	} else {
  		cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_TX;
+		tegra30_ahub_setup_rx_fifo(i2s->capture_fifo_cif, &cif_conf);
  		reg = TEGRA30_I2S_CIF_TX_CTRL;
  	}

Cheers
Jon

Signature · 4 lines
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html
maintainer
#18 lore raw

Re: [alsa-devel] [PATCH v3 6/7] ASoC: tegra: config fifos on hw_param changes

Quoted 105 lines · Ben Dooks
Quoted 95 lines · Jon Hunter
Quoted 93 lines · Ben Dooks

If the hw_params uses a different bit or channel count, then we need to change both the I2S unit's CIF configuration as well as the APBIF one.

To allow changing the APBIF, add a call to reconfigure the RX or TX FIFO without changing the DMA or allocation, and get the I2S driver to call it once the hw params have been calculate.

Signed-off-by Ben Dooks <[email protected]>

---
  sound/soc/tegra/tegra30_ahub.c | 114 ++++++++++++++++++--------------- sound/soc/tegra/tegra30_ahub.h |   5 ++ sound/soc/tegra/tegra30_i2s.c  |   2 +

  3 files changed, 69 insertions(+), 52 deletions(-)
sound/soc/tegra/tegra30_ahub.c +29 -1
index 952381260dc3..58e05ceb86da 100644
@@ -84,12 +84,40 @@ static int tegra30_ahub_runtime_resume(struct
 device *dev)
       return 0;
   }
   +int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif rxcif,
+                   struct tegra30_ahub_cif_conf *cif_conf)
+{
+    int channel = rxcif - TEGRA30_AHUB_RXCIF_APBIF_RX0;
+    u32 reg, val;
+
+    pm_runtime_get_sync(ahub->dev);
+
+    reg = TEGRA30_AHUB_CHANNEL_CTRL +
+          (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
+    val = tegra30_apbif_read(reg);
+    val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
+         TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
+    val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
+           TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
+           TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
+    tegra30_apbif_write(reg, val);
+
+    cif_conf->direction = TEGRA30_AUDIOCIF_DIRECTION_RX;
+
+    reg = TEGRA30_AHUB_CIF_RX_CTRL +
+          (channel * TEGRA30_AHUB_CIF_RX_CTRL_STRIDE);
+    ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, cif_conf);
+
+    pm_runtime_put(ahub->dev);
+    return 0;
+}
+EXPORT_SYMBOL_GPL(tegra30_ahub_setup_rx_fifo);
+
   int tegra30_ahub_allocate_rx_fifo(enum tegra30_ahub_rxcif *rxcif,
                     char *dmachan, int dmachan_len,
                     dma_addr_t *fiforeg)
   {
       int channel;
-    u32 reg, val;
       struct tegra30_ahub_cif_conf cif_conf;
         channel = find_first_zero_bit(ahub->rx_usage,

@@ -104,37 +132,14 @@ int tegra30_ahub_allocate_rx_fifo(enum
 tegra30_ahub_rxcif *rxcif,
       *fiforeg = ahub->apbif_addr + TEGRA30_AHUB_CHANNEL_RXFIFO +
              (channel * TEGRA30_AHUB_CHANNEL_RXFIFO_STRIDE);
   -    pm_runtime_get_sync(ahub->dev);
+    memset(&cif_conf, 0, sizeof(cif_conf));

  -    reg = TEGRA30_AHUB_CHANNEL_CTRL +
-          (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE); -    val = tegra30_apbif_read(reg);
-    val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK | -         TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
-    val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) | -           TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
-           TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
-    tegra30_apbif_write(reg, 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.expand = 0;
-    cif_conf.stereo_conv = 0;
-    cif_conf.replicate = 0;
-    cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_RX; -    cif_conf.truncate = 0;
-    cif_conf.mono_conv = 0;

sound/soc/tegra/tegra30_ahub.c +0 -5
-
-    reg = TEGRA30_AHUB_CIF_RX_CTRL +
-          (channel * TEGRA30_AHUB_CIF_RX_CTRL_STRIDE);
-    ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, &cif_conf);
-
Quoted 8 lines
Quoted 3 lines

-    pm_runtime_put(ahub->dev);
  -    return 0;
+    return tegra30_ahub_setup_rx_fifo(*rxcif, &cif_conf);

It seems a bit odd, that you still configure some of the cif_conf members and then call tegra30_ahub_setup_rx_fifo() here. Why not just allocate it and then move all the programming to tegra30_ahub_setup_rx_fifo()?

Quoted 2 lines

I was trying to keep the behaviour the same, IIRC the original is first called before the format information is known.

Looks like the I2S driver is the only current user of this, so splitting the function into two could make sense.

Cheers
Jon

Signature · 5 lines
nvpublic
_______________________________________________
Alsa-devel mailing list
[email protected]
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
author
#19 lore raw

PATCH v3 7/7 ASoC: tegra: take packing settings from the audio cif_config

In reply to Ben Dooks

If the CIF is not configured as 16 or 8 bit, then the packing for 8/16 bits should not be enabled as the hardware only supports 8 or 16 bit packing.

Signed-off-by Ben Dooks <[email protected]>
---
 sound/soc/tegra/tegra30_ahub.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)
sound/soc/tegra/tegra30_ahub.c +21 -8
index 58e05ceb86da..c2f2e29dd32e 100644
@@ -96,10 +96,17 @@ int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif rxcif,
 	      (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
 	val = tegra30_apbif_read(reg);
 	val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
-		 TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
-	val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
-	       TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
-	       TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
+		 TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK |
+		 TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN);
+	val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT);
+	if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_16 ||
+	    cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_8)
+		val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN;
+	if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_16)
+		val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
+	if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_8)
+		val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_8_4;
+
 	tegra30_apbif_write(reg, val);
 
 	cif_conf->direction = TEGRA30_AUDIOCIF_DIRECTION_RX;

@@ -203,10 +210,16 @@ int tegra30_ahub_setup_tx_fifo(enum tegra30_ahub_txcif txcif,
 	      (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
 	val = tegra30_apbif_read(reg);
 	val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_MASK |
-		 TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_MASK);
-	val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_SHIFT) |
-	       TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_EN |
-	       TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_16;
+		 TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_MASK |
+		 TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_EN);
+	val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_SHIFT);
+	if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_16 ||
+	    cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_8)
+		val |= TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_EN;
+	if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_16)
+		val |= TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_16;
+	if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_8)
+		val |= TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_8_4;
 	tegra30_apbif_write(reg, val);
 
 	cif_conf->direction = TEGRA30_AUDIOCIF_DIRECTION_TX;
Signature · 1 line
2.23.0