tegra30 tdm audio support

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 | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

sound/soc/tegra/tegra30_i2s.c +28 -0

index ac6983c6bd72..73f0dddeaef3 100644
@@ -254,6 +254,34 @@ static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
 	return 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);
+
+	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);

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 +17 -4

index 73f0dddeaef3..063f34c882af 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;

@@ -306,14 +317,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",

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 +17 -4

index 73f0dddeaef3..063f34c882af 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;

@@ -306,14 +317,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",

18.10.2019 18:48, Ben Dooks пишет:

Quoted 75 lines

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 +17 -4

index 73f0dddeaef3..063f34c882af 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;

@@ -306,14 +317,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",

sound/soc/tegra/tegra30_i2s.c static const struct snd_soc_dai_driver tegra30_i2s_dai_template = { · line 334

Quoted 10 lines

sound/soc/tegra/tegra30_i2s.c +3 -1

  		.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,

Hello,

This patch breaks audio on Tegra30. I don't see errors anywhere, but there is no audio and reverting this patch helps. Please fix it.

Quoted 94 lines

18.10.2019 18:48, Ben Dooks пишет:

Quoted 88 lines

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 +1 -1

index 73f0dddeaef3..063f34c882af 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;
      }

sound/soc/tegra/tegra30_i2s.c +2 -2

@@ -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;

sound/soc/tegra/tegra30_i2s.c +6 -2

@@ -306,14 +317,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,

Quoted 2 lines

      .symmetric_rates = 1,

Hello,

Quoted 2 lines

This patch breaks audio on Tegra30. I don't see errors anywhere, but there is no audio and reverting this patch helps. Please fix it.

What is the failure mode? I can try and take a look at this some time this week, but I am not sure if I have any boards with an actual useful audio output?

25.11.2019 13:37, Ben Dooks пишет:

Quoted 98 lines · Dmitry Osipenko

Quoted 96 lines

18.10.2019 18:48, Ben Dooks пишет:

Quoted 88 lines

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 +1 -1

index 73f0dddeaef3..063f34c882af 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;
      }

sound/soc/tegra/tegra30_i2s.c +2 -2

@@ -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;

sound/soc/tegra/tegra30_i2s.c +6 -2

@@ -306,14 +317,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,

Quoted 2 lines

      .symmetric_rates = 1,

Hello,

This patch breaks audio on Tegra30. I don't see errors anywhere, but there is no audio and reverting this patch helps. Please fix it.

Quoted 3 lines

What is the failure mode? I can try and take a look at this some time this week, but I am not sure if I have any boards with an actual useful audio output?

The failure mode is that there no sound. I also noticed that video playback stutters a lot if movie file has audio track, seems something times out during of the audio playback. For now I don't have any more info.

25.11.2019 20:22, Dmitry Osipenko пишет:

Quoted 99 lines

25.11.2019 13:37, Ben Dooks пишет:

Quoted 98 lines · Dmitry Osipenko

Quoted 96 lines

18.10.2019 18:48, Ben Dooks пишет:

Quoted 88 lines

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 +1 -1

index 73f0dddeaef3..063f34c882af 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;
      }

sound/soc/tegra/tegra30_i2s.c +2 -2

@@ -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;

sound/soc/tegra/tegra30_i2s.c +6 -2

@@ -306,14 +317,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,

Quoted 2 lines

      .symmetric_rates = 1,

Hello,

This patch breaks audio on Tegra30. I don't see errors anywhere, but there is no audio and reverting this patch helps. Please fix it.

Quoted 8 lines

Quoted 3 lines

What is the failure mode? I can try and take a look at this some time this week, but I am not sure if I have any boards with an actual useful audio output?

The failure mode is that there no sound. I also noticed that video playback stutters a lot if movie file has audio track, seems something times out during of the audio playback. For now I don't have any more info.

Oh, I didn't say how to reproduce it.. for example simply playing big_buck_bunny_720p_h264.mov in MPV has the audio problem.

https://download.blender.org/peach/bigbuckbunny_movies/big_buck_bunny_720p_h264.mov

25.11.2019 20:28, Dmitry Osipenko пишет:

Quoted 109 lines

25.11.2019 20:22, Dmitry Osipenko пишет:

Quoted 99 lines

25.11.2019 13:37, Ben Dooks пишет:

Quoted 98 lines · Dmitry Osipenko

Quoted 96 lines

18.10.2019 18:48, Ben Dooks пишет:

Quoted 88 lines

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 +12 -1

index 73f0dddeaef3..063f34c882af 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;

      }

sound/soc/tegra/tegra30_i2s.c +2 -2

@@ -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;

sound/soc/tegra/tegra30_i2s.c +6 -2

@@ -306,14 +317,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,

Quoted 2 lines

      .symmetric_rates = 1,

Hello,

This patch breaks audio on Tegra30. I don't see errors anywhere, but there is no audio and reverting this patch helps. Please fix it.

Quoted 8 lines

Quoted 3 lines

What is the failure mode? I can try and take a look at this some time this week, but I am not sure if I have any boards with an actual useful audio output?

The failure mode is that there no sound. I also noticed that video playback stutters a lot if movie file has audio track, seems something times out during of the audio playback. For now I don't have any more info.

Quoted 5 lines

Oh, I didn't say how to reproduce it.. for example simply playing big_buck_bunny_720p_h264.mov in MPV has the audio problem.

https://download.blender.org/peach/bigbuckbunny_movies/big_buck_bunny_720p_h264.mov

Hello Ben,

Do you have any updates? I just re-check whether problem persists and it's still there using a recent linux-next.

Interestingly, I can hear some sound now, but it's very distorted.

If you don't have a solution, then what about to revert the patches for now and try again later on?

25.11.2019 20:22, Dmitry Osipenko пишет:

Quoted 99 lines

25.11.2019 13:37, Ben Dooks пишет:

Quoted 98 lines · Dmitry Osipenko

Quoted 96 lines

18.10.2019 18:48, Ben Dooks пишет:

Quoted 88 lines

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 +1 -1

index 73f0dddeaef3..063f34c882af 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;
      }

sound/soc/tegra/tegra30_i2s.c +2 -2

@@ -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;

sound/soc/tegra/tegra30_i2s.c +6 -2

@@ -306,14 +317,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,

Quoted 2 lines

      .symmetric_rates = 1,

Hello,

This patch breaks audio on Tegra30. I don't see errors anywhere, but there is no audio and reverting this patch helps. Please fix it.

Quoted 8 lines

Quoted 3 lines

What is the failure mode? I can try and take a look at this some time this week, but I am not sure if I have any boards with an actual useful audio output?

The failure mode is that there no sound. I also noticed that video playback stutters a lot if movie file has audio track, seems something times out during of the audio playback. For now I don't have any more info.

Oh, I didn't say how to reproduce it.. for example simply playing big_buck_bunny_720p_h264.mov in MPV has the audio problem.

https://download.blender.org/peach/bigbuckbunny_movies/big_buck_bunny_720p_h264.mov

Quoted 109 lines · Dmitry Osipenko

25.11.2019 20:22, Dmitry Osipenko пишет:

Quoted 99 lines

25.11.2019 13:37, Ben Dooks пишет:

Quoted 98 lines · Dmitry Osipenko

Quoted 96 lines

18.10.2019 18:48, Ben Dooks пишет:

Quoted 88 lines

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 +12 -1

index 73f0dddeaef3..063f34c882af 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;

      }

sound/soc/tegra/tegra30_i2s.c +2 -2

@@ -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;

sound/soc/tegra/tegra30_i2s.c +6 -2

@@ -306,14 +317,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,

Quoted 2 lines

      .symmetric_rates = 1,

Hello,

This patch breaks audio on Tegra30. I don't see errors anywhere, but there is no audio and reverting this patch helps. Please fix it.

Quoted 8 lines

Quoted 3 lines

What is the failure mode? I can try and take a look at this some time this week, but I am not sure if I have any boards with an actual useful audio output?

The failure mode is that there no sound. I also noticed that video playback stutters a lot if movie file has audio track, seems something times out during of the audio playback. For now I don't have any more info.

Quoted 4 lines

Oh, I didn't say how to reproduce it.. for example simply playing big_buck_bunny_720p_h264.mov in MPV has the audio problem.

https://download.blender.org/peach/bigbuckbunny_movies/big_buck_bunny_720p_h264.mov

Given that the audio drivers uses regmap, it could be good to dump the I2S/AHUB registers while the clip if playing with and without this patch to see the differences. I am curious if the audio is now being played as 24 or 32-bit instead of 16-bit now these are available.

You could also dump the hw_params to see the format while playing as well ...

$ /proc/asound/<scard-name>/pcm0p/sub0/hw_params

Quoted 126 lines · Jon Hunter

Quoted 113 lines · Dmitry Osipenko

25.11.2019 20:22, Dmitry Osipenko пишет:

Quoted 106 lines

25.11.2019 13:37, Ben Dooks пишет:

Quoted 97 lines · Dmitry Osipenko

Quoted 96 lines

18.10.2019 18:48, Ben Dooks пишет:

Quoted 90 lines

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 +1 -1

index 73f0dddeaef3..063f34c882af 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;
       }

sound/soc/tegra/tegra30_i2s.c +2 -2

@@ -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;

sound/soc/tegra/tegra30_i2s.c +6 -2

@@ -306,14 +317,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,

Quoted 2 lines

       .symmetric_rates = 1,

Hello,

Quoted 7 lines

Quoted 2 lines

This patch breaks audio on Tegra30. I don't see errors anywhere, but there is no audio and reverting this patch helps. Please fix it.

What is the failure mode? I can try and take a look at this some time this week, but I am not sure if I have any boards with an actual useful
audio output?

Quoted 5 lines

The failure mode is that there no sound. I also noticed that video playback stutters a lot if movie file has audio track, seems something times out during of the audio playback. For now I don't have any more info.

Quoted 5 lines

Oh, I didn't say how to reproduce it.. for example simply playing big_buck_bunny_720p_h264.mov in MPV has the audio problem.

https://download.blender.org/peach/bigbuckbunny_movies/big_buck_bunny_720p_h264.mov

Given that the audio drivers uses regmap, it could be good to dump the I2S/AHUB registers while the clip if playing with and without this patch to see the differences. I am curious if the audio is now being played as 24 or 32-bit instead of 16-bit now these are available.

Quoted 4 lines

You could also dump the hw_params to see the format while playing as well ...

$ /proc/asound/<scard-name>/pcm0p/sub0/hw_params

I suppose it is also possible that the codec isn't properly doing >16 bits and the fact we now offer 24 and 32 could be an issue. I've not got anything with an audio output on it that would be easy to test.

Quoted 127 lines · Jon Hunter

Quoted 126 lines · Dmitry Osipenko

Quoted 113 lines

25.11.2019 20:22, Dmitry Osipenko пишет:

Quoted 106 lines

25.11.2019 13:37, Ben Dooks пишет:

Quoted 97 lines · Dmitry Osipenko

Quoted 96 lines

18.10.2019 18:48, Ben Dooks пишет:

Quoted 90 lines

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 +1 -1

index 73f0dddeaef3..063f34c882af 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;
       }

sound/soc/tegra/tegra30_i2s.c +2 -2

@@ -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;

sound/soc/tegra/tegra30_i2s.c +6 -2

@@ -306,14 +317,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,

Quoted 2 lines

       .symmetric_rates = 1,

Hello,

Quoted 7 lines

Quoted 2 lines

This patch breaks audio on Tegra30. I don't see errors anywhere, but there is no audio and reverting this patch helps. Please fix it.

What is the failure mode? I can try and take a look at this some time this week, but I am not sure if I have any boards with an actual useful
audio output?

Quoted 5 lines

The failure mode is that there no sound. I also noticed that video playback stutters a lot if movie file has audio track, seems something times out during of the audio playback. For now I don't have any more info.

Quoted 5 lines

Oh, I didn't say how to reproduce it.. for example simply playing big_buck_bunny_720p_h264.mov in MPV has the audio problem.

https://download.blender.org/peach/bigbuckbunny_movies/big_buck_bunny_720p_h264.mov

Given that the audio drivers uses regmap, it could be good to dump the I2S/AHUB registers while the clip if playing with and without this patch to see the differences. I am curious if the audio is now being played as 24 or 32-bit instead of 16-bit now these are available.

Quoted 8 lines

Quoted 4 lines

You could also dump the hw_params to see the format while playing as well ...

$ /proc/asound/<scard-name>/pcm0p/sub0/hw_params

I suppose it is also possible that the codec isn't properly doing >16 bits and the fact we now offer 24 and 32 could be an issue. I've not got anything with an audio output on it that would be easy to test.

I thought I had tested on a Jetson TK1 (Tegra124) but it was sometime back. However, admittedly I may have only done simple 16-bit testing with speaker-test.

We do verify that all soundcards are detected and registered as expected during daily testing, but at the moment we don't have anything that verifies actual playback.

20.12.2019 16:57, Jon Hunter пишет:

Quoted 142 lines · Ben Dooks

Quoted 127 lines · Jon Hunter

Quoted 126 lines · Dmitry Osipenko

Quoted 113 lines

25.11.2019 20:22, Dmitry Osipenko пишет:

Quoted 106 lines

25.11.2019 13:37, Ben Dooks пишет:

Quoted 97 lines · Dmitry Osipenko

Quoted 96 lines

18.10.2019 18:48, Ben Dooks пишет:

Quoted 90 lines

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 +12 -1

index 73f0dddeaef3..063f34c882af 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;

       }

sound/soc/tegra/tegra30_i2s.c +2 -2

@@ -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;

sound/soc/tegra/tegra30_i2s.c +6 -2

@@ -306,14 +317,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,

Quoted 2 lines

       .symmetric_rates = 1,

Hello,

Quoted 7 lines

Quoted 2 lines

This patch breaks audio on Tegra30. I don't see errors anywhere, but there is no audio and reverting this patch helps. Please fix it.

What is the failure mode? I can try and take a look at this some time this week, but I am not sure if I have any boards with an actual useful
audio output?

Quoted 5 lines

The failure mode is that there no sound. I also noticed that video playback stutters a lot if movie file has audio track, seems something times out during of the audio playback. For now I don't have any more info.

Quoted 5 lines

Oh, I didn't say how to reproduce it.. for example simply playing big_buck_bunny_720p_h264.mov in MPV has the audio problem.

https://download.blender.org/peach/bigbuckbunny_movies/big_buck_bunny_720p_h264.mov

Given that the audio drivers uses regmap, it could be good to dump the I2S/AHUB registers while the clip if playing with and without this patch to see the differences. I am curious if the audio is now being played as 24 or 32-bit instead of 16-bit now these are available.

Quoted 8 lines

Quoted 4 lines

You could also dump the hw_params to see the format while playing as well ...

$ /proc/asound/<scard-name>/pcm0p/sub0/hw_params

I suppose it is also possible that the codec isn't properly doing >16 bits and the fact we now offer 24 and 32 could be an issue. I've not got anything with an audio output on it that would be easy to test.

I thought I had tested on a Jetson TK1 (Tegra124) but it was sometime back. However, admittedly I may have only done simple 16-bit testing with speaker-test.

Quoted 3 lines

We do verify that all soundcards are detected and registered as expected during daily testing, but at the moment we don't have anything that verifies actual playback.

Please take a look at the attached logs.

Quoted 154 lines

20.12.2019 16:57, Jon Hunter пишет:

Quoted 152 lines · Ben Dooks

Quoted 134 lines · Jon Hunter

Quoted 133 lines · Dmitry Osipenko

Quoted 117 lines

25.11.2019 20:22, Dmitry Osipenko пишет:

Quoted 109 lines

25.11.2019 13:37, Ben Dooks пишет:

Quoted 102 lines · Dmitry Osipenko

Quoted 97 lines

18.10.2019 18:48, Ben Dooks пишет:

Quoted 91 lines

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 +1 -1

index 73f0dddeaef3..063f34c882af 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;
        }

sound/soc/tegra/tegra30_i2s.c +2 -2

@@ -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;

sound/soc/tegra/tegra30_i2s.c +6 -2

@@ -306,14 +317,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,

Quoted 2 lines

        .symmetric_rates = 1,

Hello,

Quoted 3 lines

This patch breaks audio on Tegra30. I don't see errors anywhere, but
there is no audio and reverting this patch helps. Please fix it.

Quoted 5 lines

What is the failure mode? I can try and take a look at this some time
this week, but I am not sure if I have any boards with an actual useful
audio output?

Quoted 6 lines

The failure mode is that there no sound. I also noticed that video playback stutters a lot if movie file has audio track, seems something
times out during of the audio playback. For now I don't have any more info.

Quoted 6 lines

Oh, I didn't say how to reproduce it.. for example simply playing big_buck_bunny_720p_h264.mov in MPV has the audio problem.

https://download.blender.org/peach/bigbuckbunny_movies/big_buck_bunny_720p_h264.mov

Given that the audio drivers uses regmap, it could be good to dump the I2S/AHUB registers while the clip if playing with and without this patch
to see the differences. I am curious if the audio is now being played as
24 or 32-bit instead of 16-bit now these are available.

Quoted 8 lines

Quoted 4 lines

You could also dump the hw_params to see the format while playing as well ...

$ /proc/asound/<scard-name>/pcm0p/sub0/hw_params

I suppose it is also possible that the codec isn't properly doing >16 bits and the fact we now offer 24 and 32 could be an issue. I've not got anything with an audio output on it that would be easy to test.

I thought I had tested on a Jetson TK1 (Tegra124) but it was sometime back. However, admittedly I may have only done simple 16-bit testing with speaker-test.

We do verify that all soundcards are detected and registered as expected during daily testing, but at the moment we don't have anything that verifies actual playback.

Quoted 1 line

Please take a look at the attached logs.

I wonder if we are falling into FIFO configuration issues with the non-16 bit case.

Can you try adding the following two patches?

20.12.2019 17:56, Ben Dooks пишет:

Quoted 157 lines · Dmitry Osipenko

Quoted 155 lines

20.12.2019 16:57, Jon Hunter пишет:

Quoted 145 lines · Ben Dooks

Quoted 134 lines · Jon Hunter

Quoted 133 lines · Dmitry Osipenko

Quoted 117 lines

25.11.2019 20:22, Dmitry Osipenko пишет:

Quoted 109 lines

25.11.2019 13:37, Ben Dooks пишет:

Quoted 102 lines · Dmitry Osipenko

Quoted 97 lines

18.10.2019 18:48, Ben Dooks пишет:

Quoted 91 lines

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 +12 -1

index 73f0dddeaef3..063f34c882af 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;

        }

sound/soc/tegra/tegra30_i2s.c +2 -2

@@ -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;

sound/soc/tegra/tegra30_i2s.c +6 -2

@@ -306,14 +317,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,

Quoted 2 lines

        .symmetric_rates = 1,

Hello,

Quoted 3 lines

This patch breaks audio on Tegra30. I don't see errors anywhere, but
there is no audio and reverting this patch helps. Please fix it.

Quoted 5 lines

What is the failure mode? I can try and take a look at this some time
this week, but I am not sure if I have any boards with an actual useful
audio output?

Quoted 6 lines

The failure mode is that there no sound. I also noticed that video playback stutters a lot if movie file has audio track, seems something
times out during of the audio playback. For now I don't have any more info.

Quoted 6 lines

Oh, I didn't say how to reproduce it.. for example simply playing big_buck_bunny_720p_h264.mov in MPV has the audio problem.

https://download.blender.org/peach/bigbuckbunny_movies/big_buck_bunny_720p_h264.mov

Given that the audio drivers uses regmap, it could be good to dump the I2S/AHUB registers while the clip if playing with and without this patch
to see the differences. I am curious if the audio is now being played as
24 or 32-bit instead of 16-bit now these are available.

Quoted 8 lines

Quoted 4 lines

You could also dump the hw_params to see the format while playing as well ...

$ /proc/asound/<scard-name>/pcm0p/sub0/hw_params

I suppose it is also possible that the codec isn't properly doing >16 bits and the fact we now offer 24 and 32 could be an issue. I've not got anything with an audio output on it that would be easy to test.

Quoted 7 lines

I thought I had tested on a Jetson TK1 (Tegra124) but it was sometime back. However, admittedly I may have only done simple 16-bit testing with speaker-test.

We do verify that all soundcards are detected and registered as expected during daily testing, but at the moment we don't have anything that verifies actual playback.

Please take a look at the attached logs.

Quoted 4 lines

I wonder if we are falling into FIFO configuration issues with the non-16 bit case.

Can you try adding the following two patches?

It is much better now! There is no horrible noise and no stuttering, but audio still has a "robotic" effect, like freq isn't correct.

Quoted 166 lines

20.12.2019 17:56, Ben Dooks пишет:

Quoted 165 lines · Dmitry Osipenko

Quoted 154 lines

20.12.2019 16:57, Jon Hunter пишет:

Quoted 153 lines · Ben Dooks

Quoted 138 lines · Jon Hunter

Quoted 137 lines · Dmitry Osipenko

Quoted 119 lines

25.11.2019 20:22, Dmitry Osipenko пишет:

Quoted 111 lines

25.11.2019 13:37, Ben Dooks пишет:

Quoted 104 lines · Dmitry Osipenko

Quoted 99 lines

18.10.2019 18:48, Ben Dooks пишет:

Quoted 93 lines

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 +0 -0

index 73f0dddeaef3..063f34c882af 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)

sound/soc/tegra/tegra30_i2s.c +0 -0

@@ -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;
         }

sound/soc/tegra/tegra30_i2s.c +2 -2

@@ -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;

sound/soc/tegra/tegra30_i2s.c +6 -2

@@ -306,14 +317,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,

Quoted 2 lines

         .symmetric_rates = 1,

Hello,

Quoted 3 lines

This patch breaks audio on Tegra30. I don't see errors anywhere, but
there is no audio and reverting this patch helps. Please fix it.

Quoted 5 lines

What is the failure mode? I can try and take a look at this some time
this week, but I am not sure if I have any boards with an actual useful
audio output?

Quoted 6 lines

The failure mode is that there no sound. I also noticed that video playback stutters a lot if movie file has audio track, seems something
times out during of the audio playback. For now I don't have any more info.

Quoted 7 lines

Oh, I didn't say how to reproduce it.. for example simply playing big_buck_bunny_720p_h264.mov in MPV has the audio problem.

https://download.blender.org/peach/bigbuckbunny_movies/big_buck_bunny_720p_h264.mov

Given that the audio drivers uses regmap, it could be good to dump the
I2S/AHUB registers while the clip if playing with and without this patch
to see the differences. I am curious if the audio is now being played as
24 or 32-bit instead of 16-bit now these are available.

Quoted 8 lines

Quoted 4 lines

You could also dump the hw_params to see the format while playing as well ...

$ /proc/asound/<scard-name>/pcm0p/sub0/hw_params

I suppose it is also possible that the codec isn't properly doing >16 bits and the fact we now offer 24 and 32 could be an issue. I've not got anything with an audio output on it that would be easy to test.

I thought I had tested on a Jetson TK1 (Tegra124) but it was sometime back. However, admittedly I may have only done simple 16-bit testing with speaker-test.

Quoted 6 lines

Quoted 4 lines

We do verify that all soundcards are detected and registered as expected
during daily testing, but at the moment we don't have anything that verifies actual playback.

Please take a look at the attached logs.

I wonder if we are falling into FIFO configuration issues with the non-16 bit case.

Quoted 4 lines

Quoted 1 line

Can you try adding the following two patches?

It is much better now! There is no horrible noise and no stuttering, but audio still has a "robotic" effect, like freq isn't correct.

I wonder if there's an issue with FIFO stoking? I should also possibly add the correctly stop FIFOs patch as well.

20.12.2019 18:25, Ben Dooks пишет:

Quoted 172 lines · Dmitry Osipenko

Quoted 170 lines

20.12.2019 17:56, Ben Dooks пишет:

Quoted 160 lines · Dmitry Osipenko

Quoted 159 lines

20.12.2019 16:57, Jon Hunter пишет:

Quoted 157 lines · Ben Dooks

Quoted 138 lines · Jon Hunter

Quoted 137 lines · Dmitry Osipenko

Quoted 119 lines

25.11.2019 20:22, Dmitry Osipenko пишет:

Quoted 111 lines

25.11.2019 13:37, Ben Dooks пишет:

Quoted 104 lines · Dmitry Osipenko

Quoted 99 lines

18.10.2019 18:48, Ben Dooks пишет:

Quoted 95 lines

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(-)

diff --git a/sound/soc/tegra/tegra30_i2s.c
b/sound/soc/tegra/tegra30_i2s.c
index 73f0dddeaef3..063f34c882af 100644
--- a/sound/soc/tegra/tegra30_i2s.c
+++ b/sound/soc/tegra/tegra30_i2s.c
@@ -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;
@@ -306,14 +317,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,

Hello,

Quoted 3 lines

This patch breaks audio on Tegra30. I don't see errors anywhere, but
there is no audio and reverting this patch helps. Please fix it.

Quoted 5 lines

What is the failure mode? I can try and take a look at this some time
this week, but I am not sure if I have any boards with an actual useful
audio output?

Quoted 6 lines

The failure mode is that there no sound. I also noticed that video playback stutters a lot if movie file has audio track, seems something
times out during of the audio playback. For now I don't have any more info.

Quoted 7 lines

Oh, I didn't say how to reproduce it.. for example simply playing big_buck_bunny_720p_h264.mov in MPV has the audio problem.

https://download.blender.org/peach/bigbuckbunny_movies/big_buck_bunny_720p_h264.mov

Given that the audio drivers uses regmap, it could be good to dump the
I2S/AHUB registers while the clip if playing with and without this patch
to see the differences. I am curious if the audio is now being played as
24 or 32-bit instead of 16-bit now these are available.

Quoted 8 lines

Quoted 4 lines

You could also dump the hw_params to see the format while playing as well ...

$ /proc/asound/<scard-name>/pcm0p/sub0/hw_params

I suppose it is also possible that the codec isn't properly doing >16 bits and the fact we now offer 24 and 32 could be an issue. I've not got anything with an audio output on it that would be easy to test.

I thought I had tested on a Jetson TK1 (Tegra124) but it was sometime back. However, admittedly I may have only done simple 16-bit testing with speaker-test.

We do verify that all soundcards are detected and registered as expected
during daily testing, but at the moment we don't have anything that verifies actual playback.

Quoted 6 lines

Quoted 1 line

Please take a look at the attached logs.

I wonder if we are falling into FIFO configuration issues with the non-16 bit case.

Can you try adding the following two patches?

It is much better now! There is no horrible noise and no stuttering, but audio still has a "robotic" effect, like freq isn't correct.

Quoted 2 lines

I wonder if there's an issue with FIFO stoking? I should also possibly add the correctly stop FIFOs patch as well.

I'll be happy to try more patches.

Meanwhile sound on v5.5+ is broken and thus the incomplete patches should be reverted.

Quoted 190 lines

20.12.2019 18:25, Ben Dooks пишет:

Quoted 188 lines · Dmitry Osipenko

Quoted 176 lines

20.12.2019 17:56, Ben Dooks пишет:

Quoted 175 lines · Dmitry Osipenko

Quoted 173 lines

20.12.2019 16:57, Jon Hunter пишет:

Quoted 166 lines · Ben Dooks

Quoted 151 lines · Jon Hunter

Quoted 150 lines · Dmitry Osipenko

Quoted 127 lines

25.11.2019 20:22, Dmitry Osipenko пишет:

Quoted 118 lines

25.11.2019 13:37, Ben Dooks пишет:

Quoted 109 lines · Dmitry Osipenko

Quoted 107 lines

18.10.2019 18:48, 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(-)

diff --git a/sound/soc/tegra/tegra30_i2s.c
b/sound/soc/tegra/tegra30_i2s.c
index 73f0dddeaef3..063f34c882af 100644
--- a/sound/soc/tegra/tegra30_i2s.c
+++ b/sound/soc/tegra/tegra30_i2s.c
@@ -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;
@@ -306,14 +317,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,

Hello,

This patch breaks audio on Tegra30. I don't see errors
anywhere, but
there is no audio and reverting this patch helps. Please
fix it.

Quoted 7 lines

What is the failure mode? I can try and take a look at this some
time
this week, but I am not sure if I have any boards with an actual
useful
audio output?

Quoted 7 lines

The failure mode is that there no sound. I also noticed that video
playback stutters a lot if movie file has audio track, seems something
times out during of the audio playback. For now I don't have any more info.

Quoted 8 lines

Oh, I didn't say how to reproduce it.. for example simply playing big_buck_bunny_720p_h264.mov in MPV has the audio problem.

https://download.blender.org/peach/bigbuckbunny_movies/big_buck_bunny_720p_h264.mov

Given that the audio drivers uses regmap, it could be good to dump the
I2S/AHUB registers while the clip if playing with and without this patch
to see the differences. I am curious if the audio is now being played as
24 or 32-bit instead of 16-bit now these are available.

You could also dump the hw_params to see the format while playing as
well ...

Quoted 7 lines

Quoted 1 line

$ /proc/asound/<scard-name>/pcm0p/sub0/hw_params

I suppose it is also possible that the codec isn't properly doing >16
bits and the fact we now offer 24 and 32 could be an issue. I've not
got anything with an audio output on it that would be easy to test.

I thought I had tested on a Jetson TK1 (Tegra124) but it was sometime
back. However, admittedly I may have only done simple 16-bit testing with speaker-test.

Quoted 4 lines

We do verify that all soundcards are detected and registered as expected
during daily testing, but at the moment we don't have anything that verifies actual playback.

Please take a look at the attached logs.

Quoted 8 lines

Quoted 4 lines

I wonder if we are falling into FIFO configuration issues with the non-16 bit case.

Can you try adding the following two patches?

It is much better now! There is no horrible noise and no stuttering, but
audio still has a "robotic" effect, like freq isn't correct.

I wonder if there's an issue with FIFO stoking? I should also possibly add the correctly stop FIFOs patch as well.

Quoted 4 lines

I'll be happy to try more patches.

Meanwhile sound on v5.5+ is broken and thus the incomplete patches should be reverted.

Have you checked if just removing the 24/32 bits from .formats in the dai template and see if the problem continues?

I will try and
see if I can find the code that does the fifo level checking and see if the problem is in the FIFO fill or something else in the audio hub setup.

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects 24 bit samples in 32 bit words. I can't see any packing options to do 24 bit in 24 bit, so we may have to remove 24 bit sample support (which is a shame)

My preference is to remove the 24-bit support and keep the 32 bit in.

05.01.2020 03:04, Ben Dooks пишет:

Quoted 11 lines

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects 24 bit samples in 32 bit words. I can't see any packing options to do 24 bit in 24 bit, so we may have to remove 24 bit sample support (which is a shame)

Quoted 2 lines

My preference is to remove the 24-bit support and keep the 32 bit in.

Interesting.. Jon, could you please confirm that 24bit format isn't usable on T30?

Quoted 15 lines · Dmitry Osipenko

05.01.2020 03:04, Ben Dooks пишет:

Quoted 13 lines

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects 24 bit samples in 32 bit words. I can't see any packing options to do 24 bit in 24 bit, so we may have to remove 24 bit sample support (which is a shame)

My preference is to remove the 24-bit support and keep the 32 bit in.

Quoted 2 lines

Interesting.. Jon, could you please confirm that 24bit format isn't usable on T30?

If there is an option of 24 packed into 32, then I think that would work.

I can try testing that with raw data on Monday.

Quoted 23 lines · Dmitry Osipenko

Quoted 12 lines

05.01.2020 03:04, Ben Dooks пишет:

Quoted 11 lines

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects 24 bit samples in 32 bit words. I can't see any packing options to do 24 bit in 24 bit, so we may have to remove 24 bit sample support (which is a shame)

Quoted 5 lines

Quoted 2 lines

My preference is to remove the 24-bit support and keep the 32 bit in.

Interesting.. Jon, could you please confirm that 24bit format isn't usable on T30?

If there is an option of 24 packed into 32, then I think that would work.

Quoted 1 line

I can try testing that with raw data on Monday.

I need to check some things, I assumed 24 was 24 packed bits, it looks like the default is 24 in 32 bits so we may be ok. However I need to re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).

I'll follow up later,

06.01.2020 22:00, Ben Dooks пишет:

Quoted 30 lines · Ben Dooks

Quoted 18 lines · Dmitry Osipenko

Quoted 15 lines

05.01.2020 03:04, Ben Dooks пишет:

Quoted 13 lines

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects 24 bit samples in 32 bit words. I can't see any packing options to do 24 bit in 24 bit, so we may have to remove 24 bit sample support (which is a shame)

My preference is to remove the 24-bit support and keep the 32 bit in.

Quoted 6 lines

Quoted 2 lines

Interesting.. Jon, could you please confirm that 24bit format isn't usable on T30?

If there is an option of 24 packed into 32, then I think that would work.

I can try testing that with raw data on Monday.

I need to check some things, I assumed 24 was 24 packed bits, it looks like the default is 24 in 32 bits so we may be ok. However I need to re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).

Quoted 1 line

I'll follow up later,

Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly looked through the TRM doc and got impression that AHUB could re-pack data stream into something that codec supports, but maybe it's a wrong impression.

Quoted 31 lines

06.01.2020 22:00, Ben Dooks пишет:

Quoted 30 lines · Ben Dooks

Quoted 18 lines · Dmitry Osipenko

Quoted 15 lines

05.01.2020 03:04, Ben Dooks пишет:

Quoted 13 lines

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects 24 bit samples in 32 bit words. I can't see any packing options to do 24 bit in 24 bit, so we may have to remove 24 bit sample support (which is a shame)

My preference is to remove the 24-bit support and keep the 32 bit in.

Quoted 6 lines

Quoted 2 lines

Interesting.. Jon, could you please confirm that 24bit format isn't usable on T30?

If there is an option of 24 packed into 32, then I think that would work.

I can try testing that with raw data on Monday.

I need to check some things, I assumed 24 was 24 packed bits, it looks like the default is 24 in 32 bits so we may be ok. However I need to re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).

Quoted 6 lines

Quoted 1 line

I'll follow up later,

Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly looked through the TRM doc and got impression that AHUB could re-pack data stream into something that codec supports, but maybe it's a wrong impression.

I chatted with Sameer about this, so yes the AHUB can repack, but there is a problem with S24_LE where if we try to extract 24-bits we actually get the upper 24-bits and not the lower LSBs in the 32-bit data element. So actually we don't support S24_LE.

Ben do you need 24-bit support or 32-bit or both?

08.01.2020 14:37, Jon Hunter пишет:

Quoted 46 lines · Dmitry Osipenko

Quoted 31 lines

06.01.2020 22:00, Ben Dooks пишет:

Quoted 30 lines · Ben Dooks

Quoted 18 lines · Dmitry Osipenko

Quoted 15 lines

05.01.2020 03:04, Ben Dooks пишет:

Quoted 13 lines

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects 24 bit samples in 32 bit words. I can't see any packing options to do 24 bit in 24 bit, so we may have to remove 24 bit sample support (which is a shame)

My preference is to remove the 24-bit support and keep the 32 bit in.

Quoted 6 lines

Quoted 2 lines

Interesting.. Jon, could you please confirm that 24bit format isn't usable on T30?

If there is an option of 24 packed into 32, then I think that would work.

I can try testing that with raw data on Monday.

I need to check some things, I assumed 24 was 24 packed bits, it looks like the default is 24 in 32 bits so we may be ok. However I need to re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).

Quoted 6 lines

Quoted 1 line

I'll follow up later,

Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly looked through the TRM doc and got impression that AHUB could re-pack data stream into something that codec supports, but maybe it's a wrong impression.

I chatted with Sameer about this, so yes the AHUB can repack, but there is a problem with S24_LE where if we try to extract 24-bits we actually get the upper 24-bits and not the lower LSBs in the 32-bit data element. So actually we don't support S24_LE.

Ben do you need 24-bit support or 32-bit or both?

06.01.2020 22:00, Ben Dooks пишет:

Quoted 30 lines · Ben Dooks

Quoted 18 lines · Dmitry Osipenko

Quoted 15 lines

05.01.2020 03:04, Ben Dooks пишет:

Quoted 13 lines

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects 24 bit samples in 32 bit words. I can't see any packing options to do 24 bit in 24 bit, so we may have to remove 24 bit sample support (which is a shame)

My preference is to remove the 24-bit support and keep the 32 bit in.

Quoted 6 lines

Quoted 2 lines

Interesting.. Jon, could you please confirm that 24bit format isn't usable on T30?

If there is an option of 24 packed into 32, then I think that would work.

I can try testing that with raw data on Monday.

I need to check some things, I assumed 24 was 24 packed bits, it looks like the default is 24 in 32 bits so we may be ok. However I need to re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).

Quoted 1 line

I'll follow up later,

Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly looked through the TRM doc and got impression that AHUB could re-pack data stream into something that codec supports, but maybe it's a wrong impression.
_________________________________

Quoted 31 lines · Dmitry Osipenko

06.01.2020 22:00, Ben Dooks пишет:

Quoted 30 lines · Ben Dooks

Quoted 18 lines · Dmitry Osipenko

Quoted 15 lines

05.01.2020 03:04, Ben Dooks пишет:

Quoted 13 lines

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects 24 bit samples in 32 bit words. I can't see any packing options to do 24 bit in 24 bit, so we may have to remove 24 bit sample support (which is a shame)

My preference is to remove the 24-bit support and keep the 32 bit in.

Quoted 6 lines

Quoted 2 lines

Interesting.. Jon, could you please confirm that 24bit format isn't usable on T30?

If there is an option of 24 packed into 32, then I think that would work.

I can try testing that with raw data on Monday.

I need to check some things, I assumed 24 was 24 packed bits, it looks like the default is 24 in 32 bits so we may be ok. However I need to re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).

Quoted 7 lines

Quoted 1 line

I'll follow up later,

Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly looked through the TRM doc and got impression that AHUB could re-pack data stream into something that codec supports, but maybe it's a wrong impression.
_________________________________

I did a quick test with the following:

 sox -n -b 16 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 24 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 32 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5

The 16 and 32 work fine, the 24 is showing a playback output freq of 440Hz instead of 500Hz... this suggests the clock is off, or there is something else weird going on...

Quoted 44 lines · Dmitry Osipenko

Quoted 33 lines

06.01.2020 22:00, Ben Dooks пишет:

Quoted 32 lines · Ben Dooks

Quoted 19 lines · Dmitry Osipenko

Quoted 16 lines

05.01.2020 03:04, Ben Dooks пишет:

Quoted 14 lines

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects 24 bit samples in 32 bit words. I can't see any packing options to do 24 bit in 24 bit, so we may have to remove 24 bit sample support (which is a shame)

My preference is to remove the 24-bit support and keep the 32 bit in.

Quoted 7 lines

Quoted 2 lines

Interesting.. Jon, could you please confirm that 24bit format isn't usable on T30?

If there is an option of 24 packed into 32, then I think that would work.

I can try testing that with raw data on Monday.

I need to check some things, I assumed 24 was 24 packed bits, it looks like the default is 24 in 32 bits so we may be ok. However I need to re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).

Quoted 7 lines

Quoted 1 line

I'll follow up later,

Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly looked through the TRM doc and got impression that AHUB could re-pack data stream into something that codec supports, but maybe it's a wrong impression.
_________________________________

I did a quick test with the following:

Quoted 8 lines

  sox -n -b 16 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 24 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 32 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5

The 16 and 32 work fine, the 24 is showing a playback output freq of 440Hz instead of 500Hz... this suggests the clock is off, or there is something else weird going on...

I should have checked pll_a_out0 rate, for 24bit 2ch, I get pll_a_out at which makes:

11289600/(24*2*44100) = 5.3333333333

For some reason the PLL can't get a decent divisor for this.

24.01.2020 00:59, Ben Dooks пишет:

Quoted 54 lines · Ben Dooks

Quoted 44 lines · Dmitry Osipenko

Quoted 33 lines

06.01.2020 22:00, Ben Dooks пишет:

Quoted 32 lines · Ben Dooks

Quoted 19 lines · Dmitry Osipenko

Quoted 16 lines

05.01.2020 03:04, Ben Dooks пишет:

Quoted 14 lines

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects 24 bit samples in 32 bit words. I can't see any packing options to do 24 bit in 24 bit, so we may have to remove 24 bit sample support (which is a shame)

My preference is to remove the 24-bit support and keep the 32 bit in.

Quoted 7 lines

Quoted 2 lines

Interesting.. Jon, could you please confirm that 24bit format isn't usable on T30?

If there is an option of 24 packed into 32, then I think that would work.

I can try testing that with raw data on Monday.

I need to check some things, I assumed 24 was 24 packed bits, it looks like the default is 24 in 32 bits so we may be ok. However I need to re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).

Quoted 7 lines

Quoted 1 line

I'll follow up later,

Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly looked through the TRM doc and got impression that AHUB could re-pack data stream into something that codec supports, but maybe it's a wrong impression.
_________________________________

I did a quick test with the following:

Quoted 8 lines

  sox -n -b 16 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 24 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 32 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5

The 16 and 32 work fine, the 24 is showing a playback output freq of 440Hz instead of 500Hz... this suggests the clock is off, or there is something else weird going on...

Quoted 6 lines

I should have checked pll_a_out0 rate, for 24bit 2ch, I get pll_a_out at which makes:

11289600/(24*2*44100) = 5.3333333333

For some reason the PLL can't get a decent divisor for this.

Have you tried to adjust the predefined PLLA rate? Please see tegra_clk_init_table in drivers/clk/tegra/clk-tegra30.c. Will be interesting if it works with that.

Sowjanya said that the PLLA rate setup is going to be moved to the audio driver [1], maybe that's what we already need for 24bit.

[1] https://lkml.org/lkml/2020/1/21/744

Quoted 44 lines · Ben Dooks

Quoted 33 lines · Dmitry Osipenko

06.01.2020 22:00, Ben Dooks пишет:

Quoted 32 lines · Ben Dooks

Quoted 19 lines · Dmitry Osipenko

Quoted 16 lines

05.01.2020 03:04, Ben Dooks пишет:

Quoted 14 lines

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects 24 bit samples in 32 bit words. I can't see any packing options to do 24 bit in 24 bit, so we may have to remove 24 bit sample support (which is a shame)

My preference is to remove the 24-bit support and keep the 32 bit in.

Quoted 7 lines

Quoted 2 lines

Interesting.. Jon, could you please confirm that 24bit format isn't usable on T30?

If there is an option of 24 packed into 32, then I think that would work.

I can try testing that with raw data on Monday.

I need to check some things, I assumed 24 was 24 packed bits, it looks like the default is 24 in 32 bits so we may be ok. However I need to re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).

Quoted 7 lines

Quoted 1 line

I'll follow up later,

Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly looked through the TRM doc and got impression that AHUB could re-pack data stream into something that codec supports, but maybe it's a wrong impression.
_________________________________

I did a quick test with the following:

Quoted 8 lines

  sox -n -b 16 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 24 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 32 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5

The 16 and 32 work fine, the 24 is showing a playback output freq of 440Hz instead of 500Hz... this suggests the clock is off, or there is something else weird going on...

I should have checked pll_a_out0 rate, for 24bit 2ch, I get pll_a_out at which makes:

11289600/(24*2*44100) = 5.3333333333

For some reason the PLL can't get a decent divisor for this.

Yes that is going to be a problem. I assume that your codec wants a 256*fs MCLK? Maybe in that case you are better off having the codec drive the bit clock and fsync?

Is 24-bit critical to what you are doing?

Otherwise maybe we should drop the 24-bit support for now and just keep 32-bit.

Quoted 3 lines · Jon Hunter

Yes that is going to be a problem. I assume that your codec wants a 256*fs MCLK? Maybe in that case you are better off having the codec drive the bit clock and fsync?

Quoted 4 lines

Is 24-bit critical to what you are doing?

Otherwise maybe we should drop the 24-bit support for now and just keep 32-bit.

Removing the support because one particular board has limited clocks isn't good - it'd be better to have components with clocking restrictions impose constraints as needed.

Quoted 32 lines · Dmitry Osipenko

06.01.2020 22:00, Ben Dooks пишет:

Quoted 31 lines · Ben Dooks

Quoted 18 lines · Dmitry Osipenko

Quoted 15 lines

05.01.2020 03:04, Ben Dooks пишет:

Quoted 13 lines

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects 24 bit samples in 32 bit words. I can't see any packing options to do 24 bit in 24 bit, so we may have to remove 24 bit sample support (which is a shame)

My preference is to remove the 24-bit support and keep the 32 bit in.

Quoted 7 lines

Quoted 2 lines

Interesting.. Jon, could you please confirm that 24bit format isn't usable on T30?

If there is an option of 24 packed into 32, then I think that would work.

I can try testing that with raw data on Monday.

I need to check some things, I assumed 24 was 24 packed bits, it looks like the default is 24 in 32 bits so we may be ok. However I need to re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).

Quoted 7 lines

Quoted 1 line

I'll follow up later,

Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly looked through the TRM doc and got impression that AHUB could re-pack data stream into something that codec supports, but maybe it's a wrong impression.
_________________________________

I did a quick test with the following:

 sox -n -b 16 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 24 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 32 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5

The 16 and 32 work fine, the 24 is showing a playback output freq of 440Hz instead of 500Hz... this suggests the clock is off, or there is something else weird going on...

Quoted 43 lines · Ben Dooks

Quoted 32 lines · Dmitry Osipenko

06.01.2020 22:00, Ben Dooks пишет:

Quoted 31 lines · Ben Dooks

Quoted 18 lines · Dmitry Osipenko

Quoted 15 lines

05.01.2020 03:04, Ben Dooks пишет:

Quoted 13 lines

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects 24 bit samples in 32 bit words. I can't see any packing options to do 24 bit in 24 bit, so we may have to remove 24 bit sample support (which is a shame)

My preference is to remove the 24-bit support and keep the 32 bit in.

Quoted 7 lines

Quoted 2 lines

Interesting.. Jon, could you please confirm that 24bit format isn't usable on T30?

If there is an option of 24 packed into 32, then I think that would work.

I can try testing that with raw data on Monday.

I need to check some things, I assumed 24 was 24 packed bits, it looks like the default is 24 in 32 bits so we may be ok. However I need to re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).

Quoted 7 lines

Quoted 1 line

I'll follow up later,

Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly looked through the TRM doc and got impression that AHUB could re-pack data stream into something that codec supports, but maybe it's a wrong impression.
_________________________________

I did a quick test with the following:

Quoted 7 lines

 sox -n -b 16 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 24 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 32 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5

The 16 and 32 work fine, the 24 is showing a playback output freq of 440Hz instead of 500Hz... this suggests the clock is off, or there is something else weird going on...

I was looking at using sox to create such as file, but the above command generates a S24_3LE file and not S24_LE file. The codec on Jetson-TK1 supports S24_LE but does not support S24_3LE and so I cannot test this. Anyway, we really need to test S24_LE and not S24_3LE because this is the problem that Dmitry is having.

Ben is S24_3LE what you really need to support?

So I assume aplay has turned the S24_3LE -> S24_LE

Quoted 43 lines · Ben Dooks

Quoted 32 lines · Dmitry Osipenko

06.01.2020 22:00, Ben Dooks пишет:

Quoted 31 lines · Ben Dooks

Quoted 18 lines · Dmitry Osipenko

Quoted 15 lines

05.01.2020 03:04, Ben Dooks пишет:

Quoted 13 lines

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects 24 bit samples in 32 bit words. I can't see any packing options to do 24 bit in 24 bit, so we may have to remove 24 bit sample support (which is a shame)

My preference is to remove the 24-bit support and keep the 32 bit in.

Quoted 7 lines

Quoted 2 lines

Interesting.. Jon, could you please confirm that 24bit format isn't usable on T30?

If there is an option of 24 packed into 32, then I think that would work.

I can try testing that with raw data on Monday.

I need to check some things, I assumed 24 was 24 packed bits, it looks like the default is 24 in 32 bits so we may be ok. However I need to re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).

Quoted 7 lines

Quoted 1 line

I'll follow up later,

Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly looked through the TRM doc and got impression that AHUB could re-pack data stream into something that codec supports, but maybe it's a wrong impression.
_________________________________

I did a quick test with the following:

Quoted 7 lines

 sox -n -b 16 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 24 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 32 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5

The 16 and 32 work fine, the 24 is showing a playback output freq of 440Hz instead of 500Hz... this suggests the clock is off, or there is something else weird going on...

I was looking at using sox to create such as file, but the above command generates a S24_3LE file and not S24_LE file. The codec on Jetson-TK1 supports S24_LE but does not support S24_3LE and so I cannot test this. Anyway, we really need to test S24_LE and not S24_3LE because this is the problem that Dmitry is having.

Ben is S24_3LE what you really need to support?

Quoted 43 lines · Dmitry Osipenko

Quoted 32 lines

06.01.2020 22:00, Ben Dooks пишет:

Quoted 31 lines · Ben Dooks

Quoted 18 lines · Dmitry Osipenko

Quoted 15 lines

05.01.2020 03:04, Ben Dooks пишет:

Quoted 13 lines

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects 24 bit samples in 32 bit words. I can't see any packing options to do 24 bit in 24 bit, so we may have to remove 24 bit sample support (which is a shame)

My preference is to remove the 24-bit support and keep the 32 bit in.

Quoted 7 lines

Quoted 2 lines

Interesting.. Jon, could you please confirm that 24bit format isn't usable on T30?

If there is an option of 24 packed into 32, then I think that would work.

I can try testing that with raw data on Monday.

I need to check some things, I assumed 24 was 24 packed bits, it looks like the default is 24 in 32 bits so we may be ok. However I need to re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).

Quoted 7 lines

Quoted 1 line

I'll follow up later,

Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly looked through the TRM doc and got impression that AHUB could re-pack data stream into something that codec supports, but maybe it's a wrong impression.
_________________________________

I did a quick test with the following:

Quoted 7 lines

 sox -n -b 16 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 24 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 32 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5

The 16 and 32 work fine, the 24 is showing a playback output freq of 440Hz instead of 500Hz... this suggests the clock is off, or there is something else weird going on...

I was looking at using sox to create such as file, but the above command generates a S24_3LE file and not S24_LE file. The codec on Jetson-TK1 supports S24_LE but does not support S24_3LE and so I cannot test this. Anyway, we really need to test S24_LE and not S24_3LE because this is the problem that Dmitry is having.

Ben is S24_3LE what you really need to support?

Dmitry, does the following fix your problem?

sound/soc/tegra/tegra30_i2s.c +1 -1

index dbed3c5408e7..92845c4b63f4 100644
@@ -140,7 +140,7 @@ static int tegra30_i2s_hw_params(struct
 snd_pcm_substream *substream,
                audio_bits = TEGRA30_AUDIOCIF_BITS_16;
                sample_size = 16;
                break;
-       case SNDRV_PCM_FORMAT_S24_LE:
+       case SNDRV_PCM_FORMAT_S24_3LE:
                val = TEGRA30_I2S_CTRL_BIT_SIZE_24;
                audio_bits = TEGRA30_AUDIOCIF_BITS_24;

                sample_size = 24;

sound/soc/tegra/tegra30_i2s.c +1 -1

@@ -318,7 +318,7 @@ static const struct snd_soc_dai_driver
 tegra30_i2s_dai_template = {
                .channels_max = 2,
                .rates = SNDRV_PCM_RATE_8000_96000,
                .formats = SNDRV_PCM_FMTBIT_S32_LE |
-                          SNDRV_PCM_FMTBIT_S24_LE |
+                          SNDRV_PCM_FMTBIT_S24_3LE |
                           SNDRV_PCM_FMTBIT_S16_LE,
        },

        .capture = {

sound/soc/tegra/tegra30_i2s.c +1 -1

@@ -327,7 +327,7 @@ static const struct snd_soc_dai_driver
 tegra30_i2s_dai_template = {
                .channels_max = 2,
                .rates = SNDRV_PCM_RATE_8000_96000,
                .formats = SNDRV_PCM_FMTBIT_S32_LE |
-                          SNDRV_PCM_FMTBIT_S24_LE |
+                          SNDRV_PCM_FMTBIT_S24_3LE |
                           SNDRV_PCM_FMTBIT_S16_LE,
        },

        .ops = &tegra30_i2s_dai_ops,

Jon

24.01.2020 19:50, Jon Hunter пишет:

Quoted 98 lines · Ben Dooks

Quoted 43 lines · Dmitry Osipenko

Quoted 32 lines

06.01.2020 22:00, Ben Dooks пишет:

Quoted 31 lines · Ben Dooks

Quoted 18 lines · Dmitry Osipenko

Quoted 15 lines

05.01.2020 03:04, Ben Dooks пишет:

Quoted 13 lines

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects 24 bit samples in 32 bit words. I can't see any packing options to do 24 bit in 24 bit, so we may have to remove 24 bit sample support (which is a shame)

My preference is to remove the 24-bit support and keep the 32 bit in.

Quoted 7 lines

Quoted 2 lines

Interesting.. Jon, could you please confirm that 24bit format isn't usable on T30?

If there is an option of 24 packed into 32, then I think that would work.

I can try testing that with raw data on Monday.

I need to check some things, I assumed 24 was 24 packed bits, it looks like the default is 24 in 32 bits so we may be ok. However I need to re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).

Quoted 7 lines

Quoted 1 line

I'll follow up later,

Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly looked through the TRM doc and got impression that AHUB could re-pack data stream into something that codec supports, but maybe it's a wrong impression.
_________________________________

I did a quick test with the following:

Quoted 7 lines

 sox -n -b 16 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 24 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 32 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5

The 16 and 32 work fine, the 24 is showing a playback output freq of 440Hz instead of 500Hz... this suggests the clock is off, or there is something else weird going on...

I was looking at using sox to create such as file, but the above command generates a S24_3LE file and not S24_LE file. The codec on Jetson-TK1 supports S24_LE but does not support S24_3LE and so I cannot test this. Anyway, we really need to test S24_LE and not S24_3LE because this is the problem that Dmitry is having.

Ben is S24_3LE what you really need to support?

Dmitry, does the following fix your problem?

sound/soc/tegra/tegra30_i2s.c +1 -1

index dbed3c5408e7..92845c4b63f4 100644
@@ -140,7 +140,7 @@ static int tegra30_i2s_hw_params(struct
 snd_pcm_substream *substream,
                audio_bits = TEGRA30_AUDIOCIF_BITS_16;
                sample_size = 16;
                break;
-       case SNDRV_PCM_FORMAT_S24_LE:
+       case SNDRV_PCM_FORMAT_S24_3LE:
                val = TEGRA30_I2S_CTRL_BIT_SIZE_24;
                audio_bits = TEGRA30_AUDIOCIF_BITS_24;

                sample_size = 24;

sound/soc/tegra/tegra30_i2s.c +1 -1

@@ -318,7 +318,7 @@ static const struct snd_soc_dai_driver
 tegra30_i2s_dai_template = {
                .channels_max = 2,
                .rates = SNDRV_PCM_RATE_8000_96000,
                .formats = SNDRV_PCM_FMTBIT_S32_LE |
-                          SNDRV_PCM_FMTBIT_S24_LE |
+                          SNDRV_PCM_FMTBIT_S24_3LE |
                           SNDRV_PCM_FMTBIT_S16_LE,
        },

        .capture = {

sound/soc/tegra/tegra30_i2s.c +1 -1

@@ -327,7 +327,7 @@ static const struct snd_soc_dai_driver
 tegra30_i2s_dai_template = {
                .channels_max = 2,
                .rates = SNDRV_PCM_RATE_8000_96000,
                .formats = SNDRV_PCM_FMTBIT_S32_LE |
-                          SNDRV_PCM_FMTBIT_S24_LE |
+                          SNDRV_PCM_FMTBIT_S24_3LE |
                           SNDRV_PCM_FMTBIT_S16_LE,
        },

        .ops = &tegra30_i2s_dai_ops,

Quoted 2 lines

Jon

It should solve the problem in my particular case, but I'm not sure that the solution is correct.

The v5.5 kernel is released now with the broken audio and apparently getting 24bit to work won't be trivial (if possible at all). Ben, could you please send a patch to fix v5.5 by removing the S24 support advertisement from the driver?

27.01.2020 22:20, Dmitry Osipenko пишет:

Quoted 105 lines

24.01.2020 19:50, Jon Hunter пишет:

Quoted 98 lines · Ben Dooks

Quoted 43 lines · Dmitry Osipenko

Quoted 32 lines

06.01.2020 22:00, Ben Dooks пишет:

Quoted 31 lines · Ben Dooks

Quoted 18 lines · Dmitry Osipenko

Quoted 15 lines

05.01.2020 03:04, Ben Dooks пишет:

Quoted 13 lines

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects 24 bit samples in 32 bit words. I can't see any packing options to do 24 bit in 24 bit, so we may have to remove 24 bit sample support (which is a shame)

My preference is to remove the 24-bit support and keep the 32 bit in.

Quoted 7 lines

Quoted 2 lines

Interesting.. Jon, could you please confirm that 24bit format isn't usable on T30?

If there is an option of 24 packed into 32, then I think that would work.

I can try testing that with raw data on Monday.

I need to check some things, I assumed 24 was 24 packed bits, it looks like the default is 24 in 32 bits so we may be ok. However I need to re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).

Quoted 7 lines

Quoted 1 line

I'll follow up later,

Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly looked through the TRM doc and got impression that AHUB could re-pack data stream into something that codec supports, but maybe it's a wrong impression.
_________________________________

I did a quick test with the following:

Quoted 7 lines

 sox -n -b 16 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 24 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 32 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5

The 16 and 32 work fine, the 24 is showing a playback output freq of 440Hz instead of 500Hz... this suggests the clock is off, or there is something else weird going on...

I was looking at using sox to create such as file, but the above command generates a S24_3LE file and not S24_LE file. The codec on Jetson-TK1 supports S24_LE but does not support S24_3LE and so I cannot test this. Anyway, we really need to test S24_LE and not S24_3LE because this is the problem that Dmitry is having.

Ben is S24_3LE what you really need to support?

Dmitry, does the following fix your problem?

sound/soc/tegra/tegra30_i2s.c +1 -1

index dbed3c5408e7..92845c4b63f4 100644
@@ -140,7 +140,7 @@ static int tegra30_i2s_hw_params(struct
 snd_pcm_substream *substream,
                 audio_bits = TEGRA30_AUDIOCIF_BITS_16;
                 sample_size = 16;
                 break;
-       case SNDRV_PCM_FORMAT_S24_LE:
+       case SNDRV_PCM_FORMAT_S24_3LE:
                 val = TEGRA30_I2S_CTRL_BIT_SIZE_24;
                 audio_bits = TEGRA30_AUDIOCIF_BITS_24;

                 sample_size = 24;

sound/soc/tegra/tegra30_i2s.c +1 -1

@@ -318,7 +318,7 @@ static const struct snd_soc_dai_driver
 tegra30_i2s_dai_template = {
                 .channels_max = 2,
                 .rates = SNDRV_PCM_RATE_8000_96000,
                 .formats = SNDRV_PCM_FMTBIT_S32_LE |
-                          SNDRV_PCM_FMTBIT_S24_LE |
+                          SNDRV_PCM_FMTBIT_S24_3LE |
                            SNDRV_PCM_FMTBIT_S16_LE,
         },

         .capture = {

sound/soc/tegra/tegra30_i2s.c +1 -1

@@ -327,7 +327,7 @@ static const struct snd_soc_dai_driver
 tegra30_i2s_dai_template = {
                 .channels_max = 2,
                 .rates = SNDRV_PCM_RATE_8000_96000,
                 .formats = SNDRV_PCM_FMTBIT_S32_LE |
-                          SNDRV_PCM_FMTBIT_S24_LE |
+                          SNDRV_PCM_FMTBIT_S24_3LE |
                            SNDRV_PCM_FMTBIT_S16_LE,
         },

         .ops = &tegra30_i2s_dai_ops,

Quoted 2 lines

Jon

It should solve the problem in my particular case, but I'm not sure that the solution is correct.

Quoted 4 lines

The v5.5 kernel is released now with the broken audio and apparently getting 24bit to work won't be trivial (if possible at all). Ben, could you please send a patch to fix v5.5 by removing the S24 support advertisement from the driver?

I also suspect that s32 may need some extra patches and thus could be worthwhile to stop advertising it as well.

Quoted 122 lines · Dmitry Osipenko

27.01.2020 22:20, Dmitry Osipenko пишет:

Quoted 120 lines

24.01.2020 19:50, Jon Hunter пишет:

Quoted 109 lines · Ben Dooks

Quoted 52 lines · Dmitry Osipenko

Quoted 41 lines

06.01.2020 22:00, Ben Dooks пишет:

Quoted 32 lines · Ben Dooks

Quoted 22 lines · Dmitry Osipenko

Quoted 19 lines

05.01.2020 03:04, Ben Dooks пишет:

Quoted 17 lines

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects
24 bit samples in 32 bit words. I can't see any packing options to
do 24 bit in 24 bit, so we may have to remove 24 bit sample support
(which is a shame)

My preference is to remove the 24-bit support and keep the 32 bit in.

Quoted 8 lines

Quoted 3 lines

Interesting.. Jon, could you please confirm that 24bit format isn't
usable on T30?

If there is an option of 24 packed into 32, then I think that would work.

I can try testing that with raw data on Monday.

Quoted 7 lines

I need to check some things, I assumed 24 was 24 packed bits, it looks
like the default is 24 in 32 bits so we may be ok. However I need to re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).

I'll follow up later,

Quoted 7 lines

Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly
looked through the TRM doc and got impression that AHUB could re-pack data stream into something that codec supports, but maybe it's a wrong
impression.
_________________________________

I did a quick test with the following:

Quoted 7 lines

  sox -n -b 16 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 24 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 32 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5

The 16 and 32 work fine, the 24 is showing a playback output freq of 440Hz instead of 500Hz... this suggests the clock is off, or there is something else weird going on...

I was looking at using sox to create such as file, but the above command
generates a S24_3LE file and not S24_LE file. The codec on Jetson-TK1 supports S24_LE but does not support S24_3LE and so I cannot test this. Anyway, we really need to test S24_LE and not S24_3LE because this is the problem that Dmitry is having.

Ben is S24_3LE what you really need to support?

Dmitry, does the following fix your problem?

sound/soc/tegra/tegra30_i2s.c +1 -1

index dbed3c5408e7..92845c4b63f4 100644
@@ -140,7 +140,7 @@ static int tegra30_i2s_hw_params(struct
 snd_pcm_substream *substream,
                  audio_bits = TEGRA30_AUDIOCIF_BITS_16;
                  sample_size = 16;
                  break;
-       case SNDRV_PCM_FORMAT_S24_LE:
+       case SNDRV_PCM_FORMAT_S24_3LE:
                  val = TEGRA30_I2S_CTRL_BIT_SIZE_24;
                  audio_bits = TEGRA30_AUDIOCIF_BITS_24;

                 sample_size = 24;

sound/soc/tegra/tegra30_i2s.c +0 -0

@@ -318,7 +318,7 @@ static const struct snd_soc_dai_driver
 tegra30_i2s_dai_template = {
                  .channels_max = 2,
                  .rates = SNDRV_PCM_RATE_8000_96000,

                 .formats = SNDRV_PCM_FMTBIT_S32_LE | -                          SNDRV_PCM_FMTBIT_S24_LE | +                          SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S16_LE, },
         .capture = {

sound/soc/tegra/tegra30_i2s.c +0 -0

@@ -327,7 +327,7 @@ static const struct snd_soc_dai_driver
 tegra30_i2s_dai_template = {
                  .channels_max = 2,
                  .rates = SNDRV_PCM_RATE_8000_96000,

                 .formats = SNDRV_PCM_FMTBIT_S32_LE | -                          SNDRV_PCM_FMTBIT_S24_LE | +                          SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S16_LE, },
         .ops = &tegra30_i2s_dai_ops,

Quoted 2 lines

Jon

It should solve the problem in my particular case, but I'm not sure that the solution is correct.

The v5.5 kernel is released now with the broken audio and apparently getting 24bit to work won't be trivial (if possible at all). Ben, could you please send a patch to fix v5.5 by removing the S24 support advertisement from the driver?

Quoted 2 lines

I also suspect that s32 may need some extra patches and thus could be worthwhile to stop advertising it as well.

As far as I am aware that works and we can hit the audio rates for it.

Quoted 123 lines · Dmitry Osipenko

Quoted 122 lines

27.01.2020 22:20, Dmitry Osipenko пишет:

Quoted 120 lines

24.01.2020 19:50, Jon Hunter пишет:

Quoted 109 lines · Ben Dooks

Quoted 52 lines · Dmitry Osipenko

Quoted 41 lines

06.01.2020 22:00, Ben Dooks пишет:

Quoted 32 lines · Ben Dooks

Quoted 22 lines · Dmitry Osipenko

Quoted 19 lines

05.01.2020 03:04, Ben Dooks пишет:

Quoted 17 lines

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects
24 bit samples in 32 bit words. I can't see any packing options to
do 24 bit in 24 bit, so we may have to remove 24 bit sample support
(which is a shame)

My preference is to remove the 24-bit support and keep the 32 bit in.

Quoted 8 lines

Quoted 3 lines

Interesting.. Jon, could you please confirm that 24bit format isn't
usable on T30?

If there is an option of 24 packed into 32, then I think that would work.

I can try testing that with raw data on Monday.

Quoted 7 lines

I need to check some things, I assumed 24 was 24 packed bits, it looks
like the default is 24 in 32 bits so we may be ok. However I need to re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).

I'll follow up later,

Quoted 7 lines

Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly
looked through the TRM doc and got impression that AHUB could re-pack data stream into something that codec supports, but maybe it's a wrong
impression.
_________________________________

I did a quick test with the following:

Quoted 7 lines

  sox -n -b 16 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 24 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 32 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5

The 16 and 32 work fine, the 24 is showing a playback output freq of 440Hz instead of 500Hz... this suggests the clock is off, or there is something else weird going on...

I was looking at using sox to create such as file, but the above command
generates a S24_3LE file and not S24_LE file. The codec on Jetson-TK1 supports S24_LE but does not support S24_3LE and so I cannot test this. Anyway, we really need to test S24_LE and not S24_3LE because this is the problem that Dmitry is having.

Ben is S24_3LE what you really need to support?

Dmitry, does the following fix your problem?

sound/soc/tegra/tegra30_i2s.c +1 -1

index dbed3c5408e7..92845c4b63f4 100644
@@ -140,7 +140,7 @@ static int tegra30_i2s_hw_params(struct
 snd_pcm_substream *substream,
                  audio_bits = TEGRA30_AUDIOCIF_BITS_16;
                  sample_size = 16;
                  break;
-       case SNDRV_PCM_FORMAT_S24_LE:
+       case SNDRV_PCM_FORMAT_S24_3LE:
                  val = TEGRA30_I2S_CTRL_BIT_SIZE_24;
                  audio_bits = TEGRA30_AUDIOCIF_BITS_24;

                 sample_size = 24;

sound/soc/tegra/tegra30_i2s.c +0 -0

@@ -318,7 +318,7 @@ static const struct snd_soc_dai_driver
 tegra30_i2s_dai_template = {
                  .channels_max = 2,
                  .rates = SNDRV_PCM_RATE_8000_96000,

                 .formats = SNDRV_PCM_FMTBIT_S32_LE | -                          SNDRV_PCM_FMTBIT_S24_LE | +                          SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S16_LE, },
         .capture = {

sound/soc/tegra/tegra30_i2s.c +0 -0

@@ -327,7 +327,7 @@ static const struct snd_soc_dai_driver
 tegra30_i2s_dai_template = {
                  .channels_max = 2,
                  .rates = SNDRV_PCM_RATE_8000_96000,

                 .formats = SNDRV_PCM_FMTBIT_S32_LE | -                          SNDRV_PCM_FMTBIT_S24_LE | +                          SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S16_LE, },
         .ops = &tegra30_i2s_dai_ops,

Quoted 2 lines

Jon

It should solve the problem in my particular case, but I'm not sure that the solution is correct.

The v5.5 kernel is released now with the broken audio and apparently getting 24bit to work won't be trivial (if possible at all). Ben, could you please send a patch to fix v5.5 by removing the S24 support advertisement from the driver?

Quoted 4 lines

Quoted 2 lines

I also suspect that s32 may need some extra patches and thus could be worthwhile to stop advertising it as well.

As far as I am aware that works and we can hit the audio rates for it.

I ran a test on Tegra124 Jetson-TK1 and 24-bit playback seems to work as Ben has indicated. So I don't think it is broken.

Can you try Ben's testcase on Tegra30 (ie. generate a tone using sox and use aplay to play)?

Quoted 8 lines · Ben Dooks

Quoted 7 lines · Dmitry Osipenko

27.01.2020 22:20, Dmitry Osipenko пишет:

Quoted 6 lines

24.01.2020 19:50, Jon Hunter пишет:

Quoted 5 lines · Ben Dooks

Quoted 3 lines · Dmitry Osipenko

Quoted 2 lines

06.01.2020 22:00, Ben Dooks пишет:

Quoted 1 line · Ben Dooks

Quoted 4 lines

Quoted 2 lines

I also suspect that s32 may need some extra patches and thus could be worthwhile to stop advertising it as well.

As far as I am aware that works and we can hit the audio rates for it.

I ran a test on Tegra124 Jetson-TK1 and 24-bit playback seems to work as Ben has indicated. So I don't think it is broken.

Can you try Ben's testcase on Tegra30 (ie. generate a tone using sox and use aplay to play)?

Quoted 10 lines · Ben Dooks

Quoted 6 lines · Dmitry Osipenko

Quoted 3 lines

27.01.2020 22:20, Dmitry Osipenko пишет:
I also suspect that s32 may need some extra patches and thus could be worthwhile to stop advertising it as well.

As far as I am aware that works and we can hit the audio rates for it.

I ran a test on Tegra124 Jetson-TK1 and 24-bit playback seems to work as Ben has indicated. So I don't think it is broken.

Quoted 2 lines

Can you try Ben's testcase on Tegra30 (ie. generate a tone using sox and use aplay to play)?

Another test application that's quite useful for this sort of stuff is speaker-test, it generates audio data directly in arbatrary formats and it's part of alsa-utils so if you've got aplay and friends you may already have it already installed.

I ran a test on Tegra124 Jetson-TK1 and 24-bit playback seems to work as Ben has indicated. So I don't think it is broken.

On 28/01/2020 13:19, Jon Hunter wrote:

...

Quoted 2 lines

I ran a test on Tegra124 Jetson-TK1 and 24-bit playback seems to work as Ben has indicated. So I don't think it is broken.

Actually, it does not work on TK1. Pulseaudio was converting from S24_3LE to S16_LE. If I use plughw to do the conversion it sounds distorted indeed.

Ben what audio codec are you testing with?

24.01.2020 19:50, Jon Hunter пишет:

Quoted 98 lines · Ben Dooks

Quoted 43 lines · Dmitry Osipenko

Quoted 32 lines

06.01.2020 22:00, Ben Dooks пишет:

Quoted 31 lines · Ben Dooks

Quoted 18 lines · Dmitry Osipenko

Quoted 15 lines

05.01.2020 03:04, Ben Dooks пишет:

Quoted 13 lines

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects 24 bit samples in 32 bit words. I can't see any packing options to do 24 bit in 24 bit, so we may have to remove 24 bit sample support (which is a shame)

My preference is to remove the 24-bit support and keep the 32 bit in.

Quoted 7 lines

Quoted 2 lines

Interesting.. Jon, could you please confirm that 24bit format isn't usable on T30?

If there is an option of 24 packed into 32, then I think that would work.

I can try testing that with raw data on Monday.

I need to check some things, I assumed 24 was 24 packed bits, it looks like the default is 24 in 32 bits so we may be ok. However I need to re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).

Quoted 7 lines

Quoted 1 line

I'll follow up later,

Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly looked through the TRM doc and got impression that AHUB could re-pack data stream into something that codec supports, but maybe it's a wrong impression.
_________________________________

I did a quick test with the following:

Quoted 7 lines

 sox -n -b 16 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 24 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5 sox -n -b 32 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5

The 16 and 32 work fine, the 24 is showing a playback output freq of 440Hz instead of 500Hz... this suggests the clock is off, or there is something else weird going on...

I was looking at using sox to create such as file, but the above command generates a S24_3LE file and not S24_LE file. The codec on Jetson-TK1 supports S24_LE but does not support S24_3LE and so I cannot test this. Anyway, we really need to test S24_LE and not S24_3LE because this is the problem that Dmitry is having.

Ben is S24_3LE what you really need to support?

Dmitry, does the following fix your problem?

sound/soc/tegra/tegra30_i2s.c +1 -1

index dbed3c5408e7..92845c4b63f4 100644
@@ -140,7 +140,7 @@ static int tegra30_i2s_hw_params(struct
 snd_pcm_substream *substream,
                 audio_bits = TEGRA30_AUDIOCIF_BITS_16;
                 sample_size = 16;
                 break;
-       case SNDRV_PCM_FORMAT_S24_LE:
+       case SNDRV_PCM_FORMAT_S24_3LE:
                 val = TEGRA30_I2S_CTRL_BIT_SIZE_24;
                 audio_bits = TEGRA30_AUDIOCIF_BITS_24;

                 sample_size = 24;

sound/soc/tegra/tegra30_i2s.c +1 -1

@@ -318,7 +318,7 @@ static const struct snd_soc_dai_driver
 tegra30_i2s_dai_template = {
                 .channels_max = 2,
                 .rates = SNDRV_PCM_RATE_8000_96000,
                 .formats = SNDRV_PCM_FMTBIT_S32_LE |
-                          SNDRV_PCM_FMTBIT_S24_LE |
+                          SNDRV_PCM_FMTBIT_S24_3LE |
                            SNDRV_PCM_FMTBIT_S16_LE,
         },

         .capture = {

sound/soc/tegra/tegra30_i2s.c +1 -1

@@ -327,7 +327,7 @@ static const struct snd_soc_dai_driver
 tegra30_i2s_dai_template = {
                 .channels_max = 2,
                 .rates = SNDRV_PCM_RATE_8000_96000,
                 .formats = SNDRV_PCM_FMTBIT_S32_LE |
-                          SNDRV_PCM_FMTBIT_S24_LE |
+                          SNDRV_PCM_FMTBIT_S24_3LE |
                            SNDRV_PCM_FMTBIT_S16_LE,
         },

         .ops = &tegra30_i2s_dai_ops,

Quoted 2 lines

Jon

It should solve the problem in my particular case, but I'm not sure that the solution is correct.

The v5.5 kernel is released now with the broken audio and apparently getting 24bit to work won't be trivial (if possible at all). Ben, could you please send a patch to fix v5.5 by removing the S24 support advertisement from the driver?

24.01.2020 19:50, Jon Hunter пишет:

Review

Quoted 4 lines

.rates = SNDRV_PCM_RATE_8000_96000, .formats = SNDRV_PCM_FMTBIT_S32_LE |

(quoted diff) +1 -1

-                          SNDRV_PCM_FMTBIT_S24_LE |
+                          SNDRV_PCM_FMTBIT_S24_3LE |

It should solve the problem in my particular case, but I'm not sure that the solution is correct.

The v5.5 kernel is released now with the broken audio and apparently getting 24bit to work won't be trivial (if possible at all). Ben, could you please send a patch to fix v5.5 by removing the S24 support advertisement from the driver?

Quoted 6 lines

24.01.2020 19:50, Jon Hunter пишет:

Quoted 4 lines

.rates = SNDRV_PCM_RATE_8000_96000, .formats = SNDRV_PCM_FMTBIT_S32_LE |

(quoted diff) +1 -1

-                          SNDRV_PCM_FMTBIT_S24_LE |
+                          SNDRV_PCM_FMTBIT_S24_3LE |

Quoted 2 lines

It should solve the problem in my particular case, but I'm not sure that the solution is correct.

If the format implemented by the driver is S24_3LE the driver should advertise S24_3LE.

Quoted 4 lines

The v5.5 kernel is released now with the broken audio and apparently getting 24bit to work won't be trivial (if possible at all). Ben, could you please send a patch to fix v5.5 by removing the S24 support advertisement from the driver?

Why is that the best fix rather than just advertising the format implemented by the driver?

I really don't understand why this is all taking so long, this thread just seems to be going round in interminable circles long after it looked like the issue was understood. I have to admit I've not read every single message in the thread but it's difficult to see why it doesn't seem to be making any progress.

28.01.2020 15:13, Mark Brown пишет:

Quoted 8 lines · Dmitry Osipenko

Quoted 6 lines

24.01.2020 19:50, Jon Hunter пишет:

Quoted 4 lines

.rates = SNDRV_PCM_RATE_8000_96000, .formats = SNDRV_PCM_FMTBIT_S32_LE |

(quoted diff) +1 -1

-                          SNDRV_PCM_FMTBIT_S24_LE |
+                          SNDRV_PCM_FMTBIT_S24_3LE |

Quoted 5 lines

Quoted 2 lines

It should solve the problem in my particular case, but I'm not sure that the solution is correct.

If the format implemented by the driver is S24_3LE the driver should advertise S24_3LE.

It should be S24_LE, but seems we still don't know for sure.

Quoted 5 lines

Quoted 4 lines

The v5.5 kernel is released now with the broken audio and apparently getting 24bit to work won't be trivial (if possible at all). Ben, could you please send a patch to fix v5.5 by removing the S24 support advertisement from the driver?

Quoted 2 lines

Why is that the best fix rather than just advertising the format implemented by the driver?

The currently supported format that is known to work well is S16_LE.

I'm suggesting to drop the S24_LE and S32_LE that were added by the applied patches simply because this series wasn't tested properly before it was sent out and turned out that it doesn't work well.

Quoted 12 lines

28.01.2020 15:13, Mark Brown пишет:

Quoted 11 lines · Dmitry Osipenko

Quoted 9 lines

24.01.2020 19:50, Jon Hunter пишет:

Review

Quoted 4 lines

.rates = SNDRV_PCM_RATE_8000_96000, .formats = SNDRV_PCM_FMTBIT_S32_LE |

(quoted diff) +1 -1

-                          SNDRV_PCM_FMTBIT_S24_LE |
+                          SNDRV_PCM_FMTBIT_S24_3LE |

It should solve the problem in my particular case, but I'm not sure that the solution is correct.

Quoted 4 lines

Quoted 2 lines

If the format implemented by the driver is S24_3LE the driver should advertise S24_3LE.

It should be S24_LE, but seems we still don't know for sure.

Why?

28.01.2020 21:19, Jon Hunter пишет:

Quoted 14 lines · Dmitry Osipenko

Quoted 12 lines

28.01.2020 15:13, Mark Brown пишет:

Quoted 11 lines · Dmitry Osipenko

Quoted 9 lines

24.01.2020 19:50, Jon Hunter пишет:

Review

Quoted 4 lines

.rates = SNDRV_PCM_RATE_8000_96000, .formats = SNDRV_PCM_FMTBIT_S32_LE |

(quoted diff) +1 -1

-                          SNDRV_PCM_FMTBIT_S24_LE |
+                          SNDRV_PCM_FMTBIT_S24_3LE |

It should solve the problem in my particular case, but I'm not sure that the solution is correct.

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

If the format implemented by the driver is S24_3LE the driver should advertise S24_3LE.

It should be S24_LE, but seems we still don't know for sure.

Why?

/I think/ sound should be much more distorted if it was S24_3LE, but maybe I'm wrong.

Quoted 17 lines · Dmitry Osipenko

28.01.2020 21:19, Jon Hunter пишет:

Quoted 16 lines · Dmitry Osipenko

Quoted 15 lines

28.01.2020 15:13, Mark Brown пишет:

Quoted 13 lines · Dmitry Osipenko

Quoted 9 lines

24.01.2020 19:50, Jon Hunter пишет:

Review

Quoted 4 lines

.rates = SNDRV_PCM_RATE_8000_96000, .formats = SNDRV_PCM_FMTBIT_S32_LE |

(quoted diff) +1 -1

-                          SNDRV_PCM_FMTBIT_S24_LE |
+                          SNDRV_PCM_FMTBIT_S24_3LE |

It should solve the problem in my particular case, but I'm not sure that the solution is correct.

If the format implemented by the driver is S24_3LE the driver should advertise S24_3LE.

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

It should be S24_LE, but seems we still don't know for sure.

Why?

/I think/ sound should be much more distorted if it was S24_3LE, but maybe I'm wrong.

S24_3LE is IICC the wrong thing and we can't support it on the tegra3

Quoted 18 lines · Ben Dooks

Quoted 17 lines · Dmitry Osipenko

28.01.2020 21:19, Jon Hunter пишет:

Quoted 16 lines · Dmitry Osipenko

Quoted 15 lines

28.01.2020 15:13, Mark Brown пишет:

Quoted 13 lines · Dmitry Osipenko

Quoted 9 lines

24.01.2020 19:50, Jon Hunter пишет:

Review

Quoted 4 lines

.rates = SNDRV_PCM_RATE_8000_96000, .formats = SNDRV_PCM_FMTBIT_S32_LE |

(quoted diff) +1 -1

-                          SNDRV_PCM_FMTBIT_S24_LE |
+                          SNDRV_PCM_FMTBIT_S24_3LE |

It should solve the problem in my particular case, but I'm not sure that the solution is correct.

If the format implemented by the driver is S24_3LE the driver should advertise S24_3LE.

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

It should be S24_LE, but seems we still don't know for sure.

Why?

/I think/ sound should be much more distorted if it was S24_3LE, but maybe I'm wrong.

S24_3LE is IICC the wrong thing and we can't support it on the tegra3

There are three ways of arranging 24-bit samples in memory:

S24_3LE: 24-bit samples in 24-bit words without padding
S24_LE:  24-bit samples in 32-bit words, aligned to the LSB
S32_LE:  24-bit samples in 32-bit words, aligned to the MSB

It is very unlikely that your hardware implements S24_LE.

Quoted 4 lines · Clemens Ladisch

S24_LE:  24-bit samples in 32-bit words, aligned to the LSB
S32_LE:  24-bit samples in 32-bit words, aligned to the MSB

It is very unlikely that your hardware implements S24_LE.

Which is wrong, since this is exactly what the hardware implements.

The DMA fetches 32 bit words, and then the front end dispatches only 24 bits of these to the I2S/TDM

Quoted 6 lines · Ben Dooks

Quoted 4 lines · Clemens Ladisch

S24_LE:  24-bit samples in 32-bit words, aligned to the LSB
S32_LE:  24-bit samples in 32-bit words, aligned to the MSB

It is very unlikely that your hardware implements S24_LE.

Quoted 4 lines

Which is wrong, since this is exactly what the hardware implements.

The DMA fetches 32 bit words, and then the front end dispatches only 24 bits of these to the I2S/TDM

Which 24 bits? Is the padding in the first byte (S32_LE) or in the last byte (S24_LE)?

Quoted 9 lines · Clemens Ladisch

Quoted 8 lines · Ben Dooks

Quoted 4 lines · Clemens Ladisch

S24_LE:  24-bit samples in 32-bit words, aligned to the LSB
S32_LE:  24-bit samples in 32-bit words, aligned to the MSB

It is very unlikely that your hardware implements S24_LE.

Which is wrong, since this is exactly what the hardware implements.