From: Perry Yuan perry.yuan@amd.com
[ Upstream commit 8e3967a71e6fca9c871f98b9289b59c82b88b729 ]
The variable `pm_suspend_target_state` is conditionally defined only when `CONFIG_SUSPEND` is enabled (see `include/linux/suspend.h`). Directly referencing it without guarding by `#ifdef CONFIG_SUSPEND` causes build failures when suspend functionality is disabled (e.g., `CONFIG_SUSPEND=n`).
Reviewed-by: Lijo Lazar lijo.lazar@amd.com Signed-off-by: Perry Yuan perry.yuan@amd.com Signed-off-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
Explanation
- Fixes a real build failure: The code referenced pm_suspend_target_state unconditionally in amdgpu_pmops_suspend(), but pm_suspend_target_state is only provided when suspend support is enabled. In some kernel configurations (CONFIG_SUSPEND=n), this causes a build error. Guarding these references with IS_ENABLED(CONFIG_SUSPEND) fixes the build without altering runtime behavior when suspend is enabled. See include/linux/suspend.h for the conditional exposure of suspend interfaces.
- Targeted, minimal changes: The patch only touches drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c and adds two preprocessor guards: - Around the validation block that checks mixed suspend states and error-logs the unsupported state: - if (adev->last_suspend_state != PM_SUSPEND_ON && adev->last_suspend_state != pm_suspend_target_state) { … drm_err_once(…, pm_suspend_target_state) … } - Around caching the last suspend state: - adev->last_suspend_state = pm_suspend_target_state;
- No functional change when CONFIG_SUSPEND=y: With suspend enabled, the guards pass and the logic remains identical to pre-patch behavior. The driver still validates suspend state transitions and caches the last used state.
- Safe behavior when CONFIG_SUSPEND=n: With suspend disabled, the guarded code is compiled out. The suspend PM op already returns 0 when neither s0ix nor S3 are active, and system suspend is not invocable in this configuration, so skipping references to pm_suspend_target_state has no behavioral impact, only avoids the compile-time dependency.
- Consistent with existing AMDGPU patterns: Other AMDGPU code already guards pm_suspend_target_state behind CONFIG_SUSPEND. For example: - drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c uses #if IS_ENABLED(CONFIG_SUSPEND) around pm_suspend_target_state uses, ensuring buildability across configs.
- Scope and risk assessment: - Small, contained change; no architectural refactors. - Only affects amdgpu’s system suspend path and only the compile-time inclusion of two code blocks. - No side effects for runtime PM or other subsystems. - Typical stable criteria: it’s a build fix for a valid configuration, low risk of regression, and confined to a single driver.
- Backport note: This is applicable to stable branches that already contain the unguarded uses of pm_suspend_target_state in amdgpu_pmops_suspend() within drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c. Branches that lack those references won’t need this patch.
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 65f4a76490eac..c1792e9ab126d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -2597,6 +2597,7 @@ static int amdgpu_pmops_suspend(struct device *dev) else if (amdgpu_acpi_is_s3_active(adev)) adev->in_s3 = true; if (!adev->in_s0ix && !adev->in_s3) { +#if IS_ENABLED(CONFIG_SUSPEND) /* don't allow going deep first time followed by s2idle the next time */ if (adev->last_suspend_state != PM_SUSPEND_ON && adev->last_suspend_state != pm_suspend_target_state) { @@ -2604,11 +2605,14 @@ static int amdgpu_pmops_suspend(struct device *dev) pm_suspend_target_state); return -EINVAL; } +#endif return 0; }
+#if IS_ENABLED(CONFIG_SUSPEND) /* cache the state last used for suspend */ adev->last_suspend_state = pm_suspend_target_state; +#endif
return amdgpu_device_suspend(drm_dev, true); }