On 2018-10-30 3:34 p.m., David Francis wrote:
> We were multiplying clock requests by 1000 in amdgpu_dm
> and then dividing them by 1000 in powerplay.
> 
> Also, the vega12 code was dividing by 10 when it should have been
> multiplying (to convert units of 10kHz to units of kHz).
> 
> Signed-off-by: David Francis <david.fran...@amd.com>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 6 +++---
>  drivers/gpu/drm/amd/include/dm_pp_interface.h            | 2 +-
>  drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c        | 4 ++--
>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c       | 4 ++--
>  drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c       | 4 ++--
>  drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c       | 4 ++--
>  6 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> index d9daa038fdb2..cfa9b7f545b8 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> @@ -436,7 +436,7 @@ bool dm_pp_apply_clock_for_voltage_request(
>       int ret = 0;
>  
>       pp_clock_request.clock_type = 
> dc_to_pp_clock_type(clock_for_voltage_req->clk_type);
> -     pp_clock_request.clock_freq_in_khz = 
> clock_for_voltage_req->clocks_in_khz;
> +     pp_clock_request.clock_freq_in_mhz = 
> clock_for_voltage_req->clocks_in_khz / 1000;
>  
>       if (!pp_clock_request.clock_type)
>               return false;
> @@ -485,11 +485,11 @@ void pp_rv_set_display_requirement(struct pp_smu *pp,
>               return;
>  
>       clock.clock_type = amd_pp_dcf_clock;
> -     clock.clock_freq_in_khz = req->hard_min_dcefclk_mhz * 1000;
> +     clock.clock_freq_in_mhz = req->hard_min_dcefclk_mhz;
>       pp_funcs->display_clock_voltage_request(pp_handle, &clock);
>  
>       clock.clock_type = amd_pp_f_clock;
> -     clock.clock_freq_in_khz = req->hard_min_fclk_mhz * 1000;
> +     clock.clock_freq_in_mhz = req->hard_min_fclk_mhz;
>       pp_funcs->display_clock_voltage_request(pp_handle, &clock);
>  }
>  
> diff --git a/drivers/gpu/drm/amd/include/dm_pp_interface.h 
> b/drivers/gpu/drm/amd/include/dm_pp_interface.h
> index 1d93a0c574c9..114ddd03e238 100644
> --- a/drivers/gpu/drm/amd/include/dm_pp_interface.h
> +++ b/drivers/gpu/drm/amd/include/dm_pp_interface.h
> @@ -188,7 +188,7 @@ struct pp_clock_levels_with_voltage {
>  
>  struct pp_display_clock_request {
>       enum amd_pp_clock_type clock_type;
> -     uint32_t clock_freq_in_khz;
> +     uint32_t clock_freq_in_mhz;
>  };
>  
>  #endif /* _DM_PP_INTERFACE_ */
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> index dd18cb710391..d6a6a4f4ac9d 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> @@ -57,7 +57,7 @@ static int smu10_display_clock_voltage_request(struct 
> pp_hwmgr *hwmgr,
>  {
>       struct smu10_hwmgr *smu10_data = (struct smu10_hwmgr *)(hwmgr->backend);
>       enum amd_pp_clock_type clk_type = clock_req->clock_type;
> -     uint32_t clk_freq = clock_req->clock_freq_in_khz / 1000;
> +     uint32_t clk_freq = clock_req->clock_freq_in_mhz;
>       PPSMC_Msg        msg;
>  
>       switch (clk_type) {
> @@ -203,7 +203,7 @@ static int smu10_set_clock_limit(struct pp_hwmgr *hwmgr, 
> const void *input)
>  
>       clocks.dcefClock = hwmgr->display_config->min_dcef_set_clk;
>       clock_req.clock_type = amd_pp_dcf_clock;
> -     clock_req.clock_freq_in_khz = clocks.dcefClock * 10;
> +     clock_req.clock_freq_in_mhz = clocks.dcefClock / 100;
>  
>       PP_ASSERT_WITH_CODE(!smu10_display_clock_voltage_request(hwmgr, 
> &clock_req),
>                               "Attempt to set DCF Clock Failed!", return 
> -EINVAL);
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> index 419a1d77d661..f926a46bf256 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> @@ -3740,7 +3740,7 @@ int vega10_display_clock_voltage_request(struct 
> pp_hwmgr *hwmgr,
>  {
>       int result = 0;
>       enum amd_pp_clock_type clk_type = clock_req->clock_type;
> -     uint32_t clk_freq = clock_req->clock_freq_in_khz / 1000;
> +     uint32_t clk_freq = clock_req->clock_freq_in_mhz;
>       DSPCLK_e clk_select = 0;
>       uint32_t clk_request = 0;
>  
> @@ -3825,7 +3825,7 @@ static int 
> vega10_notify_smc_display_config_after_ps_adjustment(
>  
>       if (i < dpm_table->count) {
>               clock_req.clock_type = amd_pp_dcef_clock;
> -             clock_req.clock_freq_in_khz = dpm_table->dpm_levels[i].value * 
> 10;
> +             clock_req.clock_freq_in_mhz = dpm_table->dpm_levels[i].value / 
> 100;
>               if (!vega10_display_clock_voltage_request(hwmgr, &clock_req)) {
>                       smum_send_msg_to_smc_with_parameter(
>                                       hwmgr, PPSMC_MSG_SetMinDeepSleepDcefclk,
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
> index 74bc37308dc0..6478711f0b88 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
> @@ -1349,7 +1349,7 @@ int vega12_display_clock_voltage_request(struct 
> pp_hwmgr *hwmgr,
>       int result = 0;
>       struct vega12_hwmgr *data = (struct vega12_hwmgr *)(hwmgr->backend);
>       enum amd_pp_clock_type clk_type = clock_req->clock_type;
> -     uint32_t clk_freq = clock_req->clock_freq_in_khz / 1000;
> +     uint32_t clk_freq = clock_req->clock_freq_in_mhz;
>       PPCLK_e clk_select = 0;
>       uint32_t clk_request = 0;
>  
> @@ -1405,7 +1405,7 @@ static int 
> vega12_notify_smc_display_config_after_ps_adjustment(
>  
>       if (data->smu_features[GNLD_DPM_DCEFCLK].supported) {
>               clock_req.clock_type = amd_pp_dcef_clock;
> -             clock_req.clock_freq_in_khz = min_clocks.dcefClock/10;

Was this incorrect before? If so, it's probably better to have a separate patch 
to correct it (i assume changing '/' to '*') before doing the khz to mhz change.

Harry

> +             clock_req.clock_freq_in_mhz = min_clocks.dcefClock / 100;
>               if (!vega12_display_clock_voltage_request(hwmgr, &clock_req)) {
>                       if (data->smu_features[GNLD_DS_DCEFCLK].supported)
>                               PP_ASSERT_WITH_CODE(
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> index 57143d51e3ee..7cddc39e4cd3 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> @@ -1991,7 +1991,7 @@ int vega20_display_clock_voltage_request(struct 
> pp_hwmgr *hwmgr,
>       int result = 0;
>       struct vega20_hwmgr *data = (struct vega20_hwmgr *)(hwmgr->backend);
>       enum amd_pp_clock_type clk_type = clock_req->clock_type;
> -     uint32_t clk_freq = clock_req->clock_freq_in_khz / 1000;
> +     uint32_t clk_freq = clock_req->clock_freq_in_mhz;
>       PPCLK_e clk_select = 0;
>       uint32_t clk_request = 0;
>  
> @@ -2057,7 +2057,7 @@ static int 
> vega20_notify_smc_display_config_after_ps_adjustment(
>  
>       if (data->smu_features[GNLD_DPM_DCEFCLK].supported) {
>               clock_req.clock_type = amd_pp_dcef_clock;
> -             clock_req.clock_freq_in_khz = min_clocks.dcefClock * 10;
> +             clock_req.clock_freq_in_mhz = min_clocks.dcefClock / 100;
>               if (!vega20_display_clock_voltage_request(hwmgr, &clock_req)) {
>                       if (data->smu_features[GNLD_DS_DCEFCLK].supported)
>                               PP_ASSERT_WITH_CODE((ret = 
> smum_send_msg_to_smc_with_parameter(
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to