before this patch, we have 4 apis to manage smu feature bits. the patch add a new one in them, but it is not add any feature in smu. before your patch: smu_feature_is_enabled and smu_feature_set_enabled is pair of functions, after your patch: smu_feature_is_enabled and smu_enable_smc_feature is pair of functions; so the driver don't have scenario needs to use smu_feature_set_enabeld, do you agree it? most of the time we update SMC feature, we must update software bitmap in smu_feature structure. if not, the smu_feature_is_enabled funciton is not work well. extern int smu_feature_init_dpm(struct smu_context *smu); extern int smu_feature_is_enabled(struct smu_context *smu, enum smu_feature_mask mask); extern int smu_feature_set_enabled(struct smu_context *smu, enum smu_feature_mask mask, bool enable); extern int smu_feature_is_supported(struct smu_context *smu, enum smu_feature_mask mask); extern int smu_feature_set_supported(struct smu_context *smu, enum smu_feature_mask mask, bool enable); Best Regards, Kevin ________________________________ From: amd-gfx on behalf of Evan Quan Sent: Monday, July 22, 2019 2:34:26 PM To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Cc: Quan, Evan Subject: [PATCH] drm/amd/powerplay: correct confusing naming for smu_feature_set_enabled It does more than updating the bitmask. In fact it enables also the feature. That's confusing. As for this, a new API is added for the feature enablement job. And smu_feature_set_enabled is updated to setting the bitmask only(as smu_feature_set_supported). Change-Id: I758e4461be34c0fcbdf19448e34180a5251926c4 Signed-off-by: Evan Quan --- drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 29 +++++++++++++------ .../gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 2 ++ drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 2 +- drivers/gpu/drm/amd/powerplay/vega20_ppt.c | 4 +-- 4 files changed, 25 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c index 4e18f33a1bab..9262883d4031 100644 --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c @@ -511,28 +511,39 @@ int smu_feature_set_enabled(struct smu_context *smu, enum smu_feature_mask mask, { struct smu_feature *feature = &smu->smu_feature; int feature_id; - int ret = 0; feature_id = smu_feature_get_index(smu, mask); if (feature_id < 0) return -EINVAL; - WARN_ON(feature_id > feature->feature_num); - mutex_lock(&feature->mutex); - ret = smu_feature_update_enable_state(smu, feature_id, enable); - if (ret) - goto failed; if (enable) test_and_set_bit(feature_id, feature->enabled); else test_and_clear_bit(feature_id, feature->enabled); -failed: mutex_unlock(&feature->mutex); - return ret; + return 0; +} + +int smu_enable_smc_feature(struct smu_context *smu, + enum smu_feature_mask mask, + bool enable) +{ + int feature_id; + int ret = 0; + + feature_id = smu_feature_get_index(smu, mask); + if (feature_id < 0) + return -EINVAL; + + ret = smu_feature_update_enable_state(smu, feature_id, enable); + if (ret) + return ret; + + return smu_feature_set_enabled(smu, mask, enable); } int smu_feature_is_supported(struct smu_context *smu, enum smu_feature_mask mask) @@ -1186,7 +1197,7 @@ static int smu_suspend(void *handle) return ret; if (adev->in_gpu_reset && baco_feature_is_enabled) { - ret = smu_feature_set_enabled(smu, SMU_FEATURE_BACO_BIT, true); + ret = smu_enable_smc_feature(smu, SMU_FEATURE_BACO_BIT, true); if (ret) { pr_warn("set BACO feature enabled failed, return %d\n", ret); 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 b702c9ee975f..267b879796f9 100644 --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h @@ -946,6 +946,8 @@ extern int smu_feature_is_enabled(struct smu_context *smu, enum smu_feature_mask mask); extern int smu_feature_set_enabled(struct smu_context *smu, enum smu_feature_mask mask, bool enable); +extern int smu_enable_smc_feature(struct smu_context *smu, + enum smu_feature_mask mask, bool enable); extern int smu_feature_is_supported(struct smu_context *smu, enum smu_feature_mask mask); extern int smu_feature_set_supported(struct smu_context *smu, diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c index e3a178403546..0f59d2178d01 100644 --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c @@ -1419,7 +1419,7 @@ smu_v11_0_smc_fan_control(struct smu_context *smu, bool start) if (smu_feature_is_supported(smu, SMU_FEATURE_FAN_CONTROL_BIT)) return 0; - ret = smu_feature_set_enabled(smu, SMU_FEATURE_FAN_CONTROL_BIT, start); + ret = smu_enable_smc_feature(smu, SMU_FEATURE_FAN_CONTROL_BIT, start); if (ret) pr_err("[%s]%s smc FAN CONTROL feature failed!", __func__, (start ? "Start" : "Stop")); diff --git a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c index 9ead36192787..536ff884ddca 100644 --- a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c +++ b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c @@ -2845,7 +2845,7 @@ static int vega20_dpm_set_uvd_enable(struct smu_context *smu, bool enable) if (enable == smu_feature_is_enabled(smu, SMU_FEATURE_DPM_UVD_BIT)) return 0; - return smu_feature_set_enabled(smu, SMU_FEATURE_DPM_UVD_BIT, enable); + return smu_enable_smc_feature(smu, SMU_FEATURE_DPM_UVD_BIT, enable); } static int vega20_dpm_set_vce_enable(struct smu_context *smu, bool enable) @@ -2856,7 +2856,7 @@ static int vega20_dpm_set_vce_enable(struct smu_context *smu, bool enable) if (enable == smu_feature_is_enabled(smu, SMU_FEATURE_DPM_VCE_BIT)) return 0; - return smu_feature_set_enabled(smu, SMU_FEATURE_DPM_VCE_BIT, enable); + return smu_enable_smc_feature(smu, SMU_FEATURE_DPM_VCE_BIT, enable); } static int vega20_get_enabled_smc_features(struct smu_context *smu, -- 2.22.0 _______________________________________________ amd-gfx mailing list amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx