Skip to content

Commit b6eac41

Browse files
chr[]gregkh
authored andcommitted
amdgpu/pm/legacy: fix suspend/resume issues
commit 91dcc66b34beb72dde8412421bdc1b4cd40e4fb8 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 <[email protected]> Tested-by: Maciej S. Szmigiero <[email protected]> # on Oland PRO Signed-off-by: chr[] <[email protected]> Signed-off-by: Alex Deucher <[email protected]> (cherry picked from commit ee3dc9e204d271c9c7a8d4d38a0bce4745d33e71) Cc: [email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 070fda6 commit b6eac41

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
@@ -3056,13 +3056,16 @@ static int kv_dpm_hw_init(void *handle)
30563056
if (!amdgpu_dpm)
30573057
return 0;
30583058

3059+
mutex_lock(&adev->pm.mutex);
30593060
kv_dpm_setup_asic(adev);
30603061
ret = kv_dpm_enable(adev);
30613062
if (ret)
30623063
adev->pm.dpm_enabled = false;
30633064
else
30643065
adev->pm.dpm_enabled = true;
30653066
amdgpu_legacy_dpm_compute_clocks(adev);
3067+
mutex_unlock(&adev->pm.mutex);
3068+
30663069
return ret;
30673070
}
30683071

@@ -3080,32 +3083,42 @@ static int kv_dpm_suspend(void *handle)
30803083
{
30813084
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
30823085

3086+
cancel_work_sync(&adev->pm.dpm.thermal.work);
3087+
30833088
if (adev->pm.dpm_enabled) {
3089+
mutex_lock(&adev->pm.mutex);
3090+
adev->pm.dpm_enabled = false;
30843091
/* disable dpm */
30853092
kv_dpm_disable(adev);
30863093
/* reset the power state */
30873094
adev->pm.dpm.current_ps = adev->pm.dpm.requested_ps = adev->pm.dpm.boot_ps;
3095+
mutex_unlock(&adev->pm.mutex);
30883096
}
30893097
return 0;
30903098
}
30913099

30923100
static int kv_dpm_resume(void *handle)
30933101
{
3094-
int ret;
3102+
int ret = 0;
30953103
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
30963104

3097-
if (adev->pm.dpm_enabled) {
3105+
if (!amdgpu_dpm)
3106+
return 0;
3107+
3108+
if (!adev->pm.dpm_enabled) {
3109+
mutex_lock(&adev->pm.mutex);
30983110
/* asic init will reset to the boot state */
30993111
kv_dpm_setup_asic(adev);
31003112
ret = kv_dpm_enable(adev);
3101-
if (ret)
3113+
if (ret) {
31023114
adev->pm.dpm_enabled = false;
3103-
else
3115+
} else {
31043116
adev->pm.dpm_enabled = true;
3105-
if (adev->pm.dpm_enabled)
31063117
amdgpu_legacy_dpm_compute_clocks(adev);
3118+
}
3119+
mutex_unlock(&adev->pm.mutex);
31073120
}
3108-
return 0;
3121+
return ret;
31093122
}
31103123

31113124
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
@@ -1018,9 +1018,12 @@ void amdgpu_dpm_thermal_work_handler(struct work_struct *work)
10181018
enum amd_pm_state_type dpm_state = POWER_STATE_TYPE_INTERNAL_THERMAL;
10191019
int temp, size = sizeof(temp);
10201020

1021-
if (!adev->pm.dpm_enabled)
1022-
return;
1021+
mutex_lock(&adev->pm.mutex);
10231022

1023+
if (!adev->pm.dpm_enabled) {
1024+
mutex_unlock(&adev->pm.mutex);
1025+
return;
1026+
}
10241027
if (!pp_funcs->read_sensor(adev->powerplay.pp_handle,
10251028
AMDGPU_PP_SENSOR_GPU_TEMP,
10261029
(void *)&temp,
@@ -1042,4 +1045,5 @@ void amdgpu_dpm_thermal_work_handler(struct work_struct *work)
10421045
adev->pm.dpm.state = dpm_state;
10431046

10441047
amdgpu_legacy_dpm_compute_clocks(adev->powerplay.pp_handle);
1048+
mutex_unlock(&adev->pm.mutex);
10451049
}

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

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

7799+
mutex_lock(&adev->pm.mutex);
77997800
si_dpm_setup_asic(adev);
78007801
ret = si_dpm_enable(adev);
78017802
if (ret)
78027803
adev->pm.dpm_enabled = false;
78037804
else
78047805
adev->pm.dpm_enabled = true;
78057806
amdgpu_legacy_dpm_compute_clocks(adev);
7807+
mutex_unlock(&adev->pm.mutex);
78067808
return ret;
78077809
}
78087810

@@ -7820,32 +7822,44 @@ static int si_dpm_suspend(void *handle)
78207822
{
78217823
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
78227824

7825+
cancel_work_sync(&adev->pm.dpm.thermal.work);
7826+
78237827
if (adev->pm.dpm_enabled) {
7828+
mutex_lock(&adev->pm.mutex);
7829+
adev->pm.dpm_enabled = false;
78247830
/* disable dpm */
78257831
si_dpm_disable(adev);
78267832
/* reset the power state */
78277833
adev->pm.dpm.current_ps = adev->pm.dpm.requested_ps = adev->pm.dpm.boot_ps;
7834+
mutex_unlock(&adev->pm.mutex);
78287835
}
7836+
78297837
return 0;
78307838
}
78317839

78327840
static int si_dpm_resume(void *handle)
78337841
{
7834-
int ret;
7842+
int ret = 0;
78357843
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
78367844

7837-
if (adev->pm.dpm_enabled) {
7845+
if (!amdgpu_dpm)
7846+
return 0;
7847+
7848+
if (!adev->pm.dpm_enabled) {
78387849
/* asic init will reset to the boot state */
7850+
mutex_lock(&adev->pm.mutex);
78397851
si_dpm_setup_asic(adev);
78407852
ret = si_dpm_enable(adev);
7841-
if (ret)
7853+
if (ret) {
78427854
adev->pm.dpm_enabled = false;
7843-
else
7855+
} else {
78447856
adev->pm.dpm_enabled = true;
7845-
if (adev->pm.dpm_enabled)
78467857
amdgpu_legacy_dpm_compute_clocks(adev);
7858+
}
7859+
mutex_unlock(&adev->pm.mutex);
78477860
}
7848-
return 0;
7861+
7862+
return ret;
78497863
}
78507864

78517865
static bool si_dpm_is_idle(void *handle)

0 commit comments

Comments
 (0)