On Wed, Mar 7, 2018 at 5:46 AM, Rex Zhu <Rex.Zhu at amd.com> wrote: > 1. make functions static > 2. add rv_read_arg_from_smc to smu backend function table. Please make this two patches since there are two logical changes here. Also see my additional comment below. > > Change-Id: I9baecaec95553923e59d1a4cf92a67551769b755 > Signed-off-by: Rex Zhu <Rex.Zhu at amd.com> > --- > drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c | 12 ++++++------ > drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.c | 18 ++++++++---------- > drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.h | 2 -- > 3 files changed, 14 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c > index 4b5c5fc..474612f 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c > @@ -386,11 +386,11 @@ static int rv_populate_clock_table(struct pp_hwmgr *hwmgr) > ARRAY_SIZE(VddPhyClk), &VddPhyClk[0]); > > smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetMinGfxclkFrequency); > - rv_read_arg_from_smc(hwmgr, &result); > + result = smum_get_argument(hwmgr); > rv_data->gfx_min_freq_limit = result * 100; > > smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetMaxGfxclkFrequency); > - rv_read_arg_from_smc(hwmgr, &result); > + result = smum_get_argument(hwmgr); > rv_data->gfx_max_freq_limit = result * 100; > > return 0; > @@ -726,7 +726,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr, > switch (type) { > case PP_SCLK: > smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetGfxclkFrequency); > - rv_read_arg_from_smc(hwmgr, &now); > + now = smum_get_argument(hwmgr); > > size += sprintf(buf + size, "0: %uMhz %s\n", > data->gfx_min_freq_limit / 100, > @@ -739,7 +739,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr, > break; > case PP_MCLK: > smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetFclkFrequency); > - rv_read_arg_from_smc(hwmgr, &now); > + now = smum_get_argument(hwmgr); > > for (i = 0; i < mclk_table->count; i++) > size += sprintf(buf + size, "%d: %uMhz %s\n", > @@ -971,14 +971,14 @@ static int rv_read_sensor(struct pp_hwmgr *hwmgr, int idx, > switch (idx) { > case AMDGPU_PP_SENSOR_GFX_SCLK: > smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetGfxclkFrequency); > - rv_read_arg_from_smc(hwmgr, &sclk); > + sclk = smum_get_argument(hwmgr); > /* in units of 10KHZ */ > *((uint32_t *)value) = sclk * 100; > *size = 4; > break; > case AMDGPU_PP_SENSOR_GFX_MCLK: > smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetFclkFrequency); > - rv_read_arg_from_smc(hwmgr, &mclk); > + mclk = smum_get_argument(hwmgr); > /* in units of 10KHZ */ > *((uint32_t *)value) = mclk * 100; > *size = 4; > diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.c b/drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.c > index b64bfe3..7f2866c 100644 > --- a/drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.c > +++ b/drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.c > @@ -60,7 +60,7 @@ static uint32_t rv_wait_for_response(struct pp_hwmgr *hwmgr) > return cgs_read_register(hwmgr->device, reg); > } > > -int rv_send_msg_to_smc_without_waiting(struct pp_hwmgr *hwmgr, > +static int rv_send_msg_to_smc_without_waiting(struct pp_hwmgr *hwmgr, > uint16_t msg) > { > uint32_t reg; > @@ -72,19 +72,17 @@ int rv_send_msg_to_smc_without_waiting(struct pp_hwmgr *hwmgr, > return 0; > } > > -int rv_read_arg_from_smc(struct pp_hwmgr *hwmgr, uint32_t *arg) > +static int rv_read_arg_from_smc(struct pp_hwmgr *hwmgr) > { > uint32_t reg; > > reg = soc15_get_register_offset(MP1_HWID, 0, > mmMP1_SMN_C2PMSG_82_BASE_IDX, mmMP1_SMN_C2PMSG_82); > > - *arg = cgs_read_register(hwmgr->device, reg); > - > - return 0; > + return cgs_read_register(hwmgr->device, reg); > } > > -int rv_send_msg_to_smc(struct pp_hwmgr *hwmgr, uint16_t msg) > +static int rv_send_msg_to_smc(struct pp_hwmgr *hwmgr, uint16_t msg) > { > uint32_t reg; > > @@ -103,7 +101,7 @@ int rv_send_msg_to_smc(struct pp_hwmgr *hwmgr, uint16_t msg) > } > > > -int rv_send_msg_to_smc_with_parameter(struct pp_hwmgr *hwmgr, > +static int rv_send_msg_to_smc_with_parameter(struct pp_hwmgr *hwmgr, > uint16_t msg, uint32_t parameter) > { > uint32_t reg; > @@ -190,8 +188,7 @@ static int rv_verify_smc_interface(struct pp_hwmgr *hwmgr) > > rv_send_msg_to_smc(hwmgr, > PPSMC_MSG_GetDriverIfVersion); > - rv_read_arg_from_smc(hwmgr, > - &smc_driver_if_version); > + smc_driver_if_version = rv_read_arg_from_smc(hwmgr); > > if (smc_driver_if_version != SMU10_DRIVER_IF_VERSION) { > pr_err("Attempt to read SMC IF Version Number Failed!\n"); > @@ -253,7 +250,7 @@ static int rv_start_smu(struct pp_hwmgr *hwmgr) > struct cgs_firmware_info info = {0}; > > smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetSmuVersion); > - rv_read_arg_from_smc(hwmgr, &hwmgr->smu_version); > + hwmgr->smu_version = rv_read_arg_from_smc(hwmgr); > info.version = hwmgr->smu_version >> 8; > > cgs_get_firmware_info(hwmgr->device, CGS_UCODE_ID_SMU, &info); > @@ -334,6 +331,7 @@ static int rv_smu_init(struct pp_hwmgr *hwmgr) > .send_msg_to_smc_with_parameter = &rv_send_msg_to_smc_with_parameter, > .download_pptable_settings = NULL, > .upload_pptable_settings = NULL, > + .get_argument = rv_read_arg_from_smc, > }; > > > diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.h b/drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.h > index 0ff4ac5..a3bfdee9 100644 > --- a/drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.h > +++ b/drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.h > @@ -50,8 +50,6 @@ struct rv_smumgr { > struct smu_table_array smu_tables; > }; > > -int rv_read_arg_from_smc(struct pp_hwmgr *hwmgr, uint32_t *arg); > -bool rv_is_smc_ram_running(struct pp_hwmgr *hwmgr); > int rv_copy_table_from_smc(struct pp_hwmgr *hwmgr, > uint8_t *table, int16_t table_id); > int rv_copy_table_to_smc(struct pp_hwmgr *hwmgr, This should be part of patch 3 I think. With all of these changes addressed: Reviewed-by: Alex Deucher <alexander.deucher at amd.com> > -- > 1.9.1 > > _______________________________________________ > amd-gfx mailing list > amd-gfx at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx