<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr"> Thanks Alex. I should be more careful when reorder patches . Best Regards Rex <div style="color: rgb(0, 0, 0);"> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Alex Deucher <alexdeucher@gmail.com> Sent: Thursday, March 22, 2018 9:13 PM To: Zhu, Rex Cc: amd-gfx list Subject: Re: [PATCH 2/6] drm/amd/pp: Add hwmgr_sw_init/fini functioins <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">On Thu, Mar 22, 2018 at 9:13 AM, Alex Deucher <alexdeucher@gmail.com> wrote: > On Thu, Mar 22, 2018 at 9:10 AM, Alex Deucher <alexdeucher@gmail.com> wrote: >> On Thu, Mar 22, 2018 at 7:40 AM, Rex Zhu <Rex.Zhu@amd.com> wrote: >>> Clean up pp ip functions >>> >>> Change-Id: Id06159202edabdfcb19fcc20e1291ce93db56d5d >>> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com> >> >> Reviewed-by: Alex Deucher <alexander.deucher@amd.com> > > Actually, doesn't this patch have a dependency on patch 3? I think > this part of patch 3: Sorry, patch 4. > + hwmgr->pm_en = (amdgpu_dpm && hwmgr->not_vf) ? true : false; > should be in this patch rather than the other patch. With that fixed: > Reviewed-by: Alex Deucher <alexander.deucher@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"); >>> >>> 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@lists.freedesktop.org >>> <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" id="LPlnk514271" previewremoved="true"> https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> <div id="LPBorder_GT_15217296746250.8377676973900243" style="margin-bottom: 20px; overflow: auto; width: 100%; text-indent: 0px;"> <table id="LPContainer_15217296746220.4412012350774459" role="presentation" cellspacing="0" style="width: 90%; background-color: rgb(255, 255, 255); position: relative; overflow: auto; padding-top: 20px; padding-bottom: 20px; margin-top: 20px; border-top: 1px dotted rgb(200, 200, 200); border-bottom: 1px dotted rgb(200, 200, 200);"> <tbody> <tr valign="top" style="border-spacing: 0px;"> <td id="TextCell_15217296746230.3999022866005415" colspan="2" style="vertical-align: top; position: relative; padding: 0px; display: table-cell;"> <div id="LPRemovePreviewContainer_15217296746230.7515492768466687"></div> <div id="LPTitle_15217296746230.12004872687650803" style="top: 0px; color: rgb(0, 120, 215); font-weight: normal; font-size: 21px; font-family: wf_segoe-ui_light, "Segoe UI Light", "Segoe WP Light", "Segoe UI", "Segoe WP", Tahoma, Arial, sans-serif; line-height: 21px;"> <a id="LPUrlAnchor_15217296746240.5079464424197266" href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" target="_blank" style="text-decoration: none;">amd-gfx Info Page - freedesktop.org</a></div> <div id="LPMetadata_15217296746240.5706125124067862" style="margin: 10px 0px 16px; color: rgb(102, 102, 102); font-weight: normal; font-family: wf_segoe-ui_normal, "Segoe UI", "Segoe WP", Tahoma, Arial, sans-serif; font-size: 14px; line-height: 14px;"> lists.freedesktop.org</div> <div id="LPDescription_15217296746250.7332168276446271" style="display: block; color: rgb(102, 102, 102); font-weight: normal; font-family: wf_segoe-ui_normal, "Segoe UI", "Segoe WP", Tahoma, Arial, sans-serif; font-size: 14px; line-height: 20px; max-height: 100px; overflow: hidden;"> Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the following form. Use of all freedesktop.org lists is subject to our Code of ...</div> </td> </tr> </tbody> </table> </div> </div> </div> </div> </div> </body> </html>