[PATCH 2/6] drm/amd/pp: Add hwmgr_sw_init/fini functioins

Zhu, Rex Rex.Zhu at amd.com
Thu Mar 22 14:49:14 UTC 2018


Thanks Alex.


I should be more careful when reorder patches .



Best Regards

Rex


________________________________
From: Alex Deucher <alexdeucher at 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

On Thu, Mar 22, 2018 at 9:13 AM, Alex Deucher <alexdeucher at gmail.com> wrote:
> On Thu, Mar 22, 2018 at 9:10 AM, Alex Deucher <alexdeucher at gmail.com> wrote:
>> On Thu, Mar 22, 2018 at 7:40 AM, Rex Zhu <Rex.Zhu at amd.com> wrote:
>>> Clean up pp ip functions
>>>
>>> Change-Id: Id06159202edabdfcb19fcc20e1291ce93db56d5d
>>> Signed-off-by: Rex Zhu <Rex.Zhu at amd.com>
>>
>> Reviewed-by: Alex Deucher <alexander.deucher at 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 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");
>>>
>>>         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
amd-gfx Info Page - freedesktop.org<https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
lists.freedesktop.org
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 ...



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


More information about the amd-gfx mailing list