Re: Set eapd to follow stream on or off

[Kailang <kailang@realtek.com>]

> 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)>]
>> >>
>>