[PATCH 3/6] drm/amd/powerplay: fix bug that get wrong polaris evv voltage.

[Zhu, Rex]

Hi  Alexandre,


I have applied this patch, I  will generate a new patch to fix the typos as you mentioned.


Thanks for your review.


Best Regards

Rex

________________________________
From: Alexandre Demers <alexandre.f.demers at gmail.com>
Sent: Thursday, July 7, 2016 12:49:53 PM
To: Zhu, Rex
Cc: amd-gfx at lists.freedesktop.org
Subject: Re: [PATCH 3/6] drm/amd/powerplay: fix bug that get wrong polaris evv voltage.

Small suggestion below while at it if it was not applied yet.

Alexandre Demers

On 2016-07-05 05:48, Rex Zhu wrote:
> Change-Id: Ied43d26e1b90d8d9cd6ab5ea4ab432791b48bd84
> Signed-off-by: Rex Zhu <Rex.Zhu at amd.com>
> Reviewed-by: Ken Wang <Qingqing.Wang at amd.com>
> ---
>   drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c | 7 ++++---
>   drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c      | 4 ++--
>   drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.h      | 2 +-
>   3 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c
> index baa371f..a42610ee 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c
> @@ -2878,7 +2878,7 @@ static int polaris10_get_evv_voltages(struct pp_hwmgr *hwmgr)
>   {
>        struct polaris10_hwmgr *data = (struct polaris10_hwmgr *)(hwmgr->backend);
>        uint16_t vv_id;
> -     uint16_t vddc = 0;
> +     uint32_t vddc = 0;
>        uint16_t i, j;
>        uint32_t sclk = 0;
>        struct phm_ppt_v1_information *table_info =
> @@ -2909,8 +2909,9 @@ static int polaris10_get_evv_voltages(struct pp_hwmgr *hwmgr)
>                                                continue);
>
>
> -                     /* need to make sure vddc is less than 2v or else, it could burn the ASIC. */
> -                     PP_ASSERT_WITH_CODE((vddc < 2000 && vddc != 0),
> +                     /* need to make sure vddc is less than 2v or else, it could burn the ASIC.
> +                      * real voltage level in unit of 0.01mv */
Could you fix the volt and millivolt symbols here while you're at it?
They should be "V" and "mV" respectively. Thanks.

> +                     PP_ASSERT_WITH_CODE((vddc < 200000 && vddc != 0),
>                                        "Invalid VDDC value", result = -EINVAL;);
>
>                        /* the voltage should not be zero nor equal to leakage ID */
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c b/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c
> index 5d70e2c..a64db7b 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.c
> @@ -1256,7 +1256,7 @@ int atomctrl_set_ac_timing_ai(struct pp_hwmgr *hwmgr, uint32_t memory_clock,
>   }
>
>   int atomctrl_get_voltage_evv_on_sclk_ai(struct pp_hwmgr *hwmgr, uint8_t voltage_type,
> -                             uint32_t sclk, uint16_t virtual_voltage_Id, uint16_t *voltage)
> +                             uint32_t sclk, uint16_t virtual_voltage_Id, uint32_t *voltage)
>   {
>
>        int result;
> @@ -1274,7 +1274,7 @@ int atomctrl_get_voltage_evv_on_sclk_ai(struct pp_hwmgr *hwmgr, uint8_t voltage_
>        if (0 != result)
>                return result;
>
> -     *voltage = get_voltage_info_param_space.usVoltageLevel;
> +     *voltage = ((GET_EVV_VOLTAGE_INFO_OUTPUT_PARAMETER_V1_3 *)(&get_voltage_info_param_space))->ulVoltageLevel;
>
>        return result;
>   }
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.h b/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.h
> index 248c5db..1e35a96 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.h
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/ppatomctrl.h
> @@ -305,7 +305,7 @@ extern int atomctrl_get_engine_pll_dividers_ai(struct pp_hwmgr *hwmgr, uint32_t
>   extern int atomctrl_set_ac_timing_ai(struct pp_hwmgr *hwmgr, uint32_t memory_clock,
>                                                                uint8_t level);
>   extern int atomctrl_get_voltage_evv_on_sclk_ai(struct pp_hwmgr *hwmgr, uint8_t voltage_type,
> -                             uint32_t sclk, uint16_t virtual_voltage_Id, uint16_t *voltage);
> +                             uint32_t sclk, uint16_t virtual_voltage_Id, uint32_t *voltage);
>   extern int atomctrl_get_smc_sclk_range_table(struct pp_hwmgr *hwmgr, struct pp_atom_ctrl_sclk_range_table *table);
>
>   extern int atomctrl_get_avfs_information(struct pp_hwmgr *hwmgr, struct pp_atom_ctrl__avfs_parameters *param);

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20160707/3b893cd8/attachment-0001.html>