|
Comment inline From: Quan, Evan <Evan.Quan@xxxxxxx>
Sent: Friday, July 19, 2019 1:03 PM To: Wang, Kevin(Yang) <Kevin1.Wang@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx> Cc: Feng, Kenneth <Kenneth.Feng@xxxxxxx>; Huang, Ray <Ray.Huang@xxxxxxx>; Xu, Feifei <Feifei.Xu@xxxxxxx>; Gui, Jack <Jack.Gui@xxxxxxx> Subject: RE: [PATCH] drm/amd/powerplay: custom peak clock freq for navi10 Comment inline
> -----Original Message----- > From: Wang, Kevin(Yang) <Kevin1.Wang@xxxxxxx> > Sent: Friday, July 19, 2019 11:46 AM > To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: Feng, Kenneth <Kenneth.Feng@xxxxxxx>; Quan, Evan > <Evan.Quan@xxxxxxx>; Huang, Ray <Ray.Huang@xxxxxxx>; Xu, Feifei > <Feifei.Xu@xxxxxxx>; Gui, Jack <Jack.Gui@xxxxxxx>; Wang, Kevin(Yang) > <Kevin1.Wang@xxxxxxx> > Subject: [PATCH] drm/amd/powerplay: custom peak clock freq for navi10 > > 1.NAVI10_PEAK_SCLK_XTX 1830 Mhz > 2.NAVI10_PEAK_SCLK_XT 1755 Mhz > 3.NAVI10_PEAK_SCLK_XL 1625 Mhz > > Change-Id: I48863a9d0e261b9e7778a6c0e4a8762d7c978da6 > Signed-off-by: Kevin Wang <kevin1.wang@xxxxxxx> > --- > drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 65 ++++++++++-------- > - > .../gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 4 ++ > drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 55 ++++++++++++++++ > drivers/gpu/drm/amd/powerplay/navi10_ppt.h | 4 ++ > 4 files changed, 97 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c > b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c > index 7f51bbd2ac90..ab389dde9562 100644 > --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c > +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c > @@ -1360,37 +1360,40 @@ int smu_adjust_power_state_dynamic(struct > smu_context *smu, > } > > if (smu_dpm_ctx->dpm_level != level) { > - switch (level) { > - case AMD_DPM_FORCED_LEVEL_HIGH: > - ret = smu_force_dpm_limit_value(smu, true); > - break; > - case AMD_DPM_FORCED_LEVEL_LOW: > - ret = smu_force_dpm_limit_value(smu, false); > - break; > - > - case AMD_DPM_FORCED_LEVEL_AUTO: > - case AMD_DPM_FORCED_LEVEL_PROFILE_STANDARD: > - ret = smu_unforce_dpm_levels(smu); > - break; > - > - case AMD_DPM_FORCED_LEVEL_PROFILE_MIN_SCLK: > - case AMD_DPM_FORCED_LEVEL_PROFILE_MIN_MCLK: > - case AMD_DPM_FORCED_LEVEL_PROFILE_PEAK: > - ret = smu_get_profiling_clk_mask(smu, level, > - &sclk_mask, > - &mclk_mask, > - &soc_mask); > - if (ret) > - return ret; > - smu_force_clk_levels(smu, SMU_SCLK, 1 << > sclk_mask); > - smu_force_clk_levels(smu, SMU_MCLK, 1 << > mclk_mask); > - smu_force_clk_levels(smu, SMU_SOCCLK, 1 << > soc_mask); > - break; > - > - case AMD_DPM_FORCED_LEVEL_MANUAL: > - case AMD_DPM_FORCED_LEVEL_PROFILE_EXIT: > - default: > - break; > + ret = smu_set_performance_level(smu, level); > + if (ret) { [Quan, Evan] Since this actually handles AMD_DPM_FORCED_LEVEL_PROFILE_PEAK only. Please move this under "case AMD_DPM_FORCED_LEVEL_PROFILE_PEAK:". And please give it a more meaningful name e.g. smu_set_asic_peak_profile(). [kevin]: in the future, for specifc asic, it may be custom other performance level, so we should have this capacity
to support other asic with any performance level. and i think
it is not a good way to handle performance level use case by case method for each asic.
> + switch (level) { > + case AMD_DPM_FORCED_LEVEL_HIGH: > + ret = smu_force_dpm_limit_value(smu, true); > + break; > + case AMD_DPM_FORCED_LEVEL_LOW: > + ret = smu_force_dpm_limit_value(smu, > false); > + break; > + > + case AMD_DPM_FORCED_LEVEL_AUTO: > + case > AMD_DPM_FORCED_LEVEL_PROFILE_STANDARD: > + ret = smu_unforce_dpm_levels(smu); > + break; > + > + case > AMD_DPM_FORCED_LEVEL_PROFILE_MIN_SCLK: > + case > AMD_DPM_FORCED_LEVEL_PROFILE_MIN_MCLK: > + case AMD_DPM_FORCED_LEVEL_PROFILE_PEAK: > + ret = smu_get_profiling_clk_mask(smu, level, > + &sclk_mask, > + &mclk_mask, > + &soc_mask); > + if (ret) > + return ret; > + smu_force_clk_levels(smu, SMU_SCLK, 1 << > sclk_mask); > + smu_force_clk_levels(smu, SMU_MCLK, 1 << > mclk_mask); > + smu_force_clk_levels(smu, SMU_SOCCLK, 1 > << soc_mask); > + break; > + > + case AMD_DPM_FORCED_LEVEL_MANUAL: > + case AMD_DPM_FORCED_LEVEL_PROFILE_EXIT: > + default: > + break; > + } > } > > if (!ret) > diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h > b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h > index 514d31518853..ba5ddafcbdba 100644 > --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h > +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h > @@ -631,6 +631,7 @@ struct pptable_funcs { > int (*get_thermal_temperature_range)(struct smu_context *smu, > struct smu_temperature_range *range); > int (*get_uclk_dpm_states)(struct smu_context *smu, uint32_t > *clocks_in_khz, uint32_t *num_states); > int (*set_default_od_settings)(struct smu_context *smu, bool > initialize); > + int (*set_performance_level)(struct smu_context *smu, int32_t > level); > }; > > struct smu_funcs > @@ -928,6 +929,9 @@ struct smu_funcs > ((smu)->funcs->baco_get_state? (smu)->funcs- > >baco_get_state((smu), (state)) : 0) > #define smu_baco_reset(smu) \ > ((smu)->funcs->baco_reset? (smu)->funcs->baco_reset((smu)) : 0) > +#define smu_set_performance_level(smu, level) \ > + ((smu)->ppt_funcs->set_performance_level? (smu)->ppt_funcs- > >set_performance_level((smu), (level)) : -EINVAL); > + > > extern int smu_get_atom_data_table(struct smu_context *smu, uint32_t > table, > uint16_t *size, uint8_t *frev, uint8_t *crev, > diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c > b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c > index e44041a25e64..9024d91477f0 100644 > --- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c > +++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c > @@ -1590,6 +1590,60 @@ static int navi10_set_ppfeature_status(struct > smu_context *smu, > return 0; > } > > +static int navi10_set_peak_clock_by_device(struct smu_context *smu) > +{ > + struct amdgpu_device *adev = smu->adev; > + int ret = 0; > + uint32_t sclk_freq = 0, uclk_freq = 0; > + uint32_t uclk_level = 0; > + > + switch (adev->rev_id) { > + case 0xf0: /* XTX */ > + case 0xC0: > + sclk_freq = NAVI10_PEAK_SCLK_XTX; > + break; > + case 0xf1: /* XT */ > + case 0xC1: > + sclk_freq = NAVI10_PEAK_SCLK_XT; > + break; > + default: /* XL */ > + sclk_freq = NAVI10_PEAK_SCLK_XL; > + break; > + } > + > + ret = smu_get_dpm_level_count(smu, SMU_UCLK, &uclk_level); > + if (ret) > + return ret; > + ret = smu_get_dpm_freq_by_index(smu, SMU_UCLK, uclk_level - 1, > &uclk_freq); > + if (ret) > + return ret; [Quan, Evan] I think with dpm level (3rd argument) set as 0xff directly, you can get the clock frequency for the max level. [kevin]: not right, 0xff will return dpm clock level count, see function smu_get_dpm_level_count
> + > + ret = smu_set_soft_freq_range(smu, SMU_SCLK, sclk_freq, > sclk_freq); > + if (ret) > + return ret; > + ret = smu_set_soft_freq_range(smu, SMU_UCLK, uclk_freq, > uclk_freq); > + if (ret) > + return ret; > + > + return ret; > +} > + > +static int navi10_set_performance_level(struct smu_context *smu, int32_t > level) > +{ > + int ret = 0; > + > + switch (level) { > + case AMD_DPM_FORCED_LEVEL_PROFILE_PEAK: > + ret = navi10_set_peak_clock_by_device(smu); > + break; > + default: > + ret = -EINVAL; > + break; > + } > + > + return ret; > +} > + > static const struct pptable_funcs navi10_ppt_funcs = { > .tables_init = navi10_tables_init, > .alloc_dpm_context = navi10_allocate_dpm_context, > @@ -1625,6 +1679,7 @@ static const struct pptable_funcs navi10_ppt_funcs > = { > .get_uclk_dpm_states = navi10_get_uclk_dpm_states, > .get_ppfeature_status = navi10_get_ppfeature_status, > .set_ppfeature_status = navi10_set_ppfeature_status, > + .set_performance_level = navi10_set_performance_level, > }; > > void navi10_set_ppt_funcs(struct smu_context *smu) > diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.h > b/drivers/gpu/drm/amd/powerplay/navi10_ppt.h > index 957288e22f47..620ff17c2fef 100644 > --- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.h > +++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.h > @@ -23,6 +23,10 @@ > #ifndef __NAVI10_PPT_H__ > #define __NAVI10_PPT_H__ > > +#define NAVI10_PEAK_SCLK_XTX (1830) > +#define NAVI10_PEAK_SCLK_XT (1755) > +#define NAVI10_PEAK_SCLK_XL (1625) > + > extern void navi10_set_ppt_funcs(struct smu_context *smu); > > #endif > -- > 2.22.0 |
_______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
