Re: Asoc: Intel: SST (CHT) regression in asoc/for-5.11

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 12/1/20 3:46 PM, Takashi Iwai wrote:
> On Tue, 01 Dec 2020 04:24:58 +0100,
> Pierre-Louis Bossart wrote:
>>
>>
>>
>>
>>> On 11/29/20 6:24 AM, Hans de Goede wrote:
>>>> Hi All,
>>>>
>>>> To test the code to dynamically switch between SST/SOF support on BYT/CHT
>>>> from the kernel commandline I merged:
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/log/?h=for-5.11 
>>>>
>>>>
>>>> Into my personal tree (mostly Linus' master + some pending patches from
>>>> myself).
>>>>
>>>> After this I was getting the following errors in dmesg when using
>>>> sound on
>>>> a Medion E2228T laptop with a CHT SoC + NAU8824 codec:
>>>>
>>>> [   53.805205] intel_sst_acpi 808622A8:00: Wait timed-out
>>>> condition:0x0, msg_id:0x1 fw_state 0
>>>> [   53.805479] intel_sst_acpi 808622A8:00: fw returned err -16
>>>> [   53.806281] sst-mfld-platform sst-mfld-platform: ASoC: PRE_PMD:
>>>> pcm0_in event failed: -16
>>>> [   54.829548] intel_sst_acpi 808622A8:00: Wait timed-out
>>>> condition:0x0, msg_id:0x1 fw_state 0
>>>> [   54.829596] intel_sst_acpi 808622A8:00: fw returned err -16
>>>> [   54.829668] sst-mfld-platform sst-mfld-platform: ASoC: POST_PMD:
>>>> media0_out event failed: -
>>>> [   55.853230] intel_sst_acpi 808622A8:00: Wait timed-out
>>>> condition:0x0, msg_id:0x1 fw_state 0
>>>> [   55.853244] intel_sst_acpi 808622A8:00: fw returned err -16
>>>> [   55.853269] sst-mfld-platform sst-mfld-platform: ASoC: POST_PMD:
>>>> codec_out0 mix 0 event fai
>>>> [   56.876435] intel_sst_acpi 808622A8:00: Wait timed-out
>>>> condition:0x0, msg_id:0x1 fw_state 0
>>>> [   56.876481] intel_sst_acpi 808622A8:00: fw returned err -16
>>>> [   56.876563] sst-mfld-platform sst-mfld-platform: ASoC: POST_PMD:
>>>> media0_out mix 0 event fai
>>>> [   61.847455] intel_sst_acpi 808622A8:00: FW sent error response 0x40015
>>>> [   61.847564] intel_sst_acpi 808622A8:00: fw returned err -1
>>>> [   61.847659] sst-mfld-platform sst-mfld-platform: ASoC: error at
>>>> snd_soc_dai_startup on ssp2
>>>> [   61.847722]  SSP2-Codec: ASoC: BE open failed -1
>>>> [   61.847754]  Audio Port: ASoC: failed to start some BEs -1
>>>> [   61.847786] intel_sst_acpi 808622A8:00: FW sent error response 0x40006
>>>> [   64.301284] intel_sst_acpi 808622A8:00: FW sent error response 0x90001
>>>> [   64.301545] intel_sst_acpi 808622A8:00: not suspending FW!!, Err: -2
>>>>
>>>> Dropping the asoc/for-5.11 merge and just cherry-picking
>>>> Pierre-Louis' changes
>>>> for the dynamic switching makes these go away. So this seems to be
>>>> caused by
>>>> other changes in asoc/for-5.11.
>>>>
>>>> So any clues where to start looking for this, or should I just
>>>> bisect this?
>>>
>>> Thanks for reporting this Hans.
>>>
>>> The only thing that comes to my mind is Morimoto-san's series which
>>> modified snd_soc_dai_startup, but that was back in September and
>>> should be in 5.10-rcX
>>>
>>> Will give it a try on my side as well.
>>
>> I was able to reproduce this error with Mark's for-next branch on a
>> CHT device w/ rt5640, and git bisect points to this commit:
>>
>> a27b421f1d04b201c474a15ee1591919c81fb413 is the first bad commit
>> commit a27b421f1d04b201c474a15ee1591919c81fb413
>> Author: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
>> Date:   Tue Nov 17 13:50:01 2020 -0800
>>
>>     ASoC: pcm: call snd_soc_dapm_stream_stop() in soc_pcm_hw_clean
>>
>>     Currently, the SND_SOC_DAPM_STREAM_START event is sent during
>>     pcm_prepare() but the SND_SOC_DAPM_STREAM_STOP event is
>>     sent only in dpcm_fe_dai_shutdown() after soc_pcm_close().
>>     This results in an imbalance between when the DAPM widgets
>>     receive the PRE/POST_PMU/PMD events. So call
>>     snd_soc_dapm_stream_stop() in soc_pcm_hw_clean() before the
>>     snd_soc_pcm_component_hw_free() to keep the stream_stop DAPM
>>     event balanced with the stream_start event in soc_pm_prepare().
>>
>>     Also, in order to prevent duplicate DAPM stream events,
>>     remove the call for DAPM STREAM_START event in dpcm_fe_dai_prepare()
>>     and the call for DAPM STREAM_STOP event in dpcm_fe_dai_shutdown().
>>
>>     Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
>>     Reviewed-by: Pierre-Louis Bossart
>> <pierre-louis.bossart@linux.intel.com>
>>     Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
>>     Link:
>> https://lore.kernel.org/r/20201117215001.163107-1-ranjani.sridharan@linux.intel.com
>>     Signed-off-by: Mark Brown <broonie@kernel.org>
>>
>>  sound/soc/soc-pcm.c | 10 +++-------
>>  1 file changed, 3 insertions(+), 7 deletions(-)
>>
>>
>> I am not sure why this break the Atom/SST driver, this was reviewed
>> and seemed legit - even required IIRC to deal with topology pipelines
>> initialized on-demand. Reverting this patch restores functionality. I
>> would guess it's the DAPM_STREAM_START that's now missing (or in the
>> 'wrong' location) and causing issues?

Pierre-Louis, thank you for bisecting this.

> 
> Indeed the DAPM_START_STREAM call completely disappeared after the
> patch, which looks very wrong.

Yes that probably explains this issue.

>> Hans, can you confirm if indeed this is the same issue on your devices?

I've tried reverting the commit and I can confirm that asoc/for-5.11
works fine with the commit reverted.

Regards,

Hans