[HD-A] System does not wake up Playback/Capture-> pause -> suspend->resume scenario #5035

ssavati · 2024-06-03T17:15:37Z

System is not waking up Playback/Capture-> pause -> resume scenario.

Steps to reproduce

Use Any HDA board (I have tried MTLP and ADLP Platforms)
Open a terminal and execute cmd aplay -Dhw:0,0 -fs16_le -c2 -r 48000 -vv -i /dev/zero
Press space to pause
Open another terminal and perform sudo rtcwake -m mem -s 10
sudo rtcwake -m mem -s 10
At this stage system not comeback and need reboot

Other observation

For SDW configuartion issue not observed.
This issue is observed for Headset playback/capture. for HDMI device system wakes up.
System wake up if we do suspend without pausing Playback/capture.

Issue reproduciblity : 100%

Kernel/firmware branch/commits
Linux Branch:
topic/sof-dev
Linux Commit:
fbf0b7bd4110

SOF Branch:
main
SOF Commit:
283475c0d6c8
Zephyr Commit:
9d8059b6e554

cc:

[LNL]Suspend resume with playback or capture will Fail or TIMEOUT #5080

plbossart · 2024-06-04T04:33:46Z

Thanks for testing this @ssavati, this looks like a global issue indeed. Could you test this on LNL where the HDAudio DMA is used as well, I am not sure we're tested this, ever.

@kv2019i @ranj063 @ujfalusi my money is on a transition where we keep the DMA programmed, which is known to have side effects.

ujfalusi · 2024-06-04T06:21:24Z

I can confirm this on TGL (upx-i11)

ujfalusi · 2024-06-04T09:07:44Z

I think I have a fix for this...

ujfalusi · 2024-06-04T11:24:04Z

I think I have a fix for this...

it will take some time. I can prevent the lockup, but the sequencing is pretty broken.

ujfalusi · 2024-06-04T14:48:55Z

The finding so far:
suspend while pause does not work (locks up the system) with sof-dev with IPC3 and IPC4 alike
The lockup is caused by incorrect code in hda_dai_suspend()
Fixing the function will allow the suspend/resume while paused, but it is still broken for IPC3/IPC4:
IPC3 is w/o errors but when you try to un-pause, the aplay will exit
IPC4 have IPC errors on suspend, but after resume we manage to restart the stream (resume fail, and full restart happens)

To fix the IPC4 errors and the broken IPC3 we need to move the paused stream to suspend-proper by:

int sof_pcm_suspend_paused(struct snd_sof_dev *sdev)
{
	struct snd_pcm_substream *substream;
	struct snd_sof_pcm *spcm;
	int dir, ret;

	list_for_each_entry(spcm, &sdev->pcm_list, list) {
		for_each_pcm_streams(dir) {
			substream = spcm->stream[dir].substream;
			if (!substream || !substream->runtime)
				continue;

			if (!(substream->runtime->state == SNDRV_PCM_STATE_SUSPENDED &&
			    substream->runtime->suspended_state == SNDRV_PCM_STATE_PAUSED))
				continue;

			ret = substream->ops->trigger(substream,
						      SNDRV_PCM_TRIGGER_PAUSE_RELEASE);
			if (ret)
				return ret;

			ret = substream->ops->trigger(substream,
						      SNDRV_PCM_TRIGGER_SUSPEND);
			if (ret)
				return ret;
		}
	}

	return 0;
}

and call this from early suspend.
This will make the errors go away, but still a full restart will be done instead of a clean resume...

plbossart · 2024-06-04T15:33:10Z

we had to do something manually for SoundWire to support the suspend-while-paused transition.

static int intel_component_dais_suspend(struct snd_soc_component *component)
{
	struct snd_soc_dai *dai;

	/*
	 * In the corner case where a SUSPEND happens during a PAUSE, the ALSA core
	 * does not throw the TRIGGER_SUSPEND. This leaves the DAIs in an unbalanced state.
	 * Since the component suspend is called last, we can trap this corner case
	 * and force the DAIs to release their resources.
	 */
	for_each_component_dais(component, dai) {
		struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai);
		struct sdw_cdns_dai_runtime *dai_runtime;

		dai_runtime = cdns->dai_runtime_array[dai->id];

		if (!dai_runtime)
			continue;

		if (dai_runtime->suspended)
			continue;

		if (dai_runtime->paused)
			dai_runtime->suspended = true;
	}

	return 0;
}

…suspended state Paused streams will not receive a suspend trigger, they will be marked by ALSA core as suspended and it's state is saved. Since the pause stream is not in a fully stopped state, for example DMA might be still enabled (just the trigger source is removed/disabled) we need to make sure that the hardware is ready to handle the suspend. This involves a bit more than just stopping a DMA since we also need to communicate with the firmware in a delicate sequence to follow IP programming flows. To make things a bit more challenging, these flows are different between IPC versions due to the fact that they use different messages to implement the same functionality. To avoid adding yet another path, callbacks and sequencing for handling the corner case of suspending while a stream is paused, and do this for each IPC versions and platforms, we can move the stream back to running just to put it to suspended state. In this way we will essentially reduce the suspend cases to two: no audio and running audio (the paused audio becomes a running audio case), cutting down on the probability of misaligned handling of cases. Link: thesofproject#5035 Signed-off-by: Peter Ujfalusi <[email protected]>

…or nocodec only We need to face with reality that the pause/resume is a feature that is not well tested - end users are using audio via audio servers and they don't use pause/resume, causing constant issues with no real life benefit: With IPC4 multiple pause/resume will make the delay reporting way off from the known universe, causing reported delays in tens or hundreds of years. Looks like suspend/resume with paused stream has been broken for a long time and just got noticed (since it was not tested). Pause on capture has never been supported, but it was advertised on playback by the kernel. Add a new token to allow selected PCMs to be allowed to advertise pause support (playback_pause_supported) and keep it false by default. For testing purposes enable the pause support for nocodec topologies. Signed-off-by: Peter Ujfalusi <[email protected]> Link: thesofproject/linux#5035

ujfalusi · 2024-06-05T15:40:39Z

I can confirm this on TGL (upx-i11)

It happens with both IPC3 and IPC4.

…suspended state Paused streams will not receive a suspend trigger, they will be marked by ALSA core as suspended and it's state is saved. Since the pause stream is not in a fully stopped state, for example DMA might be still enabled (just the trigger source is removed/disabled) we need to make sure that the hardware is ready to handle the suspend. This involves a bit more than just stopping a DMA since we also need to communicate with the firmware in a delicate sequence to follow IP programming flows. To make things a bit more challenging, these flows are different between IPC versions due to the fact that they use different messages to implement the same functionality. To avoid adding yet another path, callbacks and sequencing for handling the corner case of suspending while a stream is paused, and do this for each IPC versions and platforms, we can move the stream back to running just to put it to suspended state. In this way we will essentially reduce the suspend cases to two: no audio and running audio (the paused audio becomes a running audio case), cutting down on the probability of misaligned handling of cases. Link: thesofproject#5035

…or nocodec only We need to face with reality that the pause/resume is a feature that is not well tested (end users are using audio via audio servers and they don't use pause/resume) causing constant issues with no real life benefit: With IPC4 multiple pause/resume will make the delay reporting to be exponentially shoot out, making the reported delay to be unusable. Looks like suspend/resume with paused stream has been broken for a long time and just got noticed (since it was not tested). Add a new token to allow selected PCMs to advertise pause support and keep it false by default. The kernel side will allow ignoring the flag to keep the pause advertised for continued testing while protecting accidental use of it by users. Signed-off-by: Peter Ujfalusi <[email protected]> Link: thesofproject/linux#5035

… by default We need to face with reality that the pause/resume is a feature that is not well tested (end users are using audio via audio servers and they don't use pause/resume) causing constant issues with no real life benefit: With IPC4 multiple pause/resume will make the delay reporting to be exponentially shoot out, making the reported delay to be unusable. Looks like suspend/resume with paused stream has been broken for a long time and just got noticed (since it was not tested). Add a new token to allow selected PCMs to advertise pause support and keep it false by default. The kernel side will allow ignoring the flag to keep the pause advertised for continued testing while protecting accidental use of it by users. Signed-off-by: Peter Ujfalusi <[email protected]> Link: thesofproject/linux#5035

ujfalusi · 2024-06-07T13:07:23Z

Strictly avoiding the lockup: #5049
Suspend while paused remains broken, but at least the system is not locked and we got to see the nice IPC and other errors and application giving up (aplay)

… by default We need to face with reality that the pause/resume is a feature that is not well tested (end users are using audio via audio servers and they don't use pause/resume) causing constant issues with no real life benefit: With IPC4 multiple pause/resume will make the delay reporting to be exponentially shoot out, making the reported delay to be unusable. Looks like suspend/resume with paused stream has been broken for a long time and just got noticed (since it was not tested). Add a new token to allow selected PCMs to advertise pause support and keep it false by default. The kernel side will allow ignoring the flag to keep the pause advertised for continued testing while protecting accidental use of it by users. Signed-off-by: Peter Ujfalusi <[email protected]> Link: thesofproject/linux#5035

…suspended state Paused streams will not receive a suspend trigger, they will be marked by ALSA core as suspended and it's state is saved. Since the pause stream is not in a fully stopped state, for example DMA might be still enabled (just the trigger source is removed/disabled) we need to make sure that the hardware is ready to handle the suspend. This involves a bit more than just stopping a DMA since we also need to communicate with the firmware in a delicate sequence to follow IP programming flows. To make things a bit more challenging, these flows are different between IPC versions due to the fact that they use different messages to implement the same functionality. To avoid adding yet another path, callbacks and sequencing for handling the corner case of suspending while a stream is paused, and do this for each IPC versions and platforms, we can move the stream back to running just to put it to suspended state. In this way we will essentially reduce the suspend cases to two: no audio and running audio (the paused audio becomes a running audio case), cutting down on the probability of misaligned handling of cases. Link: thesofproject#5035

Paused streams will not receive a suspend trigger, they will be marked by ALSA core as suspended and it's state is saved. Since the pause stream is not in a fully stopped state, for example DMA might be still enabled (just the trigger source is removed/disabled) we need to make sure that the hardware is ready to handle the suspend. This involves a bit more than just stopping a DMA since we also need to communicate with the firmware in a delicate sequence to follow IP programming flows. To make things a bit more challenging, these flows are different between IPC versions due to the fact that they use different messages to implement the same functionality. To avoid adding yet another path, callbacks and sequencing for handling the corner case of suspending while a stream is paused, and do this for each IPC versions and platforms, we can move the stream back to running just to put it to stopped state. Explanation of the change: Streams moved to SUSPENDED state from PAUSED without trigger. If a stream does not support RESUME then on system resume the RESUME trigger is not sent, the stream's state and suspended_state remains untouched. When the user space releases the pause then the core will reject this because the state of the stream is _not_ PAUSED, it is still SUSPENDED. From this point user space will do the normal (hw_params) prepare and START, PAUSE_RELEASE trigger will not be sent by the core after the system has resumed. Link: thesofproject#5035 Signed-off-by: Peter Ujfalusi <[email protected]>

marc-hb · 2024-07-12T21:54:56Z

I haven't followed what changed exactly but in the last 4 years I've never seen a suspend/resume pass rate so high than in the last week or two. I mean across the board, not with LNL specifically.

ujfalusi · 2024-07-15T05:59:42Z

I haven't followed what changed exactly but in the last 4 years I've never seen a suspend/resume pass rate so high than in the last week or two. I mean across the board, not with LNL specifically.

It looks like the system lock is fixed by #5085 (which is similar to one of my other attempt: f710445), but the suspend while pause is still broken:
aplay
pause it
suspend (rtcwake)
after system resume, un-pause the aplay
stop aplay:

[  112.923088] snd_sof:sof_ipc4_route_free: sof-audio-pci-intel-tgl 0000:00:1f.3: unbind modules mixin.1.1:0 -> mixout.2.1:0
[  112.923096] snd_sof:sof_ipc4_log_header: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx      : 0x46000002|0x3: MOD_UNBIND
[  112.923379] snd_sof:sof_ipc4_log_header: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx reply: 0x66000000|0x3: MOD_UNBIND
[  112.923395] snd_sof:sof_ipc4_log_header: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx done : 0x46000002|0x3: MOD_UNBIND
[  112.923403] snd_sof:sof_ipc4_log_header: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx      : 0x12000000|0x0: GLB_DELETE_PIPELINE
[  112.924189] snd_sof:sof_ipc4_log_header: sof-audio-pci-intel-tgl 0000:00:1f.3: ipc tx reply: 0x3200000c|0x0: GLB_DELETE_PIPELINE
[  112.924197] sof-audio-pci-intel-tgl 0000:00:1f.3: FW reported error: 12 - Required resource is in invalid state
[  112.924222] sof-audio-pci-intel-tgl 0000:00:1f.3: ipc error for msg 0x12000000|0x0
[  112.924228] sof-audio-pci-intel-tgl 0000:00:1f.3: failed to free pipeline widget pipeline.1
...
[  112.925345] snd_sof:sof_widget_free_unlocked: sof-audio-pci-intel-tgl 0000:00:1f.3: widget pipeline.2 freed
[  112.925348] snd_sof:sof_widget_free_unlocked: sof-audio-pci-intel-tgl 0000:00:1f.3: widget dai-copier.HDA.Analog.playback freed
[  112.925353] sof-audio-pci-intel-tgl 0000:00:1f.3: Failed to free connected widgets
[  112.925359] sof-audio-pci-intel-tgl 0000:00:1f.3: sof_pcm_stream_free: sof_widget_list_free failed -22
[  112.925365] sof-audio-pci-intel-tgl 0000:00:1f.3: ASoC: error at snd_soc_pcm_component_hw_free on 0000:00:1f.3: -22

The only fix to get suspend while paused working is still #5058

marc-hb · 2024-07-15T17:07:27Z

but the suspend while pause is still broken

I don't think we test that in CI.

… by default We need to face with reality that the pause/resume is a feature that is not well tested (end users are using audio via audio servers and they don't use pause/resume) causing constant issues with no real life benefit: With IPC4 multiple pause/resume will make the delay reporting to be exponentially shoot out, making the reported delay to be unusable. Looks like suspend/resume with paused stream has been broken for a long time and just got noticed (since it was not tested). Add a new token to allow selected PCMs to advertise pause support and keep it false by default. The kernel side will allow ignoring the flag to keep the pause advertised for continued testing while protecting accidental use of it by users. Signed-off-by: Peter Ujfalusi <[email protected]> Link: thesofproject/linux#5035

marc-hb · 2024-07-15T22:46:48Z

I don't think we test that in CI.

Actually, we have a test-case/check-pause-release-suspend-resume.sh in theory but I don't think it ever really worked in practice: thesofproject/sof-test@ceb197a

@ssavati did you know about this test?

marc-hb · 2024-08-06T00:16:49Z

I think the main test case has been fixed? Focus has shifted to:

[LNL]Suspend resume with playback or capture will Fail or TIMEOUT #5080

For sure the suspend/resume pass rate (outside LNL 5080) has never been so high for years.

Paused streams will not receive a suspend trigger, they will be marked by ALSA core as suspended and it's state is saved. Since the pause stream is not in a fully stopped state, for example DMA might be still enabled (just the trigger source is removed/disabled) we need to make sure that the hardware is ready to handle the suspend. This involves a bit more than just stopping a DMA since we also need to communicate with the firmware in a delicate sequence to follow IP programming flows. To make things a bit more challenging, these flows are different between IPC versions due to the fact that they use different messages to implement the same functionality. To avoid adding yet another path, callbacks and sequencing for handling the corner case of suspending while a stream is paused, and do this for each IPC versions and platforms, we can move the stream back to running just to put it to stopped state. Explanation of the change: Streams moved to SUSPENDED state from PAUSED without trigger. If a stream does not support RESUME then on system resume the RESUME trigger is not sent, the stream's state and suspended_state remains untouched. When the user space releases the pause then the core will reject this because the state of the stream is _not_ PAUSED, it is still SUSPENDED. From this point user space will do the normal (hw_params) prepare and START, PAUSE_RELEASE trigger will not be sent by the core after the system has resumed. Link: thesofproject#5035 Signed-off-by: Peter Ujfalusi <[email protected]>

ssavati changed the title ~~System does not wake up Playback/Capture-> pause -> suspend->resume scenario~~ [HD-A] System does not wake up Playback/Capture-> pause -> suspend->resume scenario Jun 3, 2024

marc-hb added bug Something isn't working suspend resume Issues related to suspend resume (e.g. rtcwake) labels Jun 3, 2024

ujfalusi mentioned this issue Jun 5, 2024

ASoC: SOF: sof-pcm/pm: On system suspend, move the paused streams to … #5040

Closed

ujfalusi mentioned this issue Jun 5, 2024

topology2: Add new token 'playback_pause_supported' and set it true f… thesofproject/sof#9202

Merged

ujfalusi mentioned this issue Jun 5, 2024

ASoC: SOF: core/Intel: Follow the pause_supported flag from topology to disable or keep the pause support enabled #5041

Open

ujfalusi mentioned this issue Jun 11, 2024

ASoC: SOF: Fix suspend while paused corner case #5054

Closed

ujfalusi mentioned this issue Jun 12, 2024

ASoC: SOF: sof-pcm/pm: Stop paused streams before the system suspend #5058

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HD-A] System does not wake up Playback/Capture-> pause -> suspend->resume scenario #5035

[HD-A] System does not wake up Playback/Capture-> pause -> suspend->resume scenario #5035

ssavati commented Jun 3, 2024 •

edited by marc-hb

Loading

plbossart commented Jun 4, 2024

ujfalusi commented Jun 4, 2024

ujfalusi commented Jun 4, 2024

ujfalusi commented Jun 4, 2024

ujfalusi commented Jun 4, 2024

plbossart commented Jun 4, 2024

ujfalusi commented Jun 5, 2024

ujfalusi commented Jun 7, 2024

marc-hb commented Jul 12, 2024

ujfalusi commented Jul 15, 2024

marc-hb commented Jul 15, 2024

marc-hb commented Jul 15, 2024

marc-hb commented Aug 6, 2024

[HD-A] System does not wake up Playback/Capture-> pause -> suspend->resume scenario #5035

[HD-A] System does not wake up Playback/Capture-> pause -> suspend->resume scenario #5035

Comments

ssavati commented Jun 3, 2024 • edited by marc-hb Loading

plbossart commented Jun 4, 2024

ujfalusi commented Jun 4, 2024

ujfalusi commented Jun 4, 2024

ujfalusi commented Jun 4, 2024

ujfalusi commented Jun 4, 2024

plbossart commented Jun 4, 2024

ujfalusi commented Jun 5, 2024

ujfalusi commented Jun 7, 2024

marc-hb commented Jul 12, 2024

ujfalusi commented Jul 15, 2024

marc-hb commented Jul 15, 2024

marc-hb commented Jul 15, 2024

marc-hb commented Aug 6, 2024

ssavati commented Jun 3, 2024 •

edited by marc-hb

Loading