[AMD Official Use Only - AMD Internal Distribution Only]
-----Original Message-----
From: Alex Deucher <alexdeucher@xxxxxxxxx>
Sent: Monday, January 6, 2025 11:59 PM
To: Feng, Kenneth <Kenneth.Feng@xxxxxxx>
Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Pillai, Aurabindo <Aurabindo.Pillai@xxxxxxx>; Deucher, Alexander <Alexander.Deucher@xxxxxxx>
Subject: Re: [PATCH 1/2] drm/amd/pm: add the interface to set and save the bootup workload type
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
On Wed, Dec 25, 2024 at 3:08 AM Kenneth Feng <kenneth.feng@xxxxxxx> wrote:
>
> add the interface to set and save the bootup workload type
>
> Signed-off-by: Kenneth Feng <kenneth.feng@xxxxxxx>
> ---
> drivers/gpu/drm/amd/pm/amdgpu_dpm.c | 27 +++++++++++++++++++
> drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h | 3 +++
> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 1 +
> drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 1 +
> 4 files changed, 32 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> index 6a9e26905edf..92fa19cac32a 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> @@ -349,6 +349,33 @@ int amdgpu_dpm_switch_power_profile(struct amdgpu_device *adev,
> return ret;
> }
>
> +int amdgpu_dpm_set_and_save_bootup_power_profile(struct amdgpu_device *adev,
> + bool en) {
> + struct smu_context *smu = (struct
> +smu_context*)(adev->powerplay.pp_handle);
You need to check if is_support_sw_smu() before accessing the smu context.
> + int current_profile_mode = smu->power_profile_mode;
> + int saved_profile_mode = smu->saved_power_profile_mode;
> + int ret = 0;
> +
> + if (en && current_profile_mode == PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
> + return 0;
> +
> + if (!en && current_profile_mode != PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
> + return 0;
> +
> + if (en) {
> + ret = amdgpu_dpm_switch_power_profile(adev, current_profile_mode, !en);
> + smu->saved_power_profile_mode = current_profile_mode;
> + smu->power_profile_mode =
> + PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;
Should take the pm.mutex when you mess with the smu context.
[Kenneth] - the pm.mutex is taken in the called function amdgpu_dpm_switch_power_profile. Do we need it before calling amdgpu_dpm_switch_power_profile?
Thanks.
> + } else {
> + ret = amdgpu_dpm_switch_power_profile(adev, saved_profile_mode, !en);
> + smu->power_profile_mode = saved_profile_mode;
> + smu->saved_power_profile_mode =
> + PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;
Same here.
> + }
> +
> + return ret;
> +}
> +
> int amdgpu_dpm_set_xgmi_pstate(struct amdgpu_device *adev,
> uint32_t pstate) { diff --git
> a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> index 1f5ac7e0230d..5fe404bbb033 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> @@ -410,6 +410,9 @@ int amdgpu_dpm_switch_power_profile(struct amdgpu_device *adev,
> enum PP_SMC_POWER_PROFILE type,
> bool en);
>
> +int amdgpu_dpm_set_and_save_bootup_power_profile(struct amdgpu_device *adev,
> + bool en);
> +
> int amdgpu_dpm_baco_reset(struct amdgpu_device *adev);
>
> int amdgpu_dpm_mode2_reset(struct amdgpu_device *adev); diff --git
> a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index 8ca793c222ff..a6f748361198 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -1272,6 +1272,7 @@ static void smu_init_power_profile(struct smu_context *smu)
> PP_SMC_POWER_PROFILE_FULLSCREEN3D;
> }
> smu_power_profile_mode_get(smu, smu->power_profile_mode);
> + smu->saved_power_profile_mode = smu->power_profile_mode;
> }
>
> static int smu_sw_init(struct amdgpu_ip_block *ip_block) 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 3630593bce61..c58fc31fb1d7 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> @@ -560,6 +560,7 @@ struct smu_context {
> uint32_t workload_mask;
> /* default/user workload preference */
> uint32_t power_profile_mode;
> + uint32_t saved_power_profile_mode;
> uint32_t workload_refcount[PP_SMC_POWER_PROFILE_COUNT];
> /* backend specific custom workload settings */
> long *custom_profile_params;
> --
> 2.34.1
>
