[alsa-devel] 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 at suse.de]
> 寄件日期: 2019年5月13日 下午 07:30
> 至: Kailang
> 副本:  (alsa-devel at 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 at suse.de]
> > 寄件日期: 2019年5月13日 下午 05:42
> > 至: Kailang
> > 副本:  (alsa-devel at 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 at suse.de>
> > > > Sent: Monday, May 13, 2019 5:11 PM
> > > > To: Kailang <kailang at realtek.com>
> > > > Cc: (alsa-devel at alsa-project.org) <alsa-devel at 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.
> > >