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