From: "Lazar, Lijo" <lijo.lazar@amd.com>
To: Kenneth Feng <kenneth.feng@amd.com>, amd-gfx@lists.freedesktop.org
Cc: alexander.deucher@amd.com
Subject: Re: [PATCH] drm/amd/pm: correct the workload setting
Date: Wed, 23 Oct 2024 17:03:33 +0530 [thread overview]
Message-ID: <fbad3a97-10a9-4f40-b2f8-e9ead3fa352e@amd.com> (raw)
In-Reply-To: <20241023031257.14238-1-kenneth.feng@amd.com>
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.feng@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;
> }
next prev parent reply other threads:[~2024-10-23 11:33 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-23 3:12 [PATCH] drm/amd/pm: correct the workload setting Kenneth Feng
2024-10-23 11:33 ` Lazar, Lijo [this message]
2024-10-23 22:47 ` Alex Deucher
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fbad3a97-10a9-4f40-b2f8-e9ead3fa352e@amd.com \
--to=lijo.lazar@amd.com \
--cc=alexander.deucher@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=kenneth.feng@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.