| Message ID | 20221214123743.3713843-4-lukma@denx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: Fixes for WM8940 codec | expand |
On Wed, Dec 14, 2022 at 01:37:42PM +0100, 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. > > Signed-off-by: Lukasz Majewski <lukma@denx.de> > --- > + 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; This bit is also controlled by the "Speaker Playback Switch" so you probably need some locking between them to stop them clobbering each other. Thanks, Charles
On Wed, Dec 14, 2022 at 01:37:42PM +0100, 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. > @@ -465,9 +465,18 @@ static int wm8940_mute(struct snd_soc_dai *dai, int mute, int direction) > { > + 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); In addition to the issue Charles raised this is simply not what the mute callback should do, the mute callback should specifically mute the digital input (with the goal of masking any glitching on there while clocks are started/stopped). Looking at the driver the device supports analogue bypass paths to the speaker - these will be broken by your patch so if you genuinely need some workaround in this area I'd be looking at the Speaker Mixer PCM Playback Switch rather than muting the speaker as a whole. If the device just can't cope without an input then ignore_mdown_time might be what you're looking for, it looks like the device doesn't have any lengthy sleeps in the power up/down paths so that should be fine so long as it doesn't pop/click. I'd also check there's not some other system configuration issue here which is more obvious when the input from the DAC stops getting input, check that you don't see similar issues when silence is played for example. It might be worth checking that none of the analogue bypass paths are enabled.
Hi Mark, > On Wed, Dec 14, 2022 at 01:37:42PM +0100, 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. > > > @@ -465,9 +465,18 @@ static int wm8940_mute(struct snd_soc_dai > > *dai, int mute, int direction) { > > > + 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); > > In addition to the issue Charles raised this is simply not what the > mute callback should do, the mute callback should specifically mute > the digital input (with the goal of masking any glitching on there > while clocks are started/stopped). Ok > Looking at the driver the device > supports analogue bypass paths to the speaker - these will be broken > by your patch I was not aware about this side effect. I just wanted to be sure that the speaker is muted. > so if you genuinely need some workaround in this area > I'd be looking at the Speaker Mixer PCM Playback Switch rather than > muting the speaker as a whole. I would be more than happy if I could use for example the 'amixer' command to setup the audio correctly without this patch. For example - on this system - before I run any speaker test I need to call: amixer -d set 'Speaker Mixer PCM',0 on to unmute the system. > If the device just can't cope without > an input then ignore_mdown_time might be what you're looking for, it > looks like the device doesn't have any lengthy sleeps in the power > up/down paths so that should be fine so long as it doesn't pop/click. > Ok. I will check this as well. > I'd also check there's not some other system configuration issue here > which is more obvious when the input from the DAC stops getting input, > check that you don't see similar issues when silence is played for > example. It might be worth checking that none of the analogue bypass > paths are enabled. Thanks for your hints. I will investigate it further. It looks like this patch is some kind of a hack, to fix my system configuration and shall be dropped in v2. 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
Hi Mark, > Hi Mark, > > > On Wed, Dec 14, 2022 at 01:37:42PM +0100, 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. > > > > > @@ -465,9 +465,18 @@ static int wm8940_mute(struct snd_soc_dai > > > *dai, int mute, int direction) { > > > > > + 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); > > > > In addition to the issue Charles raised this is simply not what the > > mute callback should do, the mute callback should specifically mute > > the digital input (with the goal of masking any glitching on there > > while clocks are started/stopped). > > Ok > > > Looking at the driver the device > > supports analogue bypass paths to the speaker - these will be broken > > by your patch > > I was not aware about this side effect. I just wanted to be sure that > the speaker is muted. > > > so if you genuinely need some workaround in this area > > I'd be looking at the Speaker Mixer PCM Playback Switch rather than > > muting the speaker as a whole. > > I would be more than happy if I could use for example the 'amixer' > command to setup the audio correctly without this patch. > > For example - on this system - before I run any speaker test I need to > call: amixer -d set 'Speaker Mixer PCM',0 on > > to unmute the system. > This patch steamed from 4.4 Linux. On the newer Linux version the "distortion" problem is not present anymore. I will drop this patch. > > If the device just can't cope without > > an input then ignore_mdown_time might be what you're looking for, it > > looks like the device doesn't have any lengthy sleeps in the power > > up/down paths so that should be fine so long as it doesn't > > pop/click. > > Ok. I will check this as well. > > > I'd also check there's not some other system configuration issue > > here which is more obvious when the input from the DAC stops > > getting input, check that you don't see similar issues when silence > > is played for example. It might be worth checking that none of the > > analogue bypass paths are enabled. > > Thanks for your hints. I will investigate it further. > > It looks like this patch is some kind of a hack, to fix my system > configuration and shall be dropped in v2. > > > 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 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
diff --git a/sound/soc/codecs/wm8940.c b/sound/soc/codecs/wm8940.c index 094e74905df9..13cb57210b4b 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(-)