amdgpu_device_supports_boco() returns true for an AMD_IS_PX device on a
system with a hybrid ATPX method even when the device has no ACPI power
resource (_PR3). On an APU mini PC with a discrete GPU attached over
OCuLink, the iGPU's ATPX method reports hybrid graphics, the dGPU is
flagged AMD_IS_PX, and runtime PM in auto mode picks BOCO:

  amdgpu: ATPX Hybrid Graphics
  amdgpu 0000:c6:00.0: Using BOCO for runtime pm

In BOCO mode the driver leaves powering the device off to the PCI core,
which can only do it through _PR3 on the upstream bridge. No device on
this dGPU's path has _PR3, so when it runtime suspends the card stays
powered, with its fans at about 700 rpm.

Require has_pr3 for BOCO, as the function's description ("dGPU with ACPI
power resources") says. The same card then picks BACO, which powers it
down:

  amdgpu 0000:c6:00.0: Using BACO for runtime pm

This only changes systems with a hybrid ATPX method and no _PR3 on the
dGPU's upstream bridge, where BOCO cannot cut the power today. Tested on
one such system only, an RX 9070 XT (Navi 48) over OCuLink with a Ryzen
AI 9 HX 470, with the change backported to 7.2.9: the stock module logs
"Using BOCO", the patched one "Using BACO"; BACO runtime suspend powers
the card off and it resumes cleanly. Not tested on a hybrid laptop or a
Thunderbolt eGPU.

Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5980
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Luigi Lauro <[email protected]>
---
Alex asked for this on the issue. Here is the full picture, so you can
judge which approach is best:

Why _PR3: in amdgpu_pmops_runtime_suspend() the BOCO branch does nothing
itself ("PCI core handles it for _PR3"), and has_pr3 checks exactly the
bridge the PCI core would power off (link_partner, past the dGPU's own
switch). On a hybrid ATPX, amdgpu_device_supports_px() is false, so the
ATPX power-control method is not used either.

Who changes: only hybrid-ATPX systems without _PR3 on that bridge. Hybrid
laptops that have _PR3 there, which I understand is the usual design,
are unaffected.

Risks I cannot rule out:
- BACO on older laptop chips that have never used it for runtime PM.
- A platform that cuts the power in a way has_pr3 does not see (_PR3
  somewhere else on the path).
- The other users of the function change too on such a system: the
  DPM_FLAG_SMART_SUSPEND | DPM_FLAG_MAY_SKIP_RESUME flags in
  amdgpu_pci_probe(), amdgpu_pmops_prepare() returning 1, and
  amdgpu_device_supports_smart_shift().

Narrower alternative: leave amdgpu_device_supports_boco() alone and
require has_pr3 only where amdgpu_device_detect_runtime_pm_mode() picks
BOCO in the auto case, falling back to BACO. That fixes this system too
and keeps the other three users as they are, which is the same state as
booting it today with amdgpu.runpm=1.

Please tell me which you prefer; I can send a v2 with the narrower form.

 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index e001461..96cb4d3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -593,10 +593,8 @@ bool amdgpu_device_supports_boco(struct amdgpu_device 
*adev)
        if (!IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
                return false;
 
-       if (adev->has_pr3 ||
-           ((adev->flags & AMD_IS_PX) && amdgpu_is_atpx_hybrid()))
-               return true;
-       return false;
+       /* BOCO needs a platform power resource (_PR3) to cut the power */
+       return adev->has_pr3;
 }
 
 /**
-- 
2.56.0

Reply via email to