Boot procudure on HDA driver

Boot procudure on HDA driver

[Kailang]

Hi Takashi,

When System Boot up.
The Hda Driver running step was as below.

alc_init();
alc_shutup();
alc_init();

The depop procedure was put in spec->init_hook and spec->shutup.

But I find more codec which run spec->shutup at boot up. It will occur pop noise.
If it doesn't run spec->shutup, it will not occur pop noise or reduce pop noise.

How could the spec->shutup not run at boot up?

I ever test to put the check in spec->shutup() for PM_EVENT_SUSPEND and PM_EVENT_HIBERNATE and shutdown.
But if power_save=1, it was have issue for this.
Codec was idle already in power_save=1 state. If system go suspend, it will not run spec->shutup() again.

BR,
Kailang

Re: Boot procudure on HDA driver

[Takashi Iwai]

On Mon, 13 May 2019 11:00:18 +0200,
Kailang wrote:
> 
> Hi Takashi,
> 
> When System Boot up.
> The Hda Driver running step was as below.
> 
> alc_init();
> alc_shutup();
> alc_init();
> 
> The depop procedure was put in spec->init_hook and spec->shutup.
> 
> But I find more codec which run spec->shutup at boot up. It will occur pop noise.
> If it doesn't run spec->shutup, it will not occur pop noise or reduce pop noise.
> 
> How could the spec->shutup not run at boot up?
> 
> I ever test to put the check in spec->shutup() for PM_EVENT_SUSPEND and PM_EVENT_HIBERNATE and shutdown.
> But if power_save=1, it was have issue for this.
> Codec was idle already in power_save=1 state. If system go suspend, it will not run spec->shutup() again.

I guess it's a runtime PM, hence it's neither PM_EVENT_SUSPEND nor
PM_EVENT_HIBERNATE.

Actually, if the shutup procedure makes the problem on a certain
platform, just skip it.  It's an optional behavior and would be fine
without it (of course only if it's confirmed to work :)


Takashi

Re: Boot procudure on HDA driver

[Kailang]

> -----Original Message-----
> From: Takashi Iwai <tiwai@suse.de>
> Sent: Monday, May 13, 2019 5:11 PM
> To: Kailang <kailang@realtek.com>
> Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
> Subject: Re: Boot procudure on HDA driver
> 
> On Mon, 13 May 2019 11:00:18 +0200,
> Kailang wrote:
> >
> > Hi Takashi,
> >
> > When System Boot up.
> > The Hda Driver running step was as below.
> >
> > alc_init();
> > alc_shutup();
> > alc_init();
> >
> > The depop procedure was put in spec->init_hook and spec->shutup.
> >
> > But I find more codec which run spec->shutup at boot up. It will occur pop
> noise.
> > If it doesn't run spec->shutup, it will not occur pop noise or reduce pop noise.
> >
> > How could the spec->shutup not run at boot up?
> >
> > I ever test to put the check in spec->shutup() for PM_EVENT_SUSPEND and
> PM_EVENT_HIBERNATE and shutdown.
> > But if power_save=1, it was have issue for this.
> > Codec was idle already in power_save=1 state. If system go suspend, it will
> not run spec->shutup() again.
> 
> I guess it's a runtime PM, hence it's neither PM_EVENT_SUSPEND nor
> PM_EVENT_HIBERNATE.
> 
runtime PM and suspend and hibernate and shutdown need to run spec->shutup(). It's no problem.
But spec->shutup() doesn't need to run in boot up.
And it will set power_save=1 on all dell machine.

If (codec->auto_runtime_pm || codec->bus->shutdown ||
      codec->core.dev.power.power_state.event == PM_EVENT_SUSPEND ||
      codec->core.dev.power.power_state.event == PM_EVENT_HIBERNATE)

So, I need to put upper check code in spec->shutup(). Right?
Thanks.

> Actually, if the shutup procedure makes the problem on a certain platform, just
> skip it.  It's an optional behavior and would be fine without it (of course only if
> it's confirmed to work :)
> 
> 
> Takashi
> 
> ------Please consider the environment before printing this e-mail.

Re: Boot procudure on HDA driver

[Takashi Iwai]

On Mon, 13 May 2019 11:30:56 +0200,
Kailang wrote:
> 
> 
> 
> > -----Original Message-----
> > From: Takashi Iwai <tiwai@suse.de>
> > Sent: Monday, May 13, 2019 5:11 PM
> > To: Kailang <kailang@realtek.com>
> > Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
> > Subject: Re: Boot procudure on HDA driver
> > 
> > On Mon, 13 May 2019 11:00:18 +0200,
> > Kailang wrote:
> > >
> > > Hi Takashi,
> > >
> > > When System Boot up.
> > > The Hda Driver running step was as below.
> > >
> > > alc_init();
> > > alc_shutup();
> > > alc_init();
> > >
> > > The depop procedure was put in spec->init_hook and spec->shutup.
> > >
> > > But I find more codec which run spec->shutup at boot up. It will occur pop
> > noise.
> > > If it doesn't run spec->shutup, it will not occur pop noise or reduce pop noise.
> > >
> > > How could the spec->shutup not run at boot up?
> > >
> > > I ever test to put the check in spec->shutup() for PM_EVENT_SUSPEND and
> > PM_EVENT_HIBERNATE and shutdown.
> > > But if power_save=1, it was have issue for this.
> > > Codec was idle already in power_save=1 state. If system go suspend, it will
> > not run spec->shutup() again.
> > 
> > I guess it's a runtime PM, hence it's neither PM_EVENT_SUSPEND nor
> > PM_EVENT_HIBERNATE.
> > 
> runtime PM and suspend and hibernate and shutdown need to run
> spec->shutup().

Is the call really mandatory?

> It's no problem.
> But spec->shutup() doesn't need to run in boot up.

The call of spec->shutup() at boot up *is* the runtime PM.
Or any other call path I overlooked?

> And it will set power_save=1 on all dell machine.
> 
> If (codec->auto_runtime_pm || codec->bus->shutdown ||
>       codec->core.dev.power.power_state.event == PM_EVENT_SUSPEND ||
>       codec->core.dev.power.power_state.event == PM_EVENT_HIBERNATE)
> 
> So, I need to put upper check code in spec->shutup(). Right?
> Thanks.

No, the auto_runtime_pm is for a completely different purpose.

At the boot up, the runtime PM can be kicked in at any time.  So if
you disable the shutup during the runtime PM, it means you'd need to
call the shutup at runtime PM completely.


Takashi

> 
> > Actually, if the shutup procedure makes the problem on a certain platform, just
> > skip it.  It's an optional behavior and would be fine without it (of course only if
> > it's confirmed to work :)
> > 
> > 
> > Takashi
> > 
> > ------Please consider the environment before printing this e-mail.
> 

Re: Boot procudure on HDA driver

[Takashi Iwai]

On Mon, 13 May 2019 12:36:36 +0200,
Kailang wrote:
> 
> 
> Maybe I confuse and I also confuse you. Sorry!!
> 
> When system suspend, it will run alc_suspend(). alc_suspend() was include alc_shutup().

Right.

> When system shutdown(power off), will it run alc_shutup()?

Yes, it's called via alc_reboot_notify().
If spec->reboot_notify is defined, this supersedes the call, so you
may define the callback if you'd need to avoid alc_shutup() call from
there.


> in our patch_realtek.c.
> spec->shutup = alc225_shutup;
> Some codec didn't run the depop procudure during boot up. 
> 
> Maybe disable EAPD and shutup PIN will cause pop noise.
> But this procudure need to run in SUSPEND and HIBERNATE and Power off.
> It also need to run it at runtime suspend.
> 
> static void alc225_shutup(struct hda_codec *codec)
> {
> 	struct alc_spec *spec = codec->spec;
> 	hda_nid_t hp_pin = spec->gen.autocfg.hp_pins[0];
> 	bool hp1_pin_sense, hp2_pin_sense;
> 
>         *********** depop procudure ******************************************  
> 	if (!hp_pin)
>                  hp_pin = 0x21;
> 
> 	/* 3k pull low control for Headset jack. */
> 	alc_update_coef_idx(codec, 0x4a, 0, 3 << 10);
> 
> 	hp1_pin_sense = snd_hda_jack_detect(codec, hp_pin);
> 	hp2_pin_sense = snd_hda_jack_detect(codec, 0x16);
> 
> 	if (hp1_pin_sense || hp2_pin_sense)
> 		msleep(2);
> 
> 	if (hp1_pin_sense)
> 		snd_hda_codec_write(codec, hp_pin, 0,
> 			    AC_VERB_SET_AMP_GAIN_MUTE, AMP_OUT_MUTE);
> 	if (hp2_pin_sense)
> 		snd_hda_codec_write(codec, 0x16, 0,
> 			    AC_VERB_SET_AMP_GAIN_MUTE, AMP_OUT_MUTE);
> 
> 	if (hp1_pin_sense || hp2_pin_sense)
> 		msleep(85);
> 
> 	if (hp1_pin_sense)
> 		snd_hda_codec_write(codec, hp_pin, 0,
> 			    AC_VERB_SET_PIN_WIDGET_CONTROL, 0x0);
> 	if (hp2_pin_sense)
> 		snd_hda_codec_write(codec, 0x16, 0,
> 			    AC_VERB_SET_PIN_WIDGET_CONTROL, 0x0);
> 
> 	if (hp1_pin_sense || hp2_pin_sense)
> 		msleep(100);
> 
> 	alc_auto_setup_eapd(codec, false);
> 	snd_hda_shutup_pins(codec);
>         ************** depop procudure ************************************* 
> }

Well, it's not clear to me whether you'd like to reduce the depop
procedure above or to add it somewhere else.  Could you clarify?


thanks,

Takashi


> 
> 
> 
> 
> 
> ________________________________________
> 從: Takashi Iwai [tiwai@suse.de]
> 寄件日期: 2019年5月13日 下午 05:42
> 至: Kailang
> 副本:  (alsa-devel@alsa-project.org)
> 主旨: Re: Boot procudure on HDA driver
> 
> On Mon, 13 May 2019 11:30:56 +0200,
> Kailang wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Takashi Iwai <tiwai@suse.de>
> > > Sent: Monday, May 13, 2019 5:11 PM
> > > To: Kailang <kailang@realtek.com>
> > > Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
> > > Subject: Re: Boot procudure on HDA driver
> > >
> > > On Mon, 13 May 2019 11:00:18 +0200,
> > > Kailang wrote:
> > > >
> > > > Hi Takashi,
> > > >
> > > > When System Boot up.
> > > > The Hda Driver running step was as below.
> > > >
> > > > alc_init();
> > > > alc_shutup();
> > > > alc_init();
> > > >
> > > > The depop procedure was put in spec->init_hook and spec->shutup.
> > > >
> > > > But I find more codec which run spec->shutup at boot up. It will occur pop
> > > noise.
> > > > If it doesn't run spec->shutup, it will not occur pop noise or reduce pop noise.
> > > >
> > > > How could the spec->shutup not run at boot up?
> > > >
> > > > I ever test to put the check in spec->shutup() for PM_EVENT_SUSPEND and
> > > PM_EVENT_HIBERNATE and shutdown.
> > > > But if power_save=1, it was have issue for this.
> > > > Codec was idle already in power_save=1 state. If system go suspend, it will
> > > not run spec->shutup() again.
> > >
> > > I guess it's a runtime PM, hence it's neither PM_EVENT_SUSPEND nor
> > > PM_EVENT_HIBERNATE.
> > >
> > runtime PM and suspend and hibernate and shutdown need to run
> > spec->shutup().
> 
> Is the call really mandatory?
> 
> > It's no problem.
> > But spec->shutup() doesn't need to run in boot up.
> 
> The call of spec->shutup() at boot up *is* the runtime PM.
> Or any other call path I overlooked?
> 
> > And it will set power_save=1 on all dell machine.
> >
> > If (codec->auto_runtime_pm || codec->bus->shutdown ||
> >       codec->core.dev.power.power_state.event == PM_EVENT_SUSPEND ||
> >       codec->core.dev.power.power_state.event == PM_EVENT_HIBERNATE)
> >
> > So, I need to put upper check code in spec->shutup(). Right?
> > Thanks.
> 
> No, the auto_runtime_pm is for a completely different purpose.
> 
> At the boot up, the runtime PM can be kicked in at any time.  So if
> you disable the shutup during the runtime PM, it means you'd need to
> call the shutup at runtime PM completely.
> 
> 
> Takashi
> 
> >
> > > Actually, if the shutup procedure makes the problem on a certain platform, just
> > > skip it.  It's an optional behavior and would be fine without it (of course only if
> > > it's confirmed to work :)
> > >
> > >
> > > Takashi
> > >
> > > ------Please consider the environment before printing this e-mail.
> >
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Boot procudure on HDA driver

[Takashi Iwai]

On Mon, 13 May 2019 13:52:32 +0200,
Kailang wrote:
> 
> 
> 
> >Well, it's not clear to me whether you'd like to reduce the depop
> >procedure above or to add it somewhere else.  Could you clarify?
> 
> This depop procedure in alc225_shutup. it doesn't need to run in system boot up.
> 
> If system boot up, it will run alc225_init() already. alc225_init() include power up depop procedure.
> So, if system boot up, it only need to run depop procedure in alc225_init().

This is the runtime suspend and resume.  That is:


> The current boot up procedure was below.
> 1. alc225_init()

... which is called at the driver probe time, from
snd_hda_codec_build_controls().

Then the codec driver initialization finishes at this point, hence it
goes to runtime suspend.

> 2. alc225_shutup()  ==> this include power off and suspend depop procedure. some codec run this field will have pop noise at boot up.

... which is a part of the codec runtime suspend.

Then, the codec gets woken up for some access, and it performs the
runtime resume.

> 3. alc225_init()

... which is a part of the codec runtime resume.

That is, the step 2 and 3 are already out of the initialization phase,
and they are normal runtime PM procedure.  If the runtime depop code
is superfluous for the runtime PM, you can skip it by checking the
power_state, for example.  But, again, it has nothing to do with the
boot or not.  It's just the normal runtime suspend and resume.


thanks,

Takashi


> 
> 
> ________________________________________
> 從: Takashi Iwai [tiwai@suse.de]
> 寄件日期: 2019年5月13日 下午 07:30
> 至: Kailang
> 副本:  (alsa-devel@alsa-project.org)
> 主旨: Re: Boot procudure on HDA driver
> 
> On Mon, 13 May 2019 12:36:36 +0200,
> Kailang wrote:
> >
> >
> > Maybe I confuse and I also confuse you. Sorry!!
> >
> > When system suspend, it will run alc_suspend(). alc_suspend() was include alc_shutup().
> 
> Right.
> 
> > When system shutdown(power off), will it run alc_shutup()?
> 
> Yes, it's called via alc_reboot_notify().
> If spec->reboot_notify is defined, this supersedes the call, so you
> may define the callback if you'd need to avoid alc_shutup() call from
> there.
> 
> 
> > in our patch_realtek.c.
> > spec->shutup = alc225_shutup;
> > Some codec didn't run the depop procudure during boot up.
> >
> > Maybe disable EAPD and shutup PIN will cause pop noise.
> > But this procudure need to run in SUSPEND and HIBERNATE and Power off.
> > It also need to run it at runtime suspend.
> >
> > static void alc225_shutup(struct hda_codec *codec)
> > {
> >       struct alc_spec *spec = codec->spec;
> >       hda_nid_t hp_pin = spec->gen.autocfg.hp_pins[0];
> >       bool hp1_pin_sense, hp2_pin_sense;
> >
> >         *********** depop procudure ******************************************
> >       if (!hp_pin)
> >                  hp_pin = 0x21;
> >
> >       /* 3k pull low control for Headset jack. */
> >       alc_update_coef_idx(codec, 0x4a, 0, 3 << 10);
> >
> >       hp1_pin_sense = snd_hda_jack_detect(codec, hp_pin);
> >       hp2_pin_sense = snd_hda_jack_detect(codec, 0x16);
> >
> >       if (hp1_pin_sense || hp2_pin_sense)
> >               msleep(2);
> >
> >       if (hp1_pin_sense)
> >               snd_hda_codec_write(codec, hp_pin, 0,
> >                           AC_VERB_SET_AMP_GAIN_MUTE, AMP_OUT_MUTE);
> >       if (hp2_pin_sense)
> >               snd_hda_codec_write(codec, 0x16, 0,
> >                           AC_VERB_SET_AMP_GAIN_MUTE, AMP_OUT_MUTE);
> >
> >       if (hp1_pin_sense || hp2_pin_sense)
> >               msleep(85);
> >
> >       if (hp1_pin_sense)
> >               snd_hda_codec_write(codec, hp_pin, 0,
> >                           AC_VERB_SET_PIN_WIDGET_CONTROL, 0x0);
> >       if (hp2_pin_sense)
> >               snd_hda_codec_write(codec, 0x16, 0,
> >                           AC_VERB_SET_PIN_WIDGET_CONTROL, 0x0);
> >
> >       if (hp1_pin_sense || hp2_pin_sense)
> >               msleep(100);
> >
> >       alc_auto_setup_eapd(codec, false);
> >       snd_hda_shutup_pins(codec);
> >         ************** depop procudure *************************************
> > }
> 
> Well, it's not clear to me whether you'd like to reduce the depop
> procedure above or to add it somewhere else.  Could you clarify?
> 
> 
> thanks,
> 
> Takashi
> 
> 
> >
> >
> >
> >
> >
> > ________________________________________
> > 從: Takashi Iwai [tiwai@suse.de]
> > 寄件日期: 2019年5月13日 下午 05:42
> > 至: Kailang
> > 副本:  (alsa-devel@alsa-project.org)
> > 主旨: Re: Boot procudure on HDA driver
> >
> > On Mon, 13 May 2019 11:30:56 +0200,
> > Kailang wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Takashi Iwai <tiwai@suse.de>
> > > > Sent: Monday, May 13, 2019 5:11 PM
> > > > To: Kailang <kailang@realtek.com>
> > > > Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
> > > > Subject: Re: Boot procudure on HDA driver
> > > >
> > > > On Mon, 13 May 2019 11:00:18 +0200,
> > > > Kailang wrote:
> > > > >
> > > > > Hi Takashi,
> > > > >
> > > > > When System Boot up.
> > > > > The Hda Driver running step was as below.
> > > > >
> > > > > alc_init();
> > > > > alc_shutup();
> > > > > alc_init();
> > > > >
> > > > > The depop procedure was put in spec->init_hook and spec->shutup.
> > > > >
> > > > > But I find more codec which run spec->shutup at boot up. It will occur pop
> > > > noise.
> > > > > If it doesn't run spec->shutup, it will not occur pop noise or reduce pop noise.
> > > > >
> > > > > How could the spec->shutup not run at boot up?
> > > > >
> > > > > I ever test to put the check in spec->shutup() for PM_EVENT_SUSPEND and
> > > > PM_EVENT_HIBERNATE and shutdown.
> > > > > But if power_save=1, it was have issue for this.
> > > > > Codec was idle already in power_save=1 state. If system go suspend, it will
> > > > not run spec->shutup() again.
> > > >
> > > > I guess it's a runtime PM, hence it's neither PM_EVENT_SUSPEND nor
> > > > PM_EVENT_HIBERNATE.
> > > >
> > > runtime PM and suspend and hibernate and shutdown need to run
> > > spec->shutup().
> >
> > Is the call really mandatory?
> >
> > > It's no problem.
> > > But spec->shutup() doesn't need to run in boot up.
> >
> > The call of spec->shutup() at boot up *is* the runtime PM.
> > Or any other call path I overlooked?
> >
> > > And it will set power_save=1 on all dell machine.
> > >
> > > If (codec->auto_runtime_pm || codec->bus->shutdown ||
> > >       codec->core.dev.power.power_state.event == PM_EVENT_SUSPEND ||
> > >       codec->core.dev.power.power_state.event == PM_EVENT_HIBERNATE)
> > >
> > > So, I need to put upper check code in spec->shutup(). Right?
> > > Thanks.
> >
> > No, the auto_runtime_pm is for a completely different purpose.
> >
> > At the boot up, the runtime PM can be kicked in at any time.  So if
> > you disable the shutup during the runtime PM, it means you'd need to
> > call the shutup at runtime PM completely.
> >
> >
> > Takashi
> >
> > >
> > > > Actually, if the shutup procedure makes the problem on a certain platform, just
> > > > skip it.  It's an optional behavior and would be fine without it (of course only if
> > > > it's confirmed to work :)
> > > >
> > > >
> > > > Takashi
> > > >
> > > > ------Please consider the environment before printing this e-mail.
> > >
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Boot procudure on HDA driver

[Takashi Iwai]

On Mon, 13 May 2019 15:26:12 +0200,
Kailang wrote:
> 
> >That is, the step 2 and 3 are already out of the initialization phase,
> >and they are normal runtime PM procedure.  If the runtime depop code
> >is superfluous for the runtime PM, you can skip it by checking the
> >power_state, for example.  But, again, it has nothing to do with the
> >boot or not.  It's just the normal runtime suspend and resume.
> 
> Sorry! runtime suspend also need to run depop procedure.
> It's not need to run depop procedure which was system power up during boot on alc225_shutup().

Hrm, it's still not clear to me.  The alc255_shutup() is called during
the boot up because the codec goes to the runtime suspend/resume.
There should be no difference between this call during the boot time
and the normal runtime PM behavior.


Takashi


> So, I think I can put checking runtime PM and Power_state and shutdown code in  alc225_shutup().
> 
> for example: (put below code in alc225_shutup)
> 
> If ( !pm_runtime_suspended(hda_codec_dev(codec)) || !codec->bus->shutdown ||
>           codec->core.dev.power.power_state.event != PM_EVENT_SUSPEND ||
>           codec->core.dev.power.power_state.event != PM_EVENT_HIBERNATE)
>                         return;       
> But I don't know which runtime_suspend state was pm_runtime_suspended(hda_codec_dev(codec)).
> ________________________________________
> 從: Takashi Iwai [tiwai@suse.de]
> 寄件日期: 2019年5月13日 下午 08:01
> 至: Kailang
> 副本:  (alsa-devel@alsa-project.org)
> 主旨: Re: Boot procudure on HDA driver
> 
> On Mon, 13 May 2019 13:52:32 +0200,
> Kailang wrote:
> >
> >
> >
> > >Well, it's not clear to me whether you'd like to reduce the depop
> > >procedure above or to add it somewhere else.  Could you clarify?
> >
> > This depop procedure in alc225_shutup. it doesn't need to run in system boot up.
> >
> > If system boot up, it will run alc225_init() already. alc225_init() include power up depop procedure.
> > So, if system boot up, it only need to run depop procedure in alc225_init().
> 
> This is the runtime suspend and resume.  That is:
> 
> 
> > The current boot up procedure was below.
> > 1. alc225_init()
> 
> ... which is called at the driver probe time, from
> snd_hda_codec_build_controls().
> 
> Then the codec driver initialization finishes at this point, hence it
> goes to runtime suspend.
> 
> > 2. alc225_shutup()  ==> this include power off and suspend depop procedure. some codec run this field will have pop noise at boot up.
> 
> ... which is a part of the codec runtime suspend.
> 
> Then, the codec gets woken up for some access, and it performs the
> runtime resume.
> 
> > 3. alc225_init()
> 
> ... which is a part of the codec runtime resume.
> 
> That is, the step 2 and 3 are already out of the initialization phase,
> and they are normal runtime PM procedure.  If the runtime depop code
> is superfluous for the runtime PM, you can skip it by checking the
> power_state, for example.  But, again, it has nothing to do with the
> boot or not.  It's just the normal runtime suspend and resume.
> 
> 
> thanks,
> 
> Takashi
> 
> 
> >
> >
> > ________________________________________
> > 從: Takashi Iwai [tiwai@suse.de]
> > 寄件日期: 2019年5月13日 下午 07:30
> > 至: Kailang
> > 副本:  (alsa-devel@alsa-project.org)
> > 主旨: Re: Boot procudure on HDA driver
> >
> > On Mon, 13 May 2019 12:36:36 +0200,
> > Kailang wrote:
> > >
> > >
> > > Maybe I confuse and I also confuse you. Sorry!!
> > >
> > > When system suspend, it will run alc_suspend(). alc_suspend() was include alc_shutup().
> >
> > Right.
> >
> > > When system shutdown(power off), will it run alc_shutup()?
> >
> > Yes, it's called via alc_reboot_notify().
> > If spec->reboot_notify is defined, this supersedes the call, so you
> > may define the callback if you'd need to avoid alc_shutup() call from
> > there.
> >
> >
> > > in our patch_realtek.c.
> > > spec->shutup = alc225_shutup;
> > > Some codec didn't run the depop procudure during boot up.
> > >
> > > Maybe disable EAPD and shutup PIN will cause pop noise.
> > > But this procudure need to run in SUSPEND and HIBERNATE and Power off.
> > > It also need to run it at runtime suspend.
> > >
> > > static void alc225_shutup(struct hda_codec *codec)
> > > {
> > >       struct alc_spec *spec = codec->spec;
> > >       hda_nid_t hp_pin = spec->gen.autocfg.hp_pins[0];
> > >       bool hp1_pin_sense, hp2_pin_sense;
> > >
> > >         *********** depop procudure ******************************************
> > >       if (!hp_pin)
> > >                  hp_pin = 0x21;
> > >
> > >       /* 3k pull low control for Headset jack. */
> > >       alc_update_coef_idx(codec, 0x4a, 0, 3 << 10);
> > >
> > >       hp1_pin_sense = snd_hda_jack_detect(codec, hp_pin);
> > >       hp2_pin_sense = snd_hda_jack_detect(codec, 0x16);
> > >
> > >       if (hp1_pin_sense || hp2_pin_sense)
> > >               msleep(2);
> > >
> > >       if (hp1_pin_sense)
> > >               snd_hda_codec_write(codec, hp_pin, 0,
> > >                           AC_VERB_SET_AMP_GAIN_MUTE, AMP_OUT_MUTE);
> > >       if (hp2_pin_sense)
> > >               snd_hda_codec_write(codec, 0x16, 0,
> > >                           AC_VERB_SET_AMP_GAIN_MUTE, AMP_OUT_MUTE);
> > >
> > >       if (hp1_pin_sense || hp2_pin_sense)
> > >               msleep(85);
> > >
> > >       if (hp1_pin_sense)
> > >               snd_hda_codec_write(codec, hp_pin, 0,
> > >                           AC_VERB_SET_PIN_WIDGET_CONTROL, 0x0);
> > >       if (hp2_pin_sense)
> > >               snd_hda_codec_write(codec, 0x16, 0,
> > >                           AC_VERB_SET_PIN_WIDGET_CONTROL, 0x0);
> > >
> > >       if (hp1_pin_sense || hp2_pin_sense)
> > >               msleep(100);
> > >
> > >       alc_auto_setup_eapd(codec, false);
> > >       snd_hda_shutup_pins(codec);
> > >         ************** depop procudure *************************************
> > > }
> >
> > Well, it's not clear to me whether you'd like to reduce the depop
> > procedure above or to add it somewhere else.  Could you clarify?
> >
> >
> > thanks,
> >
> > Takashi
> >
> >
> > >
> > >
> > >
> > >
> > >
> > > ________________________________________
> > > 從: Takashi Iwai [tiwai@suse.de]
> > > 寄件日期: 2019年5月13日 下午 05:42
> > > 至: Kailang
> > > 副本:  (alsa-devel@alsa-project.org)
> > > 主旨: Re: Boot procudure on HDA driver
> > >
> > > On Mon, 13 May 2019 11:30:56 +0200,
> > > Kailang wrote:
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Takashi Iwai <tiwai@suse.de>
> > > > > Sent: Monday, May 13, 2019 5:11 PM
> > > > > To: Kailang <kailang@realtek.com>
> > > > > Cc: (alsa-devel@alsa-project.org) <alsa-devel@alsa-project.org>
> > > > > Subject: Re: Boot procudure on HDA driver
> > > > >
> > > > > On Mon, 13 May 2019 11:00:18 +0200,
> > > > > Kailang wrote:
> > > > > >
> > > > > > Hi Takashi,
> > > > > >
> > > > > > When System Boot up.
> > > > > > The Hda Driver running step was as below.
> > > > > >
> > > > > > alc_init();
> > > > > > alc_shutup();
> > > > > > alc_init();
> > > > > >
> > > > > > The depop procedure was put in spec->init_hook and spec->shutup.
> > > > > >
> > > > > > But I find more codec which run spec->shutup at boot up. It will occur pop
> > > > > noise.
> > > > > > If it doesn't run spec->shutup, it will not occur pop noise or reduce pop noise.
> > > > > >
> > > > > > How could the spec->shutup not run at boot up?
> > > > > >
> > > > > > I ever test to put the check in spec->shutup() for PM_EVENT_SUSPEND and
> > > > > PM_EVENT_HIBERNATE and shutdown.
> > > > > > But if power_save=1, it was have issue for this.
> > > > > > Codec was idle already in power_save=1 state. If system go suspend, it will
> > > > > not run spec->shutup() again.
> > > > >
> > > > > I guess it's a runtime PM, hence it's neither PM_EVENT_SUSPEND nor
> > > > > PM_EVENT_HIBERNATE.
> > > > >
> > > > runtime PM and suspend and hibernate and shutdown need to run
> > > > spec->shutup().
> > >
> > > Is the call really mandatory?
> > >
> > > > It's no problem.
> > > > But spec->shutup() doesn't need to run in boot up.
> > >
> > > The call of spec->shutup() at boot up *is* the runtime PM.
> > > Or any other call path I overlooked?
> > >
> > > > And it will set power_save=1 on all dell machine.
> > > >
> > > > If (codec->auto_runtime_pm || codec->bus->shutdown ||
> > > >       codec->core.dev.power.power_state.event == PM_EVENT_SUSPEND ||
> > > >       codec->core.dev.power.power_state.event == PM_EVENT_HIBERNATE)
> > > >
> > > > So, I need to put upper check code in spec->shutup(). Right?
> > > > Thanks.
> > >
> > > No, the auto_runtime_pm is for a completely different purpose.
> > >
> > > At the boot up, the runtime PM can be kicked in at any time.  So if
> > > you disable the shutup during the runtime PM, it means you'd need to
> > > call the shutup at runtime PM completely.
> > >
> > >
> > > Takashi
> > >
> > > >
> > > > > Actually, if the shutup procedure makes the problem on a certain platform, just
> > > > > skip it.  It's an optional behavior and would be fine without it (of course only if
> > > > > it's confirmed to work :)
> > > > >
> > > > >
> > > > > Takashi
> > > > >
> > > > > ------Please consider the environment before printing this e-mail.
> > > >
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel