All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] ASoC: tlv320aic31xx: Fix master mode clock I2S bus clocks
@ 2018-02-14  8:13 Peter Ujfalusi
  2018-02-14  8:20 ` Jyri Sarha
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Peter Ujfalusi @ 2018-02-14  8:13 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood, Jyri Sarha; +Cc: alsa-devel, afd

In the reset state of the codec we do not have complete playback or capture
routes.

The audio playback/capture will not work due to missing clock signals on
the I2S bus if PLL, MDAC/NDAC/DAC MADC/NADC/ADC is powered down.

To make sure that even if all output/input is disconnected the codec is
generating clocks, we need to have valid DAPM route in every case to power
up the must needed parts of the codec.

I have verified that switching DAC (during playback) or ADC (during
capture) will stop the I2S clocks, so the only solution is to connect the
'Off' routes as well to output/input.

The routes will be only added if the codec is clock master. In case the
role changes runtime, the applied routes are removed.

Tested on am43x-epos-evm with aic3111 codec in master mode.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
Hi,

Changes since v3:
- install or remove the master mode DAPM routes if needed
- move the clock master DAPM route 'management' to a separate function

Changes since v2:
- Leftover debug prints removed.

Changes since v1:
- Only apply the master mode DAPM routes when the codec is clock master
- comments added to explain the need.

Regards,
Peter

 sound/soc/codecs/tlv320aic31xx.c | 73 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/tlv320aic31xx.c b/sound/soc/codecs/tlv320aic31xx.c
index 858cb8be445f..bd659c803f14 100644
--- a/sound/soc/codecs/tlv320aic31xx.c
+++ b/sound/soc/codecs/tlv320aic31xx.c
@@ -166,6 +166,7 @@ struct aic31xx_priv {
 	unsigned int sysclk;
 	u8 p_div;
 	int rate_div_line;
+	bool master_dapm_route_applied;
 };
 
 struct aic31xx_rate_divs {
@@ -670,6 +671,29 @@ aic310x_audio_map[] = {
 	{"SPK", NULL, "SPK ClassD"},
 };
 
+/*
+ * Always connected DAPM routes for codec clock master modes.
+ * If the codec is the master on the I2S bus, we need to power on components
+ * to have valid DAC_CLK and also the DACs and ADC for playback/capture.
+ * Otherwise the codec will not generate clocks on the bus.
+ */
+static const struct snd_soc_dapm_route
+common31xx_cm_audio_map[] = {
+	{"DAC Left Input", "Off", "DAC IN"},
+	{"DAC Right Input", "Off", "DAC IN"},
+
+	{"HPL", NULL, "DAC Left"},
+	{"HPR", NULL, "DAC Right"},
+};
+
+static const struct snd_soc_dapm_route
+aic31xx_cm_audio_map[] = {
+	{"MIC1LP P-Terminal", "Off", "MIC1LP"},
+	{"MIC1RP P-Terminal", "Off", "MIC1RP"},
+	{"MIC1LM P-Terminal", "Off", "MIC1LM"},
+	{"MIC1LM M-Terminal", "Off", "MIC1LM"},
+};
+
 static int aic31xx_add_controls(struct snd_soc_codec *codec)
 {
 	int ret = 0;
@@ -912,6 +936,53 @@ static int aic31xx_dac_mute(struct snd_soc_dai *codec_dai, int mute)
 	return 0;
 }
 
+static int aic31xx_clock_master_routes(struct snd_soc_codec *codec,
+				       unsigned int fmt)
+{
+	struct aic31xx_priv *aic31xx = snd_soc_codec_get_drvdata(codec);
+	struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
+	int ret;
+
+	fmt &= SND_SOC_DAIFMT_MASTER_MASK;
+	if (fmt == SND_SOC_DAIFMT_CBS_CFS &&
+	    aic31xx->master_dapm_route_applied) {
+		/*
+		 * Remove the DAPM route(s) for codec clock master modes,
+		 * if applied
+		 */
+		ret = snd_soc_dapm_del_routes(dapm, common31xx_cm_audio_map,
+					ARRAY_SIZE(common31xx_cm_audio_map));
+		if (!ret && !(aic31xx->codec_type & DAC31XX_BIT))
+			ret = snd_soc_dapm_del_routes(dapm,
+					aic31xx_cm_audio_map,
+					ARRAY_SIZE(aic31xx_cm_audio_map));
+
+		if (ret)
+			return ret;
+
+		aic31xx->master_dapm_route_applied = false;
+	} else if (fmt != SND_SOC_DAIFMT_CBS_CFS &&
+		   !aic31xx->master_dapm_route_applied) {
+		/*
+		 * Add the needed DAPM route(s) for codec clock master modes,
+		 * if it is not done already
+		 */
+		ret = snd_soc_dapm_add_routes(dapm, common31xx_cm_audio_map,
+					ARRAY_SIZE(common31xx_cm_audio_map));
+		if (!ret && !(aic31xx->codec_type & DAC31XX_BIT))
+			ret = snd_soc_dapm_add_routes(dapm,
+					aic31xx_cm_audio_map,
+					ARRAY_SIZE(aic31xx_cm_audio_map));
+
+		if (ret)
+			return ret;
+
+		aic31xx->master_dapm_route_applied = true;
+	}
+
+	return 0;
+}
+
 static int aic31xx_set_dai_fmt(struct snd_soc_dai *codec_dai,
 			       unsigned int fmt)
 {
@@ -992,7 +1063,7 @@ static int aic31xx_set_dai_fmt(struct snd_soc_dai *codec_dai,
 			    AIC31XX_BCLKINV_MASK,
 			    iface_reg2);
 
-	return 0;
+	return aic31xx_clock_master_routes(codec, fmt);
 }
 
 static int aic31xx_set_dai_sysclk(struct snd_soc_dai *codec_dai,
-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

* Re: [PATCH v4] ASoC: tlv320aic31xx: Fix master mode clock I2S bus clocks
  2018-02-14  8:13 [PATCH v4] ASoC: tlv320aic31xx: Fix master mode clock I2S bus clocks Peter Ujfalusi
@ 2018-02-14  8:20 ` Jyri Sarha
  2018-02-14 11:57 ` Mark Brown
  2018-02-14 12:42 ` Stefan Müller-Klieser
  2 siblings, 0 replies; 6+ messages in thread
From: Jyri Sarha @ 2018-02-14  8:20 UTC (permalink / raw)
  To: Peter Ujfalusi, Mark Brown, Liam Girdwood; +Cc: alsa-devel, afd

On 14/02/18 10:13, Peter Ujfalusi wrote:
> In the reset state of the codec we do not have complete playback or capture
> routes.
> 
> The audio playback/capture will not work due to missing clock signals on
> the I2S bus if PLL, MDAC/NDAC/DAC MADC/NADC/ADC is powered down.
> 
> To make sure that even if all output/input is disconnected the codec is
> generating clocks, we need to have valid DAPM route in every case to power
> up the must needed parts of the codec.
> 
> I have verified that switching DAC (during playback) or ADC (during
> capture) will stop the I2S clocks, so the only solution is to connect the
> 'Off' routes as well to output/input.
> 
> The routes will be only added if the codec is clock master. In case the
> role changes runtime, the applied routes are removed.
> 
> Tested on am43x-epos-evm with aic3111 codec in master mode.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>

Looks good to me:

Reviewed-by: Jyri Sarha <jsarha@ti.com>

> ---
> Hi,
> 
> Changes since v3:
> - install or remove the master mode DAPM routes if needed
> - move the clock master DAPM route 'management' to a separate function
> 
> Changes since v2:
> - Leftover debug prints removed.
> 
> Changes since v1:
> - Only apply the master mode DAPM routes when the codec is clock master
> - comments added to explain the need.
> 
> Regards,
> Peter
> 
>  sound/soc/codecs/tlv320aic31xx.c | 73 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 72 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/codecs/tlv320aic31xx.c b/sound/soc/codecs/tlv320aic31xx.c
> index 858cb8be445f..bd659c803f14 100644
> --- a/sound/soc/codecs/tlv320aic31xx.c
> +++ b/sound/soc/codecs/tlv320aic31xx.c
> @@ -166,6 +166,7 @@ struct aic31xx_priv {
>  	unsigned int sysclk;
>  	u8 p_div;
>  	int rate_div_line;
> +	bool master_dapm_route_applied;
>  };
>  
>  struct aic31xx_rate_divs {
> @@ -670,6 +671,29 @@ aic310x_audio_map[] = {
>  	{"SPK", NULL, "SPK ClassD"},
>  };
>  
> +/*
> + * Always connected DAPM routes for codec clock master modes.
> + * If the codec is the master on the I2S bus, we need to power on components
> + * to have valid DAC_CLK and also the DACs and ADC for playback/capture.
> + * Otherwise the codec will not generate clocks on the bus.
> + */
> +static const struct snd_soc_dapm_route
> +common31xx_cm_audio_map[] = {
> +	{"DAC Left Input", "Off", "DAC IN"},
> +	{"DAC Right Input", "Off", "DAC IN"},
> +
> +	{"HPL", NULL, "DAC Left"},
> +	{"HPR", NULL, "DAC Right"},
> +};
> +
> +static const struct snd_soc_dapm_route
> +aic31xx_cm_audio_map[] = {
> +	{"MIC1LP P-Terminal", "Off", "MIC1LP"},
> +	{"MIC1RP P-Terminal", "Off", "MIC1RP"},
> +	{"MIC1LM P-Terminal", "Off", "MIC1LM"},
> +	{"MIC1LM M-Terminal", "Off", "MIC1LM"},
> +};
> +
>  static int aic31xx_add_controls(struct snd_soc_codec *codec)
>  {
>  	int ret = 0;
> @@ -912,6 +936,53 @@ static int aic31xx_dac_mute(struct snd_soc_dai *codec_dai, int mute)
>  	return 0;
>  }
>  
> +static int aic31xx_clock_master_routes(struct snd_soc_codec *codec,
> +				       unsigned int fmt)
> +{
> +	struct aic31xx_priv *aic31xx = snd_soc_codec_get_drvdata(codec);
> +	struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
> +	int ret;
> +
> +	fmt &= SND_SOC_DAIFMT_MASTER_MASK;
> +	if (fmt == SND_SOC_DAIFMT_CBS_CFS &&
> +	    aic31xx->master_dapm_route_applied) {
> +		/*
> +		 * Remove the DAPM route(s) for codec clock master modes,
> +		 * if applied
> +		 */
> +		ret = snd_soc_dapm_del_routes(dapm, common31xx_cm_audio_map,
> +					ARRAY_SIZE(common31xx_cm_audio_map));
> +		if (!ret && !(aic31xx->codec_type & DAC31XX_BIT))
> +			ret = snd_soc_dapm_del_routes(dapm,
> +					aic31xx_cm_audio_map,
> +					ARRAY_SIZE(aic31xx_cm_audio_map));
> +
> +		if (ret)
> +			return ret;
> +
> +		aic31xx->master_dapm_route_applied = false;
> +	} else if (fmt != SND_SOC_DAIFMT_CBS_CFS &&
> +		   !aic31xx->master_dapm_route_applied) {
> +		/*
> +		 * Add the needed DAPM route(s) for codec clock master modes,
> +		 * if it is not done already
> +		 */
> +		ret = snd_soc_dapm_add_routes(dapm, common31xx_cm_audio_map,
> +					ARRAY_SIZE(common31xx_cm_audio_map));
> +		if (!ret && !(aic31xx->codec_type & DAC31XX_BIT))
> +			ret = snd_soc_dapm_add_routes(dapm,
> +					aic31xx_cm_audio_map,
> +					ARRAY_SIZE(aic31xx_cm_audio_map));
> +
> +		if (ret)
> +			return ret;
> +
> +		aic31xx->master_dapm_route_applied = true;
> +	}
> +
> +	return 0;
> +}
> +
>  static int aic31xx_set_dai_fmt(struct snd_soc_dai *codec_dai,
>  			       unsigned int fmt)
>  {
> @@ -992,7 +1063,7 @@ static int aic31xx_set_dai_fmt(struct snd_soc_dai *codec_dai,
>  			    AIC31XX_BCLKINV_MASK,
>  			    iface_reg2);
>  
> -	return 0;
> +	return aic31xx_clock_master_routes(codec, fmt);
>  }
>  
>  static int aic31xx_set_dai_sysclk(struct snd_soc_dai *codec_dai,
> 


-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

* Re: [PATCH v4] ASoC: tlv320aic31xx: Fix master mode clock I2S bus clocks
  2018-02-14  8:13 [PATCH v4] ASoC: tlv320aic31xx: Fix master mode clock I2S bus clocks Peter Ujfalusi
  2018-02-14  8:20 ` Jyri Sarha
@ 2018-02-14 11:57 ` Mark Brown
  2018-02-14 12:31   ` Peter Ujfalusi
  2018-02-14 12:42 ` Stefan Müller-Klieser
  2 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2018-02-14 11:57 UTC (permalink / raw)
  To: Peter Ujfalusi; +Cc: alsa-devel, Liam Girdwood, Jyri Sarha, afd


[-- Attachment #1.1: Type: text/plain, Size: 316 bytes --]

On Wed, Feb 14, 2018 at 10:13:48AM +0200, Peter Ujfalusi wrote:
> In the reset state of the codec we do not have complete playback or capture
> routes.

Unfortuanetly this conflicts with the CODEC to component transition
patches that went in just after the merge window - could you rebase on
my current tree please?

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



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

* Re: [PATCH v4] ASoC: tlv320aic31xx: Fix master mode clock I2S bus clocks
  2018-02-14 11:57 ` Mark Brown
@ 2018-02-14 12:31   ` Peter Ujfalusi
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Ujfalusi @ 2018-02-14 12:31 UTC (permalink / raw)
  To: Mark Brown; +Cc: alsa-devel, Liam Girdwood, Jyri Sarha, afd



On 2018-02-14 13:57, Mark Brown wrote:
> On Wed, Feb 14, 2018 at 10:13:48AM +0200, Peter Ujfalusi wrote:
>> In the reset state of the codec we do not have complete playback or capture
>> routes.
> 
> Unfortuanetly this conflicts with the CODEC to component transition
> patches that went in just after the merge window - could you rebase on
> my current tree please?

Sure, sent v5 and at the same time tested the CODEC to component
conversion ;)

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

* Re: [PATCH v4] ASoC: tlv320aic31xx: Fix master mode clock I2S bus clocks
  2018-02-14  8:13 [PATCH v4] ASoC: tlv320aic31xx: Fix master mode clock I2S bus clocks Peter Ujfalusi
  2018-02-14  8:20 ` Jyri Sarha
  2018-02-14 11:57 ` Mark Brown
@ 2018-02-14 12:42 ` Stefan Müller-Klieser
  2018-02-14 13:40   ` Peter Ujfalusi
  2 siblings, 1 reply; 6+ messages in thread
From: Stefan Müller-Klieser @ 2018-02-14 12:42 UTC (permalink / raw)
  To: Peter Ujfalusi, Mark Brown, Liam Girdwood, Jyri Sarha; +Cc: alsa-devel, afd

Hi Peter,

On 14.02.2018 09:13, Peter Ujfalusi wrote:
> In the reset state of the codec we do not have complete playback or capture
> routes.
> 
> The audio playback/capture will not work due to missing clock signals on
> the I2S bus if PLL, MDAC/NDAC/DAC MADC/NADC/ADC is powered down.
> 

I ran into this issue with an aic3254. I don't know if I am missing something
from your case, but I think the aic's have a special register for that
use case, e.g. for 3254: P0-R29-D2:

Primary BCLK and Primary WCLK Power control
0: Priamry BCLK and Primary WCLK buffers are powered down when the codec is powered down
1: Primary BCLK and Primary WCLK buffers are powered up when they are used in clock
generation even when the codec is powered down

Might this fix your issue?

Regards, Stefan

> To make sure that even if all output/input is disconnected the codec is
> generating clocks, we need to have valid DAPM route in every case to power
> up the must needed parts of the codec.
> 
> I have verified that switching DAC (during playback) or ADC (during
> capture) will stop the I2S clocks, so the only solution is to connect the
> 'Off' routes as well to output/input.
> 
> The routes will be only added if the codec is clock master. In case the
> role changes runtime, the applied routes are removed.
> 
> Tested on am43x-epos-evm with aic3111 codec in master mode.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
> Hi,
> 
> Changes since v3:
> - install or remove the master mode DAPM routes if needed
> - move the clock master DAPM route 'management' to a separate function
> 
> Changes since v2:
> - Leftover debug prints removed.
> 
> Changes since v1:
> - Only apply the master mode DAPM routes when the codec is clock master
> - comments added to explain the need.
> 
> Regards,
> Peter
> 
>  sound/soc/codecs/tlv320aic31xx.c | 73 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 72 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/codecs/tlv320aic31xx.c b/sound/soc/codecs/tlv320aic31xx.c
> index 858cb8be445f..bd659c803f14 100644
> --- a/sound/soc/codecs/tlv320aic31xx.c
> +++ b/sound/soc/codecs/tlv320aic31xx.c
> @@ -166,6 +166,7 @@ struct aic31xx_priv {
>  	unsigned int sysclk;
>  	u8 p_div;
>  	int rate_div_line;
> +	bool master_dapm_route_applied;
>  };
>  
>  struct aic31xx_rate_divs {
> @@ -670,6 +671,29 @@ aic310x_audio_map[] = {
>  	{"SPK", NULL, "SPK ClassD"},
>  };
>  
> +/*
> + * Always connected DAPM routes for codec clock master modes.
> + * If the codec is the master on the I2S bus, we need to power on components
> + * to have valid DAC_CLK and also the DACs and ADC for playback/capture.
> + * Otherwise the codec will not generate clocks on the bus.
> + */
> +static const struct snd_soc_dapm_route
> +common31xx_cm_audio_map[] = {
> +	{"DAC Left Input", "Off", "DAC IN"},
> +	{"DAC Right Input", "Off", "DAC IN"},
> +
> +	{"HPL", NULL, "DAC Left"},
> +	{"HPR", NULL, "DAC Right"},
> +};
> +
> +static const struct snd_soc_dapm_route
> +aic31xx_cm_audio_map[] = {
> +	{"MIC1LP P-Terminal", "Off", "MIC1LP"},
> +	{"MIC1RP P-Terminal", "Off", "MIC1RP"},
> +	{"MIC1LM P-Terminal", "Off", "MIC1LM"},
> +	{"MIC1LM M-Terminal", "Off", "MIC1LM"},
> +};
> +
>  static int aic31xx_add_controls(struct snd_soc_codec *codec)
>  {
>  	int ret = 0;
> @@ -912,6 +936,53 @@ static int aic31xx_dac_mute(struct snd_soc_dai *codec_dai, int mute)
>  	return 0;
>  }
>  
> +static int aic31xx_clock_master_routes(struct snd_soc_codec *codec,
> +				       unsigned int fmt)
> +{
> +	struct aic31xx_priv *aic31xx = snd_soc_codec_get_drvdata(codec);
> +	struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
> +	int ret;
> +
> +	fmt &= SND_SOC_DAIFMT_MASTER_MASK;
> +	if (fmt == SND_SOC_DAIFMT_CBS_CFS &&
> +	    aic31xx->master_dapm_route_applied) {
> +		/*
> +		 * Remove the DAPM route(s) for codec clock master modes,
> +		 * if applied
> +		 */
> +		ret = snd_soc_dapm_del_routes(dapm, common31xx_cm_audio_map,
> +					ARRAY_SIZE(common31xx_cm_audio_map));
> +		if (!ret && !(aic31xx->codec_type & DAC31XX_BIT))
> +			ret = snd_soc_dapm_del_routes(dapm,
> +					aic31xx_cm_audio_map,
> +					ARRAY_SIZE(aic31xx_cm_audio_map));
> +
> +		if (ret)
> +			return ret;
> +
> +		aic31xx->master_dapm_route_applied = false;
> +	} else if (fmt != SND_SOC_DAIFMT_CBS_CFS &&
> +		   !aic31xx->master_dapm_route_applied) {
> +		/*
> +		 * Add the needed DAPM route(s) for codec clock master modes,
> +		 * if it is not done already
> +		 */
> +		ret = snd_soc_dapm_add_routes(dapm, common31xx_cm_audio_map,
> +					ARRAY_SIZE(common31xx_cm_audio_map));
> +		if (!ret && !(aic31xx->codec_type & DAC31XX_BIT))
> +			ret = snd_soc_dapm_add_routes(dapm,
> +					aic31xx_cm_audio_map,
> +					ARRAY_SIZE(aic31xx_cm_audio_map));
> +
> +		if (ret)
> +			return ret;
> +
> +		aic31xx->master_dapm_route_applied = true;
> +	}
> +
> +	return 0;
> +}
> +
>  static int aic31xx_set_dai_fmt(struct snd_soc_dai *codec_dai,
>  			       unsigned int fmt)
>  {
> @@ -992,7 +1063,7 @@ static int aic31xx_set_dai_fmt(struct snd_soc_dai *codec_dai,
>  			    AIC31XX_BCLKINV_MASK,
>  			    iface_reg2);
>  
> -	return 0;
> +	return aic31xx_clock_master_routes(codec, fmt);
>  }
>  
>  static int aic31xx_set_dai_sysclk(struct snd_soc_dai *codec_dai,
> 

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

* Re: [PATCH v4] ASoC: tlv320aic31xx: Fix master mode clock I2S bus clocks
  2018-02-14 12:42 ` Stefan Müller-Klieser
@ 2018-02-14 13:40   ` Peter Ujfalusi
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Ujfalusi @ 2018-02-14 13:40 UTC (permalink / raw)
  To: Stefan Müller-Klieser, Mark Brown, Liam Girdwood, Jyri Sarha
  Cc: alsa-devel, afd



On 2018-02-14 14:42, Stefan Müller-Klieser wrote:
> Hi Peter,
> 
> On 14.02.2018 09:13, Peter Ujfalusi wrote:
>> In the reset state of the codec we do not have complete playback or capture
>> routes.
>>
>> The audio playback/capture will not work due to missing clock signals on
>> the I2S bus if PLL, MDAC/NDAC/DAC MADC/NADC/ADC is powered down.
>>
> 
> I ran into this issue with an aic3254. I don't know if I am missing something
> from your case, but I think the aic's have a special register for that
> use case, e.g. for 3254: P0-R29-D2:
> 
> Primary BCLK and Primary WCLK Power control
> 0: Priamry BCLK and Primary WCLK buffers are powered down when the codec is powered down
> 1: Primary BCLK and Primary WCLK buffers are powered up when they are used in clock
> generation even when the codec is powered down

Yes, I actually did. The problem is that we don't want the clocks to run
when the codec is powered down as we do not want excess power
consumption in that case.

> Might this fix your issue?

Certainly it should. I need to find a tracepoint on my board to see how
the clocks behave, but I suspect it is not what we want.

> 
> Regards, Stefan
> 
>> To make sure that even if all output/input is disconnected the codec is
>> generating clocks, we need to have valid DAPM route in every case to power
>> up the must needed parts of the codec.
>>
>> I have verified that switching DAC (during playback) or ADC (during
>> capture) will stop the I2S clocks, so the only solution is to connect the
>> 'Off' routes as well to output/input.
>>
>> The routes will be only added if the codec is clock master. In case the
>> role changes runtime, the applied routes are removed.
>>
>> Tested on am43x-epos-evm with aic3111 codec in master mode.
>>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>> ---
>> Hi,
>>
>> Changes since v3:
>> - install or remove the master mode DAPM routes if needed
>> - move the clock master DAPM route 'management' to a separate function
>>
>> Changes since v2:
>> - Leftover debug prints removed.
>>
>> Changes since v1:
>> - Only apply the master mode DAPM routes when the codec is clock master
>> - comments added to explain the need.
>>
>> Regards,
>> Peter
>>
>>  sound/soc/codecs/tlv320aic31xx.c | 73 +++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 72 insertions(+), 1 deletion(-)
>>
>> diff --git a/sound/soc/codecs/tlv320aic31xx.c b/sound/soc/codecs/tlv320aic31xx.c
>> index 858cb8be445f..bd659c803f14 100644
>> --- a/sound/soc/codecs/tlv320aic31xx.c
>> +++ b/sound/soc/codecs/tlv320aic31xx.c
>> @@ -166,6 +166,7 @@ struct aic31xx_priv {
>>  	unsigned int sysclk;
>>  	u8 p_div;
>>  	int rate_div_line;
>> +	bool master_dapm_route_applied;
>>  };
>>  
>>  struct aic31xx_rate_divs {
>> @@ -670,6 +671,29 @@ aic310x_audio_map[] = {
>>  	{"SPK", NULL, "SPK ClassD"},
>>  };
>>  
>> +/*
>> + * Always connected DAPM routes for codec clock master modes.
>> + * If the codec is the master on the I2S bus, we need to power on components
>> + * to have valid DAC_CLK and also the DACs and ADC for playback/capture.
>> + * Otherwise the codec will not generate clocks on the bus.
>> + */
>> +static const struct snd_soc_dapm_route
>> +common31xx_cm_audio_map[] = {
>> +	{"DAC Left Input", "Off", "DAC IN"},
>> +	{"DAC Right Input", "Off", "DAC IN"},
>> +
>> +	{"HPL", NULL, "DAC Left"},
>> +	{"HPR", NULL, "DAC Right"},
>> +};
>> +
>> +static const struct snd_soc_dapm_route
>> +aic31xx_cm_audio_map[] = {
>> +	{"MIC1LP P-Terminal", "Off", "MIC1LP"},
>> +	{"MIC1RP P-Terminal", "Off", "MIC1RP"},
>> +	{"MIC1LM P-Terminal", "Off", "MIC1LM"},
>> +	{"MIC1LM M-Terminal", "Off", "MIC1LM"},
>> +};
>> +
>>  static int aic31xx_add_controls(struct snd_soc_codec *codec)
>>  {
>>  	int ret = 0;
>> @@ -912,6 +936,53 @@ static int aic31xx_dac_mute(struct snd_soc_dai *codec_dai, int mute)
>>  	return 0;
>>  }
>>  
>> +static int aic31xx_clock_master_routes(struct snd_soc_codec *codec,
>> +				       unsigned int fmt)
>> +{
>> +	struct aic31xx_priv *aic31xx = snd_soc_codec_get_drvdata(codec);
>> +	struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
>> +	int ret;
>> +
>> +	fmt &= SND_SOC_DAIFMT_MASTER_MASK;
>> +	if (fmt == SND_SOC_DAIFMT_CBS_CFS &&
>> +	    aic31xx->master_dapm_route_applied) {
>> +		/*
>> +		 * Remove the DAPM route(s) for codec clock master modes,
>> +		 * if applied
>> +		 */
>> +		ret = snd_soc_dapm_del_routes(dapm, common31xx_cm_audio_map,
>> +					ARRAY_SIZE(common31xx_cm_audio_map));
>> +		if (!ret && !(aic31xx->codec_type & DAC31XX_BIT))
>> +			ret = snd_soc_dapm_del_routes(dapm,
>> +					aic31xx_cm_audio_map,
>> +					ARRAY_SIZE(aic31xx_cm_audio_map));
>> +
>> +		if (ret)
>> +			return ret;
>> +
>> +		aic31xx->master_dapm_route_applied = false;
>> +	} else if (fmt != SND_SOC_DAIFMT_CBS_CFS &&
>> +		   !aic31xx->master_dapm_route_applied) {
>> +		/*
>> +		 * Add the needed DAPM route(s) for codec clock master modes,
>> +		 * if it is not done already
>> +		 */
>> +		ret = snd_soc_dapm_add_routes(dapm, common31xx_cm_audio_map,
>> +					ARRAY_SIZE(common31xx_cm_audio_map));
>> +		if (!ret && !(aic31xx->codec_type & DAC31XX_BIT))
>> +			ret = snd_soc_dapm_add_routes(dapm,
>> +					aic31xx_cm_audio_map,
>> +					ARRAY_SIZE(aic31xx_cm_audio_map));
>> +
>> +		if (ret)
>> +			return ret;
>> +
>> +		aic31xx->master_dapm_route_applied = true;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static int aic31xx_set_dai_fmt(struct snd_soc_dai *codec_dai,
>>  			       unsigned int fmt)
>>  {
>> @@ -992,7 +1063,7 @@ static int aic31xx_set_dai_fmt(struct snd_soc_dai *codec_dai,
>>  			    AIC31XX_BCLKINV_MASK,
>>  			    iface_reg2);
>>  
>> -	return 0;
>> +	return aic31xx_clock_master_routes(codec, fmt);
>>  }
>>  
>>  static int aic31xx_set_dai_sysclk(struct snd_soc_dai *codec_dai,
>>

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

end of thread, other threads:[~2018-02-14 13:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-14  8:13 [PATCH v4] ASoC: tlv320aic31xx: Fix master mode clock I2S bus clocks Peter Ujfalusi
2018-02-14  8:20 ` Jyri Sarha
2018-02-14 11:57 ` Mark Brown
2018-02-14 12:31   ` Peter Ujfalusi
2018-02-14 12:42 ` Stefan Müller-Klieser
2018-02-14 13:40   ` Peter Ujfalusi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.