[PATCH 2/2] drm/amd/powerplay: add sensor lock support for smu
Deucher, Alexander
Alexander.Deucher at amd.com
Thu Sep 26 15:06:18 UTC 2019
Does the older powerplay code need a similar fix?
Alex
________________________________
From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> on behalf of Wang, Kevin(Yang) <Kevin1.Wang at amd.com>
Sent: Thursday, September 26, 2019 9:06 AM
To: Quan, Evan <Evan.Quan at amd.com>; amd-gfx at lists.freedesktop.org <amd-gfx at lists.freedesktop.org>
Cc: Huang, Ray <Ray.Huang at amd.com>; Feng, Kenneth <Kenneth.Feng at amd.com>
Subject: Re: [PATCH 2/2] drm/amd/powerplay: add sensor lock support for smu
sure, you are right, the origin design should be add this lock into these functions,
but only add these functions can't fix this issue.
eg:
"watch -n 0 /sys/kenel/debug/dri0/amdgpu_pm_info"
16 threads run this command will cause smu error.
so this is workaound fix about sensor interface.
in fact, the smu driver need more lock to protect smu resource.
but too many locks can easily lead to deadlocks in smu driver.
solve the problem temporarily first and optimize this part later
1. Message + Param ==> message param lock
2. Message + Message Result ==> message result lock
3. Message1 + Message2 ==> message pair lock (eg: SetFeatureLow and SetFeatureHigh)
Best Regars,
Kevin
________________________________
From: Quan, Evan <Evan.Quan at amd.com>
Sent: Thursday, September 26, 2019 8:22 PM
To: Wang, Kevin(Yang) <Kevin1.Wang at amd.com>; amd-gfx at lists.freedesktop.org <amd-gfx at lists.freedesktop.org>
Cc: Huang, Ray <Ray.Huang at amd.com>; Feng, Kenneth <Kenneth.Feng at amd.com>; Wang, Kevin(Yang) <Kevin1.Wang at amd.com>
Subject: RE: [PATCH 2/2] drm/amd/powerplay: add sensor lock support for smu
How about adding the mutex protection in smu_v11_0_send_msg_with_param and smu_v11_0_send_msg?
That seems able to simplify the code.
-----Original Message-----
From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Wang, Kevin(Yang)
Sent: Thursday, September 26, 2019 4:56 PM
To: amd-gfx at lists.freedesktop.org
Cc: Huang, Ray <Ray.Huang at amd.com>; Feng, Kenneth <Kenneth.Feng at amd.com>; Wang, Kevin(Yang) <Kevin1.Wang at amd.com>
Subject: [PATCH 2/2] drm/amd/powerplay: add sensor lock support for smu
when multithreading access sysfs of amdgpu_pm_info at the sametime.
the swsmu driver cause smu firmware hang.
eg:
single thread access:
Message A + Param A ==> right
Message B + Param B ==> right
Message C + Param C ==> right
multithreading access:
Message A + Param B ==> error
Message B + Param A ==> error
Message C + Param C ==> right
the patch will add sensor lock(mutex) to avoid this error.
Signed-off-by: Kevin Wang <kevin1.wang at amd.com>
---
drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 2 ++
drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 2 ++
drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 1 +
drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 2 ++
drivers/gpu/drm/amd/powerplay/vega20_ppt.c | 2 ++
5 files changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 23293e15636b..df510cb86da5 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -840,6 +840,8 @@ static int smu_sw_init(void *handle)
smu->smu_baco.state = SMU_BACO_STATE_EXIT;
smu->smu_baco.platform_support = false;
+ mutex_init(&smu->sensor_lock);
+
smu->watermarks_bitmap = 0;
smu->power_profile_mode = PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;
smu->default_power_profile_mode = PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;
diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index a047a7ec3698..b9b7b73354a0 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -1025,6 +1025,7 @@ static int arcturus_read_sensor(struct smu_context *smu,
if (!data || !size)
return -EINVAL;
+ mutex_lock(&smu->sensor_lock);
switch (sensor) {
case AMDGPU_PP_SENSOR_MAX_FAN_RPM:
*(uint32_t *)data = pptable->FanMaximumRpm; @@ -1051,6 +1052,7 @@ static int arcturus_read_sensor(struct smu_context *smu,
default:
ret = smu_smc_read_sensor(smu, sensor, data, size);
}
+ mutex_unlock(&smu->sensor_lock);
return ret;
}
diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
index 5c898444ff97..bc4b73e0718e 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
@@ -350,6 +350,7 @@ struct smu_context
const struct smu_funcs *funcs;
const struct pptable_funcs *ppt_funcs;
struct mutex mutex;
+ struct mutex sensor_lock;
uint64_t pool_size;
struct smu_table_context smu_table;
diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
index db2e181ba45a..c0b640d8d9e1 100644
--- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
@@ -1387,6 +1387,7 @@ static int navi10_read_sensor(struct smu_context *smu,
if(!data || !size)
return -EINVAL;
+ mutex_lock(&smu->sensor_lock);
switch (sensor) {
case AMDGPU_PP_SENSOR_MAX_FAN_RPM:
*(uint32_t *)data = pptable->FanMaximumRpm; @@ -1410,6 +1411,7 @@ static int navi10_read_sensor(struct smu_context *smu,
default:
ret = smu_smc_read_sensor(smu, sensor, data, size);
}
+ mutex_unlock(&smu->sensor_lock);
return ret;
}
diff --git a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
index 9082da1578d1..f655ebd9ba22 100644
--- a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
@@ -3017,6 +3017,7 @@ static int vega20_read_sensor(struct smu_context *smu,
if(!data || !size)
return -EINVAL;
+ mutex_lock(&smu->sensor_lock);
switch (sensor) {
case AMDGPU_PP_SENSOR_MAX_FAN_RPM:
*(uint32_t *)data = pptable->FanMaximumRpm; @@ -3042,6 +3043,7 @@ static int vega20_read_sensor(struct smu_context *smu,
default:
ret = smu_smc_read_sensor(smu, sensor, data, size);
}
+ mutex_unlock(&smu->sensor_lock);
return ret;
}
--
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx at lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20190926/6adfa359/attachment.html>
More information about the amd-gfx
mailing list