On Thu, Oct 24, 2024 at 5:18 AM Kenneth Feng <kenneth.feng@xxxxxxx> wrote:
>
> Correct the workload setting in order not to mix the setting
> with the end user. Update the workload mask accordingly.
>
> v2: changes as below:
> 1. the end user can not erase the workload from driver except default workload.
> 2. always shows the real highest priority workoad to the end user.
> 3. the real workload mask is combined with driver workload mask and end user workload mask.
I think this can be simplified. We just need to store the user
workload profile and the mask of all of the currently active workload
profiles (the user selected profile and the any transient ones like
COMPUTE for KFD, VIDEO for VCN, and POWERSAVE for SMU13, etc.). At
init time, the driver sets the user workload profile to FS3D or
DEFAULT per the current logic. Add a new parameter to
ppt_funcs->set_power_profile_mode(), bool
update_user_workload_profile, which we set to true in
smu_set_power_profile_mode() which is used by the sysfs code to set
the user workload profile, and set to false in
smu_switch_power_profile() which is used internally for KFD and VCN.
Then the user workload profile would only get changed when the user
changes it via sysfs. Meanwhile KFD and VCN can add their workload
types dynamically at runtime.
Alex
>
> Signed-off-by: Kenneth Feng <kenneth.feng@xxxxxxx>
> ---
> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 44 +++++++++++++------
> drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 5 ++-
> .../drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 31 +++++++++++--
> .../drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 25 +++++++++--
> .../drm/amd/pm/swsmu/smu14/smu_v14_0_2_ppt.c | 28 +++++++++---
> 5 files changed, 106 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index 8d4aee4e2287..1de576461a70 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -1261,25 +1261,31 @@ static int smu_sw_init(struct amdgpu_ip_block *ip_block)
> 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_dpm_profile.user_workload_mask = 0;
> + smu->user_dpm_profile.prev_user_workload_mask = 0;
>
> atomic_set(&smu->smu_power.power_gate.vcn_gated, 1);
> atomic_set(&smu->smu_power.power_gate.jpeg_gated, 1);
> atomic_set(&smu->smu_power.power_gate.vpe_gated, 1);
> atomic_set(&smu->smu_power.power_gate.umsch_mm_gated, 1);
>
> - smu->workload_prority[PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT] = 0;
> - smu->workload_prority[PP_SMC_POWER_PROFILE_FULLSCREEN3D] = 1;
> - smu->workload_prority[PP_SMC_POWER_PROFILE_POWERSAVING] = 2;
> - smu->workload_prority[PP_SMC_POWER_PROFILE_VIDEO] = 3;
> - smu->workload_prority[PP_SMC_POWER_PROFILE_VR] = 4;
> - smu->workload_prority[PP_SMC_POWER_PROFILE_COMPUTE] = 5;
> - smu->workload_prority[PP_SMC_POWER_PROFILE_CUSTOM] = 6;
> + smu->workload_priority[PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT] = 0;
> + smu->workload_priority[PP_SMC_POWER_PROFILE_FULLSCREEN3D] = 1;
> + smu->workload_priority[PP_SMC_POWER_PROFILE_POWERSAVING] = 2;
> + smu->workload_priority[PP_SMC_POWER_PROFILE_VIDEO] = 3;
> + smu->workload_priority[PP_SMC_POWER_PROFILE_VR] = 4;
> + smu->workload_priority[PP_SMC_POWER_PROFILE_COMPUTE] = 5;
> + smu->workload_priority[PP_SMC_POWER_PROFILE_CUSTOM] = 6;
>
> if (smu->is_apu ||
> - !smu_is_workload_profile_available(smu, PP_SMC_POWER_PROFILE_FULLSCREEN3D))
> - smu->workload_mask = 1 << smu->workload_prority[PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT];
> - else
> - smu->workload_mask = 1 << smu->workload_prority[PP_SMC_POWER_PROFILE_FULLSCREEN3D];
> + !smu_is_workload_profile_available(smu, PP_SMC_POWER_PROFILE_FULLSCREEN3D)) {
> + smu->workload_mask = 1 << smu->workload_priority[PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT];
> + } else {
> + smu->workload_mask = 1 << smu->workload_priority[PP_SMC_POWER_PROFILE_FULLSCREEN3D];
> + smu->default_power_profile_mode = PP_SMC_POWER_PROFILE_FULLSCREEN3D;
> + }
> +
> + smu->driver_workload_mask = smu->workload_mask;
>
> smu->workload_setting[0] = PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;
> smu->workload_setting[1] = PP_SMC_POWER_PROFILE_FULLSCREEN3D;
> @@ -2354,12 +2360,14 @@ static int smu_switch_power_profile(void *handle,
> return -EINVAL;
>
> if (!en) {
> - smu->workload_mask &= ~(1 << smu->workload_prority[type]);
> + smu->workload_mask &= ~(1 << smu->workload_priority[type]);
> + smu->driver_workload_mask &= ~(1 << smu->workload_priority[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->workload_mask |= (1 << smu->workload_priority[type]);
> + smu->driver_workload_mask |= (1 << smu->workload_priority[type]);
> index = fls(smu->workload_mask);
> index = index <= WORKLOAD_POLICY_MAX ? index - 1 : 0;
> workload[0] = smu->workload_setting[index];
> @@ -3054,12 +3062,20 @@ 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;
>
> - return smu_bump_power_profile_mode(smu, param, param_size);
> + smu->user_dpm_profile.prev_user_workload_mask =
> + smu->user_dpm_profile.user_workload_mask;
> + smu->user_dpm_profile.user_workload_mask = (1 << smu->workload_priority[param[param_size]]);
> + ret = smu_bump_power_profile_mode(smu, param, param_size);
> + smu->user_dpm_profile.prev_user_workload_mask =
> + smu->user_dpm_profile.user_workload_mask;
> +
> + 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..88294d986b36 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> @@ -240,6 +240,8 @@ struct smu_user_dpm_profile {
> /* user clock state information */
> uint32_t clk_mask[SMU_CLK_COUNT];
> uint32_t clk_dependency;
> + uint32_t user_workload_mask;
> + uint32_t prev_user_workload_mask;
> };
>
> #define SMU_TABLE_INIT(tables, table_id, s, a, d) \
> @@ -557,7 +559,8 @@ struct smu_context {
> bool disable_uclk_switch;
>
> uint32_t workload_mask;
> - uint32_t workload_prority[WORKLOAD_POLICY_MAX];
> + uint32_t driver_workload_mask;
> + uint32_t workload_priority[WORKLOAD_POLICY_MAX];
> uint32_t workload_setting[WORKLOAD_POLICY_MAX];
> uint32_t power_profile_mode;
> uint32_t default_power_profile_mode;
> 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..0733fd3efd8b 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
> @@ -2474,9 +2474,22 @@ 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 index;
>
> smu->power_profile_mode = input[size];
>
> + if (smu->user_dpm_profile.prev_user_workload_mask !=
> + smu->user_dpm_profile.user_workload_mask) {
> + if (smu->workload_mask & smu->user_dpm_profile.prev_user_workload_mask &&
> + !(smu->driver_workload_mask & smu->user_dpm_profile.prev_user_workload_mask))
> + smu->workload_mask &= ~smu->user_dpm_profile.prev_user_workload_mask;
> +
> + if (input[size] != smu->default_power_profile_mode) {
> + smu->workload_mask &= ~(1 << smu->workload_priority[smu->default_power_profile_mode]);
> + smu->driver_workload_mask &= ~(1 << smu->workload_priority[smu->default_power_profile_mode]);
> + }
> + }
This is repeated in several places and could be split out into a
helper function.
> +
> 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 +2568,24 @@ 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..2323c74ee50b 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
> @@ -2429,9 +2429,22 @@ 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 index;
>
> smu->power_profile_mode = input[size];
>
> + if (smu->user_dpm_profile.prev_user_workload_mask !=
> + smu->user_dpm_profile.user_workload_mask) {
> + if (smu->workload_mask & smu->user_dpm_profile.prev_user_workload_mask &&
> + !(smu->driver_workload_mask & smu->user_dpm_profile.prev_user_workload_mask))
> + smu->workload_mask &= ~smu->user_dpm_profile.prev_user_workload_mask;
> +
> + if (input[size] != smu->default_power_profile_mode) {
> + smu->workload_mask &= ~(1 << smu->workload_priority[smu->default_power_profile_mode]);
> + smu->driver_workload_mask &= ~(1 << smu->workload_priority[smu->default_power_profile_mode]);
> + }
> + }
> +
> 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 +2500,19 @@ 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)
> dev_err(smu->adev->dev, "[%s] Failed to set work load mask!", __func__);
> - else
> - 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;
> }
> 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..c2fd47f04e2d 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
> @@ -1787,9 +1787,22 @@ 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 index;
> uint32_t current_profile_mode = smu->power_profile_mode;
> smu->power_profile_mode = input[size];
>
> + if (smu->user_dpm_profile.prev_user_workload_mask !=
> + smu->user_dpm_profile.user_workload_mask) {
> + if (smu->workload_mask & smu->user_dpm_profile.prev_user_workload_mask &&
> + !(smu->driver_workload_mask & smu->user_dpm_profile.prev_user_workload_mask))
> + smu->workload_mask &= ~smu->user_dpm_profile.prev_user_workload_mask;
> +
> + if (input[size] != smu->default_power_profile_mode) {
> + smu->workload_mask &= ~(1 << smu->workload_priority[smu->default_power_profile_mode]);
> + smu->driver_workload_mask &= ~(1 << smu->workload_priority[smu->default_power_profile_mode]);
> + }
> + }
> +
> 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;
> @@ -1857,12 +1870,15 @@ static int smu_v14_0_2_set_power_profile_mode(struct smu_context *smu,
> if (workload_type < 0)
> return -EINVAL;
>
> - ret = smu_cmn_send_smc_msg_with_param(smu,
> - SMU_MSG_SetWorkloadMask,
> - 1 << workload_type,
> - NULL);
> - if (!ret)
> - smu->workload_mask = 1 << workload_type;
> + smu->workload_mask |= (1 << workload_type);
> + ret = smu_cmn_send_smc_msg_with_param(smu, SMU_MSG_SetWorkloadMask,
> + smu->workload_mask, NULL);
> +
> + 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;
> }
> --
> 2.34.1
>
