<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" style="font-size: 12pt; color: rgb(0, 0, 0); font-family: Calibri, Helvetica, sans-serif, EmojiFont, "Apple Color Emoji", "Segoe UI Emoji", NotoColorEmoji, "Segoe UI Symbol", "Android Emoji", EmojiSymbols;" dir="ltr"> Comment inline <div style="color: rgb(0, 0, 0);"> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Quan, Evan <Evan.Quan@amd.com> Sent: Friday, July 19, 2019 1:03 PM To: Wang, Kevin(Yang) <Kevin1.Wang@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org> Cc: Feng, Kenneth <Kenneth.Feng@amd.com>; Huang, Ray <Ray.Huang@amd.com>; Xu, Feifei <Feifei.Xu@amd.com>; Gui, Jack <Jack.Gui@amd.com> Subject: RE: [PATCH] drm/amd/powerplay: custom peak clock freq for navi10 <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">Comment inline > -----Original Message----- > From: Wang, Kevin(Yang) <Kevin1.Wang@amd.com> > Sent: Friday, July 19, 2019 11:46 AM > To: amd-gfx@lists.freedesktop.org > Cc: Feng, Kenneth <Kenneth.Feng@amd.com>; Quan, Evan > <Evan.Quan@amd.com>; Huang, Ray <Ray.Huang@amd.com>; Xu, Feifei > <Feifei.Xu@amd.com>; Gui, Jack <Jack.Gui@amd.com>; Wang, Kevin(Yang) > <Kevin1.Wang@amd.com> > 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@amd.com> > --- > 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().</div> <div class="PlainText"> </div> <div class="PlainText">[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.</div> <div class="PlainText"> > + 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.</div> <div class="PlainText">[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 </div> </div> </div> </div> </body> </html>