[PATCH 0/4] TAS2770 fixes
The first two fixes should be straightforward.
The latter two clean up what looks to me like a mess in the setting of power levels. However we settle it, we should then do the same changes to TAS2764, which has the same template (and maybe there are other drivers).
Martin Povišer (4): ASoC: tas2770: Set correct FSYNC polarity ASoC: tas2770: Allow mono streams ASoC: tas2770: Drop conflicting set_bias_level power setting ASoC: tas2770: Fix handling of mute/unmute
sound/soc/codecs/tas2770.c | 98 +++++++++++++++++--------------------- sound/soc/codecs/tas2770.h | 5 ++ 2 files changed, 48 insertions(+), 55 deletions(-)
Fix setting of FSYNC polarity for DAI formats other than I2S. Also add support for polarity inversion.
Fixes: 1a476abc723e ("tas2770: add tas2770 smart PA kernel driver") Signed-off-by: Martin Povišer povik+lin@cutebit.org --- sound/soc/codecs/tas2770.c | 20 +++++++++++++++++++- sound/soc/codecs/tas2770.h | 3 +++ 2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/tas2770.c b/sound/soc/codecs/tas2770.c index c1dbd978d550..10b64d5ba756 100644 --- a/sound/soc/codecs/tas2770.c +++ b/sound/soc/codecs/tas2770.c @@ -337,7 +337,7 @@ static int tas2770_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) struct snd_soc_component *component = dai->component; struct tas2770_priv *tas2770 = snd_soc_component_get_drvdata(component); - u8 tdm_rx_start_slot = 0, asi_cfg_1 = 0; + u8 tdm_rx_start_slot = 0, invert_fpol = 0, fpol_preinv = 0, asi_cfg_1 = 0; int ret;
switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { @@ -349,9 +349,15 @@ static int tas2770_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) }
switch (fmt & SND_SOC_DAIFMT_INV_MASK) { + case SND_SOC_DAIFMT_NB_IF: + invert_fpol = 1; + fallthrough; case SND_SOC_DAIFMT_NB_NF: asi_cfg_1 |= TAS2770_TDM_CFG_REG1_RX_RSING; break; + case SND_SOC_DAIFMT_IB_IF: + invert_fpol = 1; + fallthrough; case SND_SOC_DAIFMT_IB_NF: asi_cfg_1 |= TAS2770_TDM_CFG_REG1_RX_FALING; break; @@ -369,15 +375,19 @@ static int tas2770_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { case SND_SOC_DAIFMT_I2S: tdm_rx_start_slot = 1; + fpol_preinv = 0; break; case SND_SOC_DAIFMT_DSP_A: tdm_rx_start_slot = 0; + fpol_preinv = 1; break; case SND_SOC_DAIFMT_DSP_B: tdm_rx_start_slot = 1; + fpol_preinv = 1; break; case SND_SOC_DAIFMT_LEFT_J: tdm_rx_start_slot = 0; + fpol_preinv = 1; break; default: dev_err(tas2770->dev, @@ -391,6 +401,14 @@ static int tas2770_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) if (ret < 0) return ret;
+ ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0, + TAS2770_TDM_CFG_REG0_FPOL_MASK, + (fpol_preinv ^ invert_fpol) + ? TAS2770_TDM_CFG_REG0_FPOL_RSING + : TAS2770_TDM_CFG_REG0_FPOL_FALING); + if (ret < 0) + return ret; + return 0; }
diff --git a/sound/soc/codecs/tas2770.h b/sound/soc/codecs/tas2770.h index d156666bcc55..d51e88d8c338 100644 --- a/sound/soc/codecs/tas2770.h +++ b/sound/soc/codecs/tas2770.h @@ -41,6 +41,9 @@ #define TAS2770_TDM_CFG_REG0_31_44_1_48KHZ 0x6 #define TAS2770_TDM_CFG_REG0_31_88_2_96KHZ 0x8 #define TAS2770_TDM_CFG_REG0_31_176_4_192KHZ 0xa +#define TAS2770_TDM_CFG_REG0_FPOL_MASK BIT(0) +#define TAS2770_TDM_CFG_REG0_FPOL_RSING 0 +#define TAS2770_TDM_CFG_REG0_FPOL_FALING 1 /* TDM Configuration Reg1 */ #define TAS2770_TDM_CFG_REG1 TAS2770_REG(0X0, 0x0B) #define TAS2770_TDM_CFG_REG1_MASK GENMASK(5, 1)
The part is a mono speaker amp, but it can do downmix and switch between left and right channel, so the right channel range is 1 to 2.
Fixes: 1a476abc723e ("tas2770: add tas2770 smart PA kernel driver") Signed-off-by: Martin Povišer povik+lin@cutebit.org --- sound/soc/codecs/tas2770.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/tas2770.c b/sound/soc/codecs/tas2770.c index 10b64d5ba756..db446db88df5 100644 --- a/sound/soc/codecs/tas2770.c +++ b/sound/soc/codecs/tas2770.c @@ -507,7 +507,7 @@ static struct snd_soc_dai_driver tas2770_dai_driver[] = { .id = 0, .playback = { .stream_name = "ASI1 Playback", - .channels_min = 2, + .channels_min = 1, .channels_max = 2, .rates = TAS2770_RATES, .formats = TAS2770_FORMATS,
The driver is setting the PWR_CTRL field in both the set_bias_level callback and on DAPM events of the DAC widget (and also in the mute_stream method). Drop the set_bias_level callback altogether as the power setting it does is in conflict with the other code paths.
Fixes: 1a476abc723e ("tas2770: add tas2770 smart PA kernel driver") Signed-off-by: Martin Povišer povik+lin@cutebit.org --- sound/soc/codecs/tas2770.c | 33 --------------------------------- 1 file changed, 33 deletions(-)
diff --git a/sound/soc/codecs/tas2770.c b/sound/soc/codecs/tas2770.c index db446db88df5..10a79f8139be 100644 --- a/sound/soc/codecs/tas2770.c +++ b/sound/soc/codecs/tas2770.c @@ -46,38 +46,6 @@ static void tas2770_reset(struct tas2770_priv *tas2770) usleep_range(1000, 2000); }
-static int tas2770_set_bias_level(struct snd_soc_component *component, - enum snd_soc_bias_level level) -{ - struct tas2770_priv *tas2770 = - snd_soc_component_get_drvdata(component); - - switch (level) { - case SND_SOC_BIAS_ON: - snd_soc_component_update_bits(component, TAS2770_PWR_CTRL, - TAS2770_PWR_CTRL_MASK, - TAS2770_PWR_CTRL_ACTIVE); - break; - case SND_SOC_BIAS_STANDBY: - case SND_SOC_BIAS_PREPARE: - snd_soc_component_update_bits(component, TAS2770_PWR_CTRL, - TAS2770_PWR_CTRL_MASK, - TAS2770_PWR_CTRL_MUTE); - break; - case SND_SOC_BIAS_OFF: - snd_soc_component_update_bits(component, TAS2770_PWR_CTRL, - TAS2770_PWR_CTRL_MASK, - TAS2770_PWR_CTRL_SHUTDOWN); - break; - - default: - dev_err(tas2770->dev, "wrong power level setting %d\n", level); - return -EINVAL; - } - - return 0; -} - #ifdef CONFIG_PM static int tas2770_codec_suspend(struct snd_soc_component *component) { @@ -555,7 +523,6 @@ static const struct snd_soc_component_driver soc_component_driver_tas2770 = { .probe = tas2770_codec_probe, .suspend = tas2770_codec_suspend, .resume = tas2770_codec_resume, - .set_bias_level = tas2770_set_bias_level, .controls = tas2770_snd_controls, .num_controls = ARRAY_SIZE(tas2770_snd_controls), .dapm_widgets = tas2770_dapm_widgets,
Because the PWR_CTRL field is modeled as the power state of the DAC widget, and at the same time it is used to implement mute/unmute, we need some additional book-keeping to have the right end result no matter the sequence of calls. Without this fix, one can mute an ongoing stream by toggling a speaker pin control.
Fixes: 1a476abc723e ("tas2770: add tas2770 smart PA kernel driver") Signed-off-by: Martin Povišer povik+lin@cutebit.org --- sound/soc/codecs/tas2770.c | 57 ++++++++++++++++++++------------------ sound/soc/codecs/tas2770.h | 2 ++ 2 files changed, 32 insertions(+), 27 deletions(-)
diff --git a/sound/soc/codecs/tas2770.c b/sound/soc/codecs/tas2770.c index 10a79f8139be..9ea2aca65e89 100644 --- a/sound/soc/codecs/tas2770.c +++ b/sound/soc/codecs/tas2770.c @@ -46,6 +46,26 @@ static void tas2770_reset(struct tas2770_priv *tas2770) usleep_range(1000, 2000); }
+static int tas2770_update_pwr_ctrl(struct tas2770_priv *tas2770) +{ + struct snd_soc_component *component = tas2770->component; + unsigned int val; + int ret; + + if (tas2770->dac_powered) + val = tas2770->unmuted ? + TAS2770_PWR_CTRL_ACTIVE : TAS2770_PWR_CTRL_MUTE; + else + val = TAS2770_PWR_CTRL_SHUTDOWN; + + ret = snd_soc_component_update_bits(component, TAS2770_PWR_CTRL, + TAS2770_PWR_CTRL_MASK, val); + if (ret < 0) + return ret; + + return 0; +} + #ifdef CONFIG_PM static int tas2770_codec_suspend(struct snd_soc_component *component) { @@ -82,9 +102,7 @@ static int tas2770_codec_resume(struct snd_soc_component *component) gpiod_set_value_cansleep(tas2770->sdz_gpio, 1); usleep_range(1000, 2000); } else { - ret = snd_soc_component_update_bits(component, TAS2770_PWR_CTRL, - TAS2770_PWR_CTRL_MASK, - TAS2770_PWR_CTRL_ACTIVE); + ret = tas2770_update_pwr_ctrl(tas2770); if (ret < 0) return ret; } @@ -120,24 +138,19 @@ static int tas2770_dac_event(struct snd_soc_dapm_widget *w,
switch (event) { case SND_SOC_DAPM_POST_PMU: - ret = snd_soc_component_update_bits(component, TAS2770_PWR_CTRL, - TAS2770_PWR_CTRL_MASK, - TAS2770_PWR_CTRL_MUTE); + tas2770->dac_powered = 1; + ret = tas2770_update_pwr_ctrl(tas2770); break; case SND_SOC_DAPM_PRE_PMD: - ret = snd_soc_component_update_bits(component, TAS2770_PWR_CTRL, - TAS2770_PWR_CTRL_MASK, - TAS2770_PWR_CTRL_SHUTDOWN); + tas2770->dac_powered = 0; + ret = tas2770_update_pwr_ctrl(tas2770); break; default: dev_err(tas2770->dev, "Not supported evevt\n"); return -EINVAL; }
- if (ret < 0) - return ret; - - return 0; + return ret; }
static const struct snd_kcontrol_new isense_switch = @@ -171,21 +184,11 @@ static const struct snd_soc_dapm_route tas2770_audio_map[] = { static int tas2770_mute(struct snd_soc_dai *dai, int mute, int direction) { struct snd_soc_component *component = dai->component; - int ret; - - if (mute) - ret = snd_soc_component_update_bits(component, TAS2770_PWR_CTRL, - TAS2770_PWR_CTRL_MASK, - TAS2770_PWR_CTRL_MUTE); - else - ret = snd_soc_component_update_bits(component, TAS2770_PWR_CTRL, - TAS2770_PWR_CTRL_MASK, - TAS2770_PWR_CTRL_ACTIVE); - - if (ret < 0) - return ret; + struct tas2770_priv *tas2770 = + snd_soc_component_get_drvdata(component);
- return 0; + tas2770->unmuted = !mute; + return tas2770_update_pwr_ctrl(tas2770); }
static int tas2770_set_bitwidth(struct tas2770_priv *tas2770, int bitwidth) diff --git a/sound/soc/codecs/tas2770.h b/sound/soc/codecs/tas2770.h index d51e88d8c338..f75f40781ab1 100644 --- a/sound/soc/codecs/tas2770.h +++ b/sound/soc/codecs/tas2770.h @@ -138,6 +138,8 @@ struct tas2770_priv { struct device *dev; int v_sense_slot; int i_sense_slot; + bool dac_powered; + bool unmuted; };
#endif /* __TAS2770__ */
On 8. 8. 2022, at 16:12, Martin Povišer povik+lin@cutebit.org wrote:
Because the PWR_CTRL field is modeled as the power state of the DAC widget, and at the same time it is used to implement mute/unmute, we need some additional book-keeping to have the right end result no matter the sequence of calls. Without this fix, one can mute an ongoing stream by toggling a speaker pin control.
Fixes: 1a476abc723e ("tas2770: add tas2770 smart PA kernel driver") Signed-off-by: Martin Povišer povik+lin@cutebit.org
Ah, should have written the end of the commit message clearer... What I meant is, if you toggle the speaker pin, you mute the stream permanently until it's restarted, since toggling the speaker pin back won't recover the PWR_CTRL_ACTIVE value set in mute_stream at unmute.
Martin
On Mon, 8 Aug 2022 16:12:42 +0200, Martin Povišer wrote:
The first two fixes should be straightforward.
The latter two clean up what looks to me like a mess in the setting of power levels. However we settle it, we should then do the same changes to TAS2764, which has the same template (and maybe there are other drivers).
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/4] ASoC: tas2770: Set correct FSYNC polarity commit: e9ac31f0a5d0e246b046c20348954519f91a297f [2/4] ASoC: tas2770: Allow mono streams commit: bf54d97a835dfe62d4d29e245e170c63d0089be7 [3/4] ASoC: tas2770: Drop conflicting set_bias_level power setting commit: 482c23fbc7e9bf5a7a74defd0735d5346215db58 [4/4] ASoC: tas2770: Fix handling of mute/unmute commit: 1e5907bcb3a3b569be0a03ebe668bba2ed320a50
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
participants (2)
-
Mark Brown -
Martin Povišer