Skip to content

Commit 7be1587

Browse files
chr[]opsiff
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> (cherry picked from commit c52d6aa)
1 parent ceda819 commit 7be1587

3 files changed

Lines changed: 45 additions & 14 deletions

File tree

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
@@ -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
@@ -7789,13 +7789,15 @@ static int si_dpm_hw_init(void *handle)
77897789
if (!amdgpu_dpm)
77907790
return 0;
77917791

7792+
mutex_lock(&adev->pm.mutex);
77927793
si_dpm_setup_asic(adev);
77937794
ret = si_dpm_enable(adev);
77947795
if (ret)
77957796
adev->pm.dpm_enabled = false;
77967797
else
77977798
adev->pm.dpm_enabled = true;
77987799
amdgpu_legacy_dpm_compute_clocks(adev);
7800+
mutex_unlock(&adev->pm.mutex);
77997801
return ret;
78007802
}
78017803

@@ -7813,32 +7815,44 @@ static int si_dpm_suspend(void *handle)
78137815
{
78147816
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
78157817

7818+
cancel_work_sync(&adev->pm.dpm.thermal.work);
7819+
78167820
if (adev->pm.dpm_enabled) {
7821+
mutex_lock(&adev->pm.mutex);
7822+
adev->pm.dpm_enabled = false;
78177823
/* disable dpm */
78187824
si_dpm_disable(adev);
78197825
/* reset the power state */
78207826
adev->pm.dpm.current_ps = adev->pm.dpm.requested_ps = adev->pm.dpm.boot_ps;
7827+
mutex_unlock(&adev->pm.mutex);
78217828
}
7829+
78227830
return 0;
78237831
}
78247832

78257833
static int si_dpm_resume(void *handle)
78267834
{
7827-
int ret;
7835+
int ret = 0;
78287836
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
78297837

7830-
if (adev->pm.dpm_enabled) {
7838+
if (!amdgpu_dpm)
7839+
return 0;
7840+
7841+
if (!adev->pm.dpm_enabled) {
78317842
/* asic init will reset to the boot state */
7843+
mutex_lock(&adev->pm.mutex);
78327844
si_dpm_setup_asic(adev);
78337845
ret = si_dpm_enable(adev);
7834-
if (ret)
7846+
if (ret) {
78357847
adev->pm.dpm_enabled = false;
7836-
else
7848+
} else {
78377849
adev->pm.dpm_enabled = true;
7838-
if (adev->pm.dpm_enabled)
78397850
amdgpu_legacy_dpm_compute_clocks(adev);
7851+
}
7852+
mutex_unlock(&adev->pm.mutex);
78407853
}
7841-
return 0;
7854+
7855+
return ret;
78427856
}
78437857

78447858
static bool si_dpm_is_idle(void *handle)

0 commit comments

Comments
 (0)