Set eapd to follow stream on or off

Set eapd to follow stream on or off

[Kailang]

[-- Attachment #1: Type: text/plain, Size: 184 bytes --]

Hi Takashi,

HP machine had pop noise via speaker.
If EAPD on or off, speaker will have pop noise.
So, they want to set eapd to follow with stream on or off.

Many Thanks.

BR,
Kailang

[-- Attachment #2: 0000-set-eapd-follow-stream-on-off.patch --]
[-- Type: application/octet-stream, Size: 3304 bytes --]

>From 8af68c6454203361cce2128df2144afe88e5842a Mon Sep 17 00:00:00 2001
From: Kailang Yang <kailang@realtek.com>
Date: Tue, 20 May 2014 15:00:19 +0800
Subject: [PATCH] ALSA: hda/realtek - Set EAPD to follow stream on or off

HP machine has pop noise via speaker.
Set EAPD on when system open stream.
Set EAPD off when system close stream.

Signed-off-by: Kailang Yang <kailang@realtek.com>

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 35acbd0..67dd06c 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -4175,6 +4175,37 @@ static void alc290_fixup_mono_speakers(struct hda_codec *codec,
 	}
 }
 
+/*
+ * ALC290 PCM hooks
+ */
+static void ALC290_playback_pcm_hook(struct hda_pcm_stream *hinfo,
+				   struct hda_codec *codec,
+				   struct snd_pcm_substream *substream,
+				   int action)
+{
+	struct sigmatel_spec *spec = codec->spec;
+
+	switch (action) {
+	case HDA_GEN_PCM_ACT_OPEN:
+		set_eapd(codec, 0x14, true);
+		break;
+	case HDA_GEN_PCM_ACT_CLOSE:
+		set_eapd(codec, 0x14, false);
+		break;
+	}
+}
+
+static void alc290_fixup_pop_noise(struct hda_codec *codec,
+				       const struct hda_fixup *fix, int action)
+{
+	struct alc_spec *spec = codec->spec;
+
+	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
+		set_eapd(codec, 0x14, false);
+		spec->gen.pcm_playback_hook = ALC290_playback_pcm_hook;
+	}
+}
+
 /* for hda_fixup_thinkpad_acpi() */
 #include "thinkpad_helper.c"
 
@@ -4237,6 +4268,7 @@ enum {
 	ALC255_FIXUP_HEADSET_MODE_NO_HP_MIC,
 	ALC293_FIXUP_DELL1_MIC_NO_PRESENCE,
 	ALC292_FIXUP_TPT440_DOCK,
+	ALC290_FIXUP_HP_DEPOP,
 };
 
 static const struct hda_fixup alc269_fixups[] = {
@@ -4650,6 +4682,12 @@ static const struct hda_fixup alc269_fixups[] = {
 		.chained = true,
 		.chain_id = ALC269_FIXUP_LIMIT_INT_MIC_BOOST
 	},
+	[ALC290_FIXUP_HP_DEPOP] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = alc290_fixup_pop_noise,
+		.chained = true,
+		.chain_id = ALC269_FIXUP_HP_MUTE_LED_MIC1,
+	},
 };
 
 static const struct snd_pci_quirk alc269_fixup_tbl[] = {
@@ -4772,12 +4810,14 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x103c, 0x2280, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x2281, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x2282, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
-	SND_PCI_QUIRK(0x103c, 0x2289, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
+	SND_PCI_QUIRK(0x103c, 0x2289, "HP", ALC290_FIXUP_HP_DEPOP),
 	SND_PCI_QUIRK(0x103c, 0x228a, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x228b, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
-	SND_PCI_QUIRK(0x103c, 0x228c, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
+	SND_PCI_QUIRK(0x103c, 0x228c, "HP", ALC290_FIXUP_HP_DEPOP),
 	SND_PCI_QUIRK(0x103c, 0x228d, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x228e, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
+	SND_PCI_QUIRK(0x103c, 0x228f, "HP", ALC290_FIXUP_HP_DEPOP),
+	SND_PCI_QUIRK(0x103c, 0x22a8, "HP", ALC290_FIXUP_HP_DEPOP),
 	SND_PCI_QUIRK(0x103c, 0x22c5, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x22c6, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x22c7, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: Set eapd to follow stream on or off

[Takashi Iwai]

At Tue, 20 May 2014 15:15:49 +0800,
Kailang wrote:
> 
> Hi Takashi,
> 
> HP machine had pop noise via speaker.
> If EAPD on or off, speaker will have pop noise.
> So, they want to set eapd to follow with stream on or off.

I can apply this as is, but still I wonder whether this results in
other problems.  Doing EAPD on/off there means that it happens far
more often.  For example, what happens if you do like below?

     while true; do
       echo -n > /dev/snd/pcmC0D0p
     done

And, what if the device is resumed from power-saving by an audio
playback?  Immediately after the power up, it'll start the streaming,
so it'll turn on EAPD.

That said, isn't it just a need for some delay before EAPD on/off?


thanks,

Takashi

> 
> Many Thanks.
> 
> BR,
> Kailang
> [2 0000-set-eapd-follow-stream-on-off.patch <application/octet-stream (quoted-printable)>]
> 

Re: Set eapd to follow stream on or off

[Kailang]

> At Tue, 20 May 2014 15:15:49 +0800,
> Kailang wrote:
>>
>> Hi Takashi,
>>
>> HP machine had pop noise via speaker.
>> If EAPD on or off, speaker will have pop noise.
>> So, they want to set eapd to follow with stream on or off.
>
> I can apply this as is, but still I wonder whether this results in
> other problems.  Doing EAPD on/off there means that it happens far
> more often.  For example, what happens if you do like below?
>
>     while true; do
>       echo -n > /dev/snd/pcmC0D0p
>     done
>
> And, what if the device is resumed from power-saving by an audio
> playback?  Immediately after the power up, it'll start the streaming,
> so it'll turn on EAPD.
>
> That said, isn't it just a need for some delay before EAPD on/off?

No, eapd on or off will cause pop noise.
Our customer just want to do eapd on when stream is open.
The pop noise will not clear. Because music gain was biger than pop noise.

>
> thanks,
>
> Takashi
>
>>
>> Many Thanks.
>>
>> BR,
>> Kailang
>> [2 0000-set-eapd-follow-stream-on-off.patch <application/octet-stream 
>> (quoted-printable)>]
>>
>
> ------Please consider the environment before printing this e-mail. 

Re: Set eapd to follow stream on or off

[Takashi Iwai]

At Tue, 20 May 2014 16:52:43 +0800,
Kailang wrote:
> 
> 
> > At Tue, 20 May 2014 15:15:49 +0800,
> > Kailang wrote:
> >>
> >> Hi Takashi,
> >>
> >> HP machine had pop noise via speaker.
> >> If EAPD on or off, speaker will have pop noise.
> >> So, they want to set eapd to follow with stream on or off.
> >
> > I can apply this as is, but still I wonder whether this results in
> > other problems.  Doing EAPD on/off there means that it happens far
> > more often.  For example, what happens if you do like below?
> >
> >     while true; do
> >       echo -n > /dev/snd/pcmC0D0p
> >     done
> >
> > And, what if the device is resumed from power-saving by an audio
> > playback?  Immediately after the power up, it'll start the streaming,
> > so it'll turn on EAPD.
> >
> > That said, isn't it just a need for some delay before EAPD on/off?
> 
> No, eapd on or off will cause pop noise.

Does it do always?

> Our customer just want to do eapd on when stream is open.
> The pop noise will not clear. Because music gain was biger than pop noise.

Opening a stream doesn't mean that it follows the loud sound
immediately.  The script above wouldn't produce any sound but just
does open/close the stream.  Did you try that?


Takashi

Re: Set eapd to follow stream on or off

[Kailang]

> At Tue, 20 May 2014 16:52:43 +0800,
> Kailang wrote:
>>
>>
>> > At Tue, 20 May 2014 15:15:49 +0800,
>> > Kailang wrote:
>> >>
>> >> Hi Takashi,
>> >>
>> >> HP machine had pop noise via speaker.
>> >> If EAPD on or off, speaker will have pop noise.
>> >> So, they want to set eapd to follow with stream on or off.
>> >
>> > I can apply this as is, but still I wonder whether this results in
>> > other problems.  Doing EAPD on/off there means that it happens far
>> > more often.  For example, what happens if you do like below?
>> >
>> >     while true; do
>> >       echo -n > /dev/snd/pcmC0D0p
>> >     done
>> >
>> > And, what if the device is resumed from power-saving by an audio
>> > playback?  Immediately after the power up, it'll start the streaming,
>> > so it'll turn on EAPD.
>> >
>> > That said, isn't it just a need for some delay before EAPD on/off?
>>
>> No, eapd on or off will cause pop noise.
>
> Does it do always?
No, pop noise just one pop sound. It can not remove by software.
But this machine need to do this always.

>
>> Our customer just want to do eapd on when stream is open.
>> The pop noise will not clear. Because music gain was biger than pop 
>> noise.
>
> Opening a stream doesn't mean that it follows the loud sound
> immediately.  The script above wouldn't produce any sound but just
> does open/close the stream.  Did you try that?
>
>
> Takashi
>
> ------Please consider the environment before printing this e-mail. 

Re: Set eapd to follow stream on or off

[Takashi Iwai]

At Tue, 20 May 2014 17:20:44 +0800,
Kailang wrote:
> 
> 
> > At Tue, 20 May 2014 16:52:43 +0800,
> > Kailang wrote:
> >>
> >>
> >> > At Tue, 20 May 2014 15:15:49 +0800,
> >> > Kailang wrote:
> >> >>
> >> >> Hi Takashi,
> >> >>
> >> >> HP machine had pop noise via speaker.
> >> >> If EAPD on or off, speaker will have pop noise.
> >> >> So, they want to set eapd to follow with stream on or off.
> >> >
> >> > I can apply this as is, but still I wonder whether this results in
> >> > other problems.  Doing EAPD on/off there means that it happens far
> >> > more often.  For example, what happens if you do like below?
> >> >
> >> >     while true; do
> >> >       echo -n > /dev/snd/pcmC0D0p
> >> >     done
> >> >
> >> > And, what if the device is resumed from power-saving by an audio
> >> > playback?  Immediately after the power up, it'll start the streaming,
> >> > so it'll turn on EAPD.
> >> >
> >> > That said, isn't it just a need for some delay before EAPD on/off?
> >>
> >> No, eapd on or off will cause pop noise.
> >
> > Does it do always?
> No, pop noise just one pop sound. It can not remove by software.
> But this machine need to do this always.

So, doing EAPD on/off often is rather worse.  It's what your patch
does.

Once again: did you try the script above?  If the pop noise always
happens, the operation like above is the worst scenario.

Then, let's think again how to *reduce* it.  We need one EAPD on at
least after each power up.  But it's needed only when you actually
plays a stream.  That is, we need only enable EAPD at playback stream
open, but leave EAPD as is at close.


Takashi

> >> Our customer just want to do eapd on when stream is open.
> >> The pop noise will not clear. Because music gain was biger than pop 
> >> noise.
> >
> > Opening a stream doesn't mean that it follows the loud sound
> > immediately.  The script above wouldn't produce any sound but just
> > does open/close the stream.  Did you try that?
> >
> >
> > Takashi
> >
> > ------Please consider the environment before printing this e-mail. 
> 

Re: Set eapd to follow stream on or off

[Kailang]

> At Tue, 20 May 2014 17:20:44 +0800,
> Kailang wrote:
>>
>>
>> > At Tue, 20 May 2014 16:52:43 +0800,
>> > Kailang wrote:
>> >>
>> >>
>> >> > At Tue, 20 May 2014 15:15:49 +0800,
>> >> > Kailang wrote:
>> >> >>
>> >> >> Hi Takashi,
>> >> >>
>> >> >> HP machine had pop noise via speaker.
>> >> >> If EAPD on or off, speaker will have pop noise.
>> >> >> So, they want to set eapd to follow with stream on or off.
>> >> >
>> >> > I can apply this as is, but still I wonder whether this results in
>> >> > other problems.  Doing EAPD on/off there means that it happens far
>> >> > more often.  For example, what happens if you do like below?
>> >> >
>> >> >     while true; do
>> >> >       echo -n > /dev/snd/pcmC0D0p
>> >> >     done
>> >> >
>> >> > And, what if the device is resumed from power-saving by an audio
>> >> > playback?  Immediately after the power up, it'll start the 
>> >> > streaming,
>> >> > so it'll turn on EAPD.
>> >> >
>> >> > That said, isn't it just a need for some delay before EAPD on/off?
>> >>
>> >> No, eapd on or off will cause pop noise.
>> >
>> > Does it do always?
>> No, pop noise just one pop sound. It can not remove by software.
>> But this machine need to do this always.
>
> So, doing EAPD on/off often is rather worse.  It's what your patch
> does.
>
> Once again: did you try the script above?  If the pop noise always

I had try it on my develop machine. I have no HP issue machine on my site.

> happens, the operation like above is the worst scenario.
>
> Then, let's think again how to *reduce* it.  We need one EAPD on at
> least after each power up.  But it's needed only when you actually
> plays a stream.  That is, we need only enable EAPD at playback stream
> open, but leave EAPD as is at close.
>

[we need only enable EAPD at playback stream open, but leave EAPD as is at 
close.]
Yes.
So, I need to create eapd low in alc269_shutup. Right?

>
> Takashi
>
>> >> Our customer just want to do eapd on when stream is open.
>> >> The pop noise will not clear. Because music gain was biger than pop
>> >> noise.
>> >
>> > Opening a stream doesn't mean that it follows the loud sound
>> > immediately.  The script above wouldn't produce any sound but just
>> > does open/close the stream.  Did you try that?
>> >
>> >
>> > Takashi
>> >
>> > ------Please consider the environment before printing this e-mail.
>> 

Re: Set eapd to follow stream on or off

[Takashi Iwai]

At Tue, 20 May 2014 18:05:42 +0800,
Kailang wrote:
> 
> > At Tue, 20 May 2014 17:20:44 +0800,
> > Kailang wrote:
> >>
> >>
> >> > At Tue, 20 May 2014 16:52:43 +0800,
> >> > Kailang wrote:
> >> >>
> >> >>
> >> >> > At Tue, 20 May 2014 15:15:49 +0800,
> >> >> > Kailang wrote:
> >> >> >>
> >> >> >> Hi Takashi,
> >> >> >>
> >> >> >> HP machine had pop noise via speaker.
> >> >> >> If EAPD on or off, speaker will have pop noise.
> >> >> >> So, they want to set eapd to follow with stream on or off.
> >> >> >
> >> >> > I can apply this as is, but still I wonder whether this results in
> >> >> > other problems.  Doing EAPD on/off there means that it happens far
> >> >> > more often.  For example, what happens if you do like below?
> >> >> >
> >> >> >     while true; do
> >> >> >       echo -n > /dev/snd/pcmC0D0p
> >> >> >     done
> >> >> >
> >> >> > And, what if the device is resumed from power-saving by an audio
> >> >> > playback?  Immediately after the power up, it'll start the 
> >> >> > streaming,
> >> >> > so it'll turn on EAPD.
> >> >> >
> >> >> > That said, isn't it just a need for some delay before EAPD on/off?
> >> >>
> >> >> No, eapd on or off will cause pop noise.
> >> >
> >> > Does it do always?
> >> No, pop noise just one pop sound. It can not remove by software.
> >> But this machine need to do this always.
> >
> > So, doing EAPD on/off often is rather worse.  It's what your patch
> > does.
> >
> > Once again: did you try the script above?  If the pop noise always
> 
> I had try it on my develop machine. I have no HP issue machine on my site.
> 
> > happens, the operation like above is the worst scenario.
> >
> > Then, let's think again how to *reduce* it.  We need one EAPD on at
> > least after each power up.  But it's needed only when you actually
> > plays a stream.  That is, we need only enable EAPD at playback stream
> > open, but leave EAPD as is at close.
> >
> 
> [we need only enable EAPD at playback stream open, but leave EAPD as is at 
> close.]
> Yes.
> So, I need to create eapd low in alc269_shutup. Right?

Better to create own shutup callback (you can call alc269_shutup()
from there, too) and set it to spec->shutup in the fixup.  The pincfg
cleanup isn't often needed or may have bad effect.  You should test
with real machines what are needed and what are not.


Takashi

> 
> >
> > Takashi
> >
> >> >> Our customer just want to do eapd on when stream is open.
> >> >> The pop noise will not clear. Because music gain was biger than pop
> >> >> noise.
> >> >
> >> > Opening a stream doesn't mean that it follows the loud sound
> >> > immediately.  The script above wouldn't produce any sound but just
> >> > does open/close the stream.  Did you try that?
> >> >
> >> >
> >> > Takashi
> >> >
> >> > ------Please consider the environment before printing this e-mail.
> >> 
> 

Re: Set eapd to follow stream on or off

[Kailang]

> At Tue, 20 May 2014 18:05:42 +0800,
> Kailang wrote:
>>
>> > At Tue, 20 May 2014 17:20:44 +0800,
>> > Kailang wrote:
>> >>
>> >>
>> >> > At Tue, 20 May 2014 16:52:43 +0800,
>> >> > Kailang wrote:
>> >> >>
>> >> >>
>> >> >> > At Tue, 20 May 2014 15:15:49 +0800,
>> >> >> > Kailang wrote:
>> >> >> >>
>> >> >> >> Hi Takashi,
>> >> >> >>
>> >> >> >> HP machine had pop noise via speaker.
>> >> >> >> If EAPD on or off, speaker will have pop noise.
>> >> >> >> So, they want to set eapd to follow with stream on or off.
>> >> >> >
>> >> >> > I can apply this as is, but still I wonder whether this results 
>> >> >> > in
>> >> >> > other problems.  Doing EAPD on/off there means that it happens 
>> >> >> > far
>> >> >> > more often.  For example, what happens if you do like below?
>> >> >> >
>> >> >> >     while true; do
>> >> >> >       echo -n > /dev/snd/pcmC0D0p
>> >> >> >     done
>> >> >> >
>> >> >> > And, what if the device is resumed from power-saving by an audio
>> >> >> > playback?  Immediately after the power up, it'll start the
>> >> >> > streaming,
>> >> >> > so it'll turn on EAPD.
>> >> >> >
>> >> >> > That said, isn't it just a need for some delay before EAPD 
>> >> >> > on/off?
>> >> >>
>> >> >> No, eapd on or off will cause pop noise.
>> >> >
>> >> > Does it do always?
>> >> No, pop noise just one pop sound. It can not remove by software.
>> >> But this machine need to do this always.
>> >
>> > So, doing EAPD on/off often is rather worse.  It's what your patch
>> > does.
>> >
>> > Once again: did you try the script above?  If the pop noise always
>>
>> I had try it on my develop machine. I have no HP issue machine on my 
>> site.
>>
>> > happens, the operation like above is the worst scenario.
>> >
>> > Then, let's think again how to *reduce* it.  We need one EAPD on at
>> > least after each power up.  But it's needed only when you actually
>> > plays a stream.  That is, we need only enable EAPD at playback stream
>> > open, but leave EAPD as is at close.
>> >
>>
>> [we need only enable EAPD at playback stream open, but leave EAPD as is 
>> at
>> close.]
>> Yes.
>> So, I need to create eapd low in alc269_shutup. Right?
>
> Better to create own shutup callback (you can call alc269_shutup()
> from there, too) and set it to spec->shutup in the fixup.  The pincfg
> cleanup isn't often needed or may have bad effect.  You should test
> with real machines what are needed and what are not.

static void alc290_fixup_pop_noise(struct hda_codec *codec,
           const struct hda_fixup *fix, int action)
{
 struct alc_spec *spec = codec->spec;

 if (action == HDA_FIXUP_ACT_PRE_PROBE) {
  spec->gen.pcm_playback_hook = ALC290_playback_pcm_hook;
 }
if (action ==   HDA_FIXUP_ACT_INIT) {
  set_eapd(codec, 0x14, false);
 }
}

Could I modify as upper code?
To set eapd low in HDA_FIXUP_ACT_INIT state.
If system resume, it will initial eapd to low.
To write eapd to same value, it will have no pop noise.
It will keep always low for eapd, only stream open and close have change 
state for eapd.

>
>
> Takashi
>
>>
>> >
>> > Takashi
>> >
>> >> >> Our customer just want to do eapd on when stream is open.
>> >> >> The pop noise will not clear. Because music gain was biger than pop
>> >> >> noise.
>> >> >
>> >> > Opening a stream doesn't mean that it follows the loud sound
>> >> > immediately.  The script above wouldn't produce any sound but just
>> >> > does open/close the stream.  Did you try that?
>> >> >
>> >> >
>> >> > Takashi
>> >> >
>> >> > ------Please consider the environment before printing this e-mail.
>> >>
>> 

Re: Set eapd to follow stream on or off

[Takashi Iwai]

At Wed, 21 May 2014 13:58:14 +0800,
Kailang wrote:
> 
> 
> > At Tue, 20 May 2014 18:05:42 +0800,
> > Kailang wrote:
> >>
> >> > At Tue, 20 May 2014 17:20:44 +0800,
> >> > Kailang wrote:
> >> >>
> >> >>
> >> >> > At Tue, 20 May 2014 16:52:43 +0800,
> >> >> > Kailang wrote:
> >> >> >>
> >> >> >>
> >> >> >> > At Tue, 20 May 2014 15:15:49 +0800,
> >> >> >> > Kailang wrote:
> >> >> >> >>
> >> >> >> >> Hi Takashi,
> >> >> >> >>
> >> >> >> >> HP machine had pop noise via speaker.
> >> >> >> >> If EAPD on or off, speaker will have pop noise.
> >> >> >> >> So, they want to set eapd to follow with stream on or off.
> >> >> >> >
> >> >> >> > I can apply this as is, but still I wonder whether this results 
> >> >> >> > in
> >> >> >> > other problems.  Doing EAPD on/off there means that it happens 
> >> >> >> > far
> >> >> >> > more often.  For example, what happens if you do like below?
> >> >> >> >
> >> >> >> >     while true; do
> >> >> >> >       echo -n > /dev/snd/pcmC0D0p
> >> >> >> >     done
> >> >> >> >
> >> >> >> > And, what if the device is resumed from power-saving by an audio
> >> >> >> > playback?  Immediately after the power up, it'll start the
> >> >> >> > streaming,
> >> >> >> > so it'll turn on EAPD.
> >> >> >> >
> >> >> >> > That said, isn't it just a need for some delay before EAPD 
> >> >> >> > on/off?
> >> >> >>
> >> >> >> No, eapd on or off will cause pop noise.
> >> >> >
> >> >> > Does it do always?
> >> >> No, pop noise just one pop sound. It can not remove by software.
> >> >> But this machine need to do this always.
> >> >
> >> > So, doing EAPD on/off often is rather worse.  It's what your patch
> >> > does.
> >> >
> >> > Once again: did you try the script above?  If the pop noise always
> >>
> >> I had try it on my develop machine. I have no HP issue machine on my 
> >> site.
> >>
> >> > happens, the operation like above is the worst scenario.
> >> >
> >> > Then, let's think again how to *reduce* it.  We need one EAPD on at
> >> > least after each power up.  But it's needed only when you actually
> >> > plays a stream.  That is, we need only enable EAPD at playback stream
> >> > open, but leave EAPD as is at close.
> >> >
> >>
> >> [we need only enable EAPD at playback stream open, but leave EAPD as is 
> >> at
> >> close.]
> >> Yes.
> >> So, I need to create eapd low in alc269_shutup. Right?
> >
> > Better to create own shutup callback (you can call alc269_shutup()
> > from there, too) and set it to spec->shutup in the fixup.  The pincfg
> > cleanup isn't often needed or may have bad effect.  You should test
> > with real machines what are needed and what are not.
> 
> static void alc290_fixup_pop_noise(struct hda_codec *codec,
>            const struct hda_fixup *fix, int action)
> {
>  struct alc_spec *spec = codec->spec;
> 
>  if (action == HDA_FIXUP_ACT_PRE_PROBE) {
>   spec->gen.pcm_playback_hook = ALC290_playback_pcm_hook;
>  }
> if (action ==   HDA_FIXUP_ACT_INIT) {
>   set_eapd(codec, 0x14, false);
>  }
> }
> 
> Could I modify as upper code?
> To set eapd low in HDA_FIXUP_ACT_INIT state.
> If system resume, it will initial eapd to low.
> To write eapd to same value, it will have no pop noise.
> It will keep always low for eapd, only stream open and close have change 
> state for eapd.

Yes, it looks OK.


Takashi


> 
> >
> >
> > Takashi
> >
> >>
> >> >
> >> > Takashi
> >> >
> >> >> >> Our customer just want to do eapd on when stream is open.
> >> >> >> The pop noise will not clear. Because music gain was biger than pop
> >> >> >> noise.
> >> >> >
> >> >> > Opening a stream doesn't mean that it follows the loud sound
> >> >> > immediately.  The script above wouldn't produce any sound but just
> >> >> > does open/close the stream.  Did you try that?
> >> >> >
> >> >> >
> >> >> > Takashi
> >> >> >
> >> >> > ------Please consider the environment before printing this e-mail.
> >> >>
> >> 
> 

Re: Set eapd to follow stream on or off

[Kailang]

[-- Attachment #1: Type: text/plain, Size: 4151 bytes --]

Hi Takashi,

Attach is the update patch.
Many thanks.

BR,
Kailang


> At Wed, 21 May 2014 13:58:14 +0800,
> Kailang wrote:
>>
>>
>> > At Tue, 20 May 2014 18:05:42 +0800,
>> > Kailang wrote:
>> >>
>> >> > At Tue, 20 May 2014 17:20:44 +0800,
>> >> > Kailang wrote:
>> >> >>
>> >> >>
>> >> >> > At Tue, 20 May 2014 16:52:43 +0800,
>> >> >> > Kailang wrote:
>> >> >> >>
>> >> >> >>
>> >> >> >> > At Tue, 20 May 2014 15:15:49 +0800,
>> >> >> >> > Kailang wrote:
>> >> >> >> >>
>> >> >> >> >> Hi Takashi,
>> >> >> >> >>
>> >> >> >> >> HP machine had pop noise via speaker.
>> >> >> >> >> If EAPD on or off, speaker will have pop noise.
>> >> >> >> >> So, they want to set eapd to follow with stream on or off.
>> >> >> >> >
>> >> >> >> > I can apply this as is, but still I wonder whether this 
>> >> >> >> > results
>> >> >> >> > in
>> >> >> >> > other problems.  Doing EAPD on/off there means that it happens
>> >> >> >> > far
>> >> >> >> > more often.  For example, what happens if you do like below?
>> >> >> >> >
>> >> >> >> >     while true; do
>> >> >> >> >       echo -n > /dev/snd/pcmC0D0p
>> >> >> >> >     done
>> >> >> >> >
>> >> >> >> > And, what if the device is resumed from power-saving by an 
>> >> >> >> > audio
>> >> >> >> > playback?  Immediately after the power up, it'll start the
>> >> >> >> > streaming,
>> >> >> >> > so it'll turn on EAPD.
>> >> >> >> >
>> >> >> >> > That said, isn't it just a need for some delay before EAPD
>> >> >> >> > on/off?
>> >> >> >>
>> >> >> >> No, eapd on or off will cause pop noise.
>> >> >> >
>> >> >> > Does it do always?
>> >> >> No, pop noise just one pop sound. It can not remove by software.
>> >> >> But this machine need to do this always.
>> >> >
>> >> > So, doing EAPD on/off often is rather worse.  It's what your patch
>> >> > does.
>> >> >
>> >> > Once again: did you try the script above?  If the pop noise always
>> >>
>> >> I had try it on my develop machine. I have no HP issue machine on my
>> >> site.
>> >>
>> >> > happens, the operation like above is the worst scenario.
>> >> >
>> >> > Then, let's think again how to *reduce* it.  We need one EAPD on at
>> >> > least after each power up.  But it's needed only when you actually
>> >> > plays a stream.  That is, we need only enable EAPD at playback 
>> >> > stream
>> >> > open, but leave EAPD as is at close.
>> >> >
>> >>
>> >> [we need only enable EAPD at playback stream open, but leave EAPD as 
>> >> is
>> >> at
>> >> close.]
>> >> Yes.
>> >> So, I need to create eapd low in alc269_shutup. Right?
>> >
>> > Better to create own shutup callback (you can call alc269_shutup()
>> > from there, too) and set it to spec->shutup in the fixup.  The pincfg
>> > cleanup isn't often needed or may have bad effect.  You should test
>> > with real machines what are needed and what are not.
>>
>> static void alc290_fixup_pop_noise(struct hda_codec *codec,
>>            const struct hda_fixup *fix, int action)
>> {
>>  struct alc_spec *spec = codec->spec;
>>
>>  if (action == HDA_FIXUP_ACT_PRE_PROBE) {
>>   spec->gen.pcm_playback_hook = ALC290_playback_pcm_hook;
>>  }
>> if (action ==   HDA_FIXUP_ACT_INIT) {
>>   set_eapd(codec, 0x14, false);
>>  }
>> }
>>
>> Could I modify as upper code?
>> To set eapd low in HDA_FIXUP_ACT_INIT state.
>> If system resume, it will initial eapd to low.
>> To write eapd to same value, it will have no pop noise.
>> It will keep always low for eapd, only stream open and close have change
>> state for eapd.
>
> Yes, it looks OK.
>
>
> Takashi
>
>
>>
>> >
>> >
>> > Takashi
>> >
>> >>
>> >> >
>> >> > Takashi
>> >> >
>> >> >> >> Our customer just want to do eapd on when stream is open.
>> >> >> >> The pop noise will not clear. Because music gain was biger than 
>> >> >> >> pop
>> >> >> >> noise.
>> >> >> >
>> >> >> > Opening a stream doesn't mean that it follows the loud sound
>> >> >> > immediately.  The script above wouldn't produce any sound but 
>> >> >> > just
>> >> >> > does open/close the stream.  Did you try that?
>> >> >> >
>> >> >> >
>> >> >> > Takashi
>> >> >> >
>> >> >> > ------Please consider the environment before printing this 
>> >> >> > e-mail.
>> >> >>
>> >>
>> 

[-- Attachment #2: 0001-set-eapd-follow-stream-on-off.patch --]
[-- Type: application/octet-stream, Size: 3400 bytes --]

>From 71a093ba3198666c4500b30938e080bd887647cb Mon Sep 17 00:00:00 2001
From: Kailang Yang <kailang@realtek.com>
Date: Wed, 21 May 2014 14:25:08 +0800
Subject: [PATCH] ALSA: hda/realtek - Set EAPD to follow stream on or off

HP machine has pop noise via speaker.
Set EAPD on when system open stream.
Set EAPD off when system close stream.

Signed-off-by: Kailang Yang <kailang@realtek.com>

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 35acbd0..87973e2 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -4175,6 +4175,39 @@ static void alc290_fixup_mono_speakers(struct hda_codec *codec,
 	}
 }
 
+/*
+ * ALC290 PCM hooks
+ */
+static void ALC290_playback_pcm_hook(struct hda_pcm_stream *hinfo,
+				   struct hda_codec *codec,
+				   struct snd_pcm_substream *substream,
+				   int action)
+{
+	switch (action) {
+	case HDA_GEN_PCM_ACT_OPEN:
+		set_eapd(codec, 0x14, true);
+		break;
+	case HDA_GEN_PCM_ACT_CLOSE:
+		set_eapd(codec, 0x14, false);
+		break;
+	}
+}
+
+static void alc290_fixup_pop_noise(struct hda_codec *codec,
+				       const struct hda_fixup *fix, int action)
+{
+	struct alc_spec *spec = codec->spec;
+
+	switch (action) {
+	case HDA_FIXUP_ACT_PRE_PROBE:
+		spec->gen.pcm_playback_hook = ALC290_playback_pcm_hook;
+		break;
+	case HDA_FIXUP_ACT_INIT:
+		set_eapd(codec, 0x14, false);
+		break;
+	}
+}
+
 /* for hda_fixup_thinkpad_acpi() */
 #include "thinkpad_helper.c"
 
@@ -4237,6 +4270,7 @@ enum {
 	ALC255_FIXUP_HEADSET_MODE_NO_HP_MIC,
 	ALC293_FIXUP_DELL1_MIC_NO_PRESENCE,
 	ALC292_FIXUP_TPT440_DOCK,
+	ALC290_FIXUP_HP_DEPOP_WITH_MUTE_LED,
 };
 
 static const struct hda_fixup alc269_fixups[] = {
@@ -4650,6 +4684,12 @@ static const struct hda_fixup alc269_fixups[] = {
 		.chained = true,
 		.chain_id = ALC269_FIXUP_LIMIT_INT_MIC_BOOST
 	},
+	[ALC290_FIXUP_HP_DEPOP_WITH_MUTE_LED] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = alc290_fixup_pop_noise,
+		.chained = true,
+		.chain_id = ALC269_FIXUP_HP_MUTE_LED_MIC1,
+	},
 };
 
 static const struct snd_pci_quirk alc269_fixup_tbl[] = {
@@ -4772,12 +4812,14 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x103c, 0x2280, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x2281, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x2282, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
-	SND_PCI_QUIRK(0x103c, 0x2289, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
+	SND_PCI_QUIRK(0x103c, 0x2289, "HP", ALC290_FIXUP_HP_DEPOP_WITH_MUTE_LED),
 	SND_PCI_QUIRK(0x103c, 0x228a, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x228b, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
-	SND_PCI_QUIRK(0x103c, 0x228c, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
+	SND_PCI_QUIRK(0x103c, 0x228c, "HP", ALC290_FIXUP_HP_DEPOP_WITH_MUTE_LED),
 	SND_PCI_QUIRK(0x103c, 0x228d, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x228e, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
+	SND_PCI_QUIRK(0x103c, 0x228f, "HP", ALC290_FIXUP_HP_DEPOP_WITH_MUTE_LED),
+	SND_PCI_QUIRK(0x103c, 0x22a8, "HP", ALC290_FIXUP_HP_DEPOP_WITH_MUTE_LED),
 	SND_PCI_QUIRK(0x103c, 0x22c5, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x22c6, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x22c7, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: Set eapd to follow stream on or off

[Takashi Iwai]

At Wed, 21 May 2014 14:28:12 +0800,
Kailang wrote:
> 
> Hi Takashi,
> 
> Attach is the update patch.

Keep the function in lower letters.

Also, update the patch description as well (and better to put more
words).


thanks,

Takashi

> Many thanks.
> 
> BR,
> Kailang
> 
> 
> > At Wed, 21 May 2014 13:58:14 +0800,
> > Kailang wrote:
> >>
> >>
> >> > At Tue, 20 May 2014 18:05:42 +0800,
> >> > Kailang wrote:
> >> >>
> >> >> > At Tue, 20 May 2014 17:20:44 +0800,
> >> >> > Kailang wrote:
> >> >> >>
> >> >> >>
> >> >> >> > At Tue, 20 May 2014 16:52:43 +0800,
> >> >> >> > Kailang wrote:
> >> >> >> >>
> >> >> >> >>
> >> >> >> >> > At Tue, 20 May 2014 15:15:49 +0800,
> >> >> >> >> > Kailang wrote:
> >> >> >> >> >>
> >> >> >> >> >> Hi Takashi,
> >> >> >> >> >>
> >> >> >> >> >> HP machine had pop noise via speaker.
> >> >> >> >> >> If EAPD on or off, speaker will have pop noise.
> >> >> >> >> >> So, they want to set eapd to follow with stream on or off.
> >> >> >> >> >
> >> >> >> >> > I can apply this as is, but still I wonder whether this 
> >> >> >> >> > results
> >> >> >> >> > in
> >> >> >> >> > other problems.  Doing EAPD on/off there means that it happens
> >> >> >> >> > far
> >> >> >> >> > more often.  For example, what happens if you do like below?
> >> >> >> >> >
> >> >> >> >> >     while true; do
> >> >> >> >> >       echo -n > /dev/snd/pcmC0D0p
> >> >> >> >> >     done
> >> >> >> >> >
> >> >> >> >> > And, what if the device is resumed from power-saving by an 
> >> >> >> >> > audio
> >> >> >> >> > playback?  Immediately after the power up, it'll start the
> >> >> >> >> > streaming,
> >> >> >> >> > so it'll turn on EAPD.
> >> >> >> >> >
> >> >> >> >> > That said, isn't it just a need for some delay before EAPD
> >> >> >> >> > on/off?
> >> >> >> >>
> >> >> >> >> No, eapd on or off will cause pop noise.
> >> >> >> >
> >> >> >> > Does it do always?
> >> >> >> No, pop noise just one pop sound. It can not remove by software.
> >> >> >> But this machine need to do this always.
> >> >> >
> >> >> > So, doing EAPD on/off often is rather worse.  It's what your patch
> >> >> > does.
> >> >> >
> >> >> > Once again: did you try the script above?  If the pop noise always
> >> >>
> >> >> I had try it on my develop machine. I have no HP issue machine on my
> >> >> site.
> >> >>
> >> >> > happens, the operation like above is the worst scenario.
> >> >> >
> >> >> > Then, let's think again how to *reduce* it.  We need one EAPD on at
> >> >> > least after each power up.  But it's needed only when you actually
> >> >> > plays a stream.  That is, we need only enable EAPD at playback 
> >> >> > stream
> >> >> > open, but leave EAPD as is at close.
> >> >> >
> >> >>
> >> >> [we need only enable EAPD at playback stream open, but leave EAPD as 
> >> >> is
> >> >> at
> >> >> close.]
> >> >> Yes.
> >> >> So, I need to create eapd low in alc269_shutup. Right?
> >> >
> >> > Better to create own shutup callback (you can call alc269_shutup()
> >> > from there, too) and set it to spec->shutup in the fixup.  The pincfg
> >> > cleanup isn't often needed or may have bad effect.  You should test
> >> > with real machines what are needed and what are not.
> >>
> >> static void alc290_fixup_pop_noise(struct hda_codec *codec,
> >>            const struct hda_fixup *fix, int action)
> >> {
> >>  struct alc_spec *spec = codec->spec;
> >>
> >>  if (action == HDA_FIXUP_ACT_PRE_PROBE) {
> >>   spec->gen.pcm_playback_hook = ALC290_playback_pcm_hook;
> >>  }
> >> if (action ==   HDA_FIXUP_ACT_INIT) {
> >>   set_eapd(codec, 0x14, false);
> >>  }
> >> }
> >>
> >> Could I modify as upper code?
> >> To set eapd low in HDA_FIXUP_ACT_INIT state.
> >> If system resume, it will initial eapd to low.
> >> To write eapd to same value, it will have no pop noise.
> >> It will keep always low for eapd, only stream open and close have change
> >> state for eapd.
> >
> > Yes, it looks OK.
> >
> >
> > Takashi
> >
> >
> >>
> >> >
> >> >
> >> > Takashi
> >> >
> >> >>
> >> >> >
> >> >> > Takashi
> >> >> >
> >> >> >> >> Our customer just want to do eapd on when stream is open.
> >> >> >> >> The pop noise will not clear. Because music gain was biger than 
> >> >> >> >> pop
> >> >> >> >> noise.
> >> >> >> >
> >> >> >> > Opening a stream doesn't mean that it follows the loud sound
> >> >> >> > immediately.  The script above wouldn't produce any sound but 
> >> >> >> > just
> >> >> >> > does open/close the stream.  Did you try that?
> >> >> >> >
> >> >> >> >
> >> >> >> > Takashi
> >> >> >> >
> >> >> >> > ------Please consider the environment before printing this 
> >> >> >> > e-mail.
> >> >> >>
> >> >>
> >> 
> [2 0001-set-eapd-follow-stream-on-off.patch <application/octet-stream (quoted-printable)>]
> 

Re: Set eapd to follow stream on or off

[Kailang]

[-- Attachment #1: Type: text/plain, Size: 5106 bytes --]

> At Wed, 21 May 2014 14:28:12 +0800,
> Kailang wrote:
>>
>> Hi Takashi,
>>
>> Attach is the update patch.
>
> Keep the function in lower letters.
>
> Also, update the patch description as well (and better to put more
> words).

Update as attach.
Thanks.

>
>
> thanks,
>
> Takashi
>
>> Many thanks.
>>
>> BR,
>> Kailang
>>
>>
>> > At Wed, 21 May 2014 13:58:14 +0800,
>> > Kailang wrote:
>> >>
>> >>
>> >> > At Tue, 20 May 2014 18:05:42 +0800,
>> >> > Kailang wrote:
>> >> >>
>> >> >> > At Tue, 20 May 2014 17:20:44 +0800,
>> >> >> > Kailang wrote:
>> >> >> >>
>> >> >> >>
>> >> >> >> > At Tue, 20 May 2014 16:52:43 +0800,
>> >> >> >> > Kailang wrote:
>> >> >> >> >>
>> >> >> >> >>
>> >> >> >> >> > At Tue, 20 May 2014 15:15:49 +0800,
>> >> >> >> >> > Kailang wrote:
>> >> >> >> >> >>
>> >> >> >> >> >> Hi Takashi,
>> >> >> >> >> >>
>> >> >> >> >> >> HP machine had pop noise via speaker.
>> >> >> >> >> >> If EAPD on or off, speaker will have pop noise.
>> >> >> >> >> >> So, they want to set eapd to follow with stream on or off.
>> >> >> >> >> >
>> >> >> >> >> > I can apply this as is, but still I wonder whether this
>> >> >> >> >> > results
>> >> >> >> >> > in
>> >> >> >> >> > other problems.  Doing EAPD on/off there means that it 
>> >> >> >> >> > happens
>> >> >> >> >> > far
>> >> >> >> >> > more often.  For example, what happens if you do like 
>> >> >> >> >> > below?
>> >> >> >> >> >
>> >> >> >> >> >     while true; do
>> >> >> >> >> >       echo -n > /dev/snd/pcmC0D0p
>> >> >> >> >> >     done
>> >> >> >> >> >
>> >> >> >> >> > And, what if the device is resumed from power-saving by an
>> >> >> >> >> > audio
>> >> >> >> >> > playback?  Immediately after the power up, it'll start the
>> >> >> >> >> > streaming,
>> >> >> >> >> > so it'll turn on EAPD.
>> >> >> >> >> >
>> >> >> >> >> > That said, isn't it just a need for some delay before EAPD
>> >> >> >> >> > on/off?
>> >> >> >> >>
>> >> >> >> >> No, eapd on or off will cause pop noise.
>> >> >> >> >
>> >> >> >> > Does it do always?
>> >> >> >> No, pop noise just one pop sound. It can not remove by software.
>> >> >> >> But this machine need to do this always.
>> >> >> >
>> >> >> > So, doing EAPD on/off often is rather worse.  It's what your 
>> >> >> > patch
>> >> >> > does.
>> >> >> >
>> >> >> > Once again: did you try the script above?  If the pop noise 
>> >> >> > always
>> >> >>
>> >> >> I had try it on my develop machine. I have no HP issue machine on 
>> >> >> my
>> >> >> site.
>> >> >>
>> >> >> > happens, the operation like above is the worst scenario.
>> >> >> >
>> >> >> > Then, let's think again how to *reduce* it.  We need one EAPD on 
>> >> >> > at
>> >> >> > least after each power up.  But it's needed only when you 
>> >> >> > actually
>> >> >> > plays a stream.  That is, we need only enable EAPD at playback
>> >> >> > stream
>> >> >> > open, but leave EAPD as is at close.
>> >> >> >
>> >> >>
>> >> >> [we need only enable EAPD at playback stream open, but leave EAPD 
>> >> >> as
>> >> >> is
>> >> >> at
>> >> >> close.]
>> >> >> Yes.
>> >> >> So, I need to create eapd low in alc269_shutup. Right?
>> >> >
>> >> > Better to create own shutup callback (you can call alc269_shutup()
>> >> > from there, too) and set it to spec->shutup in the fixup.  The 
>> >> > pincfg
>> >> > cleanup isn't often needed or may have bad effect.  You should test
>> >> > with real machines what are needed and what are not.
>> >>
>> >> static void alc290_fixup_pop_noise(struct hda_codec *codec,
>> >>            const struct hda_fixup *fix, int action)
>> >> {
>> >>  struct alc_spec *spec = codec->spec;
>> >>
>> >>  if (action == HDA_FIXUP_ACT_PRE_PROBE) {
>> >>   spec->gen.pcm_playback_hook = ALC290_playback_pcm_hook;
>> >>  }
>> >> if (action ==   HDA_FIXUP_ACT_INIT) {
>> >>   set_eapd(codec, 0x14, false);
>> >>  }
>> >> }
>> >>
>> >> Could I modify as upper code?
>> >> To set eapd low in HDA_FIXUP_ACT_INIT state.
>> >> If system resume, it will initial eapd to low.
>> >> To write eapd to same value, it will have no pop noise.
>> >> It will keep always low for eapd, only stream open and close have 
>> >> change
>> >> state for eapd.
>> >
>> > Yes, it looks OK.
>> >
>> >
>> > Takashi
>> >
>> >
>> >>
>> >> >
>> >> >
>> >> > Takashi
>> >> >
>> >> >>
>> >> >> >
>> >> >> > Takashi
>> >> >> >
>> >> >> >> >> Our customer just want to do eapd on when stream is open.
>> >> >> >> >> The pop noise will not clear. Because music gain was biger 
>> >> >> >> >> than
>> >> >> >> >> pop
>> >> >> >> >> noise.
>> >> >> >> >
>> >> >> >> > Opening a stream doesn't mean that it follows the loud sound
>> >> >> >> > immediately.  The script above wouldn't produce any sound but
>> >> >> >> > just
>> >> >> >> > does open/close the stream.  Did you try that?
>> >> >> >> >
>> >> >> >> >
>> >> >> >> > Takashi
>> >> >> >> >
>> >> >> >> > ------Please consider the environment before printing this
>> >> >> >> > e-mail.
>> >> >> >>
>> >> >>
>> >>
>> [2 0001-set-eapd-follow-stream-on-off.patch <application/octet-stream 
>> (quoted-printable)>]
>>
>
> ------Please consider the environment before printing this e-mail. 

[-- Attachment #2: 0002-set-eapd-follow-stream-on-off.patch --]
[-- Type: application/octet-stream, Size: 3465 bytes --]

>From 71a093ba3198666c4500b30938e080bd887647cb Mon Sep 17 00:00:00 2001
From: Kailang Yang <kailang@realtek.com>
Date: Wed, 21 May 2014 14:25:08 +0800
Subject: [PATCH] ALSA: hda/realtek - Set EAPD to follow stream on or off

HP machine has pop noise via speaker. To change state for EAPD will show the pop noise.
This patch will keep EAPD always low. Only stream open or close will change state for EAPD.

Signed-off-by: Kailang Yang <kailang@realtek.com>

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 35acbd0..87973e2 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -4175,6 +4175,39 @@ static void alc290_fixup_mono_speakers(struct hda_codec *codec,
 	}
 }
 
+/*
+ * ALC290 PCM hooks
+ */
+static void alc290_playback_pcm_hook(struct hda_pcm_stream *hinfo,
+				   struct hda_codec *codec,
+				   struct snd_pcm_substream *substream,
+				   int action)
+{
+	switch (action) {
+	case HDA_GEN_PCM_ACT_OPEN:
+		set_eapd(codec, 0x14, true);
+		break;
+	case HDA_GEN_PCM_ACT_CLOSE:
+		set_eapd(codec, 0x14, false);
+		break;
+	}
+}
+
+static void alc290_fixup_pop_noise(struct hda_codec *codec,
+				       const struct hda_fixup *fix, int action)
+{
+	struct alc_spec *spec = codec->spec;
+
+	switch (action) {
+	case HDA_FIXUP_ACT_PRE_PROBE:
+		spec->gen.pcm_playback_hook = alc290_playback_pcm_hook;
+		break;
+	case HDA_FIXUP_ACT_INIT:
+		set_eapd(codec, 0x14, false);
+		break;
+	}
+}
+
 /* for hda_fixup_thinkpad_acpi() */
 #include "thinkpad_helper.c"
 
@@ -4237,6 +4270,7 @@ enum {
 	ALC255_FIXUP_HEADSET_MODE_NO_HP_MIC,
 	ALC293_FIXUP_DELL1_MIC_NO_PRESENCE,
 	ALC292_FIXUP_TPT440_DOCK,
+	ALC290_FIXUP_HP_DEPOP_WITH_MUTE_LED,
 };
 
 static const struct hda_fixup alc269_fixups[] = {
@@ -4650,6 +4684,12 @@ static const struct hda_fixup alc269_fixups[] = {
 		.chained = true,
 		.chain_id = ALC269_FIXUP_LIMIT_INT_MIC_BOOST
 	},
+	[ALC290_FIXUP_HP_DEPOP_WITH_MUTE_LED] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = alc290_fixup_pop_noise,
+		.chained = true,
+		.chain_id = ALC269_FIXUP_HP_MUTE_LED_MIC1,
+	},
 };
 
 static const struct snd_pci_quirk alc269_fixup_tbl[] = {
@@ -4772,12 +4812,14 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x103c, 0x2280, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x2281, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x2282, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
-	SND_PCI_QUIRK(0x103c, 0x2289, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
+	SND_PCI_QUIRK(0x103c, 0x2289, "HP", ALC290_FIXUP_HP_DEPOP_WITH_MUTE_LED),
 	SND_PCI_QUIRK(0x103c, 0x228a, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x228b, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
-	SND_PCI_QUIRK(0x103c, 0x228c, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
+	SND_PCI_QUIRK(0x103c, 0x228c, "HP", ALC290_FIXUP_HP_DEPOP_WITH_MUTE_LED),
 	SND_PCI_QUIRK(0x103c, 0x228d, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x228e, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
+	SND_PCI_QUIRK(0x103c, 0x228f, "HP", ALC290_FIXUP_HP_DEPOP_WITH_MUTE_LED),
+	SND_PCI_QUIRK(0x103c, 0x22a8, "HP", ALC290_FIXUP_HP_DEPOP_WITH_MUTE_LED),
 	SND_PCI_QUIRK(0x103c, 0x22c5, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x22c6, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),
 	SND_PCI_QUIRK(0x103c, 0x22c7, "HP", ALC269_FIXUP_HP_MUTE_LED_MIC1),

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: Set eapd to follow stream on or off

[Takashi Iwai]

At Wed, 21 May 2014 17:24:16 +0800,
Kailang wrote:
> 
> > At Wed, 21 May 2014 14:28:12 +0800,
> > Kailang wrote:
> >>
> >> Hi Takashi,
> >>
> >> Attach is the update patch.
> >
> > Keep the function in lower letters.
> >
> > Also, update the patch description as well (and better to put more
> > words).
> 
> Update as attach.
> Thanks.

Well, do you really want to turn off EAPD at closing the stream?
As I repeatedly explained, this results in more frequent EAPD on/off,
i.e. more pop noises.


Takashi

> 
> >
> >
> > thanks,
> >
> > Takashi
> >
> >> Many thanks.
> >>
> >> BR,
> >> Kailang
> >>
> >>
> >> > At Wed, 21 May 2014 13:58:14 +0800,
> >> > Kailang wrote:
> >> >>
> >> >>
> >> >> > At Tue, 20 May 2014 18:05:42 +0800,
> >> >> > Kailang wrote:
> >> >> >>
> >> >> >> > At Tue, 20 May 2014 17:20:44 +0800,
> >> >> >> > Kailang wrote:
> >> >> >> >>
> >> >> >> >>
> >> >> >> >> > At Tue, 20 May 2014 16:52:43 +0800,
> >> >> >> >> > Kailang wrote:
> >> >> >> >> >>
> >> >> >> >> >>
> >> >> >> >> >> > At Tue, 20 May 2014 15:15:49 +0800,
> >> >> >> >> >> > Kailang wrote:
> >> >> >> >> >> >>
> >> >> >> >> >> >> Hi Takashi,
> >> >> >> >> >> >>
> >> >> >> >> >> >> HP machine had pop noise via speaker.
> >> >> >> >> >> >> If EAPD on or off, speaker will have pop noise.
> >> >> >> >> >> >> So, they want to set eapd to follow with stream on or off.
> >> >> >> >> >> >
> >> >> >> >> >> > I can apply this as is, but still I wonder whether this
> >> >> >> >> >> > results
> >> >> >> >> >> > in
> >> >> >> >> >> > other problems.  Doing EAPD on/off there means that it 
> >> >> >> >> >> > happens
> >> >> >> >> >> > far
> >> >> >> >> >> > more often.  For example, what happens if you do like 
> >> >> >> >> >> > below?
> >> >> >> >> >> >
> >> >> >> >> >> >     while true; do
> >> >> >> >> >> >       echo -n > /dev/snd/pcmC0D0p
> >> >> >> >> >> >     done
> >> >> >> >> >> >
> >> >> >> >> >> > And, what if the device is resumed from power-saving by an
> >> >> >> >> >> > audio
> >> >> >> >> >> > playback?  Immediately after the power up, it'll start the
> >> >> >> >> >> > streaming,
> >> >> >> >> >> > so it'll turn on EAPD.
> >> >> >> >> >> >
> >> >> >> >> >> > That said, isn't it just a need for some delay before EAPD
> >> >> >> >> >> > on/off?
> >> >> >> >> >>
> >> >> >> >> >> No, eapd on or off will cause pop noise.
> >> >> >> >> >
> >> >> >> >> > Does it do always?
> >> >> >> >> No, pop noise just one pop sound. It can not remove by software.
> >> >> >> >> But this machine need to do this always.
> >> >> >> >
> >> >> >> > So, doing EAPD on/off often is rather worse.  It's what your 
> >> >> >> > patch
> >> >> >> > does.
> >> >> >> >
> >> >> >> > Once again: did you try the script above?  If the pop noise 
> >> >> >> > always
> >> >> >>
> >> >> >> I had try it on my develop machine. I have no HP issue machine on 
> >> >> >> my
> >> >> >> site.
> >> >> >>
> >> >> >> > happens, the operation like above is the worst scenario.
> >> >> >> >
> >> >> >> > Then, let's think again how to *reduce* it.  We need one EAPD on 
> >> >> >> > at
> >> >> >> > least after each power up.  But it's needed only when you 
> >> >> >> > actually
> >> >> >> > plays a stream.  That is, we need only enable EAPD at playback
> >> >> >> > stream
> >> >> >> > open, but leave EAPD as is at close.
> >> >> >> >
> >> >> >>
> >> >> >> [we need only enable EAPD at playback stream open, but leave EAPD 
> >> >> >> as
> >> >> >> is
> >> >> >> at
> >> >> >> close.]
> >> >> >> Yes.
> >> >> >> So, I need to create eapd low in alc269_shutup. Right?
> >> >> >
> >> >> > Better to create own shutup callback (you can call alc269_shutup()
> >> >> > from there, too) and set it to spec->shutup in the fixup.  The 
> >> >> > pincfg
> >> >> > cleanup isn't often needed or may have bad effect.  You should test
> >> >> > with real machines what are needed and what are not.
> >> >>
> >> >> static void alc290_fixup_pop_noise(struct hda_codec *codec,
> >> >>            const struct hda_fixup *fix, int action)
> >> >> {
> >> >>  struct alc_spec *spec = codec->spec;
> >> >>
> >> >>  if (action == HDA_FIXUP_ACT_PRE_PROBE) {
> >> >>   spec->gen.pcm_playback_hook = ALC290_playback_pcm_hook;
> >> >>  }
> >> >> if (action ==   HDA_FIXUP_ACT_INIT) {
> >> >>   set_eapd(codec, 0x14, false);
> >> >>  }
> >> >> }
> >> >>
> >> >> Could I modify as upper code?
> >> >> To set eapd low in HDA_FIXUP_ACT_INIT state.
> >> >> If system resume, it will initial eapd to low.
> >> >> To write eapd to same value, it will have no pop noise.
> >> >> It will keep always low for eapd, only stream open and close have 
> >> >> change
> >> >> state for eapd.
> >> >
> >> > Yes, it looks OK.
> >> >
> >> >
> >> > Takashi
> >> >
> >> >
> >> >>
> >> >> >
> >> >> >
> >> >> > Takashi
> >> >> >
> >> >> >>
> >> >> >> >
> >> >> >> > Takashi
> >> >> >> >
> >> >> >> >> >> Our customer just want to do eapd on when stream is open.
> >> >> >> >> >> The pop noise will not clear. Because music gain was biger 
> >> >> >> >> >> than
> >> >> >> >> >> pop
> >> >> >> >> >> noise.
> >> >> >> >> >
> >> >> >> >> > Opening a stream doesn't mean that it follows the loud sound
> >> >> >> >> > immediately.  The script above wouldn't produce any sound but
> >> >> >> >> > just
> >> >> >> >> > does open/close the stream.  Did you try that?
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> > Takashi
> >> >> >> >> >
> >> >> >> >> > ------Please consider the environment before printing this
> >> >> >> >> > e-mail.
> >> >> >> >>
> >> >> >>
> >> >>
> >> [2 0001-set-eapd-follow-stream-on-off.patch <application/octet-stream 
> >> (quoted-printable)>]
> >>
> >
> > ------Please consider the environment before printing this e-mail. 
> [2 0002-set-eapd-follow-stream-on-off.patch <application/octet-stream (quoted-printable)>]
> 

Re: Set eapd to follow stream on or off

[Kailang]

> At Wed, 21 May 2014 17:24:16 +0800,
> Kailang wrote:
>>
>> > At Wed, 21 May 2014 14:28:12 +0800,
>> > Kailang wrote:
>> >>
>> >> Hi Takashi,
>> >>
>> >> Attach is the update patch.
>> >
>> > Keep the function in lower letters.
>> >
>> > Also, update the patch description as well (and better to put more
>> > words).
>>
>> Update as attach.
>> Thanks.
>
> Well, do you really want to turn off EAPD at closing the stream?

Yes.

> As I repeatedly explained, this results in more frequent EAPD on/off,
> i.e. more pop noises.
>

I have sent the patch for HP.
I will wait for feedback from customer.
If it is solve the issue, I will advise you.

Our AE says, windows use this method. HP is agree this usage.

>
> Takashi
>
>>
>> >
>> >
>> > thanks,
>> >
>> > Takashi
>> >
>> >> Many thanks.
>> >>
>> >> BR,
>> >> Kailang
>> >>
>> >>
>> >> > At Wed, 21 May 2014 13:58:14 +0800,
>> >> > Kailang wrote:
>> >> >>
>> >> >>
>> >> >> > At Tue, 20 May 2014 18:05:42 +0800,
>> >> >> > Kailang wrote:
>> >> >> >>
>> >> >> >> > At Tue, 20 May 2014 17:20:44 +0800,
>> >> >> >> > Kailang wrote:
>> >> >> >> >>
>> >> >> >> >>
>> >> >> >> >> > At Tue, 20 May 2014 16:52:43 +0800,
>> >> >> >> >> > Kailang wrote:
>> >> >> >> >> >>
>> >> >> >> >> >>
>> >> >> >> >> >> > At Tue, 20 May 2014 15:15:49 +0800,
>> >> >> >> >> >> > Kailang wrote:
>> >> >> >> >> >> >>
>> >> >> >> >> >> >> Hi Takashi,
>> >> >> >> >> >> >>
>> >> >> >> >> >> >> HP machine had pop noise via speaker.
>> >> >> >> >> >> >> If EAPD on or off, speaker will have pop noise.
>> >> >> >> >> >> >> So, they want to set eapd to follow with stream on or 
>> >> >> >> >> >> >> off.
>> >> >> >> >> >> >
>> >> >> >> >> >> > I can apply this as is, but still I wonder whether this
>> >> >> >> >> >> > results
>> >> >> >> >> >> > in
>> >> >> >> >> >> > other problems.  Doing EAPD on/off there means that it
>> >> >> >> >> >> > happens
>> >> >> >> >> >> > far
>> >> >> >> >> >> > more often.  For example, what happens if you do like
>> >> >> >> >> >> > below?
>> >> >> >> >> >> >
>> >> >> >> >> >> >     while true; do
>> >> >> >> >> >> >       echo -n > /dev/snd/pcmC0D0p
>> >> >> >> >> >> >     done
>> >> >> >> >> >> >
>> >> >> >> >> >> > And, what if the device is resumed from power-saving by 
>> >> >> >> >> >> > an
>> >> >> >> >> >> > audio
>> >> >> >> >> >> > playback?  Immediately after the power up, it'll start 
>> >> >> >> >> >> > the
>> >> >> >> >> >> > streaming,
>> >> >> >> >> >> > so it'll turn on EAPD.
>> >> >> >> >> >> >
>> >> >> >> >> >> > That said, isn't it just a need for some delay before 
>> >> >> >> >> >> > EAPD
>> >> >> >> >> >> > on/off?
>> >> >> >> >> >>
>> >> >> >> >> >> No, eapd on or off will cause pop noise.
>> >> >> >> >> >
>> >> >> >> >> > Does it do always?
>> >> >> >> >> No, pop noise just one pop sound. It can not remove by 
>> >> >> >> >> software.
>> >> >> >> >> But this machine need to do this always.
>> >> >> >> >
>> >> >> >> > So, doing EAPD on/off often is rather worse.  It's what your
>> >> >> >> > patch
>> >> >> >> > does.
>> >> >> >> >
>> >> >> >> > Once again: did you try the script above?  If the pop noise
>> >> >> >> > always
>> >> >> >>
>> >> >> >> I had try it on my develop machine. I have no HP issue machine 
>> >> >> >> on
>> >> >> >> my
>> >> >> >> site.
>> >> >> >>
>> >> >> >> > happens, the operation like above is the worst scenario.
>> >> >> >> >
>> >> >> >> > Then, let's think again how to *reduce* it.  We need one EAPD 
>> >> >> >> > on
>> >> >> >> > at
>> >> >> >> > least after each power up.  But it's needed only when you
>> >> >> >> > actually
>> >> >> >> > plays a stream.  That is, we need only enable EAPD at playback
>> >> >> >> > stream
>> >> >> >> > open, but leave EAPD as is at close.
>> >> >> >> >
>> >> >> >>
>> >> >> >> [we need only enable EAPD at playback stream open, but leave 
>> >> >> >> EAPD
>> >> >> >> as
>> >> >> >> is
>> >> >> >> at
>> >> >> >> close.]
>> >> >> >> Yes.
>> >> >> >> So, I need to create eapd low in alc269_shutup. Right?
>> >> >> >
>> >> >> > Better to create own shutup callback (you can call 
>> >> >> > alc269_shutup()
>> >> >> > from there, too) and set it to spec->shutup in the fixup.  The
>> >> >> > pincfg
>> >> >> > cleanup isn't often needed or may have bad effect.  You should 
>> >> >> > test
>> >> >> > with real machines what are needed and what are not.
>> >> >>
>> >> >> static void alc290_fixup_pop_noise(struct hda_codec *codec,
>> >> >>            const struct hda_fixup *fix, int action)
>> >> >> {
>> >> >>  struct alc_spec *spec = codec->spec;
>> >> >>
>> >> >>  if (action == HDA_FIXUP_ACT_PRE_PROBE) {
>> >> >>   spec->gen.pcm_playback_hook = ALC290_playback_pcm_hook;
>> >> >>  }
>> >> >> if (action ==   HDA_FIXUP_ACT_INIT) {
>> >> >>   set_eapd(codec, 0x14, false);
>> >> >>  }
>> >> >> }
>> >> >>
>> >> >> Could I modify as upper code?
>> >> >> To set eapd low in HDA_FIXUP_ACT_INIT state.
>> >> >> If system resume, it will initial eapd to low.
>> >> >> To write eapd to same value, it will have no pop noise.
>> >> >> It will keep always low for eapd, only stream open and close have
>> >> >> change
>> >> >> state for eapd.
>> >> >
>> >> > Yes, it looks OK.
>> >> >
>> >> >
>> >> > Takashi
>> >> >
>> >> >
>> >> >>
>> >> >> >
>> >> >> >
>> >> >> > Takashi
>> >> >> >
>> >> >> >>
>> >> >> >> >
>> >> >> >> > Takashi
>> >> >> >> >
>> >> >> >> >> >> Our customer just want to do eapd on when stream is open.
>> >> >> >> >> >> The pop noise will not clear. Because music gain was biger
>> >> >> >> >> >> than
>> >> >> >> >> >> pop
>> >> >> >> >> >> noise.
>> >> >> >> >> >
>> >> >> >> >> > Opening a stream doesn't mean that it follows the loud 
>> >> >> >> >> > sound
>> >> >> >> >> > immediately.  The script above wouldn't produce any sound 
>> >> >> >> >> > but
>> >> >> >> >> > just
>> >> >> >> >> > does open/close the stream.  Did you try that?
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> > Takashi
>> >> >> >> >> >
>> >> >> >> >> > ------Please consider the environment before printing this
>> >> >> >> >> > e-mail.
>> >> >> >> >>
>> >> >> >>
>> >> >>
>> >> [2 0001-set-eapd-follow-stream-on-off.patch <application/octet-stream
>> >> (quoted-printable)>]
>> >>
>> >
>> > ------Please consider the environment before printing this e-mail.
>> [2 0002-set-eapd-follow-stream-on-off.patch <application/octet-stream 
>> (quoted-printable)>]
>> 

Re: Set eapd to follow stream on or off

[Takashi Iwai]

At Thu, 22 May 2014 14:26:04 +0800,
Kailang wrote:
> 
> 
> 
> > At Wed, 21 May 2014 17:24:16 +0800,
> > Kailang wrote:
> >>
> >> > At Wed, 21 May 2014 14:28:12 +0800,
> >> > Kailang wrote:
> >> >>
> >> >> Hi Takashi,
> >> >>
> >> >> Attach is the update patch.
> >> >
> >> > Keep the function in lower letters.
> >> >
> >> > Also, update the patch description as well (and better to put more
> >> > words).
> >>
> >> Update as attach.
> >> Thanks.
> >
> > Well, do you really want to turn off EAPD at closing the stream?
> 
> Yes.
> 
> > As I repeatedly explained, this results in more frequent EAPD on/off,
> > i.e. more pop noises.
> >
> 
> I have sent the patch for HP.
> I will wait for feedback from customer.

Did you send the test script, too?

> If it is solve the issue, I will advise you.
> 
> Our AE says, windows use this method. HP is agree this usage.

Windows does this likely because of possible power saving, not because
of pop noises.

I'm fine to take the patch, but only when I'm convinced why it really
fixes the pop noise problem in the scenario below:
- playing a silent stream
- frequent open / close a stream


Takashi


> 
> >
> > Takashi
> >
> >>
> >> >
> >> >
> >> > thanks,
> >> >
> >> > Takashi
> >> >
> >> >> Many thanks.
> >> >>
> >> >> BR,
> >> >> Kailang
> >> >>
> >> >>
> >> >> > At Wed, 21 May 2014 13:58:14 +0800,
> >> >> > Kailang wrote:
> >> >> >>
> >> >> >>
> >> >> >> > At Tue, 20 May 2014 18:05:42 +0800,
> >> >> >> > Kailang wrote:
> >> >> >> >>
> >> >> >> >> > At Tue, 20 May 2014 17:20:44 +0800,
> >> >> >> >> > Kailang wrote:
> >> >> >> >> >>
> >> >> >> >> >>
> >> >> >> >> >> > At Tue, 20 May 2014 16:52:43 +0800,
> >> >> >> >> >> > Kailang wrote:
> >> >> >> >> >> >>
> >> >> >> >> >> >>
> >> >> >> >> >> >> > At Tue, 20 May 2014 15:15:49 +0800,
> >> >> >> >> >> >> > Kailang wrote:
> >> >> >> >> >> >> >>
> >> >> >> >> >> >> >> Hi Takashi,
> >> >> >> >> >> >> >>
> >> >> >> >> >> >> >> HP machine had pop noise via speaker.
> >> >> >> >> >> >> >> If EAPD on or off, speaker will have pop noise.
> >> >> >> >> >> >> >> So, they want to set eapd to follow with stream on or 
> >> >> >> >> >> >> >> off.
> >> >> >> >> >> >> >
> >> >> >> >> >> >> > I can apply this as is, but still I wonder whether this
> >> >> >> >> >> >> > results
> >> >> >> >> >> >> > in
> >> >> >> >> >> >> > other problems.  Doing EAPD on/off there means that it
> >> >> >> >> >> >> > happens
> >> >> >> >> >> >> > far
> >> >> >> >> >> >> > more often.  For example, what happens if you do like
> >> >> >> >> >> >> > below?
> >> >> >> >> >> >> >
> >> >> >> >> >> >> >     while true; do
> >> >> >> >> >> >> >       echo -n > /dev/snd/pcmC0D0p
> >> >> >> >> >> >> >     done
> >> >> >> >> >> >> >
> >> >> >> >> >> >> > And, what if the device is resumed from power-saving by 
> >> >> >> >> >> >> > an
> >> >> >> >> >> >> > audio
> >> >> >> >> >> >> > playback?  Immediately after the power up, it'll start 
> >> >> >> >> >> >> > the
> >> >> >> >> >> >> > streaming,
> >> >> >> >> >> >> > so it'll turn on EAPD.
> >> >> >> >> >> >> >
> >> >> >> >> >> >> > That said, isn't it just a need for some delay before 
> >> >> >> >> >> >> > EAPD
> >> >> >> >> >> >> > on/off?
> >> >> >> >> >> >>
> >> >> >> >> >> >> No, eapd on or off will cause pop noise.
> >> >> >> >> >> >
> >> >> >> >> >> > Does it do always?
> >> >> >> >> >> No, pop noise just one pop sound. It can not remove by 
> >> >> >> >> >> software.
> >> >> >> >> >> But this machine need to do this always.
> >> >> >> >> >
> >> >> >> >> > So, doing EAPD on/off often is rather worse.  It's what your
> >> >> >> >> > patch
> >> >> >> >> > does.
> >> >> >> >> >
> >> >> >> >> > Once again: did you try the script above?  If the pop noise
> >> >> >> >> > always
> >> >> >> >>
> >> >> >> >> I had try it on my develop machine. I have no HP issue machine 
> >> >> >> >> on
> >> >> >> >> my
> >> >> >> >> site.
> >> >> >> >>
> >> >> >> >> > happens, the operation like above is the worst scenario.
> >> >> >> >> >
> >> >> >> >> > Then, let's think again how to *reduce* it.  We need one EAPD 
> >> >> >> >> > on
> >> >> >> >> > at
> >> >> >> >> > least after each power up.  But it's needed only when you
> >> >> >> >> > actually
> >> >> >> >> > plays a stream.  That is, we need only enable EAPD at playback
> >> >> >> >> > stream
> >> >> >> >> > open, but leave EAPD as is at close.
> >> >> >> >> >
> >> >> >> >>
> >> >> >> >> [we need only enable EAPD at playback stream open, but leave 
> >> >> >> >> EAPD
> >> >> >> >> as
> >> >> >> >> is
> >> >> >> >> at
> >> >> >> >> close.]
> >> >> >> >> Yes.
> >> >> >> >> So, I need to create eapd low in alc269_shutup. Right?
> >> >> >> >
> >> >> >> > Better to create own shutup callback (you can call 
> >> >> >> > alc269_shutup()
> >> >> >> > from there, too) and set it to spec->shutup in the fixup.  The
> >> >> >> > pincfg
> >> >> >> > cleanup isn't often needed or may have bad effect.  You should 
> >> >> >> > test
> >> >> >> > with real machines what are needed and what are not.
> >> >> >>
> >> >> >> static void alc290_fixup_pop_noise(struct hda_codec *codec,
> >> >> >>            const struct hda_fixup *fix, int action)
> >> >> >> {
> >> >> >>  struct alc_spec *spec = codec->spec;
> >> >> >>
> >> >> >>  if (action == HDA_FIXUP_ACT_PRE_PROBE) {
> >> >> >>   spec->gen.pcm_playback_hook = ALC290_playback_pcm_hook;
> >> >> >>  }
> >> >> >> if (action ==   HDA_FIXUP_ACT_INIT) {
> >> >> >>   set_eapd(codec, 0x14, false);
> >> >> >>  }
> >> >> >> }
> >> >> >>
> >> >> >> Could I modify as upper code?
> >> >> >> To set eapd low in HDA_FIXUP_ACT_INIT state.
> >> >> >> If system resume, it will initial eapd to low.
> >> >> >> To write eapd to same value, it will have no pop noise.
> >> >> >> It will keep always low for eapd, only stream open and close have
> >> >> >> change
> >> >> >> state for eapd.
> >> >> >
> >> >> > Yes, it looks OK.
> >> >> >
> >> >> >
> >> >> > Takashi
> >> >> >
> >> >> >
> >> >> >>
> >> >> >> >
> >> >> >> >
> >> >> >> > Takashi
> >> >> >> >
> >> >> >> >>
> >> >> >> >> >
> >> >> >> >> > Takashi
> >> >> >> >> >
> >> >> >> >> >> >> Our customer just want to do eapd on when stream is open.
> >> >> >> >> >> >> The pop noise will not clear. Because music gain was biger
> >> >> >> >> >> >> than
> >> >> >> >> >> >> pop
> >> >> >> >> >> >> noise.
> >> >> >> >> >> >
> >> >> >> >> >> > Opening a stream doesn't mean that it follows the loud 
> >> >> >> >> >> > sound
> >> >> >> >> >> > immediately.  The script above wouldn't produce any sound 
> >> >> >> >> >> > but
> >> >> >> >> >> > just
> >> >> >> >> >> > does open/close the stream.  Did you try that?
> >> >> >> >> >> >
> >> >> >> >> >> >
> >> >> >> >> >> > Takashi
> >> >> >> >> >> >
> >> >> >> >> >> > ------Please consider the environment before printing this
> >> >> >> >> >> > e-mail.
> >> >> >> >> >>
> >> >> >> >>
> >> >> >>
> >> >> [2 0001-set-eapd-follow-stream-on-off.patch <application/octet-stream
> >> >> (quoted-printable)>]
> >> >>
> >> >
> >> > ------Please consider the environment before printing this e-mail.
> >> [2 0002-set-eapd-follow-stream-on-off.patch <application/octet-stream 
> >> (quoted-printable)>]
> >> 
> 

Re: Set eapd to follow stream on or off

[Kailang]

> At Thu, 22 May 2014 14:26:04 +0800,
> Kailang wrote:
>>
>>
>>
>> > At Wed, 21 May 2014 17:24:16 +0800,
>> > Kailang wrote:
>> >>
>> >> > At Wed, 21 May 2014 14:28:12 +0800,
>> >> > Kailang wrote:
>> >> >>
>> >> >> Hi Takashi,
>> >> >>
>> >> >> Attach is the update patch.
>> >> >
>> >> > Keep the function in lower letters.
>> >> >
>> >> > Also, update the patch description as well (and better to put more
>> >> > words).
>> >>
>> >> Update as attach.
>> >> Thanks.
>> >
>> > Well, do you really want to turn off EAPD at closing the stream?
>>
>> Yes.
>>
>> > As I repeatedly explained, this results in more frequent EAPD on/off,
>> > i.e. more pop noises.
>> >
>>
>> I have sent the patch for HP.
>> I will wait for feedback from customer.
>
> Did you send the test script, too?
No.

>
>> If it is solve the issue, I will advise you.
>>
>> Our AE says, windows use this method. HP is agree this usage.
>
> Windows does this likely because of possible power saving, not because
> of pop noises.
>
> I'm fine to take the patch, but only when I'm convinced why it really
> fixes the pop noise problem in the scenario below:
> - playing a silent stream
> - frequent open / close a stream
>

You are right. It is the key points. But I think first stream will show pop 
noise. It was hard to remove.
Keep EAPD to high after first stream start maybe a good idea.
Thanks.
I think to request the machine is better.
I will let our AE to request the machine.

>
> Takashi
>
>
>>
>> >
>> > Takashi
>> >
>> >>
>> >> >
>> >> >
>> >> > thanks,
>> >> >
>> >> > Takashi
>> >> >
>> >> >> Many thanks.
>> >> >>
>> >> >> BR,
>> >> >> Kailang
>> >> >>
>> >> >>
>> >> >> > At Wed, 21 May 2014 13:58:14 +0800,
>> >> >> > Kailang wrote:
>> >> >> >>
>> >> >> >>
>> >> >> >> > At Tue, 20 May 2014 18:05:42 +0800,
>> >> >> >> > Kailang wrote:
>> >> >> >> >>
>> >> >> >> >> > At Tue, 20 May 2014 17:20:44 +0800,
>> >> >> >> >> > Kailang wrote:
>> >> >> >> >> >>
>> >> >> >> >> >>
>> >> >> >> >> >> > At Tue, 20 May 2014 16:52:43 +0800,
>> >> >> >> >> >> > Kailang wrote:
>> >> >> >> >> >> >>
>> >> >> >> >> >> >>
>> >> >> >> >> >> >> > At Tue, 20 May 2014 15:15:49 +0800,
>> >> >> >> >> >> >> > Kailang wrote:
>> >> >> >> >> >> >> >>
>> >> >> >> >> >> >> >> Hi Takashi,
>> >> >> >> >> >> >> >>
>> >> >> >> >> >> >> >> HP machine had pop noise via speaker.
>> >> >> >> >> >> >> >> If EAPD on or off, speaker will have pop noise.
>> >> >> >> >> >> >> >> So, they want to set eapd to follow with stream on 
>> >> >> >> >> >> >> >> or
>> >> >> >> >> >> >> >> off.
>> >> >> >> >> >> >> >
>> >> >> >> >> >> >> > I can apply this as is, but still I wonder whether 
>> >> >> >> >> >> >> > this
>> >> >> >> >> >> >> > results
>> >> >> >> >> >> >> > in
>> >> >> >> >> >> >> > other problems.  Doing EAPD on/off there means that 
>> >> >> >> >> >> >> > it
>> >> >> >> >> >> >> > happens
>> >> >> >> >> >> >> > far
>> >> >> >> >> >> >> > more often.  For example, what happens if you do like
>> >> >> >> >> >> >> > below?
>> >> >> >> >> >> >> >
>> >> >> >> >> >> >> >     while true; do
>> >> >> >> >> >> >> >       echo -n > /dev/snd/pcmC0D0p
>> >> >> >> >> >> >> >     done
>> >> >> >> >> >> >> >
>> >> >> >> >> >> >> > And, what if the device is resumed from power-saving 
>> >> >> >> >> >> >> > by
>> >> >> >> >> >> >> > an
>> >> >> >> >> >> >> > audio
>> >> >> >> >> >> >> > playback?  Immediately after the power up, it'll 
>> >> >> >> >> >> >> > start
>> >> >> >> >> >> >> > the
>> >> >> >> >> >> >> > streaming,
>> >> >> >> >> >> >> > so it'll turn on EAPD.
>> >> >> >> >> >> >> >
>> >> >> >> >> >> >> > That said, isn't it just a need for some delay before
>> >> >> >> >> >> >> > EAPD
>> >> >> >> >> >> >> > on/off?
>> >> >> >> >> >> >>
>> >> >> >> >> >> >> No, eapd on or off will cause pop noise.
>> >> >> >> >> >> >
>> >> >> >> >> >> > Does it do always?
>> >> >> >> >> >> No, pop noise just one pop sound. It can not remove by
>> >> >> >> >> >> software.
>> >> >> >> >> >> But this machine need to do this always.
>> >> >> >> >> >
>> >> >> >> >> > So, doing EAPD on/off often is rather worse.  It's what 
>> >> >> >> >> > your
>> >> >> >> >> > patch
>> >> >> >> >> > does.
>> >> >> >> >> >
>> >> >> >> >> > Once again: did you try the script above?  If the pop noise
>> >> >> >> >> > always
>> >> >> >> >>
>> >> >> >> >> I had try it on my develop machine. I have no HP issue 
>> >> >> >> >> machine
>> >> >> >> >> on
>> >> >> >> >> my
>> >> >> >> >> site.
>> >> >> >> >>
>> >> >> >> >> > happens, the operation like above is the worst scenario.
>> >> >> >> >> >
>> >> >> >> >> > Then, let's think again how to *reduce* it.  We need one 
>> >> >> >> >> > EAPD
>> >> >> >> >> > on
>> >> >> >> >> > at
>> >> >> >> >> > least after each power up.  But it's needed only when you
>> >> >> >> >> > actually
>> >> >> >> >> > plays a stream.  That is, we need only enable EAPD at 
>> >> >> >> >> > playback
>> >> >> >> >> > stream
>> >> >> >> >> > open, but leave EAPD as is at close.
>> >> >> >> >> >
>> >> >> >> >>
>> >> >> >> >> [we need only enable EAPD at playback stream open, but leave
>> >> >> >> >> EAPD
>> >> >> >> >> as
>> >> >> >> >> is
>> >> >> >> >> at
>> >> >> >> >> close.]
>> >> >> >> >> Yes.
>> >> >> >> >> So, I need to create eapd low in alc269_shutup. Right?
>> >> >> >> >
>> >> >> >> > Better to create own shutup callback (you can call
>> >> >> >> > alc269_shutup()
>> >> >> >> > from there, too) and set it to spec->shutup in the fixup.  The
>> >> >> >> > pincfg
>> >> >> >> > cleanup isn't often needed or may have bad effect.  You should
>> >> >> >> > test
>> >> >> >> > with real machines what are needed and what are not.
>> >> >> >>
>> >> >> >> static void alc290_fixup_pop_noise(struct hda_codec *codec,
>> >> >> >>            const struct hda_fixup *fix, int action)
>> >> >> >> {
>> >> >> >>  struct alc_spec *spec = codec->spec;
>> >> >> >>
>> >> >> >>  if (action == HDA_FIXUP_ACT_PRE_PROBE) {
>> >> >> >>   spec->gen.pcm_playback_hook = ALC290_playback_pcm_hook;
>> >> >> >>  }
>> >> >> >> if (action ==   HDA_FIXUP_ACT_INIT) {
>> >> >> >>   set_eapd(codec, 0x14, false);
>> >> >> >>  }
>> >> >> >> }
>> >> >> >>
>> >> >> >> Could I modify as upper code?
>> >> >> >> To set eapd low in HDA_FIXUP_ACT_INIT state.
>> >> >> >> If system resume, it will initial eapd to low.
>> >> >> >> To write eapd to same value, it will have no pop noise.
>> >> >> >> It will keep always low for eapd, only stream open and close 
>> >> >> >> have
>> >> >> >> change
>> >> >> >> state for eapd.
>> >> >> >
>> >> >> > Yes, it looks OK.
>> >> >> >
>> >> >> >
>> >> >> > Takashi
>> >> >> >
>> >> >> >
>> >> >> >>
>> >> >> >> >
>> >> >> >> >
>> >> >> >> > Takashi
>> >> >> >> >
>> >> >> >> >>
>> >> >> >> >> >
>> >> >> >> >> > Takashi
>> >> >> >> >> >
>> >> >> >> >> >> >> Our customer just want to do eapd on when stream is 
>> >> >> >> >> >> >> open.
>> >> >> >> >> >> >> The pop noise will not clear. Because music gain was 
>> >> >> >> >> >> >> biger
>> >> >> >> >> >> >> than
>> >> >> >> >> >> >> pop
>> >> >> >> >> >> >> noise.
>> >> >> >> >> >> >
>> >> >> >> >> >> > Opening a stream doesn't mean that it follows the loud
>> >> >> >> >> >> > sound
>> >> >> >> >> >> > immediately.  The script above wouldn't produce any 
>> >> >> >> >> >> > sound
>> >> >> >> >> >> > but
>> >> >> >> >> >> > just
>> >> >> >> >> >> > does open/close the stream.  Did you try that?
>> >> >> >> >> >> >
>> >> >> >> >> >> >
>> >> >> >> >> >> > Takashi
>> >> >> >> >> >> >
>> >> >> >> >> >> > ------Please consider the environment before printing 
>> >> >> >> >> >> > this
>> >> >> >> >> >> > e-mail.
>> >> >> >> >> >>
>> >> >> >> >>
>> >> >> >>
>> >> >> [2 0001-set-eapd-follow-stream-on-off.patch 
>> >> >> <application/octet-stream
>> >> >> (quoted-printable)>]
>> >> >>
>> >> >
>> >> > ------Please consider the environment before printing this e-mail.
>> >> [2 0002-set-eapd-follow-stream-on-off.patch <application/octet-stream
>> >> (quoted-printable)>]
>> >>
>> 

Add support headset mode for alc233

[Kailang]

[-- Attachment #1: Type: text/plain, Size: 74 bytes --]

Hi Takashi,

Attach is support headset mode for alc233 patch.

BR,
Kailang

[-- Attachment #2: 0000-add-support-headset-mode-for-alc233.patch --]
[-- Type: application/octet-stream, Size: 2382 bytes --]

>From f84879b9898a582319b15076562f1758028e1263 Mon Sep 17 00:00:00 2001
From: Kailang Yang <kailang@realtek.com>
Date: Wed, 21 May 2014 16:49:48 +0800
Subject: [PATCH] ALSA: hda/realtek - Add support headset mode for ALC233

Signed-off-by: Kailang Yang <kailang@realtek.com>

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index c6c35f7..069767b 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -3525,6 +3525,7 @@ static void alc_headset_mode_unplugged(struct hda_codec *codec)
 		/* Direct Drive HP Amp control */
 		alc_write_coefex_idx(codec, 0x57, 0x03, 0x8aa6);
 		break;
+	case 0x10ec0233:
 	case 0x10ec0283:
 		alc_write_coef_idx(codec, 0x1b, 0x0c0b);
 		alc_write_coef_idx(codec, 0x45, 0xc429);
@@ -3596,6 +3597,7 @@ static void alc_headset_mode_mic_in(struct hda_codec *codec, hda_nid_t hp_pin,
 		alc_write_coef_idx(codec, 0x06, 0x6100);
 		snd_hda_set_pin_ctl_cache(codec, mic_pin, PIN_VREF50);
 		break;
+	case 0x10ec0233:
 	case 0x10ec0283:
 		alc_write_coef_idx(codec, 0x45, 0xc429);
 		snd_hda_set_pin_ctl_cache(codec, hp_pin, 0);
@@ -3667,6 +3669,7 @@ static void alc_headset_mode_default(struct hda_codec *codec)
 		alc_write_coefex_idx(codec, 0x57, 0x03, 0x8ea6);
 		alc_write_coef_idx(codec, 0x49, 0x0049);
 		break;
+	case 0x10ec0233:
 	case 0x10ec0283:
 		alc_write_coef_idx(codec, 0x06, 0x2100);
 		alc_write_coef_idx(codec, 0x32, 0x4ea3);
@@ -3721,6 +3724,7 @@ static void alc_headset_mode_ctia(struct hda_codec *codec)
 		alc_write_coef_idx(codec, 0x1b, 0x0c2b);
 		alc_write_coefex_idx(codec, 0x57, 0x03, 0x8ea6);
 		break;
+	case 0x10ec0233:
 	case 0x10ec0283:
 		alc_write_coef_idx(codec, 0x45, 0xd429);
 		alc_write_coef_idx(codec, 0x1b, 0x0c2b);
@@ -3773,6 +3777,7 @@ static void alc_headset_mode_omtp(struct hda_codec *codec)
 		alc_write_coef_idx(codec, 0x1b, 0x0c2b);
 		alc_write_coefex_idx(codec, 0x57, 0x03, 0x8ea6);
 		break;
+	case 0x10ec0233:
 	case 0x10ec0283:
 		alc_write_coef_idx(codec, 0x45, 0xe429);
 		alc_write_coef_idx(codec, 0x1b, 0x0c2b);
@@ -3829,6 +3834,7 @@ static void alc_determine_headset_type(struct hda_codec *codec)
 		val = alc_read_coef_idx(codec, 0x46);
 		is_ctia = (val & 0x0070) == 0x0070;
 		break;
+	case 0x10ec0233:
 	case 0x10ec0283:
 		alc_write_coef_idx(codec, 0x45, 0xd029);
 		msleep(300);

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: Add support headset mode for alc233

[Takashi Iwai]

At Wed, 21 May 2014 16:55:14 +0800,
Kailang wrote:
> 
> Hi Takashi,
> 
> Attach is support headset mode for alc233 patch.

Thanks, applied.


Takashi

> 
> BR,
> Kailang
> [2 0000-add-support-headset-mode-for-alc233.patch <application/octet-stream (quoted-printable)>]
>