Skip to contentSkip to conversation

Properly fix headphone noise on the XPS 13 and other ALC256 devices

PATCH In review Thomas Hebb 3 participants 7 messages Mar 30, 2020 – Mar 31, 2020

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v2 cover Properly fix headphone noise on the XPS 13 and other ALC256 devices

The root cause of various pervasive audio problems on the XPS 13 9350/9360, mostly relating to the headphone jack, turns out to be an undocumented feature of the ALC256 and similar codecs that routes audio along paths not exposed in the HDA node graph. The best we've had so far to configure this feature is magic numbers provided by Realtek, none of which have fully fixed all issues.

This series documents the "PC Beep Hidden Register", which controls the feature and which I've reverse engineered using black box techniques, and uses my findings to hopefully fix the headphone issues on my XPS 13 once and for all.

Changes in v2:
- Change fixed value from 0x4727 to 0x5757, which should behave identically, on advice from Kailang.

Thomas Hebb (3):
ALSA: doc: Document PC Beep Hidden Register on Realtek ALC256
ALSA: hda/realtek - Set principled PC Beep configuration for ALC256
ALSA: hda/realtek - Remove now-unnecessary XPS 13 headphone noise
fixups

 Documentation/sound/hd-audio/index.rst        |   1 +
 Documentation/sound/hd-audio/models.rst       |   2 -
 .../sound/hd-audio/realtek-pc-beep.rst        | 129 ++++++++++++++++++
 sound/pci/hda/patch_realtek.c                 |  49 ++-----
 4 files changed, 139 insertions(+), 42 deletions(-)
 create mode 100644 Documentation/sound/hd-audio/realtek-pc-beep.rst
Signature · 1 line
2.25.2
#1 lore raw

PATCH v2 1/3 ALSA: doc: Document PC Beep Hidden Register on Realtek ALC256

This codec (among others) has a hidden set of audio routes, apparently designed to allow PC Beep output without a mixer widget on the output path, which are controlled by an undocumented Realtek vendor register. The default configuration of these routes means that certain inputs aren't accessible, necessitating driver control of the register. However, Realtek has provided no documentation of the register, instead opting to fix issues by providing magic numbers, most of which have been at least somewhat erroneous. These magic numbers then get copied by others into model-specific fixups, leading to a fragmented and buggy set of configurations.

To get out of this situation, I've reverse engineered the register by flipping bits and observing how the codec's behavior changes. This commit documents my findings. It does not change any code.

---

Changes in v2: None

 Documentation/sound/hd-audio/index.rst        |   1 +
 .../sound/hd-audio/realtek-pc-beep.rst        | 129 ++++++++++++++++++
 2 files changed, 130 insertions(+)
 create mode 100644 Documentation/sound/hd-audio/realtek-pc-beep.rst
Documentation/sound/hd-audio/index.rst +1 -0
index f8a72ffffe66..6e12de9fc34e 100644
@@ -8,3 +8,4 @@ HD-Audio
    models
    controls
    dp-mst
+   realtek-pc-beep
Documentation/sound/hd-audio/realtek-pc-beep.rst new file +129 -0

129 lines not on this page · raw message

Signature · 1 line
2.25.2
maintainer
#2 lore raw

Re: [PATCH v2 1/3] ALSA: doc: Document PC Beep Hidden Register on Realtek ALC256

Hi

[This is an automated email]

This commit has been processed because it contains a -stable tag. The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v5.5.13, v5.4.28, v4.19.113, v4.14.174, v4.9.217, v4.4.217.

v5.5.13: Build OK!
v5.4.28: Build OK!
v4.19.113: Build OK!
v4.14.174: Build OK!
v4.9.217: Failed to apply! Possible dependencies:

    76ab4e15158c ("ALSA: doc: ReSTize HD-Audio-DP-MST-audio.txt")
    8551914a5e19 ("ALSA: doc: ReSTize alsa-driver-api document")
    9000d69925ac ("ALSA: doc: ReSTize HD-Audio document")
    a4caad753f0c ("ALSA: doc: ReSTize HD-Audio-Models document")
    fe0abd18e1ef ("ALSA: doc: ReSTize HD-Audio-Controls document")

v4.4.217: Failed to apply! Possible dependencies:
34d505193bd1 ("cfg80211: basic support for PBSS network type") 35eb8f7b1a37 ("cfg80211: Improve Connect/Associate command documentation") 38de03d2a289 ("nl80211: add feature for BSS selection support") 463c35fb7981 ("ALSA: Add documentation about HD-audio DP MST") 76ab4e15158c ("ALSA: doc: ReSTize HD-Audio-DP-MST-audio.txt") 819bf593767c ("docs-rst: sphinxify 802.11 documentation")
8551914a5e19 ("ALSA: doc: ReSTize alsa-driver-api document") 9000d69925ac ("ALSA: doc: ReSTize HD-Audio document")
a4caad753f0c ("ALSA: doc: ReSTize HD-Audio-Models document") ba6fbacf9c07 ("cfg80211: Add option to specify previous BSSID for Connect command") bf1ecd210541 ("cfg80211: Allow cfg80211_connect_result() errors to be distinguished") e705498945ad ("cfg80211: Add option to report the bss entry in connect result") fa44b7ec9bc4 ("ALSA: hda - Update documentation")
fe0abd18e1ef ("ALSA: doc: ReSTize HD-Audio-Controls document")

NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

Signature · 2 lines
Thanks
Sasha
#3 lore raw

PATCH v2 2/3 ALSA: hda/realtek - Set principled PC Beep configuration for ALC256

In reply to Thomas Hebb

The Realtek PC Beep Hidden Register[1] is currently set by patch_realtek.c in two different places:

In alc_fill_eapd_coef(), it's set to the value 0x5757, corresponding to non-beep input on 1Ah and no 1Ah loopback to either headphones or speakers. (Although, curiously, the loopback amp is still enabled.) This write was added fairly recently by commit e3743f431143 ("ALSA:
hda/realtek - Dell headphone has noise on unmute for ALC236") and is a safe default. However, it happens in the wrong place:
alc_fill_eapd_coef() runs on module load and cold boot but not on S3 resume, meaning the register loses its value after suspend.

Conversely, in alc256_init(), the register is updated to unset bit 13 (disable speaker loopback) and set bit 5 (set non-beep input on 1Ah). Although this write does run on S3 resume, it's not quite enough to fix up the register's default value of 0x3717. What's missing is a set of bit 14 to disable headphone loopback. Without that, we end up with a feedback loop where the headphone jack is being driven by amplified samples of itself[2].

This change eliminates the update in alc256_init() and replaces it with the 0x5757 write from alc_fill_eapd_coef(). Kailang says that 0x5757 is supposed to be the codec's default value, so using it will make debugging easier for Realtek.

Affects the ALC255, ALC256, ALC257, ALC235, and ALC236 codecs.

[1] Newly documented in Documentation/sound/hd-audio/realtek-pc-beep.rst

[2] Setting the "Headphone Mic Boost" control from userspace changes this feedback loop and has been a widely-shared workaround for headphone noise on laptops like the Dell XPS 13 9350. This commit eliminates the feedback loop and makes the workaround unnecessary.

Fixes e3743f431143 ("ALSA: hda/realtek - Dell headphone has noise on unmute for ALC236")

---

Changes in v2:
- Change fixed value from 0x4727 to 0x5757, which should behave identically, on advice from Kailang.

 sound/pci/hda/patch_realtek.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)
sound/pci/hda/patch_realtek.c +9 -6
index 63e1a56f705b..9efb0a858c64 100644
@@ -367,7 +367,9 @@ static void alc_fill_eapd_coef(struct hda_codec *codec)
 	case 0x10ec0215:
 	case 0x10ec0233:
 	case 0x10ec0235:
+	case 0x10ec0236:
 	case 0x10ec0255:
+	case 0x10ec0256:
 	case 0x10ec0257:
 	case 0x10ec0282:
 	case 0x10ec0283:
@@ -379,11 +381,6 @@ static void alc_fill_eapd_coef(struct hda_codec *codec)
 	case 0x10ec0300:
 		alc_update_coef_idx(codec, 0x10, 1<<9, 0);
 		break;
-	case 0x10ec0236:
-	case 0x10ec0256:
-		alc_write_coef_idx(codec, 0x36, 0x5757);
-		alc_update_coef_idx(codec, 0x10, 1<<9, 0);
-		break;
 	case 0x10ec0275:
 		alc_update_coef_idx(codec, 0xe, 0, 1<<0);
 		break;
@@ -3269,7 +3266,13 @@ static void alc256_init(struct hda_codec *codec)
 	alc_update_coefex_idx(codec, 0x57, 0x04, 0x0007, 0x4); /* Hight power */
 	alc_update_coefex_idx(codec, 0x53, 0x02, 0x8000, 1 << 15); /* Clear bit */
 	alc_update_coefex_idx(codec, 0x53, 0x02, 0x8000, 0 << 15);
-	alc_update_coef_idx(codec, 0x36, 1 << 13, 1 << 5); /* Switch pcbeep path to Line in path*/
+	/*
+	 * Expose headphone mic (or possibly Line In on some machines) instead
+	 * of PC Beep on 1Ah, and disable 1Ah loopback for all outputs. See
+	 * Documentation/sound/hd-audio/realtek-pc-beep.rst for details of
+	 * this register.
+	 */
+	alc_write_coef_idx(codec, 0x36, 0x5757);
 }
 
 static void alc256_shutup(struct hda_codec *codec)
Signature · 1 line
2.25.2
#4 lore raw

PATCH v2 3/3 ALSA: hda/realtek - Remove now-unnecessary XPS 13 headphone noise fixups

In reply to Thomas Hebb

patch_realtek.c has historically failed to properly configure the PC Beep Hidden Register for the ALC256 codec (among others). Depending on your kernel version, symptoms of this misconfiguration can range from chassis noise, picked up by a poorly-shielded PCBEEP trace, getting amplified and played on your internal speaker and/or headphones to loud feedback, which responds to the "Headphone Mic Boost" ALSA control, getting played through your headphones. For details of the problem, see the patch in this series titled "ALSA: hda/realtek - Set principled PC Beep configuration for ALC256", which fixes the configuration.

These symptoms have been most noticed on the Dell XPS 13 9350 and 9360, popular laptops that use the ALC256. As a result, several model-specific fixups have been introduced to try and fix the problem, the most egregious of which locks the "Headphone Mic Boost" control as a hack to minimize noise from a feedback loop that shouldn't have been there in the first place.

Now that the underlying issue has been fixed, remove all these fixups. Remaining fixups needed by the XPS 13 are all picked up by existing pin quirks.

This change should, for the XPS 13 9350/9360

- Significantly increase volume and audio quality on headphones
- Eliminate headphone popping on suspend/resume
- Allow "Headphone Mic Boost" to be set again, making the headphone jack fully usable as a microphone jack too.

Fixes 8c69729b4439 ("ALSA: hda - Fix headphone noise after Dell XPS 13 resume back from S3")
Fixes 423cd785619a ("ALSA: hda - Fix headphone noise on Dell XPS 13 9360")
Fixes e4c9fd10eb21 ("ALSA: hda - Apply headphone noise quirk for another Dell XPS 13 variant")
Fixes 1099f48457d0 ("ALSA: hda/realtek: Reduce the Headphone static noise on XPS 9350/9360")

---

Changes in v2: None

 Documentation/sound/hd-audio/models.rst |  2 --
 sound/pci/hda/patch_realtek.c           | 34 -------------------------
 2 files changed, 36 deletions(-)
Documentation/sound/hd-audio/models.rst +0 -2
index 11298f0ce44d..0ea967d34583 100644
@@ -216,8 +216,6 @@ alc298-dell-aio
     ALC298 fixups on Dell AIO machines
 alc275-dell-xps
     ALC275 fixups on Dell XPS models
-alc256-dell-xps13
-    ALC256 fixups on Dell XPS13
 lenovo-spk-noise
     Workaround for speaker noise on Lenovo machines
 lenovo-hotkey
sound/pci/hda/patch_realtek.c +0 -34
index 9efb0a858c64..38511f3fccfd 100644
@@ -5378,17 +5378,6 @@ static void alc271_hp_gate_mic_jack(struct hda_codec *codec,
 	}
 }
 
-static void alc256_fixup_dell_xps_13_headphone_noise2(struct hda_codec *codec,
-						      const struct hda_fixup *fix,
-						      int action)
-{
-	if (action != HDA_FIXUP_ACT_PRE_PROBE)
-		return;
-
-	snd_hda_codec_amp_stereo(codec, 0x1a, HDA_INPUT, 0, HDA_AMP_VOLMASK, 1);
-	snd_hda_override_wcaps(codec, 0x1a, get_wcaps(codec, 0x1a) & ~AC_WCAP_IN_AMP);
-}
-
 static void alc269_fixup_limit_int_mic_boost(struct hda_codec *codec,
 					     const struct hda_fixup *fix,
 					     int action)
@@ -5866,8 +5855,6 @@ enum {
 	ALC298_FIXUP_DELL1_MIC_NO_PRESENCE,
 	ALC298_FIXUP_DELL_AIO_MIC_NO_PRESENCE,
 	ALC275_FIXUP_DELL_XPS,
-	ALC256_FIXUP_DELL_XPS_13_HEADPHONE_NOISE,
-	ALC256_FIXUP_DELL_XPS_13_HEADPHONE_NOISE2,
 	ALC293_FIXUP_LENOVO_SPK_NOISE,
 	ALC233_FIXUP_LENOVO_LINE2_MIC_HOTKEY,
 	ALC255_FIXUP_DELL_SPK_NOISE,
@@ -6607,23 +6594,6 @@ static const struct hda_fixup alc269_fixups[] = {
 			{}
 		}
 	},
-	[ALC256_FIXUP_DELL_XPS_13_HEADPHONE_NOISE] = {
-		.type = HDA_FIXUP_VERBS,
-		.v.verbs = (const struct hda_verb[]) {
-			/* Disable pass-through path for FRONT 14h */
-			{0x20, AC_VERB_SET_COEF_INDEX, 0x36},
-			{0x20, AC_VERB_SET_PROC_COEF, 0x1737},
-			{}
-		},
-		.chained = true,
-		.chain_id = ALC255_FIXUP_DELL1_MIC_NO_PRESENCE
-	},
-	[ALC256_FIXUP_DELL_XPS_13_HEADPHONE_NOISE2] = {
-		.type = HDA_FIXUP_FUNC,
-		.v.func = alc256_fixup_dell_xps_13_headphone_noise2,
-		.chained = true,
-		.chain_id = ALC256_FIXUP_DELL_XPS_13_HEADPHONE_NOISE
-	},
 	[ALC293_FIXUP_LENOVO_SPK_NOISE] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = alc_fixup_disable_aamix,
@@ -7117,17 +7087,14 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x1028, 0x06de, "Dell", ALC293_FIXUP_DISABLE_AAMIX_MULTIJACK),
 	SND_PCI_QUIRK(0x1028, 0x06df, "Dell", ALC293_FIXUP_DISABLE_AAMIX_MULTIJACK),
 	SND_PCI_QUIRK(0x1028, 0x06e0, "Dell", ALC293_FIXUP_DISABLE_AAMIX_MULTIJACK),
-	SND_PCI_QUIRK(0x1028, 0x0704, "Dell XPS 13 9350", ALC256_FIXUP_DELL_XPS_13_HEADPHONE_NOISE2),
 	SND_PCI_QUIRK(0x1028, 0x0706, "Dell Inspiron 7559", ALC256_FIXUP_DELL_INSPIRON_7559_SUBWOOFER),
 	SND_PCI_QUIRK(0x1028, 0x0725, "Dell Inspiron 3162", ALC255_FIXUP_DELL_SPK_NOISE),
 	SND_PCI_QUIRK(0x1028, 0x0738, "Dell Precision 5820", ALC269_FIXUP_NO_SHUTUP),
-	SND_PCI_QUIRK(0x1028, 0x075b, "Dell XPS 13 9360", ALC256_FIXUP_DELL_XPS_13_HEADPHONE_NOISE2),
 	SND_PCI_QUIRK(0x1028, 0x075c, "Dell XPS 27 7760", ALC298_FIXUP_SPK_VOLUME),
 	SND_PCI_QUIRK(0x1028, 0x075d, "Dell AIO", ALC298_FIXUP_SPK_VOLUME),
 	SND_PCI_QUIRK(0x1028, 0x07b0, "Dell Precision 7520", ALC295_FIXUP_DISABLE_DAC3),
 	SND_PCI_QUIRK(0x1028, 0x0798, "Dell Inspiron 17 7000 Gaming", ALC256_FIXUP_DELL_INSPIRON_7559_SUBWOOFER),
 	SND_PCI_QUIRK(0x1028, 0x080c, "Dell WYSE", ALC225_FIXUP_DELL_WYSE_MIC_NO_PRESENCE),
-	SND_PCI_QUIRK(0x1028, 0x082a, "Dell XPS 13 9360", ALC256_FIXUP_DELL_XPS_13_HEADPHONE_NOISE2),
 	SND_PCI_QUIRK(0x1028, 0x084b, "Dell", ALC274_FIXUP_DELL_AIO_LINEOUT_VERB),
 	SND_PCI_QUIRK(0x1028, 0x084e, "Dell", ALC274_FIXUP_DELL_AIO_LINEOUT_VERB),
 	SND_PCI_QUIRK(0x1028, 0x0871, "Dell Precision 3630", ALC255_FIXUP_DELL_HEADSET_MIC),
@@ -7480,7 +7447,6 @@ static const struct hda_model_fixup alc269_fixup_models[] = {
 	{.id = ALC298_FIXUP_DELL1_MIC_NO_PRESENCE, .name = "alc298-dell1"},
 	{.id = ALC298_FIXUP_DELL_AIO_MIC_NO_PRESENCE, .name = "alc298-dell-aio"},
 	{.id = ALC275_FIXUP_DELL_XPS, .name = "alc275-dell-xps"},
-	{.id = ALC256_FIXUP_DELL_XPS_13_HEADPHONE_NOISE, .name = "alc256-dell-xps13"},
 	{.id = ALC293_FIXUP_LENOVO_SPK_NOISE, .name = "lenovo-spk-noise"},
 	{.id = ALC233_FIXUP_LENOVO_LINE2_MIC_HOTKEY, .name = "lenovo-hotkey"},
 	{.id = ALC255_FIXUP_DELL_SPK_NOISE, .name = "dell-spk-noise"},
Signature · 1 line
2.25.2
maintainer
#5 lore raw

Re: [PATCH v2 3/3] ALSA: hda/realtek - Remove now-unnecessary XPS 13 headphone noise fixups

Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: 8c69729b4439 ("ALSA: hda - Fix headphone noise after Dell XPS 13 resume back from S3").

The bot has tested the following trees: v5.5.13, v5.4.28, v4.19.113, v4.14.174, v4.9.217, v4.4.217.

v5.5.13: Build OK!
v5.4.28: Build OK!
v4.19.113: Build OK!
v4.14.174: Failed to apply! Possible dependencies:
1099f48457d0 ("ALSA: hda/realtek: Reduce the Headphone static noise on XPS 9350/9360") 28d1d6d2f314 ("ALSA: hda - Add model string for Intel reference board quirk") a26d96c7802e ("ALSA: hda/realtek - Comprehensive model list for ALC259 & co") c0ca5eced222 ("ALSA: hda/realtek - Reduce click noise on Dell Precision 5820 headphone") c1350bff69d1 ("ALSA: hda - Clean up ALC299 init code")
da911b1f5e98 ("ALSA: hda/realtek - update ALC225 depop optimize")

v4.9.217: Failed to apply! Possible dependencies:
8551914a5e19 ("ALSA: doc: ReSTize alsa-driver-api document") 9000d69925ac ("ALSA: doc: ReSTize HD-Audio document") a26d96c7802e ("ALSA: hda/realtek - Comprehensive model list for ALC259 & co") a4caad753f0c ("ALSA: doc: ReSTize HD-Audio-Models document") a79e7df97592 ("ALSA: hda - Update the list of quirk models")

v4.4.217: Failed to apply! Possible dependencies:
34d505193bd1 ("cfg80211: basic support for PBSS network type") 35eb8f7b1a37 ("cfg80211: Improve Connect/Associate command documentation") 38de03d2a289 ("nl80211: add feature for BSS selection support") 819bf593767c ("docs-rst: sphinxify 802.11 documentation")
8551914a5e19 ("ALSA: doc: ReSTize alsa-driver-api document") 9000d69925ac ("ALSA: doc: ReSTize HD-Audio document")
a26d96c7802e ("ALSA: hda/realtek - Comprehensive model list for ALC259 & co") a4caad753f0c ("ALSA: doc: ReSTize HD-Audio-Models document") ba6fbacf9c07 ("cfg80211: Add option to specify previous BSSID for Connect command") bf1ecd210541 ("cfg80211: Allow cfg80211_connect_result() errors to be distinguished") e705498945ad ("cfg80211: Add option to report the bss entry in connect result") fa44b7ec9bc4 ("ALSA: hda - Update documentation")

NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

Signature · 2 lines
Thanks
Sasha
#6 lore raw
In reply to Thomas Hebb
Quoted 18 lines · Thomas Hebb

The root cause of various pervasive audio problems on the XPS 13 9350/9360, mostly relating to the headphone jack, turns out to be an undocumented feature of the ALC256 and similar codecs that routes audio along paths not exposed in the HDA node graph. The best we've had so far to configure this feature is magic numbers provided by Realtek, none of which have fully fixed all issues.

This series documents the "PC Beep Hidden Register", which controls the feature and which I've reverse engineered using black box techniques, and uses my findings to hopefully fix the headphone issues on my XPS 13 once and for all.

Changes in v2:
- Change fixed value from 0x4727 to 0x5757, which should behave identically, on advice from Kailang.

Quoted 5 lines

Thomas Hebb (3):
ALSA: doc: Document PC Beep Hidden Register on Realtek ALC256
ALSA: hda/realtek - Set principled PC Beep configuration for ALC256
ALSA: hda/realtek - Remove now-unnecessary XPS 13 headphone noise
fixups

Now applied all three patches. The patch 2 had an incorrect Fixes tag (pointing to the stable commit instead of the original commit) so I corrected locally.

thanks,

Takashi