Re: [PATCH] ALSA: hda - More pop noise fixes for Dell XPS 13 9333

[Gabriele Mazzotta <gabriele.mzt@gmail.com>]

On Friday 08 August 2014 08:11:18 Takashi Iwai wrote:
> At Thu, 07 Aug 2014 18:35:39 +0200,
> Gabriele Mazzotta wrote:
> > 
> > On init, mic-in is always set as input source, indipendently on what
> > is plugged in. Since setting/unsetting mic-in as input source causes
> > a pop noise, make sure the internal microphone is selected as input
> > source on boot.
> > 
> > On shutdown, make sure the codec is not suspended as that would cause
> > a pop noise.
> > 
> > Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> > ---
> >  sound/pci/hda/hda_codec.c     |  2 ++
> >  sound/pci/hda/hda_codec.h     |  1 +
> >  sound/pci/hda/patch_realtek.c | 12 ++++++++++++
> >  3 files changed, 15 insertions(+)
> > 
> > diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> > index 4c20277..92d8292 100644
> > --- a/sound/pci/hda/hda_codec.c
> > +++ b/sound/pci/hda/hda_codec.c
> > @@ -5348,6 +5348,8 @@ void snd_hda_bus_reboot_notify(struct hda_bus *bus)
> >  	if (!bus)
> >  		return;
> >  	list_for_each_entry(codec, &bus->codec_list, list) {
> > +		if (codec->resume_at_reboot)
> > +			hda_call_codec_resume(codec);
> 
> Do you really need the resume at reboot?  I thought the codec needs
> power down at reboot, i.e. rather it needs suspend.

Yes, the pop noise happens if it's suspended.
Now that I payed more attention, the pop noise happens only when 
rebooting the laptop and not when powering it down.
Also, the noise is heard quite late, I think the kernel is no longer 
running when I hear it.
I will try to better understand what is actually happening and why
resuming prevents the noise.

> Since you apply actually two individual changes, it's better to split
> patches: one for adding the power-down at reboot, another for
> initializing cur_mux properly.
> 
> Also, it's recommended to put a bugzilla link in the changelog, so
> that other people can follow the information and discussion.
> 
> Last but not least, please put maintainers to Cc at the next time, so
> that it won't be missed.

I will do it.

Thanks,

Gabriele

> 
> thanks,
> 
> Takashi
> 
> >  		if (hda_codec_is_power_on(codec) &&
> >  		    codec->patch_ops.reboot_notify)
> >  			codec->patch_ops.reboot_notify(codec);
> > diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
> > index 5825aa1..5c3c66e 100644
> > --- a/sound/pci/hda/hda_codec.h
> > +++ b/sound/pci/hda/hda_codec.h
> > @@ -366,6 +366,7 @@ struct hda_codec {
> >  	unsigned int cached_write:1;	/* write only to caches */
> >  	unsigned int dp_mst:1; /* support DP1.2 Multi-stream transport */
> >  	unsigned int dump_coef:1; /* dump processing coefs in codec proc file */
> > +	unsigned int resume_at_reboot:1; /* resume codec at reboot */
> >  #ifdef CONFIG_PM
> >  	unsigned int power_on :1;	/* current (global) power-state */
> >  	unsigned int d3_stop_clk:1;	/* support D3 operation without BCLK */
> > diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> > index b60824e..defaa2a 100644
> > --- a/sound/pci/hda/patch_realtek.c
> > +++ b/sound/pci/hda/patch_realtek.c
> > @@ -4018,8 +4018,20 @@ static void alc_fixup_dell_xps13(struct hda_codec *codec,
> >  {
> >  	if (action == HDA_FIXUP_ACT_PROBE) {
> >  		struct alc_spec *spec = codec->spec;
> > +		struct hda_input_mux *imux = &spec->gen.input_mux;
> > +		int i;
> > +
> >  		spec->shutup = alc_no_shutup;
> >  		codec->power_filter = alc_power_filter_xps13;
> > +		codec->resume_at_reboot = 1;
> > +
> > +		/* Make the internal mic the default input source. */
> > +		for (i = 0; i < imux->num_items; i++) {
> > +			if (spec->gen.imux_pins[i] == 0x12) {
> > +				spec->gen.cur_mux[0] = i;
> > +				break;
> > +			}
> > +		}
> >  	}
> >  }
> >  
>