| Message ID | 20180214081348.4322-1-peter.ujfalusi@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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, >
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?
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
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, >
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
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,
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(-)