RE: [PATCH 1/2] drm/amd/powerplay: Enable "disable dpm" feature to support swSMU debug
Reviewed-by: Hawking Zhang Regards, Hawking _ From: Gui, Jack Sent: 2019年5月13日 11:32 To: Zhang, Hawking ; amd-gfx@lists.freedesktop.org Subject: RE: [PATCH 1/2] drm/amd/powerplay: Enable "disable dpm" feature to support swSMU debug << File: 0001-drm-amd-powerplay-Enable-disable-dpm-feature-to-supp.patch >> Hi Hawking, V2 patch attached: Return 0 directly and merge some check code. Please help review again, thanks. BR, Jack Gui _ From: Zhang, Hawking Sent: Sunday, May 12, 2019 11:18 AM To: Gui, Jack ; amd-gfx@lists.freedesktop.org Cc: Gui, Jack Subject: RE: [PATCH 1/2] drm/amd/powerplay: Enable "disable dpm" feature to support swSMU debug Please check my comments inline. Regards, Hawking -Original Message- From: amd-gfx mailto:amd-gfx-boun...@lists.freedesktop.org>> On Behalf Of Chengming Gui Sent: 2019年5月10日 16:49 To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> Cc: Gui, Jack mailto:jack@amd.com>> Subject: [PATCH 1/2] drm/amd/powerplay: Enable "disable dpm" feature to support swSMU debug [CAUTION: External Email] add pm_enabled to control the dpm off/on. Signed-off-by: Chengming Gui mailto:jack@amd.com>> --- drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 34 +++--- drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 1 + drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 34 ++ 3 files changed, 60 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c index 52d919a..99b2082 100644 --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c @@ -198,6 +198,8 @@ int smu_sys_set_pp_table(struct smu_context *smu, void *buf, size_t size) ATOM_COMMON_TABLE_HEADER *header = (ATOM_COMMON_TABLE_HEADER *)buf; int ret = 0; + if (!smu->pm_enabled) + return -EINVAL; if (header->usStructureSize != size) { pr_err("pp table size not matched !\n"); return -EIO; @@ -233,6 +235,8 @@ int smu_feature_init_dpm(struct smu_context *smu) int ret = 0; uint32_t unallowed_feature_mask[SMU_FEATURE_MAX/32]; + if (!smu->pm_enabled) + return ret; mutex_lock(&feature->mutex); bitmap_fill(feature->allowed, SMU_FEATURE_MAX); mutex_unlock(&feature->mutex); @@ -344,6 +348,7 @@ static int smu_early_init(void *handle) struct smu_context *smu = &adev->smu; smu->adev = adev; + smu->pm_enabled = amdgpu_dpm; mutex_init(&smu->mutex); return smu_set_funcs(adev); @@ -353,6 +358,9 @@ static int smu_late_init(void *handle) { struct amdgpu_device *adev = (struct amdgpu_device *)handle; struct smu_context *smu = &adev->smu; + + if (!smu->pm_enabled) + return 0; mutex_lock(&smu->mutex); smu_handle_task(&adev->smu, smu->smu_dpm.dpm_level, @@ -746,6 +754,9 @@ static int smu_smc_table_hw_init(struct smu_context *smu, */ ret = smu_set_tool_table_location(smu); + if (!smu_is_dpm_running(smu)) + pr_info("dpm has been disabled\n"); + return ret; } @@ -861,7 +872,10 @@ static int smu_hw_init(void *handle) mutex_unlock(&smu->mutex); - adev->pm.dpm_enabled = true; + if (!smu->pm_enabled) + adev->pm.dpm_enabled = false; + else + adev->pm.dpm_enabled = true; pr_info("SMU is initialized successfully!\n"); @@ -879,6 +893,8 @@ static int smu_hw_fini(void *handle) struct smu_table_context *table_context = &smu->smu_table; int ret = 0; + if (!smu->pm_enabled) + return ret; if (!is_support_sw_smu(adev)) return -EINVAL; @@ -932,10 +948,12 @@ int smu_reset(struct smu_context *smu) static int smu_suspend(void *handle) { - int ret; + int ret = 0; struct amdgpu_device *adev = (struct amdgpu_device *)handle; struct smu_context *smu = &adev->smu; + if (!smu->pm_enabled) + return ret; [Hawking]: Just return 0 directly if dpm was disabled. Don't change the default return value to 0 which means resume complete successfully if (!is_support_sw_smu(adev)) return -EINVAL; @@ -950,10 +968,12 @@ static int smu_suspend(void *handle) static int smu_resume(void *handle) { - int ret; + int ret = 0; struct amdgpu_device *adev = (struct amdgpu_device *)handle; struct smu_context *smu = &adev->smu; + if (!smu->pm_enabled) +
RE: [PATCH 1/2] drm/amd/powerplay: Enable "disable dpm" feature to support swSMU debug
Hi Hawking, V2 patch attached: Return 0 directly and merge some check code. Please help review again, thanks. BR, Jack Gui _ From: Zhang, Hawking Sent: Sunday, May 12, 2019 11:18 AM To: Gui, Jack ; amd-gfx@lists.freedesktop.org Cc: Gui, Jack Subject: RE: [PATCH 1/2] drm/amd/powerplay: Enable "disable dpm" feature to support swSMU debug Please check my comments inline. Regards, Hawking -Original Message- From: amd-gfx mailto:amd-gfx-boun...@lists.freedesktop.org>> On Behalf Of Chengming Gui Sent: 2019年5月10日 16:49 To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> Cc: Gui, Jack mailto:jack@amd.com>> Subject: [PATCH 1/2] drm/amd/powerplay: Enable "disable dpm" feature to support swSMU debug [CAUTION: External Email] add pm_enabled to control the dpm off/on. Signed-off-by: Chengming Gui mailto:jack@amd.com>> --- drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 34 +++--- drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 1 + drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 34 ++ 3 files changed, 60 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c index 52d919a..99b2082 100644 --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c @@ -198,6 +198,8 @@ int smu_sys_set_pp_table(struct smu_context *smu, void *buf, size_t size) ATOM_COMMON_TABLE_HEADER *header = (ATOM_COMMON_TABLE_HEADER *)buf; int ret = 0; + if (!smu->pm_enabled) + return -EINVAL; if (header->usStructureSize != size) { pr_err("pp table size not matched !\n"); return -EIO; @@ -233,6 +235,8 @@ int smu_feature_init_dpm(struct smu_context *smu) int ret = 0; uint32_t unallowed_feature_mask[SMU_FEATURE_MAX/32]; + if (!smu->pm_enabled) + return ret; mutex_lock(&feature->mutex); bitmap_fill(feature->allowed, SMU_FEATURE_MAX); mutex_unlock(&feature->mutex); @@ -344,6 +348,7 @@ static int smu_early_init(void *handle) struct smu_context *smu = &adev->smu; smu->adev = adev; + smu->pm_enabled = amdgpu_dpm; mutex_init(&smu->mutex); return smu_set_funcs(adev); @@ -353,6 +358,9 @@ static int smu_late_init(void *handle) { struct amdgpu_device *adev = (struct amdgpu_device *)handle; struct smu_context *smu = &adev->smu; + + if (!smu->pm_enabled) + return 0; mutex_lock(&smu->mutex); smu_handle_task(&adev->smu, smu->smu_dpm.dpm_level, @@ -746,6 +754,9 @@ static int smu_smc_table_hw_init(struct smu_context *smu, */ ret = smu_set_tool_table_location(smu); + if (!smu_is_dpm_running(smu)) + pr_info("dpm has been disabled\n"); + return ret; } @@ -861,7 +872,10 @@ static int smu_hw_init(void *handle) mutex_unlock(&smu->mutex); - adev->pm.dpm_enabled = true; + if (!smu->pm_enabled) + adev->pm.dpm_enabled = false; + else + adev->pm.dpm_enabled = true; pr_info("SMU is initialized successfully!\n"); @@ -879,6 +893,8 @@ static int smu_hw_fini(void *handle) struct smu_table_context *table_context = &smu->smu_table; int ret = 0; + if (!smu->pm_enabled) + return ret; if (!is_support_sw_smu(adev)) return -EINVAL; @@ -932,10 +948,12 @@ int smu_reset(struct smu_context *smu) static int smu_suspend(void *handle) { - int ret; + int ret = 0; struct amdgpu_device *adev = (struct amdgpu_device *)handle; struct smu_context *smu = &adev->smu; + if (!smu->pm_enabled) + return ret; [Hawking]: Just return 0 directly if dpm was disabled. Don't change the default return value to 0 which means resume complete successfully if (!is_support_sw_smu(adev)) return -EINVAL; @@ -950,10 +968,12 @@ static int smu_suspend(void *handle) static int smu_resume(void *handle) { - int ret; + int ret = 0; struct amdgpu_device *adev = (struct amdgpu_device *)handle; struct smu_context *smu = &adev->smu; + if (!smu->pm_enabled) + return ret; [Hawking]: similar as suspend, directly return 0 if dpm was disabled. if (!is_support_sw_smu(adev)) return -EINVAL; @@ -985,7 +1005,7 @@ int smu_display_configuration_change(struct smu_context *smu, int index = 0; int num_of_active_display = 0; - if (!is_support_sw_smu(smu->adev)) + if (!smu->p
RE: [PATCH 1/2] drm/amd/powerplay: Enable "disable dpm" feature to support swSMU debug
Please check my comments inline. Regards, Hawking -Original Message- From: amd-gfx On Behalf Of Chengming Gui Sent: 2019年5月10日 16:49 To: amd-gfx@lists.freedesktop.org Cc: Gui, Jack Subject: [PATCH 1/2] drm/amd/powerplay: Enable "disable dpm" feature to support swSMU debug [CAUTION: External Email] add pm_enabled to control the dpm off/on. Signed-off-by: Chengming Gui mailto:jack@amd.com>> --- drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 34 +++--- drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 1 + drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 34 ++ 3 files changed, 60 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c index 52d919a..99b2082 100644 --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c @@ -198,6 +198,8 @@ int smu_sys_set_pp_table(struct smu_context *smu, void *buf, size_t size) ATOM_COMMON_TABLE_HEADER *header = (ATOM_COMMON_TABLE_HEADER *)buf; int ret = 0; + if (!smu->pm_enabled) + return -EINVAL; if (header->usStructureSize != size) { pr_err("pp table size not matched !\n"); return -EIO; @@ -233,6 +235,8 @@ int smu_feature_init_dpm(struct smu_context *smu) int ret = 0; uint32_t unallowed_feature_mask[SMU_FEATURE_MAX/32]; + if (!smu->pm_enabled) + return ret; mutex_lock(&feature->mutex); bitmap_fill(feature->allowed, SMU_FEATURE_MAX); mutex_unlock(&feature->mutex); @@ -344,6 +348,7 @@ static int smu_early_init(void *handle) struct smu_context *smu = &adev->smu; smu->adev = adev; + smu->pm_enabled = amdgpu_dpm; mutex_init(&smu->mutex); return smu_set_funcs(adev); @@ -353,6 +358,9 @@ static int smu_late_init(void *handle) { struct amdgpu_device *adev = (struct amdgpu_device *)handle; struct smu_context *smu = &adev->smu; + + if (!smu->pm_enabled) + return 0; mutex_lock(&smu->mutex); smu_handle_task(&adev->smu, smu->smu_dpm.dpm_level, @@ -746,6 +754,9 @@ static int smu_smc_table_hw_init(struct smu_context *smu, */ ret = smu_set_tool_table_location(smu); + if (!smu_is_dpm_running(smu)) + pr_info("dpm has been disabled\n"); + return ret; } @@ -861,7 +872,10 @@ static int smu_hw_init(void *handle) mutex_unlock(&smu->mutex); - adev->pm.dpm_enabled = true; + if (!smu->pm_enabled) + adev->pm.dpm_enabled = false; + else + adev->pm.dpm_enabled = true; pr_info("SMU is initialized successfully!\n"); @@ -879,6 +893,8 @@ static int smu_hw_fini(void *handle) struct smu_table_context *table_context = &smu->smu_table; int ret = 0; + if (!smu->pm_enabled) + return ret; if (!is_support_sw_smu(adev)) return -EINVAL; @@ -932,10 +948,12 @@ int smu_reset(struct smu_context *smu) static int smu_suspend(void *handle) { - int ret; + int ret = 0; struct amdgpu_device *adev = (struct amdgpu_device *)handle; struct smu_context *smu = &adev->smu; + if (!smu->pm_enabled) + return ret; [Hawking]: Just return 0 directly if dpm was disabled. Don't change the default return value to 0 which means resume complete successfully if (!is_support_sw_smu(adev)) return -EINVAL; @@ -950,10 +968,12 @@ static int smu_suspend(void *handle) static int smu_resume(void *handle) { - int ret; + int ret = 0; struct amdgpu_device *adev = (struct amdgpu_device *)handle; struct smu_context *smu = &adev->smu; + if (!smu->pm_enabled) + return ret; [Hawking]: similar as suspend, directly return 0 if dpm was disabled. if (!is_support_sw_smu(adev)) return -EINVAL; @@ -985,7 +1005,7 @@ int smu_display_configuration_change(struct smu_context *smu, int index = 0; int num_of_active_display = 0; - if (!is_support_sw_smu(smu->adev)) + if (!smu->pm_enabled || !is_support_sw_smu(smu->adev)) return -EINVAL; if (!display_config) @@ -1113,6 +1133,8 @@ static int smu_enable_umd_pstate(void *handle, struct smu_context *smu = (struct smu_context*)(handle); struct smu_dpm_context *smu_dpm_ctx = &(smu->smu_dpm); + if (!smu->pm_enabled) + return -EINVAL; [Hawking]: please merge this with the dpm_context check if (!smu_dpm_ctx->dpm_context) return -EINVAL; @@ -1156,6 +1178,8 @@ int smu_adjust_power_state_dynamic(struct smu_context *smu, long workload; struct smu_dpm_context *smu_dpm_ctx = &(smu->smu_dpm); + if (!smu->pm_enabled) + return