[PATCH 0/4] ASoC: Intel: boards: Remove ignore_suspend flag from SSP0 dai link
As of commit: ASoC: soc-core: care .ignore_suspend for Component suspend
function soc-core::snd_soc_suspend no longer ignores 'ignore_suspend' flag for dai links. While BE dai link for System Pin is supposed to follow standard suspend-resume flow, appended 'ignore_suspend' flag disturbs that flow and causes audio to break right after resume. Remove the flag to address this.
Link to first message in conversation: https://lkml.org/lkml/2020/3/18/54
Cezary Rojewski (4): ASoC: Intel: broadwell: Remove ignore_suspend flag from SSP0 dai link ASoC: Intel: haswell: Remove ignore_suspend flag from SSP0 dai link ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link ASoC: Intel: bdw-rt5650: Remove ignore_suspend flag from SSP0 dai link
sound/soc/intel/boards/bdw-rt5650.c | 1 - sound/soc/intel/boards/bdw-rt5677.c | 1 - sound/soc/intel/boards/broadwell.c | 1 - sound/soc/intel/boards/haswell.c | 1 - 4 files changed, 4 deletions(-)
As of commit: ASoC: soc-core: care .ignore_suspend for Component suspend
function soc-core::snd_soc_suspend no longer ignores 'ignore_suspend' flag for dai links. While BE dai link for System Pin is supposed to follow standard suspend-resume flow, appended 'ignore_suspend' flag disturbs that flow and causes audio to break right after resume. Remove the flag to address this.
Link to first message in conversation: https://lkml.org/lkml/2020/3/18/54
Cc: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reported-by: Dominik Brodowski linux@dominikbrodowski.net Suggested-by: Mark Brown broonie@kernel.org Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/boards/broadwell.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/soc/intel/boards/broadwell.c b/sound/soc/intel/boards/broadwell.c index 25178000c6a5..0776ea2d4f36 100644 --- a/sound/soc/intel/boards/broadwell.c +++ b/sound/soc/intel/boards/broadwell.c @@ -220,7 +220,6 @@ static struct snd_soc_dai_link broadwell_rt286_dais[] = { .init = broadwell_rt286_codec_init, .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS, - .ignore_suspend = 1, .ignore_pmdown_time = 1, .be_hw_params_fixup = broadwell_ssp0_fixup, .ops = &broadwell_rt286_ops,
The patch
ASoC: Intel: broadwell: Remove ignore_suspend flag from SSP0 dai link
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From ec14b65ab6bcd583967880edd9688c7540cf5496 Mon Sep 17 00:00:00 2001
From: Cezary Rojewski cezary.rojewski@intel.com Date: Thu, 19 Mar 2020 21:49:44 +0100 Subject: [PATCH] ASoC: Intel: broadwell: Remove ignore_suspend flag from SSP0 dai link
As of commit: ASoC: soc-core: care .ignore_suspend for Component suspend
function soc-core::snd_soc_suspend no longer ignores 'ignore_suspend' flag for dai links. While BE dai link for System Pin is supposed to follow standard suspend-resume flow, appended 'ignore_suspend' flag disturbs that flow and causes audio to break right after resume. Remove the flag to address this.
Link to first message in conversation: https://lkml.org/lkml/2020/3/18/54
Reported-by: Dominik Brodowski linux@dominikbrodowski.net Suggested-by: Mark Brown broonie@kernel.org Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Acked-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Link: https://lore.kernel.org/r/20200319204947.18963-2-cezary.rojewski@intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/boards/broadwell.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/soc/intel/boards/broadwell.c b/sound/soc/intel/boards/broadwell.c index acb4e36682cb..f9a8336a0541 100644 --- a/sound/soc/intel/boards/broadwell.c +++ b/sound/soc/intel/boards/broadwell.c @@ -217,7 +217,6 @@ static struct snd_soc_dai_link broadwell_rt286_dais[] = { .init = broadwell_rt286_codec_init, .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS, - .ignore_suspend = 1, .ignore_pmdown_time = 1, .be_hw_params_fixup = broadwell_ssp0_fixup, .ops = &broadwell_rt286_ops,
As of commit: ASoC: soc-core: care .ignore_suspend for Component suspend
function soc-core::snd_soc_suspend no longer ignores 'ignore_suspend' flag for dai links. While BE dai link for System Pin is supposed to follow standard suspend-resume flow, appended 'ignore_suspend' flag disturbs that flow and causes audio to break right after resume. Remove the flag to address this.
Cc: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Dominik Brodowski linux@dominikbrodowski.net Cc: Mark Brown broonie@kernel.org Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/boards/haswell.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/soc/intel/boards/haswell.c b/sound/soc/intel/boards/haswell.c index 6589fa56873f..650c3c618ee4 100644 --- a/sound/soc/intel/boards/haswell.c +++ b/sound/soc/intel/boards/haswell.c @@ -162,7 +162,6 @@ static struct snd_soc_dai_link haswell_rt5640_dais[] = { .no_pcm = 1, .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS, - .ignore_suspend = 1, .ignore_pmdown_time = 1, .be_hw_params_fixup = haswell_ssp0_fixup, .ops = &haswell_rt5640_ops,
The patch
ASoC: Intel: haswell: Remove ignore_suspend flag from SSP0 dai link
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From a99661531e129f41f356bcbf6f57aee3695b6940 Mon Sep 17 00:00:00 2001
From: Cezary Rojewski cezary.rojewski@intel.com Date: Thu, 19 Mar 2020 21:49:45 +0100 Subject: [PATCH] ASoC: Intel: haswell: Remove ignore_suspend flag from SSP0 dai link
As of commit: ASoC: soc-core: care .ignore_suspend for Component suspend
function soc-core::snd_soc_suspend no longer ignores 'ignore_suspend' flag for dai links. While BE dai link for System Pin is supposed to follow standard suspend-resume flow, appended 'ignore_suspend' flag disturbs that flow and causes audio to break right after resume. Remove the flag to address this.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Acked-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Dominik Brodowski linux@dominikbrodowski.net Cc: Mark Brown broonie@kernel.org Link: https://lore.kernel.org/r/20200319204947.18963-3-cezary.rojewski@intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/boards/haswell.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/soc/intel/boards/haswell.c b/sound/soc/intel/boards/haswell.c index 3ed53d7db4e6..74af090f2657 100644 --- a/sound/soc/intel/boards/haswell.c +++ b/sound/soc/intel/boards/haswell.c @@ -162,7 +162,6 @@ static struct snd_soc_dai_link haswell_rt5640_dais[] = { .no_pcm = 1, .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS, - .ignore_suspend = 1, .ignore_pmdown_time = 1, .be_hw_params_fixup = haswell_ssp0_fixup, .ops = &haswell_rt5640_ops,
As of commit: ASoC: soc-core: care .ignore_suspend for Component suspend
function soc-core::snd_soc_suspend no longer ignores 'ignore_suspend' flag for dai links. While BE dai link for System Pin is supposed to follow standard suspend-resume flow, appended 'ignore_suspend' flag disturbs that flow and causes audio to break right after resume. Remove the flag to address this.
Cc: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Dominik Brodowski linux@dominikbrodowski.net Cc: Mark Brown broonie@kernel.org Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/boards/bdw-rt5677.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/soc/intel/boards/bdw-rt5677.c b/sound/soc/intel/boards/bdw-rt5677.c index a94f498388e1..713ef48b36a8 100644 --- a/sound/soc/intel/boards/bdw-rt5677.c +++ b/sound/soc/intel/boards/bdw-rt5677.c @@ -343,7 +343,6 @@ static struct snd_soc_dai_link bdw_rt5677_dais[] = { .no_pcm = 1, .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS, - .ignore_suspend = 1, .ignore_pmdown_time = 1, .be_hw_params_fixup = broadwell_ssp0_fixup, .ops = &bdw_rt5677_ops,
On 3/19/20 3:49 PM, Cezary Rojewski wrote:
As of commit: ASoC: soc-core: care .ignore_suspend for Component suspend
function soc-core::snd_soc_suspend no longer ignores 'ignore_suspend' flag for dai links. While BE dai link for System Pin is supposed to follow standard suspend-resume flow, appended 'ignore_suspend' flag disturbs that flow and causes audio to break right after resume. Remove the flag to address this.
Cc: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Dominik Brodowski linux@dominikbrodowski.net Cc: Mark Brown broonie@kernel.org Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com
we should ask Ben and Curtis @ Google if the changes related to suspend interfere with the wake-on-voice support?
Btw the .ignore_suspend is also set in bytcr_rt5640/51 drivers, so wondering if additional devices are broken, or if there's something off about Broadwell in general. Hans, have you heard of any regressions on Baytrail devices?
sound/soc/intel/boards/bdw-rt5677.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/soc/intel/boards/bdw-rt5677.c b/sound/soc/intel/boards/bdw-rt5677.c index a94f498388e1..713ef48b36a8 100644 --- a/sound/soc/intel/boards/bdw-rt5677.c +++ b/sound/soc/intel/boards/bdw-rt5677.c @@ -343,7 +343,6 @@ static struct snd_soc_dai_link bdw_rt5677_dais[] = { .no_pcm = 1, .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS,
.ignore_suspend = 1,
On Thu, Mar 19, 2020 at 3:15 PM Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
On 3/19/20 3:49 PM, Cezary Rojewski wrote:
As of commit: ASoC: soc-core: care .ignore_suspend for Component suspend
function soc-core::snd_soc_suspend no longer ignores 'ignore_suspend' flag for dai links. While BE dai link for System Pin is supposed to follow standard suspend-resume flow, appended 'ignore_suspend' flag disturbs that flow and causes audio to break right after resume. Remove the flag to address this.
Cc: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Dominik Brodowski linux@dominikbrodowski.net Cc: Mark Brown broonie@kernel.org Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com
we should ask Ben and Curtis @ Google if the changes related to suspend interfere with the wake-on-voice support?
I have a samus with me so I can test it but my backlog is definitely growing due to WFH slowness. I will see if I can take a look tomorrow.
On 2020-03-19 23:43, Curtis Malainey wrote:
On Thu, Mar 19, 2020 at 3:15 PM Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
On 3/19/20 3:49 PM, Cezary Rojewski wrote:
As of commit: ASoC: soc-core: care .ignore_suspend for Component suspend
function soc-core::snd_soc_suspend no longer ignores 'ignore_suspend' flag for dai links. While BE dai link for System Pin is supposed to follow standard suspend-resume flow, appended 'ignore_suspend' flag disturbs that flow and causes audio to break right after resume. Remove the flag to address this.
Cc: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Dominik Brodowski linux@dominikbrodowski.net Cc: Mark Brown broonie@kernel.org Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com
we should ask Ben and Curtis @ Google if the changes related to suspend interfere with the wake-on-voice support?
I have a samus with me so I can test it but my backlog is definitely growing due to WFH slowness. I will see if I can take a look tomorrow.
Any update?
Maybe let's leave bdw-rt5650/ bdw-rt5677 behind so people have more time to test and merge just the broadwell & haswell part. Hmm?
On 3/25/20 8:28 AM, Cezary Rojewski wrote:
On 2020-03-19 23:43, Curtis Malainey wrote:
On Thu, Mar 19, 2020 at 3:15 PM Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
On 3/19/20 3:49 PM, Cezary Rojewski wrote:
As of commit: ASoC: soc-core: care .ignore_suspend for Component suspend
function soc-core::snd_soc_suspend no longer ignores 'ignore_suspend' flag for dai links. While BE dai link for System Pin is supposed to follow standard suspend-resume flow, appended 'ignore_suspend' flag disturbs that flow and causes audio to break right after resume. Remove the flag to address this.
Cc: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Dominik Brodowski linux@dominikbrodowski.net Cc: Mark Brown broonie@kernel.org Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com
we should ask Ben and Curtis @ Google if the changes related to suspend interfere with the wake-on-voice support?
I have a samus with me so I can test it but my backlog is definitely growing due to WFH slowness. I will see if I can take a look tomorrow.
Any update?
Maybe let's leave bdw-rt5650/ bdw-rt5677 behind so people have more time to test and merge just the broadwell & haswell part. Hmm?
Can you give us a bit more time (couple of days tops)? we also see a problem with SOF when playing after suspend-resume, and I just thought this might be related to the same issue.
Hi,
On 3/19/20 11:14 PM, Pierre-Louis Bossart wrote:
On 3/19/20 3:49 PM, Cezary Rojewski wrote:
As of commit: ASoC: soc-core: care .ignore_suspend for Component suspend
function soc-core::snd_soc_suspend no longer ignores 'ignore_suspend' flag for dai links. While BE dai link for System Pin is supposed to follow standard suspend-resume flow, appended 'ignore_suspend' flag disturbs that flow and causes audio to break right after resume. Remove the flag to address this.
Cc: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Dominik Brodowski linux@dominikbrodowski.net Cc: Mark Brown broonie@kernel.org Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com
we should ask Ben and Curtis @ Google if the changes related to suspend interfere with the wake-on-voice support?
Btw the .ignore_suspend is also set in bytcr_rt5640/51 drivers, so wondering if additional devices are broken, or if there's something off about Broadwell in general. Hans, have you heard of any regressions on Baytrail devices?
I've just tested 5.6.0 on Bay Trail + a rt5651 codec, using the bytcr_rt5651 machine driver which sets ignore_suspend, as well as on a Cherry Trail + rt5645 device using the chtrt5645 machine driver which does _not_ set ignore suspend.
Suspend/resume work fine on both and music playing before suspend continues playing after suspend.
Note that the bytcr_rt5651 machine driver also does:
snd_soc_dapm_ignore_suspend(&card->dapm, "Headphone"); snd_soc_dapm_ignore_suspend(&card->dapm, "Speaker");
Which the bdw-rt5677 seems to not do...
Regards,
Hans
sound/soc/intel/boards/bdw-rt5677.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/soc/intel/boards/bdw-rt5677.c b/sound/soc/intel/boards/bdw-rt5677.c index a94f498388e1..713ef48b36a8 100644 --- a/sound/soc/intel/boards/bdw-rt5677.c +++ b/sound/soc/intel/boards/bdw-rt5677.c @@ -343,7 +343,6 @@ static struct snd_soc_dai_link bdw_rt5677_dais[] = { .no_pcm = 1, .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS, - .ignore_suspend = 1, .ignore_pmdown_time = 1, .be_hw_params_fixup = broadwell_ssp0_fixup, .ops = &bdw_rt5677_ops,
Btw the .ignore_suspend is also set in bytcr_rt5640/51 drivers, so wondering if additional devices are broken, or if there's something off about Broadwell in general. Hans, have you heard of any regressions on Baytrail devices?
I've just tested 5.6.0 on Bay Trail + a rt5651 codec, using the bytcr_rt5651 machine driver which sets ignore_suspend, as well as on a Cherry Trail + rt5645 device using the chtrt5645 machine driver which does _not_ set ignore suspend.
Suspend/resume work fine on both and music playing before suspend continues playing after suspend.
Thanks for testing Hans.
I think we should remove those .ignore_suspend from all Baytrail/Cherrytrail drivers, no one ever enabled advanced power management except in very specific Android distributions that are no longer maintained.
Note that the bytcr_rt5651 machine driver also does:
snd_soc_dapm_ignore_suspend(&card->dapm, "Headphone"); snd_soc_dapm_ignore_suspend(&card->dapm, "Speaker");
Which the bdw-rt5677 seems to not do...
On the bytcr_rt5661, these two lines were added in the initial code in 2016, and it's also part of the legacy byt-rt5640, so it's likely a copy/paste more than a feature added on purpose.
Hi,
On 3/31/20 1:49 AM, Pierre-Louis Bossart wrote:
Btw the .ignore_suspend is also set in bytcr_rt5640/51 drivers, so wondering if additional devices are broken, or if there's something off about Broadwell in general. Hans, have you heard of any regressions on Baytrail devices?
I've just tested 5.6.0 on Bay Trail + a rt5651 codec, using the bytcr_rt5651 machine driver which sets ignore_suspend, as well as on a Cherry Trail + rt5645 device using the chtrt5645 machine driver which does _not_ set ignore suspend.
Suspend/resume work fine on both and music playing before suspend continues playing after suspend.
Thanks for testing Hans.
I think we should remove those .ignore_suspend from all Baytrail/Cherrytrail drivers, no one ever enabled advanced power management except in very specific Android distributions that are no longer maintained.
I agree, I believe I even submitted patches for that a couple of years ago, but back then I think there was still some hope to get S0i1 playback to work so the patches where not accepted.
Note that the bytcr_rt5651 machine driver also does:
snd_soc_dapm_ignore_suspend(&card->dapm, "Headphone"); snd_soc_dapm_ignore_suspend(&card->dapm, "Speaker");
Which the bdw-rt5677 seems to not do...
On the bytcr_rt5661, these two lines were added in the initial code in 2016, and it's also part of the legacy byt-rt5640, so it's likely a copy/paste more than a feature added on purpose.
Could be, we should probably also drop those 2 calls together with dropping the setting of the ignore_suspend flag.
Regards,
Hans
On 2020-03-30 23:39, Hans de Goede wrote:
Hi,
On 3/19/20 11:14 PM, Pierre-Louis Bossart wrote:
we should ask Ben and Curtis @ Google if the changes related to suspend interfere with the wake-on-voice support?
Btw the .ignore_suspend is also set in bytcr_rt5640/51 drivers, so wondering if additional devices are broken, or if there's something off about Broadwell in general. Hans, have you heard of any regressions on Baytrail devices?
I've just tested 5.6.0 on Bay Trail + a rt5651 codec, using the bytcr_rt5651 machine driver which sets ignore_suspend, as well as on a Cherry Trail + rt5645 device using the chtrt5645 machine driver which does _not_ set ignore suspend.
Suspend/resume work fine on both and music playing before suspend continues playing after suspend.
Note that the bytcr_rt5651 machine driver also does:
snd_soc_dapm_ignore_suspend(&card->dapm, "Headphone"); snd_soc_dapm_ignore_suspend(&card->dapm, "Speaker");
Which the bdw-rt5677 seems to not do...
Regards,
Hans
Thanks for your report Hans!
As all streams (whether that's playback routed through Headphones or Speaker, or capture) were connected to SSP0 dai link, there is a question to be raised: Is Capture path needed to be up during suspend for broadwell solutions?
Guess these two lines you mentioned above have the exact same impact on playback as complete .ignore_suspend flag removal from SSP0 dai link.
Don't believe WoV for WPT has been added for SST linux so .ignore_suspend=1 achieves nothing. The offload part is probably just limited to bigger buffer size compared to system pin than anything else. So, nothing prevents from removing .ignore_suspend for SST solutions until WoV is verified. Don't know about SOF side. Pierre, what's your take on this?
Czarek
Don't believe WoV for WPT has been added for SST linux so .ignore_suspend=1 achieves nothing. The offload part is probably just limited to bigger buffer size compared to system pin than anything else. So, nothing prevents from removing .ignore_suspend for SST solutions until WoV is verified. Don't know about SOF side. Pierre, what's your take on this?
I think on Baytrail and Broadwell we will most likely never enable hotwording on the Intel DSP, and S0i1-Playback isn't planned either.
However hotwording is enabled on the RT5677 and there may be clocking dependencies so that the RT5677 remains operational - at least the mclk needs to be on, so the bdw-rt5677 case probably needs more work.
While I am at it, I think the notion of .ignore_suspend has an assumption baked in. It will work if the only suspend mode is S0i1. If the platform can choose between S0i1 and S3, then we would still want to suspend all paths in S3, which currently isn't really possible.
Hi,
On 3/31/20 12:54 PM, Pierre-Louis Bossart wrote:
Don't believe WoV for WPT has been added for SST linux so .ignore_suspend=1 achieves nothing. The offload part is probably just limited to bigger buffer size compared to system pin than anything else. So, nothing prevents from removing .ignore_suspend for SST solutions until WoV is verified. Don't know about SOF side. Pierre, what's your take on this?
I think on Baytrail and Broadwell we will most likely never enable hotwording on the Intel DSP, and S0i1-Playback isn't planned either.
However hotwording is enabled on the RT5677 and there may be clocking dependencies so that the RT5677 remains operational - at least the mclk needs to be on, so the bdw-rt5677 case probably needs more work.
While I am at it, I think the notion of .ignore_suspend has an assumption baked in. It will work if the only suspend mode is S0i1. If the platform can choose between S0i1 and S3, then we would still want to suspend all paths in S3, which currently isn't really possible.
You can check if S0i1 will be used by using:
pm_suspend_default_s2idle()
If this returns true then S0i1 will be used, otherwise S3.
This is defined in kernel/power/suspend.c and always build, so it should be ok to also use this in non x86 specific code-paths (it will simply always return false there I believe).
Regards,
Hans
On 3/31/20 7:12 AM, Hans de Goede wrote:
Hi,
On 3/31/20 12:54 PM, Pierre-Louis Bossart wrote:
Don't believe WoV for WPT has been added for SST linux so .ignore_suspend=1 achieves nothing. The offload part is probably just limited to bigger buffer size compared to system pin than anything else. So, nothing prevents from removing .ignore_suspend for SST solutions until WoV is verified. Don't know about SOF side. Pierre, what's your take on this?
I think on Baytrail and Broadwell we will most likely never enable hotwording on the Intel DSP, and S0i1-Playback isn't planned either.
However hotwording is enabled on the RT5677 and there may be clocking dependencies so that the RT5677 remains operational - at least the mclk needs to be on, so the bdw-rt5677 case probably needs more work.
While I am at it, I think the notion of .ignore_suspend has an assumption baked in. It will work if the only suspend mode is S0i1. If the platform can choose between S0i1 and S3, then we would still want to suspend all paths in S3, which currently isn't really possible.
You can check if S0i1 will be used by using:
pm_suspend_default_s2idle()
If this returns true then S0i1 will be used, otherwise S3.
This is defined in kernel/power/suspend.c and always build, so it should be ok to also use this in non x86 specific code-paths (it will simply always return false there I believe).
Yes, but what I meant is that when the target is S3, we have no way of disabling those .ignore_suspend that were added for S0i1 usages.
Hi,
On 3/30/20 11:39 PM, Hans de Goede wrote:
Hi,
On 3/19/20 11:14 PM, Pierre-Louis Bossart wrote:
On 3/19/20 3:49 PM, Cezary Rojewski wrote:
As of commit: ASoC: soc-core: care .ignore_suspend for Component suspend
function soc-core::snd_soc_suspend no longer ignores 'ignore_suspend' flag for dai links. While BE dai link for System Pin is supposed to follow standard suspend-resume flow, appended 'ignore_suspend' flag disturbs that flow and causes audio to break right after resume. Remove the flag to address this.
Cc: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Dominik Brodowski linux@dominikbrodowski.net Cc: Mark Brown broonie@kernel.org Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com
we should ask Ben and Curtis @ Google if the changes related to suspend interfere with the wake-on-voice support?
Btw the .ignore_suspend is also set in bytcr_rt5640/51 drivers, so wondering if additional devices are broken, or if there's something off about Broadwell in general. Hans, have you heard of any regressions on Baytrail devices?
I've just tested 5.6.0 on Bay Trail + a rt5651 codec, using the bytcr_rt5651 machine driver which sets ignore_suspend, as well as on a Cherry Trail + rt5645 device using the chtrt5645 machine driver which does _not_ set ignore suspend.
Suspend/resume work fine on both and music playing before suspend continues playing after suspend.
Note that the bytcr_rt5651 machine driver also does:
snd_soc_dapm_ignore_suspend(&card->dapm, "Headphone"); snd_soc_dapm_ignore_suspend(&card->dapm, "Speaker");
Which the bdw-rt5677 seems to not do...
I just noticed something which is somewhat related to this discussion (and also somewhat off topic).
I just noticed on a Bay Trail device with a RT5672 codec and on a Cherry Trail device with a RT5645 codec that if an input / recording audio stream is active while suspending the tablet, then after resume audio is broken, playback seems to work (audio samples get consumed at normal speed) but it is silent. Recording also is broken, not sure if it is broken, or just silent too.
When this happens, closing all apps which consume audio and waiting 5 seconds for a runtime-suspend to kick in fixes things. After this both record and playback works again.
Any idea what the cause for this problem might be?
I can reproduce this in 2 ways:
1. Have the sound capplet of GNOME's control-panel open, this shows a VU meter for the default audio input, this VU meter stops working after a suspend resume and playback also stops working if a suspend/resume is done with the VU meter active when suspending.
2. Start a sound recording in gnome-sound-recorder and then suspend + resume.
Regards,
Hans
I just noticed something which is somewhat related to this discussion (and also somewhat off topic).
I just noticed on a Bay Trail device with a RT5672 codec and on a Cherry Trail device with a RT5645 codec that if an input / recording audio stream is active while suspending the tablet, then after resume audio is broken, playback seems to work (audio samples get consumed at normal speed) but it is silent. Recording also is broken, not sure if it is broken, or just silent too.
When this happens, closing all apps which consume audio and waiting 5 seconds for a runtime-suspend to kick in fixes things. After this both record and playback works again.
Any idea what the cause for this problem might be?
Power management for the Atom/sst stuff is far from clear for me, it uses .prepare/.complete callbacks where snd_soc_suspend()/poweroff()/resume() are invoked, so there's a bit of confusion IMO between that the framework does and what the driver should do.
It's also unclear to me why the INFO_RESUME flag was set, since the driver cannot restart from the same position.
I would try and triangulate with the more traditional bytcr-rt5640, to rule out a codec-specific or machine driver issue (handling of rt5645 and 5672 was done by different people and the machine driver is quite different).
I would also remove the INFO_RESUME, that will force the ALSA core to call the .prepare steps and maybe restore settings that are not applied with the current resume.
Either way, it's a bit of a shot in the dark :-(
My 2 cents -Pierre
The patch
ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From b0ada40cb80d7e427fb719a4e6029935639fa668 Mon Sep 17 00:00:00 2001
From: Cezary Rojewski cezary.rojewski@intel.com Date: Thu, 19 Mar 2020 21:49:46 +0100 Subject: [PATCH] ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link
As of commit: ASoC: soc-core: care .ignore_suspend for Component suspend
function soc-core::snd_soc_suspend no longer ignores 'ignore_suspend' flag for dai links. While BE dai link for System Pin is supposed to follow standard suspend-resume flow, appended 'ignore_suspend' flag disturbs that flow and causes audio to break right after resume. Remove the flag to address this.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Acked-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Dominik Brodowski linux@dominikbrodowski.net Cc: Mark Brown broonie@kernel.org Link: https://lore.kernel.org/r/20200319204947.18963-4-cezary.rojewski@intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/boards/bdw-rt5677.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/soc/intel/boards/bdw-rt5677.c b/sound/soc/intel/boards/bdw-rt5677.c index 6b4b64098d36..cc41a348295e 100644 --- a/sound/soc/intel/boards/bdw-rt5677.c +++ b/sound/soc/intel/boards/bdw-rt5677.c @@ -340,7 +340,6 @@ static struct snd_soc_dai_link bdw_rt5677_dais[] = { .no_pcm = 1, .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS, - .ignore_suspend = 1, .ignore_pmdown_time = 1, .be_hw_params_fixup = broadwell_ssp0_fixup, .ops = &bdw_rt5677_ops,
As of commit: ASoC: soc-core: care .ignore_suspend for Component suspend
function soc-core::snd_soc_suspend no longer ignores 'ignore_suspend' flag for dai links. While BE dai link for System Pin is supposed to follow standard suspend-resume flow, appended 'ignore_suspend' flag disturbs that flow and causes audio to break right after resume. Remove the flag to address this.
Cc: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Dominik Brodowski linux@dominikbrodowski.net Cc: Mark Brown broonie@kernel.org Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/boards/bdw-rt5650.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/soc/intel/boards/bdw-rt5650.c b/sound/soc/intel/boards/bdw-rt5650.c index 058abf3eec50..4545dbd48879 100644 --- a/sound/soc/intel/boards/bdw-rt5650.c +++ b/sound/soc/intel/boards/bdw-rt5650.c @@ -257,7 +257,6 @@ static struct snd_soc_dai_link bdw_rt5650_dais[] = { .no_pcm = 1, .dai_fmt = SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS, - .ignore_suspend = 1, .ignore_pmdown_time = 1, .be_hw_params_fixup = broadwell_ssp0_fixup, .ops = &bdw_rt5650_ops,
The patch
ASoC: Intel: bdw-rt5650: Remove ignore_suspend flag from SSP0 dai link
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 793012c6c586fefef3abd45c9d2b94df042907b0 Mon Sep 17 00:00:00 2001
From: Cezary Rojewski cezary.rojewski@intel.com Date: Thu, 19 Mar 2020 21:49:47 +0100 Subject: [PATCH] ASoC: Intel: bdw-rt5650: Remove ignore_suspend flag from SSP0 dai link
As of commit: ASoC: soc-core: care .ignore_suspend for Component suspend
function soc-core::snd_soc_suspend no longer ignores 'ignore_suspend' flag for dai links. While BE dai link for System Pin is supposed to follow standard suspend-resume flow, appended 'ignore_suspend' flag disturbs that flow and causes audio to break right after resume. Remove the flag to address this.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com Acked-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Cc: Dominik Brodowski linux@dominikbrodowski.net Cc: Mark Brown broonie@kernel.org Link: https://lore.kernel.org/r/20200319204947.18963-5-cezary.rojewski@intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/boards/bdw-rt5650.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/soc/intel/boards/bdw-rt5650.c b/sound/soc/intel/boards/bdw-rt5650.c index 6c2fdb5659ed..af2f50293208 100644 --- a/sound/soc/intel/boards/bdw-rt5650.c +++ b/sound/soc/intel/boards/bdw-rt5650.c @@ -254,7 +254,6 @@ static struct snd_soc_dai_link bdw_rt5650_dais[] = { .no_pcm = 1, .dai_fmt = SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS, - .ignore_suspend = 1, .ignore_pmdown_time = 1, .be_hw_params_fixup = broadwell_ssp0_fixup, .ops = &bdw_rt5650_ops,
On 3/19/20 3:49 PM, Cezary Rojewski wrote:
As of commit: ASoC: soc-core: care .ignore_suspend for Component suspend
function soc-core::snd_soc_suspend no longer ignores 'ignore_suspend' flag for dai links. While BE dai link for System Pin is supposed to follow standard suspend-resume flow, appended 'ignore_suspend' flag disturbs that flow and causes audio to break right after resume. Remove the flag to address this.
Link to first message in conversation: https://lkml.org/lkml/2020/3/18/54
Cezary Rojewski (4): ASoC: Intel: broadwell: Remove ignore_suspend flag from SSP0 dai link ASoC: Intel: haswell: Remove ignore_suspend flag from SSP0 dai link ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link ASoC: Intel: bdw-rt5650: Remove ignore_suspend flag from SSP0 dai link
tested on Broadwell XPS 13 and bdw-rt5677, so
Acked-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
sound/soc/intel/boards/bdw-rt5650.c | 1 - sound/soc/intel/boards/bdw-rt5677.c | 1 - sound/soc/intel/boards/broadwell.c | 1 - sound/soc/intel/boards/haswell.c | 1 - 4 files changed, 4 deletions(-)
participants (5)
-
Cezary Rojewski -
Curtis Malainey -
Hans de Goede -
Mark Brown -
Pierre-Louis Bossart