[alsa-devel] Set eapd to follow stream on or off

[Kailang]

Hi Takashi,

I abandon this update.
Playing a slient stream will be had pop noise.
It is hardware issue. It is not to avoid.
But it need to read and write our hidden register to control eapd.
Resume back need to fill eapd to keep low state.
I think I just send the patch to ubuntu team.
Thanks.

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