* Asoc: Intel: SST (CHT) regression in asoc/for-5.11
@ 2020-11-29 12:24 Hans de Goede
2020-11-30 16:15 ` Pierre-Louis Bossart
0 siblings, 1 reply; 8+ messages in thread
From: Hans de Goede @ 2020-11-29 12:24 UTC (permalink / raw)
To: Pierre-Louis Bossart, Liam Girdwood, Jie Yang, Mark Brown; +Cc: alsa-devel
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?
Regards,
Hans
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Asoc: Intel: SST (CHT) regression in asoc/for-5.11
2020-11-29 12:24 Asoc: Intel: SST (CHT) regression in asoc/for-5.11 Hans de Goede
@ 2020-11-30 16:15 ` Pierre-Louis Bossart
2020-12-01 3:24 ` Pierre-Louis Bossart
0 siblings, 1 reply; 8+ messages in thread
From: Pierre-Louis Bossart @ 2020-11-30 16:15 UTC (permalink / raw)
To: Hans de Goede, Liam Girdwood, Jie Yang, Mark Brown; +Cc: alsa-devel
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Asoc: Intel: SST (CHT) regression in asoc/for-5.11
2020-11-30 16:15 ` Pierre-Louis Bossart
@ 2020-12-01 3:24 ` Pierre-Louis Bossart
2020-12-01 14:46 ` Takashi Iwai
0 siblings, 1 reply; 8+ messages in thread
From: Pierre-Louis Bossart @ 2020-12-01 3:24 UTC (permalink / raw)
To: Hans de Goede, Liam Girdwood, Jie Yang, Mark Brown
Cc: alsa-devel, Bard liao, Ranjani Sridharan
> 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?
Hans, can you confirm if indeed this is the same issue on your devices?
Thanks
-Pierre
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Asoc: Intel: SST (CHT) regression in asoc/for-5.11
2020-12-01 3:24 ` Pierre-Louis Bossart
@ 2020-12-01 14:46 ` Takashi Iwai
2020-12-01 15:37 ` Hans de Goede
2020-12-01 16:15 ` Ranjani Sridharan
0 siblings, 2 replies; 8+ messages in thread
From: Takashi Iwai @ 2020-12-01 14:46 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: alsa-devel, Jie Yang, Ranjani Sridharan, Liam Girdwood,
Hans de Goede, Mark Brown, Bard liao
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?
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Asoc: Intel: SST (CHT) regression in asoc/for-5.11
2020-12-01 14:46 ` Takashi Iwai
@ 2020-12-01 15:37 ` Hans de Goede
2020-12-01 16:15 ` Ranjani Sridharan
1 sibling, 0 replies; 8+ messages in thread
From: Hans de Goede @ 2020-12-01 15:37 UTC (permalink / raw)
To: Takashi Iwai, Pierre-Louis Bossart
Cc: alsa-devel, Jie Yang, Ranjani Sridharan, Liam Girdwood,
Mark Brown, Bard liao
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Asoc: Intel: SST (CHT) regression in asoc/for-5.11
2020-12-01 14:46 ` Takashi Iwai
2020-12-01 15:37 ` Hans de Goede
@ 2020-12-01 16:15 ` Ranjani Sridharan
2020-12-01 16:23 ` Takashi Iwai
1 sibling, 1 reply; 8+ messages in thread
From: Ranjani Sridharan @ 2020-12-01 16:15 UTC (permalink / raw)
To: Takashi Iwai, Pierre-Louis Bossart
Cc: alsa-devel, Jie Yang, Liam Girdwood, Hans de Goede, Mark Brown,
Bard liao
> >
> > 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?
>
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Asoc: Intel: SST (CHT) regression in asoc/for-5.11
2020-12-01 16:15 ` Ranjani Sridharan
@ 2020-12-01 16:23 ` Takashi Iwai
2020-12-01 23:52 ` Ranjani Sridharan
0 siblings, 1 reply; 8+ messages in thread
From: Takashi Iwai @ 2020-12-01 16:23 UTC (permalink / raw)
To: Ranjani Sridharan
Cc: alsa-devel, Jie Yang, Pierre-Louis Bossart, Liam Girdwood,
Hans de Goede, Mark Brown, Bard liao
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.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?
> >
> > 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Asoc: Intel: SST (CHT) regression in asoc/for-5.11
2020-12-01 16:23 ` Takashi Iwai
@ 2020-12-01 23:52 ` Ranjani Sridharan
0 siblings, 0 replies; 8+ messages in thread
From: Ranjani Sridharan @ 2020-12-01 23:52 UTC (permalink / raw)
To: Takashi Iwai
Cc: alsa-devel, Jie Yang, Pierre-Louis Bossart, Liam Girdwood,
Hans de Goede, Mark Brown, Bard liao
> > 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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-12-01 23:53 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-29 12:24 Asoc: Intel: SST (CHT) regression in asoc/for-5.11 Hans de Goede
2020-11-30 16:15 ` Pierre-Louis Bossart
2020-12-01 3:24 ` Pierre-Louis Bossart
2020-12-01 14:46 ` Takashi Iwai
2020-12-01 15:37 ` Hans de Goede
2020-12-01 16:15 ` Ranjani Sridharan
2020-12-01 16:23 ` Takashi Iwai
2020-12-01 23:52 ` Ranjani Sridharan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.