[PATCH 1/2] drm/amd/pm: add the interface to set and save the bootup workload type

[Feng, Kenneth]

[AMD Official Use Only - AMD Internal Distribution Only]

-----Original Message-----
From: Alex Deucher <alexdeucher at gmail.com>
Sent: Monday, January 6, 2025 11:59 PM
To: Feng, Kenneth <Kenneth.Feng at amd.com>
Cc: amd-gfx at lists.freedesktop.org; Pillai, Aurabindo <Aurabindo.Pillai at amd.com>; Deucher, Alexander <Alexander.Deucher at amd.com>
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 at amd.com> wrote:
>
> add the interface to set and save the bootup workload type
>
> Signed-off-by: Kenneth Feng <kenneth.feng at amd.com>
> ---
>  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
>