All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lukasz Majewski <lukma@denx.de>
To: Mark Brown <broonie@kernel.org>
Cc: alsa-devel@alsa-project.org, Stephen Kitt <steve@sk2.org>,
	patches@opensource.cirrus.com, Takashi Iwai <tiwai@suse.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Charles Keepax <ckeepax@opensource.cirrus.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/4] ASoC: wm8940: Mute also the speaker output
Date: Wed, 14 Dec 2022 21:55:46 +0100	[thread overview]
Message-ID: <20221214215546.657a04f3@wsk> (raw)
In-Reply-To: <Y5nbf72ksywcXK65@sirena.org.uk>

[-- Attachment #1: Type: text/plain, Size: 2852 bytes --]

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Lukasz Majewski <lukma@denx.de>
To: Mark Brown <broonie@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>,
	Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
	Charles Keepax <ckeepax@opensource.cirrus.com>,
	Stephen Kitt <steve@sk2.org>,
	patches@opensource.cirrus.com, alsa-devel@alsa-project.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/4] ASoC: wm8940: Mute also the speaker output
Date: Wed, 14 Dec 2022 21:55:46 +0100	[thread overview]
Message-ID: <20221214215546.657a04f3@wsk> (raw)
In-Reply-To: <Y5nbf72ksywcXK65@sirena.org.uk>

[-- Attachment #1: Type: text/plain, Size: 2852 bytes --]

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2022-12-14 20:57 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-14 12:37 [PATCH 0/4] ASoC: Fixes for WM8940 codec Lukasz Majewski
2022-12-14 12:37 ` [PATCH 1/4] ASoC: wm8940: Remove warning when no plat data present Lukasz Majewski
2022-12-14 13:10   ` Charles Keepax
2022-12-14 13:10     ` Charles Keepax
2022-12-14 12:37 ` [PATCH 2/4] ASoC: wm8940: Rewrite code to set proper clocks Lukasz Majewski
2022-12-14 13:31   ` Charles Keepax
2022-12-14 13:31     ` Charles Keepax
2022-12-14 21:01     ` Lukasz Majewski
2022-12-14 21:01       ` Lukasz Majewski
2022-12-14 13:56   ` Mark Brown
2022-12-14 13:56     ` Mark Brown
2022-12-14 21:10     ` Lukasz Majewski
2022-12-14 21:10       ` Lukasz Majewski
2022-12-14 12:37 ` [PATCH 3/4] ASoC: wm8940: Mute also the speaker output Lukasz Majewski
2022-12-14 13:09   ` Charles Keepax
2022-12-14 13:09     ` Charles Keepax
2022-12-14 14:19   ` Mark Brown
2022-12-14 14:19     ` Mark Brown
2022-12-14 20:55     ` Lukasz Majewski [this message]
2022-12-14 20:55       ` Lukasz Majewski
2022-12-15  8:53       ` Lukasz Majewski
2022-12-15  8:53         ` Lukasz Majewski
2022-12-14 12:37 ` [PATCH 4/4] ASoC: wm8940: Read chip ID when wm8940 codec probing Lukasz Majewski
2022-12-14 13:10   ` Charles Keepax
2022-12-14 13:10     ` Charles Keepax
2022-12-15  9:36 ` [PATCH v2 1/3] ASoC: wm8940: Remove warning when no plat data present Lukasz Majewski
2022-12-15  9:36   ` Lukasz Majewski
2022-12-15  9:36   ` [PATCH v2 2/3] ASoC: wm8940: Rewrite code to set proper clocks Lukasz Majewski
2022-12-15  9:36     ` Lukasz Majewski
2022-12-15 10:13     ` Charles Keepax
2022-12-15 10:13       ` Charles Keepax
2022-12-15  9:36   ` [PATCH v2 3/3] ASoC: wm8940: Read chip ID when wm8940 codec probing Lukasz Majewski
2022-12-15  9:36     ` Lukasz Majewski
2022-12-15 11:08   ` [PATCH v2 1/3] ASoC: wm8940: Remove warning when no plat data present Mark Brown
2022-12-15 11:08     ` Mark Brown
2022-12-15 13:59     ` Lukasz Majewski
2022-12-15 13:59       ` Lukasz Majewski
2022-12-27 11:57 ` (subset) [PATCH 0/4] ASoC: Fixes for WM8940 codec Mark Brown
2022-12-27 11:57 ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221214215546.657a04f3@wsk \
    --to=lukma@denx.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=ckeepax@opensource.cirrus.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@opensource.cirrus.com \
    --cc=steve@sk2.org \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.