Created attachment 298291 [details] Fix to prevent link reset On a 7820HK KBL / ALC662 rev3 Analog machine, shutdown hangs starting with my commit 472e18f63c425dda97b888f40f858ea54e3efc17 Author: Imre Deak <imre.deak@intel.com> Date: Wed Jun 23 16:46:00 2021 +0300 ALSA: hda: Release controller display power during shutdown/reboot commit, bisected by youling257@gmail.com. For the original report see https://gitlab.freedesktop.org/drm/intel/-/issues/3618#note_1024665 The above commit resets the HDA controller-codec link besides powering down the controller. The power down step is a NOP on this machine, since the pre-shutdown runtime resume will power on and back down the controller. So the likely culprit is resetting the link (Youling's test confirms this), and all I can think of is that for some reason link reset itself would hang (not sure why since during system suspend this works fine) or some BIOS shutdown code depends on the link not to be reset. Takshi, any idea what the actual root cause could be? Does the attached fix looks ok to you (also considering differences between HSW/BDW and other platforms like this)?
I'm fine with your suggested patch if that's confirmed to work.
My slight remaining suspect is about the leftover codec driver activity. We have the shutdown hook for each codec driver, but it's not always set up, and it might be wise to forcibly runtime-suspend at shutdown. The patch set below (consisting of 3 patches) does it. It's totally untested.
Created attachment 298295 [details] Patch1/3 for codec suspend-at-shutdown
Created attachment 298297 [details] Patch2/3 for codec suspend-at-shutdown
Created attachment 298299 [details] Patch3/3 for codec suspend-at-shutdown
Takashi, thanks. Youling could you please give a go to Takashi's 3 patches instead of the one from me?
test 3 patches can shutdown and suspend.
OK, thanks, that's a good news! Though, those patches are a bit too intrusive for 5.14 release, as it has too wide influence. So I'd like to push this for 5.15 at first, while we can take a less intrusive workaround by Imre. Do I understand correctly that Imre's patch worked for you?
Yes, Imre's patch worked for me.
Then Imre, could you submit a proper patch for merge? I'd merge and push for 5.14-rc7.
Created attachment 298331 [details] Fix to prevent link reset2 Takashi, ok will send it. I suppose the final correct state is still that the link needs to be reset before power-off. In that case are you going to revert later this fixup from me and would the attached patch be better with less changes?
Yes, that looks better! BTW, I'm thinking whether we might want to address it in PCI core level. Basically applying the runtime resume forcibly at PCI shutdown is utterly redundant for the devices like HD-audio that will do suspend again at its shutdown callback. Instead, those devices prefer runtime-suspended at shutdown from the beginning. It can be achieved by introducing a flag in struct pci_dev. But this needs discussion with PCI people in anyway.
test kernel 5.15-rc1. failed shutdown again.
test Revert "ALSA: hda: Drop workaround for a hang at shutdown again" can shutdown. the three patches not fix problem.
Hrm, that's bad. Then I revert the change again. Thanks for checking.
Looking at https://patchwork.freedesktop.org/series/94719/ I don't think the revert helps. Under 'known issues' in BAT: igt@core_hotunplug@unbind-rebind: fi-cfl-8700k: PASS -> INCOMPLETE (i915#4130) fi-rkl-11600: PASS -> INCOMPLETE (i915#4130) fi-cfl-8109u: PASS -> INCOMPLETE (i915#4130)
(In reply to Maarten Lankhorst from comment #16) > Looking at https://patchwork.freedesktop.org/series/94719/ > I don't think the revert helps. > > Under 'known issues' in BAT: > > igt@core_hotunplug@unbind-rebind: > > fi-cfl-8700k: PASS -> INCOMPLETE (i915#4130) > > fi-rkl-11600: PASS -> INCOMPLETE (i915#4130) > > fi-cfl-8109u: PASS -> INCOMPLETE (i915#4130) OK, could you try bisection please?