Skip to content

Commit b0f47c8

Browse files
chr[]gregkh
authored andcommitted
amdgpu/pm/legacy: fix suspend/resume issues
commit 91dcc66 upstream. resume and irq handler happily races in set_power_state() * amdgpu_legacy_dpm_compute_clocks() needs lock * protect irq work handler * fix dpm_enabled usage v2: fix clang build, integrate Lijo's comments (Alex) Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/2524 Fixes: 3712e7a ("drm/amd/pm: unified lock protections in amdgpu_dpm.c") Reviewed-by: Lijo Lazar <lijo.lazar@amd.com> Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name> # on Oland PRO Signed-off-by: chr[] <chris@rudorff.com> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> (cherry picked from commit ee3dc9e) Cc: stable@vger.kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 65f4aeb commit b0f47c8

File tree

3 files changed

+45
-14
lines changed

3 files changed

+45
-14
lines changed

drivers/gpu/drm/amd/pm/legacy-dpm/kv_dpm.c

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3043,13 +3043,16 @@ static int kv_dpm_hw_init(void *handle)
30433043
if (!amdgpu_dpm)
30443044
return 0;
30453045

3046+
mutex_lock(&adev->pm.mutex);
30463047
kv_dpm_setup_asic(adev);
30473048
ret = kv_dpm_enable(adev);
30483049
if (ret)
30493050
adev->pm.dpm_enabled = false;
30503051
else
30513052
adev->pm.dpm_enabled = true;
30523053
amdgpu_legacy_dpm_compute_clocks(adev);
3054+
mutex_unlock(&adev->pm.mutex);
3055+
30533056
return ret;
30543057
}
30553058

@@ -3067,32 +3070,42 @@ static int kv_dpm_suspend(void *handle)
30673070
{
30683071
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
30693072

3073+
cancel_work_sync(&adev->pm.dpm.thermal.work);
3074+
30703075
if (adev->pm.dpm_enabled) {
3076+
mutex_lock(&adev->pm.mutex);
3077+
adev->pm.dpm_enabled = false;
30713078
/* disable dpm */
30723079
kv_dpm_disable(adev);
30733080
/* reset the power state */
30743081
adev->pm.dpm.current_ps = adev->pm.dpm.requested_ps = adev->pm.dpm.boot_ps;
3082+
mutex_unlock(&adev->pm.mutex);
30753083
}
30763084
return 0;
30773085
}
30783086

30793087
static int kv_dpm_resume(void *handle)
30803088
{
3081-
int ret;
3089+
int ret = 0;
30823090
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
30833091

3084-
if (adev->pm.dpm_enabled) {
3092+
if (!amdgpu_dpm)
3093+
return 0;
3094+
3095+
if (!adev->pm.dpm_enabled) {
3096+
mutex_lock(&adev->pm.mutex);
30853097
/* asic init will reset to the boot state */
30863098
kv_dpm_setup_asic(adev);
30873099
ret = kv_dpm_enable(adev);
3088-
if (ret)
3100+
if (ret) {
30893101
adev->pm.dpm_enabled = false;
3090-
else
3102+
} else {
30913103
adev->pm.dpm_enabled = true;
3092-
if (adev->pm.dpm_enabled)
30933104
amdgpu_legacy_dpm_compute_clocks(adev);
3105+
}
3106+
mutex_unlock(&adev->pm.mutex);
30943107
}
3095-
return 0;
3108+
return ret;
30963109
}
30973110

30983111
static bool kv_dpm_is_idle(void *handle)

drivers/gpu/drm/amd/pm/legacy-dpm/legacy_dpm.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,9 +1009,12 @@ void amdgpu_dpm_thermal_work_handler(struct work_struct *work)
10091009
enum amd_pm_state_type dpm_state = POWER_STATE_TYPE_INTERNAL_THERMAL;
10101010
int temp, size = sizeof(temp);
10111011

1012-
if (!adev->pm.dpm_enabled)
1013-
return;
1012+
mutex_lock(&adev->pm.mutex);
10141013

1014+
if (!adev->pm.dpm_enabled) {
1015+
mutex_unlock(&adev->pm.mutex);
1016+
return;
1017+
}
10151018
if (!pp_funcs->read_sensor(adev->powerplay.pp_handle,
10161019
AMDGPU_PP_SENSOR_GPU_TEMP,
10171020
(void *)&temp,
@@ -1033,4 +1036,5 @@ void amdgpu_dpm_thermal_work_handler(struct work_struct *work)
10331036
adev->pm.dpm.state = dpm_state;
10341037

10351038
amdgpu_legacy_dpm_compute_clocks(adev->powerplay.pp_handle);
1039+
mutex_unlock(&adev->pm.mutex);
10361040
}

drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7783,13 +7783,15 @@ static int si_dpm_hw_init(void *handle)
77837783
if (!amdgpu_dpm)
77847784
return 0;
77857785

7786+
mutex_lock(&adev->pm.mutex);
77867787
si_dpm_setup_asic(adev);
77877788
ret = si_dpm_enable(adev);
77887789
if (ret)
77897790
adev->pm.dpm_enabled = false;
77907791
else
77917792
adev->pm.dpm_enabled = true;
77927793
amdgpu_legacy_dpm_compute_clocks(adev);
7794+
mutex_unlock(&adev->pm.mutex);
77937795
return ret;
77947796
}
77957797

@@ -7807,32 +7809,44 @@ static int si_dpm_suspend(void *handle)
78077809
{
78087810
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
78097811

7812+
cancel_work_sync(&adev->pm.dpm.thermal.work);
7813+
78107814
if (adev->pm.dpm_enabled) {
7815+
mutex_lock(&adev->pm.mutex);
7816+
adev->pm.dpm_enabled = false;
78117817
/* disable dpm */
78127818
si_dpm_disable(adev);
78137819
/* reset the power state */
78147820
adev->pm.dpm.current_ps = adev->pm.dpm.requested_ps = adev->pm.dpm.boot_ps;
7821+
mutex_unlock(&adev->pm.mutex);
78157822
}
7823+
78167824
return 0;
78177825
}
78187826

78197827
static int si_dpm_resume(void *handle)
78207828
{
7821-
int ret;
7829+
int ret = 0;
78227830
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
78237831

7824-
if (adev->pm.dpm_enabled) {
7832+
if (!amdgpu_dpm)
7833+
return 0;
7834+
7835+
if (!adev->pm.dpm_enabled) {
78257836
/* asic init will reset to the boot state */
7837+
mutex_lock(&adev->pm.mutex);
78267838
si_dpm_setup_asic(adev);
78277839
ret = si_dpm_enable(adev);
7828-
if (ret)
7840+
if (ret) {
78297841
adev->pm.dpm_enabled = false;
7830-
else
7842+
} else {
78317843
adev->pm.dpm_enabled = true;
7832-
if (adev->pm.dpm_enabled)
78337844
amdgpu_legacy_dpm_compute_clocks(adev);
7845+
}
7846+
mutex_unlock(&adev->pm.mutex);
78347847
}
7835-
return 0;
7848+
7849+
return ret;
78367850
}
78377851

78387852
static bool si_dpm_is_idle(void *handle)

0 commit comments

Comments
 (0)