Re: [PATCH 2/2] drm/amdgpu: Change pp clock requests to mHz
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
Reviewed-by: Harry Wentland
Harry
> ---
> 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_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, );
> }
>
> 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_Msgmsg;
>
> 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,
> _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
Re: [PATCH 2/2] drm/amdgpu: Change pp clock requests to mHz
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
> ---
> 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_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, );
> }
>
> 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_Msgmsg;
>
> 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,
> _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, _req)) {
>
RE: [PATCH 2/2] drm/amdgpu: Change pp clock requests to mHz
Series is reviewed-by: Evan Quan
Regards,
Evan
> -Original Message-
> From: amd-gfx On Behalf Of
> David Francis
> Sent: Wednesday, October 31, 2018 3:35 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Francis, David
> Subject: [PATCH 2/2] drm/amdgpu: Change pp clock requests to mHz
>
> 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
> ---
> 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_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, ); }
>
> 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_Msgmsg;
>
> 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(h
> wmgr, _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;
> -
