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.

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 deadlock when shutdown a frozen userspace #4072

Merged
merged 2 commits into from Dec 9, 2022

Conversation

kv2019i
Copy link
Collaborator

@kv2019i kv2019i commented Dec 5, 2022

An alternative to https://mailman.alsa-project.org/pipermail/alsa-devel/2022-December/209429.html

This PR reworks the shutdown and removes the unregister calls from SOF core, leaving responsibility for any cleanup actions to DSP-specific shutdown ops.

If system shutdown has not been completed cleanly, it is possible the
DMA stream shutdown has not been done, or was not clean.

If this is the case, Intel TGL/ADL HDA platforms may fail to shutdown
cleanly due to pending HDA DMA transactions. To avoid this, detect this
scenario in the shutdown callback, and perform an additional controller
reset. This has been tested to unblock S5 entry if this condition is
hit.

Co-developed-by: Archana Patni <archana.patni@intel.com>
Signed-off-by: Archana Patni <archana.patni@intel.com>
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
…shutdown"

The unregister machine drivers call is not safe to do when
kexec is used. Kexec-lite gets blocked with following backtrace:

[   84.943749] Freezing user space processes ... (elapsed 0.111 seconds) done.
[  246.784446] INFO: task kexec-lite:5123 blocked for more than 122 seconds.
[  246.819035] Call Trace:
[  246.821782]  <TASK>
[  246.824186]  __schedule+0x5f9/0x1263
[  246.828231]  schedule+0x87/0xc5
[  246.831779]  snd_card_disconnect_sync+0xb5/0x127
...
[  246.889249]  snd_sof_device_shutdown+0xb4/0x150
[  246.899317]  pci_device_shutdown+0x37/0x61
[  246.903990]  device_shutdown+0x14c/0x1d6
[  246.908391]  kernel_kexec+0x45/0xb9

This reverts commit 83bfc7e.

Reported-by: Ricardo Ribalda <ribalda@chromium.org>
Cc: Ricardo Ribalda <ribalda@chromium.org>
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i
Copy link
Collaborator Author

kv2019i commented Dec 5, 2022

@thesofproject/nxp @thesofproject/amd @thesofproject/mediatek please check, this will impact the shutdown sequence.

@kv2019i
Copy link
Collaborator Author

kv2019i commented Dec 5, 2022

Alternative would be to do an explicit snd_card_disconnect() like Takashi drafted in
https://mailman.alsa-project.org/pipermail/alsa-devel/2022-December/209437.html

@ranj063
Copy link
Collaborator

ranj063 commented Dec 5, 2022

@kv2019i is the revert also necessary with the first patch?

@kv2019i
Copy link
Collaborator Author

kv2019i commented Dec 7, 2022

@ranj063 The revert is needed as the machine driver unregister triggers the problem with freeze. But we need the first patch, as otherwise the revert will bring the known issue with S5 entry back. So both are needed.

}

return ret;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick: I would rather add the hda_dsp_shutdown_dma_flush() under the hda_dsp_shutdown(), just like if the hda_dsp_shutdown() would been a static function.

@kv2019i
Copy link
Collaborator Author

kv2019i commented Dec 8, 2022

@kv2019i wrote

@thesofproject/nxp @thesofproject/amd @thesofproject/mediatek please check, this will impact the shutdown sequence.

Let's wait one more day. I'll send tomorrow upstream. You can comment on alsa-devel as well of course.

@bhiregoudar
Copy link

@kv2019i wrote

@thesofproject/nxp @thesofproject/amd @thesofproject/mediatek please check, this will impact the shutdown sequence.

Let's wait one more day. I'll send tomorrow upstream. You can comment on alsa-devel as well of course.

Please go ahead, no comments from us.

@kv2019i kv2019i merged commit f2a1d50 into thesofproject:topic/sof-dev Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants