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

[Alex Deucher]

On Mon, Jan 6, 2025 at 10:56 PM Feng, Kenneth <Kenneth.Feng at amd.com> wrote:
>
> [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?

no, wrap the smu->saved_power_profile_mode and smu->power_profile_mode
with the mutex.  E.g.,

+       if (en) {
+               ret = amdgpu_dpm_switch_power_profile(adev,
current_profile_mode, !en);

mutex_lock()

+               smu->saved_power_profile_mode = current_profile_mode;
+               smu->power_profile_mode = PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;

mutex_unlock()

+       } else {
+               ret = amdgpu_dpm_switch_power_profile(adev,
saved_profile_mode, !en);

mutex_lock()

+               smu->power_profile_mode = saved_profile_mode;
+               smu->saved_power_profile_mode =
PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;

mutex_unlock()

+       }

Alex

> 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
> >