* 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@alsa-project.org 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@alsa-project.org 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@alsa-project.org, 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@alsa-project.org, 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@alsa-project.org, 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@alsa-project.org, 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@alsa-project.org, 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@alsa-project.org, 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).