On Tue, Oct 22, 2024 at 11:23 PM 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. Might be better to actually treat the workload like a mask rather than as a discrete setting since that mirrors how the firmware actually works. The default value set at init time or whatever the user selects via sysfs should be what is reflected in sysfs, however, when KFD selects COMPUTE or the VCN code selects VIDEO, rather than clearing all of the other bits and just setting the selected profile, we should just add or remove the additional bits to the bit mask and store the whole bit mask in the driver. E.g., smu->workload_mask would be the actual mask and smu->power_profile_mode would be the value reflected in sysfs. Alex > > Signed-off-by: Kenneth Feng <kenneth.feng@xxxxxxx> > --- > 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; > + } > > 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; > > - 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; > } > -- > 2.34.1 >