diff mbox series

[v2,12/17] ASoC: sun8i-codec: Protect the clock rate while streams are open

Message ID 20201014061941.4306-13-samuel@sholland.org
State New
Headers show
Series ASoC: sun8i-codec: support for AIF2 and AIF3 | expand

Commit Message

Samuel Holland Oct. 14, 2020, 6:19 a.m. UTC 
The codec's clock input is shared among all AIFs, and shared with other
audio-related hardware in the SoC, including I2S and SPDIF controllers.
To ensure sample rates selected by userspace or by codec2codec DAI links
are maintained, the clock rate must be protected while it is in use.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 sound/soc/sunxi/Kconfig       |  1 +
 sound/soc/sunxi/sun8i-codec.c | 29 +++++++++++++++++++++++++++--
 2 files changed, 28 insertions(+), 2 deletions(-)

Comments

Maxime Ripard Oct. 19, 2020, 8:28 a.m. UTC | #1 
On Wed, Oct 14, 2020 at 01:19:36AM -0500, Samuel Holland wrote:
> The codec's clock input is shared among all AIFs, and shared with other
> audio-related hardware in the SoC, including I2S and SPDIF controllers.
> To ensure sample rates selected by userspace or by codec2codec DAI links
> are maintained, the clock rate must be protected while it is in use.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>  sound/soc/sunxi/Kconfig       |  1 +
>  sound/soc/sunxi/sun8i-codec.c | 29 +++++++++++++++++++++++++++--
>  2 files changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/sunxi/Kconfig b/sound/soc/sunxi/Kconfig
> index 9cd7009cb570..69b9d8515335 100644
> --- a/sound/soc/sunxi/Kconfig
> +++ b/sound/soc/sunxi/Kconfig
> @@ -9,16 +9,17 @@ config SND_SUN4I_CODEC
>  	help
>  	  Select Y or M to add support for the Codec embedded in the Allwinner
>  	  A10 and affiliated SoCs.
>  
>  config SND_SUN8I_CODEC
>  	tristate "Allwinner SUN8I audio codec"
>  	depends on OF
>  	depends on MACH_SUN8I || (ARM64 && ARCH_SUNXI) || COMPILE_TEST
> +	select COMMON_CLK

Wouldn't a depends on make more sense here? It's kind of weird to pull
it from a driver when the platform that would run it has no CCF support.

With this changed,
Acked-by: Maxime Ripard <mripard@kernel.org>

Maxime
diff mbox series

Patch

diff --git a/sound/soc/sunxi/Kconfig b/sound/soc/sunxi/Kconfig
index 9cd7009cb570..69b9d8515335 100644
--- a/sound/soc/sunxi/Kconfig
+++ b/sound/soc/sunxi/Kconfig
@@ -9,16 +9,17 @@  config SND_SUN4I_CODEC
 	help
 	  Select Y or M to add support for the Codec embedded in the Allwinner
 	  A10 and affiliated SoCs.
 
 config SND_SUN8I_CODEC
 	tristate "Allwinner SUN8I audio codec"
 	depends on OF
 	depends on MACH_SUN8I || (ARM64 && ARCH_SUNXI) || COMPILE_TEST
+	select COMMON_CLK
 	select REGMAP_MMIO
 	help
 	  This option enables the digital part of the internal audio codec for
 	  Allwinner sun8i SoC (and particularly A33).
 
 	  Say Y or M if you want to add sun8i digital audio codec support.
 
 config SND_SUN8I_CODEC_ANALOG
diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c
index 0e8b0ac31fed..253857e66f6f 100644
--- a/sound/soc/sunxi/sun8i-codec.c
+++ b/sound/soc/sunxi/sun8i-codec.c
@@ -416,27 +416,32 @@  static int sun8i_codec_get_lrck_div_order(unsigned int slots,
 	unsigned int div = slots * slot_width;
 
 	if (div < 16 || div > 256)
 		return -EINVAL;
 
 	return order_base_2(div);
 }
 
+static unsigned int sun8i_codec_get_sysclk_rate(unsigned int sample_rate)
+{
+	return sample_rate % 4000 ? 22579200 : 24576000;
+}
+
 static int sun8i_codec_hw_params(struct snd_pcm_substream *substream,
 				 struct snd_pcm_hw_params *params,
 				 struct snd_soc_dai *dai)
 {
 	struct sun8i_codec *scodec = snd_soc_dai_get_drvdata(dai);
 	struct sun8i_codec_aif *aif = &scodec->aifs[dai->id];
 	unsigned int sample_rate = params_rate(params);
 	unsigned int slots = aif->slots ?: params_channels(params);
 	unsigned int slot_width = aif->slot_width ?: params_width(params);
-	unsigned int sysclk_rate = clk_get_rate(scodec->clk_module);
-	int lrck_div_order, word_size;
+	unsigned int sysclk_rate = sun8i_codec_get_sysclk_rate(sample_rate);
+	int lrck_div_order, ret, word_size;
 	u8 bclk_div;
 
 	/* word size */
 	switch (params_width(params)) {
 	case 8:
 		word_size = 0x0;
 		break;
 	case 16:
@@ -466,35 +471,55 @@  static int sun8i_codec_hw_params(struct snd_pcm_substream *substream,
 			   (lrck_div_order - 4) << SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV);
 
 	/* BCLK divider (SYSCLK/BCLK ratio) */
 	bclk_div = sun8i_codec_get_bclk_div(sysclk_rate, lrck_div_order, sample_rate);
 	regmap_update_bits(scodec->regmap, SUN8I_AIF1CLK_CTRL,
 			   SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV_MASK,
 			   bclk_div << SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV);
 
+	/*
+	 * SYSCLK rate
+	 *
+	 * Clock rate protection is reference counted; but hw_params may be
+	 * called many times per substream, without matching calls to hw_free.
+	 * Protect the clock rate once per AIF, on the first hw_params call
+	 * for the first substream. clk_set_rate() will allow clock rate
+	 * changes on subsequent calls if only one AIF has open streams.
+	 */
+	ret = (aif->open_streams ? clk_set_rate : clk_set_rate_exclusive)(scodec->clk_module,
+									  sysclk_rate);
+	if (ret == -EBUSY)
+		dev_err(dai->dev,
+			"%s sample rate (%u Hz) conflicts with other audio streams\n",
+			dai->name, sample_rate);
+	if (ret < 0)
+		return ret;
+
 	if (!aif->open_streams)
 		scodec->sysclk_refcnt++;
 	scodec->sysclk_rate = sysclk_rate;
 
 	aif->sample_rate = sample_rate;
 	aif->open_streams |= BIT(substream->stream);
 
 	return sun8i_codec_update_sample_rate(scodec);
 }
 
 static int sun8i_codec_hw_free(struct snd_pcm_substream *substream,
 			       struct snd_soc_dai *dai)
 {
 	struct sun8i_codec *scodec = snd_soc_dai_get_drvdata(dai);
 	struct sun8i_codec_aif *aif = &scodec->aifs[dai->id];
 
+	/* Drop references when the last substream for the AIF is freed. */
 	if (aif->open_streams != BIT(substream->stream))
 		goto done;
 
+	clk_rate_exclusive_put(scodec->clk_module);
 	scodec->sysclk_refcnt--;
 	aif->sample_rate = 0;
 
 done:
 	aif->open_streams &= ~BIT(substream->stream);
 	return 0;
 }