2022-08-08 14:39:35

by Martin Povišer

[permalink] [raw]
Subject: [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(-)

--
2.33.0


2022-08-08 14:51:04

by Martin Povišer

[permalink] [raw]
Subject: [PATCH 4/4] ASoC: tas2770: Fix handling of mute/unmute

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 <[email protected]>
---
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__ */
--
2.33.0

2022-08-08 17:01:38

by Martin Povišer

[permalink] [raw]
Subject: Re: [PATCH 4/4] ASoC: tas2770: Fix handling of mute/unmute


> On 8. 8. 2022, at 16:12, Martin Povišer <[email protected]> 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 <[email protected]>

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