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@xxxxxxx]
> 寄件日期: 2019年5月13日 下午 08:01
> 至: Kailang
> 副本: (alsa-devel@xxxxxxxxxxxxxxxx)
> 主旨: 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@xxxxxxx]
> > 寄件日期: 2019年5月13日 下午 07:30
> > 至: Kailang
> > 副本: (alsa-devel@xxxxxxxxxxxxxxxx)
> > 主旨: 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@xxxxxxx]
> > > 寄件日期: 2019年5月13日 下午 05:42
> > > 至: Kailang
> > > 副本: (alsa-devel@xxxxxxxxxxxxxxxx)
> > > 主旨: Re: Boot procudure on HDA driver
> > >
> > > On Mon, 13 May 2019 11:30:56 +0200,
> > > Kailang wrote:
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Takashi Iwai <tiwai@xxxxxxx>
> > > > > Sent: Monday, May 13, 2019 5:11 PM
> > > > > To: Kailang <kailang@xxxxxxxxxxx>
> > > > > Cc: (alsa-devel@xxxxxxxxxxxxxxxx) <alsa-devel@xxxxxxxxxxxxxxxx>
> > > > > 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@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
