alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] ASoC: nau8xxx: Implement hw constraint for rates
@ 2022-08-23  8:09 Takashi Iwai
  2022-08-23  8:09 ` [PATCH 1/5] ASoC: nau8821: " Takashi Iwai
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Takashi Iwai @ 2022-08-23  8:09 UTC (permalink / raw)
  To: Mark Brown; +Cc: alsa-devel

Hi,

this is a series of patches to address the issues on nau8xxx codecs
I've stumbled upon while dealing with a bug report for Steam Deck.
Most of them are to implement the missing hw constraint for rate
restrictions while one patch is to fix the semaphore unbalance in
nau8824 driver.


Takashi

===

Takashi Iwai (5):
  ASoC: nau8821: Implement hw constraint for rates
  ASoC: nau8824: Fix semaphore unbalance at error paths
  ASoC: nau8824: Implement hw constraint for rates
  ASoC: nau8825: Implement hw constraint for rates
  ASoC: nau8540: Implement hw constraint for rates

 sound/soc/codecs/nau8540.c | 40 +++++++++++++-----
 sound/soc/codecs/nau8821.c | 66 ++++++++++++++++--------------
 sound/soc/codecs/nau8824.c | 80 ++++++++++++++++++++----------------
 sound/soc/codecs/nau8825.c | 83 +++++++++++++++++++++-----------------
 4 files changed, 156 insertions(+), 113 deletions(-)

-- 
2.35.3


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/5] ASoC: nau8821: Implement hw constraint for rates
  2022-08-23  8:09 [PATCH 0/5] ASoC: nau8xxx: Implement hw constraint for rates Takashi Iwai
@ 2022-08-23  8:09 ` Takashi Iwai
  2022-08-23  8:09 ` [PATCH 2/5] ASoC: nau8824: Fix semaphore unbalance at error paths Takashi Iwai
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Takashi Iwai @ 2022-08-23  8:09 UTC (permalink / raw)
  To: Mark Brown; +Cc: alsa-devel

nau8821 driver restricts the sample rate with over sampling rate, but
currently it barely bails out at hw_params with -EINVAL error (with a
kernel message); this doesn't help for user-space to recognize which
rate can be actually used.

This patch introduces the proper hw constraint for adjusting the
available range of the sample rate depending on the OSR setup, as well
as some code cleanup, for improving the communication with
user-space.  Now applications can know the valid rate beforehand and
reduces the rate appropriately without errors.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/soc/codecs/nau8821.c | 66 +++++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 30 deletions(-)

diff --git a/sound/soc/codecs/nau8821.c b/sound/soc/codecs/nau8821.c
index 2d21339932e6..4a72b94e8410 100644
--- a/sound/soc/codecs/nau8821.c
+++ b/sound/soc/codecs/nau8821.c
@@ -670,28 +670,40 @@ static const struct snd_soc_dapm_route nau8821_dapm_routes[] = {
 	{"HPOR", NULL, "Class G"},
 };
 
-static int nau8821_clock_check(struct nau8821 *nau8821,
-	int stream, int rate, int osr)
+static const struct nau8821_osr_attr *
+nau8821_get_osr(struct nau8821 *nau8821, int stream)
 {
-	int osrate = 0;
+	unsigned int osr;
 
 	if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		regmap_read(nau8821->regmap, NAU8821_R2C_DAC_CTRL1, &osr);
+		osr &= NAU8821_DAC_OVERSAMPLE_MASK;
 		if (osr >= ARRAY_SIZE(osr_dac_sel))
-			return -EINVAL;
-		osrate = osr_dac_sel[osr].osr;
+			return NULL;
+		return &osr_dac_sel[osr];
 	} else {
+		regmap_read(nau8821->regmap, NAU8821_R2B_ADC_RATE, &osr);
+		osr &= NAU8821_ADC_SYNC_DOWN_MASK;
 		if (osr >= ARRAY_SIZE(osr_adc_sel))
-			return -EINVAL;
-		osrate = osr_adc_sel[osr].osr;
+			return NULL;
+		return &osr_adc_sel[osr];
 	}
+}
 
-	if (!osrate || rate * osrate > CLK_DA_AD_MAX) {
-		dev_err(nau8821->dev,
-			"exceed the maximum frequency of CLK_ADC or CLK_DAC");
+static int nau8821_dai_startup(struct snd_pcm_substream *substream,
+			       struct snd_soc_dai *dai)
+{
+	struct snd_soc_component *component = dai->component;
+	struct nau8821 *nau8821 = snd_soc_component_get_drvdata(component);
+	const struct nau8821_osr_attr *osr;
+
+	osr = nau8821_get_osr(nau8821, substream->stream);
+	if (!osr || !osr->osr)
 		return -EINVAL;
-	}
 
-	return 0;
+	return snd_pcm_hw_constraint_minmax(substream->runtime,
+					    SNDRV_PCM_HW_PARAM_RATE,
+					    0, CLK_DA_AD_MAX / osr->osr);
 }
 
 static int nau8821_hw_params(struct snd_pcm_substream *substream,
@@ -699,7 +711,8 @@ static int nau8821_hw_params(struct snd_pcm_substream *substream,
 {
 	struct snd_soc_component *component = dai->component;
 	struct nau8821 *nau8821 = snd_soc_component_get_drvdata(component);
-	unsigned int val_len = 0, osr, ctrl_val, bclk_fs, clk_div;
+	unsigned int val_len = 0, ctrl_val, bclk_fs, clk_div;
+	const struct nau8821_osr_attr *osr;
 
 	nau8821->fs = params_rate(params);
 	/* CLK_DAC or CLK_ADC = OSR * FS
@@ -708,27 +721,19 @@ static int nau8821_hw_params(struct snd_pcm_substream *substream,
 	 * values must be selected such that the maximum frequency is less
 	 * than 6.144 MHz.
 	 */
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		regmap_read(nau8821->regmap, NAU8821_R2C_DAC_CTRL1, &osr);
-		osr &= NAU8821_DAC_OVERSAMPLE_MASK;
-		if (nau8821_clock_check(nau8821, substream->stream,
-			nau8821->fs, osr)) {
-			return -EINVAL;
-		}
+	osr = nau8821_get_osr(nau8821, substream->stream);
+	if (!osr || !osr->osr)
+		return -EINVAL;
+	if (nau8821->fs * osr->osr > CLK_DA_AD_MAX)
+		return -EINVAL;
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 		regmap_update_bits(nau8821->regmap, NAU8821_R03_CLK_DIVIDER,
 			NAU8821_CLK_DAC_SRC_MASK,
-			osr_dac_sel[osr].clk_src << NAU8821_CLK_DAC_SRC_SFT);
-	} else {
-		regmap_read(nau8821->regmap, NAU8821_R2B_ADC_RATE, &osr);
-		osr &= NAU8821_ADC_SYNC_DOWN_MASK;
-		if (nau8821_clock_check(nau8821, substream->stream,
-			nau8821->fs, osr)) {
-			return -EINVAL;
-		}
+			osr->clk_src << NAU8821_CLK_DAC_SRC_SFT);
+	else
 		regmap_update_bits(nau8821->regmap, NAU8821_R03_CLK_DIVIDER,
 			NAU8821_CLK_ADC_SRC_MASK,
-			osr_adc_sel[osr].clk_src << NAU8821_CLK_ADC_SRC_SFT);
-	}
+			osr->clk_src << NAU8821_CLK_ADC_SRC_SFT);
 
 	/* make BCLK and LRC divde configuration if the codec as master. */
 	regmap_read(nau8821->regmap, NAU8821_R1D_I2S_PCM_CTRL2, &ctrl_val);
@@ -843,6 +848,7 @@ static int nau8821_digital_mute(struct snd_soc_dai *dai, int mute,
 }
 
 static const struct snd_soc_dai_ops nau8821_dai_ops = {
+	.startup = nau8821_dai_startup,
 	.hw_params = nau8821_hw_params,
 	.set_fmt = nau8821_set_dai_fmt,
 	.mute_stream = nau8821_digital_mute,
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/5] ASoC: nau8824: Fix semaphore unbalance at error paths
  2022-08-23  8:09 [PATCH 0/5] ASoC: nau8xxx: Implement hw constraint for rates Takashi Iwai
  2022-08-23  8:09 ` [PATCH 1/5] ASoC: nau8821: " Takashi Iwai
@ 2022-08-23  8:09 ` Takashi Iwai
  2022-08-23  8:09 ` [PATCH 3/5] ASoC: nau8824: Implement hw constraint for rates Takashi Iwai
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Takashi Iwai @ 2022-08-23  8:09 UTC (permalink / raw)
  To: Mark Brown; +Cc: alsa-devel

The semaphore of nau8824 wasn't properly unlocked at some error
handling code paths, hence this may result in the unbalance (and
potential lock-up).  Fix them to handle the semaphore up properly.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/soc/codecs/nau8824.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/sound/soc/codecs/nau8824.c b/sound/soc/codecs/nau8824.c
index ad54d70f7d8e..10bdfebe92d5 100644
--- a/sound/soc/codecs/nau8824.c
+++ b/sound/soc/codecs/nau8824.c
@@ -1043,6 +1043,7 @@ static int nau8824_hw_params(struct snd_pcm_substream *substream,
 	struct snd_soc_component *component = dai->component;
 	struct nau8824 *nau8824 = snd_soc_component_get_drvdata(component);
 	unsigned int val_len = 0, osr, ctrl_val, bclk_fs, bclk_div;
+	int err = -EINVAL;
 
 	nau8824_sema_acquire(nau8824, HZ);
 
@@ -1059,7 +1060,7 @@ static int nau8824_hw_params(struct snd_pcm_substream *substream,
 		osr &= NAU8824_DAC_OVERSAMPLE_MASK;
 		if (nau8824_clock_check(nau8824, substream->stream,
 			nau8824->fs, osr))
-			return -EINVAL;
+			goto error;
 		regmap_update_bits(nau8824->regmap, NAU8824_REG_CLK_DIVIDER,
 			NAU8824_CLK_DAC_SRC_MASK,
 			osr_dac_sel[osr].clk_src << NAU8824_CLK_DAC_SRC_SFT);
@@ -1069,7 +1070,7 @@ static int nau8824_hw_params(struct snd_pcm_substream *substream,
 		osr &= NAU8824_ADC_SYNC_DOWN_MASK;
 		if (nau8824_clock_check(nau8824, substream->stream,
 			nau8824->fs, osr))
-			return -EINVAL;
+			goto error;
 		regmap_update_bits(nau8824->regmap, NAU8824_REG_CLK_DIVIDER,
 			NAU8824_CLK_ADC_SRC_MASK,
 			osr_adc_sel[osr].clk_src << NAU8824_CLK_ADC_SRC_SFT);
@@ -1090,7 +1091,7 @@ static int nau8824_hw_params(struct snd_pcm_substream *substream,
 		else if (bclk_fs <= 256)
 			bclk_div = 0;
 		else
-			return -EINVAL;
+			goto error;
 		regmap_update_bits(nau8824->regmap,
 			NAU8824_REG_PORT0_I2S_PCM_CTRL_2,
 			NAU8824_I2S_LRC_DIV_MASK | NAU8824_I2S_BLK_DIV_MASK,
@@ -1111,15 +1112,17 @@ static int nau8824_hw_params(struct snd_pcm_substream *substream,
 		val_len |= NAU8824_I2S_DL_32;
 		break;
 	default:
-		return -EINVAL;
+		goto error;
 	}
 
 	regmap_update_bits(nau8824->regmap, NAU8824_REG_PORT0_I2S_PCM_CTRL_1,
 		NAU8824_I2S_DL_MASK, val_len);
+	err = 0;
 
+ error:
 	nau8824_sema_release(nau8824);
 
-	return 0;
+	return err;
 }
 
 static int nau8824_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
@@ -1128,8 +1131,6 @@ static int nau8824_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 	struct nau8824 *nau8824 = snd_soc_component_get_drvdata(component);
 	unsigned int ctrl1_val = 0, ctrl2_val = 0;
 
-	nau8824_sema_acquire(nau8824, HZ);
-
 	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
 	case SND_SOC_DAIFMT_CBM_CFM:
 		ctrl2_val |= NAU8824_I2S_MS_MASTER;
@@ -1171,6 +1172,8 @@ static int nau8824_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 		return -EINVAL;
 	}
 
+	nau8824_sema_acquire(nau8824, HZ);
+
 	regmap_update_bits(nau8824->regmap, NAU8824_REG_PORT0_I2S_PCM_CTRL_1,
 		NAU8824_I2S_DF_MASK | NAU8824_I2S_BP_MASK |
 		NAU8824_I2S_PCMB_EN, ctrl1_val);
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/5] ASoC: nau8824: Implement hw constraint for rates
  2022-08-23  8:09 [PATCH 0/5] ASoC: nau8xxx: Implement hw constraint for rates Takashi Iwai
  2022-08-23  8:09 ` [PATCH 1/5] ASoC: nau8821: " Takashi Iwai
  2022-08-23  8:09 ` [PATCH 2/5] ASoC: nau8824: Fix semaphore unbalance at error paths Takashi Iwai
@ 2022-08-23  8:09 ` Takashi Iwai
  2022-08-23  8:09 ` [PATCH 4/5] ASoC: nau8825: " Takashi Iwai
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Takashi Iwai @ 2022-08-23  8:09 UTC (permalink / raw)
  To: Mark Brown; +Cc: alsa-devel

nau8824 driver restricts the sample rate with over sampling rate, but
currently it barely bails out at hw_params with -EINVAL error (with a
kernel message); this doesn't help for user-space to recognize which
rate can be actually used.

This patch introduces the proper hw constraint for adjusting the
available range of the sample rate depending on the OSR setup, as well
as some code cleanup, for improving the communication with
user-space.  Now applications can know the valid rate beforehand and
reduces the rate appropriately without errors.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/soc/codecs/nau8824.c | 67 +++++++++++++++++++++-----------------
 1 file changed, 38 insertions(+), 29 deletions(-)

diff --git a/sound/soc/codecs/nau8824.c b/sound/soc/codecs/nau8824.c
index 10bdfebe92d5..15596452ca37 100644
--- a/sound/soc/codecs/nau8824.c
+++ b/sound/soc/codecs/nau8824.c
@@ -1014,27 +1014,42 @@ static irqreturn_t nau8824_interrupt(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static int nau8824_clock_check(struct nau8824 *nau8824,
-	int stream, int rate, int osr)
+static const struct nau8824_osr_attr *
+nau8824_get_osr(struct nau8824 *nau8824, int stream)
 {
-	int osrate;
+	unsigned int osr;
 
 	if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		regmap_read(nau8824->regmap,
+			    NAU8824_REG_DAC_FILTER_CTRL_1, &osr);
+		osr &= NAU8824_DAC_OVERSAMPLE_MASK;
 		if (osr >= ARRAY_SIZE(osr_dac_sel))
-			return -EINVAL;
-		osrate = osr_dac_sel[osr].osr;
+			return NULL;
+		return &osr_dac_sel[osr];
 	} else {
+		regmap_read(nau8824->regmap,
+			    NAU8824_REG_ADC_FILTER_CTRL, &osr);
+		osr &= NAU8824_ADC_SYNC_DOWN_MASK;
 		if (osr >= ARRAY_SIZE(osr_adc_sel))
-			return -EINVAL;
-		osrate = osr_adc_sel[osr].osr;
+			return NULL;
+		return &osr_adc_sel[osr];
 	}
+}
 
-	if (!osrate || rate * osr > CLK_DA_AD_MAX) {
-		dev_err(nau8824->dev, "exceed the maximum frequency of CLK_ADC or CLK_DAC\n");
+static int nau8824_dai_startup(struct snd_pcm_substream *substream,
+			       struct snd_soc_dai *dai)
+{
+	struct snd_soc_component *component = dai->component;
+	struct nau8824 *nau8824 = snd_soc_component_get_drvdata(component);
+	const struct nau8824_osr_attr *osr;
+
+	osr = nau8824_get_osr(nau8824, substream->stream);
+	if (!osr || !osr->osr)
 		return -EINVAL;
-	}
 
-	return 0;
+	return snd_pcm_hw_constraint_minmax(substream->runtime,
+					    SNDRV_PCM_HW_PARAM_RATE,
+					    0, CLK_DA_AD_MAX / osr->osr);
 }
 
 static int nau8824_hw_params(struct snd_pcm_substream *substream,
@@ -1042,7 +1057,8 @@ static int nau8824_hw_params(struct snd_pcm_substream *substream,
 {
 	struct snd_soc_component *component = dai->component;
 	struct nau8824 *nau8824 = snd_soc_component_get_drvdata(component);
-	unsigned int val_len = 0, osr, ctrl_val, bclk_fs, bclk_div;
+	unsigned int val_len = 0, ctrl_val, bclk_fs, bclk_div;
+	const struct nau8824_osr_attr *osr;
 	int err = -EINVAL;
 
 	nau8824_sema_acquire(nau8824, HZ);
@@ -1054,27 +1070,19 @@ static int nau8824_hw_params(struct snd_pcm_substream *substream,
 	 * than 6.144 MHz.
 	 */
 	nau8824->fs = params_rate(params);
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		regmap_read(nau8824->regmap,
-			NAU8824_REG_DAC_FILTER_CTRL_1, &osr);
-		osr &= NAU8824_DAC_OVERSAMPLE_MASK;
-		if (nau8824_clock_check(nau8824, substream->stream,
-			nau8824->fs, osr))
-			goto error;
+	osr = nau8824_get_osr(nau8824, substream->stream);
+	if (!osr || !osr->osr)
+		goto error;
+	if (nau8824->fs * osr->osr > CLK_DA_AD_MAX)
+		goto error;
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 		regmap_update_bits(nau8824->regmap, NAU8824_REG_CLK_DIVIDER,
 			NAU8824_CLK_DAC_SRC_MASK,
-			osr_dac_sel[osr].clk_src << NAU8824_CLK_DAC_SRC_SFT);
-	} else {
-		regmap_read(nau8824->regmap,
-			NAU8824_REG_ADC_FILTER_CTRL, &osr);
-		osr &= NAU8824_ADC_SYNC_DOWN_MASK;
-		if (nau8824_clock_check(nau8824, substream->stream,
-			nau8824->fs, osr))
-			goto error;
+			osr->clk_src << NAU8824_CLK_DAC_SRC_SFT);
+	else
 		regmap_update_bits(nau8824->regmap, NAU8824_REG_CLK_DIVIDER,
 			NAU8824_CLK_ADC_SRC_MASK,
-			osr_adc_sel[osr].clk_src << NAU8824_CLK_ADC_SRC_SFT);
-	}
+			osr->clk_src << NAU8824_CLK_ADC_SRC_SFT);
 
 	/* make BCLK and LRC divde configuration if the codec as master. */
 	regmap_read(nau8824->regmap,
@@ -1550,6 +1558,7 @@ static const struct snd_soc_component_driver nau8824_component_driver = {
 };
 
 static const struct snd_soc_dai_ops nau8824_dai_ops = {
+	.startup = nau8824_dai_startup,
 	.hw_params = nau8824_hw_params,
 	.set_fmt = nau8824_set_fmt,
 	.set_tdm_slot = nau8824_set_tdm_slot,
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/5] ASoC: nau8825: Implement hw constraint for rates
  2022-08-23  8:09 [PATCH 0/5] ASoC: nau8xxx: Implement hw constraint for rates Takashi Iwai
                   ` (2 preceding siblings ...)
  2022-08-23  8:09 ` [PATCH 3/5] ASoC: nau8824: Implement hw constraint for rates Takashi Iwai
@ 2022-08-23  8:09 ` Takashi Iwai
  2022-08-23  8:10 ` [PATCH 5/5] ASoC: nau8540: " Takashi Iwai
  2022-08-23 18:37 ` [PATCH 0/5] ASoC: nau8xxx: " Mark Brown
  5 siblings, 0 replies; 7+ messages in thread
From: Takashi Iwai @ 2022-08-23  8:09 UTC (permalink / raw)
  To: Mark Brown; +Cc: alsa-devel

nau8825 driver restricts the sample rate with over sampling rate, but
currently it barely bails out at hw_params with -EINVAL error (with a
kernel message); this doesn't help for user-space to recognize which
rate can be actually used.

This patch introduces the proper hw constraint for adjusting the
available range of the sample rate depending on the OSR setup, as well
as some code cleanup, for improving the communication with
user-space.  Now applications can know the valid rate beforehand and
reduces the rate appropriately without errors.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/soc/codecs/nau8825.c | 83 +++++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 38 deletions(-)

diff --git a/sound/soc/codecs/nau8825.c b/sound/soc/codecs/nau8825.c
index 54ef7b0fa878..8213273f501e 100644
--- a/sound/soc/codecs/nau8825.c
+++ b/sound/soc/codecs/nau8825.c
@@ -1247,27 +1247,42 @@ static const struct snd_soc_dapm_route nau8825_dapm_routes[] = {
 	{"HPOR", NULL, "Class G"},
 };
 
-static int nau8825_clock_check(struct nau8825 *nau8825,
-	int stream, int rate, int osr)
+static const struct nau8825_osr_attr *
+nau8825_get_osr(struct nau8825 *nau8825, int stream)
 {
-	int osrate;
+	unsigned int osr;
 
 	if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		regmap_read(nau8825->regmap,
+			    NAU8825_REG_DAC_CTRL1, &osr);
+		osr &= NAU8825_DAC_OVERSAMPLE_MASK;
 		if (osr >= ARRAY_SIZE(osr_dac_sel))
-			return -EINVAL;
-		osrate = osr_dac_sel[osr].osr;
+			return NULL;
+		return &osr_dac_sel[osr];
 	} else {
+		regmap_read(nau8825->regmap,
+			    NAU8825_REG_ADC_RATE, &osr);
+		osr &= NAU8825_ADC_SYNC_DOWN_MASK;
 		if (osr >= ARRAY_SIZE(osr_adc_sel))
-			return -EINVAL;
-		osrate = osr_adc_sel[osr].osr;
+			return NULL;
+		return &osr_adc_sel[osr];
 	}
+}
 
-	if (!osrate || rate * osr > CLK_DA_AD_MAX) {
-		dev_err(nau8825->dev, "exceed the maximum frequency of CLK_ADC or CLK_DAC\n");
+static int nau8825_dai_startup(struct snd_pcm_substream *substream,
+			       struct snd_soc_dai *dai)
+{
+	struct snd_soc_component *component = dai->component;
+	struct nau8825 *nau8825 = snd_soc_component_get_drvdata(component);
+	const struct nau8825_osr_attr *osr;
+
+	osr = nau8825_get_osr(nau8825, substream->stream);
+	if (!osr || !osr->osr)
 		return -EINVAL;
-	}
 
-	return 0;
+	return snd_pcm_hw_constraint_minmax(substream->runtime,
+					    SNDRV_PCM_HW_PARAM_RATE,
+					    0, CLK_DA_AD_MAX / osr->osr);
 }
 
 static int nau8825_hw_params(struct snd_pcm_substream *substream,
@@ -1276,7 +1291,9 @@ static int nau8825_hw_params(struct snd_pcm_substream *substream,
 {
 	struct snd_soc_component *component = dai->component;
 	struct nau8825 *nau8825 = snd_soc_component_get_drvdata(component);
-	unsigned int val_len = 0, osr, ctrl_val, bclk_fs, bclk_div;
+	unsigned int val_len = 0, ctrl_val, bclk_fs, bclk_div;
+	const struct nau8825_osr_attr *osr;
+	int err = -EINVAL;
 
 	nau8825_sema_acquire(nau8825, 3 * HZ);
 
@@ -1286,29 +1303,19 @@ static int nau8825_hw_params(struct snd_pcm_substream *substream,
 	 * values must be selected such that the maximum frequency is less
 	 * than 6.144 MHz.
 	 */
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		regmap_read(nau8825->regmap, NAU8825_REG_DAC_CTRL1, &osr);
-		osr &= NAU8825_DAC_OVERSAMPLE_MASK;
-		if (nau8825_clock_check(nau8825, substream->stream,
-			params_rate(params), osr)) {
-			nau8825_sema_release(nau8825);
-			return -EINVAL;
-		}
+	osr = nau8825_get_osr(nau8825, substream->stream);
+	if (!osr || !osr->osr)
+		goto error;
+	if (params_rate(params) * osr->osr > CLK_DA_AD_MAX)
+		goto error;
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 		regmap_update_bits(nau8825->regmap, NAU8825_REG_CLK_DIVIDER,
 			NAU8825_CLK_DAC_SRC_MASK,
-			osr_dac_sel[osr].clk_src << NAU8825_CLK_DAC_SRC_SFT);
-	} else {
-		regmap_read(nau8825->regmap, NAU8825_REG_ADC_RATE, &osr);
-		osr &= NAU8825_ADC_SYNC_DOWN_MASK;
-		if (nau8825_clock_check(nau8825, substream->stream,
-			params_rate(params), osr)) {
-			nau8825_sema_release(nau8825);
-			return -EINVAL;
-		}
+			osr->clk_src << NAU8825_CLK_DAC_SRC_SFT);
+	else
 		regmap_update_bits(nau8825->regmap, NAU8825_REG_CLK_DIVIDER,
 			NAU8825_CLK_ADC_SRC_MASK,
-			osr_adc_sel[osr].clk_src << NAU8825_CLK_ADC_SRC_SFT);
-	}
+			osr->clk_src << NAU8825_CLK_ADC_SRC_SFT);
 
 	/* make BCLK and LRC divde configuration if the codec as master. */
 	regmap_read(nau8825->regmap, NAU8825_REG_I2S_PCM_CTRL2, &ctrl_val);
@@ -1321,10 +1328,8 @@ static int nau8825_hw_params(struct snd_pcm_substream *substream,
 			bclk_div = 1;
 		else if (bclk_fs <= 128)
 			bclk_div = 0;
-		else {
-			nau8825_sema_release(nau8825);
-			return -EINVAL;
-		}
+		else
+			goto error;
 		regmap_update_bits(nau8825->regmap, NAU8825_REG_I2S_PCM_CTRL2,
 			NAU8825_I2S_LRC_DIV_MASK | NAU8825_I2S_BLK_DIV_MASK,
 			((bclk_div + 1) << NAU8825_I2S_LRC_DIV_SFT) | bclk_div);
@@ -1344,17 +1349,18 @@ static int nau8825_hw_params(struct snd_pcm_substream *substream,
 		val_len |= NAU8825_I2S_DL_32;
 		break;
 	default:
-		nau8825_sema_release(nau8825);
-		return -EINVAL;
+		goto error;
 	}
 
 	regmap_update_bits(nau8825->regmap, NAU8825_REG_I2S_PCM_CTRL1,
 		NAU8825_I2S_DL_MASK, val_len);
+	err = 0;
 
+ error:
 	/* Release the semaphore. */
 	nau8825_sema_release(nau8825);
 
-	return 0;
+	return err;
 }
 
 static int nau8825_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
@@ -1420,6 +1426,7 @@ static int nau8825_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
 }
 
 static const struct snd_soc_dai_ops nau8825_dai_ops = {
+	.startup	= nau8825_dai_startup,
 	.hw_params	= nau8825_hw_params,
 	.set_fmt	= nau8825_set_dai_fmt,
 };
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 5/5] ASoC: nau8540: Implement hw constraint for rates
  2022-08-23  8:09 [PATCH 0/5] ASoC: nau8xxx: Implement hw constraint for rates Takashi Iwai
                   ` (3 preceding siblings ...)
  2022-08-23  8:09 ` [PATCH 4/5] ASoC: nau8825: " Takashi Iwai
@ 2022-08-23  8:10 ` Takashi Iwai
  2022-08-23 18:37 ` [PATCH 0/5] ASoC: nau8xxx: " Mark Brown
  5 siblings, 0 replies; 7+ messages in thread
From: Takashi Iwai @ 2022-08-23  8:10 UTC (permalink / raw)
  To: Mark Brown; +Cc: alsa-devel

nau8540 driver restricts the sample rate with over sampling rate, but
currently it barely bails out at hw_params with -EINVAL error (with a
kernel message); this doesn't help for user-space to recognize which
rate can be actually used.

This patch introduces the proper hw constraint for adjusting the
available range of the sample rate depending on the OSR setup, as well
as some code cleanup, for improving the communication with
user-space.  Now applications can know the valid rate beforehand and
reduces the rate appropriately without errors.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/soc/codecs/nau8540.c | 40 +++++++++++++++++++++++++++-----------
 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/sound/soc/codecs/nau8540.c b/sound/soc/codecs/nau8540.c
index 58f70a02f18a..0626d5694c22 100644
--- a/sound/soc/codecs/nau8540.c
+++ b/sound/soc/codecs/nau8540.c
@@ -357,17 +357,32 @@ static const struct snd_soc_dapm_route nau8540_dapm_routes[] = {
 	{"AIFTX", NULL, "Digital CH4 Mux"},
 };
 
-static int nau8540_clock_check(struct nau8540 *nau8540, int rate, int osr)
+static const struct nau8540_osr_attr *
+nau8540_get_osr(struct nau8540 *nau8540)
 {
+	unsigned int osr;
+
+	regmap_read(nau8540->regmap, NAU8540_REG_ADC_SAMPLE_RATE, &osr);
+	osr &= NAU8540_ADC_OSR_MASK;
 	if (osr >= ARRAY_SIZE(osr_adc_sel))
-		return -EINVAL;
+		return NULL;
+	return &osr_adc_sel[osr];
+}
+
+static int nau8540_dai_startup(struct snd_pcm_substream *substream,
+			       struct snd_soc_dai *dai)
+{
+	struct snd_soc_component *component = dai->component;
+	struct nau8540 *nau8540 = snd_soc_component_get_drvdata(component);
+	const struct nau8540_osr_attr *osr;
 
-	if (rate * osr > CLK_ADC_MAX) {
-		dev_err(nau8540->dev, "exceed the maximum frequency of CLK_ADC\n");
+	osr = nau8540_get_osr(nau8540);
+	if (!osr || !osr->osr)
 		return -EINVAL;
-	}
 
-	return 0;
+	return snd_pcm_hw_constraint_minmax(substream->runtime,
+					    SNDRV_PCM_HW_PARAM_RATE,
+					    0, CLK_ADC_MAX / osr->osr);
 }
 
 static int nau8540_hw_params(struct snd_pcm_substream *substream,
@@ -375,7 +390,8 @@ static int nau8540_hw_params(struct snd_pcm_substream *substream,
 {
 	struct snd_soc_component *component = dai->component;
 	struct nau8540 *nau8540 = snd_soc_component_get_drvdata(component);
-	unsigned int val_len = 0, osr;
+	unsigned int val_len = 0;
+	const struct nau8540_osr_attr *osr;
 
 	/* CLK_ADC = OSR * FS
 	 * ADC clock frequency is defined as Over Sampling Rate (OSR)
@@ -383,13 +399,14 @@ static int nau8540_hw_params(struct snd_pcm_substream *substream,
 	 * values must be selected such that the maximum frequency is less
 	 * than 6.144 MHz.
 	 */
-	regmap_read(nau8540->regmap, NAU8540_REG_ADC_SAMPLE_RATE, &osr);
-	osr &= NAU8540_ADC_OSR_MASK;
-	if (nau8540_clock_check(nau8540, params_rate(params), osr))
+	osr = nau8540_get_osr(nau8540);
+	if (!osr || !osr->osr)
+		return -EINVAL;
+	if (params_rate(params) * osr->osr > CLK_ADC_MAX)
 		return -EINVAL;
 	regmap_update_bits(nau8540->regmap, NAU8540_REG_CLOCK_SRC,
 		NAU8540_CLK_ADC_SRC_MASK,
-		osr_adc_sel[osr].clk_src << NAU8540_CLK_ADC_SRC_SFT);
+		osr->clk_src << NAU8540_CLK_ADC_SRC_SFT);
 
 	switch (params_width(params)) {
 	case 16:
@@ -515,6 +532,7 @@ static int nau8540_set_tdm_slot(struct snd_soc_dai *dai,
 
 
 static const struct snd_soc_dai_ops nau8540_dai_ops = {
+	.startup = nau8540_dai_startup,
 	.hw_params = nau8540_hw_params,
 	.set_fmt = nau8540_set_fmt,
 	.set_tdm_slot = nau8540_set_tdm_slot,
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/5] ASoC: nau8xxx: Implement hw constraint for rates
  2022-08-23  8:09 [PATCH 0/5] ASoC: nau8xxx: Implement hw constraint for rates Takashi Iwai
                   ` (4 preceding siblings ...)
  2022-08-23  8:10 ` [PATCH 5/5] ASoC: nau8540: " Takashi Iwai
@ 2022-08-23 18:37 ` Mark Brown
  5 siblings, 0 replies; 7+ messages in thread
From: Mark Brown @ 2022-08-23 18:37 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

On Tue, 23 Aug 2022 10:09:55 +0200, Takashi Iwai wrote:
> this is a series of patches to address the issues on nau8xxx codecs
> I've stumbled upon while dealing with a bug report for Steam Deck.
> Most of them are to implement the missing hw constraint for rate
> restrictions while one patch is to fix the semaphore unbalance in
> nau8824 driver.
> 
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/5] ASoC: nau8821: Implement hw constraint for rates
      commit: cf5071876baf995f8f98e86ef06f85a58feda63c
[2/5] ASoC: nau8824: Fix semaphore unbalance at error paths
      commit: 5628560e90395d3812800a8e44a01c32ffa429ec
[3/5] ASoC: nau8824: Implement hw constraint for rates
      commit: 92283c86260d8712b55f97eada13b3c8b2f469b2
[4/5] ASoC: nau8825: Implement hw constraint for rates
      commit: bed41de0f679c516de45cfeb2c40c412bc5e0c0b
[5/5] ASoC: nau8540: Implement hw constraint for rates
      commit: be919239fbcab19290bfd6802c7ad1dc946c515b

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-08-23 18:38 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-23  8:09 [PATCH 0/5] ASoC: nau8xxx: Implement hw constraint for rates Takashi Iwai
2022-08-23  8:09 ` [PATCH 1/5] ASoC: nau8821: " Takashi Iwai
2022-08-23  8:09 ` [PATCH 2/5] ASoC: nau8824: Fix semaphore unbalance at error paths Takashi Iwai
2022-08-23  8:09 ` [PATCH 3/5] ASoC: nau8824: Implement hw constraint for rates Takashi Iwai
2022-08-23  8:09 ` [PATCH 4/5] ASoC: nau8825: " Takashi Iwai
2022-08-23  8:10 ` [PATCH 5/5] ASoC: nau8540: " Takashi Iwai
2022-08-23 18:37 ` [PATCH 0/5] ASoC: nau8xxx: " Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).