<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="#0563C1" vlink="#954F72"> <div class="WordSection1"> Parameter 0xff for message PPSMC_MSG_GetDpmFreqByIndex will get the dpm numbers, so it’s ok.<o:p></o:p> Reviewed-by: Kenneth Feng <<a href="mailto:kenneth.feng@amd.com">kenneth.feng@amd.com</a>><o:p></o:p> <o:p> </o:p> <o:p> </o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> From: Wang, Kevin(Yang) Sent: Friday, July 19, 2019 1:36 PM To: Quan, Evan <Evan.Quan@amd.com>; 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<o:p></o:p> </div> </div> <o:p> </o:p> <div id="divtagdefaultwrapper"> Comment inline<o:p></o:p> <div> <div class="MsoNormal" align="center" style="text-align:center"> <hr size="2" width="98%" align="center"> </div> <div id="divRplyFwdMsg"> From: Quan, Evan <<a href="mailto:Evan.Quan@amd.com">Evan.Quan@amd.com</a>> Sent: Friday, July 19, 2019 1:03 PM To: Wang, Kevin(Yang) <<a href="mailto:Kevin1.Wang@amd.com">Kevin1.Wang@amd.com</a>>; <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> <<a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>> Cc: Feng, Kenneth <<a href="mailto:Kenneth.Feng@amd.com">Kenneth.Feng@amd.com</a>>; Huang, Ray <<a href="mailto:Ray.Huang@amd.com">Ray.Huang@amd.com</a>>; Xu, Feifei <<a href="mailto:Feifei.Xu@amd.com">Feifei.Xu@amd.com</a>>; Gui, Jack <<a href="mailto:Jack.Gui@amd.com">Jack.Gui@amd.com</a>> Subject: RE: [PATCH] drm/amd/powerplay: custom peak clock freq for navi10 <o:p></o:p> <div> <o:p></o:p> </div> </div> <div> <div> Comment inline > -----Original Message----- > From: Wang, Kevin(Yang) <<a href="mailto:Kevin1.Wang@amd.com">Kevin1.Wang@amd.com</a>> > Sent: Friday, July 19, 2019 11:46 AM > To: <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> > Cc: Feng, Kenneth <<a href="mailto:Kenneth.Feng@amd.com">Kenneth.Feng@amd.com</a>>; Quan, Evan > <<a href="mailto:Evan.Quan@amd.com">Evan.Quan@amd.com</a>>; Huang, Ray <<a href="mailto:Ray.Huang@amd.com">Ray.Huang@amd.com</a>>; Xu, Feifei > <<a href="mailto:Feifei.Xu@amd.com">Feifei.Xu@amd.com</a>>; Gui, Jack <<a href="mailto:Jack.Gui@amd.com">Jack.Gui@amd.com</a>>; Wang, Kevin(Yang) > <<a href="mailto:Kevin1.Wang@amd.com">Kevin1.Wang@amd.com</a>> > 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 <<a href="mailto:kevin1.wang@amd.com">kevin1.wang@amd.com</a>> > --- > 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().<o:p></o:p> </div> <div> <o:p> </o:p> </div> <div> [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.<o:p></o:p> </div> <div> > + 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.<o:p></o:p> </div> <div> [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<o:p></o:p> </div> </div> </div> </div> </div> </body> </html>