<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <meta name="Generator" content="Microsoft Exchange Server"> <style></style> </head> <body> <meta content="text/html; charset=UTF-8"> <style type="text/css" style="">  </style> <div dir="ltr"> <div id="x_divtagdefaultwrapper" style="font-size:12pt; color:#000000; background-color:#FFFFFF; font-family:Calibri,Arial,Helvetica,sans-serif"> 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 </div> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="x_divRplyFwdMsg" dir="ltr">From: Alexandre Demers <alexandre.f.demers@gmail.com> Sent: Thursday, July 7, 2016 12:49:53 PM To: Zhu, Rex Cc: amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 3/6] drm/amd/powerplay: fix bug that get wrong polaris evv voltage. <div> </div> </div> </div> <div class="PlainText">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@amd.com> > Reviewed-by: Ken Wang <Qingqing.Wang@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); </div> </body> </html>