Bug 76611

Summary: Dell XPS 13 9333 - Intel HDA Realtek ALC668: frequent pop noises
Product: Drivers Reporter: Gabriele Mazzotta (gabriele.mzt)
Component: Sound(ALSA)Assignee: Jaroslav Kysela (perex)
Status: RESOLVED CODE_FIX    
Severity: normal CC: gabriele.mzt, superquad.vortex2, tiwai
Priority: P1    
Hardware: x86-64   
OS: Linux   
Kernel Version: 3.14 Subsystem:
Regression: No Bisected commit-id:
Attachments: alsa-info.sh output
Keep AFG to D0
verbs trace
Pop noises fix
Pop noise fix
Pop noise fix
Fix boot pop noise
dmesg: headphones

Description Gabriele Mazzotta 2014-05-21 11:34:10 UTC
A pop noise can be heard whenever the audio card goes from D0 to D3. The same noise is heard on startup.

A louder pop noise can be heard when the audio card goes from D0 to D3 and an even louder pop noise is heard on shutdown.
Unloading snd_hda_intel right before the machine is turned off greatly reduces the pop noise on shutodwn.

These noises are reproduced by both the laptop speakers and plugged headphones. The speakers reproduces these noises even when headphones are plugged in.

It's also worth mentioning that no pop noises are heard when all the nodes but AFG are set to D3. Keeping AFG in D0 also prevents some other electrical background noises that can be usually heard when headphones are used.

When Windoes 8 is used, no noises can be heard (including the background electrical noises), except for a barely noticeable pop noise that can be heard with headphones.

This is the audio card state in Windows 8 when no sounds are played
(I guess "S0 -> D0" is what prevents the electrical noises):
-------
Current power state:
D3

Power capabilities:
00000089
PDCAP_D0_SUPPORTED
PDCAP_D3_SUPPORTED
PDCAP_WAKE_FROM_D3_SUPPORTED

Power state mappings:
S0 -> D0
S1 -> D3
S2 -> Unspecified
S3 -> D3
S4 -> D3
S5 -> D3
Comment 1 Takashi Iwai 2014-05-21 12:17:43 UTC
It's likely the power-saving behavior.  Please give alsa-info.sh output (run with --no-upload option) for further analysis.
Comment 2 Gabriele Mazzotta 2014-05-21 12:45:10 UTC
Created attachment 136921 [details]
alsa-info.sh output

Here the output of alsa-info.sh
Comment 3 Takashi Iwai 2014-05-21 12:55:13 UTC
Try to comment out the line "spec->shutup = alc_eapd_shutup;" in patch_alc662().
Does it improve?  If yes, revert it and comment out either alc_auto_setup_eapd() or alc_hda_shutup_pins() call, and figure out which causes the problem.  (It might be both.)
Comment 4 Gabriele Mazzotta 2014-05-21 13:02:11 UTC
I tried it already and unfortunately nothing changes.

Just an additional note: the pop noise on shutdown is loud only if the audio card is in D0. If it's already in D3, there are no noises.
Comment 5 Takashi Iwai 2014-05-21 13:13:58 UTC
OK, so an easy solution is to keep AFG to D0.
You can add the own power_filter to keep AFG to D0, for example,

static unsigned int afg_d0_power_filter(struct hda_codec *codec,
					hda_nid_t nid,
					unsigned int power_state)
{
	if (nid == codec->afg)
		return AC_PWRST_D0;
	return power_state;
}

and set codec->power_filter = afg_d0_power_filter in the fixup.
Comment 6 Gabriele Mazzotta 2014-05-21 13:24:16 UTC
Created attachment 136931 [details]
Keep AFG to D0

The problem is that there are no pop noises when I set the power_save parameter of snd_hda_intel to 0 and then change the states of all the nodes except AFG with hda-verb.

If I apply the patch here attached, there are still pop noises. I don't know what I'm doing wrong.

Is there a way to see what's the current power state? running "cat /proc/asound/card1/codec#0" will automatically set everything to D0, so I need an alternative way.
Comment 7 Takashi Iwai 2014-05-21 13:35:38 UTC
Maybe the combination of both power_filter and shutup disablement is needed?

You can't see the state from proc file, but you can trace the all verbs via tracepoint.  There is a brief section in HD-AUdio.txt.
Comment 8 Gabriele Mazzotta 2014-05-21 14:25:31 UTC
Created attachment 136941 [details]
verbs trace

Unfortunately the combination of the two has no effect.

I attached the trace of the verbs, I separated the two transitions (D3 -> D0 and D0 -> D3) with two comments. This is what I get forcing AFG to D0 and disabling the shutup.

hda-decode-verb returns "verbname = unknown" for most of them, so I don't know exactly what's happening.
Comment 9 Takashi Iwai 2014-05-21 14:56:22 UTC
You have to pass the value with "0x" prefix to hda-decode-verb while tracing output doesn't have the prefix.

The trace above was the result after disabling the shutup call?  It still contains EAPD and pinctl clearance.

for i in $(grep 'hda_send_cmd.*val=' trace | sed -e's/^.*val=//g'); do
  hda-decode-verb 0x$i
done

raw value = 0x01470c00
cid = 0, nid = 0x14, verb = 0x70c, parm = 0x00
raw value: verb = 0x70c, parm = 0x0
verbname = set_eapd_btl
raw value = 0x01570c00
cid = 0, nid = 0x15, verb = 0x70c, parm = 0x00
raw value: verb = 0x70c, parm = 0x0
verbname = set_eapd_btl
raw value = 0x01270700
cid = 0, nid = 0x12, verb = 0x707, parm = 0x00
raw value: verb = 0x707, parm = 0x0
verbname = set_pin_ctl
raw value = 0x01470700
cid = 0, nid = 0x14, verb = 0x707, parm = 0x00
raw value: verb = 0x707, parm = 0x0
verbname = set_pin_ctl
.....
raw value = 0x00270503
cid = 0, nid = 0x02, verb = 0x705, parm = 0x03
raw value: verb = 0x705, parm = 0x3
verbname = set_power_state
raw value = 0x00370503
cid = 0, nid = 0x03, verb = 0x705, parm = 0x03
raw value: verb = 0x705, parm = 0x3
verbname = set_power_state
raw value = 0x00470503
cid = 0, nid = 0x04, verb = 0x705, parm = 0x03
....
raw value: verb = 0xf05, parm = 0x0
verbname = get_power_state
....
Comment 10 Gabriele Mazzotta 2014-05-21 15:30:43 UTC
Instead of commenting out "spec->shutup = alc_eapd_shutup;" I changed the line: "spec->shutup = alc_fixup_no_shutup;". I think the problem was the call of snd_hda_shutup_pins() from alc_suspend().
 
Now the click noise is much lower, but still there, for both the transitions. It doesn't matter whether I force AFG to D0 or not, but I can't consider my tests reliable now as the cable of my headphones is damaged.
Comment 11 Takashi Iwai 2014-05-21 15:39:31 UTC
Right, I forgot that the driver calls snd_hda_shutup_pins() when spec->shutup is NULL.  So, this is the place to paper over.

Now the question is whether filtering D3 for AFG still makes sense...
Comment 12 Gabriele Mazzotta 2014-05-21 16:42:51 UTC
Created attachment 137001 [details]
Pop noises fix

I should test the attached patch with better headphones, but it seems to work.

On shutdown, if the sound card is not in D3 (which usually happens when the power_save parameter of snd_hda_intel is 0), there's still a pop noise.

I'm not sure about the speakers, I've rarely heard pop noises coming from them, but they are rare and not loud.
Comment 13 Gabriele Mazzotta 2014-05-23 14:17:02 UTC
I tested this patch a bit more and seems to work as expected.

The only problem now is the pop noise I can hear when the machine is turned on. The faint pop noise on shutdown is normal, it's really hardly noticeable and also Windows reproduces it, unlike the noisy one on startup.
What could be the reason of the pop noise on startup?

In addition to that, as I already wrote, if power_save is zero, the pop noise on shutdown becomes loud. Why is that? Any idea on how to prevent this from happening?
Comment 14 Takashi Iwai 2014-05-23 14:29:23 UTC
At shutdown/reboot, only reboot_notify callback is called, and in patch_realtek.c, it's alc_shutup().  That is, the power state doesn't change at shutdown/reboot.  This is the likely difference from the power-saved state.

You can try to create another function to call alc_shutdown() and the explicit power down of some widget nodes and pass it to codec->patch_ops.reboot_notifier  You can override it in HDA_FIXUP_ACT_PROBE action where it's called after the setup of the default patch_ops.

It'd difficult to guess what's the start up noise.  You need to identify by yourself by adding delays and debug print to each start up action, then blacklist the module from auto-start and start the module afterward.
Comment 15 Gabriele Mazzotta 2014-05-23 15:29:02 UTC
Created attachment 137261 [details]
Pop noise fix

Thank you for the hints.

Is what I did in the attached patched acceptable? Since the pop noise is heard when the codec is not suspended, I thought to enforce the suspension regardless the state of power_state. I don't know if there's a better way to do it, I tried few other things, but this is the only working way I found. If this solution is good enough, I won't look for alternative ways.

Regarding the startup noise, I will investigate further when I have some spare time and see if I can find the problem.
Comment 16 Takashi Iwai 2014-05-23 15:57:23 UTC
Calling it there isn't really good because in theory there can be multiple codecs on the bus.

Maybe it's better to introduce a new flag in struct hda_codec (e.g. hda_codec.suspend_at_reboot), and change snd_hda_bus_reboot_notify() accordingly like:

void snd_hda_bus_reboot_notify(struct hda_bus *bus)
{
	struct hda_codec *codec;

	if (!bus)
		return;
	list_for_each_entry(codec, &bus->codec_list, list) {
		if (hda_codec_is_power_on(codec)) {
                        if (codec->suspend_at_reboot)
                                hda_call_codec_suspend(codec, false);
		        else if (codec->patch_ops.reboot_notify)
			        codec->patch_ops.reboot_notify(codec);
                }
	}
}
Comment 17 Gabriele Mazzotta 2014-05-26 14:52:48 UTC
Created attachment 137441 [details]
Pop noise fix

Sorry for the late reply, but I had too many issues with my headphones.

It turned out that actually resuming the codec is better with properly working headphones. With my broken headphones, I noticed that in those moments when they worked correctly, no loud pop noises were heard when the codec wasn't suspended. It's worth mentioning that Windows always prevents unwanted noises.

Using your code above, this is more or less what I did:

void snd_hda_bus_reboot_notify(struct hda_bus *bus)
{
	struct hda_codec *codec;

	if (!bus)
		return;
	list_for_each_entry(codec, &bus->codec_list, list) {
		if (codec->resume_at_reboot)
			hda_call_codec_resume(codec);
		if (hda_codec_is_power_on(codec) &&
		    codec->patch_ops.reboot_notify)
			codec->patch_ops.reboot_notify(codec);
	}
}

I can't do proper tests though, I borrewed the headphones just for a short amount of time.

However, I could verify that the patch attached prevents the very frequent pop noises when power_save is not zero.

I couldn't find the reason of the pop noise on startup.
Comment 18 Takashi Iwai 2014-05-26 15:04:11 UTC
OK, then could you submit the current patch to alsa-devel ML so that I can apply it for 3.16 kernel?  We can continue debugging further after that.
Comment 19 Gabriele Mazzotta 2014-07-06 15:17:44 UTC
I finally got properly working headphones.

I confirm that the pop noise on is present when the audio card is suspended and resuming the codec does the trick.

The pop noise on boot disappears if I remove the pin override of node 0x19 (https://bugs.launchpad.net/ubuntu/+source/alsa-driver/+bug/1252733/comments/38)
Is there any reason to override the pin configuration?
Comment 20 Takashi Iwai 2014-07-07 08:35:46 UTC
The NID 0x19 override should be better asked on alsa-devel ML, Cc'ed Canonical guys found in the git commit.  IIRC, the pin is set up for enabling headset and headphone mic on the TRRS jack.
Comment 21 Gabriele Mazzotta 2014-08-06 15:39:10 UTC
Created attachment 145301 [details]
Fix boot pop noise

The pin configuration of 0x19 is required for microphones, I've just tested it. Headset works fine without it.

I found that the pop noise is heard when the vref of 0x19 is changed from 50 (mic) to HIZ (headphones and headset) and vice versa.

The pop noise on boot is due to the fact that my laptop cannot distinguish what's plugged in. No matter what I plug in, I always get "Headset jack set to headphone (default) mode."
What happens on boot is that the headset mode is wrongly set to mic-in mode and then changed soon after. This causes pop noises.

Is the misdetection a bug or a hardware limitation?

If it's not a bug, the patch attached prevent the pop noises on boot. I don't know if something better can be done.
Comment 22 Gabriele Mazzotta 2014-08-06 15:47:16 UTC
Sorry, the previous patch is completely wrong, the mode can't never be changed, even manually. I'll think of a better way to solve the problem.
Comment 23 Takashi Iwai 2014-08-06 15:55:14 UTC
At which timing does it show wrongly and correctly?  The alc_headset_mode_unplugged() may be called multiple times by different places.

The first call should be the initialization in snd_hda_codec_build_controls():
  snd_hda_codec_build_controls() -> alc_init() -> init_input_src() -> alc_update_headset_mode_hook()

and the second call should be in the fixup with HDA_FIXUP_ACT_INIT.  This forces to update the headset mode.

The rest calls should be at plugging/unplugging.  You can see the stack trace by adding dump_stack() call to alc_update_headset_mode().
Comment 24 Gabriele Mazzotta 2014-08-06 17:29:04 UTC
Created attachment 145311 [details]
dmesg: headphones

It seems that it's called more often. For the attached dmesg, I booted my laptop with the headphones plugged in.
I get the same result using a microphone or a headset.
Surprisingly, I get nothing pluggin and unplugging, I don't why. I remember I got something doing that.

I added dump_stack() at the beginning of alc_update_headset_mode() and you can see when the mode is changed by looking for "sound hdaudioC1D0: Headset jack set to".
What I get is "mic-in" -> "mic-in" -> "headphones", no matter what I plug.


Another thing. When I test some changes, I sometimes simply reload snd_hda_codec_realtek and snd_hda_intel instead of rebooting. Every once in a while, this makes the headphones reproduce a lot of loud noises. Booting Windows or powering off the machine is the only solution. A simple reboot does nothing. I don't know what this means.
Comment 25 Raymond 2014-08-07 03:42:10 UTC
https://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/commit/?id=2041d56464a067461d7cc21734a0f024587ed2ff

i think you have to ask the author to clarify the usage of headset mic jack and headset mic phantom jack is only applicable to dell notebook

so far the driver does not create headset mic jack control for any dell notebook 

this mean no dell notebook can disintinguish headset and headphone

http://bazaar.launchpad.net/~unity-settings-daemon-team/unity-settings-daemon/trunk/view/head:/plugins/media-keys/what-did-you-plug-in/pa-backend.c


Headphone Mic Jack - indicates headphone and mic-in mode share the same jack,
     i e, not two separate jacks. Hardware cannot distinguish between a
     headphone and a mic.
   Headset Mic Phantom Jack - indicates headset jack where hardware can not
     distinguish between headphones and headsets
   Headset Mic Jack - indicates headset jack where hardware can distinguish
     between headphones and headsets. There is no use popping up a dialog in
     this case, unless we already need to do this for the mic-in mode.
Comment 26 Gabriele Mazzotta 2014-08-07 05:30:18 UTC
I think the hardware is not capable of detecting the correct mode, I have to manually set the correct mode in Windows.

As for the pop noise on boot, I added few lines so that spec->gen.imux_pins[spec->gen.cur_mux[0]] will point to 0x12 (internal mic) when alc_update_headset_mode() is called the first time to avoid unwanted vref changes. spec->gen.cur_mux[0] is otherwise 0 and 0x19 (mic) is selected, even if no microphones is plugged in.

Regarding the jack state, alc_update_headset_mode() is not called every time the jack is plugged/unplugged because snd_hda_gen_hp_automute() returns before call_update_outputs() is called (both automute_speaker and automute_lo are 0) and so alc_update_headset_mode() is also never called.
I remembered it was called because I once added it at the end of alc_update_headset_jack_cb().

I also found out why 493a52a9b6645f61954580c7d4bd52fa62110934 didn't work when I tried it. Since call_update_outputs() is never called, also snd_hda_gen_update_outputs()->do_automute() is never called.
Comment 27 Gabriele Mazzotta 2014-08-07 05:39:22 UTC
Forget the last bit I wrote, the white noise does not go away with 493a52a9, sorry.
Comment 28 Raymond 2014-08-07 07:45:58 UTC
it seem that there is no way for you to switch between headset, headphone or Mic without using pulseaudio

the driver should setup the combo Jack match with the icon next to the Jack be default , 


may be headset Jack mode control which allow you to switch the combo Jack to headphone or mic when the icon is headset
Comment 29 Raymond 2014-08-08 02:33:43 UTC
https://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/commit/sound/pci/hda?id=73bdd597823e2231dc882577dbbaf8df92fe1775

the patch assume that there is only one combo Jack with/without dock station but those dell alienware notbooks using alc668 have one headset, one headphone Jack and one Mic jack which support 5.1 external speakers and have internal subwoofer


https://bugs.launchpad.net/ubuntu/+source/alsa-driver/+bug/1302090



https://bugs.launchpad.net/ubuntu/+source/alsa-driver/+bug/1332900



http://www.dell.com/us/p/alienware-18/pd

(2x) Audio Out 1/8" Ports (One compatible with inline mic headset)
  (1x) Line In Microphone 1/8" Port (retaskable for 5.1 analog audio output)

this is rather strange that the driver setup the headset Jack as headphone by default
Comment 30 Raymond 2014-08-08 07:44:54 UTC
seem it use capture source to switch the mode of the combo Jack but how do it handle the case when those alienware  notebooks have Mic jack


static void alc_update_headset_mode(struct hda_codec *codec)
+{
+	struct alc_spec *spec = codec->spec;
+
+	hda_nid_t mux_pin = spec->gen.imux_pins[spec->gen.cur_mux[0]];
+	hda_nid_t hp_pin = spec->gen.autocfg.hp_pins[0];
+
+	int new_headset_mode;
+
+	if (!snd_hda_jack_detect(codec, hp_pin))
+		new_headset_mode = ALC_HEADSET_MODE_UNPLUGGED;
+	else if (mux_pin == spec->headset_mic_pin)
+		new_headset_mode = ALC_HEADSET_MODE_HEADSET;
+	else if (mux_pin == spec->headphone_mic_pin)
+		new_headset_mode = ALC_HEADSET_MODE_MIC;
+	else
+		new_headset_mode = ALC_HEADSET_MODE_HEADPHONE;
+
Comment 31 Gabriele Mazzotta 2014-08-10 12:32:14 UTC
I'd say the code assumes there's just one combo jack and it can't really work as expected with those laptops.

Closing this bug as it's now fixed.