[PATCH] drm/amd/pm: fix Navi1x runtime resume failure V2
Quan, Evan
Evan.Quan at amd.com
Fri Mar 19 02:47:04 UTC 2021
[AMD Public Use]
Thanks Guchun & Jiansong. Yes, I had same concern as Jiansong.
BR
Evan
-----Original Message-----
From: Chen, Jiansong (Simon) <Jiansong.Chen at amd.com>
Sent: Thursday, March 18, 2021 5:36 PM
To: Chen, Guchun <Guchun.Chen at amd.com>; Quan, Evan <Evan.Quan at amd.com>; amd-gfx at lists.freedesktop.org
Cc: Lazar, Lijo <Lijo.Lazar at amd.com>; Quan, Evan <Evan.Quan at amd.com>
Subject: RE: [PATCH] drm/amd/pm: fix Navi1x runtime resume failure V2
We still need reserve "return 0", otherwise may trigger warning "not all control paths return a value".
Regards,
Jiansong
-----Original Message-----
From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Chen, Guchun
Sent: Thursday, March 18, 2021 5:28 PM
To: Quan, Evan <Evan.Quan at amd.com>; amd-gfx at lists.freedesktop.org
Cc: Lazar, Lijo <Lijo.Lazar at amd.com>; Quan, Evan <Evan.Quan at amd.com>
Subject: RE: [PATCH] drm/amd/pm: fix Navi1x runtime resume failure V2
[AMD Public Use]
One comment inline. Other than this, the patch is:
Reviewed-by: Guchun Chen <guchun.chen at amd.com>
Regards,
Guchun
-----Original Message-----
From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Evan Quan
Sent: Thursday, March 18, 2021 5:21 PM
To: amd-gfx at lists.freedesktop.org
Cc: Lazar, Lijo <Lijo.Lazar at amd.com>; Quan, Evan <Evan.Quan at amd.com>
Subject: [PATCH] drm/amd/pm: fix Navi1x runtime resume failure V2
The RLC was put into a wrong state on runtime suspend. Thus the RLC autoload will fail on the succeeding runtime resume. By adding an intermediate PPSMC_MSG_PrepareMp1ForUnload(some GC hard reset involved, designed for PnP), we can bring RLC back into the desired state.
V2: integrate INTERRUPTS_ENABLED flag clearing into current
mp1 state set routines
Change-Id: I4eb3d5d76068412a6ab228af7fe7f794e53c1eaa
Signed-off-by: Evan Quan <evan.quan at amd.com>
Reviewed-by: Lijo Lazar <lijo.lazar at amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 9 ++++--
drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 7 +++++
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 28 +++----------------
.../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 1 +
.../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 25 +++++++++++++++++
.../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 14 ++++++++++
.../drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 14 ++++++++++
drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c | 28 +++++++++++++++++++
drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h | 3 ++
9 files changed, 102 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 809f4cb738f4..ab6f4059630c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -2160,9 +2160,12 @@ static int psp_load_smu_fw(struct psp_context *psp)
if (!ucode->fw || amdgpu_sriov_vf(psp->adev))
return 0;
-
- if (amdgpu_in_reset(adev) && ras && ras->supported &&
- adev->asic_type == CHIP_ARCTURUS) {
+ if ((amdgpu_in_reset(adev) &&
+ ras && ras->supported &&
+ adev->asic_type == CHIP_ARCTURUS) ||
+ (adev->in_runpm &&
+ adev->asic_type >= CHIP_NAVI10 &&
+ adev->asic_type <= CHIP_NAVI12)) {
ret = amdgpu_dpm_set_mp1_state(adev, PP_MP1_STATE_UNLOAD);
if (ret) {
DRM_WARN("Failed to set MP1 state prepare for reload\n"); diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index 629a988b069d..21c3c149614c 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -836,6 +836,13 @@ struct pptable_funcs {
*/
int (*check_fw_status)(struct smu_context *smu);
+ /**
+ * @set_mp1_state: put SMU into a correct state for comming
+ * resume from runpm or gpu reset.
+ */
+ int (*set_mp1_state)(struct smu_context *smu,
+ enum pp_mp1_state mp1_state);
+
/**
* @setup_pptable: Initialize the power play table and populate it with
* default values.
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index bae9016fedea..470ca4e5d4d7 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -1927,36 +1927,16 @@ int smu_set_mp1_state(void *handle,
enum pp_mp1_state mp1_state)
{
struct smu_context *smu = handle;
- uint16_t msg;
- int ret;
+ int ret = 0;
if (!smu->pm_enabled)
return -EOPNOTSUPP;
mutex_lock(&smu->mutex);
- switch (mp1_state) {
- case PP_MP1_STATE_SHUTDOWN:
- msg = SMU_MSG_PrepareMp1ForShutdown;
- break;
- case PP_MP1_STATE_UNLOAD:
- msg = SMU_MSG_PrepareMp1ForUnload;
- break;
- case PP_MP1_STATE_RESET:
- msg = SMU_MSG_PrepareMp1ForReset;
- break;
- case PP_MP1_STATE_NONE:
- default:
- mutex_unlock(&smu->mutex);
- return 0;
- }
-
- ret = smu_send_smc_msg(smu, msg, NULL);
- /* some asics may not support those messages */
- if (ret == -EINVAL)
- ret = 0;
- if (ret)
- dev_err(smu->adev->dev, "[PrepareMp1] Failed!\n");
+ if (smu->ppt_funcs &&
+ smu->ppt_funcs->set_mp1_state)
+ ret = smu->ppt_funcs->set_mp1_state(smu, mp1_state);
mutex_unlock(&smu->mutex);
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
index 69aa60a2e8b7..e033aa6c7d9b 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
@@ -2367,6 +2367,7 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
.get_fan_parameters = arcturus_get_fan_parameters,
.interrupt_work = smu_v11_0_interrupt_work,
.set_light_sbr = smu_v11_0_set_light_sbr,
+ .set_mp1_state = smu_cmn_set_mp1_state,
};
void arcturus_set_ppt_funcs(struct smu_context *smu) diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index 88634b44f8ff..bd95d41fe7a9 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -431,6 +431,30 @@ static int navi10_store_powerplay_table(struct smu_context *smu)
return 0;
}
+static int navi10_set_mp1_state(struct smu_context *smu,
+ enum pp_mp1_state mp1_state)
+{
+ struct amdgpu_device *adev = smu->adev;
+ uint32_t mp1_fw_flags;
+ int ret = 0;
+
+ ret = smu_cmn_set_mp1_state(smu, mp1_state);
+ if (ret)
+ return ret;
+
+ if (mp1_state == PP_MP1_STATE_UNLOAD) {
+ mp1_fw_flags = RREG32_PCIE(MP1_Public |
+ (smnMP1_FIRMWARE_FLAGS & 0xffffffff));
+
+ mp1_fw_flags &= ~MP1_FIRMWARE_FLAGS__INTERRUPTS_ENABLED_MASK;
+
+ WREG32_PCIE(MP1_Public |
+ (smnMP1_FIRMWARE_FLAGS & 0xffffffff), mp1_fw_flags);
+ }
+
+ return 0;
+}
+
static int navi10_setup_pptable(struct smu_context *smu) {
int ret = 0;
@@ -3036,6 +3060,7 @@ static const struct pptable_funcs navi10_ppt_funcs = {
.post_init = navi10_post_smu_init,
.interrupt_work = smu_v11_0_interrupt_work,
.get_gfx_off_status = smu_v11_0_get_gfxoff_status,
+ .set_mp1_state = navi10_set_mp1_state,
};
void navi10_set_ppt_funcs(struct smu_context *smu) diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index 00575d452f08..5a5771785e10 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -3281,6 +3281,19 @@ static int sienna_cichlid_system_features_control(struct smu_context *smu,
return smu_v11_0_system_features_control(smu, en); }
+static int sienna_cichlid_set_mp1_state(struct smu_context *smu,
+ enum pp_mp1_state mp1_state)
+{
+ switch (mp1_state) {
+ case PP_MP1_STATE_UNLOAD:
+ return smu_cmn_set_mp1_state(smu, mp1_state);
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
[Guchun] return 0 looks uesless. Same as other similar functions.
+}
+
static const struct pptable_funcs sienna_cichlid_ppt_funcs = {
.get_allowed_feature_mask = sienna_cichlid_get_allowed_feature_mask,
.set_default_dpm_table = sienna_cichlid_set_default_dpm_table,
@@ -3369,6 +3382,7 @@ static const struct pptable_funcs sienna_cichlid_ppt_funcs = {
.interrupt_work = smu_v11_0_interrupt_work,
.gpo_control = sienna_cichlid_gpo_control,
.get_gfx_off_status = smu_v11_0_get_gfxoff_status,
+ .set_mp1_state = sienna_cichlid_set_mp1_state,
};
void sienna_cichlid_set_ppt_funcs(struct smu_context *smu) diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
index 9813a86ca31a..7d38b92a78dc 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
@@ -1460,6 +1460,19 @@ static bool aldebaran_is_mode2_reset_supported(struct smu_context *smu)
return true;
}
+static int aldebaran_set_mp1_state(struct smu_context *smu,
+ enum pp_mp1_state mp1_state)
+{
+ switch (mp1_state) {
+ case PP_MP1_STATE_UNLOAD:
+ return smu_cmn_set_mp1_state(smu, mp1_state);
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static const struct pptable_funcs aldebaran_ppt_funcs = {
/* init dpm */
.get_allowed_feature_mask = aldebaran_get_allowed_feature_mask,
@@ -1518,6 +1531,7 @@ static const struct pptable_funcs aldebaran_ppt_funcs = {
.mode2_reset_is_support = aldebaran_is_mode2_reset_supported,
.mode1_reset = smu_v13_0_mode1_reset,
.mode2_reset = smu_v13_0_mode2_reset,
+ .set_mp1_state = aldebaran_set_mp1_state,
};
void aldebaran_set_ppt_funcs(struct smu_context *smu) diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
index 917f8afbcee0..d423315aa2b3 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
@@ -782,3 +782,31 @@ void smu_cmn_init_soft_gpu_metrics(void *table, uint8_t frev, uint8_t crev)
header->structure_size = structure_size;
}
+
+int smu_cmn_set_mp1_state(struct smu_context *smu,
+ enum pp_mp1_state mp1_state)
+{
+ enum smu_message_type msg;
+ int ret;
+
+ switch (mp1_state) {
+ case PP_MP1_STATE_SHUTDOWN:
+ msg = SMU_MSG_PrepareMp1ForShutdown;
+ break;
+ case PP_MP1_STATE_UNLOAD:
+ msg = SMU_MSG_PrepareMp1ForUnload;
+ break;
+ case PP_MP1_STATE_RESET:
+ msg = SMU_MSG_PrepareMp1ForReset;
+ break;
+ case PP_MP1_STATE_NONE:
+ default:
+ return 0;
+ }
+
+ ret = smu_cmn_send_smc_msg(smu, msg, NULL);
+ if (ret)
+ dev_err(smu->adev->dev, "[PrepareMp1] Failed!\n");
+
+ return ret;
+}
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
index c69250185575..155e2a68fa1c 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
@@ -99,5 +99,8 @@ int smu_cmn_get_metrics_table(struct smu_context *smu,
void smu_cmn_init_soft_gpu_metrics(void *table, uint8_t frev, uint8_t crev);
+int smu_cmn_set_mp1_state(struct smu_context *smu,
+ enum pp_mp1_state mp1_state);
+
#endif
#endif
--
2.29.0
_______________________________________________
amd-gfx mailing list
amd-gfx at lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7CJiansong.Chen%40amd.com%7Ceed70d2cf21147324beb08d8e9f01a90%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637516564768126914%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=%2B9aXu324Wqr%2FE%2B4FUk57g%2FtTY%2F%2F9EzGKvSJ3vezKEl0%3D&reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx at lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7CJiansong.Chen%40amd.com%7Ceed70d2cf21147324beb08d8e9f01a90%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637516564768126914%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=%2B9aXu324Wqr%2FE%2B4FUk57g%2FtTY%2F%2F9EzGKvSJ3vezKEl0%3D&reserved=0
More information about the amd-gfx
mailing list