Message-ID: <a7bf2aee-78e7-f905-bcc3-cd21bf16a976@intel.com>
Date:   Thu, 19 Mar 2020 18:33:50 +0100
From:   Cezary Rojewski <cezary.rojewski@...el.com>
To:     Dominik Brodowski <linux@...inikbrodowski.net>
Cc:     Mark Brown <broonie@...nel.org>, kuninori.morimoto.gx@...esas.com,
        Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        Keyon Jie <yang.jie@...ux.intel.com>,
        alsa-devel@...a-project.org, curtis@...ainey.com,
        linux-kernel@...r.kernel.org, tiwai@...e.com,
        liam.r.girdwood@...ux.intel.com
Subject: Re: snd_hda_intel/sst-acpi sound breakage on suspend/resume since
 5.6-rc1

On 2020-03-19 17:51, Dominik Brodowski wrote:
> On Thu, Mar 19, 2020 at 04:48:03PM +0100, Cezary Rojewski wrote:
>> On 2020-03-19 14:41, Mark Brown wrote:
>>> On Thu, Mar 19, 2020 at 02:00:49PM +0100, Dominik Brodowski wrote:
>>>
>>>> Have some good news now, namely that a bisect is complete: That pointed to
>>>> 1272063a7ee4 ("ASoC: soc-core: care .ignore_suspend for Component suspend");
>>>> therefore I've added Kuninori Morimoto to this e-mail thread.
>>>
>>> If that's an issue it feels more like a driver bug in that if the driver
>>> asked for ignore_suspend then it should expect not to have the suspend
>>> callback called.
>>>
>>
>> Requested for tests with following diff applied:
>>
>> diff --git a/sound/soc/intel/boards/broadwell.c
>> b/sound/soc/intel/boards/broadwell.c
>> index db7e1e87156d..6ed4c1b0a515 100644
>> --- a/sound/soc/intel/boards/broadwell.c
>> +++ b/sound/soc/intel/boards/broadwell.c
>> @@ -212,7 +212,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,
> 
> That patch fixes the issue(s). I didn't even need to revert 64df6afa0dab
> ("ASoC: Intel: broadwell: change cpu_dai and platform components for SOF")
> on top of that. But you can assess better whether that patch needs care for
> other reasons; for me, this one-liner you have suggested is perfect.
> 
> Many thanks -- it's been a pleasure to work with you on tracking this issue
> down.
> 
> 	Dominik
> 

Thank you for being so cooperative during this 2day debug session.

The patch I mentioned earlier unintentionally (?) changed 'platform' 
component param for ssp0_port from 'dummy' to 'platform' for non-SOF 
solution:

diff --git a/sound/soc/intel/boards/broadwell.c 
b/sound/soc/intel/boards/broadwell.c
index b9c12e24c70b..db7e1e87156d 100644
--- a/sound/soc/intel/boards/broadwell.c
+++ b/sound/soc/intel/boards/broadwell.c
@@ -164,14 +164,6 @@ SND_SOC_DAILINK_DEF(platform,
  SND_SOC_DAILINK_DEF(codec,
         DAILINK_COMP_ARRAY(COMP_CODEC("i2c-INT343A:00", "rt286-aif1")));

-#if IS_ENABLED(CONFIG_SND_SOC_SOF_BROADWELL)
-SND_SOC_DAILINK_DEF(ssp0_port,
-           DAILINK_COMP_ARRAY(COMP_CPU("ssp0-port")));
-#else
-SND_SOC_DAILINK_DEF(ssp0_port,
-           DAILINK_COMP_ARRAY(COMP_DUMMY()));
-#endif
-
  /* broadwell digital audio interface glue - connects codec <--> CPU */
  static struct snd_soc_dai_link broadwell_rt286_dais[] = {
         /* Front End DAI links */
@@ -226,7 +218,7 @@ static struct snd_soc_dai_link 
broadwell_rt286_dais[] = {
                 .ops = &broadwell_rt286_ops,
                 .dpcm_playback = 1,
                 .dpcm_capture = 1,
-               SND_SOC_DAILINK_REG(ssp0_port, codec, platform),
+               SND_SOC_DAILINK_REG(dummy, codec, dummy),
         },


Said change causes following to occur:


(stream start)
[  113.251950] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000003000000 size: 77
[  113.252090] haswell-pcm-audio haswell-pcm-audio: > rx: 
0x0000000043000000 size: 48
[  113.252097] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006301000 size: 20
[  113.252147] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006301000 size: 20
[  113.252179] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006100000 size: 0
[  113.252219] snd_soc_core:dpcm_fe_dai_hw_params:  System PCM: ASoC: 
hw_params FE System PCM rate 48000 chan 2 fmt 2
[  113.252229] snd_soc_core:dapm_update_dai_unlocked: haswell-pcm-audio 
haswell-pcm-audio: Update DAI routes for System Pin playback
[  113.252236] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006000000 size: 0
[  113.252304] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000004000000 size: 4
[  113.252425] snd_soc_sst_haswell_pcm:create_adsp_page_table:  System 
PCM: generating page table for 00000000a8c2b8a6 size 0x17700 pages 24


(In essence these tx'es denote sequence for stream initialization while 
the last two for stream RESET (0x6000000) and FREE (0x4000000))

and that is only to recreate the stream once again:


[  113.252673] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000003000000 size: 77
[  113.252803] haswell-pcm-audio haswell-pcm-audio: > rx: 
0x0000000043000000 size: 48
[  113.252810] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006301000 size: 20
[  113.252864] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006301000 size: 20
[  113.252900] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006100000 size: 0
[  113.252987] snd_soc_core:dpcm_fe_dai_prepare:  System PCM: ASoC: 
prepare FE System PCM
[  113.252993] snd_soc_core:dpcm_be_dai_prepare:  Codec: ASoC: prepare 
BE Codec
[  113.253028] snd_soc_core:dpcm_dapm_stream_event:  Codec: ASoC: BE 
Codec event 1 dir 0
[  113.254962] snd_soc_core:dpcm_do_trigger:  Codec: ASoC: trigger BE 
Codec cmd 1


Because of that we ended up in _reset and _free being called twice:


[  113.254969] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006200000 size: 0
[  113.254980] snd_soc_core:dpcm_dai_trigger_fe_be:  System PCM: ASoC: 
post trigger FE System PCM cmd 1
[  113.254983] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006200000 size: 0
[  113.254996] snd_soc_sst_ipc:ipc_tx_msgs: haswell-pcm-audio 
haswell-pcm-audio: ipc_tx_msgs dsp busy
[  118.486291]  System PCM: ASoC: trigger FE cmd: 7 failed: -22
[  118.486431] snd_soc_core:dpcm_dai_trigger_fe_be:  System PCM: ASoC: 
pre trigger FE System PCM cmd 0
[  118.486464] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006100000 size: 0
[  118.486495] snd_soc_core:dpcm_do_trigger:  Codec: ASoC: trigger BE 
Codec cmd 0
[  118.486514] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006100000 size: 0
[  118.486550] snd_soc_core:dpcm_fe_dai_hw_free:  System PCM: ASoC: 
hw_free FE System PCM
[  118.486569] snd_soc_core:dpcm_be_dai_hw_free:  Codec: ASoC: hw_free 
BE Codec
[  118.486719] snd_soc_core:dpcm_fe_dai_hw_free:  System PCM: ASoC: 
hw_free FE System PCM
[  118.486734] snd_soc_core:dpcm_be_dai_hw_free:  Codec: ASoC: hw_free 
BE Codec
[  118.486751] snd_soc_core:dpcm_be_dai_shutdown:  Codec: ASoC: close BE 
Codec
[  118.486801] snd_soc_sst_ipc:ipc_tx_msgs: haswell-pcm-audio 
haswell-pcm-audio: ipc_tx_msgs dsp busy
[  118.489279] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000006000000 size: 0
[  118.489382] haswell-pcm-audio haswell-pcm-audio: tx: 
0x0000000004000000 size: 4
[  118.489535] snd_soc_core:dpcm_fe_dai_shutdown:  System PCM: ASoC: 
close FE System PCM
[  118.489547] haswell-pcm-audio haswell-pcm-audio: warning: stream is 
NULL, no stream to reset, ignore it.
[  118.489553] haswell-pcm-audio haswell-pcm-audio: warning: stream is 
NULL, no stream to free, ignore it.
[  118.489571] snd_soc_core:dpcm_be_disconnect:  System PCM: ASoC: BE 
playback disconnect check for Codec
[  118.489580] snd_soc_core:dpcm_be_disconnect:  System PCM: freed DSP 
playback path System PCM -> Codec


Could you confirm the same happens on your machine when revert of 
mentioned patch is not applied ("stream is NULL" messages occur)? Issue 
may be harmless but explained sequence does not look right.

Czarek

Powered by blists - more mailing lists