On 10/23/2024 8:42 AM, Kenneth Feng wrote:
> Correct the workload setting in order not to mix the setting
> with the end user. Update the workload mask accordingly.
>
> Signed-off-by: Kenneth Feng <kenneth.f...@amd.com>
> ---
> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 17 ++++++--
> drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 4 +-
> .../drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 39 +++++++++++++++--
> .../drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 33 ++++++++++++---
> .../drm/amd/pm/swsmu/smu14/smu_v14_0_2_ppt.c | 42 ++++++++++++++++---
> 5 files changed, 115 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index accc96a03bd9..f1984736ff4a 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -1252,7 +1252,8 @@ static int smu_sw_init(struct amdgpu_ip_block *ip_block)
> atomic64_set(&smu->throttle_int_counter, 0);
> 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;
> + smu->user_profile_mode = PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;
> + smu->driver_workload_mask = 0;
>
> atomic_set(&smu->smu_power.power_gate.vcn_gated, 1);
> atomic_set(&smu->smu_power.power_gate.jpeg_gated, 1);
> @@ -1267,10 +1268,12 @@ static int smu_sw_init(struct amdgpu_ip_block
> *ip_block)
> smu->workload_prority[PP_SMC_POWER_PROFILE_COMPUTE] = 5;
> smu->workload_prority[PP_SMC_POWER_PROFILE_CUSTOM] = 6;
>
> - if (smu->is_apu)
> + if (smu->is_apu) {
> smu->workload_mask = 1 <<
> smu->workload_prority[PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT];
> - else
> + } else {
> smu->workload_mask = 1 <<
> smu->workload_prority[PP_SMC_POWER_PROFILE_FULLSCREEN3D];
> + smu->user_profile_mode = PP_SMC_POWER_PROFILE_FULLSCREEN3D;
This is driver decided, not user decided. Keeping it as user decided is
confusing.
Also, ideally user settings is preferred to be kept here -
smu_user_dpm_profile
> + }
>
> smu->workload_setting[0] = PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;
> smu->workload_setting[1] = PP_SMC_POWER_PROFILE_FULLSCREEN3D;
> @@ -2346,11 +2349,13 @@ static int smu_switch_power_profile(void *handle,
>
> if (!en) {
> smu->workload_mask &= ~(1 << smu->workload_prority[type]);
> + smu->driver_workload_mask &= ~(1 <<
> smu->workload_prority[type]);
> index = fls(smu->workload_mask);
> index = index > 0 && index <= WORKLOAD_POLICY_MAX ? index - 1 :
> 0;
> workload[0] = smu->workload_setting[index];
> } else {
> smu->workload_mask |= (1 << smu->workload_prority[type]);
> + smu->driver_workload_mask |= (1 << smu->workload_prority[type]);
> index = fls(smu->workload_mask);
> index = index <= WORKLOAD_POLICY_MAX ? index - 1 : 0;
> workload[0] = smu->workload_setting[index];
> @@ -3045,12 +3050,16 @@ static int smu_set_power_profile_mode(void *handle,
> uint32_t param_size)
> {
> struct smu_context *smu = handle;
> + int ret;
>
> if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled ||
> !smu->ppt_funcs->set_power_profile_mode)
> return -EOPNOTSUPP;
> + smu->user_profile_mode_setting = true;
> + ret = smu_bump_power_profile_mode(smu, param, param_size);
> + smu->user_profile_mode_setting = false;
Instead of doing this way, I think it's better to save the user
preference as a mask here. Later decide what to be applied whenever
workload mask is applied.
if (user_profile = default or unknown) // No user preference
user_mask = 0 ; // user driver default mask
else if (user_profile = xyz)
user_mask = xyz_workload_mask;
Save user_mask to user_dpm profile.
Whenever workload mask is applied, select user_mask if non-zero or
driver_mask, or a combination of both.
Thanks,
Lijo
>
> - return smu_bump_power_profile_mode(smu, param, param_size);
> + return ret;
> }
>
> static int smu_get_fan_control_mode(void *handle, u32 *fan_mode)
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> index 8bb32b3f0d9c..e43b23dd3457 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> @@ -557,10 +557,11 @@ struct smu_context {
> bool disable_uclk_switch;
>
> uint32_t workload_mask;
> + uint32_t driver_workload_mask;
> uint32_t workload_prority[WORKLOAD_POLICY_MAX];
> uint32_t workload_setting[WORKLOAD_POLICY_MAX];
> uint32_t power_profile_mode;
> - uint32_t default_power_profile_mode;
> + uint32_t user_profile_mode;
> bool pm_enabled;
> bool is_apu;
>
> @@ -572,6 +573,7 @@ struct smu_context {
>
> bool uploading_custom_pp_table;
> bool dc_controlled_by_gpio;
> + bool user_profile_mode_setting;
>
> struct work_struct throttling_logging_work;
> atomic64_t throttle_int_counter;
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> index 3e2277abc754..7250b2bad198 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> @@ -2433,7 +2433,7 @@ static int smu_v13_0_0_get_power_profile_mode(struct
> smu_context *smu,
> }
>
> size += sysfs_emit_at(buf, size, "%2d %14s%s:\n",
> - i, amdgpu_pp_profile_name[i], (i ==
> smu->power_profile_mode) ? "*" : " ");
> + i, amdgpu_pp_profile_name[i], (i ==
> smu->user_profile_mode) ? "*" : " ");
>
> size += sysfs_emit_at(buf, size, "%19s %d(%13s) %7d %7d %7d %7d
> %7d %7d %7d %7d\n",
> " ",
> @@ -2474,9 +2474,27 @@ static int smu_v13_0_0_set_power_profile_mode(struct
> smu_context *smu,
> &(activity_monitor_external.DpmActivityMonitorCoeffInt);
> int workload_type, ret = 0;
> u32 workload_mask;
> + uint32_t current_user_profile_mode;
> + uint32_t index;
> +
> + if (smu->user_profile_mode_setting && smu->user_profile_mode ==
> input[size])
> + return 0;
>
> smu->power_profile_mode = input[size];
>
> + if (smu->user_profile_mode_setting) {
> + current_user_profile_mode = smu->user_profile_mode;
> + smu->user_profile_mode = input[size];
> + workload_type = smu_cmn_to_asic_specific_index(smu,
> +
> CMN2ASIC_MAPPING_WORKLOAD,
> +
> current_user_profile_mode);
> + if (workload_type < 0)
> + return -EINVAL;
> +
> + if (!(smu->driver_workload_mask & (1 << workload_type)))
> + smu->workload_mask &= ~(1 << workload_type);
> + }
> +
> if (smu->power_profile_mode >= PP_SMC_POWER_PROFILE_COUNT) {
> dev_err(smu->adev->dev, "Invalid power profile mode %d\n",
> smu->power_profile_mode);
> return -EINVAL;
> @@ -2555,12 +2573,25 @@ static int smu_v13_0_0_set_power_profile_mode(struct
> smu_context *smu,
> workload_mask |= 1 << workload_type;
> }
>
> + smu->workload_mask |= workload_mask;
> +
> ret = smu_cmn_send_smc_msg_with_param(smu,
> SMU_MSG_SetWorkloadMask,
> - workload_mask,
> + smu->workload_mask,
> NULL);
> - if (!ret)
> - smu->workload_mask = workload_mask;
> + if (!ret) {
> + index = fls(smu->workload_mask);
> + index = index > 0 && index <= WORKLOAD_POLICY_MAX ? index - 1 :
> 0;
> + smu->power_profile_mode = smu->workload_setting[index];
> + if (smu->power_profile_mode ==
> PP_SMC_POWER_PROFILE_POWERSAVING) {
> + workload_type = smu_cmn_to_asic_specific_index(smu,
> +
> CMN2ASIC_MAPPING_WORKLOAD,
> +
> PP_SMC_POWER_PROFILE_FULLSCREEN3D);
> + smu->power_profile_mode = smu->workload_mask & (1 <<
> workload_type)
> +
> ? PP_SMC_POWER_PROFILE_FULLSCREEN3D
> +
> : PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;
> + }
> + }
>
> return ret;
> }
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> index 23f13388455f..8afd43e78ebc 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> @@ -2367,7 +2367,7 @@ static int smu_v13_0_7_get_power_profile_mode(struct
> smu_context *smu, char *buf
> size += sysfs_emit_at(buf, size, " ");
> for (i = 0; i <= PP_SMC_POWER_PROFILE_WINDOW3D; i++)
> size += sysfs_emit_at(buf, size, "%d %-14s%s", i,
> amdgpu_pp_profile_name[i],
> - (i == smu->power_profile_mode) ? "* " : " ");
> + (i == smu->user_profile_mode) ? "* " : " ");
>
> size += sysfs_emit_at(buf, size, "\n");
>
> @@ -2429,9 +2429,27 @@ static int smu_v13_0_7_set_power_profile_mode(struct
> smu_context *smu, long *inp
> DpmActivityMonitorCoeffInt_t *activity_monitor =
> &(activity_monitor_external.DpmActivityMonitorCoeffInt);
> int workload_type, ret = 0;
> + uint32_t current_user_profile_mode;
> + uint32_t index;
> +
> + if (smu->user_profile_mode_setting && smu->user_profile_mode ==
> input[size])
> + return 0;
>
> smu->power_profile_mode = input[size];
>
> + if (smu->user_profile_mode_setting) {
> + current_user_profile_mode = smu->user_profile_mode;
> + smu->user_profile_mode = input[size];
> + workload_type = smu_cmn_to_asic_specific_index(smu,
> +
> CMN2ASIC_MAPPING_WORKLOAD,
> +
> current_user_profile_mode);
> + if (workload_type < 0)
> + return -EINVAL;
> +
> + if (!(smu->driver_workload_mask & (1 << workload_type)))
> + smu->workload_mask &= ~(1 << workload_type);
> + }
> +
> if (smu->power_profile_mode > PP_SMC_POWER_PROFILE_WINDOW3D) {
> dev_err(smu->adev->dev, "Invalid power profile mode %d\n",
> smu->power_profile_mode);
> return -EINVAL;
> @@ -2487,13 +2505,18 @@ static int smu_v13_0_7_set_power_profile_mode(struct
> smu_context *smu, long *inp
> smu->power_profile_mode);
> if (workload_type < 0)
> return -EINVAL;
> +
> + smu->workload_mask |= (1 << workload_type);
> ret = smu_cmn_send_smc_msg_with_param(smu, SMU_MSG_SetWorkloadMask,
> - 1 << workload_type, NULL);
> + smu->workload_mask, NULL);
>
> - if (ret)
> + if (ret) {
> dev_err(smu->adev->dev, "[%s] Failed to set work load mask!",
> __func__);
> - else
> - smu->workload_mask = (1 << workload_type);
> + } else {
> + index = fls(smu->workload_mask);
> + index = index > 0 && index <= WORKLOAD_POLICY_MAX ? index - 1 :
> 0;
> + smu->power_profile_mode = smu->workload_setting[index];
> + }
>
> return ret;
> }
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0_2_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0_2_ppt.c
> index cefe10b95d8e..d88bf9bad313 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0_2_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0_2_ppt.c
> @@ -1747,7 +1747,7 @@ static int smu_v14_0_2_get_power_profile_mode(struct
> smu_context *smu,
> }
>
> size += sysfs_emit_at(buf, size, "%2d %14s%s:\n",
> - i, amdgpu_pp_profile_name[i], (i ==
> smu->power_profile_mode) ? "*" : " ");
> + i, amdgpu_pp_profile_name[i], (i ==
> smu->user_profile_mode) ? "*" : " ");
>
> size += sysfs_emit_at(buf, size, "%19s %d(%13s) %7d %7d %7d %7d
> %7d %7d %7d %7d\n",
> " ",
> @@ -1787,9 +1787,27 @@ static int smu_v14_0_2_set_power_profile_mode(struct
> smu_context *smu,
> DpmActivityMonitorCoeffInt_t *activity_monitor =
> &(activity_monitor_external.DpmActivityMonitorCoeffInt);
> int workload_type, ret = 0;
> - uint32_t current_profile_mode = smu->power_profile_mode;
> + uint32_t current_user_profile_mode;
> + uint32_t index;
> +
> + if (smu->user_profile_mode_setting && smu->user_profile_mode ==
> input[size])
> + return 0;
> +
> smu->power_profile_mode = input[size];
>
> + if (smu->user_profile_mode_setting) {
> + current_user_profile_mode = smu->user_profile_mode;
> + smu->user_profile_mode = input[size];
> + workload_type = smu_cmn_to_asic_specific_index(smu,
> +
> CMN2ASIC_MAPPING_WORKLOAD,
> +
> current_user_profile_mode);
> + if (workload_type < 0)
> + return -EINVAL;
> +
> + if (!(smu->driver_workload_mask & (1 << workload_type)))
> + smu->workload_mask &= ~(1 << workload_type);
> + }
> +
> if (smu->power_profile_mode >= PP_SMC_POWER_PROFILE_COUNT) {
> dev_err(smu->adev->dev, "Invalid power profile mode %d\n",
> smu->power_profile_mode);
> return -EINVAL;
> @@ -1845,9 +1863,16 @@ static int smu_v14_0_2_set_power_profile_mode(struct
> smu_context *smu,
> }
> }
>
> + workload_type = smu_cmn_to_asic_specific_index(smu,
> +
> CMN2ASIC_MAPPING_WORKLOAD,
> +
> PP_SMC_POWER_PROFILE_COMPUTE);
> +
> + if (workload_type < 0)
> + return -EINVAL;
> +
> if (smu->power_profile_mode == PP_SMC_POWER_PROFILE_COMPUTE)
> smu_v14_0_deep_sleep_control(smu, false);
> - else if (current_profile_mode == PP_SMC_POWER_PROFILE_COMPUTE)
> + else if (smu->workload_mask & (1 << workload_type))
> smu_v14_0_deep_sleep_control(smu, true);
>
> /* conv PP_SMC_POWER_PROFILE* to WORKLOAD_PPLIB_*_BIT */
> @@ -1857,12 +1882,17 @@ static int smu_v14_0_2_set_power_profile_mode(struct
> smu_context *smu,
> if (workload_type < 0)
> return -EINVAL;
>
> + smu->workload_mask |= (1 << workload_type);
> ret = smu_cmn_send_smc_msg_with_param(smu,
> SMU_MSG_SetWorkloadMask,
> - 1 << workload_type,
> + smu->workload_mask,
> NULL);
> - if (!ret)
> - smu->workload_mask = 1 << workload_type;
> +
> + if (!ret) {
> + index = fls(smu->workload_mask);
> + index = index > 0 && index <= WORKLOAD_POLICY_MAX ? index - 1 :
> 0;
> + smu->power_profile_mode = smu->workload_setting[index];
> + }
>
> return ret;
> }
