On Tue, 12 Jan 2021 14:06:59 +0100,
Kai-Heng Feng wrote:
>
> System takes a very long time to suspend after commit 215a22ed31a1
> ("ALSA: hda: Refactor codec PM to use direct-complete optimization"):
> [ 90.065964] PM: suspend entry (s2idle)
> [ 90.067337] Filesystems sync: 0.001 seconds
> [ 90.185758] Freezing user space processes ... (elapsed 0.002 seconds) done.
> [ 90.188713] OOM killer disabled.
> [ 90.188714] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
> [ 90.190024] printk: Suspending console(s) (use no_console_suspend to debug)
> [ 90.904912] intel_pch_thermal 0000:00:12.0: CPU-PCH is cool [49C], continue to suspend
> [ 321.262505] snd_hda_codec_realtek ehdaudio0D0: Unable to sync register 0x2b8000. -5
> [ 328.426919] snd_hda_codec_realtek ehdaudio0D0: Unable to sync register 0x2b8000. -5
> [ 329.490933] ACPI: EC: interrupt blocked
>
> That commit keeps the codec suspended during the system suspend. However,
> led_suspend() for mute and micmute led writes codec register, triggers
> a pending wake up. The wakeup is then handled in __device_suspend() by
> the following:
> - pm_runtime_disable() to handle the wakeup event.
> - device is no longer is suspended state, direct-complete isn't taken.
> - pm_runtime_enable() to balance disable_depth.
>
> if (dev->power.direct_complete) {
> if (pm_runtime_status_suspended(dev)) {
> pm_runtime_disable(dev);
> if (pm_runtime_status_suspended(dev)) {
> pm_dev_dbg(dev, state, "direct-complete ");
> goto Complete;
> }
>
> pm_runtime_enable(dev);
> }
> dev->power.direct_complete = false;
> }
>
> Since direct-complete doens't apply anymore, the codec's system suspend
> routine is used.
>
> This doesn't play well with SOF driver. When its runtime resume is
> called for system suspend, hda_codec_jack_check() schedules
> jackpoll_work which uses snd_hdac_is_power_on() to check whether codec
> is suspended. Because of the previous pm_runtime_enable(),
> snd_hdac_is_power_on() returns false and jackpoll continues to run, and
> snd_hda_power_up_pm() cannot power up an already suspended codec in
> multiple attempts, causes the long delay on system suspend.
>
> When direct-complete path is taken, snd_hdac_is_power_on() returns true
> and hda_jackpoll_work() is skipped by accident. This is still not
> correct, and it will be addressed by later patch.
>
> Explicitly runtime resume codec on setting LED to solve the issue.
>
> Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete optimization")
Hmm, I really don't get this.
alc_update_gpio_data() calls snd_hda_codec_write() in the end, and
this function goes over bus->exec_verb call. And for the legacy HDA
codec, it's codec_exec_verb in hda_codec.c, which has already
snd_hda_power_up_pm().
What's the missing piece?
thanks,
Takashi
> ---
> v3:
> New patch.
>
> sound/pci/hda/patch_realtek.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index 3c1d2a3fb1a4..304a7bc89fcd 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -4164,7 +4164,10 @@ static void alc_update_gpio_led(struct hda_codec *codec, unsigned int mask,
> {
> if (polarity)
> enabled = !enabled;
> + /* temporarily power up/down for setting GPIO */
> + snd_hda_power_up_pm(codec);
> alc_update_gpio_data(codec, mask, !enabled); /* muted -> LED on */
> + snd_hda_power_down_pm(codec);
> }
>
> /* turn on/off mute LED via GPIO per vmaster hook */
> @@ -4287,8 +4290,10 @@ static void alc_update_coef_led(struct hda_codec *codec,
> if (polarity)
> on = !on;
> /* temporarily power up/down for setting COEF bit */
> + snd_hda_power_up_pm(codec);
> alc_update_coef_idx(codec, led->idx, led->mask,
> on ? led->on : led->off);
> + snd_hda_power_down_pm(codec);
> }
>
> /* update mute-LED according to the speaker mute state via COEF bit */
> --
> 2.29.2
>
