Skip to content

Commit 7374023

Browse files
committed
ASoC: SOF: remove unregister calls from shutdown
Merge series from Kai Vehmanen <[email protected]>: This patchset is an alternative solution to problems reported by Ricardo Ribalda <[email protected]> and Zhen Ni <[email protected]>, as discussed in - "[PATCH] ALSA: core: Fix deadlock when shutdown a frozen userspace" https://mailman.alsa-project.org/pipermail/alsa-devel/2022-November/209248.html - "[PATCH] ASoc: SOF: Fix sof-audio-pci-intel-tgl shutdown timeout during hibernation" https://mailman.alsa-project.org/pipermail/alsa-devel/2022-December/209685.html It was raised by Oliver Neukum <[email protected]> that kernel should not let user-space stall the shutdown process in any scenario and the unregister call in current SOF shutdown code is not right. This series reverts the ASoC SOF patch to unregister clients and the machine drivers at shutdown. To avoid bringing back old bugs, the series includes a patch to fix S5 entry on certain Intel platforms.
2 parents 0612d74 + 44fda61 commit 7374023

File tree

4 files changed

+74
-10
lines changed

4 files changed

+74
-10
lines changed

sound/soc/sof/core.c

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -475,19 +475,10 @@ EXPORT_SYMBOL(snd_sof_device_remove);
475475
int snd_sof_device_shutdown(struct device *dev)
476476
{
477477
struct snd_sof_dev *sdev = dev_get_drvdata(dev);
478-
struct snd_sof_pdata *pdata = sdev->pdata;
479478

480479
if (IS_ENABLED(CONFIG_SND_SOC_SOF_PROBE_WORK_QUEUE))
481480
cancel_work_sync(&sdev->probe_work);
482481

483-
/*
484-
* make sure clients and machine driver(s) are unregistered to force
485-
* all userspace devices to be closed prior to the DSP shutdown sequence
486-
*/
487-
sof_unregister_clients(sdev);
488-
489-
snd_sof_machine_unregister(sdev, pdata);
490-
491482
if (sdev->fw_state == SOF_FW_BOOT_COMPLETE)
492483
return snd_sof_shutdown(sdev);
493484

sound/soc/sof/intel/hda-dsp.c

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -878,6 +878,78 @@ int hda_dsp_suspend(struct snd_sof_dev *sdev, u32 target_state)
878878
return snd_sof_dsp_set_power_state(sdev, &target_dsp_state);
879879
}
880880

881+
static unsigned int hda_dsp_check_for_dma_streams(struct snd_sof_dev *sdev)
882+
{
883+
struct hdac_bus *bus = sof_to_bus(sdev);
884+
struct hdac_stream *s;
885+
unsigned int active_streams = 0;
886+
int sd_offset;
887+
u32 val;
888+
889+
list_for_each_entry(s, &bus->stream_list, list) {
890+
sd_offset = SOF_STREAM_SD_OFFSET(s);
891+
val = snd_sof_dsp_read(sdev, HDA_DSP_HDA_BAR,
892+
sd_offset);
893+
if (val & SOF_HDA_SD_CTL_DMA_START)
894+
active_streams |= BIT(s->index);
895+
}
896+
897+
return active_streams;
898+
}
899+
900+
static int hda_dsp_s5_quirk(struct snd_sof_dev *sdev)
901+
{
902+
int ret;
903+
904+
/*
905+
* Do not assume a certain timing between the prior
906+
* suspend flow, and running of this quirk function.
907+
* This is needed if the controller was just put
908+
* to reset before calling this function.
909+
*/
910+
usleep_range(500, 1000);
911+
912+
/*
913+
* Take controller out of reset to flush DMA
914+
* transactions.
915+
*/
916+
ret = hda_dsp_ctrl_link_reset(sdev, false);
917+
if (ret < 0)
918+
return ret;
919+
920+
usleep_range(500, 1000);
921+
922+
/* Restore state for shutdown, back to reset */
923+
ret = hda_dsp_ctrl_link_reset(sdev, true);
924+
if (ret < 0)
925+
return ret;
926+
927+
return ret;
928+
}
929+
930+
int hda_dsp_shutdown_dma_flush(struct snd_sof_dev *sdev)
931+
{
932+
unsigned int active_streams;
933+
int ret, ret2;
934+
935+
/* check if DMA cleanup has been successful */
936+
active_streams = hda_dsp_check_for_dma_streams(sdev);
937+
938+
sdev->system_suspend_target = SOF_SUSPEND_S3;
939+
ret = snd_sof_suspend(sdev->dev);
940+
941+
if (active_streams) {
942+
dev_warn(sdev->dev,
943+
"There were active DSP streams (%#x) at shutdown, trying to recover\n",
944+
active_streams);
945+
ret2 = hda_dsp_s5_quirk(sdev);
946+
if (ret2 < 0)
947+
dev_err(sdev->dev, "shutdown recovery failed (%d)\n", ret2);
948+
}
949+
950+
return ret;
951+
}
952+
881953
int hda_dsp_shutdown(struct snd_sof_dev *sdev)
882954
{
883955
sdev->system_suspend_target = SOF_SUSPEND_S3;

sound/soc/sof/intel/hda.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,7 @@ int hda_dsp_resume(struct snd_sof_dev *sdev);
592592
int hda_dsp_runtime_suspend(struct snd_sof_dev *sdev);
593593
int hda_dsp_runtime_resume(struct snd_sof_dev *sdev);
594594
int hda_dsp_runtime_idle(struct snd_sof_dev *sdev);
595+
int hda_dsp_shutdown_dma_flush(struct snd_sof_dev *sdev);
595596
int hda_dsp_shutdown(struct snd_sof_dev *sdev);
596597
int hda_dsp_set_hw_params_upon_resume(struct snd_sof_dev *sdev);
597598
void hda_dsp_dump(struct snd_sof_dev *sdev, u32 flags);

sound/soc/sof/intel/tgl.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ int sof_tgl_ops_init(struct snd_sof_dev *sdev)
6060
memcpy(&sof_tgl_ops, &sof_hda_common_ops, sizeof(struct snd_sof_dsp_ops));
6161

6262
/* probe/remove/shutdown */
63-
sof_tgl_ops.shutdown = hda_dsp_shutdown;
63+
sof_tgl_ops.shutdown = hda_dsp_shutdown_dma_flush;
6464

6565
if (sdev->pdata->ipc_type == SOF_IPC) {
6666
/* doorbell */

0 commit comments

Comments
 (0)