Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 8 commits into from
Closed

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

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5a2443b
ASoC: SOF: Intel: hda-dai: Be really consistent with TRIGGER_SUSPEND …
ujfalusi Jun 10, 2024
9a7c7ab
ASoC: SOF: Intel: hda-dai-ops: ipc4: Reset the paused counter on STOP…
ujfalusi Jun 11, 2024
365cd78
ASoC: SOF: Add new optional suspend_early() dsp_ops callback function
ujfalusi Jun 10, 2024
5b5897a
ASoC: SOF: Intel: hda: Switch to use the suspend_early callback from …
ujfalusi Jun 11, 2024
d897da6
ASoC: SOF: Remove unused set_hw_params_upon_resume callback
ujfalusi Jun 11, 2024
09d8c8a
ASoC: SOF: sof-audio: Send STOP trigger to platforms during system su…
ujfalusi Jun 10, 2024
c3aa4bd
ASoC: SOF: ipc3-topology: Set the suspending flag for paused streams …
ujfalusi Jun 11, 2024
5c6fa89
ASoC: SOF: ipc4: Use the suspending stream flag to handle paused stre…
ujfalusi Jun 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions sound/soc/sof/intel/hda-dai-ops.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,10 +416,12 @@ static int hda_ipc4_post_trigger(struct snd_sof_dev *sdev, struct snd_soc_dai *c
case SNDRV_PCM_TRIGGER_SUSPEND:
case SNDRV_PCM_TRIGGER_STOP:
/*
* STOP/SUSPEND trigger is invoked only once when all users of this pipeline have
* been stopped. So, clear the started_count so that the pipeline can be reset
* STOP/SUSPEND trigger is invoked only once when all users of
* this pipeline have been stopped. So, clear the started and
* paused count so that the pipeline can be reset
*/
swidget->spipe->started_count = 0;
swidget->spipe->paused_count = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what would be the behavior on resume then? The stream would restart? How can we keep the stream paused on resume?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we cannot keep the streams paused, we don't support RESUME trigger, so all streams would be restarted anyways, paused or not paused.
In practice: on resume the audio will remain paused (will not run), but when you PAUSE_RELEASE it, then we will restart it. We need to re-initialize everything to get working audio.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how will the audio remain paused if the pause_count remains zero? I don't understand how this counter is used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This counter is used internally to ipc4 (to track the individual pipeline states) , ALSA keeps track of the PCM state. On suspend we need to stop everything, so we can put the DSP to off.
So, when we suspend all pipelines will be reset.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ujfalusi this is too confusing. We increment the paused_count when we pause the stream. So lets assume this sequence:

  1. Start the stream : started_count = 1
  2. Pause the stream : paused_count = 1
  3. Suspend the system: The stream doesnt get the suspend trigger, so the started/paused_counts remain at 1
  4. hda_dai_suspend() gets invoked so both counts get reset to 0.
  5. Resume the system
  6. Pause_release the stream: this will decrement the paused_count to -1?
    Am I understanding this correctly?

break;
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
break;
Expand Down