| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <871pmbh31z.wl-tiwai@suse.de>
Date: Thu, 06 Nov 2025 11:24:08 +0100
From: Takashi Iwai <tiwai@...e.de>
To: wangdich9700@....com
Cc: lgirdwood@...il.com,
broonie@...nel.org,
perex@...ex.cz,
tiwai@...e.com,
cezary.rojewski@...el.com,
linux-kernel@...r.kernel.org,
linux-sound@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
wangdicheng <wangdicheng@...inos.cn>
Subject: Re: [PATCH] [PATCH v3] ALSA: hda/conexant: Fix pop noise on CX11880/SN6140 codecs
On Thu, 06 Nov 2025 07:34:59 +0100,
wangdich9700@....com wrote:
>
> From: wangdicheng <wangdich9700@....com>
>
> Pop noise mitigation: When headphones are unplugged during playback, mute
> speaker DAC (0x17) immediately and restore after 20ms delay to avoid
> audible popping. This fix is specifically for CX11880 (0x14f11f86) and
> SN6140 (0x14f11f87) codecs based on testing verification.
>
> Signed-off-by: wangdicheng <wangdicheng@...inos.cn>
> ---
> V2 -> V3:
> - Fixed container_of usage by storing codec pointer in spec structure
> - Added cancellation of delayed work when headphone is re-plugged
> - Limited the fix to specific device IDs (0x14f11f86, 0x14f11f87) based on testing
> - Added proper cleanup in remove function
Thanks for the patch, but unfortunately the patch isn't in a good
enough shape.
First off, the code change doesn't consider about the runtime PM.
That is, the work itself might wake up the runtime resume.
The other points are:
- The patch contains lots of magic numbers that should have been
represented with the defined constants.
- It touches the HD-audio codec verbs directly for pin detections and
amps, which should be rather done via the standard helpers --
otherwise it will break the whole regmap caches. This might be
intentional, but if so, it must be clearly commented.
- The work must be synced properly at suspend or similar operations,
too.
- The code assumes the fixed pin definitions without checking the
actual pin configurations. It won't work when the pin config
differs.
And, the most important point is that it's better to do the root cause
analysis again. Does this happen with the driver's auto-mute feature?
Or it's triggered by user-space like pulseaudio / pipewire?
And, if it's drivers' auto-mute, setting spec->gen.auto_mute_via_amp
has any influence?
thanks,
Takashi
>
> sound/hda/codecs/conexant.c | 73 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 73 insertions(+)
>
> diff --git a/sound/hda/codecs/conexant.c b/sound/hda/codecs/conexant.c
> index 5fcbc1312c69..f2f447ab749e 100644
> --- a/sound/hda/codecs/conexant.c
> +++ b/sound/hda/codecs/conexant.c
> @@ -43,6 +43,10 @@ struct conexant_spec {
> unsigned int gpio_mute_led_mask;
> unsigned int gpio_mic_led_mask;
> bool is_cx11880_sn6140;
> +
> + /* Pop noise mitigation */
> + struct hda_codec *codec;
> + struct delayed_work pop_mitigation_work;
> };
>
>
> @@ -212,10 +216,74 @@ static void cx_auto_shutdown(struct hda_codec *codec)
>
> static void cx_remove(struct hda_codec *codec)
> {
> + struct conexant_spec *spec = codec->spec;
> +
> + cancel_delayed_work_sync(&spec->pop_mitigation_work);
> cx_auto_shutdown(codec);
> snd_hda_gen_remove(codec);
> }
>
> +static void mute_unmute_speaker(struct hda_codec *codec, hda_nid_t nid, bool mute)
> +{
> + unsigned int conn_sel, dac, conn_list, gain_left, gain_right;
> +
> + conn_sel = snd_hda_codec_read(codec, nid, 0, 0xf01, 0x0);
> + conn_list = snd_hda_codec_read(codec, nid, 0, 0xf02, 0x0);
> +
> + dac = ((conn_list >> (conn_sel * 8)) & 0xff);
> + if (dac == 0)
> + return;
> +
> + gain_left = snd_hda_codec_read(codec, dac, 0, 0xba0, 0x0);
> + gain_right = snd_hda_codec_read(codec, dac, 0, 0xb80, 0x0);
> +
> + if (mute) {
> + gain_left |= 0x80;
> + gain_right |= 0x80;
> + } else {
> + gain_left &= (~(0x80));
> + gain_right &= (~(0x80));
> + }
> +
> + snd_hda_codec_write(codec, dac, 0, 0x3a0, gain_left);
> + snd_hda_codec_write(codec, dac, 0, 0x390, gain_right);
> +
> + if (mute) {
> + snd_hda_codec_write(codec, nid, 0, 0x707, 0);
> + codec_dbg(codec, "mute_speaker, set 0x%x PinCtrl to 0.\n", nid);
> + } else {
> + snd_hda_codec_write(codec, nid, 0, 0x707, 0x40);
> + codec_dbg(codec, "unmute_speaker, set 0x%x PinCtrl to 0x40.\n", nid);
> + }
> +}
> +
> +static void pop_mitigation_worker(struct work_struct *work)
> +{
> + struct conexant_spec *spec = container_of(work, struct conexant_spec,
> + pop_mitigation_work.work);
> + struct hda_codec *codec = spec->codec;
> +
> + mute_unmute_speaker(codec, 0x17, false);
> +}
> +
> +static void cx_auto_pop_mitigation(struct hda_codec *codec,
> + struct hda_jack_callback *event)
> +{
> + struct conexant_spec *spec = codec->spec;
> + int phone_present;
> +
> + phone_present = snd_hda_codec_read(codec, 0x16, 0, 0xf09, 0x0);
> + if (!(phone_present & 0x80000000)) {
> + /* Headphone unplugged, mute speaker immediately */
> + mute_unmute_speaker(codec, 0x17, true);
> + /* Schedule unmute after 20ms delay */
> + schedule_delayed_work(&spec->pop_mitigation_work, msecs_to_jiffies(20));
> + } else {
> + /* Headphone plugged in, cancel any pending unmute */
> + cancel_delayed_work_sync(&spec->pop_mitigation_work);
> + }
> +}
> +
> static void cx_process_headset_plugin(struct hda_codec *codec)
> {
> unsigned int val;
> @@ -1178,6 +1246,9 @@ static int cx_probe(struct hda_codec *codec, const struct hda_device_id *id)
> spec = kzalloc(sizeof(*spec), GFP_KERNEL);
> if (!spec)
> return -ENOMEM;
> +
> + spec->codec = codec;
> + INIT_DELAYED_WORK(&spec->pop_mitigation_work, pop_mitigation_worker);
> snd_hda_gen_spec_init(&spec->gen);
> codec->spec = spec;
>
> @@ -1187,6 +1258,8 @@ static int cx_probe(struct hda_codec *codec, const struct hda_device_id *id)
> case 0x14f11f87:
> spec->is_cx11880_sn6140 = true;
> snd_hda_jack_detect_enable_callback(codec, 0x19, cx_update_headset_mic_vref);
> + /* Enable pop noise mitigation for both codecs */
> + snd_hda_jack_detect_enable_callback(codec, 0x16, cx_auto_pop_mitigation);
> break;
> }
>
> --
> 2.25.1
>
Powered by blists - more mailing lists