Good. Thanks Best Regards Rex -----Original Message----- From: Huang Rui [mailto:ray.huang@xxxxxxx] Sent: Friday, March 23, 2018 9:49 AM To: Zhu, Rex Cc: amd-gfx at lists.freedesktop.org Subject: Re: [PATCH 2/6] drm/amd/pp: Add hwmgr_sw_init/fini functioins On Thu, Mar 22, 2018 at 07:40:14PM +0800, Rex Zhu wrote: > Clean up pp ip functions > > Change-Id: Id06159202edabdfcb19fcc20e1291ce93db56d5d > Signed-off-by: Rex Zhu <Rex.Zhu at amd.com> > --- > drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 368 ++++++-------------------- > drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c | 74 ++++-- > drivers/gpu/drm/amd/powerplay/inc/hwmgr.h | 7 +- > 3 files changed, 145 insertions(+), 304 deletions(-) > > diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > index 7e8ad30..5d7d9ec 100644 > --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > @@ -31,24 +31,11 @@ > #include "amdgpu.h" > #include "hwmgr.h" > > -#define PP_DPM_DISABLED 0xCCCC > - > static int pp_dpm_dispatch_tasks(void *handle, enum amd_pp_task task_id, > enum amd_pm_state_type *user_state); > > static const struct amd_pm_funcs pp_dpm_funcs; > > -static inline int pp_check(struct pp_hwmgr *hwmgr) -{ > - if (hwmgr == NULL || hwmgr->smumgr_funcs == NULL) > - return -EINVAL; > - > - if (hwmgr->pm_en == 0 || hwmgr->hwmgr_func == NULL) > - return PP_DPM_DISABLED; > - > - return 0; > -} > - > static int amd_powerplay_create(struct amdgpu_device *adev) { > struct pp_hwmgr *hwmgr; > @@ -73,7 +60,7 @@ static int amd_powerplay_create(struct amdgpu_device > *adev) } > > > -static int amd_powerplay_destroy(struct amdgpu_device *adev) > +static void amd_powerplay_destroy(struct amdgpu_device *adev) > { > struct pp_hwmgr *hwmgr = adev->powerplay.pp_handle; > > @@ -82,8 +69,6 @@ static int amd_powerplay_destroy(struct > amdgpu_device *adev) > > kfree(hwmgr); > hwmgr = NULL; > - > - return 0; > } > > static int pp_early_init(void *handle) @@ -109,18 +94,9 @@ static int > pp_sw_init(void *handle) > struct pp_hwmgr *hwmgr = adev->powerplay.pp_handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - > - if (ret >= 0) { > - if (hwmgr->smumgr_funcs->smu_init == NULL) > - return -EINVAL; > + ret = hwmgr_sw_init(hwmgr); > > - ret = hwmgr->smumgr_funcs->smu_init(hwmgr); > - > - phm_register_irq_handlers(hwmgr); > - > - pr_debug("amdgpu: powerplay sw initialized\n"); > - } > + pr_debug("amdgpu: powerplay sw init %s\n", ret ? "failed" : > +"successfully"); Actually, we don't need to add the prefix like "amdgpu: ", because in pr_* prints, we already added the powerplay specific prefix. [ 105.758104] amdgpu: [powerplay] amdgpu: powerplay sw initialized ^^^^^^^ The same with comments with other part of this patch. Thanks, Ray > > return ret; > } > @@ -129,13 +105,8 @@ static int pp_sw_fini(void *handle) { > struct amdgpu_device *adev = handle; > struct pp_hwmgr *hwmgr = adev->powerplay.pp_handle; > - int ret = 0; > > - ret = pp_check(hwmgr); > - if (ret >= 0) { > - if (hwmgr->smumgr_funcs->smu_fini != NULL) > - hwmgr->smumgr_funcs->smu_fini(hwmgr); > - } > + hwmgr_sw_fini(hwmgr); > > if (adev->firmware.load_type == AMDGPU_FW_LOAD_SMU) > amdgpu_ucode_fini_bo(adev); > @@ -152,40 +123,20 @@ static int pp_hw_init(void *handle) > if (adev->firmware.load_type == AMDGPU_FW_LOAD_SMU) > amdgpu_ucode_init_bo(adev); > > - ret = pp_check(hwmgr); > + ret = hwmgr_hw_init(hwmgr); > > - if (ret >= 0) { > - if (hwmgr->smumgr_funcs->start_smu == NULL) > - return -EINVAL; > + if (ret) > + pr_err("amdgpu: powerplay hw init failed\n"); > > - if (hwmgr->smumgr_funcs->start_smu(hwmgr)) { > - pr_err("smc start failed\n"); > - hwmgr->smumgr_funcs->smu_fini(hwmgr); > - return -EINVAL; > - } > - if (ret == PP_DPM_DISABLED) > - goto exit; > - ret = hwmgr_hw_init(hwmgr); > - if (ret) > - goto exit; > - } > return ret; > -exit: > - hwmgr->pm_en = 0; > - cgs_notify_dpm_enabled(hwmgr->device, false); > - return 0; > - > } > > static int pp_hw_fini(void *handle) > { > struct amdgpu_device *adev = handle; > struct pp_hwmgr *hwmgr = adev->powerplay.pp_handle; > - int ret = 0; > > - ret = pp_check(hwmgr); > - if (ret == 0) > - hwmgr_hw_fini(hwmgr); > + hwmgr_hw_fini(hwmgr); > > return 0; > } > @@ -194,11 +145,8 @@ static int pp_late_init(void *handle) { > struct amdgpu_device *adev = handle; > struct pp_hwmgr *hwmgr = adev->powerplay.pp_handle; > - int ret = 0; > - > - ret = pp_check(hwmgr); > > - if (ret == 0) > + if (hwmgr && hwmgr->pm_en) > pp_dpm_dispatch_tasks(hwmgr, > AMD_PP_TASK_COMPLETE_INIT, NULL); > > @@ -233,12 +181,9 @@ static int pp_set_powergating_state(void *handle, > { > struct amdgpu_device *adev = handle; > struct pp_hwmgr *hwmgr = adev->powerplay.pp_handle; > - int ret = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return 0; > > if (hwmgr->hwmgr_func->enable_per_cu_power_gating == NULL) { > pr_info("%s was not implemented.\n", __func__); @@ -254,38 +199,16 > @@ static int pp_suspend(void *handle) { > struct amdgpu_device *adev = handle; > struct pp_hwmgr *hwmgr = adev->powerplay.pp_handle; > - int ret = 0; > > - ret = pp_check(hwmgr); > - if (ret == 0) > - hwmgr_hw_suspend(hwmgr); > - return 0; > + return hwmgr_suspend(hwmgr); > } > > static int pp_resume(void *handle) > { > struct amdgpu_device *adev = handle; > struct pp_hwmgr *hwmgr = adev->powerplay.pp_handle; > - int ret; > - > - ret = pp_check(hwmgr); > - > - if (ret < 0) > - return ret; > - > - if (hwmgr->smumgr_funcs->start_smu == NULL) > - return -EINVAL; > - > - if (hwmgr->smumgr_funcs->start_smu(hwmgr)) { > - pr_err("smc start failed\n"); > - hwmgr->smumgr_funcs->smu_fini(hwmgr); > - return -EINVAL; > - } > - > - if (ret == PP_DPM_DISABLED) > - return 0; > > - return hwmgr_hw_resume(hwmgr); > + return hwmgr_resume(hwmgr); > } > > static int pp_set_clockgating_state(void *handle, @@ -334,12 +257,9 > @@ static int pp_dpm_fw_loading_complete(void *handle) static int > pp_set_clockgating_by_smu(void *handle, uint32_t msg_id) { > struct pp_hwmgr *hwmgr = handle; > - int ret = 0; > - > - ret = pp_check(hwmgr); > > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return -EINVAL; > > if (hwmgr->hwmgr_func->update_clock_gatings == NULL) { > pr_info("%s was not implemented.\n", __func__); @@ -389,12 +309,9 > @@ static int pp_dpm_force_performance_level(void *handle, > enum amd_dpm_forced_level level) { > struct pp_hwmgr *hwmgr = handle; > - int ret = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return -EINVAL; > > if (level == hwmgr->dpm_level) > return 0; > @@ -412,13 +329,10 @@ static enum amd_dpm_forced_level pp_dpm_get_performance_level( > void *handle) > { > struct pp_hwmgr *hwmgr = handle; > - int ret = 0; > enum amd_dpm_forced_level level; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return -EINVAL; > > mutex_lock(&hwmgr->smu_lock); > level = hwmgr->dpm_level; > @@ -429,13 +343,10 @@ static enum amd_dpm_forced_level > pp_dpm_get_performance_level( static uint32_t pp_dpm_get_sclk(void > *handle, bool low) { > struct pp_hwmgr *hwmgr = handle; > - int ret = 0; > uint32_t clk = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return 0; > > if (hwmgr->hwmgr_func->get_sclk == NULL) { > pr_info("%s was not implemented.\n", __func__); @@ -450,13 +361,10 > @@ static uint32_t pp_dpm_get_sclk(void *handle, bool low) static > uint32_t pp_dpm_get_mclk(void *handle, bool low) { > struct pp_hwmgr *hwmgr = handle; > - int ret = 0; > uint32_t clk = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return 0; > > if (hwmgr->hwmgr_func->get_mclk == NULL) { > pr_info("%s was not implemented.\n", __func__); @@ -471,11 +379,8 > @@ static uint32_t pp_dpm_get_mclk(void *handle, bool low) static > void pp_dpm_powergate_vce(void *handle, bool gate) { > struct pp_hwmgr *hwmgr = handle; > - int ret = 0; > - > - ret = pp_check(hwmgr); > > - if (ret) > + if (!hwmgr || !hwmgr->pm_en) > return; > > if (hwmgr->hwmgr_func->powergate_vce == NULL) { @@ -490,11 +395,8 @@ > static void pp_dpm_powergate_vce(void *handle, bool gate) static void > pp_dpm_powergate_uvd(void *handle, bool gate) { > struct pp_hwmgr *hwmgr = handle; > - int ret = 0; > - > - ret = pp_check(hwmgr); > > - if (ret) > + if (!hwmgr || !hwmgr->pm_en) > return; > > if (hwmgr->hwmgr_func->powergate_uvd == NULL) { @@ -512,10 +414,8 @@ > static int pp_dpm_dispatch_tasks(void *handle, enum amd_pp_task task_id, > int ret = 0; > struct pp_hwmgr *hwmgr = handle; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return -EINVAL; > > mutex_lock(&hwmgr->smu_lock); > ret = hwmgr_handle_task(hwmgr, task_id, user_state); @@ -528,15 > +428,9 @@ static enum amd_pm_state_type > pp_dpm_get_current_power_state(void *handle) { > struct pp_hwmgr *hwmgr = handle; > struct pp_power_state *state; > - int ret = 0; > enum amd_pm_state_type pm_type; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > - > - if (hwmgr->current_ps == NULL) > + if (!hwmgr || !hwmgr->pm_en || !hwmgr->current_ps) > return -EINVAL; > > mutex_lock(&hwmgr->smu_lock); > @@ -568,11 +462,8 @@ static enum amd_pm_state_type > pp_dpm_get_current_power_state(void *handle) static void > pp_dpm_set_fan_control_mode(void *handle, uint32_t mode) { > struct pp_hwmgr *hwmgr = handle; > - int ret = 0; > - > - ret = pp_check(hwmgr); > > - if (ret) > + if (!hwmgr || !hwmgr->pm_en) > return; > > if (hwmgr->hwmgr_func->set_fan_control_mode == NULL) { @@ -587,13 > +478,10 @@ static void pp_dpm_set_fan_control_mode(void *handle, > uint32_t mode) static uint32_t pp_dpm_get_fan_control_mode(void > *handle) { > struct pp_hwmgr *hwmgr = handle; > - int ret = 0; > uint32_t mode = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return 0; > > if (hwmgr->hwmgr_func->get_fan_control_mode == NULL) { > pr_info("%s was not implemented.\n", __func__); @@ -610,10 +498,8 > @@ static int pp_dpm_set_fan_speed_percent(void *handle, uint32_t percent) > struct pp_hwmgr *hwmgr = handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return -EINVAL; > > if (hwmgr->hwmgr_func->set_fan_speed_percent == NULL) { > pr_info("%s was not implemented.\n", __func__); @@ -630,10 +516,8 > @@ static int pp_dpm_get_fan_speed_percent(void *handle, uint32_t *speed) > struct pp_hwmgr *hwmgr = handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return -EINVAL; > > if (hwmgr->hwmgr_func->get_fan_speed_percent == NULL) { > pr_info("%s was not implemented.\n", __func__); @@ -651,10 +535,8 > @@ static int pp_dpm_get_fan_speed_rpm(void *handle, uint32_t *rpm) > struct pp_hwmgr *hwmgr = handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return -EINVAL; > > if (hwmgr->hwmgr_func->get_fan_speed_rpm == NULL) > return -EINVAL; > @@ -670,16 +552,10 @@ static int pp_dpm_get_pp_num_states(void > *handle, { > struct pp_hwmgr *hwmgr = handle; > int i; > - int ret = 0; > > memset(data, 0, sizeof(*data)); > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > - > - if (hwmgr->ps == NULL) > + if (!hwmgr || !hwmgr->pm_en ||!hwmgr->ps) > return -EINVAL; > > mutex_lock(&hwmgr->smu_lock); > @@ -713,15 +589,9 @@ static int pp_dpm_get_pp_num_states(void *handle, > static int pp_dpm_get_pp_table(void *handle, char **table) { > struct pp_hwmgr *hwmgr = handle; > - int ret = 0; > int size = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > - > - if (!hwmgr->soft_pp_table) > + if (!hwmgr || !hwmgr->pm_en ||!hwmgr->soft_pp_table) > return -EINVAL; > > mutex_lock(&hwmgr->smu_lock); > @@ -736,10 +606,6 @@ static int amd_powerplay_reset(void *handle) > struct pp_hwmgr *hwmgr = handle; > int ret; > > - ret = pp_check(hwmgr); > - if (ret) > - return ret; > - > ret = hwmgr_hw_fini(hwmgr); > if (ret) > return ret; > @@ -756,10 +622,8 @@ static int pp_dpm_set_pp_table(void *handle, const char *buf, size_t size) > struct pp_hwmgr *hwmgr = handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return -EINVAL; > > mutex_lock(&hwmgr->smu_lock); > if (!hwmgr->hardcode_pp_table) { > @@ -796,10 +660,8 @@ static int pp_dpm_force_clock_level(void *handle, > struct pp_hwmgr *hwmgr = handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return -EINVAL; > > if (hwmgr->hwmgr_func->force_clock_level == NULL) { > pr_info("%s was not implemented.\n", __func__); @@ -820,10 +682,8 > @@ static int pp_dpm_print_clock_levels(void *handle, > struct pp_hwmgr *hwmgr = handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return -EINVAL; > > if (hwmgr->hwmgr_func->print_clock_levels == NULL) { > pr_info("%s was not implemented.\n", __func__); @@ -840,10 +700,8 > @@ static int pp_dpm_get_sclk_od(void *handle) > struct pp_hwmgr *hwmgr = handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return -EINVAL; > > if (hwmgr->hwmgr_func->get_sclk_od == NULL) { > pr_info("%s was not implemented.\n", __func__); @@ -860,10 +718,8 > @@ static int pp_dpm_set_sclk_od(void *handle, uint32_t value) > struct pp_hwmgr *hwmgr = handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return -EINVAL; > > if (hwmgr->hwmgr_func->set_sclk_od == NULL) { > pr_info("%s was not implemented.\n", __func__); @@ -881,10 +737,8 > @@ static int pp_dpm_get_mclk_od(void *handle) > struct pp_hwmgr *hwmgr = handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return -EINVAL; > > if (hwmgr->hwmgr_func->get_mclk_od == NULL) { > pr_info("%s was not implemented.\n", __func__); @@ -901,10 +755,8 > @@ static int pp_dpm_set_mclk_od(void *handle, uint32_t value) > struct pp_hwmgr *hwmgr = handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return -EINVAL; > > if (hwmgr->hwmgr_func->set_mclk_od == NULL) { > pr_info("%s was not implemented.\n", __func__); @@ -922,11 +774,7 > @@ static int pp_dpm_read_sensor(void *handle, int idx, > struct pp_hwmgr *hwmgr = handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - if (ret) > - return ret; > - > - if (value == NULL) > + if (!hwmgr || !hwmgr->pm_en || !value) > return -EINVAL; > > switch (idx) { > @@ -948,14 +796,11 @@ static int pp_dpm_read_sensor(void *handle, int > idx, pp_dpm_get_vce_clock_state(void *handle, unsigned idx) { > struct pp_hwmgr *hwmgr = handle; > - int ret = 0; > - > - ret = pp_check(hwmgr); > > - if (ret) > + if (!hwmgr || !hwmgr->pm_en) > return NULL; > > - if (hwmgr && idx < hwmgr->num_vce_state_tables) > + if (idx < hwmgr->num_vce_state_tables) > return &hwmgr->vce_states[idx]; > return NULL; > } > @@ -964,7 +809,7 @@ static int pp_get_power_profile_mode(void *handle, > char *buf) { > struct pp_hwmgr *hwmgr = handle; > > - if (!buf || pp_check(hwmgr)) > + if (!hwmgr || !hwmgr->pm_en || !buf) > return -EINVAL; > > if (hwmgr->hwmgr_func->get_power_profile_mode == NULL) { @@ -980,12 > +825,12 @@ static int pp_set_power_profile_mode(void *handle, long *input, uint32_t size) > struct pp_hwmgr *hwmgr = handle; > int ret = -EINVAL; > > - if (pp_check(hwmgr)) > - return -EINVAL; > + if (!hwmgr || !hwmgr->pm_en) > + return ret; > > if (hwmgr->hwmgr_func->set_power_profile_mode == NULL) { > pr_info("%s was not implemented.\n", __func__); > - return -EINVAL; > + return ret; > } > mutex_lock(&hwmgr->smu_lock); > if (hwmgr->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL) @@ -998,7 > +843,7 @@ static int pp_odn_edit_dpm_table(void *handle, uint32_t > type, long *input, uint3 { > struct pp_hwmgr *hwmgr = handle; > > - if (pp_check(hwmgr)) > + if (!hwmgr || !hwmgr->pm_en) > return -EINVAL; > > if (hwmgr->hwmgr_func->odn_edit_dpm_table == NULL) { @@ -1016,7 > +861,7 @@ static int pp_dpm_switch_power_profile(void *handle, > long workload; > uint32_t index; > > - if (pp_check(hwmgr)) > + if (!hwmgr || !hwmgr->pm_en) > return -EINVAL; > > if (hwmgr->hwmgr_func->set_power_profile_mode == NULL) { @@ -1058,10 > +903,8 @@ static int pp_dpm_notify_smu_memory_info(void *handle, > struct pp_hwmgr *hwmgr = handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return -EINVAL; > > if (hwmgr->hwmgr_func->notify_cac_buffer_info == NULL) { > pr_info("%s was not implemented.\n", __func__); @@ -1082,12 +925,9 > @@ static int pp_dpm_notify_smu_memory_info(void *handle, static int > pp_set_power_limit(void *handle, uint32_t limit) { > struct pp_hwmgr *hwmgr = handle; > - int ret = 0; > - > - ret = pp_check(hwmgr); > > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return -EINVAL; > > if (hwmgr->hwmgr_func->set_power_limit == NULL) { > pr_info("%s was not implemented.\n", __func__); @@ -1104,20 +944,14 > @@ static int pp_set_power_limit(void *handle, uint32_t limit) > hwmgr->hwmgr_func->set_power_limit(hwmgr, limit); > hwmgr->power_limit = limit; > mutex_unlock(&hwmgr->smu_lock); > - return ret; > + return 0; > } > > static int pp_get_power_limit(void *handle, uint32_t *limit, bool > default_limit) { > struct pp_hwmgr *hwmgr = handle; > - int ret = 0; > - > - ret = pp_check(hwmgr); > > - if (ret) > - return ret; > - > - if (limit == NULL) > + if (!hwmgr || !hwmgr->pm_en ||!limit) > return -EINVAL; > > mutex_lock(&hwmgr->smu_lock); > @@ -1129,19 +963,16 @@ static int pp_get_power_limit(void *handle, > uint32_t *limit, bool default_limit) > > mutex_unlock(&hwmgr->smu_lock); > > - return ret; > + return 0; > } > > static int pp_display_configuration_change(void *handle, > const struct amd_pp_display_configuration *display_config) { > struct pp_hwmgr *hwmgr = handle; > - int ret = 0; > - > - ret = pp_check(hwmgr); > > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return -EINVAL; > > mutex_lock(&hwmgr->smu_lock); > phm_store_dal_configuration_data(hwmgr, display_config); @@ -1155,12 > +986,7 @@ static int pp_get_display_power_level(void *handle, > struct pp_hwmgr *hwmgr = handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > - > - if (output == NULL) > + if (!hwmgr || !hwmgr->pm_en ||!output) > return -EINVAL; > > mutex_lock(&hwmgr->smu_lock); > @@ -1177,10 +1003,8 @@ static int pp_get_current_clocks(void *handle, > struct pp_hwmgr *hwmgr = handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return -EINVAL; > > mutex_lock(&hwmgr->smu_lock); > > @@ -1225,10 +1049,8 @@ static int pp_get_clock_by_type(void *handle, enum amd_pp_clock_type type, struc > struct pp_hwmgr *hwmgr = handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return -EINVAL; > > if (clocks == NULL) > return -EINVAL; > @@ -1246,11 +1068,7 @@ static int pp_get_clock_by_type_with_latency(void *handle, > struct pp_hwmgr *hwmgr = handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - if (ret) > - return ret; > - > - if (!clocks) > + if (!hwmgr || !hwmgr->pm_en ||!clocks) > return -EINVAL; > > mutex_lock(&hwmgr->smu_lock); > @@ -1266,11 +1084,7 @@ static int pp_get_clock_by_type_with_voltage(void *handle, > struct pp_hwmgr *hwmgr = handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - if (ret) > - return ret; > - > - if (!clocks) > + if (!hwmgr || !hwmgr->pm_en ||!clocks) > return -EINVAL; > > mutex_lock(&hwmgr->smu_lock); > @@ -1287,11 +1101,7 @@ static int pp_set_watermarks_for_clocks_ranges(void *handle, > struct pp_hwmgr *hwmgr = handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - if (ret) > - return ret; > - > - if (!wm_with_clock_ranges) > + if (!hwmgr || !hwmgr->pm_en ||!wm_with_clock_ranges) > return -EINVAL; > > mutex_lock(&hwmgr->smu_lock); > @@ -1308,11 +1118,7 @@ static int pp_display_clock_voltage_request(void *handle, > struct pp_hwmgr *hwmgr = handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - if (ret) > - return ret; > - > - if (!clock) > + if (!hwmgr || !hwmgr->pm_en ||!clock) > return -EINVAL; > > mutex_lock(&hwmgr->smu_lock); > @@ -1328,12 +1134,7 @@ static int pp_get_display_mode_validation_clocks(void *handle, > struct pp_hwmgr *hwmgr = handle; > int ret = 0; > > - ret = pp_check(hwmgr); > - > - if (ret) > - return ret; > - > - if (clocks == NULL) > + if (!hwmgr || !hwmgr->pm_en ||!clocks) > return -EINVAL; > > mutex_lock(&hwmgr->smu_lock); > @@ -1348,12 +1149,9 @@ static int > pp_get_display_mode_validation_clocks(void *handle, static int > pp_set_mmhub_powergating_by_smu(void *handle) { > struct pp_hwmgr *hwmgr = handle; > - int ret = 0; > - > - ret = pp_check(hwmgr); > > - if (ret) > - return ret; > + if (!hwmgr || !hwmgr->pm_en) > + return -EINVAL; > > if (hwmgr->hwmgr_func->set_mmhub_powergating_by_smu == NULL) { > pr_info("%s was not implemented.\n", __func__); diff --git > a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c > b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c > index 4298205..96a2d01 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c > @@ -76,7 +76,7 @@ static void hwmgr_init_workload_prority(struct > pp_hwmgr *hwmgr) > > int hwmgr_early_init(struct pp_hwmgr *hwmgr) { > - if (hwmgr == NULL) > + if (!hwmgr) > return -EINVAL; > > hwmgr->usec_timeout = AMD_MAX_USEC_TIMEOUT; @@ -170,17 +170,51 @@ > int hwmgr_early_init(struct pp_hwmgr *hwmgr) > return 0; > } > > +int hwmgr_sw_init(struct pp_hwmgr *hwmgr) { > + if (!hwmgr|| !hwmgr->smumgr_funcs || !hwmgr->smumgr_funcs->smu_init) > + return -EINVAL; > + > + phm_register_irq_handlers(hwmgr); > + > + return hwmgr->smumgr_funcs->smu_init(hwmgr); > +} > + > + > +int hwmgr_sw_fini(struct pp_hwmgr *hwmgr) { > + if (hwmgr && hwmgr->smumgr_funcs && hwmgr->smumgr_funcs->smu_fini) > + hwmgr->smumgr_funcs->smu_fini(hwmgr); > + > + return 0; > +} > + > int hwmgr_hw_init(struct pp_hwmgr *hwmgr) { > int ret = 0; > > - if (hwmgr == NULL) > + if (!hwmgr || !hwmgr->smumgr_funcs) > return -EINVAL; > > - if (hwmgr->pptable_func == NULL || > - hwmgr->pptable_func->pptable_init == NULL || > - hwmgr->hwmgr_func->backend_init == NULL) > - return -EINVAL; > + if (hwmgr->smumgr_funcs->start_smu) { > + ret = hwmgr->smumgr_funcs->start_smu(hwmgr); > + if (ret) { > + pr_err("smc start failed\n"); > + return -EINVAL; > + } > + } > + > + if (!hwmgr->pm_en) > + return 0; > + > + if (!hwmgr->pptable_func || > + !hwmgr->pptable_func->pptable_init || > + !hwmgr->hwmgr_func->backend_init) { > + hwmgr->pm_en = false; > + cgs_notify_dpm_enabled(hwmgr->device, false); > + pr_info("dpm not supported \n"); > + return 0; > + } > > ret = hwmgr->pptable_func->pptable_init(hwmgr); > if (ret) > @@ -214,14 +248,13 @@ int hwmgr_hw_init(struct pp_hwmgr *hwmgr) > if (hwmgr->pptable_func->pptable_fini) > hwmgr->pptable_func->pptable_fini(hwmgr); > err: > - pr_err("amdgpu: powerplay initialization failed\n"); > return ret; > } > > int hwmgr_hw_fini(struct pp_hwmgr *hwmgr) { > - if (hwmgr == NULL) > - return -EINVAL; > + if (!hwmgr || !hwmgr->pm_en) > + return 0; > > phm_stop_thermal_controller(hwmgr); > psm_set_boot_states(hwmgr); > @@ -236,12 +269,12 @@ int hwmgr_hw_fini(struct pp_hwmgr *hwmgr) > return psm_fini_power_state_table(hwmgr); > } > > -int hwmgr_hw_suspend(struct pp_hwmgr *hwmgr) > +int hwmgr_suspend(struct pp_hwmgr *hwmgr) > { > int ret = 0; > > - if (hwmgr == NULL) > - return -EINVAL; > + if (!hwmgr || !hwmgr->pm_en) > + return 0; > > phm_disable_smc_firmware_ctf(hwmgr); > ret = psm_set_boot_states(hwmgr); > @@ -255,13 +288,23 @@ int hwmgr_hw_suspend(struct pp_hwmgr *hwmgr) > return ret; > } > > -int hwmgr_hw_resume(struct pp_hwmgr *hwmgr) > +int hwmgr_resume(struct pp_hwmgr *hwmgr) > { > int ret = 0; > > - if (hwmgr == NULL) > + if (!hwmgr) > return -EINVAL; > > + if (hwmgr->smumgr_funcs && hwmgr->smumgr_funcs->start_smu) { > + if (hwmgr->smumgr_funcs->start_smu(hwmgr)) { > + pr_err("smc start failed\n"); > + return -EINVAL; > + } > + } > + > + if (!hwmgr->pm_en) > + return 0; > + > ret = phm_setup_asic(hwmgr); > if (ret) > return ret; > @@ -270,9 +313,6 @@ int hwmgr_hw_resume(struct pp_hwmgr *hwmgr) > if (ret) > return ret; > ret = phm_start_thermal_controller(hwmgr); > - if (ret) > - return ret; > - > ret |= psm_set_performance_states(hwmgr); > if (ret) > return ret; > diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h > b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h > index 17f811d..d6c9a3b 100644 > --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h > +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h > @@ -782,10 +782,13 @@ struct pp_hwmgr { }; > > int hwmgr_early_init(struct pp_hwmgr *hwmgr); > +int hwmgr_sw_init(struct pp_hwmgr *hwmgr); int hwmgr_sw_fini(struct > +pp_hwmgr *hwmgr); > int hwmgr_hw_init(struct pp_hwmgr *hwmgr); int hwmgr_hw_fini(struct > pp_hwmgr *hwmgr); -int hwmgr_hw_suspend(struct pp_hwmgr *hwmgr); -int > hwmgr_hw_resume(struct pp_hwmgr *hwmgr); > +int hwmgr_suspend(struct pp_hwmgr *hwmgr); int hwmgr_resume(struct > +pp_hwmgr *hwmgr); > + > int hwmgr_handle_task(struct pp_hwmgr *hwmgr, > enum amd_pp_task task_id, > enum amd_pm_state_type *user_state); > -- > 1.9.1 > > _______________________________________________ > amd-gfx mailing list > amd-gfx at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
