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

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

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

Thanks -Pierre

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

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

On Tue, 01 Dec 2020 17:15:15 +0100, Ranjani Sridharan wrote:

...

...

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

Indeed the DAPM_START_STREAM call completely disappeared after the patch, which looks very wrong. This has to be revisited before 5.11 merge.

Hi Pierre/Takashi,

The DAPM_STREAM_START event is still there in soc_pcm_prepare() and this patch only removed the duplicate call in dpcm_fe_dai_prepare().

Ah, thanks, I see now.

But note that the PCM prepare callback may be called multiple times in row; i.e. it's not always paired with hw_clean (that is via either hw_params error path or hw_free). So if the balance really matters, we need another type of checks, not relying on the call pattern.

Takashi