Re: [PATCH v3 2/3] drm/amdgpu: add Apple GMUX runtime PM support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The system suspend and hibernation poweroff callbacks fail to handle the runtime-suspended state for GMUX devices, leading to unhandled MMIO accesses on powered-off hardware if `direct_complete` is aborted.
--

--- Patch [2]: [PATCH v3 2/3] drm/amdgpu: add Apple GMUX runtime PM support ---
commit bd409ba52fe58e23b69e9e8883bcbcd307329cae
Author: Andre Eikmeyer <dev@deq.rocks>
Subject: drm/amdgpu: add Apple GMUX runtime PM support

The MacBookPro15,1 uses Apple GMUX to control power to the discrete GPU,
but it does not expose the ATPX or ACPI power-resource interfaces currently
recognized by amdgpu. A dedicated GMUX runtime PM mode uses the existing
vga_switcheroo PM domain to sequence GPU power. A runtime-suspended GMUX GPU
can remain powered off across system sleep, following the smart-suspend
handling already used by BOCO devices.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 0ab380ca7e645..a231a002831bb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c

[ ... ]

> @@ -2494,11 +2494,12 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
>  		DRM_ERROR("Creating debugfs files failed (%d).\n", ret);
>  
>  	if (adev->pm.rpm_mode != AMDGPU_RUNPM_NONE) {
> -		/* only need to skip on ATPX */
> +		/* ATPX requires a full system-sleep transition. */
>  		if (amdgpu_device_supports_px(adev))
>  			dev_pm_set_driver_flags(ddev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
> -		/* we want direct complete for BOCO */
> -		if (amdgpu_device_supports_boco(adev))
> +		/* BOCO and GMUX can remain runtime suspended across system sleep. */
> +		if (amdgpu_device_supports_boco(adev) ||
> +		    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX)
>  			dev_pm_set_driver_flags(ddev->dev, DPM_FLAG_SMART_PREPARE |
>  						DPM_FLAG_SMART_SUSPEND |
>  						DPM_FLAG_MAY_SKIP_RESUME);

[Severity: High]
Does this setup require handling the runtime-suspended state in the system
suspend and hibernation callbacks for GMUX devices?

If a child device like the HDA audio controller aborts direct_complete for
the GPU, or if the GPU supports PME from D3cold, the PCI PM core will skip
runtime-resuming the GPU during system suspend.

This causes amdgpu_pmops_suspend() and amdgpu_pmops_poweroff() in
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c to be called while the GPU is still
in the D3cold state powered off by GMUX.

Since these callbacks unconditionally fall through to amdgpu_device_suspend()
without checking the runtime PM state:

drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:amdgpu_pmops_suspend() {
    ...
    return amdgpu_device_suspend(drm_dev, true);
}

and

drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:amdgpu_pmops_poweroff() {
    ...
    if (adev->in_s4 && adev->in_suspend)
        return 0;

    return amdgpu_device_suspend(drm_dev, true);
}

Could this lead to unhandled MMIO accesses on the powered-off hardware
during the suspension phase and cause system hangs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812144750.36797-1-dev@deq.rocks?part=2