[PATCH 0/4] ASoC: Intel: boards: Remove ignore_suspend flag from SSP0 dai link

[PATCH 0/4] ASoC: Intel: boards: Remove ignore_suspend flag from SSP0 dai link

[Cezary Rojewski]

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

-- 
2.17.1

[PATCH 1/4] ASoC: Intel: broadwell: Remove ignore_suspend flag from SSP0 dai link

[Cezary Rojewski]

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,
-- 
2.17.1

Applied "ASoC: Intel: broadwell: Remove ignore_suspend flag from SSP0 dai link" to the asoc tree

[Mark Brown]

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,
-- 
2.20.1

[PATCH 2/4] ASoC: Intel: haswell: Remove ignore_suspend flag from SSP0 dai link

[Cezary Rojewski]

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,
-- 
2.17.1

Applied "ASoC: Intel: haswell: Remove ignore_suspend flag from SSP0 dai link" to the asoc tree

[Mark Brown]

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,
-- 
2.20.1

[PATCH 3/4] ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link

[Cezary Rojewski]

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,
-- 
2.17.1

Re: [PATCH 3/4] ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link

[Pierre-Louis Bossart]

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,
>   		.ignore_pmdown_time = 1,
>   		.be_hw_params_fixup = broadwell_ssp0_fixup,
>   		.ops = &bdw_rt5677_ops,
> 

Re: [PATCH 3/4] ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link

[Curtis Malainey]

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.

Re: [PATCH 3/4] ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link

[Cezary Rojewski]

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?

Re: [PATCH 3/4] ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link

[Pierre-Louis Bossart]

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.

Re: [PATCH 3/4] ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link

[Hans de Goede]

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,
>>
> 

Re: [PATCH 3/4] ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link

[Pierre-Louis Bossart]

>> 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.

Re: [PATCH 3/4] ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link

[Hans de Goede]

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

Re: [PATCH 3/4] ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link

[Cezary Rojewski]

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

Re: [PATCH 3/4] ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link

[Pierre-Louis Bossart]

> 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.

Re: [PATCH 3/4] ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link

[Hans de Goede]

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

Re: [PATCH 3/4] ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link

[Pierre-Louis Bossart]

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.

Re: [PATCH 3/4] ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link

[Hans de Goede]

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

Re: [PATCH 3/4] ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link

[Pierre-Louis Bossart]

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

Applied "ASoC: Intel: bdw-rt5677: Remove ignore_suspend flag from SSP0 dai link" to the asoc tree

[Mark Brown]

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,
-- 
2.20.1

[PATCH 4/4] ASoC: Intel: bdw-rt5650: Remove ignore_suspend flag from SSP0 dai link

[Cezary Rojewski]

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,
-- 
2.17.1

Applied "ASoC: Intel: bdw-rt5650: Remove ignore_suspend flag from SSP0 dai link" to the asoc tree

[Mark Brown]

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,
-- 
2.20.1

Re: [PATCH 0/4] ASoC: Intel: boards: Remove ignore_suspend flag from SSP0 dai link

[Pierre-Louis Bossart]

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