[PATCH 00/13] ALSA: update sample rate definitions

[PATCH 00/13] ALSA: update sample rate definitions

[Jerome Brunet]

This patchset adds rate definitions for 12kHz, 24kHz and 128kHz.

It is follow-up on the series/discussion [0] about adding 128kHz for
spdif/eARC support. The outcome was to add 12kHz and 24kHz as well and
clean up the drivers that no longer require custom rules to allow these
rates.

Identifying these drivers is not straight forward, I tried my best but I
may have missed some. Those will continue to work anyway so it is not
critical. Some drivers using these rates have not been changed on
purpose. The reason for this may be:
* The driver used other uncommon rates that still don't have a definition
  so a custom rule is still required.
* The constraint structure is used in some other way by the driver and
  removing it would not help the readability or maintainability. This is
  the case the freescale asrc drivers for example.

There is one change per driver so, if there is a problem later on, it will
easier to properly add Fixes tag.

The series has been tested with
* ARM64 defconfig - spdif 128kHz at runtime.
* x86_64 allmodconfig - compile test only

Last, the change adding IEC958 definitions has been dropped for this
patchset and will be resent separately

[0]: https://lore.kernel.org/all/20240628122429.2018059-1-jbrunet@baylibre.com/

---
Jerome Brunet (13):
      ALSA: pcm: add more sample rate definitions
      ALSA: cmipci: drop SNDRV_PCM_RATE_KNOT
      ALSA: emu10k1: drop SNDRV_PCM_RATE_KNOT
      ALSA: hdsp: drop SNDRV_PCM_RATE_KNOT
      ALSA: hdspm: drop SNDRV_PCM_RATE_KNOT
      ASoC: cs35l36: drop SNDRV_PCM_RATE_KNOT
      ASoC: cs35l41: drop SNDRV_PCM_RATE_KNOT
      ASoC: cs53l30: drop SNDRV_PCM_RATE_KNOT
      ASoC: Intel: avs: drop SNDRV_PCM_RATE_KNOT
      ASoC: qcom: q6asm-dai: drop SNDRV_PCM_RATE_KNOT
      ASoC: sunxi: sun4i-codec: drop SNDRV_PCM_RATE_KNOT
      ASoC: cs35l34: drop useless rate contraint
      ASoC: spdif: extend supported rates to 768kHz

 include/sound/pcm.h                  | 31 +++++++++++++++++--------------
 sound/core/pcm_native.c              |  6 +++---
 sound/pci/cmipci.c                   | 32 +++++++++-----------------------
 sound/pci/emu10k1/emupcm.c           | 31 +++++--------------------------
 sound/pci/rme9652/hdsp.c             | 18 ++++++------------
 sound/pci/rme9652/hdspm.c            | 16 +---------------
 sound/soc/codecs/cs35l34.c           | 21 ---------------------
 sound/soc/codecs/cs35l36.c           | 34 ++++++++++++----------------------
 sound/soc/codecs/cs35l41.c           | 34 +++++++++++-----------------------
 sound/soc/codecs/cs53l30.c           | 24 +++---------------------
 sound/soc/codecs/spdif_receiver.c    |  3 ++-
 sound/soc/codecs/spdif_transmitter.c |  3 ++-
 sound/soc/intel/avs/pcm.c            | 22 ++++++----------------
 sound/soc/qcom/qdsp6/q6asm-dai.c     | 31 ++++++++++---------------------
 sound/soc/sunxi/sun4i-codec.c        | 28 +++++++++-------------------
 15 files changed, 96 insertions(+), 238 deletions(-)
---
base-commit: 785f4052380684dbcc156203c537c799e2f4be09
change-id: 20240905-alsa-12-24-128-8edab4c08dd4

Best regards,
-- 
Jerome

[PATCH 01/13] ALSA: pcm: add more sample rate definitions

[Jerome Brunet]

This adds a sample rate definition for 12kHz, 24kHz and 128kHz.

Admittedly, just a few drivers are currently using these sample
rates but there is enough of a recurrence to justify adding a definition
for them and remove some custom rate constraint code while at it.

The new definitions are not added to the interval definitions, such as
SNDRV_PCM_RATE_8000_44100, because it would silently add new supported
rates to drivers that may or may not support them. For sure the drivers
have not been tested for these new rates so it is better to leave them out
of interval definitions.

That being said, the added rates are multiples of well know rates families,
it is very likely that a lot of devices out there actually supports them.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 include/sound/pcm.h     | 31 +++++++++++++++++--------------
 sound/core/pcm_native.c |  6 +++---
 2 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 732121b934fd..c993350975a9 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -109,20 +109,23 @@ struct snd_pcm_ops {
 #define SNDRV_PCM_RATE_5512		(1U<<0)		/* 5512Hz */
 #define SNDRV_PCM_RATE_8000		(1U<<1)		/* 8000Hz */
 #define SNDRV_PCM_RATE_11025		(1U<<2)		/* 11025Hz */
-#define SNDRV_PCM_RATE_16000		(1U<<3)		/* 16000Hz */
-#define SNDRV_PCM_RATE_22050		(1U<<4)		/* 22050Hz */
-#define SNDRV_PCM_RATE_32000		(1U<<5)		/* 32000Hz */
-#define SNDRV_PCM_RATE_44100		(1U<<6)		/* 44100Hz */
-#define SNDRV_PCM_RATE_48000		(1U<<7)		/* 48000Hz */
-#define SNDRV_PCM_RATE_64000		(1U<<8)		/* 64000Hz */
-#define SNDRV_PCM_RATE_88200		(1U<<9)		/* 88200Hz */
-#define SNDRV_PCM_RATE_96000		(1U<<10)	/* 96000Hz */
-#define SNDRV_PCM_RATE_176400		(1U<<11)	/* 176400Hz */
-#define SNDRV_PCM_RATE_192000		(1U<<12)	/* 192000Hz */
-#define SNDRV_PCM_RATE_352800		(1U<<13)	/* 352800Hz */
-#define SNDRV_PCM_RATE_384000		(1U<<14)	/* 384000Hz */
-#define SNDRV_PCM_RATE_705600		(1U<<15)	/* 705600Hz */
-#define SNDRV_PCM_RATE_768000		(1U<<16)	/* 768000Hz */
+#define SNDRV_PCM_RATE_12000		(1U<<3)		/* 12000Hz */
+#define SNDRV_PCM_RATE_16000		(1U<<4)		/* 16000Hz */
+#define SNDRV_PCM_RATE_22050		(1U<<5)		/* 22050Hz */
+#define SNDRV_PCM_RATE_24000		(1U<<6)		/* 24000Hz */
+#define SNDRV_PCM_RATE_32000		(1U<<7)		/* 32000Hz */
+#define SNDRV_PCM_RATE_44100		(1U<<8)		/* 44100Hz */
+#define SNDRV_PCM_RATE_48000		(1U<<9)		/* 48000Hz */
+#define SNDRV_PCM_RATE_64000		(1U<<10)	/* 64000Hz */
+#define SNDRV_PCM_RATE_88200		(1U<<11)	/* 88200Hz */
+#define SNDRV_PCM_RATE_96000		(1U<<12)	/* 96000Hz */
+#define SNDRV_PCM_RATE_128000		(1U<<13)	/* 128000Hz */
+#define SNDRV_PCM_RATE_176400		(1U<<14)	/* 176400Hz */
+#define SNDRV_PCM_RATE_192000		(1U<<15)	/* 192000Hz */
+#define SNDRV_PCM_RATE_352800		(1U<<16)	/* 352800Hz */
+#define SNDRV_PCM_RATE_384000		(1U<<17)	/* 384000Hz */
+#define SNDRV_PCM_RATE_705600		(1U<<18)	/* 705600Hz */
+#define SNDRV_PCM_RATE_768000		(1U<<19)	/* 768000Hz */
 
 #define SNDRV_PCM_RATE_CONTINUOUS	(1U<<30)	/* continuous range */
 #define SNDRV_PCM_RATE_KNOT		(1U<<31)	/* supports more non-continuous rates */
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 44381514f695..7461a727615c 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -2418,13 +2418,13 @@ static int snd_pcm_hw_rule_sample_bits(struct snd_pcm_hw_params *params,
 	return snd_interval_refine(hw_param_interval(params, rule->var), &t);
 }
 
-#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_192000 != 1 << 12
+#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_768000 != 1 << 19
 #error "Change this table"
 #endif
 
 static const unsigned int rates[] = {
-	5512, 8000, 11025, 16000, 22050, 32000, 44100,
-	48000, 64000, 88200, 96000, 176400, 192000, 352800, 384000, 705600, 768000
+	5512, 8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000, 64000,
+	88200, 96000, 128000, 176400, 192000, 352800, 384000, 705600, 768000,
 };
 
 const struct snd_pcm_hw_constraint_list snd_pcm_known_rates = {

-- 
2.45.2

[PATCH 02/13] ALSA: cmipci: drop SNDRV_PCM_RATE_KNOT

[Jerome Brunet]

The custom rate constraint list was necessary to support 128kHz.
This rate is now available through SNDRV_PCM_RATE_128000.

Use it and drop the custom rate constraint rule.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 sound/pci/cmipci.c | 32 +++++++++-----------------------
 1 file changed, 9 insertions(+), 23 deletions(-)

diff --git a/sound/pci/cmipci.c b/sound/pci/cmipci.c
index 36014501f7ed..e3cac73517d6 100644
--- a/sound/pci/cmipci.c
+++ b/sound/pci/cmipci.c
@@ -1570,14 +1570,6 @@ static const struct snd_pcm_hardware snd_cmipci_capture_spdif =
 	.fifo_size =		0,
 };
 
-static const unsigned int rate_constraints[] = { 5512, 8000, 11025, 16000, 22050,
-			32000, 44100, 48000, 88200, 96000, 128000 };
-static const struct snd_pcm_hw_constraint_list hw_constraints_rates = {
-		.count = ARRAY_SIZE(rate_constraints),
-		.list = rate_constraints,
-		.mask = 0,
-};
-
 /*
  * check device open/close
  */
@@ -1649,11 +1641,9 @@ static int snd_cmipci_playback_open(struct snd_pcm_substream *substream)
 				     SNDRV_PCM_RATE_96000;
 		runtime->hw.rate_max = 96000;
 	} else if (cm->chip_version == 55) {
-		err = snd_pcm_hw_constraint_list(runtime, 0,
-			SNDRV_PCM_HW_PARAM_RATE, &hw_constraints_rates);
-		if (err < 0)
-			return err;
-		runtime->hw.rates |= SNDRV_PCM_RATE_KNOT;
+		runtime->hw.rates |= SNDRV_PCM_RATE_88200 |
+				     SNDRV_PCM_RATE_96000 |
+				     SNDRV_PCM_RATE_128000;
 		runtime->hw.rate_max = 128000;
 	}
 	snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_BUFFER_SIZE, 0, 0x10000);
@@ -1675,11 +1665,9 @@ static int snd_cmipci_capture_open(struct snd_pcm_substream *substream)
 		runtime->hw.rate_min = 41000;
 		runtime->hw.rates = SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000;
 	} else if (cm->chip_version == 55) {
-		err = snd_pcm_hw_constraint_list(runtime, 0,
-			SNDRV_PCM_HW_PARAM_RATE, &hw_constraints_rates);
-		if (err < 0)
-			return err;
-		runtime->hw.rates |= SNDRV_PCM_RATE_KNOT;
+		runtime->hw.rates |= SNDRV_PCM_RATE_88200 |
+				     SNDRV_PCM_RATE_96000 |
+				     SNDRV_PCM_RATE_128000;
 		runtime->hw.rate_max = 128000;
 	}
 	snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_BUFFER_SIZE, 0, 0x10000);
@@ -1715,11 +1703,9 @@ static int snd_cmipci_playback2_open(struct snd_pcm_substream *substream)
 				     SNDRV_PCM_RATE_96000;
 		runtime->hw.rate_max = 96000;
 	} else if (cm->chip_version == 55) {
-		err = snd_pcm_hw_constraint_list(runtime, 0,
-			SNDRV_PCM_HW_PARAM_RATE, &hw_constraints_rates);
-		if (err < 0)
-			return err;
-		runtime->hw.rates |= SNDRV_PCM_RATE_KNOT;
+		runtime->hw.rates |= SNDRV_PCM_RATE_88200 |
+				     SNDRV_PCM_RATE_96000 |
+				     SNDRV_PCM_RATE_128000;
 		runtime->hw.rate_max = 128000;
 	}
 	snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_BUFFER_SIZE, 0, 0x10000);

-- 
2.45.2

[PATCH 03/13] ALSA: emu10k1: drop SNDRV_PCM_RATE_KNOT

[Jerome Brunet]

The custom rate constraint lists were necessary to support 12kHz and 24kHz.
These rates are now available through SNDRV_PCM_RATE_12000 and
SNDRV_PCM_RATE_24000.

Use them and drop the custom rate constraint rules.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 sound/pci/emu10k1/emupcm.c | 31 +++++--------------------------
 1 file changed, 5 insertions(+), 26 deletions(-)

diff --git a/sound/pci/emu10k1/emupcm.c b/sound/pci/emu10k1/emupcm.c
index 7f4c1b38d6ec..1bf6e3d652f8 100644
--- a/sound/pci/emu10k1/emupcm.c
+++ b/sound/pci/emu10k1/emupcm.c
@@ -147,16 +147,6 @@ static const struct snd_pcm_hw_constraint_list hw_constraints_capture_buffer_siz
 	.mask = 0
 };
 
-static const unsigned int capture_rates[8] = {
-	8000, 11025, 16000, 22050, 24000, 32000, 44100, 48000
-};
-
-static const struct snd_pcm_hw_constraint_list hw_constraints_capture_rates = {
-	.count = 8,
-	.list = capture_rates,
-	.mask = 0
-};
-
 static unsigned int snd_emu10k1_capture_rate_reg(unsigned int rate)
 {
 	switch (rate) {
@@ -174,16 +164,6 @@ static unsigned int snd_emu10k1_capture_rate_reg(unsigned int rate)
 	}
 }
 
-static const unsigned int audigy_capture_rates[9] = {
-	8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000
-};
-
-static const struct snd_pcm_hw_constraint_list hw_constraints_audigy_capture_rates = {
-	.count = 9,
-	.list = audigy_capture_rates,
-	.mask = 0
-};
-
 static unsigned int snd_emu10k1_audigy_capture_rate_reg(unsigned int rate)
 {
 	switch (rate) {
@@ -207,17 +187,16 @@ static void snd_emu10k1_constrain_capture_rates(struct snd_emu10k1 *emu,
 {
 	if (emu->card_capabilities->emu_model &&
 	    emu->emu1010.word_clock == 44100) {
-		// This also sets the rate constraint by deleting SNDRV_PCM_RATE_KNOT
 		runtime->hw.rates = SNDRV_PCM_RATE_11025 | \
 				    SNDRV_PCM_RATE_22050 | \
 				    SNDRV_PCM_RATE_44100;
 		runtime->hw.rate_min = 11025;
 		runtime->hw.rate_max = 44100;
-		return;
+	} else if (emu->audigy) {
+		runtime->hw.rates = SNDRV_PCM_RATE_8000_48000 |
+				    SNDRV_PCM_RATE_12000 |
+				    SNDRV_PCM_RATE_24000;
 	}
-	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
-				   emu->audigy ? &hw_constraints_audigy_capture_rates :
-						 &hw_constraints_capture_rates);
 }
 
 static void snd_emu1010_constrain_efx_rate(struct snd_emu10k1 *emu,
@@ -1053,7 +1032,7 @@ static const struct snd_pcm_hardware snd_emu10k1_capture =
 				 SNDRV_PCM_INFO_RESUME |
 				 SNDRV_PCM_INFO_MMAP_VALID),
 	.formats =		SNDRV_PCM_FMTBIT_S16_LE,
-	.rates =		SNDRV_PCM_RATE_8000_48000 | SNDRV_PCM_RATE_KNOT,
+	.rates =		SNDRV_PCM_RATE_8000_48000 | SNDRV_PCM_RATE_24000,
 	.rate_min =		8000,
 	.rate_max =		48000,
 	.channels_min =		1,

-- 
2.45.2

[PATCH 04/13] ALSA: hdsp: drop SNDRV_PCM_RATE_KNOT

[Jerome Brunet]

The custom rate constraint list was necessary to support 128kHz.
This rate is now available through SNDRV_PCM_RATE_128000.

Use it and drop the custom rate constraint rule.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 sound/pci/rme9652/hdsp.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c
index 713ca262a0e9..1c504a591948 100644
--- a/sound/pci/rme9652/hdsp.c
+++ b/sound/pci/rme9652/hdsp.c
@@ -4301,14 +4301,6 @@ static const struct snd_pcm_hw_constraint_list hdsp_hw_constraints_period_sizes
 	.mask = 0
 };
 
-static const unsigned int hdsp_9632_sample_rates[] = { 32000, 44100, 48000, 64000, 88200, 96000, 128000, 176400, 192000 };
-
-static const struct snd_pcm_hw_constraint_list hdsp_hw_constraints_9632_sample_rates = {
-	.count = ARRAY_SIZE(hdsp_9632_sample_rates),
-	.list = hdsp_9632_sample_rates,
-	.mask = 0
-};
-
 static int snd_hdsp_hw_rule_in_channels(struct snd_pcm_hw_params *params,
 					struct snd_pcm_hw_rule *rule)
 {
@@ -4499,8 +4491,9 @@ static int snd_hdsp_playback_open(struct snd_pcm_substream *substream)
 		runtime->hw.rate_min = runtime->hw.rate_max = hdsp->system_sample_rate;
 	} else if (hdsp->io_type == H9632) {
 		runtime->hw.rate_max = 192000;
-		runtime->hw.rates = SNDRV_PCM_RATE_KNOT;
-		snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &hdsp_hw_constraints_9632_sample_rates);
+		runtime->hw.rates |= (SNDRV_PCM_RATE_128000 |
+				      SNDRV_PCM_RATE_176400 |
+				      SNDRV_PCM_RATE_192000);
 	}
 	if (hdsp->io_type == H9632) {
 		runtime->hw.channels_min = hdsp->qs_out_channels;
@@ -4575,8 +4568,9 @@ static int snd_hdsp_capture_open(struct snd_pcm_substream *substream)
 		runtime->hw.channels_min = hdsp->qs_in_channels;
 		runtime->hw.channels_max = hdsp->ss_in_channels;
 		runtime->hw.rate_max = 192000;
-		runtime->hw.rates = SNDRV_PCM_RATE_KNOT;
-		snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &hdsp_hw_constraints_9632_sample_rates);
+		runtime->hw.rates |= (SNDRV_PCM_RATE_128000 |
+				      SNDRV_PCM_RATE_176400 |
+				      SNDRV_PCM_RATE_192000);
 	}
 	snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
 			     snd_hdsp_hw_rule_in_channels, hdsp,

-- 
2.45.2

[PATCH 05/13] ALSA: hdspm: drop SNDRV_PCM_RATE_KNOT

[Jerome Brunet]

The custom rate constraint list was necessary to support 128kHz.
This rate is now available through SNDRV_PCM_RATE_128000.

Use it and drop the custom rate constraint rule.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 sound/pci/rme9652/hdspm.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/sound/pci/rme9652/hdspm.c b/sound/pci/rme9652/hdspm.c
index c3f930a8f78d..dad974378e00 100644
--- a/sound/pci/rme9652/hdspm.c
+++ b/sound/pci/rme9652/hdspm.c
@@ -6032,18 +6032,6 @@ static int snd_hdspm_hw_rule_out_channels(struct snd_pcm_hw_params *params,
 	return snd_interval_list(c, 3, list, 0);
 }
 
-
-static const unsigned int hdspm_aes32_sample_rates[] = {
-	32000, 44100, 48000, 64000, 88200, 96000, 128000, 176400, 192000
-};
-
-static const struct snd_pcm_hw_constraint_list
-hdspm_hw_constraints_aes32_sample_rates = {
-	.count = ARRAY_SIZE(hdspm_aes32_sample_rates),
-	.list = hdspm_aes32_sample_rates,
-	.mask = 0
-};
-
 static int snd_hdspm_open(struct snd_pcm_substream *substream)
 {
 	struct hdspm *hdspm = snd_pcm_substream_chip(substream);
@@ -6096,9 +6084,7 @@ static int snd_hdspm_open(struct snd_pcm_substream *substream)
 	}
 
 	if (AES32 == hdspm->io_type) {
-		runtime->hw.rates |= SNDRV_PCM_RATE_KNOT;
-		snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
-				&hdspm_hw_constraints_aes32_sample_rates);
+		runtime->hw.rates |= SNDRV_PCM_RATE_128000;
 	} else {
 		snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
 				(playback ?

-- 
2.45.2

[PATCH 06/13] ASoC: cs35l36: drop SNDRV_PCM_RATE_KNOT

[Jerome Brunet]

The custom rate constraint list was necessary to support 12kHz and 24kHz.
These rates are now available through SNDRV_PCM_RATE_12000 and
SNDRV_PCM_RATE_24000.

Use them and drop the custom rate constraint rule.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 sound/soc/codecs/cs35l36.c | 34 ++++++++++++----------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/sound/soc/codecs/cs35l36.c b/sound/soc/codecs/cs35l36.c
index cbea79bd8980..b49c6905e872 100644
--- a/sound/soc/codecs/cs35l36.c
+++ b/sound/soc/codecs/cs35l36.c
@@ -949,32 +949,22 @@ static const struct cs35l36_pll_config *cs35l36_get_clk_config(
 	return NULL;
 }
 
-static const unsigned int cs35l36_src_rates[] = {
-	8000, 12000, 11025, 16000, 22050, 24000, 32000,
-	44100, 48000, 88200, 96000, 176400, 192000, 384000
-};
-
-static const struct snd_pcm_hw_constraint_list cs35l36_constraints = {
-	.count  = ARRAY_SIZE(cs35l36_src_rates),
-	.list   = cs35l36_src_rates,
-};
-
-static int cs35l36_pcm_startup(struct snd_pcm_substream *substream,
-			       struct snd_soc_dai *dai)
-{
-	snd_pcm_hw_constraint_list(substream->runtime, 0,
-				SNDRV_PCM_HW_PARAM_RATE, &cs35l36_constraints);
-
-	return 0;
-}
-
 static const struct snd_soc_dai_ops cs35l36_ops = {
-	.startup = cs35l36_pcm_startup,
 	.set_fmt = cs35l36_set_dai_fmt,
 	.hw_params = cs35l36_pcm_hw_params,
 	.set_sysclk = cs35l36_dai_set_sysclk,
 };
 
+#define CS35L36_RATES (		    \
+	SNDRV_PCM_RATE_8000_48000 | \
+	SNDRV_PCM_RATE_12000 |	    \
+	SNDRV_PCM_RATE_24000 |	    \
+	SNDRV_PCM_RATE_88200 |	    \
+	SNDRV_PCM_RATE_96000 |	    \
+	SNDRV_PCM_RATE_176400 |	    \
+	SNDRV_PCM_RATE_192000 |	    \
+	SNDRV_PCM_RATE_384000)
+
 static struct snd_soc_dai_driver cs35l36_dai[] = {
 	{
 		.name = "cs35l36-pcm",
@@ -983,14 +973,14 @@ static struct snd_soc_dai_driver cs35l36_dai[] = {
 			.stream_name = "AMP Playback",
 			.channels_min = 1,
 			.channels_max = 8,
-			.rates = SNDRV_PCM_RATE_KNOT,
+			.rates = CS35L36_RATES,
 			.formats = CS35L36_RX_FORMATS,
 		},
 		.capture = {
 			.stream_name = "AMP Capture",
 			.channels_min = 1,
 			.channels_max = 8,
-			.rates = SNDRV_PCM_RATE_KNOT,
+			.rates = CS35L36_RATES,
 			.formats = CS35L36_TX_FORMATS,
 		},
 		.ops = &cs35l36_ops,

-- 
2.45.2

[PATCH 07/13] ASoC: cs35l41: drop SNDRV_PCM_RATE_KNOT

[Jerome Brunet]

The custom rate constraint list was necessary to support 12kHz and 24kHz.
These rates are now available through SNDRV_PCM_RATE_12000 and
SNDRV_PCM_RATE_24000.

Use them and drop the custom rate constraint rule.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 sound/soc/codecs/cs35l41.c | 34 +++++++++++-----------------------
 1 file changed, 11 insertions(+), 23 deletions(-)

diff --git a/sound/soc/codecs/cs35l41.c b/sound/soc/codecs/cs35l41.c
index 1688c2c688f0..07a5cab35fe1 100644
--- a/sound/soc/codecs/cs35l41.c
+++ b/sound/soc/codecs/cs35l41.c
@@ -808,26 +808,6 @@ static int cs35l41_get_clk_config(int freq)
 	return -EINVAL;
 }
 
-static const unsigned int cs35l41_src_rates[] = {
-	8000, 12000, 11025, 16000, 22050, 24000, 32000,
-	44100, 48000, 88200, 96000, 176400, 192000
-};
-
-static const struct snd_pcm_hw_constraint_list cs35l41_constraints = {
-	.count = ARRAY_SIZE(cs35l41_src_rates),
-	.list = cs35l41_src_rates,
-};
-
-static int cs35l41_pcm_startup(struct snd_pcm_substream *substream,
-			       struct snd_soc_dai *dai)
-{
-	if (substream->runtime)
-		return snd_pcm_hw_constraint_list(substream->runtime, 0,
-						  SNDRV_PCM_HW_PARAM_RATE,
-						  &cs35l41_constraints);
-	return 0;
-}
-
 static int cs35l41_component_set_sysclk(struct snd_soc_component *component,
 					int clk_id, int source,
 					unsigned int freq, int dir)
@@ -974,13 +954,21 @@ static void cs35l41_component_remove(struct snd_soc_component *component)
 }
 
 static const struct snd_soc_dai_ops cs35l41_ops = {
-	.startup = cs35l41_pcm_startup,
 	.set_fmt = cs35l41_set_dai_fmt,
 	.hw_params = cs35l41_pcm_hw_params,
 	.set_sysclk = cs35l41_dai_set_sysclk,
 	.set_channel_map = cs35l41_set_channel_map,
 };
 
+#define CS35L41_RATES (		    \
+	SNDRV_PCM_RATE_8000_48000 | \
+	SNDRV_PCM_RATE_12000 |	    \
+	SNDRV_PCM_RATE_24000 |	    \
+	SNDRV_PCM_RATE_88200 |	    \
+	SNDRV_PCM_RATE_96000 |	    \
+	SNDRV_PCM_RATE_176400 |	    \
+	SNDRV_PCM_RATE_192000)
+
 static struct snd_soc_dai_driver cs35l41_dai[] = {
 	{
 		.name = "cs35l41-pcm",
@@ -989,14 +977,14 @@ static struct snd_soc_dai_driver cs35l41_dai[] = {
 			.stream_name = "AMP Playback",
 			.channels_min = 1,
 			.channels_max = 2,
-			.rates = SNDRV_PCM_RATE_KNOT,
+			.rates = CS35L41_RATES,
 			.formats = CS35L41_RX_FORMATS,
 		},
 		.capture = {
 			.stream_name = "AMP Capture",
 			.channels_min = 1,
 			.channels_max = 4,
-			.rates = SNDRV_PCM_RATE_KNOT,
+			.rates = CS35L41_RATES,
 			.formats = CS35L41_TX_FORMATS,
 		},
 		.ops = &cs35l41_ops,

-- 
2.45.2

[PATCH 08/13] ASoC: cs53l30: drop SNDRV_PCM_RATE_KNOT

[Jerome Brunet]

The custom rate constraint list was necessary to support 12kHz and 24kHz.
These rates are now available through SNDRV_PCM_RATE_12000 and
SNDRV_PCM_RATE_24000.

Use them and drop the custom rate constraint rule.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 sound/soc/codecs/cs53l30.c | 24 +++---------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/sound/soc/codecs/cs53l30.c b/sound/soc/codecs/cs53l30.c
index bcbaf28a0b2d..28f4be37dec1 100644
--- a/sound/soc/codecs/cs53l30.c
+++ b/sound/soc/codecs/cs53l30.c
@@ -739,24 +739,6 @@ static int cs53l30_set_tristate(struct snd_soc_dai *dai, int tristate)
 				  CS53L30_ASP_3ST_MASK, val);
 }
 
-static unsigned int const cs53l30_src_rates[] = {
-	8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000
-};
-
-static const struct snd_pcm_hw_constraint_list src_constraints = {
-	.count = ARRAY_SIZE(cs53l30_src_rates),
-	.list = cs53l30_src_rates,
-};
-
-static int cs53l30_pcm_startup(struct snd_pcm_substream *substream,
-			       struct snd_soc_dai *dai)
-{
-	snd_pcm_hw_constraint_list(substream->runtime, 0,
-				   SNDRV_PCM_HW_PARAM_RATE, &src_constraints);
-
-	return 0;
-}
-
 /*
  * Note: CS53L30 counts the slot number per byte while ASoC counts the slot
  * number per slot_width. So there is a difference between the slots of ASoC
@@ -843,14 +825,14 @@ static int cs53l30_mute_stream(struct snd_soc_dai *dai, int mute, int stream)
 	return 0;
 }
 
-/* SNDRV_PCM_RATE_KNOT -> 12000, 24000 Hz, limit with constraint list */
-#define CS53L30_RATES (SNDRV_PCM_RATE_8000_48000 | SNDRV_PCM_RATE_KNOT)
+#define CS53L30_RATES (SNDRV_PCM_RATE_8000_48000 |	\
+		       SNDRV_PCM_RATE_12000 |		\
+		       SNDRV_PCM_RATE_24000)
 
 #define CS53L30_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\
 			SNDRV_PCM_FMTBIT_S24_LE)
 
 static const struct snd_soc_dai_ops cs53l30_ops = {
-	.startup = cs53l30_pcm_startup,
 	.hw_params = cs53l30_pcm_hw_params,
 	.set_fmt = cs53l30_set_dai_fmt,
 	.set_sysclk = cs53l30_set_sysclk,

-- 
2.45.2

[PATCH 09/13] ASoC: Intel: avs: drop SNDRV_PCM_RATE_KNOT

[Jerome Brunet]

The custom rate constraint list was necessary to support 12kHz, 24kHz and
128kHz. These rates are now available through SNDRV_PCM_RATE_12000,
SNDRV_PCM_RATE_24000 and SNDRV_PCM_RATE_128000.

Use them and drop the custom rate constraint rule.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 sound/soc/intel/avs/pcm.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c
index c76b86254a8b..afc0fc74cf94 100644
--- a/sound/soc/intel/avs/pcm.c
+++ b/sound/soc/intel/avs/pcm.c
@@ -471,16 +471,6 @@ static int hw_rule_param_size(struct snd_pcm_hw_params *params, struct snd_pcm_h
 static int avs_pcm_hw_constraints_init(struct snd_pcm_substream *substream)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	static const unsigned int rates[] = {
-		8000, 11025, 12000, 16000,
-		22050, 24000, 32000, 44100,
-		48000, 64000, 88200, 96000,
-		128000, 176400, 192000,
-	};
-	static const struct snd_pcm_hw_constraint_list rate_list = {
-		.count = ARRAY_SIZE(rates),
-		.list = rates,
-	};
 	int ret;
 
 	ret = snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
@@ -492,10 +482,6 @@ static int avs_pcm_hw_constraints_init(struct snd_pcm_substream *substream)
 	if (ret < 0)
 		return ret;
 
-	ret = snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &rate_list);
-	if (ret < 0)
-		return ret;
-
 	/* Adjust buffer and period size based on the audio format. */
 	snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_BUFFER_SIZE, hw_rule_param_size, NULL,
 			    SNDRV_PCM_HW_PARAM_FORMAT, SNDRV_PCM_HW_PARAM_CHANNELS,
@@ -1332,7 +1318,9 @@ static const struct snd_soc_dai_driver i2s_dai_template = {
 		.channels_min	= 1,
 		.channels_max	= 8,
 		.rates		= SNDRV_PCM_RATE_8000_192000 |
-				  SNDRV_PCM_RATE_KNOT,
+				  SNDRV_PCM_RATE_12000 |
+				  SNDRV_PCM_RATE_24000 |
+				  SNDRV_PCM_RATE_128000,
 		.formats	= SNDRV_PCM_FMTBIT_S16_LE |
 				  SNDRV_PCM_FMTBIT_S32_LE,
 		.subformats	= SNDRV_PCM_SUBFMTBIT_MSBITS_20 |
@@ -1343,7 +1331,9 @@ static const struct snd_soc_dai_driver i2s_dai_template = {
 		.channels_min	= 1,
 		.channels_max	= 8,
 		.rates		= SNDRV_PCM_RATE_8000_192000 |
-				  SNDRV_PCM_RATE_KNOT,
+				  SNDRV_PCM_RATE_12000 |
+				  SNDRV_PCM_RATE_24000 |
+				  SNDRV_PCM_RATE_128000,
 		.formats	= SNDRV_PCM_FMTBIT_S16_LE |
 				  SNDRV_PCM_FMTBIT_S32_LE,
 		.subformats	= SNDRV_PCM_SUBFMTBIT_MSBITS_20 |

-- 
2.45.2

[PATCH 10/13] ASoC: qcom: q6asm-dai: drop SNDRV_PCM_RATE_KNOT

[Jerome Brunet]

The custom rate constraint list was necessary to support 12kHz and 24kHz.
These rates are now available through SNDRV_PCM_RATE_12000 and
SNDRV_PCM_RATE_24000.

Use them and drop the custom rate constraint rule.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 sound/soc/qcom/qdsp6/q6asm-dai.c | 31 ++++++++++---------------------
 1 file changed, 10 insertions(+), 21 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/q6asm-dai.c b/sound/soc/qcom/qdsp6/q6asm-dai.c
index 3913706ccdc5..045100c94352 100644
--- a/sound/soc/qcom/qdsp6/q6asm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6asm-dai.c
@@ -128,8 +128,13 @@ static const struct snd_pcm_hardware q6asm_dai_hardware_playback = {
 #define Q6ASM_FEDAI_DRIVER(num) { \
 		.playback = {						\
 			.stream_name = "MultiMedia"#num" Playback",	\
-			.rates = (SNDRV_PCM_RATE_8000_192000|		\
-					SNDRV_PCM_RATE_KNOT),		\
+			.rates = (SNDRV_PCM_RATE_8000_48000 |		\
+				  SNDRV_PCM_RATE_12000 |		\
+				  SNDRV_PCM_RATE_24000 |		\
+				  SNDRV_PCM_RATE_88200 |		\
+				  SNDRV_PCM_RATE_96000 |		\
+				  SNDRV_PCM_RATE_176400 |		\
+				  SNDRV_PCM_RATE_192000),		\
 			.formats = (SNDRV_PCM_FMTBIT_S16_LE |		\
 					SNDRV_PCM_FMTBIT_S24_LE),	\
 			.channels_min = 1,				\
@@ -139,8 +144,9 @@ static const struct snd_pcm_hardware q6asm_dai_hardware_playback = {
 		},							\
 		.capture = {						\
 			.stream_name = "MultiMedia"#num" Capture",	\
-			.rates = (SNDRV_PCM_RATE_8000_48000|		\
-					SNDRV_PCM_RATE_KNOT),		\
+			.rates = (SNDRV_PCM_RATE_8000_48000 |		\
+				  SNDRV_PCM_RATE_12000 |		\
+				  SNDRV_PCM_RATE_24000),		\
 			.formats = (SNDRV_PCM_FMTBIT_S16_LE |		\
 				    SNDRV_PCM_FMTBIT_S24_LE),		\
 			.channels_min = 1,				\
@@ -152,18 +158,6 @@ static const struct snd_pcm_hardware q6asm_dai_hardware_playback = {
 		.id = MSM_FRONTEND_DAI_MULTIMEDIA##num,			\
 	}
 
-/* Conventional and unconventional sample rate supported */
-static unsigned int supported_sample_rates[] = {
-	8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000,
-	88200, 96000, 176400, 192000
-};
-
-static struct snd_pcm_hw_constraint_list constraints_sample_rates = {
-	.count = ARRAY_SIZE(supported_sample_rates),
-	.list = supported_sample_rates,
-	.mask = 0,
-};
-
 static const struct snd_compr_codec_caps q6asm_compr_caps = {
 	.num_descriptors = 1,
 	.descriptor[0].max_ch = 2,
@@ -390,11 +384,6 @@ static int q6asm_dai_open(struct snd_soc_component *component,
 	else if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
 		runtime->hw = q6asm_dai_hardware_capture;
 
-	ret = snd_pcm_hw_constraint_list(runtime, 0,
-				SNDRV_PCM_HW_PARAM_RATE,
-				&constraints_sample_rates);
-	if (ret < 0)
-		dev_info(dev, "snd_pcm_hw_constraint_list failed\n");
 	/* Ensure that buffer size is a multiple of period size */
 	ret = snd_pcm_hw_constraint_integer(runtime,
 					    SNDRV_PCM_HW_PARAM_PERIODS);

-- 
2.45.2

[PATCH 11/13] ASoC: sunxi: sun4i-codec: drop SNDRV_PCM_RATE_KNOT

[Jerome Brunet]

The custom rate constraint lists was necessary to support 12kHz and 24kHz.
These rates are now available through SNDRV_PCM_RATE_12000 and
SNDRV_PCM_RATE_24000.

Use them and drop the custom rate constraint rule.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 sound/soc/sunxi/sun4i-codec.c | 28 +++++++++-------------------
 1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index a2618ed650b0..25af47b63bdd 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -577,28 +577,12 @@ static int sun4i_codec_hw_params(struct snd_pcm_substream *substream,
 					     hwrate);
 }
 
-
-static unsigned int sun4i_codec_src_rates[] = {
-	8000, 11025, 12000, 16000, 22050, 24000, 32000,
-	44100, 48000, 96000, 192000
-};
-
-
-static struct snd_pcm_hw_constraint_list sun4i_codec_constraints = {
-	.count  = ARRAY_SIZE(sun4i_codec_src_rates),
-	.list   = sun4i_codec_src_rates,
-};
-
-
 static int sun4i_codec_startup(struct snd_pcm_substream *substream,
 			       struct snd_soc_dai *dai)
 {
 	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
 	struct sun4i_codec *scodec = snd_soc_card_get_drvdata(rtd->card);
 
-	snd_pcm_hw_constraint_list(substream->runtime, 0,
-				SNDRV_PCM_HW_PARAM_RATE, &sun4i_codec_constraints);
-
 	/*
 	 * Stop issuing DRQ when we have room for less than 16 samples
 	 * in our TX FIFO
@@ -626,6 +610,13 @@ static const struct snd_soc_dai_ops sun4i_codec_dai_ops = {
 	.prepare	= sun4i_codec_prepare,
 };
 
+#define SUN4I_CODEC_RATES (			\
+		SNDRV_PCM_RATE_8000_48000 |	\
+		SNDRV_PCM_RATE_12000 |		\
+		SNDRV_PCM_RATE_24000 |		\
+		SNDRV_PCM_RATE_96000 |		\
+		SNDRV_PCM_RATE_192000)
+
 static struct snd_soc_dai_driver sun4i_codec_dai = {
 	.name	= "Codec",
 	.ops	= &sun4i_codec_dai_ops,
@@ -635,7 +626,7 @@ static struct snd_soc_dai_driver sun4i_codec_dai = {
 		.channels_max	= 2,
 		.rate_min	= 8000,
 		.rate_max	= 192000,
-		.rates		= SNDRV_PCM_RATE_CONTINUOUS,
+		.rates		= SUN4I_CODEC_RATES,
 		.formats	= SNDRV_PCM_FMTBIT_S16_LE |
 				  SNDRV_PCM_FMTBIT_S32_LE,
 		.sig_bits	= 24,
@@ -646,7 +637,7 @@ static struct snd_soc_dai_driver sun4i_codec_dai = {
 		.channels_max	= 2,
 		.rate_min	= 8000,
 		.rate_max	= 48000,
-		.rates		= SNDRV_PCM_RATE_CONTINUOUS,
+		.rates		= SUN4I_CODEC_RATES,
 		.formats	= SNDRV_PCM_FMTBIT_S16_LE |
 				  SNDRV_PCM_FMTBIT_S32_LE,
 		.sig_bits	= 24,
@@ -1233,7 +1224,6 @@ static const struct snd_soc_component_driver sun4i_codec_component = {
 #endif
 };
 
-#define SUN4I_CODEC_RATES	SNDRV_PCM_RATE_CONTINUOUS
 #define SUN4I_CODEC_FORMATS	(SNDRV_PCM_FMTBIT_S16_LE | \
 				 SNDRV_PCM_FMTBIT_S32_LE)
 

-- 
2.45.2

[PATCH 12/13] ASoC: cs35l34: drop useless rate contraint

[Jerome Brunet]

The cs35l34 adds a useless rate constraint on startup.
It does not set SNDRV_PCM_RATE_KNOT and the rates set are already a subset
of the ones provided in the constraint list, so it is a no-op.

From the rest of the code, it is likely HW supports more than the 32, 44.1
and 48kHz listed in CS35L34_RATES but there is no way to know for sure
without proper documentation.

Keep the driver as it is for now and just drop the useless constraint.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 sound/soc/codecs/cs35l34.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/sound/soc/codecs/cs35l34.c b/sound/soc/codecs/cs35l34.c
index e63a518e3b8e..287b27476a10 100644
--- a/sound/soc/codecs/cs35l34.c
+++ b/sound/soc/codecs/cs35l34.c
@@ -562,26 +562,6 @@ static int cs35l34_pcm_hw_params(struct snd_pcm_substream *substream,
 	return ret;
 }
 
-static const unsigned int cs35l34_src_rates[] = {
-	8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000
-};
-
-
-static const struct snd_pcm_hw_constraint_list cs35l34_constraints = {
-	.count  = ARRAY_SIZE(cs35l34_src_rates),
-	.list   = cs35l34_src_rates,
-};
-
-static int cs35l34_pcm_startup(struct snd_pcm_substream *substream,
-			       struct snd_soc_dai *dai)
-{
-
-	snd_pcm_hw_constraint_list(substream->runtime, 0,
-				SNDRV_PCM_HW_PARAM_RATE, &cs35l34_constraints);
-	return 0;
-}
-
-
 static int cs35l34_set_tristate(struct snd_soc_dai *dai, int tristate)
 {
 
@@ -639,7 +619,6 @@ static int cs35l34_dai_set_sysclk(struct snd_soc_dai *dai,
 }
 
 static const struct snd_soc_dai_ops cs35l34_ops = {
-	.startup = cs35l34_pcm_startup,
 	.set_tristate = cs35l34_set_tristate,
 	.set_fmt = cs35l34_set_dai_fmt,
 	.hw_params = cs35l34_pcm_hw_params,

-- 
2.45.2

[PATCH 13/13] ASoC: spdif: extend supported rates to 768kHz

[Jerome Brunet]

IEC958-3 defines sampling rate up to 768 kHz.
Such rates maybe used with high bandwidth IEC958 links, such as eARC.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 sound/soc/codecs/spdif_receiver.c    | 3 ++-
 sound/soc/codecs/spdif_transmitter.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/spdif_receiver.c b/sound/soc/codecs/spdif_receiver.c
index 862e0b654a1c..310123d2bb5f 100644
--- a/sound/soc/codecs/spdif_receiver.c
+++ b/sound/soc/codecs/spdif_receiver.c
@@ -28,7 +28,8 @@ static const struct snd_soc_dapm_route dir_routes[] = {
 	{ "Capture", NULL, "spdif-in" },
 };
 
-#define STUB_RATES	SNDRV_PCM_RATE_8000_192000
+#define STUB_RATES	(SNDRV_PCM_RATE_8000_768000 | \
+			 SNDRV_PCM_RATE_128000)
 #define STUB_FORMATS	(SNDRV_PCM_FMTBIT_S16_LE | \
 			SNDRV_PCM_FMTBIT_S20_3LE | \
 			SNDRV_PCM_FMTBIT_S24_LE  | \
diff --git a/sound/soc/codecs/spdif_transmitter.c b/sound/soc/codecs/spdif_transmitter.c
index 736518921555..db51a46e689d 100644
--- a/sound/soc/codecs/spdif_transmitter.c
+++ b/sound/soc/codecs/spdif_transmitter.c
@@ -21,7 +21,8 @@
 
 #define DRV_NAME "spdif-dit"
 
-#define STUB_RATES	SNDRV_PCM_RATE_8000_192000
+#define STUB_RATES	(SNDRV_PCM_RATE_8000_768000 | \
+			 SNDRV_PCM_RATE_128000)
 #define STUB_FORMATS	(SNDRV_PCM_FMTBIT_S16_LE | \
 			SNDRV_PCM_FMTBIT_S20_3LE | \
 			SNDRV_PCM_FMTBIT_S24_LE  | \

-- 
2.45.2

Re: [PATCH 00/13] ALSA: update sample rate definitions

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 412 bytes --]

On Thu, Sep 05, 2024 at 04:12:51PM +0200, Jerome Brunet wrote:
> This patchset adds rate definitions for 12kHz, 24kHz and 128kHz.
> 
> It is follow-up on the series/discussion [0] about adding 128kHz for
> spdif/eARC support. The outcome was to add 12kHz and 24kHz as well and
> clean up the drivers that no longer require custom rules to allow these
> rates.

Acked-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 00/13] ALSA: update sample rate definitions

[Jaroslav Kysela]

On 05. 09. 24 16:12, Jerome Brunet wrote:
> This patchset adds rate definitions for 12kHz, 24kHz and 128kHz.
> 
> It is follow-up on the series/discussion [0] about adding 128kHz for
> spdif/eARC support. The outcome was to add 12kHz and 24kHz as well and
> clean up the drivers that no longer require custom rules to allow these
> rates.
> 
> Identifying these drivers is not straight forward, I tried my best but I
> may have missed some. Those will continue to work anyway so it is not
> critical. Some drivers using these rates have not been changed on
> purpose. The reason for this may be:
> * The driver used other uncommon rates that still don't have a definition
>    so a custom rule is still required.
> * The constraint structure is used in some other way by the driver and
>    removing it would not help the readability or maintainability. This is
>    the case the freescale asrc drivers for example.
> 
> There is one change per driver so, if there is a problem later on, it will
> easier to properly add Fixes tag.
> 
> The series has been tested with
> * ARM64 defconfig - spdif 128kHz at runtime.
> * x86_64 allmodconfig - compile test only
> 
> Last, the change adding IEC958 definitions has been dropped for this
> patchset and will be resent separately
> 
> [0]: https://lore.kernel.org/all/20240628122429.2018059-1-jbrunet@baylibre.com/
> 
> ---
> Jerome Brunet (13):
>        ALSA: pcm: add more sample rate definitions
>        ALSA: cmipci: drop SNDRV_PCM_RATE_KNOT
>        ALSA: emu10k1: drop SNDRV_PCM_RATE_KNOT
>        ALSA: hdsp: drop SNDRV_PCM_RATE_KNOT
>        ALSA: hdspm: drop SNDRV_PCM_RATE_KNOT
>        ASoC: cs35l36: drop SNDRV_PCM_RATE_KNOT
>        ASoC: cs35l41: drop SNDRV_PCM_RATE_KNOT
>        ASoC: cs53l30: drop SNDRV_PCM_RATE_KNOT
>        ASoC: Intel: avs: drop SNDRV_PCM_RATE_KNOT
>        ASoC: qcom: q6asm-dai: drop SNDRV_PCM_RATE_KNOT
>        ASoC: sunxi: sun4i-codec: drop SNDRV_PCM_RATE_KNOT
>        ASoC: cs35l34: drop useless rate contraint
>        ASoC: spdif: extend supported rates to 768kHz
> 
>   include/sound/pcm.h                  | 31 +++++++++++++++++--------------
>   sound/core/pcm_native.c              |  6 +++---
>   sound/pci/cmipci.c                   | 32 +++++++++-----------------------
>   sound/pci/emu10k1/emupcm.c           | 31 +++++--------------------------
>   sound/pci/rme9652/hdsp.c             | 18 ++++++------------
>   sound/pci/rme9652/hdspm.c            | 16 +---------------
>   sound/soc/codecs/cs35l34.c           | 21 ---------------------
>   sound/soc/codecs/cs35l36.c           | 34 ++++++++++++----------------------
>   sound/soc/codecs/cs35l41.c           | 34 +++++++++++-----------------------
>   sound/soc/codecs/cs53l30.c           | 24 +++---------------------
>   sound/soc/codecs/spdif_receiver.c    |  3 ++-
>   sound/soc/codecs/spdif_transmitter.c |  3 ++-
>   sound/soc/intel/avs/pcm.c            | 22 ++++++----------------
>   sound/soc/qcom/qdsp6/q6asm-dai.c     | 31 ++++++++++---------------------
>   sound/soc/sunxi/sun4i-codec.c        | 28 +++++++++-------------------
>   15 files changed, 96 insertions(+), 238 deletions(-)
> ---
> base-commit: 785f4052380684dbcc156203c537c799e2f4be09
> change-id: 20240905-alsa-12-24-128-8edab4c08dd4
> 
> Best regards,

Thanks,

Reviewed-by: Jaroslav Kysela <perex@perex.cz>

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: [PATCH 00/13] ALSA: update sample rate definitions

[Rhodes, David]

On 9/5/24 9:12 AM, Jerome Brunet wrote:
> This patchset adds rate definitions for 12kHz, 24kHz and 128kHz.
> 
> It is follow-up on the series/discussion [0] about adding 128kHz for
> spdif/eARC support. The outcome was to add 12kHz and 24kHz as well and
> clean up the drivers that no longer require custom rules to allow these
> rates.
> 

Reviewed-by: David Rhodes <drhodes@opensource.cirrus.com>

Thanks,
David

Re: [PATCH 00/13] ALSA: update sample rate definitions

[Takashi Iwai]

On Thu, 05 Sep 2024 16:12:51 +0200,
Jerome Brunet wrote:
> 
> This patchset adds rate definitions for 12kHz, 24kHz and 128kHz.
> 
> It is follow-up on the series/discussion [0] about adding 128kHz for
> spdif/eARC support. The outcome was to add 12kHz and 24kHz as well and
> clean up the drivers that no longer require custom rules to allow these
> rates.
> 
> Identifying these drivers is not straight forward, I tried my best but I
> may have missed some. Those will continue to work anyway so it is not
> critical. Some drivers using these rates have not been changed on
> purpose. The reason for this may be:
> * The driver used other uncommon rates that still don't have a definition
>   so a custom rule is still required.
> * The constraint structure is used in some other way by the driver and
>   removing it would not help the readability or maintainability. This is
>   the case the freescale asrc drivers for example.
> 
> There is one change per driver so, if there is a problem later on, it will
> easier to properly add Fixes tag.
> 
> The series has been tested with
> * ARM64 defconfig - spdif 128kHz at runtime.
> * x86_64 allmodconfig - compile test only
> 
> Last, the change adding IEC958 definitions has been dropped for this
> patchset and will be resent separately
> 
> [0]: https://lore.kernel.org/all/20240628122429.2018059-1-jbrunet@baylibre.com/
> 
> ---
> Jerome Brunet (13):
>       ALSA: pcm: add more sample rate definitions
>       ALSA: cmipci: drop SNDRV_PCM_RATE_KNOT
>       ALSA: emu10k1: drop SNDRV_PCM_RATE_KNOT
>       ALSA: hdsp: drop SNDRV_PCM_RATE_KNOT
>       ALSA: hdspm: drop SNDRV_PCM_RATE_KNOT
>       ASoC: cs35l36: drop SNDRV_PCM_RATE_KNOT
>       ASoC: cs35l41: drop SNDRV_PCM_RATE_KNOT
>       ASoC: cs53l30: drop SNDRV_PCM_RATE_KNOT
>       ASoC: Intel: avs: drop SNDRV_PCM_RATE_KNOT
>       ASoC: qcom: q6asm-dai: drop SNDRV_PCM_RATE_KNOT
>       ASoC: sunxi: sun4i-codec: drop SNDRV_PCM_RATE_KNOT
>       ASoC: cs35l34: drop useless rate contraint
>       ASoC: spdif: extend supported rates to 768kHz

A nice cleanup series.
Applied all now to for-next branch.


Thanks!

Takashi

Re: [PATCH 12/13] ASoC: cs35l34: drop useless rate contraint

[Charles Keepax]

On Thu, Sep 05, 2024 at 04:13:03PM +0200, Jerome Brunet wrote:
> The cs35l34 adds a useless rate constraint on startup.
> It does not set SNDRV_PCM_RATE_KNOT and the rates set are already a subset
> of the ones provided in the constraint list, so it is a no-op.
> 
> >From the rest of the code, it is likely HW supports more than the 32, 44.1
> and 48kHz listed in CS35L34_RATES but there is no way to know for sure
> without proper documentation.
> 
> Keep the driver as it is for now and just drop the useless constraint.
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---

Yeah according to the datasheet it should support all the rates
listed in the cs35l34_src_rates list. But given the weird way
that CS35L34_RATES is implemented I think you are right best to
leave it as it is for now, incase there was a reason. Perhaps if
I find some time I might see if I have one in a draw somewhere in
the future.

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles
>  sound/soc/codecs/cs35l34.c | 21 ---------------------
>  1 file changed, 21 deletions(-)
> 
> diff --git a/sound/soc/codecs/cs35l34.c b/sound/soc/codecs/cs35l34.c
> index e63a518e3b8e..287b27476a10 100644
> --- a/sound/soc/codecs/cs35l34.c
> +++ b/sound/soc/codecs/cs35l34.c
> @@ -562,26 +562,6 @@ static int cs35l34_pcm_hw_params(struct snd_pcm_substream *substream,
>  	return ret;
>  }
>  
> -static const unsigned int cs35l34_src_rates[] = {
> -	8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000
> -};
> -
> -
> -static const struct snd_pcm_hw_constraint_list cs35l34_constraints = {
> -	.count  = ARRAY_SIZE(cs35l34_src_rates),
> -	.list   = cs35l34_src_rates,
> -};
> -
> -static int cs35l34_pcm_startup(struct snd_pcm_substream *substream,
> -			       struct snd_soc_dai *dai)
> -{
> -
> -	snd_pcm_hw_constraint_list(substream->runtime, 0,
> -				SNDRV_PCM_HW_PARAM_RATE, &cs35l34_constraints);
> -	return 0;
> -}
> -
> -
>  static int cs35l34_set_tristate(struct snd_soc_dai *dai, int tristate)
>  {
>  
> @@ -639,7 +619,6 @@ static int cs35l34_dai_set_sysclk(struct snd_soc_dai *dai,
>  }
>  
>  static const struct snd_soc_dai_ops cs35l34_ops = {
> -	.startup = cs35l34_pcm_startup,
>  	.set_tristate = cs35l34_set_tristate,
>  	.set_fmt = cs35l34_set_dai_fmt,
>  	.hw_params = cs35l34_pcm_hw_params,
> 
> -- 
> 2.45.2
> 

Re: [PATCH 06/13] ASoC: cs35l36: drop SNDRV_PCM_RATE_KNOT

[Charles Keepax]

On Thu, Sep 05, 2024 at 04:12:57PM +0200, Jerome Brunet wrote:
> The custom rate constraint list was necessary to support 12kHz and 24kHz.
> These rates are now available through SNDRV_PCM_RATE_12000 and
> SNDRV_PCM_RATE_24000.
> 
> Use them and drop the custom rate constraint rule.
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

Re: [PATCH 07/13] ASoC: cs35l41: drop SNDRV_PCM_RATE_KNOT

[Charles Keepax]

On Thu, Sep 05, 2024 at 04:12:58PM +0200, Jerome Brunet wrote:
> The custom rate constraint list was necessary to support 12kHz and 24kHz.
> These rates are now available through SNDRV_PCM_RATE_12000 and
> SNDRV_PCM_RATE_24000.
> 
> Use them and drop the custom rate constraint rule.
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

Re: [PATCH 08/13] ASoC: cs53l30: drop SNDRV_PCM_RATE_KNOT

[Charles Keepax]

On Thu, Sep 05, 2024 at 04:12:59PM +0200, Jerome Brunet wrote:
> The custom rate constraint list was necessary to support 12kHz and 24kHz.
> These rates are now available through SNDRV_PCM_RATE_12000 and
> SNDRV_PCM_RATE_24000.
> 
> Use them and drop the custom rate constraint rule.
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

Re: [PATCH 01/13] ALSA: pcm: add more sample rate definitions

[Charles Keepax]

On Thu, Sep 05, 2024 at 04:12:52PM +0200, Jerome Brunet wrote:
> This adds a sample rate definition for 12kHz, 24kHz and 128kHz.
> 
> Admittedly, just a few drivers are currently using these sample
> rates but there is enough of a recurrence to justify adding a definition
> for them and remove some custom rate constraint code while at it.
> 
> The new definitions are not added to the interval definitions, such as
> SNDRV_PCM_RATE_8000_44100, because it would silently add new supported
> rates to drivers that may or may not support them. For sure the drivers
> have not been tested for these new rates so it is better to leave them out
> of interval definitions.
> 
> That being said, the added rates are multiples of well know rates families,
> it is very likely that a lot of devices out there actually supports them.
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---

Almost wonder if a comment with the SNDRV_PCM_RATE_8000_xxx
defines might also be an idea to warn they don't include all the
rates, although it is I guess easily seen from the define itself
so not sure if it might be over kill. But I am happy either way.

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

Re: [PATCH 09/13] ASoC: Intel: avs: drop SNDRV_PCM_RATE_KNOT

[Cezary Rojewski]

On 2024-09-05 4:13 PM, Jerome Brunet wrote:
> The custom rate constraint list was necessary to support 12kHz, 24kHz and
> 128kHz. These rates are now available through SNDRV_PCM_RATE_12000,
> SNDRV_PCM_RATE_24000 and SNDRV_PCM_RATE_128000.
> 
> Use them and drop the custom rate constraint rule.
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---

Thank you for this cleanup.

Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>

>   sound/soc/intel/avs/pcm.c | 22 ++++++----------------
>   1 file changed, 6 insertions(+), 16 deletions(-)
> 
> diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c
> index c76b86254a8b..afc0fc74cf94 100644
> --- a/sound/soc/intel/avs/pcm.c
> +++ b/sound/soc/intel/avs/pcm.c
> @@ -471,16 +471,6 @@ static int hw_rule_param_size(struct snd_pcm_hw_params *params, struct snd_pcm_h
>   static int avs_pcm_hw_constraints_init(struct snd_pcm_substream *substream)
>   {
>   	struct snd_pcm_runtime *runtime = substream->runtime;
> -	static const unsigned int rates[] = {
> -		8000, 11025, 12000, 16000,
> -		22050, 24000, 32000, 44100,
> -		48000, 64000, 88200, 96000,
> -		128000, 176400, 192000,
> -	};
> -	static const struct snd_pcm_hw_constraint_list rate_list = {
> -		.count = ARRAY_SIZE(rates),
> -		.list = rates,
> -	};
>   	int ret;
>   
>   	ret = snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
> @@ -492,10 +482,6 @@ static int avs_pcm_hw_constraints_init(struct snd_pcm_substream *substream)
>   	if (ret < 0)
>   		return ret;
>   
> -	ret = snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &rate_list);
> -	if (ret < 0)
> -		return ret;
> -
>   	/* Adjust buffer and period size based on the audio format. */
>   	snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_BUFFER_SIZE, hw_rule_param_size, NULL,
>   			    SNDRV_PCM_HW_PARAM_FORMAT, SNDRV_PCM_HW_PARAM_CHANNELS,
> @@ -1332,7 +1318,9 @@ static const struct snd_soc_dai_driver i2s_dai_template = {
>   		.channels_min	= 1,
>   		.channels_max	= 8,
>   		.rates		= SNDRV_PCM_RATE_8000_192000 |
> -				  SNDRV_PCM_RATE_KNOT,
> +				  SNDRV_PCM_RATE_12000 |
> +				  SNDRV_PCM_RATE_24000 |
> +				  SNDRV_PCM_RATE_128000,
>   		.formats	= SNDRV_PCM_FMTBIT_S16_LE |
>   				  SNDRV_PCM_FMTBIT_S32_LE,
>   		.subformats	= SNDRV_PCM_SUBFMTBIT_MSBITS_20 |
> @@ -1343,7 +1331,9 @@ static const struct snd_soc_dai_driver i2s_dai_template = {
>   		.channels_min	= 1,
>   		.channels_max	= 8,
>   		.rates		= SNDRV_PCM_RATE_8000_192000 |
> -				  SNDRV_PCM_RATE_KNOT,
> +				  SNDRV_PCM_RATE_12000 |
> +				  SNDRV_PCM_RATE_24000 |
> +				  SNDRV_PCM_RATE_128000,
>   		.formats	= SNDRV_PCM_FMTBIT_S16_LE |
>   				  SNDRV_PCM_FMTBIT_S32_LE,
>   		.subformats	= SNDRV_PCM_SUBFMTBIT_MSBITS_20 |
> 

Re: [PATCH 01/13] ALSA: pcm: add more sample rate definitions

[Pierre-Louis Bossart]

On 9/5/24 16:12, Jerome Brunet wrote:
> This adds a sample rate definition for 12kHz, 24kHz and 128kHz.
> 
> Admittedly, just a few drivers are currently using these sample
> rates but there is enough of a recurrence to justify adding a definition
> for them and remove some custom rate constraint code while at it.
> 
> The new definitions are not added to the interval definitions, such as
> SNDRV_PCM_RATE_8000_44100, because it would silently add new supported
> rates to drivers that may or may not support them. For sure the drivers
> have not been tested for these new rates so it is better to leave them out
> of interval definitions.
> 
> That being said, the added rates are multiples of well know rates families,
> it is very likely that a lot of devices out there actually supports them.
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>  include/sound/pcm.h     | 31 +++++++++++++++++--------------
>  sound/core/pcm_native.c |  6 +++---
>  2 files changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index 732121b934fd..c993350975a9 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -109,20 +109,23 @@ struct snd_pcm_ops {
>  #define SNDRV_PCM_RATE_5512		(1U<<0)		/* 5512Hz */
>  #define SNDRV_PCM_RATE_8000		(1U<<1)		/* 8000Hz */
>  #define SNDRV_PCM_RATE_11025		(1U<<2)		/* 11025Hz */
> -#define SNDRV_PCM_RATE_16000		(1U<<3)		/* 16000Hz */
> -#define SNDRV_PCM_RATE_22050		(1U<<4)		/* 22050Hz */
> -#define SNDRV_PCM_RATE_32000		(1U<<5)		/* 32000Hz */
> -#define SNDRV_PCM_RATE_44100		(1U<<6)		/* 44100Hz */
> -#define SNDRV_PCM_RATE_48000		(1U<<7)		/* 48000Hz */
> -#define SNDRV_PCM_RATE_64000		(1U<<8)		/* 64000Hz */
> -#define SNDRV_PCM_RATE_88200		(1U<<9)		/* 88200Hz */
> -#define SNDRV_PCM_RATE_96000		(1U<<10)	/* 96000Hz */
> -#define SNDRV_PCM_RATE_176400		(1U<<11)	/* 176400Hz */
> -#define SNDRV_PCM_RATE_192000		(1U<<12)	/* 192000Hz */
> -#define SNDRV_PCM_RATE_352800		(1U<<13)	/* 352800Hz */
> -#define SNDRV_PCM_RATE_384000		(1U<<14)	/* 384000Hz */
> -#define SNDRV_PCM_RATE_705600		(1U<<15)	/* 705600Hz */
> -#define SNDRV_PCM_RATE_768000		(1U<<16)	/* 768000Hz */
> +#define SNDRV_PCM_RATE_12000		(1U<<3)		/* 12000Hz */
> +#define SNDRV_PCM_RATE_16000		(1U<<4)		/* 16000Hz */
> +#define SNDRV_PCM_RATE_22050		(1U<<5)		/* 22050Hz */
> +#define SNDRV_PCM_RATE_24000		(1U<<6)		/* 24000Hz */
> +#define SNDRV_PCM_RATE_32000		(1U<<7)		/* 32000Hz */
> +#define SNDRV_PCM_RATE_44100		(1U<<8)		/* 44100Hz */
> +#define SNDRV_PCM_RATE_48000		(1U<<9)		/* 48000Hz */
> +#define SNDRV_PCM_RATE_64000		(1U<<10)	/* 64000Hz */
> +#define SNDRV_PCM_RATE_88200		(1U<<11)	/* 88200Hz */
> +#define SNDRV_PCM_RATE_96000		(1U<<12)	/* 96000Hz */
> +#define SNDRV_PCM_RATE_128000		(1U<<13)	/* 128000Hz */
> +#define SNDRV_PCM_RATE_176400		(1U<<14)	/* 176400Hz */
> +#define SNDRV_PCM_RATE_192000		(1U<<15)	/* 192000Hz */
> +#define SNDRV_PCM_RATE_352800		(1U<<16)	/* 352800Hz */
> +#define SNDRV_PCM_RATE_384000		(1U<<17)	/* 384000Hz */
> +#define SNDRV_PCM_RATE_705600		(1U<<18)	/* 705600Hz */
> +#define SNDRV_PCM_RATE_768000		(1U<<19)	/* 768000Hz */
>  
>  #define SNDRV_PCM_RATE_CONTINUOUS	(1U<<30)	/* continuous range */
>  #define SNDRV_PCM_RATE_KNOT		(1U<<31)	/* supports more non-continuous rates */
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index 44381514f695..7461a727615c 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -2418,13 +2418,13 @@ static int snd_pcm_hw_rule_sample_bits(struct snd_pcm_hw_params *params,
>  	return snd_interval_refine(hw_param_interval(params, rule->var), &t);
>  }
>  
> -#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_192000 != 1 << 12
> +#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_768000 != 1 << 19
>  #error "Change this table"
>  #endif
>  
>  static const unsigned int rates[] = {
> -	5512, 8000, 11025, 16000, 22050, 32000, 44100,
> -	48000, 64000, 88200, 96000, 176400, 192000, 352800, 384000, 705600, 768000
> +	5512, 8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000, 64000,
> +	88200, 96000, 128000, 176400, 192000, 352800, 384000, 705600, 768000,
>  };
>  
>  const struct snd_pcm_hw_constraint_list snd_pcm_known_rates = {

Wondering if this is backwards compatible with the alsa-lib definitions,
specifically the topology parts which did unfortunately have a list of
rates that will map to a different index now:


typedef enum _snd_pcm_rates {
	SND_PCM_RATE_UNKNOWN = -1,
	SND_PCM_RATE_5512 = 0,
	SND_PCM_RATE_8000,
	SND_PCM_RATE_11025,
	SND_PCM_RATE_16000,
	SND_PCM_RATE_22050,
	SND_PCM_RATE_32000,
	SND_PCM_RATE_44100,
	SND_PCM_RATE_48000,
	SND_PCM_RATE_64000,
	SND_PCM_RATE_88200,
	SND_PCM_RATE_96000,
	SND_PCM_RATE_176400,
	SND_PCM_RATE_192000,
	SND_PCM_RATE_CONTINUOUS = 30,
	SND_PCM_RATE_KNOT = 31,
	SND_PCM_RATE_LAST = SND_PCM_RATE_KNOT,
} snd_pcm_rates_t;

Re: [PATCH 01/13] ALSA: pcm: add more sample rate definitions

[Takashi Iwai]

On Wed, 11 Sep 2024 11:09:59 +0200,
Pierre-Louis Bossart wrote:
> 
> 
> 
> On 9/5/24 16:12, Jerome Brunet wrote:
> > This adds a sample rate definition for 12kHz, 24kHz and 128kHz.
> > 
> > Admittedly, just a few drivers are currently using these sample
> > rates but there is enough of a recurrence to justify adding a definition
> > for them and remove some custom rate constraint code while at it.
> > 
> > The new definitions are not added to the interval definitions, such as
> > SNDRV_PCM_RATE_8000_44100, because it would silently add new supported
> > rates to drivers that may or may not support them. For sure the drivers
> > have not been tested for these new rates so it is better to leave them out
> > of interval definitions.
> > 
> > That being said, the added rates are multiples of well know rates families,
> > it is very likely that a lot of devices out there actually supports them.
> > 
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > ---
> >  include/sound/pcm.h     | 31 +++++++++++++++++--------------
> >  sound/core/pcm_native.c |  6 +++---
> >  2 files changed, 20 insertions(+), 17 deletions(-)
> > 
> > diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> > index 732121b934fd..c993350975a9 100644
> > --- a/include/sound/pcm.h
> > +++ b/include/sound/pcm.h
> > @@ -109,20 +109,23 @@ struct snd_pcm_ops {
> >  #define SNDRV_PCM_RATE_5512		(1U<<0)		/* 5512Hz */
> >  #define SNDRV_PCM_RATE_8000		(1U<<1)		/* 8000Hz */
> >  #define SNDRV_PCM_RATE_11025		(1U<<2)		/* 11025Hz */
> > -#define SNDRV_PCM_RATE_16000		(1U<<3)		/* 16000Hz */
> > -#define SNDRV_PCM_RATE_22050		(1U<<4)		/* 22050Hz */
> > -#define SNDRV_PCM_RATE_32000		(1U<<5)		/* 32000Hz */
> > -#define SNDRV_PCM_RATE_44100		(1U<<6)		/* 44100Hz */
> > -#define SNDRV_PCM_RATE_48000		(1U<<7)		/* 48000Hz */
> > -#define SNDRV_PCM_RATE_64000		(1U<<8)		/* 64000Hz */
> > -#define SNDRV_PCM_RATE_88200		(1U<<9)		/* 88200Hz */
> > -#define SNDRV_PCM_RATE_96000		(1U<<10)	/* 96000Hz */
> > -#define SNDRV_PCM_RATE_176400		(1U<<11)	/* 176400Hz */
> > -#define SNDRV_PCM_RATE_192000		(1U<<12)	/* 192000Hz */
> > -#define SNDRV_PCM_RATE_352800		(1U<<13)	/* 352800Hz */
> > -#define SNDRV_PCM_RATE_384000		(1U<<14)	/* 384000Hz */
> > -#define SNDRV_PCM_RATE_705600		(1U<<15)	/* 705600Hz */
> > -#define SNDRV_PCM_RATE_768000		(1U<<16)	/* 768000Hz */
> > +#define SNDRV_PCM_RATE_12000		(1U<<3)		/* 12000Hz */
> > +#define SNDRV_PCM_RATE_16000		(1U<<4)		/* 16000Hz */
> > +#define SNDRV_PCM_RATE_22050		(1U<<5)		/* 22050Hz */
> > +#define SNDRV_PCM_RATE_24000		(1U<<6)		/* 24000Hz */
> > +#define SNDRV_PCM_RATE_32000		(1U<<7)		/* 32000Hz */
> > +#define SNDRV_PCM_RATE_44100		(1U<<8)		/* 44100Hz */
> > +#define SNDRV_PCM_RATE_48000		(1U<<9)		/* 48000Hz */
> > +#define SNDRV_PCM_RATE_64000		(1U<<10)	/* 64000Hz */
> > +#define SNDRV_PCM_RATE_88200		(1U<<11)	/* 88200Hz */
> > +#define SNDRV_PCM_RATE_96000		(1U<<12)	/* 96000Hz */
> > +#define SNDRV_PCM_RATE_128000		(1U<<13)	/* 128000Hz */
> > +#define SNDRV_PCM_RATE_176400		(1U<<14)	/* 176400Hz */
> > +#define SNDRV_PCM_RATE_192000		(1U<<15)	/* 192000Hz */
> > +#define SNDRV_PCM_RATE_352800		(1U<<16)	/* 352800Hz */
> > +#define SNDRV_PCM_RATE_384000		(1U<<17)	/* 384000Hz */
> > +#define SNDRV_PCM_RATE_705600		(1U<<18)	/* 705600Hz */
> > +#define SNDRV_PCM_RATE_768000		(1U<<19)	/* 768000Hz */
> >  
> >  #define SNDRV_PCM_RATE_CONTINUOUS	(1U<<30)	/* continuous range */
> >  #define SNDRV_PCM_RATE_KNOT		(1U<<31)	/* supports more non-continuous rates */
> > diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> > index 44381514f695..7461a727615c 100644
> > --- a/sound/core/pcm_native.c
> > +++ b/sound/core/pcm_native.c
> > @@ -2418,13 +2418,13 @@ static int snd_pcm_hw_rule_sample_bits(struct snd_pcm_hw_params *params,
> >  	return snd_interval_refine(hw_param_interval(params, rule->var), &t);
> >  }
> >  
> > -#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_192000 != 1 << 12
> > +#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_768000 != 1 << 19
> >  #error "Change this table"
> >  #endif
> >  
> >  static const unsigned int rates[] = {
> > -	5512, 8000, 11025, 16000, 22050, 32000, 44100,
> > -	48000, 64000, 88200, 96000, 176400, 192000, 352800, 384000, 705600, 768000
> > +	5512, 8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000, 64000,
> > +	88200, 96000, 128000, 176400, 192000, 352800, 384000, 705600, 768000,
> >  };
> >  
> >  const struct snd_pcm_hw_constraint_list snd_pcm_known_rates = {
> 
> Wondering if this is backwards compatible with the alsa-lib definitions,
> specifically the topology parts which did unfortunately have a list of
> rates that will map to a different index now:
> 
> 
> typedef enum _snd_pcm_rates {
> 	SND_PCM_RATE_UNKNOWN = -1,
> 	SND_PCM_RATE_5512 = 0,
> 	SND_PCM_RATE_8000,
> 	SND_PCM_RATE_11025,
> 	SND_PCM_RATE_16000,
> 	SND_PCM_RATE_22050,
> 	SND_PCM_RATE_32000,
> 	SND_PCM_RATE_44100,
> 	SND_PCM_RATE_48000,
> 	SND_PCM_RATE_64000,
> 	SND_PCM_RATE_88200,
> 	SND_PCM_RATE_96000,
> 	SND_PCM_RATE_176400,
> 	SND_PCM_RATE_192000,
> 	SND_PCM_RATE_CONTINUOUS = 30,
> 	SND_PCM_RATE_KNOT = 31,
> 	SND_PCM_RATE_LAST = SND_PCM_RATE_KNOT,
> } snd_pcm_rates_t;

As far as I understand correctly, those rate bits used for topology
are independent from the bits used for PCM core, although it used to
be the same.  Maybe better to rename (such as SND_TPLG_RATE_*) so that
it's clearer only for topology stuff.

But it'd be better if anyone can double-check.


thanks,

Takashi

Re: [PATCH 01/13] ALSA: pcm: add more sample rate definitions

[Péter Ujfalusi]

On 11/09/2024 12:21, Takashi Iwai wrote:
>> Wondering if this is backwards compatible with the alsa-lib definitions,
>> specifically the topology parts which did unfortunately have a list of
>> rates that will map to a different index now:
>>
>>
>> typedef enum _snd_pcm_rates {
>> 	SND_PCM_RATE_UNKNOWN = -1,
>> 	SND_PCM_RATE_5512 = 0,
>> 	SND_PCM_RATE_8000,
>> 	SND_PCM_RATE_11025,
>> 	SND_PCM_RATE_16000,
>> 	SND_PCM_RATE_22050,
>> 	SND_PCM_RATE_32000,
>> 	SND_PCM_RATE_44100,
>> 	SND_PCM_RATE_48000,
>> 	SND_PCM_RATE_64000,
>> 	SND_PCM_RATE_88200,
>> 	SND_PCM_RATE_96000,
>> 	SND_PCM_RATE_176400,
>> 	SND_PCM_RATE_192000,
>> 	SND_PCM_RATE_CONTINUOUS = 30,
>> 	SND_PCM_RATE_KNOT = 31,
>> 	SND_PCM_RATE_LAST = SND_PCM_RATE_KNOT,
>> } snd_pcm_rates_t;
> 
> As far as I understand correctly, those rate bits used for topology
> are independent from the bits used for PCM core, although it used to
> be the same.  Maybe better to rename (such as SND_TPLG_RATE_*) so that
> it's clearer only for topology stuff.

Even if we rename these in alsa-lib we will need translation from
SND_TPLG_RATE_ to SND_PCM_RATE_ in kernel likely?

The topology files are out there and this is an ABI...

> But it'd be better if anyone can double-check.

Since the kernel just copies the rates bitfield, any rate above 11025
will be misaligned and result broken setup.

> 
> 
> thanks,
> 
> Takashi

-- 
Péter

Re: [PATCH 01/13] ALSA: pcm: add more sample rate definitions

[Takashi Iwai]

On Wed, 11 Sep 2024 11:21:41 +0200,
Takashi Iwai wrote:
> 
> On Wed, 11 Sep 2024 11:09:59 +0200,
> Pierre-Louis Bossart wrote:
> > 
> > 
> > 
> > On 9/5/24 16:12, Jerome Brunet wrote:
> > > This adds a sample rate definition for 12kHz, 24kHz and 128kHz.
> > > 
> > > Admittedly, just a few drivers are currently using these sample
> > > rates but there is enough of a recurrence to justify adding a definition
> > > for them and remove some custom rate constraint code while at it.
> > > 
> > > The new definitions are not added to the interval definitions, such as
> > > SNDRV_PCM_RATE_8000_44100, because it would silently add new supported
> > > rates to drivers that may or may not support them. For sure the drivers
> > > have not been tested for these new rates so it is better to leave them out
> > > of interval definitions.
> > > 
> > > That being said, the added rates are multiples of well know rates families,
> > > it is very likely that a lot of devices out there actually supports them.
> > > 
> > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > > ---
> > >  include/sound/pcm.h     | 31 +++++++++++++++++--------------
> > >  sound/core/pcm_native.c |  6 +++---
> > >  2 files changed, 20 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> > > index 732121b934fd..c993350975a9 100644
> > > --- a/include/sound/pcm.h
> > > +++ b/include/sound/pcm.h
> > > @@ -109,20 +109,23 @@ struct snd_pcm_ops {
> > >  #define SNDRV_PCM_RATE_5512		(1U<<0)		/* 5512Hz */
> > >  #define SNDRV_PCM_RATE_8000		(1U<<1)		/* 8000Hz */
> > >  #define SNDRV_PCM_RATE_11025		(1U<<2)		/* 11025Hz */
> > > -#define SNDRV_PCM_RATE_16000		(1U<<3)		/* 16000Hz */
> > > -#define SNDRV_PCM_RATE_22050		(1U<<4)		/* 22050Hz */
> > > -#define SNDRV_PCM_RATE_32000		(1U<<5)		/* 32000Hz */
> > > -#define SNDRV_PCM_RATE_44100		(1U<<6)		/* 44100Hz */
> > > -#define SNDRV_PCM_RATE_48000		(1U<<7)		/* 48000Hz */
> > > -#define SNDRV_PCM_RATE_64000		(1U<<8)		/* 64000Hz */
> > > -#define SNDRV_PCM_RATE_88200		(1U<<9)		/* 88200Hz */
> > > -#define SNDRV_PCM_RATE_96000		(1U<<10)	/* 96000Hz */
> > > -#define SNDRV_PCM_RATE_176400		(1U<<11)	/* 176400Hz */
> > > -#define SNDRV_PCM_RATE_192000		(1U<<12)	/* 192000Hz */
> > > -#define SNDRV_PCM_RATE_352800		(1U<<13)	/* 352800Hz */
> > > -#define SNDRV_PCM_RATE_384000		(1U<<14)	/* 384000Hz */
> > > -#define SNDRV_PCM_RATE_705600		(1U<<15)	/* 705600Hz */
> > > -#define SNDRV_PCM_RATE_768000		(1U<<16)	/* 768000Hz */
> > > +#define SNDRV_PCM_RATE_12000		(1U<<3)		/* 12000Hz */
> > > +#define SNDRV_PCM_RATE_16000		(1U<<4)		/* 16000Hz */
> > > +#define SNDRV_PCM_RATE_22050		(1U<<5)		/* 22050Hz */
> > > +#define SNDRV_PCM_RATE_24000		(1U<<6)		/* 24000Hz */
> > > +#define SNDRV_PCM_RATE_32000		(1U<<7)		/* 32000Hz */
> > > +#define SNDRV_PCM_RATE_44100		(1U<<8)		/* 44100Hz */
> > > +#define SNDRV_PCM_RATE_48000		(1U<<9)		/* 48000Hz */
> > > +#define SNDRV_PCM_RATE_64000		(1U<<10)	/* 64000Hz */
> > > +#define SNDRV_PCM_RATE_88200		(1U<<11)	/* 88200Hz */
> > > +#define SNDRV_PCM_RATE_96000		(1U<<12)	/* 96000Hz */
> > > +#define SNDRV_PCM_RATE_128000		(1U<<13)	/* 128000Hz */
> > > +#define SNDRV_PCM_RATE_176400		(1U<<14)	/* 176400Hz */
> > > +#define SNDRV_PCM_RATE_192000		(1U<<15)	/* 192000Hz */
> > > +#define SNDRV_PCM_RATE_352800		(1U<<16)	/* 352800Hz */
> > > +#define SNDRV_PCM_RATE_384000		(1U<<17)	/* 384000Hz */
> > > +#define SNDRV_PCM_RATE_705600		(1U<<18)	/* 705600Hz */
> > > +#define SNDRV_PCM_RATE_768000		(1U<<19)	/* 768000Hz */
> > >  
> > >  #define SNDRV_PCM_RATE_CONTINUOUS	(1U<<30)	/* continuous range */
> > >  #define SNDRV_PCM_RATE_KNOT		(1U<<31)	/* supports more non-continuous rates */
> > > diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> > > index 44381514f695..7461a727615c 100644
> > > --- a/sound/core/pcm_native.c
> > > +++ b/sound/core/pcm_native.c
> > > @@ -2418,13 +2418,13 @@ static int snd_pcm_hw_rule_sample_bits(struct snd_pcm_hw_params *params,
> > >  	return snd_interval_refine(hw_param_interval(params, rule->var), &t);
> > >  }
> > >  
> > > -#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_192000 != 1 << 12
> > > +#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_768000 != 1 << 19
> > >  #error "Change this table"
> > >  #endif
> > >  
> > >  static const unsigned int rates[] = {
> > > -	5512, 8000, 11025, 16000, 22050, 32000, 44100,
> > > -	48000, 64000, 88200, 96000, 176400, 192000, 352800, 384000, 705600, 768000
> > > +	5512, 8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000, 64000,
> > > +	88200, 96000, 128000, 176400, 192000, 352800, 384000, 705600, 768000,
> > >  };
> > >  
> > >  const struct snd_pcm_hw_constraint_list snd_pcm_known_rates = {
> > 
> > Wondering if this is backwards compatible with the alsa-lib definitions,
> > specifically the topology parts which did unfortunately have a list of
> > rates that will map to a different index now:
> > 
> > 
> > typedef enum _snd_pcm_rates {
> > 	SND_PCM_RATE_UNKNOWN = -1,
> > 	SND_PCM_RATE_5512 = 0,
> > 	SND_PCM_RATE_8000,
> > 	SND_PCM_RATE_11025,
> > 	SND_PCM_RATE_16000,
> > 	SND_PCM_RATE_22050,
> > 	SND_PCM_RATE_32000,
> > 	SND_PCM_RATE_44100,
> > 	SND_PCM_RATE_48000,
> > 	SND_PCM_RATE_64000,
> > 	SND_PCM_RATE_88200,
> > 	SND_PCM_RATE_96000,
> > 	SND_PCM_RATE_176400,
> > 	SND_PCM_RATE_192000,
> > 	SND_PCM_RATE_CONTINUOUS = 30,
> > 	SND_PCM_RATE_KNOT = 31,
> > 	SND_PCM_RATE_LAST = SND_PCM_RATE_KNOT,
> > } snd_pcm_rates_t;
> 
> As far as I understand correctly, those rate bits used for topology
> are independent from the bits used for PCM core, although it used to
> be the same.  Maybe better to rename (such as SND_TPLG_RATE_*) so that
> it's clearer only for topology stuff.
> 
> But it'd be better if anyone can double-check.

... and I double-check by myself and proved I was wrong :-<

In soc-topology.c, set_stream_info() takes the
snd_soc_pcm_stream.rates bits as is from the given topology data,
so the changes of the bits can break this.

The topology takes both rates and formats bits.  The formats are a
part of uapi, but the rates aren't.  We should move those into uapi,
if any...


Takashi

Re: [PATCH 01/13] ALSA: pcm: add more sample rate definitions

[Takashi Iwai]

On Wed, 11 Sep 2024 12:33:01 +0200,
Péter Ujfalusi wrote:
> 
> On 11/09/2024 12:21, Takashi Iwai wrote:
> >> Wondering if this is backwards compatible with the alsa-lib definitions,
> >> specifically the topology parts which did unfortunately have a list of
> >> rates that will map to a different index now:
> >>
> >>
> >> typedef enum _snd_pcm_rates {
> >> 	SND_PCM_RATE_UNKNOWN = -1,
> >> 	SND_PCM_RATE_5512 = 0,
> >> 	SND_PCM_RATE_8000,
> >> 	SND_PCM_RATE_11025,
> >> 	SND_PCM_RATE_16000,
> >> 	SND_PCM_RATE_22050,
> >> 	SND_PCM_RATE_32000,
> >> 	SND_PCM_RATE_44100,
> >> 	SND_PCM_RATE_48000,
> >> 	SND_PCM_RATE_64000,
> >> 	SND_PCM_RATE_88200,
> >> 	SND_PCM_RATE_96000,
> >> 	SND_PCM_RATE_176400,
> >> 	SND_PCM_RATE_192000,
> >> 	SND_PCM_RATE_CONTINUOUS = 30,
> >> 	SND_PCM_RATE_KNOT = 31,
> >> 	SND_PCM_RATE_LAST = SND_PCM_RATE_KNOT,
> >> } snd_pcm_rates_t;
> > 
> > As far as I understand correctly, those rate bits used for topology
> > are independent from the bits used for PCM core, although it used to
> > be the same.  Maybe better to rename (such as SND_TPLG_RATE_*) so that
> > it's clearer only for topology stuff.
> 
> Even if we rename these in alsa-lib we will need translation from
> SND_TPLG_RATE_ to SND_PCM_RATE_ in kernel likely?
> 
> The topology files are out there and this is an ABI...
> 
> > But it'd be better if anyone can double-check.
> 
> Since the kernel just copies the rates bitfield, any rate above 11025
> will be misaligned and result broken setup.

Yep, I noticed it now, too.

Below is the fix patch, totally untested.
It'd be appreciated if anyone can test it quickly.


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: pcm: Fix breakage of PCM rates used for topology

It turned out that the topology ABI takes the standard PCM rate bits
as is, and it means that the recent change of the PCM rate bits would
lead to the inconsistent rate values used for topology.

This patch reverts the original PCM rate bit definitions while adding
the new rates to the extended bits instead.  This needed the change of
snd_pcm_known_rates, too.  And this also required to fix the handling
in snd_pcm_hw_limit_rates() that blindly assumed that the list is
sorted while it became unsorted now.

Fixes: 090624b7dc83 ("ALSA: pcm: add more sample rate definitions")
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/pcm.h     | 35 ++++++++++++++++++-----------------
 sound/core/pcm_misc.c   | 18 ++++++++++--------
 sound/core/pcm_native.c | 10 +++++++---
 3 files changed, 35 insertions(+), 28 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index c993350975a9..824216799070 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -109,23 +109,24 @@ struct snd_pcm_ops {
 #define SNDRV_PCM_RATE_5512		(1U<<0)		/* 5512Hz */
 #define SNDRV_PCM_RATE_8000		(1U<<1)		/* 8000Hz */
 #define SNDRV_PCM_RATE_11025		(1U<<2)		/* 11025Hz */
-#define SNDRV_PCM_RATE_12000		(1U<<3)		/* 12000Hz */
-#define SNDRV_PCM_RATE_16000		(1U<<4)		/* 16000Hz */
-#define SNDRV_PCM_RATE_22050		(1U<<5)		/* 22050Hz */
-#define SNDRV_PCM_RATE_24000		(1U<<6)		/* 24000Hz */
-#define SNDRV_PCM_RATE_32000		(1U<<7)		/* 32000Hz */
-#define SNDRV_PCM_RATE_44100		(1U<<8)		/* 44100Hz */
-#define SNDRV_PCM_RATE_48000		(1U<<9)		/* 48000Hz */
-#define SNDRV_PCM_RATE_64000		(1U<<10)	/* 64000Hz */
-#define SNDRV_PCM_RATE_88200		(1U<<11)	/* 88200Hz */
-#define SNDRV_PCM_RATE_96000		(1U<<12)	/* 96000Hz */
-#define SNDRV_PCM_RATE_128000		(1U<<13)	/* 128000Hz */
-#define SNDRV_PCM_RATE_176400		(1U<<14)	/* 176400Hz */
-#define SNDRV_PCM_RATE_192000		(1U<<15)	/* 192000Hz */
-#define SNDRV_PCM_RATE_352800		(1U<<16)	/* 352800Hz */
-#define SNDRV_PCM_RATE_384000		(1U<<17)	/* 384000Hz */
-#define SNDRV_PCM_RATE_705600		(1U<<18)	/* 705600Hz */
-#define SNDRV_PCM_RATE_768000		(1U<<19)	/* 768000Hz */
+#define SNDRV_PCM_RATE_16000		(1U<<3)		/* 16000Hz */
+#define SNDRV_PCM_RATE_22050		(1U<<4)		/* 22050Hz */
+#define SNDRV_PCM_RATE_32000		(1U<<5)		/* 32000Hz */
+#define SNDRV_PCM_RATE_44100		(1U<<6)		/* 44100Hz */
+#define SNDRV_PCM_RATE_48000		(1U<<7)		/* 48000Hz */
+#define SNDRV_PCM_RATE_64000		(1U<<8)		/* 64000Hz */
+#define SNDRV_PCM_RATE_88200		(1U<<9)		/* 88200Hz */
+#define SNDRV_PCM_RATE_96000		(1U<<10)	/* 96000Hz */
+#define SNDRV_PCM_RATE_176400		(1U<<11)	/* 176400Hz */
+#define SNDRV_PCM_RATE_192000		(1U<<12)	/* 192000Hz */
+#define SNDRV_PCM_RATE_352800		(1U<<13)	/* 352800Hz */
+#define SNDRV_PCM_RATE_384000		(1U<<14)	/* 384000Hz */
+#define SNDRV_PCM_RATE_705600		(1U<<15)	/* 705600Hz */
+#define SNDRV_PCM_RATE_768000		(1U<<16)	/* 768000Hz */
+/* extended rates */
+#define SNDRV_PCM_RATE_12000		(1U<<17)	/* 12000Hz */
+#define SNDRV_PCM_RATE_24000		(1U<<18)	/* 24000Hz */
+#define SNDRV_PCM_RATE_128000		(1U<<19)	/* 128000Hz */
 
 #define SNDRV_PCM_RATE_CONTINUOUS	(1U<<30)	/* continuous range */
 #define SNDRV_PCM_RATE_KNOT		(1U<<31)	/* supports more non-continuous rates */
diff --git a/sound/core/pcm_misc.c b/sound/core/pcm_misc.c
index 5588b6a1ee8b..4f556211bb56 100644
--- a/sound/core/pcm_misc.c
+++ b/sound/core/pcm_misc.c
@@ -494,18 +494,20 @@ EXPORT_SYMBOL(snd_pcm_format_set_silence);
 int snd_pcm_hw_limit_rates(struct snd_pcm_hardware *hw)
 {
 	int i;
+	unsigned int rmin, rmax;
+
+	rmin = UINT_MAX;
+	rmax = 0;
 	for (i = 0; i < (int)snd_pcm_known_rates.count; i++) {
 		if (hw->rates & (1 << i)) {
-			hw->rate_min = snd_pcm_known_rates.list[i];
-			break;
-		}
-	}
-	for (i = (int)snd_pcm_known_rates.count - 1; i >= 0; i--) {
-		if (hw->rates & (1 << i)) {
-			hw->rate_max = snd_pcm_known_rates.list[i];
-			break;
+			rmin = min(rmin, snd_pcm_known_rates.list[i]);
+			rmax = max(rmax, snd_pcm_known_rates.list[i]);
 		}
 	}
+	if (rmin > rmax)
+		return -EINVAL;
+	hw->rate_min = rmin;
+	hw->rate_max = rmax;
 	return 0;
 }
 EXPORT_SYMBOL(snd_pcm_hw_limit_rates);
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 7461a727615c..5e1e6006707b 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -2418,13 +2418,17 @@ static int snd_pcm_hw_rule_sample_bits(struct snd_pcm_hw_params *params,
 	return snd_interval_refine(hw_param_interval(params, rule->var), &t);
 }
 
-#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_768000 != 1 << 19
+#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_192000 != 1 << 12 ||\
+	SNDRV_PCM_RATE_128000 != 1 << 19
 #error "Change this table"
 #endif
 
+/* NOTE: the list is unsorted! */
 static const unsigned int rates[] = {
-	5512, 8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000, 64000,
-	88200, 96000, 128000, 176400, 192000, 352800, 384000, 705600, 768000,
+	5512, 8000, 11025, 16000, 22050, 32000, 44100,
+	48000, 64000, 88200, 96000, 176400, 192000, 352800, 384000, 705600, 768000,
+	/* extended */
+	12000, 24000, 128000
 };
 
 const struct snd_pcm_hw_constraint_list snd_pcm_known_rates = {
-- 
2.43.0

Re: [PATCH 01/13] ALSA: pcm: add more sample rate definitions

[Jaroslav Kysela]

On 11. 09. 24 12:51, Takashi Iwai wrote:
> On Wed, 11 Sep 2024 12:33:01 +0200,
> Péter Ujfalusi wrote:
>>
>> On 11/09/2024 12:21, Takashi Iwai wrote:
>>>> Wondering if this is backwards compatible with the alsa-lib definitions,
>>>> specifically the topology parts which did unfortunately have a list of
>>>> rates that will map to a different index now:
>>>>
>>>>
>>>> typedef enum _snd_pcm_rates {
>>>> 	SND_PCM_RATE_UNKNOWN = -1,
>>>> 	SND_PCM_RATE_5512 = 0,
>>>> 	SND_PCM_RATE_8000,
>>>> 	SND_PCM_RATE_11025,
>>>> 	SND_PCM_RATE_16000,
>>>> 	SND_PCM_RATE_22050,
>>>> 	SND_PCM_RATE_32000,
>>>> 	SND_PCM_RATE_44100,
>>>> 	SND_PCM_RATE_48000,
>>>> 	SND_PCM_RATE_64000,
>>>> 	SND_PCM_RATE_88200,
>>>> 	SND_PCM_RATE_96000,
>>>> 	SND_PCM_RATE_176400,
>>>> 	SND_PCM_RATE_192000,
>>>> 	SND_PCM_RATE_CONTINUOUS = 30,
>>>> 	SND_PCM_RATE_KNOT = 31,
>>>> 	SND_PCM_RATE_LAST = SND_PCM_RATE_KNOT,
>>>> } snd_pcm_rates_t;
>>>
>>> As far as I understand correctly, those rate bits used for topology
>>> are independent from the bits used for PCM core, although it used to
>>> be the same.  Maybe better to rename (such as SND_TPLG_RATE_*) so that
>>> it's clearer only for topology stuff.
>>
>> Even if we rename these in alsa-lib we will need translation from
>> SND_TPLG_RATE_ to SND_PCM_RATE_ in kernel likely?
>>
>> The topology files are out there and this is an ABI...
>>
>>> But it'd be better if anyone can double-check.
>>
>> Since the kernel just copies the rates bitfield, any rate above 11025
>> will be misaligned and result broken setup.
> 
> Yep, I noticed it now, too.
> 
> Below is the fix patch, totally untested.
> It'd be appreciated if anyone can test it quickly.
> 
> 
> thanks,
> 
> Takashi
> 
> -- 8< --
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] ALSA: pcm: Fix breakage of PCM rates used for topology
> 
> It turned out that the topology ABI takes the standard PCM rate bits
> as is, and it means that the recent change of the PCM rate bits would
> lead to the inconsistent rate values used for topology.
> 
> This patch reverts the original PCM rate bit definitions while adding
> the new rates to the extended bits instead.  This needed the change of
> snd_pcm_known_rates, too.  And this also required to fix the handling
> in snd_pcm_hw_limit_rates() that blindly assumed that the list is
> sorted while it became unsorted now.
> 
> Fixes: 090624b7dc83 ("ALSA: pcm: add more sample rate definitions")
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

This looks fine. But the topology rate bits should not depend on those bits. 
It's a bit pitty that the standard PCM ABI does not use those bits for user 
space and we are doing this change just for topology ABI.

Reviewed-by: Jaroslav Kysela <perex@perex.cz>

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: [PATCH 01/13] ALSA: pcm: add more sample rate definitions

[Takashi Iwai]

On Wed, 11 Sep 2024 12:58:53 +0200,
Jaroslav Kysela wrote:
> 
> On 11. 09. 24 12:51, Takashi Iwai wrote:
> > On Wed, 11 Sep 2024 12:33:01 +0200,
> > Péter Ujfalusi wrote:
> >> 
> >> On 11/09/2024 12:21, Takashi Iwai wrote:
> >>>> Wondering if this is backwards compatible with the alsa-lib definitions,
> >>>> specifically the topology parts which did unfortunately have a list of
> >>>> rates that will map to a different index now:
> >>>> 
> >>>> 
> >>>> typedef enum _snd_pcm_rates {
> >>>> 	SND_PCM_RATE_UNKNOWN = -1,
> >>>> 	SND_PCM_RATE_5512 = 0,
> >>>> 	SND_PCM_RATE_8000,
> >>>> 	SND_PCM_RATE_11025,
> >>>> 	SND_PCM_RATE_16000,
> >>>> 	SND_PCM_RATE_22050,
> >>>> 	SND_PCM_RATE_32000,
> >>>> 	SND_PCM_RATE_44100,
> >>>> 	SND_PCM_RATE_48000,
> >>>> 	SND_PCM_RATE_64000,
> >>>> 	SND_PCM_RATE_88200,
> >>>> 	SND_PCM_RATE_96000,
> >>>> 	SND_PCM_RATE_176400,
> >>>> 	SND_PCM_RATE_192000,
> >>>> 	SND_PCM_RATE_CONTINUOUS = 30,
> >>>> 	SND_PCM_RATE_KNOT = 31,
> >>>> 	SND_PCM_RATE_LAST = SND_PCM_RATE_KNOT,
> >>>> } snd_pcm_rates_t;
> >>> 
> >>> As far as I understand correctly, those rate bits used for topology
> >>> are independent from the bits used for PCM core, although it used to
> >>> be the same.  Maybe better to rename (such as SND_TPLG_RATE_*) so that
> >>> it's clearer only for topology stuff.
> >> 
> >> Even if we rename these in alsa-lib we will need translation from
> >> SND_TPLG_RATE_ to SND_PCM_RATE_ in kernel likely?
> >> 
> >> The topology files are out there and this is an ABI...
> >> 
> >>> But it'd be better if anyone can double-check.
> >> 
> >> Since the kernel just copies the rates bitfield, any rate above 11025
> >> will be misaligned and result broken setup.
> > 
> > Yep, I noticed it now, too.
> > 
> > Below is the fix patch, totally untested.
> > It'd be appreciated if anyone can test it quickly.
> > 
> > 
> > thanks,
> > 
> > Takashi
> > 
> > -- 8< --
> > From: Takashi Iwai <tiwai@suse.de>
> > Subject: [PATCH] ALSA: pcm: Fix breakage of PCM rates used for topology
> > 
> > It turned out that the topology ABI takes the standard PCM rate bits
> > as is, and it means that the recent change of the PCM rate bits would
> > lead to the inconsistent rate values used for topology.
> > 
> > This patch reverts the original PCM rate bit definitions while adding
> > the new rates to the extended bits instead.  This needed the change of
> > snd_pcm_known_rates, too.  And this also required to fix the handling
> > in snd_pcm_hw_limit_rates() that blindly assumed that the list is
> > sorted while it became unsorted now.
> > 
> > Fixes: 090624b7dc83 ("ALSA: pcm: add more sample rate definitions")
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> 
> This looks fine. But the topology rate bits should not depend on those
> bits. It's a bit pitty that the standard PCM ABI does not use those
> bits for user space and we are doing this change just for topology
> ABI.

Yeah, and theoretically it's possible to fix in topology side, but
it'll be more cumbersome.

Although it's not really a part of PCM ABI, I believe we should move
the PCM rate bit definitions to uapi, for showing that it's set in
stone.  Something like below.


Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: pcm: Move standard rate bit definitions into uapi

Since the standard PCM rate bits are used for the topology ABI, it's a
part of public ABI that must not be changed.  Move the definitions
into uapi to indicate it more clearly.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/pcm.h         | 26 --------------------------
 include/uapi/sound/asound.h | 26 ++++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 824216799070..f28f6d6ac996 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -105,32 +105,6 @@ struct snd_pcm_ops {
 
 #define SNDRV_PCM_POS_XRUN		((snd_pcm_uframes_t)-1)
 
-/* If you change this don't forget to change rates[] table in pcm_native.c */
-#define SNDRV_PCM_RATE_5512		(1U<<0)		/* 5512Hz */
-#define SNDRV_PCM_RATE_8000		(1U<<1)		/* 8000Hz */
-#define SNDRV_PCM_RATE_11025		(1U<<2)		/* 11025Hz */
-#define SNDRV_PCM_RATE_16000		(1U<<3)		/* 16000Hz */
-#define SNDRV_PCM_RATE_22050		(1U<<4)		/* 22050Hz */
-#define SNDRV_PCM_RATE_32000		(1U<<5)		/* 32000Hz */
-#define SNDRV_PCM_RATE_44100		(1U<<6)		/* 44100Hz */
-#define SNDRV_PCM_RATE_48000		(1U<<7)		/* 48000Hz */
-#define SNDRV_PCM_RATE_64000		(1U<<8)		/* 64000Hz */
-#define SNDRV_PCM_RATE_88200		(1U<<9)		/* 88200Hz */
-#define SNDRV_PCM_RATE_96000		(1U<<10)	/* 96000Hz */
-#define SNDRV_PCM_RATE_176400		(1U<<11)	/* 176400Hz */
-#define SNDRV_PCM_RATE_192000		(1U<<12)	/* 192000Hz */
-#define SNDRV_PCM_RATE_352800		(1U<<13)	/* 352800Hz */
-#define SNDRV_PCM_RATE_384000		(1U<<14)	/* 384000Hz */
-#define SNDRV_PCM_RATE_705600		(1U<<15)	/* 705600Hz */
-#define SNDRV_PCM_RATE_768000		(1U<<16)	/* 768000Hz */
-/* extended rates */
-#define SNDRV_PCM_RATE_12000		(1U<<17)	/* 12000Hz */
-#define SNDRV_PCM_RATE_24000		(1U<<18)	/* 24000Hz */
-#define SNDRV_PCM_RATE_128000		(1U<<19)	/* 128000Hz */
-
-#define SNDRV_PCM_RATE_CONTINUOUS	(1U<<30)	/* continuous range */
-#define SNDRV_PCM_RATE_KNOT		(1U<<31)	/* supports more non-continuous rates */
-
 #define SNDRV_PCM_RATE_8000_44100	(SNDRV_PCM_RATE_8000|SNDRV_PCM_RATE_11025|\
 					 SNDRV_PCM_RATE_16000|SNDRV_PCM_RATE_22050|\
 					 SNDRV_PCM_RATE_32000|SNDRV_PCM_RATE_44100)
diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
index 4cd513215bcd..715ceb3eac7c 100644
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -272,6 +272,32 @@ typedef int __bitwise snd_pcm_subformat_t;
 #define	SNDRV_PCM_SUBFORMAT_MSBITS_24	((__force snd_pcm_subformat_t) 3)
 #define	SNDRV_PCM_SUBFORMAT_LAST	SNDRV_PCM_SUBFORMAT_MSBITS_24
 
+/* Standard rate bits */
+#define SNDRV_PCM_RATE_5512		(1U<<0)		/* 5512Hz */
+#define SNDRV_PCM_RATE_8000		(1U<<1)		/* 8000Hz */
+#define SNDRV_PCM_RATE_11025		(1U<<2)		/* 11025Hz */
+#define SNDRV_PCM_RATE_16000		(1U<<3)		/* 16000Hz */
+#define SNDRV_PCM_RATE_22050		(1U<<4)		/* 22050Hz */
+#define SNDRV_PCM_RATE_32000		(1U<<5)		/* 32000Hz */
+#define SNDRV_PCM_RATE_44100		(1U<<6)		/* 44100Hz */
+#define SNDRV_PCM_RATE_48000		(1U<<7)		/* 48000Hz */
+#define SNDRV_PCM_RATE_64000		(1U<<8)		/* 64000Hz */
+#define SNDRV_PCM_RATE_88200		(1U<<9)		/* 88200Hz */
+#define SNDRV_PCM_RATE_96000		(1U<<10)	/* 96000Hz */
+#define SNDRV_PCM_RATE_176400		(1U<<11)	/* 176400Hz */
+#define SNDRV_PCM_RATE_192000		(1U<<12)	/* 192000Hz */
+#define SNDRV_PCM_RATE_352800		(1U<<13)	/* 352800Hz */
+#define SNDRV_PCM_RATE_384000		(1U<<14)	/* 384000Hz */
+#define SNDRV_PCM_RATE_705600		(1U<<15)	/* 705600Hz */
+#define SNDRV_PCM_RATE_768000		(1U<<16)	/* 768000Hz */
+/* extended rates */
+#define SNDRV_PCM_RATE_12000		(1U<<17)	/* 12000Hz */
+#define SNDRV_PCM_RATE_24000		(1U<<18)	/* 24000Hz */
+#define SNDRV_PCM_RATE_128000		(1U<<19)	/* 128000Hz */
+
+#define SNDRV_PCM_RATE_CONTINUOUS	(1U<<30)	/* continuous range */
+#define SNDRV_PCM_RATE_KNOT		(1U<<31)	/* supports more non-continuous rates */
+
 #define SNDRV_PCM_INFO_MMAP		0x00000001	/* hardware supports mmap */
 #define SNDRV_PCM_INFO_MMAP_VALID	0x00000002	/* period data are valid during transfer */
 #define SNDRV_PCM_INFO_DOUBLE		0x00000004	/* Double buffering needed for PCM start/stop */
-- 
2.43.0

Re: [PATCH 01/13] ALSA: pcm: add more sample rate definitions

[Jerome Brunet]

On Wed 11 Sep 2024 at 12:51, Takashi Iwai <tiwai@suse.de> wrote:

> On Wed, 11 Sep 2024 12:33:01 +0200,
> Péter Ujfalusi wrote:
>> 
>> On 11/09/2024 12:21, Takashi Iwai wrote:
>> >> Wondering if this is backwards compatible with the alsa-lib definitions,
>> >> specifically the topology parts which did unfortunately have a list of
>> >> rates that will map to a different index now:
>> >>
>> >>
>> >> typedef enum _snd_pcm_rates {
>> >> 	SND_PCM_RATE_UNKNOWN = -1,
>> >> 	SND_PCM_RATE_5512 = 0,
>> >> 	SND_PCM_RATE_8000,
>> >> 	SND_PCM_RATE_11025,
>> >> 	SND_PCM_RATE_16000,
>> >> 	SND_PCM_RATE_22050,
>> >> 	SND_PCM_RATE_32000,
>> >> 	SND_PCM_RATE_44100,
>> >> 	SND_PCM_RATE_48000,
>> >> 	SND_PCM_RATE_64000,
>> >> 	SND_PCM_RATE_88200,
>> >> 	SND_PCM_RATE_96000,
>> >> 	SND_PCM_RATE_176400,
>> >> 	SND_PCM_RATE_192000,
>> >> 	SND_PCM_RATE_CONTINUOUS = 30,
>> >> 	SND_PCM_RATE_KNOT = 31,
>> >> 	SND_PCM_RATE_LAST = SND_PCM_RATE_KNOT,
>> >> } snd_pcm_rates_t;
>> > 
>> > As far as I understand correctly, those rate bits used for topology
>> > are independent from the bits used for PCM core, although it used to
>> > be the same.  Maybe better to rename (such as SND_TPLG_RATE_*) so that
>> > it's clearer only for topology stuff.
>> 
>> Even if we rename these in alsa-lib we will need translation from
>> SND_TPLG_RATE_ to SND_PCM_RATE_ in kernel likely?
>> 
>> The topology files are out there and this is an ABI...
>> 
>> > But it'd be better if anyone can double-check.
>> 
>> Since the kernel just copies the rates bitfield, any rate above 11025
>> will be misaligned and result broken setup.
>
> Yep, I noticed it now, too.
>
> Below is the fix patch, totally untested.
> It'd be appreciated if anyone can test it quickly.
>
>
> thanks,
>
> Takashi
>
> -- 8< --
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] ALSA: pcm: Fix breakage of PCM rates used for topology
>
> It turned out that the topology ABI takes the standard PCM rate bits
> as is, and it means that the recent change of the PCM rate bits would
> lead to the inconsistent rate values used for topology.
>
> This patch reverts the original PCM rate bit definitions while adding
> the new rates to the extended bits instead.  This needed the change of
> snd_pcm_known_rates, too.  And this also required to fix the handling
> in snd_pcm_hw_limit_rates() that blindly assumed that the list is
> sorted while it became unsorted now.
>
> Fixes: 090624b7dc83 ("ALSA: pcm: add more sample rate definitions")
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  include/sound/pcm.h     | 35 ++++++++++++++++++-----------------
>  sound/core/pcm_misc.c   | 18 ++++++++++--------
>  sound/core/pcm_native.c | 10 +++++++---
>  3 files changed, 35 insertions(+), 28 deletions(-)
>
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index c993350975a9..824216799070 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -109,23 +109,24 @@ struct snd_pcm_ops {
>  #define SNDRV_PCM_RATE_5512		(1U<<0)		/* 5512Hz */
>  #define SNDRV_PCM_RATE_8000		(1U<<1)		/* 8000Hz */
>  #define SNDRV_PCM_RATE_11025		(1U<<2)		/* 11025Hz */
> -#define SNDRV_PCM_RATE_12000		(1U<<3)		/* 12000Hz */
> -#define SNDRV_PCM_RATE_16000		(1U<<4)		/* 16000Hz */
> -#define SNDRV_PCM_RATE_22050		(1U<<5)		/* 22050Hz */
> -#define SNDRV_PCM_RATE_24000		(1U<<6)		/* 24000Hz */
> -#define SNDRV_PCM_RATE_32000		(1U<<7)		/* 32000Hz */
> -#define SNDRV_PCM_RATE_44100		(1U<<8)		/* 44100Hz */
> -#define SNDRV_PCM_RATE_48000		(1U<<9)		/* 48000Hz */
> -#define SNDRV_PCM_RATE_64000		(1U<<10)	/* 64000Hz */
> -#define SNDRV_PCM_RATE_88200		(1U<<11)	/* 88200Hz */
> -#define SNDRV_PCM_RATE_96000		(1U<<12)	/* 96000Hz */
> -#define SNDRV_PCM_RATE_128000		(1U<<13)	/* 128000Hz */
> -#define SNDRV_PCM_RATE_176400		(1U<<14)	/* 176400Hz */
> -#define SNDRV_PCM_RATE_192000		(1U<<15)	/* 192000Hz */
> -#define SNDRV_PCM_RATE_352800		(1U<<16)	/* 352800Hz */
> -#define SNDRV_PCM_RATE_384000		(1U<<17)	/* 384000Hz */
> -#define SNDRV_PCM_RATE_705600		(1U<<18)	/* 705600Hz */
> -#define SNDRV_PCM_RATE_768000		(1U<<19)	/* 768000Hz */
> +#define SNDRV_PCM_RATE_16000		(1U<<3)		/* 16000Hz */
> +#define SNDRV_PCM_RATE_22050		(1U<<4)		/* 22050Hz */
> +#define SNDRV_PCM_RATE_32000		(1U<<5)		/* 32000Hz */
> +#define SNDRV_PCM_RATE_44100		(1U<<6)		/* 44100Hz */
> +#define SNDRV_PCM_RATE_48000		(1U<<7)		/* 48000Hz */
> +#define SNDRV_PCM_RATE_64000		(1U<<8)		/* 64000Hz */
> +#define SNDRV_PCM_RATE_88200		(1U<<9)		/* 88200Hz */
> +#define SNDRV_PCM_RATE_96000		(1U<<10)	/* 96000Hz */
> +#define SNDRV_PCM_RATE_176400		(1U<<11)	/* 176400Hz */
> +#define SNDRV_PCM_RATE_192000		(1U<<12)	/* 192000Hz */
> +#define SNDRV_PCM_RATE_352800		(1U<<13)	/* 352800Hz */
> +#define SNDRV_PCM_RATE_384000		(1U<<14)	/* 384000Hz */
> +#define SNDRV_PCM_RATE_705600		(1U<<15)	/* 705600Hz */
> +#define SNDRV_PCM_RATE_768000		(1U<<16)	/* 768000Hz */
> +/* extended rates */
> +#define SNDRV_PCM_RATE_12000		(1U<<17)	/* 12000Hz */
> +#define SNDRV_PCM_RATE_24000		(1U<<18)	/* 24000Hz */
> +#define SNDRV_PCM_RATE_128000		(1U<<19)	/* 128000Hz */
>  
>  #define SNDRV_PCM_RATE_CONTINUOUS	(1U<<30)	/* continuous range */
>  #define SNDRV_PCM_RATE_KNOT		(1U<<31)	/* supports more non-continuous rates */
> diff --git a/sound/core/pcm_misc.c b/sound/core/pcm_misc.c
> index 5588b6a1ee8b..4f556211bb56 100644
> --- a/sound/core/pcm_misc.c
> +++ b/sound/core/pcm_misc.c
> @@ -494,18 +494,20 @@ EXPORT_SYMBOL(snd_pcm_format_set_silence);
>  int snd_pcm_hw_limit_rates(struct snd_pcm_hardware *hw)
>  {
>  	int i;
> +	unsigned int rmin, rmax;
> +
> +	rmin = UINT_MAX;
> +	rmax = 0;
>  	for (i = 0; i < (int)snd_pcm_known_rates.count; i++) {
>  		if (hw->rates & (1 << i)) {
> -			hw->rate_min = snd_pcm_known_rates.list[i];
> -			break;
> -		}
> -	}
> -	for (i = (int)snd_pcm_known_rates.count - 1; i >= 0; i--) {
> -		if (hw->rates & (1 << i)) {
> -			hw->rate_max = snd_pcm_known_rates.list[i];
> -			break;
> +			rmin = min(rmin, snd_pcm_known_rates.list[i]);
> +			rmax = max(rmax, snd_pcm_known_rates.list[i]);
>  		}
>  	}
> +	if (rmin > rmax)
> +		return -EINVAL;
> +	hw->rate_min = rmin;
> +	hw->rate_max = rmax;
>  	return 0;
>  }
>  EXPORT_SYMBOL(snd_pcm_hw_limit_rates);
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index 7461a727615c..5e1e6006707b 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -2418,13 +2418,17 @@ static int snd_pcm_hw_rule_sample_bits(struct snd_pcm_hw_params *params,
>  	return snd_interval_refine(hw_param_interval(params, rule->var), &t);
>  }
>  
> -#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_768000 != 1 << 19
> +#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_192000 != 1 << 12 ||\
> +	SNDRV_PCM_RATE_128000 != 1 << 19
>  #error "Change this table"
>  #endif
>  
> +/* NOTE: the list is unsorted! */
>  static const unsigned int rates[] = {
> -	5512, 8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000, 64000,
> -	88200, 96000, 128000, 176400, 192000, 352800, 384000, 705600, 768000,
> +	5512, 8000, 11025, 16000, 22050, 32000, 44100,
> +	48000, 64000, 88200, 96000, 176400, 192000, 352800, 384000, 705600, 768000,
> +	/* extended */
> +	12000, 24000, 128000
>  };
>  
>  const struct snd_pcm_hw_constraint_list snd_pcm_known_rates = {

I've quickly tested this version with a few rates (48, 128, 768k),
amlogic device.

Looks Ok.

Tested-by: Jerome Brunet <jbrunet@baylibre.com>

Can't say for topology.

-- 
Jerome

Re: [PATCH 01/13] ALSA: pcm: add more sample rate definitions

[Liao, Bard]

On 9/11/2024 6:51 PM, Takashi Iwai wrote:
> On Wed, 11 Sep 2024 12:33:01 +0200,
> Péter Ujfalusi wrote:
>> On 11/09/2024 12:21, Takashi Iwai wrote:
>>>> Wondering if this is backwards compatible with the alsa-lib definitions,
>>>> specifically the topology parts which did unfortunately have a list of
>>>> rates that will map to a different index now:
>>>>
>>>>
>>>> typedef enum _snd_pcm_rates {
>>>> 	SND_PCM_RATE_UNKNOWN = -1,
>>>> 	SND_PCM_RATE_5512 = 0,
>>>> 	SND_PCM_RATE_8000,
>>>> 	SND_PCM_RATE_11025,
>>>> 	SND_PCM_RATE_16000,
>>>> 	SND_PCM_RATE_22050,
>>>> 	SND_PCM_RATE_32000,
>>>> 	SND_PCM_RATE_44100,
>>>> 	SND_PCM_RATE_48000,
>>>> 	SND_PCM_RATE_64000,
>>>> 	SND_PCM_RATE_88200,
>>>> 	SND_PCM_RATE_96000,
>>>> 	SND_PCM_RATE_176400,
>>>> 	SND_PCM_RATE_192000,
>>>> 	SND_PCM_RATE_CONTINUOUS = 30,
>>>> 	SND_PCM_RATE_KNOT = 31,
>>>> 	SND_PCM_RATE_LAST = SND_PCM_RATE_KNOT,
>>>> } snd_pcm_rates_t;
>>> As far as I understand correctly, those rate bits used for topology
>>> are independent from the bits used for PCM core, although it used to
>>> be the same.  Maybe better to rename (such as SND_TPLG_RATE_*) so that
>>> it's clearer only for topology stuff.
>> Even if we rename these in alsa-lib we will need translation from
>> SND_TPLG_RATE_ to SND_PCM_RATE_ in kernel likely?
>>
>> The topology files are out there and this is an ABI...
>>
>>> But it'd be better if anyone can double-check.
>> Since the kernel just copies the rates bitfield, any rate above 11025
>> will be misaligned and result broken setup.
> Yep, I noticed it now, too.
>
> Below is the fix patch, totally untested.
> It'd be appreciated if anyone can test it quickly.
>
>
> thanks,
>
> Takashi


I tested it on my laptop and it fixed the issue.

Tested-by: Bard Liao <yung-chuan.liao@linux.intel.com>

> -- 8< --
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] ALSA: pcm: Fix breakage of PCM rates used for topology
>
> It turned out that the topology ABI takes the standard PCM rate bits
> as is, and it means that the recent change of the PCM rate bits would
> lead to the inconsistent rate values used for topology.
>
> This patch reverts the original PCM rate bit definitions while adding
> the new rates to the extended bits instead.  This needed the change of
> snd_pcm_known_rates, too.  And this also required to fix the handling
> in snd_pcm_hw_limit_rates() that blindly assumed that the list is
> sorted while it became unsorted now.
>
> Fixes: 090624b7dc83 ("ALSA: pcm: add more sample rate definitions")
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>   include/sound/pcm.h     | 35 ++++++++++++++++++-----------------
>   sound/core/pcm_misc.c   | 18 ++++++++++--------
>   sound/core/pcm_native.c | 10 +++++++---
>   3 files changed, 35 insertions(+), 28 deletions(-)
>
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index c993350975a9..824216799070 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -109,23 +109,24 @@ struct snd_pcm_ops {
>   #define SNDRV_PCM_RATE_5512		(1U<<0)		/* 5512Hz */
>   #define SNDRV_PCM_RATE_8000		(1U<<1)		/* 8000Hz */
>   #define SNDRV_PCM_RATE_11025		(1U<<2)		/* 11025Hz */
> -#define SNDRV_PCM_RATE_12000		(1U<<3)		/* 12000Hz */
> -#define SNDRV_PCM_RATE_16000		(1U<<4)		/* 16000Hz */
> -#define SNDRV_PCM_RATE_22050		(1U<<5)		/* 22050Hz */
> -#define SNDRV_PCM_RATE_24000		(1U<<6)		/* 24000Hz */
> -#define SNDRV_PCM_RATE_32000		(1U<<7)		/* 32000Hz */
> -#define SNDRV_PCM_RATE_44100		(1U<<8)		/* 44100Hz */
> -#define SNDRV_PCM_RATE_48000		(1U<<9)		/* 48000Hz */
> -#define SNDRV_PCM_RATE_64000		(1U<<10)	/* 64000Hz */
> -#define SNDRV_PCM_RATE_88200		(1U<<11)	/* 88200Hz */
> -#define SNDRV_PCM_RATE_96000		(1U<<12)	/* 96000Hz */
> -#define SNDRV_PCM_RATE_128000		(1U<<13)	/* 128000Hz */
> -#define SNDRV_PCM_RATE_176400		(1U<<14)	/* 176400Hz */
> -#define SNDRV_PCM_RATE_192000		(1U<<15)	/* 192000Hz */
> -#define SNDRV_PCM_RATE_352800		(1U<<16)	/* 352800Hz */
> -#define SNDRV_PCM_RATE_384000		(1U<<17)	/* 384000Hz */
> -#define SNDRV_PCM_RATE_705600		(1U<<18)	/* 705600Hz */
> -#define SNDRV_PCM_RATE_768000		(1U<<19)	/* 768000Hz */
> +#define SNDRV_PCM_RATE_16000		(1U<<3)		/* 16000Hz */
> +#define SNDRV_PCM_RATE_22050		(1U<<4)		/* 22050Hz */
> +#define SNDRV_PCM_RATE_32000		(1U<<5)		/* 32000Hz */
> +#define SNDRV_PCM_RATE_44100		(1U<<6)		/* 44100Hz */
> +#define SNDRV_PCM_RATE_48000		(1U<<7)		/* 48000Hz */
> +#define SNDRV_PCM_RATE_64000		(1U<<8)		/* 64000Hz */
> +#define SNDRV_PCM_RATE_88200		(1U<<9)		/* 88200Hz */
> +#define SNDRV_PCM_RATE_96000		(1U<<10)	/* 96000Hz */
> +#define SNDRV_PCM_RATE_176400		(1U<<11)	/* 176400Hz */
> +#define SNDRV_PCM_RATE_192000		(1U<<12)	/* 192000Hz */
> +#define SNDRV_PCM_RATE_352800		(1U<<13)	/* 352800Hz */
> +#define SNDRV_PCM_RATE_384000		(1U<<14)	/* 384000Hz */
> +#define SNDRV_PCM_RATE_705600		(1U<<15)	/* 705600Hz */
> +#define SNDRV_PCM_RATE_768000		(1U<<16)	/* 768000Hz */
> +/* extended rates */
> +#define SNDRV_PCM_RATE_12000		(1U<<17)	/* 12000Hz */
> +#define SNDRV_PCM_RATE_24000		(1U<<18)	/* 24000Hz */
> +#define SNDRV_PCM_RATE_128000		(1U<<19)	/* 128000Hz */
>   
>   #define SNDRV_PCM_RATE_CONTINUOUS	(1U<<30)	/* continuous range */
>   #define SNDRV_PCM_RATE_KNOT		(1U<<31)	/* supports more non-continuous rates */
> diff --git a/sound/core/pcm_misc.c b/sound/core/pcm_misc.c
> index 5588b6a1ee8b..4f556211bb56 100644
> --- a/sound/core/pcm_misc.c
> +++ b/sound/core/pcm_misc.c
> @@ -494,18 +494,20 @@ EXPORT_SYMBOL(snd_pcm_format_set_silence);
>   int snd_pcm_hw_limit_rates(struct snd_pcm_hardware *hw)
>   {
>   	int i;
> +	unsigned int rmin, rmax;
> +
> +	rmin = UINT_MAX;
> +	rmax = 0;
>   	for (i = 0; i < (int)snd_pcm_known_rates.count; i++) {
>   		if (hw->rates & (1 << i)) {
> -			hw->rate_min = snd_pcm_known_rates.list[i];
> -			break;
> -		}
> -	}
> -	for (i = (int)snd_pcm_known_rates.count - 1; i >= 0; i--) {
> -		if (hw->rates & (1 << i)) {
> -			hw->rate_max = snd_pcm_known_rates.list[i];
> -			break;
> +			rmin = min(rmin, snd_pcm_known_rates.list[i]);
> +			rmax = max(rmax, snd_pcm_known_rates.list[i]);
>   		}
>   	}
> +	if (rmin > rmax)
> +		return -EINVAL;
> +	hw->rate_min = rmin;
> +	hw->rate_max = rmax;
>   	return 0;
>   }
>   EXPORT_SYMBOL(snd_pcm_hw_limit_rates);
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index 7461a727615c..5e1e6006707b 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -2418,13 +2418,17 @@ static int snd_pcm_hw_rule_sample_bits(struct snd_pcm_hw_params *params,
>   	return snd_interval_refine(hw_param_interval(params, rule->var), &t);
>   }
>   
> -#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_768000 != 1 << 19
> +#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_192000 != 1 << 12 ||\
> +	SNDRV_PCM_RATE_128000 != 1 << 19
>   #error "Change this table"
>   #endif
>   
> +/* NOTE: the list is unsorted! */
>   static const unsigned int rates[] = {
> -	5512, 8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000, 64000,
> -	88200, 96000, 128000, 176400, 192000, 352800, 384000, 705600, 768000,
> +	5512, 8000, 11025, 16000, 22050, 32000, 44100,
> +	48000, 64000, 88200, 96000, 176400, 192000, 352800, 384000, 705600, 768000,
> +	/* extended */
> +	12000, 24000, 128000
>   };
>   
>   const struct snd_pcm_hw_constraint_list snd_pcm_known_rates = {

Re: [PATCH 01/13] ALSA: pcm: add more sample rate definitions

[Jerome Brunet]

On Wed 11 Sep 2024 at 14:42, Takashi Iwai <tiwai@suse.de> wrote:

> On Wed, 11 Sep 2024 12:58:53 +0200,
> Jaroslav Kysela wrote:
>> 
>> On 11. 09. 24 12:51, Takashi Iwai wrote:
>> > On Wed, 11 Sep 2024 12:33:01 +0200,
>> > Péter Ujfalusi wrote:
>> >> 
>> >> On 11/09/2024 12:21, Takashi Iwai wrote:
>> >>>> Wondering if this is backwards compatible with the alsa-lib definitions,
>> >>>> specifically the topology parts which did unfortunately have a list of
>> >>>> rates that will map to a different index now:
>> >>>> 
>> >>>> 
>> >>>> typedef enum _snd_pcm_rates {
>> >>>> 	SND_PCM_RATE_UNKNOWN = -1,
>> >>>> 	SND_PCM_RATE_5512 = 0,
>> >>>> 	SND_PCM_RATE_8000,
>> >>>> 	SND_PCM_RATE_11025,
>> >>>> 	SND_PCM_RATE_16000,
>> >>>> 	SND_PCM_RATE_22050,
>> >>>> 	SND_PCM_RATE_32000,
>> >>>> 	SND_PCM_RATE_44100,
>> >>>> 	SND_PCM_RATE_48000,
>> >>>> 	SND_PCM_RATE_64000,
>> >>>> 	SND_PCM_RATE_88200,
>> >>>> 	SND_PCM_RATE_96000,
>> >>>> 	SND_PCM_RATE_176400,
>> >>>> 	SND_PCM_RATE_192000,
>> >>>> 	SND_PCM_RATE_CONTINUOUS = 30,
>> >>>> 	SND_PCM_RATE_KNOT = 31,
>> >>>> 	SND_PCM_RATE_LAST = SND_PCM_RATE_KNOT,
>> >>>> } snd_pcm_rates_t;
>> >>> 
>> >>> As far as I understand correctly, those rate bits used for topology
>> >>> are independent from the bits used for PCM core, although it used to
>> >>> be the same.  Maybe better to rename (such as SND_TPLG_RATE_*) so that
>> >>> it's clearer only for topology stuff.
>> >> 
>> >> Even if we rename these in alsa-lib we will need translation from
>> >> SND_TPLG_RATE_ to SND_PCM_RATE_ in kernel likely?
>> >> 
>> >> The topology files are out there and this is an ABI...
>> >> 
>> >>> But it'd be better if anyone can double-check.
>> >> 
>> >> Since the kernel just copies the rates bitfield, any rate above 11025
>> >> will be misaligned and result broken setup.
>> > 
>> > Yep, I noticed it now, too.
>> > 
>> > Below is the fix patch, totally untested.
>> > It'd be appreciated if anyone can test it quickly.
>> > 
>> > 
>> > thanks,
>> > 
>> > Takashi
>> > 
>> > -- 8< --
>> > From: Takashi Iwai <tiwai@suse.de>
>> > Subject: [PATCH] ALSA: pcm: Fix breakage of PCM rates used for topology
>> > 
>> > It turned out that the topology ABI takes the standard PCM rate bits
>> > as is, and it means that the recent change of the PCM rate bits would
>> > lead to the inconsistent rate values used for topology.
>> > 
>> > This patch reverts the original PCM rate bit definitions while adding
>> > the new rates to the extended bits instead.  This needed the change of
>> > snd_pcm_known_rates, too.  And this also required to fix the handling
>> > in snd_pcm_hw_limit_rates() that blindly assumed that the list is
>> > sorted while it became unsorted now.
>> > 
>> > Fixes: 090624b7dc83 ("ALSA: pcm: add more sample rate definitions")
>> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
>> 
>> This looks fine. But the topology rate bits should not depend on those
>> bits. It's a bit pitty that the standard PCM ABI does not use those
>> bits for user space and we are doing this change just for topology
>> ABI.
>
> Yeah, and theoretically it's possible to fix in topology side, but
> it'll be more cumbersome.
>
> Although it's not really a part of PCM ABI, I believe we should move
> the PCM rate bit definitions to uapi, for showing that it's set in
> stone.  Something like below.
>
>
> Takashi
>
> -- 8< --
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] ALSA: pcm: Move standard rate bit definitions into uapi
>
> Since the standard PCM rate bits are used for the topology ABI, it's a
> part of public ABI that must not be changed.  Move the definitions
> into uapi to indicate it more clearly.
>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  include/sound/pcm.h         | 26 --------------------------
>  include/uapi/sound/asound.h | 26 ++++++++++++++++++++++++++
>  2 files changed, 26 insertions(+), 26 deletions(-)
>
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index 824216799070..f28f6d6ac996 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -105,32 +105,6 @@ struct snd_pcm_ops {
>  
>  #define SNDRV_PCM_POS_XRUN		((snd_pcm_uframes_t)-1)
>  
> -/* If you change this don't forget to change rates[] table in pcm_native.c */
> -#define SNDRV_PCM_RATE_5512		(1U<<0)		/* 5512Hz */
> -#define SNDRV_PCM_RATE_8000		(1U<<1)		/* 8000Hz */
> -#define SNDRV_PCM_RATE_11025		(1U<<2)		/* 11025Hz */
> -#define SNDRV_PCM_RATE_16000		(1U<<3)		/* 16000Hz */
> -#define SNDRV_PCM_RATE_22050		(1U<<4)		/* 22050Hz */
> -#define SNDRV_PCM_RATE_32000		(1U<<5)		/* 32000Hz */
> -#define SNDRV_PCM_RATE_44100		(1U<<6)		/* 44100Hz */
> -#define SNDRV_PCM_RATE_48000		(1U<<7)		/* 48000Hz */
> -#define SNDRV_PCM_RATE_64000		(1U<<8)		/* 64000Hz */
> -#define SNDRV_PCM_RATE_88200		(1U<<9)		/* 88200Hz */
> -#define SNDRV_PCM_RATE_96000		(1U<<10)	/* 96000Hz */
> -#define SNDRV_PCM_RATE_176400		(1U<<11)	/* 176400Hz */
> -#define SNDRV_PCM_RATE_192000		(1U<<12)	/* 192000Hz */
> -#define SNDRV_PCM_RATE_352800		(1U<<13)	/* 352800Hz */
> -#define SNDRV_PCM_RATE_384000		(1U<<14)	/* 384000Hz */
> -#define SNDRV_PCM_RATE_705600		(1U<<15)	/* 705600Hz */
> -#define SNDRV_PCM_RATE_768000		(1U<<16)	/* 768000Hz */
> -/* extended rates */

It is cosmetic but I wonder, does the extended really start here ?

From the table Pierre-Louis sent, I suppose that all the recently added rates could
been seen as extended too (352.8 to 768). Those did not mess with the
order though 

> -#define SNDRV_PCM_RATE_12000		(1U<<17)	/* 12000Hz */
> -#define SNDRV_PCM_RATE_24000		(1U<<18)	/* 24000Hz */
> -#define SNDRV_PCM_RATE_128000		(1U<<19)	/* 128000Hz */
> -
> -#define SNDRV_PCM_RATE_CONTINUOUS	(1U<<30)	/* continuous range */
> -#define SNDRV_PCM_RATE_KNOT		(1U<<31)	/* supports more non-continuous rates */
> -
>  #define SNDRV_PCM_RATE_8000_44100	(SNDRV_PCM_RATE_8000|SNDRV_PCM_RATE_11025|\
>  					 SNDRV_PCM_RATE_16000|SNDRV_PCM_RATE_22050|\
>  					 SNDRV_PCM_RATE_32000|SNDRV_PCM_RATE_44100)
> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
> index 4cd513215bcd..715ceb3eac7c 100644
> --- a/include/uapi/sound/asound.h
> +++ b/include/uapi/sound/asound.h
> @@ -272,6 +272,32 @@ typedef int __bitwise snd_pcm_subformat_t;
>  #define	SNDRV_PCM_SUBFORMAT_MSBITS_24	((__force snd_pcm_subformat_t) 3)
>  #define	SNDRV_PCM_SUBFORMAT_LAST	SNDRV_PCM_SUBFORMAT_MSBITS_24
>  
> +/* Standard rate bits */
> +#define SNDRV_PCM_RATE_5512		(1U<<0)		/* 5512Hz */
> +#define SNDRV_PCM_RATE_8000		(1U<<1)		/* 8000Hz */
> +#define SNDRV_PCM_RATE_11025		(1U<<2)		/* 11025Hz */
> +#define SNDRV_PCM_RATE_16000		(1U<<3)		/* 16000Hz */
> +#define SNDRV_PCM_RATE_22050		(1U<<4)		/* 22050Hz */
> +#define SNDRV_PCM_RATE_32000		(1U<<5)		/* 32000Hz */
> +#define SNDRV_PCM_RATE_44100		(1U<<6)		/* 44100Hz */
> +#define SNDRV_PCM_RATE_48000		(1U<<7)		/* 48000Hz */
> +#define SNDRV_PCM_RATE_64000		(1U<<8)		/* 64000Hz */
> +#define SNDRV_PCM_RATE_88200		(1U<<9)		/* 88200Hz */
> +#define SNDRV_PCM_RATE_96000		(1U<<10)	/* 96000Hz */
> +#define SNDRV_PCM_RATE_176400		(1U<<11)	/* 176400Hz */
> +#define SNDRV_PCM_RATE_192000		(1U<<12)	/* 192000Hz */
> +#define SNDRV_PCM_RATE_352800		(1U<<13)	/* 352800Hz */
> +#define SNDRV_PCM_RATE_384000		(1U<<14)	/* 384000Hz */
> +#define SNDRV_PCM_RATE_705600		(1U<<15)	/* 705600Hz */
> +#define SNDRV_PCM_RATE_768000		(1U<<16)	/* 768000Hz */
> +/* extended rates */
> +#define SNDRV_PCM_RATE_12000		(1U<<17)	/* 12000Hz */
> +#define SNDRV_PCM_RATE_24000		(1U<<18)	/* 24000Hz */
> +#define SNDRV_PCM_RATE_128000		(1U<<19)	/* 128000Hz */
> +
> +#define SNDRV_PCM_RATE_CONTINUOUS	(1U<<30)	/* continuous range */
> +#define SNDRV_PCM_RATE_KNOT		(1U<<31)	/* supports more non-continuous rates */
> +
>  #define SNDRV_PCM_INFO_MMAP		0x00000001	/* hardware supports mmap */
>  #define SNDRV_PCM_INFO_MMAP_VALID	0x00000002	/* period data are valid during transfer */
>  #define SNDRV_PCM_INFO_DOUBLE		0x00000004	/* Double buffering needed for PCM start/stop */

-- 
Jerome

Re: [PATCH 01/13] ALSA: pcm: add more sample rate definitions

[Takashi Iwai]

On Wed, 11 Sep 2024 14:59:39 +0200,
Jerome Brunet wrote:
> 
> On Wed 11 Sep 2024 at 14:42, Takashi Iwai <tiwai@suse.de> wrote:
> 
> > On Wed, 11 Sep 2024 12:58:53 +0200,
> > Jaroslav Kysela wrote:
> >> 
> >> On 11. 09. 24 12:51, Takashi Iwai wrote:
> >> > On Wed, 11 Sep 2024 12:33:01 +0200,
> >> > Péter Ujfalusi wrote:
> >> >> 
> >> >> On 11/09/2024 12:21, Takashi Iwai wrote:
> >> >>>> Wondering if this is backwards compatible with the alsa-lib definitions,
> >> >>>> specifically the topology parts which did unfortunately have a list of
> >> >>>> rates that will map to a different index now:
> >> >>>> 
> >> >>>> 
> >> >>>> typedef enum _snd_pcm_rates {
> >> >>>> 	SND_PCM_RATE_UNKNOWN = -1,
> >> >>>> 	SND_PCM_RATE_5512 = 0,
> >> >>>> 	SND_PCM_RATE_8000,
> >> >>>> 	SND_PCM_RATE_11025,
> >> >>>> 	SND_PCM_RATE_16000,
> >> >>>> 	SND_PCM_RATE_22050,
> >> >>>> 	SND_PCM_RATE_32000,
> >> >>>> 	SND_PCM_RATE_44100,
> >> >>>> 	SND_PCM_RATE_48000,
> >> >>>> 	SND_PCM_RATE_64000,
> >> >>>> 	SND_PCM_RATE_88200,
> >> >>>> 	SND_PCM_RATE_96000,
> >> >>>> 	SND_PCM_RATE_176400,
> >> >>>> 	SND_PCM_RATE_192000,
> >> >>>> 	SND_PCM_RATE_CONTINUOUS = 30,
> >> >>>> 	SND_PCM_RATE_KNOT = 31,
> >> >>>> 	SND_PCM_RATE_LAST = SND_PCM_RATE_KNOT,
> >> >>>> } snd_pcm_rates_t;
> >> >>> 
> >> >>> As far as I understand correctly, those rate bits used for topology
> >> >>> are independent from the bits used for PCM core, although it used to
> >> >>> be the same.  Maybe better to rename (such as SND_TPLG_RATE_*) so that
> >> >>> it's clearer only for topology stuff.
> >> >> 
> >> >> Even if we rename these in alsa-lib we will need translation from
> >> >> SND_TPLG_RATE_ to SND_PCM_RATE_ in kernel likely?
> >> >> 
> >> >> The topology files are out there and this is an ABI...
> >> >> 
> >> >>> But it'd be better if anyone can double-check.
> >> >> 
> >> >> Since the kernel just copies the rates bitfield, any rate above 11025
> >> >> will be misaligned and result broken setup.
> >> > 
> >> > Yep, I noticed it now, too.
> >> > 
> >> > Below is the fix patch, totally untested.
> >> > It'd be appreciated if anyone can test it quickly.
> >> > 
> >> > 
> >> > thanks,
> >> > 
> >> > Takashi
> >> > 
> >> > -- 8< --
> >> > From: Takashi Iwai <tiwai@suse.de>
> >> > Subject: [PATCH] ALSA: pcm: Fix breakage of PCM rates used for topology
> >> > 
> >> > It turned out that the topology ABI takes the standard PCM rate bits
> >> > as is, and it means that the recent change of the PCM rate bits would
> >> > lead to the inconsistent rate values used for topology.
> >> > 
> >> > This patch reverts the original PCM rate bit definitions while adding
> >> > the new rates to the extended bits instead.  This needed the change of
> >> > snd_pcm_known_rates, too.  And this also required to fix the handling
> >> > in snd_pcm_hw_limit_rates() that blindly assumed that the list is
> >> > sorted while it became unsorted now.
> >> > 
> >> > Fixes: 090624b7dc83 ("ALSA: pcm: add more sample rate definitions")
> >> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> >> 
> >> This looks fine. But the topology rate bits should not depend on those
> >> bits. It's a bit pitty that the standard PCM ABI does not use those
> >> bits for user space and we are doing this change just for topology
> >> ABI.
> >
> > Yeah, and theoretically it's possible to fix in topology side, but
> > it'll be more cumbersome.
> >
> > Although it's not really a part of PCM ABI, I believe we should move
> > the PCM rate bit definitions to uapi, for showing that it's set in
> > stone.  Something like below.
> >
> >
> > Takashi
> >
> > -- 8< --
> > From: Takashi Iwai <tiwai@suse.de>
> > Subject: [PATCH] ALSA: pcm: Move standard rate bit definitions into uapi
> >
> > Since the standard PCM rate bits are used for the topology ABI, it's a
> > part of public ABI that must not be changed.  Move the definitions
> > into uapi to indicate it more clearly.
> >
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  include/sound/pcm.h         | 26 --------------------------
> >  include/uapi/sound/asound.h | 26 ++++++++++++++++++++++++++
> >  2 files changed, 26 insertions(+), 26 deletions(-)
> >
> > diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> > index 824216799070..f28f6d6ac996 100644
> > --- a/include/sound/pcm.h
> > +++ b/include/sound/pcm.h
> > @@ -105,32 +105,6 @@ struct snd_pcm_ops {
> >  
> >  #define SNDRV_PCM_POS_XRUN		((snd_pcm_uframes_t)-1)
> >  
> > -/* If you change this don't forget to change rates[] table in pcm_native.c */
> > -#define SNDRV_PCM_RATE_5512		(1U<<0)		/* 5512Hz */
> > -#define SNDRV_PCM_RATE_8000		(1U<<1)		/* 8000Hz */
> > -#define SNDRV_PCM_RATE_11025		(1U<<2)		/* 11025Hz */
> > -#define SNDRV_PCM_RATE_16000		(1U<<3)		/* 16000Hz */
> > -#define SNDRV_PCM_RATE_22050		(1U<<4)		/* 22050Hz */
> > -#define SNDRV_PCM_RATE_32000		(1U<<5)		/* 32000Hz */
> > -#define SNDRV_PCM_RATE_44100		(1U<<6)		/* 44100Hz */
> > -#define SNDRV_PCM_RATE_48000		(1U<<7)		/* 48000Hz */
> > -#define SNDRV_PCM_RATE_64000		(1U<<8)		/* 64000Hz */
> > -#define SNDRV_PCM_RATE_88200		(1U<<9)		/* 88200Hz */
> > -#define SNDRV_PCM_RATE_96000		(1U<<10)	/* 96000Hz */
> > -#define SNDRV_PCM_RATE_176400		(1U<<11)	/* 176400Hz */
> > -#define SNDRV_PCM_RATE_192000		(1U<<12)	/* 192000Hz */
> > -#define SNDRV_PCM_RATE_352800		(1U<<13)	/* 352800Hz */
> > -#define SNDRV_PCM_RATE_384000		(1U<<14)	/* 384000Hz */
> > -#define SNDRV_PCM_RATE_705600		(1U<<15)	/* 705600Hz */
> > -#define SNDRV_PCM_RATE_768000		(1U<<16)	/* 768000Hz */
> > -/* extended rates */
> 
> It is cosmetic but I wonder, does the extended really start here ?

Maybe a bad choice of the words.  This was rather meant as the
extension since 6.12.  So I'll replace it with "extended rates since
6.12", to be clearer.

> From the table Pierre-Louis sent, I suppose that all the recently added rates could
> been seen as extended too (352.8 to 768). Those did not mess with the
> order though 

AFAIU, the topology stuff seems supporting only up to 192kHz for now,
but it's a matter of topology-only; the limitation could be commented
in somewhere in topology's headers, but it's basically independent
from the definitions in pcm.h.


thanks,

Takashi

Re: [PATCH 01/13] ALSA: pcm: add more sample rate definitions

[Amadeusz Sławiński]

On 9/11/2024 2:42 PM, Takashi Iwai wrote:
> On Wed, 11 Sep 2024 12:58:53 +0200,
> Jaroslav Kysela wrote:
>>
>> On 11. 09. 24 12:51, Takashi Iwai wrote:
>>> On Wed, 11 Sep 2024 12:33:01 +0200,
>>> Péter Ujfalusi wrote:
>>>>
>>>> On 11/09/2024 12:21, Takashi Iwai wrote:
>>>>>> Wondering if this is backwards compatible with the alsa-lib definitions,
>>>>>> specifically the topology parts which did unfortunately have a list of
>>>>>> rates that will map to a different index now:
>>>>>>
>>>>>>
>>>>>> typedef enum _snd_pcm_rates {
>>>>>> 	SND_PCM_RATE_UNKNOWN = -1,
>>>>>> 	SND_PCM_RATE_5512 = 0,
>>>>>> 	SND_PCM_RATE_8000,
>>>>>> 	SND_PCM_RATE_11025,
>>>>>> 	SND_PCM_RATE_16000,
>>>>>> 	SND_PCM_RATE_22050,
>>>>>> 	SND_PCM_RATE_32000,
>>>>>> 	SND_PCM_RATE_44100,
>>>>>> 	SND_PCM_RATE_48000,
>>>>>> 	SND_PCM_RATE_64000,
>>>>>> 	SND_PCM_RATE_88200,
>>>>>> 	SND_PCM_RATE_96000,
>>>>>> 	SND_PCM_RATE_176400,
>>>>>> 	SND_PCM_RATE_192000,
>>>>>> 	SND_PCM_RATE_CONTINUOUS = 30,
>>>>>> 	SND_PCM_RATE_KNOT = 31,
>>>>>> 	SND_PCM_RATE_LAST = SND_PCM_RATE_KNOT,
>>>>>> } snd_pcm_rates_t;
>>>>>
>>>>> As far as I understand correctly, those rate bits used for topology
>>>>> are independent from the bits used for PCM core, although it used to
>>>>> be the same.  Maybe better to rename (such as SND_TPLG_RATE_*) so that
>>>>> it's clearer only for topology stuff.
>>>>
>>>> Even if we rename these in alsa-lib we will need translation from
>>>> SND_TPLG_RATE_ to SND_PCM_RATE_ in kernel likely?
>>>>
>>>> The topology files are out there and this is an ABI...
>>>>
>>>>> But it'd be better if anyone can double-check.
>>>>
>>>> Since the kernel just copies the rates bitfield, any rate above 11025
>>>> will be misaligned and result broken setup.
>>>
>>> Yep, I noticed it now, too.
>>>
>>> Below is the fix patch, totally untested.
>>> It'd be appreciated if anyone can test it quickly.
>>>
>>>
>>> thanks,
>>>
>>> Takashi
>>>
>>> -- 8< --
>>> From: Takashi Iwai <tiwai@suse.de>
>>> Subject: [PATCH] ALSA: pcm: Fix breakage of PCM rates used for topology
>>>
>>> It turned out that the topology ABI takes the standard PCM rate bits
>>> as is, and it means that the recent change of the PCM rate bits would
>>> lead to the inconsistent rate values used for topology.
>>>
>>> This patch reverts the original PCM rate bit definitions while adding
>>> the new rates to the extended bits instead.  This needed the change of
>>> snd_pcm_known_rates, too.  And this also required to fix the handling
>>> in snd_pcm_hw_limit_rates() that blindly assumed that the list is
>>> sorted while it became unsorted now.
>>>
>>> Fixes: 090624b7dc83 ("ALSA: pcm: add more sample rate definitions")
>>> Signed-off-by: Takashi Iwai <tiwai@suse.de>
>>
>> This looks fine. But the topology rate bits should not depend on those
>> bits. It's a bit pitty that the standard PCM ABI does not use those
>> bits for user space and we are doing this change just for topology
>> ABI.
> 
> Yeah, and theoretically it's possible to fix in topology side, but
> it'll be more cumbersome.
> 
> Although it's not really a part of PCM ABI, I believe we should move
> the PCM rate bit definitions to uapi, for showing that it's set in
> stone.  Something like below.
> 
> 
> Takashi
> 
> -- 8< --
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] ALSA: pcm: Move standard rate bit definitions into uapi
> 
> Since the standard PCM rate bits are used for the topology ABI, it's a
> part of public ABI that must not be changed.  Move the definitions
> into uapi to indicate it more clearly.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>   include/sound/pcm.h         | 26 --------------------------
>   include/uapi/sound/asound.h | 26 ++++++++++++++++++++++++++
>   2 files changed, 26 insertions(+), 26 deletions(-)
> 
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index 824216799070..f28f6d6ac996 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -105,32 +105,6 @@ struct snd_pcm_ops {
>   
>   #define SNDRV_PCM_POS_XRUN		((snd_pcm_uframes_t)-1)
>   
> -/* If you change this don't forget to change rates[] table in pcm_native.c */
> -#define SNDRV_PCM_RATE_5512		(1U<<0)		/* 5512Hz */
> -#define SNDRV_PCM_RATE_8000		(1U<<1)		/* 8000Hz */
> -#define SNDRV_PCM_RATE_11025		(1U<<2)		/* 11025Hz */
> -#define SNDRV_PCM_RATE_16000		(1U<<3)		/* 16000Hz */
> -#define SNDRV_PCM_RATE_22050		(1U<<4)		/* 22050Hz */
> -#define SNDRV_PCM_RATE_32000		(1U<<5)		/* 32000Hz */
> -#define SNDRV_PCM_RATE_44100		(1U<<6)		/* 44100Hz */
> -#define SNDRV_PCM_RATE_48000		(1U<<7)		/* 48000Hz */
> -#define SNDRV_PCM_RATE_64000		(1U<<8)		/* 64000Hz */
> -#define SNDRV_PCM_RATE_88200		(1U<<9)		/* 88200Hz */
> -#define SNDRV_PCM_RATE_96000		(1U<<10)	/* 96000Hz */
> -#define SNDRV_PCM_RATE_176400		(1U<<11)	/* 176400Hz */
> -#define SNDRV_PCM_RATE_192000		(1U<<12)	/* 192000Hz */
> -#define SNDRV_PCM_RATE_352800		(1U<<13)	/* 352800Hz */
> -#define SNDRV_PCM_RATE_384000		(1U<<14)	/* 384000Hz */
> -#define SNDRV_PCM_RATE_705600		(1U<<15)	/* 705600Hz */
> -#define SNDRV_PCM_RATE_768000		(1U<<16)	/* 768000Hz */
> -/* extended rates */
> -#define SNDRV_PCM_RATE_12000		(1U<<17)	/* 12000Hz */
> -#define SNDRV_PCM_RATE_24000		(1U<<18)	/* 24000Hz */
> -#define SNDRV_PCM_RATE_128000		(1U<<19)	/* 128000Hz */
> -
> -#define SNDRV_PCM_RATE_CONTINUOUS	(1U<<30)	/* continuous range */
> -#define SNDRV_PCM_RATE_KNOT		(1U<<31)	/* supports more non-continuous rates */
> -
>   #define SNDRV_PCM_RATE_8000_44100	(SNDRV_PCM_RATE_8000|SNDRV_PCM_RATE_11025|\
>   					 SNDRV_PCM_RATE_16000|SNDRV_PCM_RATE_22050|\
>   					 SNDRV_PCM_RATE_32000|SNDRV_PCM_RATE_44100)
> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
> index 4cd513215bcd..715ceb3eac7c 100644
> --- a/include/uapi/sound/asound.h
> +++ b/include/uapi/sound/asound.h
> @@ -272,6 +272,32 @@ typedef int __bitwise snd_pcm_subformat_t;
>   #define	SNDRV_PCM_SUBFORMAT_MSBITS_24	((__force snd_pcm_subformat_t) 3)
>   #define	SNDRV_PCM_SUBFORMAT_LAST	SNDRV_PCM_SUBFORMAT_MSBITS_24
>   
> +/* Standard rate bits */
> +#define SNDRV_PCM_RATE_5512		(1U<<0)		/* 5512Hz */
> +#define SNDRV_PCM_RATE_8000		(1U<<1)		/* 8000Hz */
> +#define SNDRV_PCM_RATE_11025		(1U<<2)		/* 11025Hz */
> +#define SNDRV_PCM_RATE_16000		(1U<<3)		/* 16000Hz */
> +#define SNDRV_PCM_RATE_22050		(1U<<4)		/* 22050Hz */
> +#define SNDRV_PCM_RATE_32000		(1U<<5)		/* 32000Hz */
> +#define SNDRV_PCM_RATE_44100		(1U<<6)		/* 44100Hz */
> +#define SNDRV_PCM_RATE_48000		(1U<<7)		/* 48000Hz */
> +#define SNDRV_PCM_RATE_64000		(1U<<8)		/* 64000Hz */
> +#define SNDRV_PCM_RATE_88200		(1U<<9)		/* 88200Hz */
> +#define SNDRV_PCM_RATE_96000		(1U<<10)	/* 96000Hz */
> +#define SNDRV_PCM_RATE_176400		(1U<<11)	/* 176400Hz */
> +#define SNDRV_PCM_RATE_192000		(1U<<12)	/* 192000Hz */
> +#define SNDRV_PCM_RATE_352800		(1U<<13)	/* 352800Hz */
> +#define SNDRV_PCM_RATE_384000		(1U<<14)	/* 384000Hz */
> +#define SNDRV_PCM_RATE_705600		(1U<<15)	/* 705600Hz */
> +#define SNDRV_PCM_RATE_768000		(1U<<16)	/* 768000Hz */
> +/* extended rates */
> +#define SNDRV_PCM_RATE_12000		(1U<<17)	/* 12000Hz */
> +#define SNDRV_PCM_RATE_24000		(1U<<18)	/* 24000Hz */
> +#define SNDRV_PCM_RATE_128000		(1U<<19)	/* 128000Hz */
> +
> +#define SNDRV_PCM_RATE_CONTINUOUS	(1U<<30)	/* continuous range */
> +#define SNDRV_PCM_RATE_KNOT		(1U<<31)	/* supports more non-continuous rates */
> +
>   #define SNDRV_PCM_INFO_MMAP		0x00000001	/* hardware supports mmap */
>   #define SNDRV_PCM_INFO_MMAP_VALID	0x00000002	/* period data are valid during transfer */
>   #define SNDRV_PCM_INFO_DOUBLE		0x00000004	/* Double buffering needed for PCM start/stop */

I will just note that alternatively we could bump topology ABI (wouldn't 
be first time) and provide mapping in soc-topology.c for current one.

For ABI+1 we could remove the field from problematic topology struct to 
not make SNDRV_PCM_RATE_* part of UAPI and provide some other way to 
pass expected rates.