ASUS ROG Strix G513QY fails to resume from suspend [regression]

Brief summary of the problem:

Laptop ASUS ROG Strix G513QY fails to resume from suspend. The problem can be fixed by removing amdgpu asic reset from the suspend routine. Patches that introduced the problem:

drm/amdgpu: always reset the asic in suspend (v2)
drm/amd/pm: keep the BACO feature enabled for suspend
drm/amdgpu: Ensure HDA function is suspended before ASIC reset

Hardware description:

• CPU: AMD Ryzen 9 5900HX with Radeon Graphics
• dGPU: 03:00.0 Display controller: Advanced Micro Devices, Inc. [AMD/ATI] Navi 22 [Radeon RX 6700/6700 XT/6750 XT / 6800M] (rev c3)
• iGPU: 07:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI] Cezanne (rev c4)
• System Memory: 32GB
• Display(s): internal laptop screen
• Type of Display Connection: eDP

System information:

• Distro name and Version: Fedora Linux 35 (Workstation Edition)
• Kernel version: Linux foo 5.17.5-200.fc35.x86_64 SMP PREEMPT Sat May 7 08:01:30 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Can you please share a dmesg and acpidump? And are you saying that if you revert those 3 patches on this kernel things are "working"? Did you bisect down to those 3 commits?

So, to be precise, I bisect to first 2 patches between Fedora kernel versions 5.15.13 and 5.15.14 and the last patch appears later.
At now, for Fedora kernel 5.17.5 I have applied this patch linux-kernel-test.patch
Here is the adcpidump acpidump.zip and dmesg with single suspend – resume procedure dmesg.txt
Yes, it working now.

added s0ix label

With your W/A patch, are you getting into the deepest state across the suspend/resume cycle? I'd guess yes since we don't see the amd_pmc warning but you can confirm by setting this dynamic debug statement.

The suspend / resume cycle seems to have reached its deepest state:
[ 336.983818] amd_pmc AMDI0005:00: Last suspend in deepest state for 9315246us
and log file dmesg-suspend.txt

During the failure to resume on 5.17.5 with no patches applied, a few questions:

1. Have you tried to SSH in during this time from another machine?
2. By any chance do you have anything from /var/lib/systemd/pstore associated with this?

I can't connect via ssh and the ping is lost.
/var/lib/systemd/pstore is empty

Hmm. It seems to me that we need an s2idle version of https://github.com/torvalds/linux/commit/ebc002e3ee78409c42156e62e4e27ad1d09c5a75

@lijo, @agd5f thoughts on this one?

in_s0i3 is never set for dGPUs and BACO is not supports for APUs.

@spin83 can you try https://gitlab.freedesktop.org/drm/amd/uploads/443ef3fd6bab3004ce3f3ffa85cab2e8/baco_reset.diff which was attached to #1977 (closed) instead of your patch?

mentioned in issue #1982 (closed)

Unfortunately, it did not help.

OK, how about this: 0001-drm-amdgpu-Set-that-the-device-is-not-in-S3-after-it.patch

It doesn't work either.

Could you take the latest and just revert "drm/amd/pm: keep the BACO feature enabled for suspend" alone? In the latest code, we don't do reset through BACO, this shouldn't be there as well.

To explain further - that above patch was added assuming that there is an upcoming GPU reset as part of suspend using BACO and that BACO entry would subsequently disable DPMs. That assumption is no longer true with this - https://github.com/torvalds/linux/commit/ebc002e3ee78409c42156e62e4e27ad1d09c5a75

Even if it doesn't fix the issue, I guess the patch should be reverted. @agd5f, what do you think?

Edit: Didn't go through the entire thread earlier. Just saw this https://gitlab.freedesktop.org/drm/amd/uploads/81513b7759a7abb3651c35a79286469c/linux-kernel-test.patch and this is in line with the above explanation.

I'm fine to drop the reset on suspend. Patches attached.

0001-Revert-drm-amdgpu-don-t-use-BACO-for-reset-in-S3.patch

0002-Revert-drm-amdgpu-Ensure-HDA-function-is-suspended-b.patch

0004-Revert-drm-amdgpu-always-reset-the-asic-in-suspend-v.patch

0003-Revert-drm-amd-pm-keep-the-BACO-feature-enabled-for-.patch

Can't you keep the W/A for interrupted suspend and instead look at pm_suspend_target_state == PM_SUSPEND_TO_IDLE to unbreak this system? Although I guess that's a W/A on top of W/A.

I thought we don't need to drop all, just revert drm/amd/pm: keep the BACO feature enabled for suspend as BACO reset on suspend is disabled now.

Did that fix it?

Yeah that's what @spin83 did as part of #2008 (comment 1372664)

Ah, I missed that. yeah, so we can just revert that change. 0001-Revert-drm-amd-pm-keep-the-BACO-feature-enabled-for-.patch

Yeah. Add #1982 (closed) to that as well, as I'm pretty sure at least the suspend part of that issue is actually the same thing as this one.

Done.

But I wrote about these two patches

• drm/amdgpu: always reset the asic in suspend (v2)
• drm/amd/pm: keep the BACO feature enabled for suspend

for kernel 5.15.14

Your W/A that you said was getting to deepest state was just reverting that one.

Sorry - for further clarification the reason that this revert makes sense WITHOUT the other one is because of https://github.com/torvalds/linux/commit/ebc002e3ee78, which also went to stable.

Ok, thanks for clarification.

mentioned in issue #1998 (closed)

mentioned in commit agd5f/linux@db0eec65

mentioned in commit agd5f/linux@0eb73fee

mentioned in commit agd5f/linux@a56f445f

This landed in 5.18-rc7 and is CC to stable, so closing this issue.

closed