[alsa-devel] [PATCH 1/3] ALSA: Fix handling of multiple msbits constraints on the same runtime
If the sound card is made up of discrete components, each with their own driver (e.g. like in the ASoC case), we might end up with multiple msbits constraint rules installed. Currently this will result in msbits being set to whatever the last rule set it to.
This patch updates the behavior of the rule to choose the minimum (other than zero) of all the installed rules.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/core/pcm_lib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index ec9e786..b0c1535 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -1300,7 +1300,7 @@ static int snd_pcm_hw_rule_msbits(struct snd_pcm_hw_params *params, unsigned int msbits = l >> 16; struct snd_interval *i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS); if (snd_interval_single(i) && snd_interval_value(i) == width) - params->msbits = msbits; + params->msbits = min_not_zero(params->msbits, msbits); return 0; }
Currently the msbits constraints requires to specify a specific sample format width for which the constraint should be applied. But often the number of most significant bits is not sample format specific, but rather a absolute limit. E.g. the PCM interface might accept 32-bit and 24-bit samples, but the DAC has a 16-bit resolution and throws away the LSBs. In this case for both 32-bit and 24-bit format msbits should be set to 16. This patch extends snd_pcm_hw_constraint_msbits() so that a wildcard constraint can be setup that is applied for all formats with a sample width larger than the specified msbits. Choosing the wildcard constraint is done by setting the sample width parameter of the function to 0.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/core/pcm_lib.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index b0c1535..db05e04 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -1299,8 +1299,14 @@ static int snd_pcm_hw_rule_msbits(struct snd_pcm_hw_params *params, int width = l & 0xffff; unsigned int msbits = l >> 16; struct snd_interval *i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS); - if (snd_interval_single(i) && snd_interval_value(i) == width) + + if (!snd_interval_single(i)) + return 0; + + if ((snd_interval_value(i) == width) || + (width == 0 && snd_interval_value(i) > msbits)) params->msbits = min_not_zero(params->msbits, msbits); + return 0; }
@@ -1311,6 +1317,11 @@ static int snd_pcm_hw_rule_msbits(struct snd_pcm_hw_params *params, * @width: sample bits width * @msbits: msbits width * + * This constraint will set the number of most significant bits (msbits) if a + * sample format with the specified width has been select. If width is set to 0 + * the msbits will be set for any sample format with a width larger than the + * specified msbits. + * * Return: Zero if successful, or a negative error code on failure. */ int snd_pcm_hw_constraint_msbits(struct snd_pcm_runtime *runtime,
Use the new wildcard msbits constraints instead of installing a constraint for each available sample format width.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/soc-pcm.c | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index eb87d96..d62d6a5 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -301,15 +301,6 @@ static bool soc_pcm_has_symmetry(struct snd_pcm_substream *substream) return symmetry; }
-/* - * List of sample sizes that might go over the bus for parameter - * application. There ought to be a wildcard sample size for things - * like the DAC/ADC resolution to use but there isn't right now. - */ -static int sample_sizes[] = { - 24, 32, -}; - static void soc_pcm_set_msb(struct snd_pcm_substream *substream, int bits) { struct snd_soc_pcm_runtime *rtd = substream->private_data; @@ -318,17 +309,10 @@ static void soc_pcm_set_msb(struct snd_pcm_substream *substream, int bits) if (!bits) return;
- for (i = 0; i < ARRAY_SIZE(sample_sizes); i++) { - if (bits >= sample_sizes[i]) - continue; - - ret = snd_pcm_hw_constraint_msbits(substream->runtime, 0, - sample_sizes[i], bits); - if (ret != 0) - dev_warn(rtd->dev, - "ASoC: Failed to set MSB %d/%d: %d\n", - bits, sample_sizes[i], ret); - } + ret = snd_pcm_hw_constraint_msbits(substream->runtime, 0, 0, bits); + if (ret != 0) + dev_warn(rtd->dev, "ASoC: Failed to set MSB %d: %d\n", + bits, ret); }
static void soc_pcm_apply_msb(struct snd_pcm_substream *substream)
On Mon, Dec 29, 2014 at 06:43:38PM +0100, Lars-Peter Clausen wrote:
Use the new wildcard msbits constraints instead of installing a constraint for each available sample format width.
Acked-by: Mark Brown broonie@kernel.org
At Mon, 29 Dec 2014 18:43:38 +0100, Lars-Peter Clausen wrote:
Use the new wildcard msbits constraints instead of installing a constraint for each available sample format width.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de
FYI, as this resulted in a warning, I applied the patch below in addition. Found in topic/msbits branch as well.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ASoC: pcm: Fix unused variable warning
sound/soc/soc-pcm.c: In function ‘soc_pcm_set_msb’: sound/soc/soc-pcm.c:307:11: warning: unused variable ‘i’ [-Wunused-variable]
Fixes: 0e2a37513a1f ('ASoC: pcm: Use wildcard msbits constraints') Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/soc/soc-pcm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index d62d6a5340a8..c076993a20d0 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -304,7 +304,7 @@ static bool soc_pcm_has_symmetry(struct snd_pcm_substream *substream) static void soc_pcm_set_msb(struct snd_pcm_substream *substream, int bits) { struct snd_soc_pcm_runtime *rtd = substream->private_data; - int ret, i; + int ret;
if (!bits) return;
On 12/31/2014 05:16 PM, Takashi Iwai wrote:
At Mon, 29 Dec 2014 18:43:38 +0100, Lars-Peter Clausen wrote:
Use the new wildcard msbits constraints instead of installing a constraint for each available sample format width.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de
FYI, as this resulted in a warning, I applied the patch below in addition. Found in topic/msbits branch as well.
Thanks. I noticed this today as well when building with a more recent gcc, for some reason it didn't show up with an older version.
- Lars
At Mon, 29 Dec 2014 18:43:36 +0100, Lars-Peter Clausen wrote:
If the sound card is made up of discrete components, each with their own driver (e.g. like in the ASoC case), we might end up with multiple msbits constraint rules installed. Currently this will result in msbits being set to whatever the last rule set it to.
This patch updates the behavior of the rule to choose the minimum (other than zero) of all the installed rules.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de
Thanks, applied all three patches now. They were applied to topic/msbits branch so that Mark can merge these more easily.
Takashi
sound/core/pcm_lib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index ec9e786..b0c1535 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -1300,7 +1300,7 @@ static int snd_pcm_hw_rule_msbits(struct snd_pcm_hw_params *params, unsigned int msbits = l >> 16; struct snd_interval *i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS); if (snd_interval_single(i) && snd_interval_value(i) == width)
params->msbits = msbits;
params->msbits = min_not_zero(params->msbits, msbits);}
-- 1.8.0
participants (3)
-
Lars-Peter Clausen -
Mark Brown -
Takashi Iwai