| Message ID | 20220606154441.20848-3-lukma@denx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] ASoC: wm8940: Remove warning when no plat data | expand |
On Mon, Jun 06, 2022 at 05:44:41PM +0200, Lukasz Majewski wrote: > Without this change the BTL speaker produces some > "distortion" noise when test program > (speaker-test -t waw) is ended with ctrl+c. > As our design uses speaker outputs to drive BTL speaker, > it was necessary to also mute the speaker via the codec > internal WM8940_SPKVOL register with setting > WM8940_SPKMUTE bit. This will not interact well with both the user visible control of the speaker volume via the Speaker Playback Volume control and the analog bypass paths that the device has - it'll change the state of the control without generating any events, and cut off any bypassed audio that's mixed in. You can probably achieve a similar effect by making the control an _AUTODISABLE one which will allow the core to mute the control when it's not being used in a way that's not visible to userspace.
Hi Mark, > On Mon, Jun 06, 2022 at 05:44:41PM +0200, Lukasz Majewski wrote: > > > Without this change the BTL speaker produces some > > "distortion" noise when test program > > (speaker-test -t waw) is ended with ctrl+c. > > > As our design uses speaker outputs to drive BTL speaker, > > it was necessary to also mute the speaker via the codec > > internal WM8940_SPKVOL register with setting > > WM8940_SPKMUTE bit. > > This will not interact well with both the user visible control of the > speaker volume via the Speaker Playback Volume control and the analog > bypass paths that the device has - it'll change the state of the > control without generating any events, and cut off any bypassed audio > that's mixed in. > I'm wondering why it is safe to call DAI's .digital_mute() callback, which explicitly changes state of the "DAC soft mute enable" bit (DACMU) ? And on the other hand it is not correct to just mute the speakers? > You can probably achieve a similar effect by making the control an > _AUTODISABLE one which will allow the core to mute the control when > it's not being used in a way that's not visible to userspace. The exact definition for the event, which I'm forcing above: SOC_SINGLE("Speaker Playback Switch", WM8940_SPKVOL, 6, 1, 1), And there is no SOC_SINGLE_AUTODISABLE() macro available. The issue I'm trying to fix: - The mclk clock is stopped (after some time) by imx SOC when I end 'speaker-test' program with ctrl+c. - When the clock is not provided (after ~1sec) I do hear a single short noise from speakers. - The other solution (which also works) would be to enable clock once (during probe) and then do not disable it till system is powered off (yes it is a hack :-) ). I'm wondering if this can be fixed by some 'amixer' user space switch? Thanks in advance for help. Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On Fri, Jun 10, 2022 at 11:23:31AM +0200, Lukasz Majewski wrote: > > On Mon, Jun 06, 2022 at 05:44:41PM +0200, Lukasz Majewski wrote: > > > Without this change the BTL speaker produces some > > > "distortion" noise when test program > > > (speaker-test -t waw) is ended with ctrl+c. > > > As our design uses speaker outputs to drive BTL speaker, > > > it was necessary to also mute the speaker via the codec > > > internal WM8940_SPKVOL register with setting > > > WM8940_SPKMUTE bit. > > This will not interact well with both the user visible control of the > > speaker volume via the Speaker Playback Volume control and the analog > > bypass paths that the device has - it'll change the state of the > > control without generating any events, and cut off any bypassed audio > > that's mixed in. > I'm wondering why it is safe to call DAI's .digital_mute() > callback, which explicitly changes state of the "DAC soft mute enable" > bit (DACMU) ? If there's a user visible control for the same register bit that's a bug. If there's no user visible control for it then there's nothing to conflict with. > And on the other hand it is not correct to just mute the speakers? No, that's not what we're muting playback for - the digital mute is there specifically to deal with issues with host controllers outputing noise during startup/teardown. If there are issues with the speaker output then they need to be addressed at that point, especially given that the device has bypass paths. > > You can probably achieve a similar effect by making the control an > > _AUTODISABLE one which will allow the core to mute the control when > > it's not being used in a way that's not visible to userspace. > The exact definition for the event, which I'm forcing above: > SOC_SINGLE("Speaker Playback Switch", WM8940_SPKVOL, 6, 1, 1), > And there is no SOC_SINGLE_AUTODISABLE() macro available. That seems solvable? Though if the issue isn't triggered in connection with a DAPM event (which sounds like the case) then it's probably not going to help. > The issue I'm trying to fix: > - The mclk clock is stopped (after some time) by imx SOC when I end > 'speaker-test' program with ctrl+c. > - When the clock is not provided (after ~1sec) I do hear a single short > noise from speakers. > - The other solution (which also works) would be to enable clock once > (during probe) and then do not disable it till system is powered > off (yes it is a hack :-) ). If the issue is triggered by the MCLK being disabled prematurely then the simplest fix is probably to wire up the CODEC MCLK to the clock API and manage it during set_bias_level() (probably on transition out of and into _STANDBY) - that should have a similar effect to leaving it enabled all the time.
diff --git a/sound/soc/codecs/wm8940.c b/sound/soc/codecs/wm8940.c index 6fb1c3780439..a8596f4089dd 100644 --- a/sound/soc/codecs/wm8940.c +++ b/sound/soc/codecs/wm8940.c @@ -465,9 +465,18 @@ static int wm8940_mute(struct snd_soc_dai *dai, int mute, int direction) { struct snd_soc_component *component = dai->component; u16 mute_reg = snd_soc_component_read(component, WM8940_DAC) & 0xffbf; + u16 spkvol_reg = snd_soc_component_read(component, WM8940_SPKVOL); + int ret; - if (mute) + spkvol_reg &= ~WM8940_SPKMUTE; + if (mute) { mute_reg |= 0x40; + spkvol_reg |= WM8940_SPKMUTE; + } + + ret = snd_soc_component_write(component, WM8940_SPKVOL, spkvol_reg); + if (ret) + return ret; return snd_soc_component_write(component, WM8940_DAC, mute_reg); } diff --git a/sound/soc/codecs/wm8940.h b/sound/soc/codecs/wm8940.h index 0d4f53ada2e6..eb051ed29bb8 100644 --- a/sound/soc/codecs/wm8940.h +++ b/sound/soc/codecs/wm8940.h @@ -95,5 +95,8 @@ struct wm8940_setup_data { #define WM8940_OPCLKDIV_3 2 #define WM8940_OPCLKDIV_4 3 +/* Bit definitions */ +#define WM8940_SPKMUTE BIT(6) + #endif /* _WM8940_H */
Without this change the BTL speaker produces some "distortion" noise when test program (speaker-test -t waw) is ended with ctrl+c. As our design uses speaker outputs to drive BTL speaker, it was necessary to also mute the speaker via the codec internal WM8940_SPKVOL register with setting WM8940_SPKMUTE bit. Signed-off-by: Lukasz Majewski <lukma@denx.de> --- sound/soc/codecs/wm8940.c | 11 ++++++++++- sound/soc/codecs/wm8940.h | 3 +++ 2 files changed, 13 insertions(+), 1 deletion(-)