From: Takashi Iwai <tiwai@suse.de>
To: Kai-Heng Feng <kai.heng.feng@canonical.com>
Cc: tiwai@suse.com, pierre-louis.bossart@linux.intel.com,
lgirdwood@gmail.com, ranjani.sridharan@linux.intel.com,
kai.vehmanen@linux.intel.com, daniel.baluta@nxp.com,
broonie@kernel.org, Jaroslav Kysela <perex@perex.cz>,
Kailang Yang <kailang@realtek.com>,
Jian-Hong Pan <jhp@endlessos.org>,
Hui Wang <hui.wang@canonical.com>,
Huacai Chen <chenhuacai@kernel.org>,
Thomas Hebb <tommyhebb@gmail.com>,
alsa-devel@alsa-project.org (moderated list:SOUND),
linux-kernel@vger.kernel.org (open list)
Subject: Re: [PATCH v3 1/4] ALSA: hda/realtek: Power up codec when setting LED via COEF and GPIO
Date: Tue, 12 Jan 2021 14:16:56 +0100 [thread overview]
Message-ID: <s5h1reqs3av.wl-tiwai@suse.de> (raw)
In-Reply-To: <20210112130704.1201406-1-kai.heng.feng@canonical.com>
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
>
next prev parent reply other threads:[~2021-01-12 13:18 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-12 13:06 [PATCH v3 1/4] ALSA: hda/realtek: Power up codec when setting LED via COEF and GPIO Kai-Heng Feng
2021-01-12 13:07 ` [PATCH v3 2/4] ASoC: SOF: Intel: hda: Resume codec to do jack detection Kai-Heng Feng
2021-01-12 13:07 ` [PATCH v3 3/4] ASoC: SOF: Intel: hda: Modify existing helper to disable WAKEEN Kai-Heng Feng
2021-01-12 13:07 ` [PATCH v3 4/4] ASoC: SOF: Intel: hda: Avoid checking jack on system suspend Kai-Heng Feng
2021-01-12 17:42 ` Mark Brown
2021-01-12 13:16 ` Takashi Iwai [this message]
2021-01-12 17:44 ` [PATCH v3 1/4] ALSA: hda/realtek: Power up codec when setting LED via COEF and GPIO Kai-Heng Feng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=s5h1reqs3av.wl-tiwai@suse.de \
--to=tiwai@suse.de \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=chenhuacai@kernel.org \
--cc=daniel.baluta@nxp.com \
--cc=hui.wang@canonical.com \
--cc=jhp@endlessos.org \
--cc=kai.heng.feng@canonical.com \
--cc=kai.vehmanen@linux.intel.com \
--cc=kailang@realtek.com \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=perex@perex.cz \
--cc=pierre-louis.bossart@linux.intel.com \
--cc=ranjani.sridharan@linux.intel.com \
--cc=tiwai@suse.com \
--cc=tommyhebb@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).