All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kai-Heng Feng <kai.heng.feng@canonical.com>
To: 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
Cc: broonie@kernel.org, Kai-Heng Feng <kai.heng.feng@canonical.com>,
	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: [PATCH v3 1/4] ALSA: hda/realtek: Power up codec when setting LED via COEF and GPIO
Date: Tue, 12 Jan 2021 21:06:59 +0800	[thread overview]
Message-ID: <20210112130704.1201406-1-kai.heng.feng@canonical.com> (raw)

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")
---
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


WARNING: multiple messages have this Message-ID (diff)
From: Kai-Heng Feng <kai.heng.feng@canonical.com>
To: 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
Cc: "moderated list:SOUND" <alsa-devel@alsa-project.org>,
	Kailang Yang <kailang@realtek.com>,
	Thomas Hebb <tommyhebb@gmail.com>,
	open list <linux-kernel@vger.kernel.org>,
	broonie@kernel.org, Huacai Chen <chenhuacai@kernel.org>,
	Jian-Hong Pan <jhp@endlessos.org>,
	Hui Wang <hui.wang@canonical.com>,
	Kai-Heng Feng <kai.heng.feng@canonical.com>
Subject: [PATCH v3 1/4] ALSA: hda/realtek: Power up codec when setting LED via COEF and GPIO
Date: Tue, 12 Jan 2021 21:06:59 +0800	[thread overview]
Message-ID: <20210112130704.1201406-1-kai.heng.feng@canonical.com> (raw)

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")
---
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


             reply	other threads:[~2021-01-12 13:08 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-12 13:06 Kai-Heng Feng [this message]
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   ` 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   ` 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 13:07   ` Kai-Heng Feng
2021-01-12 17:42   ` Mark Brown
2021-01-12 17:42     ` Mark Brown
2021-01-12 13:16 ` [PATCH v3 1/4] ALSA: hda/realtek: Power up codec when setting LED via COEF and GPIO Takashi Iwai
2021-01-12 13:16   ` Takashi Iwai
2021-01-12 17:44   ` Kai-Heng Feng
2021-01-12 17:44     ` 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=20210112130704.1201406-1-kai.heng.feng@canonical.com \
    --to=kai.heng.feng@canonical.com \
    --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.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.