On Sat, Dec 26, 2020 at 3:46 PM Takashi Iwai <tiwai@xxxxxxx> wrote:
>
> On Fri, 25 Dec 2020 17:47:23 +0100,
> Kai-Heng Feng wrote:
> >
> > Laptop with codec controlled LEDs 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 codec suspended during the system suspend. This
> > doesn't play well with codec controlled mute and micmute LEDs, because
> > LED core will use codec registers to turn off those LEDs.
> >
> > Currently, all users of create_mute_led_cdev() use codec to control
> > LED, so unconditionally runtime resume those codecs before system
> > suspend to solve the problem.
> >
> > Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete optimization")
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
>
> A puzzling point is that it's applied only to the cases with the led
> cdev. Or does it basically apply for the ASoC controller?
> That is, if you run with legacy HDA (snd-intel-dspcfg.dsp_driver=1),
> does the issue appear as well on those machines?
No, the issue doesn't happen with snd-intel-dspcfg.dsp_driver=1. It
only happens when SOF driver is in use.
My analysis was wrong, I will send a new patch to address the root cause.
Kai-Heng
>
>
> thanks,
>
> Takashi
>
> > ---
> > include/sound/hda_codec.h | 1 +
> > sound/pci/hda/hda_codec.c | 7 +++++++
> > sound/pci/hda/hda_generic.c | 1 +
> > 3 files changed, 9 insertions(+)
> >
> > diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h
> > index 2e8d51937acd..b01d76abe008 100644
> > --- a/include/sound/hda_codec.h
> > +++ b/include/sound/hda_codec.h
> > @@ -255,6 +255,7 @@ struct hda_codec {
> > unsigned int relaxed_resume:1; /* don't resume forcibly for jack */
> > unsigned int forced_resume:1; /* forced resume for jack */
> > unsigned int mst_no_extra_pcms:1; /* no backup PCMs for DP-MST */
> > + unsigned int resume_for_sleep:1; /* runtime resume for system sleep */
> >
> > #ifdef CONFIG_PM
> > unsigned long power_on_acct;
> > diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> > index 687216e74526..b890d9b4339e 100644
> > --- a/sound/pci/hda/hda_codec.c
> > +++ b/sound/pci/hda/hda_codec.c
> > @@ -2983,6 +2983,13 @@ static int hda_codec_runtime_resume(struct device *dev)
> > #ifdef CONFIG_PM_SLEEP
> > static int hda_codec_pm_prepare(struct device *dev)
> > {
> > + struct hda_codec *codec = dev_to_hda_codec(dev);
> > +
> > + if (codec->resume_for_sleep) {
> > + pm_runtime_resume(dev);
> > + return 0;
> > + }
> > +
> > return pm_runtime_suspended(dev);
> > }
> >
> > diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c
> > index 8060cc86dfea..6d259d5bb5bb 100644
> > --- a/sound/pci/hda/hda_generic.c
> > +++ b/sound/pci/hda/hda_generic.c
> > @@ -3913,6 +3913,7 @@ static int create_mute_led_cdev(struct hda_codec *codec,
> > cdev->brightness_set_blocking = callback;
> > cdev->brightness = ledtrig_audio_get(micmute ? LED_AUDIO_MICMUTE : LED_AUDIO_MUTE);
> > cdev->flags = LED_CORE_SUSPENDRESUME;
> > + codec->resume_for_sleep = 1;
> >
> > return devm_led_classdev_register(&codec->core.dev, cdev);
> > }
> > --
> > 2.29.2
> >
