Asoc: Intel: SST (CHT) regression in asoc/for-5.11
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? Regards, Hans
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.
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
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?
Indeed the DAPM_START_STREAM call completely disappeared after the patch, which looks very wrong. This has to be revisited before 5.11 merge. Takashi
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
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(). I wonder if it is the placement of the DAPM_STREAM_STOP is the issue. I will look into this today. Thanks, Ranjani
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
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.
Hi Takashi, It seems like it is indeed a problem with prepare not being paired with hw_free. Adding the stream_stop() event back to dpcm_fe_dai_shutdown() as it was before seems to resolve the issue. I am running further tests to confirm it doesnt have adverse effects on SOF. Will post the patch shortly. Thanks, Ranjani
participants (4)
-
Hans de Goede -
Pierre-Louis Bossart -
Ranjani Sridharan -
Takashi Iwai