Hi Kenneth, I don't think there is sequence dependency between disabling power feature and setting specific workload, correct? If so, we can move the enabling/disabling power feature logic out of the "out" goto tag.
Secondly, It seems to me the new logical will result to duplicate power features enablement if profile mode is not PROFILE_COMPUTE, will that any side effect? Since KFD has a flag to indicate enter/exit PROFILE_COMPUTE mode in amdgpu_amdkfd_set_compute_idle, would it make sense to apply power feature disablement in pp_dpm_switch_power_profile? of course, we need to create a call back function like pp_dpm_disable_power_feature_for_compute_performce (and also the equivalent one for the new smu driver). Let me know your insights on these. Thanks. Regards, Hawking -----Original Message----- From: amd-gfx <[email protected]> On Behalf Of Kenneth Feng Sent: 2019年11月7日 16:59 To: [email protected] Cc: Feng, Kenneth <[email protected]> Subject: [PATCH] drm/amd/powerplay: disable ds and ulv for compute This is to improve the performance in the compute mode for vega10. For example, the original performance for a rocm bandwidth test: 2G internal GPU copy, is about 99GB/s. With the idle power features disabled dynamically, the porformance is promoted to about 215GB/s. Signed-off-by: Kenneth Feng <[email protected]> --- drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 62 +++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c index 4ea63a2..c0dbb26 100644 --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c @@ -4940,6 +4940,59 @@ static int vega10_get_power_profile_mode(struct pp_hwmgr *hwmgr, char *buf) return size; } +static int vega10_disable_power_features_for_compute_performance(struct +pp_hwmgr *hwmgr, bool disable) { + struct vega10_hwmgr *data = hwmgr->backend; + uint32_t feature_mask = 0; + + if (disable) { + feature_mask |= data->smu_features[GNLD_ULV].enabled ? + data->smu_features[GNLD_ULV].smu_feature_bitmap : 0; + feature_mask |= data->smu_features[GNLD_DS_GFXCLK].enabled ? + data->smu_features[GNLD_DS_GFXCLK].smu_feature_bitmap : 0; + feature_mask |= data->smu_features[GNLD_DS_SOCCLK].enabled ? + data->smu_features[GNLD_DS_SOCCLK].smu_feature_bitmap : 0; + feature_mask |= data->smu_features[GNLD_DS_LCLK].enabled ? + data->smu_features[GNLD_DS_LCLK].smu_feature_bitmap : 0; + feature_mask |= data->smu_features[GNLD_DS_DCEFCLK].enabled ? + data->smu_features[GNLD_DS_DCEFCLK].smu_feature_bitmap : 0; + } else { + feature_mask |= (!data->smu_features[GNLD_ULV].enabled) ? + data->smu_features[GNLD_ULV].smu_feature_bitmap : 0; + feature_mask |= (!data->smu_features[GNLD_DS_GFXCLK].enabled) ? + data->smu_features[GNLD_DS_GFXCLK].smu_feature_bitmap : 0; + feature_mask |= (!data->smu_features[GNLD_DS_SOCCLK].enabled) ? + data->smu_features[GNLD_DS_SOCCLK].smu_feature_bitmap : 0; + feature_mask |= (!data->smu_features[GNLD_DS_LCLK].enabled) ? + data->smu_features[GNLD_DS_LCLK].smu_feature_bitmap : 0; + feature_mask |= (!data->smu_features[GNLD_DS_DCEFCLK].enabled) ? + data->smu_features[GNLD_DS_DCEFCLK].smu_feature_bitmap : 0; + } + + if (feature_mask) + PP_ASSERT_WITH_CODE(!vega10_enable_smc_features(hwmgr, + !disable, feature_mask), + "enable/disable power features for compute performance Failed!", + return -EINVAL); + + if (disable) { + data->smu_features[GNLD_ULV].enabled = false; + data->smu_features[GNLD_DS_GFXCLK].enabled = false; + data->smu_features[GNLD_DS_SOCCLK].enabled = false; + data->smu_features[GNLD_DS_LCLK].enabled = false; + data->smu_features[GNLD_DS_DCEFCLK].enabled = false; + } else { + data->smu_features[GNLD_ULV].enabled = true; + data->smu_features[GNLD_DS_GFXCLK].enabled = true; + data->smu_features[GNLD_DS_SOCCLK].enabled = true; + data->smu_features[GNLD_DS_LCLK].enabled = true; + data->smu_features[GNLD_DS_DCEFCLK].enabled = true; + } + + return 0; + +} + static int vega10_set_power_profile_mode(struct pp_hwmgr *hwmgr, long *input, uint32_t size) { struct vega10_hwmgr *data = hwmgr->backend; @@ -4948,6 +5001,7 @@ static int vega10_set_power_profile_mode(struct pp_hwmgr *hwmgr, long *input, ui uint8_t use_rlc_busy; uint8_t min_active_level; uint32_t power_profile_mode = input[size]; + int ret = 0; if (power_profile_mode == PP_SMC_POWER_PROFILE_CUSTOM) { if (size != 0 && size != 4) @@ -4975,11 +5029,17 @@ static int vega10_set_power_profile_mode(struct pp_hwmgr *hwmgr, long *input, ui } out: + if (power_profile_mode == PP_SMC_POWER_PROFILE_COMPUTE) + ret = vega10_disable_power_features_for_compute_performance(hwmgr, true); + else + ret = vega10_disable_power_features_for_compute_performance(hwmgr, +false); + smum_send_msg_to_smc_with_parameter(hwmgr, PPSMC_MSG_SetWorkloadMask, 1 << power_profile_mode); + hwmgr->power_profile_mode = power_profile_mode; - return 0; + return ret; } -- 2.7.4 _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
