> -----Original Message----- > From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf > Of Rex Zhu > Sent: Tuesday, September 26, 2017 1:24 AM > To: amd-gfx at lists.freedesktop.org > Cc: Zhu, Rex > Subject: [PATCH] drm/amd/powerplay: refine code in amd_powerplay.c > > delete flag of PP_DPM_DISABLED > pp_en in pp_handle is enough > > Change-Id: Iad7d48690e91e736da0f76b3a11a87d2cb519b05 > Signed-off-by: Rex Zhu <Rex.Zhu at amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c | 2 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_powerplay.c | 9 -- > drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 152 ++++++++++--- > --------- > drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h | 2 - > 4 files changed, 70 insertions(+), 95 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c > index fc9a50a..0ee33f3 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c > @@ -969,7 +969,7 @@ static int amdgpu_cgs_notify_dpm_enabled(struct > cgs_device *cgs_device, bool ena > { > CGS_FUNC_ADEV; > > - adev->pm.dpm_enabled = enabled; > + amdgpu_dpm = adev->pm.dpm_enabled = enabled; I don't think we want to set the global module option here. Alex > > return 0; > } > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_powerplay.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_powerplay.c > index 1649b1e..1be2e1f 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_powerplay.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_powerplay.c > @@ -98,10 +98,6 @@ static int amdgpu_pp_early_init(void *handle) > amd_pp->cgs_device ? amd_pp- > >cgs_device : > amd_pp->pp_handle); > > - if (ret == PP_DPM_DISABLED) { > - adev->pm.dpm_enabled = false; > - return 0; > - } > return ret; > } > > @@ -154,11 +150,6 @@ static int amdgpu_pp_hw_init(void *handle) > ret = adev->powerplay.ip_funcs->hw_init( > adev->powerplay.pp_handle); > > - if (ret == PP_DPM_DISABLED) { > - adev->pm.dpm_enabled = false; > - return 0; > - } > - > if ((amdgpu_dpm != 0) && !amdgpu_sriov_vf(adev)) > adev->pm.dpm_enabled = true; > > diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > index 7c1a974..f475438 100644 > --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > @@ -41,11 +41,8 @@ static inline int pp_check(struct pp_instance *handle) > if (handle->hwmgr == NULL || handle->hwmgr->smumgr_funcs == > NULL) > return -EINVAL; > > - if (handle->pm_en == 0) > - return PP_DPM_DISABLED; > - > if (handle->hwmgr->hwmgr_func == NULL) > - return PP_DPM_DISABLED; > + handle->pm_en = 0; > > return 0; > } > @@ -93,14 +90,8 @@ static int pp_early_init(void *handle) > pp_handle = cgs_register_pp_handle(handle, > amd_powerplay_create); > > ret = hwmgr_early_init(pp_handle); > - if (ret) > - return -EINVAL; > - > - if ((pp_handle->pm_en == 0) > - || cgs_is_virtualization_enabled(pp_handle->device)) > - return PP_DPM_DISABLED; > > - return 0; > + return ret; > } > > static int pp_sw_init(void *handle) > @@ -111,7 +102,7 @@ static int pp_sw_init(void *handle) > > ret = pp_check(pp_handle); > > - if (ret == 0 || ret == PP_DPM_DISABLED) { > + if (!ret) { > hwmgr = pp_handle->hwmgr; > > if (hwmgr->smumgr_funcs->smu_init == NULL) > @@ -131,7 +122,7 @@ static int pp_sw_fini(void *handle) > struct pp_instance *pp_handle = (struct pp_instance *)handle; > > ret = pp_check(pp_handle); > - if (ret == 0 || ret == PP_DPM_DISABLED) { > + if (!ret) { > hwmgr = pp_handle->hwmgr; > > if (hwmgr->smumgr_funcs->smu_fini == NULL) > @@ -150,7 +141,7 @@ static int pp_hw_init(void *handle) > > ret = pp_check(pp_handle); > > - if (ret == 0 || ret == PP_DPM_DISABLED) { > + if (!ret) { > hwmgr = pp_handle->hwmgr; > > if (hwmgr->smumgr_funcs->start_smu == NULL) > @@ -161,17 +152,16 @@ static int pp_hw_init(void *handle) > hwmgr->smumgr_funcs->smu_fini(pp_handle- > >hwmgr); > return -EINVAL;; > } > - if (ret == PP_DPM_DISABLED) > - return PP_DPM_DISABLED; > } > > - ret = hwmgr_hw_init(pp_handle); > - if (ret) > - goto err; > + if (pp_handle->pm_en) { > + ret = hwmgr_hw_init(pp_handle); > + if (ret) { > + pp_handle->pm_en = 0; > + cgs_notify_dpm_enabled(hwmgr->device, false); > + } > + } > return 0; > -err: > - pp_handle->pm_en = 0; > - return PP_DPM_DISABLED; > } > > static int pp_hw_fini(void *handle) > @@ -180,7 +170,7 @@ static int pp_hw_fini(void *handle) > int ret = 0; > > ret = pp_check(pp_handle); > - if (ret == 0) > + if (!ret && pp_handle->pm_en) > hwmgr_hw_fini(pp_handle); > > return 0; > @@ -192,7 +182,7 @@ static int pp_late_init(void *handle) > int ret = 0; > > ret = pp_check(pp_handle); > - if (ret == 0) > + if (!ret && pp_handle->pm_en) > pp_dpm_dispatch_tasks(pp_handle, > AMD_PP_TASK_COMPLETE_INIT, > NULL, NULL); > > @@ -229,7 +219,7 @@ int amd_set_clockgating_by_smu(void *handle, > uint32_t msg_id) > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -250,8 +240,7 @@ static int pp_set_powergating_state(void *handle, > int ret = 0; > > ret = pp_check(pp_handle); > - > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -273,24 +262,25 @@ static int pp_suspend(void *handle) > > ret = pp_check(pp_handle); > > - if (ret == PP_DPM_DISABLED) > - return 0; > - else if (ret != 0) > + if (ret) > return ret; > > - return hwmgr_hw_suspend(pp_handle); > + if (pp_handle->pm_en) > + ret = hwmgr_hw_suspend(pp_handle); > + > + return ret; > } > > static int pp_resume(void *handle) > { > struct pp_hwmgr *hwmgr; > - int ret, ret1; > + int ret; > struct pp_instance *pp_handle = (struct pp_instance *)handle; > > - ret1 = pp_check(pp_handle); > + ret = pp_check(pp_handle); > > - if (ret1 != 0 && ret1 != PP_DPM_DISABLED) > - return ret1; > + if (ret) > + return ret; > > hwmgr = pp_handle->hwmgr; > > @@ -303,11 +293,10 @@ static int pp_resume(void *handle) > hwmgr->smumgr_funcs->smu_fini(pp_handle->hwmgr); > return ret; > } > + if (pp_handle->pm_en) > + ret = hwmgr_hw_resume(pp_handle); > > - if (ret1 == PP_DPM_DISABLED) > - return 0; > - > - return hwmgr_hw_resume(pp_handle); > + return ret; > } > > const struct amd_ip_funcs pp_ip_funcs = { > @@ -383,7 +372,7 @@ static int pp_dpm_force_performance_level(void > *handle, > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -418,7 +407,7 @@ static enum amd_dpm_forced_level > pp_dpm_get_performance_level( > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -437,7 +426,7 @@ static uint32_t pp_dpm_get_sclk(void *handle, bool > low) > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -461,7 +450,7 @@ static uint32_t pp_dpm_get_mclk(void *handle, bool > low) > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -484,7 +473,7 @@ static void pp_dpm_powergate_vce(void *handle, > bool gate) > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return; > > hwmgr = pp_handle->hwmgr; > @@ -506,7 +495,7 @@ static void pp_dpm_powergate_uvd(void *handle, > bool gate) > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return; > > hwmgr = pp_handle->hwmgr; > @@ -528,7 +517,7 @@ static int pp_dpm_dispatch_tasks(void *handle, > enum amd_pp_task task_id, > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > mutex_lock(&pp_handle->pp_lock); > @@ -548,7 +537,7 @@ static enum amd_pm_state_type > pp_dpm_get_current_power_state(void *handle) > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -590,7 +579,7 @@ static void pp_dpm_set_fan_control_mode(void > *handle, uint32_t mode) > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return; > > hwmgr = pp_handle->hwmgr; > @@ -613,7 +602,7 @@ static uint32_t pp_dpm_get_fan_control_mode(void > *handle) > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -636,7 +625,7 @@ static int pp_dpm_set_fan_speed_percent(void > *handle, uint32_t percent) > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -659,7 +648,7 @@ static int pp_dpm_get_fan_speed_percent(void > *handle, uint32_t *speed) > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -683,7 +672,7 @@ static int pp_dpm_get_fan_speed_rpm(void *handle, > uint32_t *rpm) > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -705,7 +694,7 @@ static int pp_dpm_get_temperature(void *handle) > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -730,7 +719,7 @@ static int pp_dpm_get_pp_num_states(void *handle, > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -775,7 +764,7 @@ static int pp_dpm_get_pp_table(void *handle, char > **table) > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -798,7 +787,7 @@ static int pp_dpm_set_pp_table(void *handle, const > char *buf, size_t size) > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -822,13 +811,10 @@ static int pp_dpm_set_pp_table(void *handle, > const char *buf, size_t size) > if (ret) > return ret; > > - if (hwmgr->hwmgr_func->avfs_control) { > + if (hwmgr->hwmgr_func->avfs_control) > ret = hwmgr->hwmgr_func->avfs_control(hwmgr, false); > - if (ret) > - return ret; > - } > > - return 0; > + return ret; > } > > static int pp_dpm_force_clock_level(void *handle, > @@ -840,7 +826,7 @@ static int pp_dpm_force_clock_level(void *handle, > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -864,7 +850,7 @@ static int pp_dpm_print_clock_levels(void *handle, > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -887,7 +873,7 @@ static int pp_dpm_get_sclk_od(void *handle) > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -910,7 +896,7 @@ static int pp_dpm_set_sclk_od(void *handle, uint32_t > value) > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -934,7 +920,7 @@ static int pp_dpm_get_mclk_od(void *handle) > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -957,7 +943,7 @@ static int pp_dpm_set_mclk_od(void *handle, > uint32_t value) > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -981,7 +967,7 @@ static int pp_dpm_read_sensor(void *handle, int idx, > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -1007,7 +993,7 @@ static int pp_dpm_read_sensor(void *handle, int > idx, > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return NULL; > > hwmgr = pp_handle->hwmgr; > @@ -1187,11 +1173,11 @@ int amd_powerplay_reset(void *handle) > struct pp_instance *instance = (struct pp_instance *)handle; > int ret; > > - if (cgs_is_virtualization_enabled(instance->hwmgr->device)) > - return PP_DPM_DISABLED; > + if (!instance->pm_en) > + return 0; > > ret = pp_check(instance); > - if (ret != 0) > + if (ret) > return ret; > > ret = pp_hw_fini(instance); > @@ -1200,7 +1186,7 @@ int amd_powerplay_reset(void *handle) > > ret = hwmgr_hw_init(instance); > if (ret) > - return PP_DPM_DISABLED; > + return ret; > > return hwmgr_handle_task(instance, > AMD_PP_TASK_COMPLETE_INIT, NULL, NULL); > } > @@ -1216,7 +1202,7 @@ int > amd_powerplay_display_configuration_change(void *handle, > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -1235,7 +1221,7 @@ int amd_powerplay_get_display_power_level(void > *handle, > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -1260,7 +1246,7 @@ int amd_powerplay_get_current_clocks(void > *handle, > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -1277,7 +1263,7 @@ int amd_powerplay_get_current_clocks(void > *handle, > ret = phm_get_clock_info(hwmgr, &hwmgr->current_ps- > >hardware, > &hw_clocks, > PHM_PerformanceLevelDesignation_Activity); > > - if (ret != 0) { > + if (ret) { > pr_info("Error in phm_get_clock_info \n"); > mutex_unlock(&pp_handle->pp_lock); > return -EINVAL; > @@ -1311,7 +1297,7 @@ int amd_powerplay_get_clock_by_type(void > *handle, enum amd_pp_clock_type type, s > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > @@ -1334,7 +1320,7 @@ int > amd_powerplay_get_clock_by_type_with_latency(void *handle, > int ret = 0; > > ret = pp_check(pp_handle); > - if (ret != 0) > + if (ret) > return ret; > > if (!clocks) > @@ -1356,7 +1342,7 @@ int > amd_powerplay_get_clock_by_type_with_voltage(void *handle, > int ret = 0; > > ret = pp_check(pp_handle); > - if (ret != 0) > + if (ret) > return ret; > > if (!clocks) > @@ -1380,7 +1366,7 @@ int > amd_powerplay_set_watermarks_for_clocks_ranges(void *handle, > int ret = 0; > > ret = pp_check(pp_handle); > - if (ret != 0) > + if (ret) > return ret; > > if (!wm_with_clock_ranges) > @@ -1404,7 +1390,7 @@ int > amd_powerplay_display_clock_voltage_request(void *handle, > int ret = 0; > > ret = pp_check(pp_handle); > - if (ret != 0) > + if (ret) > return ret; > > if (!clock) > @@ -1428,7 +1414,7 @@ int > amd_powerplay_get_display_mode_validation_clocks(void *handle, > > ret = pp_check(pp_handle); > > - if (ret != 0) > + if (ret) > return ret; > > hwmgr = pp_handle->hwmgr; > diff --git a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h > b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h > index 916b6c4..e52adc8 100644 > --- a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h > +++ b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h > @@ -33,8 +33,6 @@ > extern const struct amd_ip_funcs pp_ip_funcs; > extern const struct amd_pm_funcs pp_dpm_funcs; > > -#define PP_DPM_DISABLED 0xCCCC > - > enum amd_pp_sensors { > AMDGPU_PP_SENSOR_GFX_SCLK = 0, > AMDGPU_PP_SENSOR_VDDNB, > -- > 1.9.1 > > _______________________________________________ > amd-gfx mailing list > amd-gfx at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
