[PATCH 00/13] ALSA: update sample rate definitions
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,
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 = {
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
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;
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
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
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 = {
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
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 */
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 */
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
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.
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.
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 = {
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
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);
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,
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,
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 ?
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,
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
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,
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
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,
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
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 |
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,@@ -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,
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);
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)
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,
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
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 | \
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
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
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
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
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
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
participants (11)
-
Amadeusz Sławiński -
Cezary Rojewski -
Charles Keepax -
Jaroslav Kysela -
Jerome Brunet -
Liao, Bard -
Mark Brown -
Pierre-Louis Bossart -
Péter Ujfalusi -
Rhodes, David -
Takashi Iwai