Instead of queueing jackpoll_work, runtime resume the codec to let it use different jack detection methods based on jackpoll_interval.

This partially matches SOF driver's behavior with commit a6e7d0a4bdb0 ("ALSA: hda: fix jack detection with Realtek codecs when in D3"), the difference is SOF unconditionally resumes the codec. --- v3: Remove wrong assumption that only Realtek codec is used by SOF. v2: No change.

sound/soc/sof/intel/hda-codec.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c index 6875fa570c2c..df59c79cfdfc 100644 --- a/sound/soc/sof/intel/hda-codec.c +++ b/sound/soc/sof/intel/hda-codec.c @@ -93,8 +93,7 @@ void hda_codec_jack_check(struct snd_sof_dev *sdev) * has been recorded in STATESTS */ if (codec->jacktbl.used) - schedule_delayed_work(&codec->jackpoll_work, - codec->jackpoll_interval); + pm_request_resume(&codec->core.dev); } #else void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev) {}

When runtime resume is for system suspend, hda_codec_jack_check() schedules jackpoll_work which uses snd_hdac_is_power_on() to check whether codec is suspended.

If we were to use snd_hdac_is_power_on() in system PM path, pm_runtime_status_suspended() should be used instead of pm_runtime_suspended(), otherwise pm_runtime_{enable,disable}() changes the result of snd_hdac_is_power_on().

Because devices suspend in reverse order (i.e. child first), it doesn't make much sense to resume already suspended codec from audio controller.

So instead of using pm_runtime_status_suspended(), the better approach here is to make sure jackpoll isn't used in system PM process.

Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete optimization") --- v3: Clarify the root cause and why it's needed. v2: No change.

sound/soc/sof/intel/hda-dsp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c index 7d00107cf3b2..1c5e05b88a90 100644 --- a/sound/soc/sof/intel/hda-dsp.c +++ b/sound/soc/sof/intel/hda-dsp.c @@ -685,7 +685,8 @@ static int hda_resume(struct snd_sof_dev *sdev, bool runtime_resume) /* check jack status */ if (runtime_resume) { hda_codec_jack_wake_enable(sdev, false); - hda_codec_jack_check(sdev); + if (sdev->system_suspend_target == SOF_SUSPEND_NONE) + hda_codec_jack_check(sdev); }

/* turn off the links that were off before suspend */

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