All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ALSA: hda - More pop noise fixes for Dell XPS 13 9333
@ 2014-08-07 16:35 Gabriele Mazzotta
  2014-08-08  6:11 ` Takashi Iwai
  0 siblings, 1 reply; 6+ messages in thread
From: Gabriele Mazzotta @ 2014-08-07 16:35 UTC (permalink / raw)
  To: alsa-devel

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);
 		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;
+			}
+		}
 	}
 }
 
-- 
2.1.0.rc1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ALSA: hda - More pop noise fixes for Dell XPS 13 9333
  2014-08-07 16:35 [PATCH] ALSA: hda - More pop noise fixes for Dell XPS 13 9333 Gabriele Mazzotta
@ 2014-08-08  6:11 ` Takashi Iwai
  2014-08-08  8:49   ` Gabriele Mazzotta
  0 siblings, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2014-08-08  6:11 UTC (permalink / raw)
  To: Gabriele Mazzotta; +Cc: alsa-devel

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.

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.


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;
> +			}
> +		}
>  	}
>  }
>  
> -- 
> 2.1.0.rc1
> 
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ALSA: hda - More pop noise fixes for Dell XPS 13 9333
  2014-08-08  6:11 ` Takashi Iwai
@ 2014-08-08  8:49   ` Gabriele Mazzotta
  2014-08-08 10:13     ` Takashi Iwai
  0 siblings, 1 reply; 6+ messages in thread
From: Gabriele Mazzotta @ 2014-08-08  8:49 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

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;
> > +			}
> > +		}
> >  	}
> >  }
> >  
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ALSA: hda - More pop noise fixes for Dell XPS 13 9333
  2014-08-08  8:49   ` Gabriele Mazzotta
@ 2014-08-08 10:13     ` Takashi Iwai
  2014-08-08 15:01       ` Gabriele Mazzotta
  0 siblings, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2014-08-08 10:13 UTC (permalink / raw)
  To: Gabriele Mazzotta; +Cc: alsa-devel

At Fri, 08 Aug 2014 10:49:25 +0200,
Gabriele Mazzotta wrote:
> 
> 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.

Then the patch is wrong in anyway.  You cannot call it
unconditionally.  The codec might be still active, and calling resume
in that state does re-initialize the whole things doubly.

> 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.

If so, it might be BIOS causes the noise by re-initializing the
codec.


Takashi

> > 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;
> > > +			}
> > > +		}
> > >  	}
> > >  }
> > >  
> > 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ALSA: hda - More pop noise fixes for Dell XPS 13 9333
  2014-08-08 10:13     ` Takashi Iwai
@ 2014-08-08 15:01       ` Gabriele Mazzotta
  2014-08-08 15:06         ` Takashi Iwai
  0 siblings, 1 reply; 6+ messages in thread
From: Gabriele Mazzotta @ 2014-08-08 15:01 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

On Friday 08 August 2014 12:13:38 you wrote:
> At Fri, 08 Aug 2014 10:49:25 +0200,
> Gabriele Mazzotta wrote:
> > 
> > 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.
> 
> Then the patch is wrong in anyway.  You cannot call it
> unconditionally.  The codec might be still active, and calling resume
> in that state does re-initialize the whole things doubly.

Now I see that it's wrong. In addition, I guess it also breaks the build 
when CONFIG_PM is not set.

> > 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.
> 
> If so, it might be BIOS causes the noise by re-initializing the
> codec.

It seems that if 0x15 (Headphone playback switch) is in D0, the pop noise 
cannot be heard.
I tried to see using powertop whether it's worth making the current code more 
complex and force nid 0x15 in D0 only at reboot or not and I couldn't notice 
any major difference. Between 0x02, 0x15 and AFG only the last is causing a 
noticeable difference in the power consumption.

That said, I would simply add 0x15 to alc_power_filter_xps13(). I will send the 
patches soon.

Gabriele

> Takashi
> 
> > > 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;
> > > > +			}
> > > > +		}
> > > >  	}
> > > >  }
> > > >  
> > > 
> > 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ALSA: hda - More pop noise fixes for Dell XPS 13 9333
  2014-08-08 15:01       ` Gabriele Mazzotta
@ 2014-08-08 15:06         ` Takashi Iwai
  0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2014-08-08 15:06 UTC (permalink / raw)
  To: Gabriele Mazzotta; +Cc: alsa-devel

At Fri, 08 Aug 2014 17:01:44 +0200,
Gabriele Mazzotta wrote:
> 
> On Friday 08 August 2014 12:13:38 you wrote:
> > At Fri, 08 Aug 2014 10:49:25 +0200,
> > Gabriele Mazzotta wrote:
> > > 
> > > 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.
> > 
> > Then the patch is wrong in anyway.  You cannot call it
> > unconditionally.  The codec might be still active, and calling resume
> > in that state does re-initialize the whole things doubly.
> 
> Now I see that it's wrong. In addition, I guess it also breaks the build 
> when CONFIG_PM is not set.
> 
> > > 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.
> > 
> > If so, it might be BIOS causes the noise by re-initializing the
> > codec.
> 
> It seems that if 0x15 (Headphone playback switch) is in D0, the pop noise 
> cannot be heard.
> I tried to see using powertop whether it's worth making the current code more 
> complex and force nid 0x15 in D0 only at reboot or not and I couldn't notice 
> any major difference. Between 0x02, 0x15 and AFG only the last is causing a 
> noticeable difference in the power consumption.
> 
> That said, I would simply add 0x15 to alc_power_filter_xps13(). I will send the 
> patches soon.

Thanks, it's a good catch.


Takashi

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-08-08 15:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-07 16:35 [PATCH] ALSA: hda - More pop noise fixes for Dell XPS 13 9333 Gabriele Mazzotta
2014-08-08  6:11 ` Takashi Iwai
2014-08-08  8:49   ` Gabriele Mazzotta
2014-08-08 10:13     ` Takashi Iwai
2014-08-08 15:01       ` Gabriele Mazzotta
2014-08-08 15:06         ` Takashi Iwai

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.