* [PATCH 1/2] ASoC: SOF: Intel: hda: Modify existing helper to disable WAKEEN
@ 2020-12-29 13:38 ` Kai-Heng Feng
0 siblings, 0 replies; 16+ messages in thread
From: Kai-Heng Feng @ 2020-12-29 13:38 UTC (permalink / raw)
To: pierre-louis.bossart, lgirdwood, ranjani.sridharan, kai.vehmanen,
daniel.baluta
Cc: Kai-Heng Feng, Guennadi Liakhovetski, Cezary Rojewski,
Marcin Rajwa, Kuninori Morimoto, open list, Keyon Jie,
Takashi Iwai, Rander Wang, Fred Oh,
moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
Mark Brown, Payal Kshirsagar, Amery Song, Bard Liao,
moderated list:SOUND - SOUND OPEN FIRMWARE SOF DRIVERS
Modify hda_codec_jack_wake_enable() to also support disable WAKEEN.
This is a preparation for next patch.
No functional change intended.
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
sound/soc/sof/intel/hda-codec.c | 16 +++++++---------
sound/soc/sof/intel/hda-dsp.c | 6 ++++--
sound/soc/sof/intel/hda.h | 2 +-
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c
index 6875fa570c2c..bc9ac4abdab5 100644
--- a/sound/soc/sof/intel/hda-codec.c
+++ b/sound/soc/sof/intel/hda-codec.c
@@ -63,16 +63,18 @@ static int hda_codec_load_module(struct hda_codec *codec)
}
/* enable controller wake up event for all codecs with jack connectors */
-void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev)
+void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable)
{
struct hda_bus *hbus = sof_to_hbus(sdev);
struct hdac_bus *bus = sof_to_bus(sdev);
struct hda_codec *codec;
unsigned int mask = 0;
- list_for_each_codec(codec, hbus)
- if (codec->jacktbl.used)
- mask |= BIT(codec->core.addr);
+ if (enable) {
+ list_for_each_codec(codec, hbus)
+ if (codec->jacktbl.used)
+ mask |= BIT(codec->core.addr);
+ }
snd_hdac_chip_updatew(bus, WAKEEN, STATESTS_INT_MASK, mask);
}
@@ -81,12 +83,8 @@ void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev)
void hda_codec_jack_check(struct snd_sof_dev *sdev)
{
struct hda_bus *hbus = sof_to_hbus(sdev);
- struct hdac_bus *bus = sof_to_bus(sdev);
struct hda_codec *codec;
- /* disable controller Wake Up event*/
- snd_hdac_chip_updatew(bus, WAKEEN, STATESTS_INT_MASK, 0);
-
list_for_each_codec(codec, hbus)
/*
* Wake up all jack-detecting codecs regardless whether an event
@@ -97,7 +95,7 @@ void hda_codec_jack_check(struct snd_sof_dev *sdev)
codec->jackpoll_interval);
}
#else
-void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev) {}
+void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable) {}
void hda_codec_jack_check(struct snd_sof_dev *sdev) {}
#endif /* CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC */
EXPORT_SYMBOL_NS(hda_codec_jack_wake_enable, SND_SOC_SOF_HDA_AUDIO_CODEC);
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
index 2b001151fe37..7d00107cf3b2 100644
--- a/sound/soc/sof/intel/hda-dsp.c
+++ b/sound/soc/sof/intel/hda-dsp.c
@@ -617,7 +617,7 @@ static int hda_suspend(struct snd_sof_dev *sdev, bool runtime_suspend)
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
if (runtime_suspend)
- hda_codec_jack_wake_enable(sdev);
+ hda_codec_jack_wake_enable(sdev, true);
/* power down all hda link */
snd_hdac_ext_bus_link_power_down_all(bus);
@@ -683,8 +683,10 @@ static int hda_resume(struct snd_sof_dev *sdev, bool runtime_resume)
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
/* check jack status */
- if (runtime_resume)
+ if (runtime_resume) {
+ hda_codec_jack_wake_enable(sdev, false);
hda_codec_jack_check(sdev);
+ }
/* turn off the links that were off before suspend */
list_for_each_entry(hlink, &bus->hlink_list, list) {
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 9ec8ae0fd649..a3b6f3e9121c 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -650,7 +650,7 @@ void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev);
*/
void hda_codec_probe_bus(struct snd_sof_dev *sdev,
bool hda_codec_use_common_hdmi);
-void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev);
+void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable);
void hda_codec_jack_check(struct snd_sof_dev *sdev);
#endif /* CONFIG_SND_SOC_SOF_HDA */
--
2.29.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 2/2] ASoC: SOF: Intel: hda: Avoid checking jack on system suspend
2020-12-29 13:38 ` Kai-Heng Feng
@ 2020-12-29 13:38 ` Kai-Heng Feng
-1 siblings, 0 replies; 16+ messages in thread
From: Kai-Heng Feng @ 2020-12-29 13:38 UTC (permalink / raw)
To: pierre-louis.bossart, lgirdwood, ranjani.sridharan, kai.vehmanen,
daniel.baluta
Cc: Kai-Heng Feng, Mark Brown, Jaroslav Kysela, Takashi Iwai,
Keyon Jie, Kuninori Morimoto, Marcin Rajwa, Payal Kshirsagar,
moderated list:SOUND - SOUND OPEN FIRMWARE (SOF) DRIVERS,
moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
open list
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 codec suspended during the system suspend. However,
SOF driver's runtime resume, which is for system suspend, calls
hda_codec_jack_check() and schedules jackpoll_work. The jackpoll
work uses snd_hda_power_up_pm() which tries to resume the codec in
system suspend path, hence blocking the suspend process.
So only check jack status if it's not in system PM process.
Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete optimization")
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
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 */
--
2.29.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 2/2] ASoC: SOF: Intel: hda: Avoid checking jack on system suspend
@ 2020-12-29 13:38 ` Kai-Heng Feng
0 siblings, 0 replies; 16+ messages in thread
From: Kai-Heng Feng @ 2020-12-29 13:38 UTC (permalink / raw)
To: pierre-louis.bossart, lgirdwood, ranjani.sridharan, kai.vehmanen,
daniel.baluta
Cc: Kai-Heng Feng,
moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
Marcin Rajwa, Kuninori Morimoto, open list, Keyon Jie,
Takashi Iwai, Mark Brown, Payal Kshirsagar,
moderated list:SOUND - SOUND OPEN FIRMWARE SOF DRIVERS
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 codec suspended during the system suspend. However,
SOF driver's runtime resume, which is for system suspend, calls
hda_codec_jack_check() and schedules jackpoll_work. The jackpoll
work uses snd_hda_power_up_pm() which tries to resume the codec in
system suspend path, hence blocking the suspend process.
So only check jack status if it's not in system PM process.
Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete optimization")
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
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 */
--
2.29.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] ASoC: SOF: Intel: hda: Avoid checking jack on system suspend
2020-12-29 13:38 ` Kai-Heng Feng
@ 2020-12-31 10:55 ` Takashi Iwai
-1 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2020-12-31 10:55 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: pierre-louis.bossart, lgirdwood, ranjani.sridharan, kai.vehmanen,
daniel.baluta, Mark Brown, Jaroslav Kysela, Takashi Iwai,
Keyon Jie, Kuninori Morimoto, Marcin Rajwa, Payal Kshirsagar,
moderated list:SOUND - SOUND OPEN FIRMWARE (SOF) DRIVERS,
moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
open list
On Tue, 29 Dec 2020 14:38:15 +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 codec suspended during the system suspend. However,
> SOF driver's runtime resume, which is for system suspend, calls
> hda_codec_jack_check() and schedules jackpoll_work. The jackpoll
> work uses snd_hda_power_up_pm() which tries to resume the codec in
> system suspend path, hence blocking the suspend process.
>
> So only check jack status if it's not in system PM process.
After your previous patch set, the legacy HDA does queue the
jackpoll_work only if jackpoll_interval is set. I suppose rather the
same rule would be applied?
thanks,
Takashi
>
> Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete optimization")
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> 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 */
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] ASoC: SOF: Intel: hda: Avoid checking jack on system suspend
@ 2020-12-31 10:55 ` Takashi Iwai
0 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2020-12-31 10:55 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
Kuninori Morimoto, kai.vehmanen, lgirdwood, open list,
Takashi Iwai, Keyon Jie, ranjani.sridharan, pierre-louis.bossart,
Mark Brown, Payal Kshirsagar, daniel.baluta, Marcin Rajwa,
moderated list:SOUND - SOUND OPEN FIRMWARE SOF DRIVERS
On Tue, 29 Dec 2020 14:38:15 +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 codec suspended during the system suspend. However,
> SOF driver's runtime resume, which is for system suspend, calls
> hda_codec_jack_check() and schedules jackpoll_work. The jackpoll
> work uses snd_hda_power_up_pm() which tries to resume the codec in
> system suspend path, hence blocking the suspend process.
>
> So only check jack status if it's not in system PM process.
After your previous patch set, the legacy HDA does queue the
jackpoll_work only if jackpoll_interval is set. I suppose rather the
same rule would be applied?
thanks,
Takashi
>
> Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete optimization")
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> 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 */
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] ASoC: SOF: Intel: hda: Avoid checking jack on system suspend
2020-12-31 10:55 ` Takashi Iwai
@ 2020-12-31 18:06 ` Kai-Heng Feng
-1 siblings, 0 replies; 16+ messages in thread
From: Kai-Heng Feng @ 2020-12-31 18:06 UTC (permalink / raw)
To: Takashi Iwai
Cc: Pierre-Louis Bossart, Liam Girdwood, Ranjani Sridharan,
Kai Vehmanen, daniel.baluta, Mark Brown, Jaroslav Kysela,
Takashi Iwai, Keyon Jie, Kuninori Morimoto, Marcin Rajwa,
Payal Kshirsagar,
moderated list:SOUND - SOUND OPEN FIRMWARE (SOF) DRIVERS,
moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
open list
On Thu, Dec 31, 2020 at 6:55 PM Takashi Iwai <tiwai@suse.de> wrote:
>
> On Tue, 29 Dec 2020 14:38:15 +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 codec suspended during the system suspend. However,
> > SOF driver's runtime resume, which is for system suspend, calls
> > hda_codec_jack_check() and schedules jackpoll_work. The jackpoll
> > work uses snd_hda_power_up_pm() which tries to resume the codec in
> > system suspend path, hence blocking the suspend process.
> >
> > So only check jack status if it's not in system PM process.
>
> After your previous patch set, the legacy HDA does queue the
> jackpoll_work only if jackpoll_interval is set. I suppose rather the
> same rule would be applied?
It's queued in hda_codec_pm_complete(), which happens at the end of PM process.
This one is queued in the middle of PM suspend, so it's not the same here.
Kai-Heng
>
>
> thanks,
>
> Takashi
>
> >
> > Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete optimization")
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > ---
> > 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 */
> > --
> > 2.29.2
> >
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] ASoC: SOF: Intel: hda: Avoid checking jack on system suspend
@ 2020-12-31 18:06 ` Kai-Heng Feng
0 siblings, 0 replies; 16+ messages in thread
From: Kai-Heng Feng @ 2020-12-31 18:06 UTC (permalink / raw)
To: Takashi Iwai
Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
Kuninori Morimoto, Kai Vehmanen, Liam Girdwood, open list,
Takashi Iwai, Keyon Jie, Ranjani Sridharan, Pierre-Louis Bossart,
Mark Brown, Payal Kshirsagar, daniel.baluta, Marcin Rajwa,
moderated list:SOUND - SOUND OPEN FIRMWARE (SOF) DRIVERS
On Thu, Dec 31, 2020 at 6:55 PM Takashi Iwai <tiwai@suse.de> wrote:
>
> On Tue, 29 Dec 2020 14:38:15 +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 codec suspended during the system suspend. However,
> > SOF driver's runtime resume, which is for system suspend, calls
> > hda_codec_jack_check() and schedules jackpoll_work. The jackpoll
> > work uses snd_hda_power_up_pm() which tries to resume the codec in
> > system suspend path, hence blocking the suspend process.
> >
> > So only check jack status if it's not in system PM process.
>
> After your previous patch set, the legacy HDA does queue the
> jackpoll_work only if jackpoll_interval is set. I suppose rather the
> same rule would be applied?
It's queued in hda_codec_pm_complete(), which happens at the end of PM process.
This one is queued in the middle of PM suspend, so it's not the same here.
Kai-Heng
>
>
> thanks,
>
> Takashi
>
> >
> > Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete optimization")
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > ---
> > 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 */
> > --
> > 2.29.2
> >
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] ASoC: SOF: Intel: hda: Avoid checking jack on system suspend
2020-12-31 18:06 ` Kai-Heng Feng
@ 2021-01-01 8:07 ` Takashi Iwai
-1 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2021-01-01 8:07 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: Pierre-Louis Bossart, Liam Girdwood, Ranjani Sridharan,
Kai Vehmanen, daniel.baluta, Mark Brown, Jaroslav Kysela,
Takashi Iwai, Keyon Jie, Kuninori Morimoto, Marcin Rajwa,
Payal Kshirsagar,
moderated list:SOUND - SOUND OPEN FIRMWARE (SOF) DRIVERS,
moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
open list
On Thu, 31 Dec 2020 19:06:43 +0100,
Kai-Heng Feng wrote:
>
> On Thu, Dec 31, 2020 at 6:55 PM Takashi Iwai <tiwai@suse.de> wrote:
> >
> > On Tue, 29 Dec 2020 14:38:15 +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 codec suspended during the system suspend. However,
> > > SOF driver's runtime resume, which is for system suspend, calls
> > > hda_codec_jack_check() and schedules jackpoll_work. The jackpoll
> > > work uses snd_hda_power_up_pm() which tries to resume the codec in
> > > system suspend path, hence blocking the suspend process.
> > >
> > > So only check jack status if it's not in system PM process.
> >
> > After your previous patch set, the legacy HDA does queue the
> > jackpoll_work only if jackpoll_interval is set. I suppose rather the
> > same rule would be applied?
>
> It's queued in hda_codec_pm_complete(), which happens at the end of PM process.
> This one is queued in the middle of PM suspend, so it's not the same here.
But why do we need the jack status check explicitly there if
hda_codec_pm_complete() already does it (via re-queuing the resume)?
thanks,
Takashi
>
> Kai-Heng
>
> >
> >
> > thanks,
> >
> > Takashi
> >
> > >
> > > Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete optimization")
> > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > > ---
> > > 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 */
> > > --
> > > 2.29.2
> > >
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] ASoC: SOF: Intel: hda: Avoid checking jack on system suspend
@ 2021-01-01 8:07 ` Takashi Iwai
0 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2021-01-01 8:07 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
Kuninori Morimoto, Kai Vehmanen, Liam Girdwood, open list,
Takashi Iwai, Keyon Jie, Ranjani Sridharan, Pierre-Louis Bossart,
Mark Brown, Payal Kshirsagar, daniel.baluta, Marcin Rajwa,
moderated list:SOUND - SOUND OPEN FIRMWARE (SOF) DRIVERS
On Thu, 31 Dec 2020 19:06:43 +0100,
Kai-Heng Feng wrote:
>
> On Thu, Dec 31, 2020 at 6:55 PM Takashi Iwai <tiwai@suse.de> wrote:
> >
> > On Tue, 29 Dec 2020 14:38:15 +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 codec suspended during the system suspend. However,
> > > SOF driver's runtime resume, which is for system suspend, calls
> > > hda_codec_jack_check() and schedules jackpoll_work. The jackpoll
> > > work uses snd_hda_power_up_pm() which tries to resume the codec in
> > > system suspend path, hence blocking the suspend process.
> > >
> > > So only check jack status if it's not in system PM process.
> >
> > After your previous patch set, the legacy HDA does queue the
> > jackpoll_work only if jackpoll_interval is set. I suppose rather the
> > same rule would be applied?
>
> It's queued in hda_codec_pm_complete(), which happens at the end of PM process.
> This one is queued in the middle of PM suspend, so it's not the same here.
But why do we need the jack status check explicitly there if
hda_codec_pm_complete() already does it (via re-queuing the resume)?
thanks,
Takashi
>
> Kai-Heng
>
> >
> >
> > thanks,
> >
> > Takashi
> >
> > >
> > > Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete optimization")
> > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > > ---
> > > 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 */
> > > --
> > > 2.29.2
> > >
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] ASoC: SOF: Intel: hda: Avoid checking jack on system suspend
2021-01-01 8:07 ` Takashi Iwai
@ 2021-01-04 8:15 ` Kai-Heng Feng
-1 siblings, 0 replies; 16+ messages in thread
From: Kai-Heng Feng @ 2021-01-04 8:15 UTC (permalink / raw)
To: Takashi Iwai
Cc: Pierre-Louis Bossart, Liam Girdwood, Ranjani Sridharan,
Kai Vehmanen, daniel.baluta, Mark Brown, Jaroslav Kysela,
Takashi Iwai, Keyon Jie, Kuninori Morimoto, Marcin Rajwa,
Payal Kshirsagar,
moderated list:SOUND - SOUND OPEN FIRMWARE (SOF) DRIVERS,
moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
open list
On Fri, Jan 1, 2021 at 4:07 PM Takashi Iwai <tiwai@suse.de> wrote:
>
> On Thu, 31 Dec 2020 19:06:43 +0100,
> Kai-Heng Feng wrote:
> >
> > On Thu, Dec 31, 2020 at 6:55 PM Takashi Iwai <tiwai@suse.de> wrote:
> > >
> > > On Tue, 29 Dec 2020 14:38:15 +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 codec suspended during the system suspend. However,
> > > > SOF driver's runtime resume, which is for system suspend, calls
> > > > hda_codec_jack_check() and schedules jackpoll_work. The jackpoll
> > > > work uses snd_hda_power_up_pm() which tries to resume the codec in
> > > > system suspend path, hence blocking the suspend process.
> > > >
> > > > So only check jack status if it's not in system PM process.
> > >
> > > After your previous patch set, the legacy HDA does queue the
> > > jackpoll_work only if jackpoll_interval is set. I suppose rather the
> > > same rule would be applied?
> >
> > It's queued in hda_codec_pm_complete(), which happens at the end of PM process.
> > This one is queued in the middle of PM suspend, so it's not the same here.
>
> But why do we need the jack status check explicitly there if
> hda_codec_pm_complete() already does it (via re-queuing the resume)?
Because hda_resume() is called for both system and runtime resume, but
hda_codec_pm_complete() only gets called for system resume. So we
still need to cover the runtime resume case.
However, we can use pm_request_resume() to wakeup the codec and do the
jack detection, instead of queueing jackpoll_work directly. I'll do
the change in v2.
Kai-Heng
>
> thanks,
>
> Takashi
>
> >
> > Kai-Heng
> >
> > >
> > >
> > > thanks,
> > >
> > > Takashi
> > >
> > > >
> > > > Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete optimization")
> > > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > > > ---
> > > > 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 */
> > > > --
> > > > 2.29.2
> > > >
> >
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/2] ASoC: SOF: Intel: hda: Avoid checking jack on system suspend
@ 2021-01-04 8:15 ` Kai-Heng Feng
0 siblings, 0 replies; 16+ messages in thread
From: Kai-Heng Feng @ 2021-01-04 8:15 UTC (permalink / raw)
To: Takashi Iwai
Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
Kuninori Morimoto, Kai Vehmanen, Liam Girdwood, open list,
Takashi Iwai, Keyon Jie, Ranjani Sridharan, Pierre-Louis Bossart,
Mark Brown, Payal Kshirsagar, daniel.baluta, Marcin Rajwa,
moderated list:SOUND - SOUND OPEN FIRMWARE (SOF) DRIVERS
On Fri, Jan 1, 2021 at 4:07 PM Takashi Iwai <tiwai@suse.de> wrote:
>
> On Thu, 31 Dec 2020 19:06:43 +0100,
> Kai-Heng Feng wrote:
> >
> > On Thu, Dec 31, 2020 at 6:55 PM Takashi Iwai <tiwai@suse.de> wrote:
> > >
> > > On Tue, 29 Dec 2020 14:38:15 +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 codec suspended during the system suspend. However,
> > > > SOF driver's runtime resume, which is for system suspend, calls
> > > > hda_codec_jack_check() and schedules jackpoll_work. The jackpoll
> > > > work uses snd_hda_power_up_pm() which tries to resume the codec in
> > > > system suspend path, hence blocking the suspend process.
> > > >
> > > > So only check jack status if it's not in system PM process.
> > >
> > > After your previous patch set, the legacy HDA does queue the
> > > jackpoll_work only if jackpoll_interval is set. I suppose rather the
> > > same rule would be applied?
> >
> > It's queued in hda_codec_pm_complete(), which happens at the end of PM process.
> > This one is queued in the middle of PM suspend, so it's not the same here.
>
> But why do we need the jack status check explicitly there if
> hda_codec_pm_complete() already does it (via re-queuing the resume)?
Because hda_resume() is called for both system and runtime resume, but
hda_codec_pm_complete() only gets called for system resume. So we
still need to cover the runtime resume case.
However, we can use pm_request_resume() to wakeup the codec and do the
jack detection, instead of queueing jackpoll_work directly. I'll do
the change in v2.
Kai-Heng
>
> thanks,
>
> Takashi
>
> >
> > Kai-Heng
> >
> > >
> > >
> > > thanks,
> > >
> > > Takashi
> > >
> > > >
> > > > Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete optimization")
> > > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > > > ---
> > > > 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 */
> > > > --
> > > > 2.29.2
> > > >
> >
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] ASoC: SOF: Intel: hda: Modify existing helper to disable WAKEEN
2020-12-29 13:38 ` Kai-Heng Feng
@ 2020-12-31 10:52 ` Takashi Iwai
-1 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2020-12-31 10:52 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: pierre-louis.bossart, lgirdwood, ranjani.sridharan, kai.vehmanen,
daniel.baluta, Mark Brown, Jaroslav Kysela, Takashi Iwai,
Guennadi Liakhovetski, Rander Wang, Payal Kshirsagar, Keyon Jie,
Kuninori Morimoto, Marcin Rajwa, Cezary Rojewski, Fred Oh,
Bard Liao, Amery Song,
moderated list:SOUND - SOUND OPEN FIRMWARE (SOF) DRIVERS,
moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
open list
On Tue, 29 Dec 2020 14:38:14 +0100,
Kai-Heng Feng wrote:
>
> Modify hda_codec_jack_wake_enable() to also support disable WAKEEN.
> This is a preparation for next patch.
>
> No functional change intended.
Maybe it's better to mention that this patch moves the WAKEEN
disablement call out of hda_codec_jack_check() into
hda_codec_jack_wake_enable(), too.
thanks,
Takashi
>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> sound/soc/sof/intel/hda-codec.c | 16 +++++++---------
> sound/soc/sof/intel/hda-dsp.c | 6 ++++--
> sound/soc/sof/intel/hda.h | 2 +-
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c
> index 6875fa570c2c..bc9ac4abdab5 100644
> --- a/sound/soc/sof/intel/hda-codec.c
> +++ b/sound/soc/sof/intel/hda-codec.c
> @@ -63,16 +63,18 @@ static int hda_codec_load_module(struct hda_codec *codec)
> }
>
> /* enable controller wake up event for all codecs with jack connectors */
> -void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev)
> +void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable)
> {
> struct hda_bus *hbus = sof_to_hbus(sdev);
> struct hdac_bus *bus = sof_to_bus(sdev);
> struct hda_codec *codec;
> unsigned int mask = 0;
>
> - list_for_each_codec(codec, hbus)
> - if (codec->jacktbl.used)
> - mask |= BIT(codec->core.addr);
> + if (enable) {
> + list_for_each_codec(codec, hbus)
> + if (codec->jacktbl.used)
> + mask |= BIT(codec->core.addr);
> + }
>
> snd_hdac_chip_updatew(bus, WAKEEN, STATESTS_INT_MASK, mask);
> }
> @@ -81,12 +83,8 @@ void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev)
> void hda_codec_jack_check(struct snd_sof_dev *sdev)
> {
> struct hda_bus *hbus = sof_to_hbus(sdev);
> - struct hdac_bus *bus = sof_to_bus(sdev);
> struct hda_codec *codec;
>
> - /* disable controller Wake Up event*/
> - snd_hdac_chip_updatew(bus, WAKEEN, STATESTS_INT_MASK, 0);
> -
> list_for_each_codec(codec, hbus)
> /*
> * Wake up all jack-detecting codecs regardless whether an event
> @@ -97,7 +95,7 @@ void hda_codec_jack_check(struct snd_sof_dev *sdev)
> codec->jackpoll_interval);
> }
> #else
> -void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev) {}
> +void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable) {}
> void hda_codec_jack_check(struct snd_sof_dev *sdev) {}
> #endif /* CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC */
> EXPORT_SYMBOL_NS(hda_codec_jack_wake_enable, SND_SOC_SOF_HDA_AUDIO_CODEC);
> diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
> index 2b001151fe37..7d00107cf3b2 100644
> --- a/sound/soc/sof/intel/hda-dsp.c
> +++ b/sound/soc/sof/intel/hda-dsp.c
> @@ -617,7 +617,7 @@ static int hda_suspend(struct snd_sof_dev *sdev, bool runtime_suspend)
>
> #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
> if (runtime_suspend)
> - hda_codec_jack_wake_enable(sdev);
> + hda_codec_jack_wake_enable(sdev, true);
>
> /* power down all hda link */
> snd_hdac_ext_bus_link_power_down_all(bus);
> @@ -683,8 +683,10 @@ static int hda_resume(struct snd_sof_dev *sdev, bool runtime_resume)
>
> #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
> /* check jack status */
> - if (runtime_resume)
> + if (runtime_resume) {
> + hda_codec_jack_wake_enable(sdev, false);
> hda_codec_jack_check(sdev);
> + }
>
> /* turn off the links that were off before suspend */
> list_for_each_entry(hlink, &bus->hlink_list, list) {
> diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
> index 9ec8ae0fd649..a3b6f3e9121c 100644
> --- a/sound/soc/sof/intel/hda.h
> +++ b/sound/soc/sof/intel/hda.h
> @@ -650,7 +650,7 @@ void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev);
> */
> void hda_codec_probe_bus(struct snd_sof_dev *sdev,
> bool hda_codec_use_common_hdmi);
> -void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev);
> +void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable);
> void hda_codec_jack_check(struct snd_sof_dev *sdev);
>
> #endif /* CONFIG_SND_SOC_SOF_HDA */
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/2] ASoC: SOF: Intel: hda: Modify existing helper to disable WAKEEN
@ 2020-12-31 10:52 ` Takashi Iwai
0 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2020-12-31 10:52 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
Guennadi Liakhovetski, Cezary Rojewski, Kuninori Morimoto,
kai.vehmanen, lgirdwood, Rander Wang, Bard Liao, Takashi Iwai,
Keyon Jie, ranjani.sridharan, pierre-louis.bossart, Fred Oh,
Mark Brown, Payal Kshirsagar, Amery Song, daniel.baluta,
Marcin Rajwa, open list,
moderated list:SOUND - SOUND OPEN FIRMWARE SOF DRIVERS
On Tue, 29 Dec 2020 14:38:14 +0100,
Kai-Heng Feng wrote:
>
> Modify hda_codec_jack_wake_enable() to also support disable WAKEEN.
> This is a preparation for next patch.
>
> No functional change intended.
Maybe it's better to mention that this patch moves the WAKEEN
disablement call out of hda_codec_jack_check() into
hda_codec_jack_wake_enable(), too.
thanks,
Takashi
>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> sound/soc/sof/intel/hda-codec.c | 16 +++++++---------
> sound/soc/sof/intel/hda-dsp.c | 6 ++++--
> sound/soc/sof/intel/hda.h | 2 +-
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c
> index 6875fa570c2c..bc9ac4abdab5 100644
> --- a/sound/soc/sof/intel/hda-codec.c
> +++ b/sound/soc/sof/intel/hda-codec.c
> @@ -63,16 +63,18 @@ static int hda_codec_load_module(struct hda_codec *codec)
> }
>
> /* enable controller wake up event for all codecs with jack connectors */
> -void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev)
> +void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable)
> {
> struct hda_bus *hbus = sof_to_hbus(sdev);
> struct hdac_bus *bus = sof_to_bus(sdev);
> struct hda_codec *codec;
> unsigned int mask = 0;
>
> - list_for_each_codec(codec, hbus)
> - if (codec->jacktbl.used)
> - mask |= BIT(codec->core.addr);
> + if (enable) {
> + list_for_each_codec(codec, hbus)
> + if (codec->jacktbl.used)
> + mask |= BIT(codec->core.addr);
> + }
>
> snd_hdac_chip_updatew(bus, WAKEEN, STATESTS_INT_MASK, mask);
> }
> @@ -81,12 +83,8 @@ void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev)
> void hda_codec_jack_check(struct snd_sof_dev *sdev)
> {
> struct hda_bus *hbus = sof_to_hbus(sdev);
> - struct hdac_bus *bus = sof_to_bus(sdev);
> struct hda_codec *codec;
>
> - /* disable controller Wake Up event*/
> - snd_hdac_chip_updatew(bus, WAKEEN, STATESTS_INT_MASK, 0);
> -
> list_for_each_codec(codec, hbus)
> /*
> * Wake up all jack-detecting codecs regardless whether an event
> @@ -97,7 +95,7 @@ void hda_codec_jack_check(struct snd_sof_dev *sdev)
> codec->jackpoll_interval);
> }
> #else
> -void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev) {}
> +void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable) {}
> void hda_codec_jack_check(struct snd_sof_dev *sdev) {}
> #endif /* CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC */
> EXPORT_SYMBOL_NS(hda_codec_jack_wake_enable, SND_SOC_SOF_HDA_AUDIO_CODEC);
> diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
> index 2b001151fe37..7d00107cf3b2 100644
> --- a/sound/soc/sof/intel/hda-dsp.c
> +++ b/sound/soc/sof/intel/hda-dsp.c
> @@ -617,7 +617,7 @@ static int hda_suspend(struct snd_sof_dev *sdev, bool runtime_suspend)
>
> #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
> if (runtime_suspend)
> - hda_codec_jack_wake_enable(sdev);
> + hda_codec_jack_wake_enable(sdev, true);
>
> /* power down all hda link */
> snd_hdac_ext_bus_link_power_down_all(bus);
> @@ -683,8 +683,10 @@ static int hda_resume(struct snd_sof_dev *sdev, bool runtime_resume)
>
> #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
> /* check jack status */
> - if (runtime_resume)
> + if (runtime_resume) {
> + hda_codec_jack_wake_enable(sdev, false);
> hda_codec_jack_check(sdev);
> + }
>
> /* turn off the links that were off before suspend */
> list_for_each_entry(hlink, &bus->hlink_list, list) {
> diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
> index 9ec8ae0fd649..a3b6f3e9121c 100644
> --- a/sound/soc/sof/intel/hda.h
> +++ b/sound/soc/sof/intel/hda.h
> @@ -650,7 +650,7 @@ void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev);
> */
> void hda_codec_probe_bus(struct snd_sof_dev *sdev,
> bool hda_codec_use_common_hdmi);
> -void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev);
> +void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable);
> void hda_codec_jack_check(struct snd_sof_dev *sdev);
>
> #endif /* CONFIG_SND_SOC_SOF_HDA */
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/2] ASoC: SOF: Intel: hda: Modify existing helper to disable WAKEEN
2020-12-31 10:52 ` Takashi Iwai
@ 2020-12-31 18:07 ` Kai-Heng Feng
-1 siblings, 0 replies; 16+ messages in thread
From: Kai-Heng Feng @ 2020-12-31 18:07 UTC (permalink / raw)
To: Takashi Iwai
Cc: Pierre-Louis Bossart, Liam Girdwood, Ranjani Sridharan,
Kai Vehmanen, daniel.baluta, Mark Brown, Jaroslav Kysela,
Takashi Iwai, Guennadi Liakhovetski, Rander Wang,
Payal Kshirsagar, Keyon Jie, Kuninori Morimoto, Marcin Rajwa,
Cezary Rojewski, Fred Oh, Bard Liao, Amery Song,
moderated list:SOUND - SOUND OPEN FIRMWARE (SOF) DRIVERS,
moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
open list
On Thu, Dec 31, 2020 at 6:52 PM Takashi Iwai <tiwai@suse.de> wrote:
>
> On Tue, 29 Dec 2020 14:38:14 +0100,
> Kai-Heng Feng wrote:
> >
> > Modify hda_codec_jack_wake_enable() to also support disable WAKEEN.
> > This is a preparation for next patch.
> >
> > No functional change intended.
>
> Maybe it's better to mention that this patch moves the WAKEEN
> disablement call out of hda_codec_jack_check() into
> hda_codec_jack_wake_enable(), too.
Ok, will update the commit log in v2.
Kai-Heng
>
>
> thanks,
>
> Takashi
>
> >
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > ---
> > sound/soc/sof/intel/hda-codec.c | 16 +++++++---------
> > sound/soc/sof/intel/hda-dsp.c | 6 ++++--
> > sound/soc/sof/intel/hda.h | 2 +-
> > 3 files changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c
> > index 6875fa570c2c..bc9ac4abdab5 100644
> > --- a/sound/soc/sof/intel/hda-codec.c
> > +++ b/sound/soc/sof/intel/hda-codec.c
> > @@ -63,16 +63,18 @@ static int hda_codec_load_module(struct hda_codec *codec)
> > }
> >
> > /* enable controller wake up event for all codecs with jack connectors */
> > -void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev)
> > +void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable)
> > {
> > struct hda_bus *hbus = sof_to_hbus(sdev);
> > struct hdac_bus *bus = sof_to_bus(sdev);
> > struct hda_codec *codec;
> > unsigned int mask = 0;
> >
> > - list_for_each_codec(codec, hbus)
> > - if (codec->jacktbl.used)
> > - mask |= BIT(codec->core.addr);
> > + if (enable) {
> > + list_for_each_codec(codec, hbus)
> > + if (codec->jacktbl.used)
> > + mask |= BIT(codec->core.addr);
> > + }
> >
> > snd_hdac_chip_updatew(bus, WAKEEN, STATESTS_INT_MASK, mask);
> > }
> > @@ -81,12 +83,8 @@ void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev)
> > void hda_codec_jack_check(struct snd_sof_dev *sdev)
> > {
> > struct hda_bus *hbus = sof_to_hbus(sdev);
> > - struct hdac_bus *bus = sof_to_bus(sdev);
> > struct hda_codec *codec;
> >
> > - /* disable controller Wake Up event*/
> > - snd_hdac_chip_updatew(bus, WAKEEN, STATESTS_INT_MASK, 0);
> > -
> > list_for_each_codec(codec, hbus)
> > /*
> > * Wake up all jack-detecting codecs regardless whether an event
> > @@ -97,7 +95,7 @@ void hda_codec_jack_check(struct snd_sof_dev *sdev)
> > codec->jackpoll_interval);
> > }
> > #else
> > -void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev) {}
> > +void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable) {}
> > void hda_codec_jack_check(struct snd_sof_dev *sdev) {}
> > #endif /* CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC */
> > EXPORT_SYMBOL_NS(hda_codec_jack_wake_enable, SND_SOC_SOF_HDA_AUDIO_CODEC);
> > diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
> > index 2b001151fe37..7d00107cf3b2 100644
> > --- a/sound/soc/sof/intel/hda-dsp.c
> > +++ b/sound/soc/sof/intel/hda-dsp.c
> > @@ -617,7 +617,7 @@ static int hda_suspend(struct snd_sof_dev *sdev, bool runtime_suspend)
> >
> > #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
> > if (runtime_suspend)
> > - hda_codec_jack_wake_enable(sdev);
> > + hda_codec_jack_wake_enable(sdev, true);
> >
> > /* power down all hda link */
> > snd_hdac_ext_bus_link_power_down_all(bus);
> > @@ -683,8 +683,10 @@ static int hda_resume(struct snd_sof_dev *sdev, bool runtime_resume)
> >
> > #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
> > /* check jack status */
> > - if (runtime_resume)
> > + if (runtime_resume) {
> > + hda_codec_jack_wake_enable(sdev, false);
> > hda_codec_jack_check(sdev);
> > + }
> >
> > /* turn off the links that were off before suspend */
> > list_for_each_entry(hlink, &bus->hlink_list, list) {
> > diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
> > index 9ec8ae0fd649..a3b6f3e9121c 100644
> > --- a/sound/soc/sof/intel/hda.h
> > +++ b/sound/soc/sof/intel/hda.h
> > @@ -650,7 +650,7 @@ void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev);
> > */
> > void hda_codec_probe_bus(struct snd_sof_dev *sdev,
> > bool hda_codec_use_common_hdmi);
> > -void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev);
> > +void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable);
> > void hda_codec_jack_check(struct snd_sof_dev *sdev);
> >
> > #endif /* CONFIG_SND_SOC_SOF_HDA */
> > --
> > 2.29.2
> >
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/2] ASoC: SOF: Intel: hda: Modify existing helper to disable WAKEEN
@ 2020-12-31 18:07 ` Kai-Heng Feng
0 siblings, 0 replies; 16+ messages in thread
From: Kai-Heng Feng @ 2020-12-31 18:07 UTC (permalink / raw)
To: Takashi Iwai
Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
Guennadi Liakhovetski, Cezary Rojewski, Kuninori Morimoto,
Kai Vehmanen, Liam Girdwood, Rander Wang, Bard Liao,
Takashi Iwai, Keyon Jie, Ranjani Sridharan, Pierre-Louis Bossart,
Fred Oh, Mark Brown, Payal Kshirsagar, Amery Song, daniel.baluta,
Marcin Rajwa, open list,
moderated list:SOUND - SOUND OPEN FIRMWARE (SOF) DRIVERS
On Thu, Dec 31, 2020 at 6:52 PM Takashi Iwai <tiwai@suse.de> wrote:
>
> On Tue, 29 Dec 2020 14:38:14 +0100,
> Kai-Heng Feng wrote:
> >
> > Modify hda_codec_jack_wake_enable() to also support disable WAKEEN.
> > This is a preparation for next patch.
> >
> > No functional change intended.
>
> Maybe it's better to mention that this patch moves the WAKEEN
> disablement call out of hda_codec_jack_check() into
> hda_codec_jack_wake_enable(), too.
Ok, will update the commit log in v2.
Kai-Heng
>
>
> thanks,
>
> Takashi
>
> >
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > ---
> > sound/soc/sof/intel/hda-codec.c | 16 +++++++---------
> > sound/soc/sof/intel/hda-dsp.c | 6 ++++--
> > sound/soc/sof/intel/hda.h | 2 +-
> > 3 files changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c
> > index 6875fa570c2c..bc9ac4abdab5 100644
> > --- a/sound/soc/sof/intel/hda-codec.c
> > +++ b/sound/soc/sof/intel/hda-codec.c
> > @@ -63,16 +63,18 @@ static int hda_codec_load_module(struct hda_codec *codec)
> > }
> >
> > /* enable controller wake up event for all codecs with jack connectors */
> > -void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev)
> > +void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable)
> > {
> > struct hda_bus *hbus = sof_to_hbus(sdev);
> > struct hdac_bus *bus = sof_to_bus(sdev);
> > struct hda_codec *codec;
> > unsigned int mask = 0;
> >
> > - list_for_each_codec(codec, hbus)
> > - if (codec->jacktbl.used)
> > - mask |= BIT(codec->core.addr);
> > + if (enable) {
> > + list_for_each_codec(codec, hbus)
> > + if (codec->jacktbl.used)
> > + mask |= BIT(codec->core.addr);
> > + }
> >
> > snd_hdac_chip_updatew(bus, WAKEEN, STATESTS_INT_MASK, mask);
> > }
> > @@ -81,12 +83,8 @@ void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev)
> > void hda_codec_jack_check(struct snd_sof_dev *sdev)
> > {
> > struct hda_bus *hbus = sof_to_hbus(sdev);
> > - struct hdac_bus *bus = sof_to_bus(sdev);
> > struct hda_codec *codec;
> >
> > - /* disable controller Wake Up event*/
> > - snd_hdac_chip_updatew(bus, WAKEEN, STATESTS_INT_MASK, 0);
> > -
> > list_for_each_codec(codec, hbus)
> > /*
> > * Wake up all jack-detecting codecs regardless whether an event
> > @@ -97,7 +95,7 @@ void hda_codec_jack_check(struct snd_sof_dev *sdev)
> > codec->jackpoll_interval);
> > }
> > #else
> > -void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev) {}
> > +void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable) {}
> > void hda_codec_jack_check(struct snd_sof_dev *sdev) {}
> > #endif /* CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC */
> > EXPORT_SYMBOL_NS(hda_codec_jack_wake_enable, SND_SOC_SOF_HDA_AUDIO_CODEC);
> > diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
> > index 2b001151fe37..7d00107cf3b2 100644
> > --- a/sound/soc/sof/intel/hda-dsp.c
> > +++ b/sound/soc/sof/intel/hda-dsp.c
> > @@ -617,7 +617,7 @@ static int hda_suspend(struct snd_sof_dev *sdev, bool runtime_suspend)
> >
> > #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
> > if (runtime_suspend)
> > - hda_codec_jack_wake_enable(sdev);
> > + hda_codec_jack_wake_enable(sdev, true);
> >
> > /* power down all hda link */
> > snd_hdac_ext_bus_link_power_down_all(bus);
> > @@ -683,8 +683,10 @@ static int hda_resume(struct snd_sof_dev *sdev, bool runtime_resume)
> >
> > #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
> > /* check jack status */
> > - if (runtime_resume)
> > + if (runtime_resume) {
> > + hda_codec_jack_wake_enable(sdev, false);
> > hda_codec_jack_check(sdev);
> > + }
> >
> > /* turn off the links that were off before suspend */
> > list_for_each_entry(hlink, &bus->hlink_list, list) {
> > diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
> > index 9ec8ae0fd649..a3b6f3e9121c 100644
> > --- a/sound/soc/sof/intel/hda.h
> > +++ b/sound/soc/sof/intel/hda.h
> > @@ -650,7 +650,7 @@ void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev);
> > */
> > void hda_codec_probe_bus(struct snd_sof_dev *sdev,
> > bool hda_codec_use_common_hdmi);
> > -void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev);
> > +void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable);
> > void hda_codec_jack_check(struct snd_sof_dev *sdev);
> >
> > #endif /* CONFIG_SND_SOC_SOF_HDA */
> > --
> > 2.29.2
> >
^ permalink raw reply [flat|nested] 16+ messages in thread