> -----Original Message----- > From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf > Of Shirish S > Sent: Wednesday, May 30, 2018 6:21 AM > To: amd-gfx at lists.freedesktop.org; Wentland, Harry > <Harry.Wentland at amd.com>; Zhu, Rex <Rex.Zhu at amd.com> > Cc: S, Shirish <Shirish.S at amd.com> > Subject: [PATCH] drm/amdgpu/pp: replace mutex with spin_lock > > This patch replaces usage of mutex with spin_lock to avoid sleep in atomic > context. > > Signed-off-by: Shirish S <shirish.s at amd.com> Same question for this patch. Do we actually hit these paths in atomic contexts? Alex > --- > drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 152 > +++++++++++++------------- > drivers/gpu/drm/amd/powerplay/inc/hwmgr.h | 2 +- > 2 files changed, 77 insertions(+), 77 deletions(-) > > diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > index b493369..2d9c120 100644 > --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > @@ -183,10 +183,10 @@ static int pp_late_init(void *handle) > int ret; > > if (hwmgr && hwmgr->pm_en) { > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > hwmgr_handle_task(hwmgr, > AMD_PP_TASK_COMPLETE_INIT, > NULL); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > } > if (adev->pm.smu_prv_buffer_size != 0) > pp_reserve_vram_for_smu(adev); > @@ -375,11 +375,11 @@ static int pp_dpm_force_performance_level(void > *handle, > if (level == hwmgr->dpm_level) > return 0; > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > pp_dpm_en_umd_pstate(hwmgr, &level); > hwmgr->request_dpm_level = level; > hwmgr_handle_task(hwmgr, > AMD_PP_TASK_READJUST_POWER_STATE, NULL); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > > return 0; > } > @@ -393,9 +393,9 @@ static enum amd_dpm_forced_level > pp_dpm_get_performance_level( > if (!hwmgr || !hwmgr->pm_en) > return -EINVAL; > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > level = hwmgr->dpm_level; > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return level; > } > > @@ -411,9 +411,9 @@ static uint32_t pp_dpm_get_sclk(void *handle, bool > low) > pr_info("%s was not implemented.\n", __func__); > return 0; > } > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > clk = hwmgr->hwmgr_func->get_sclk(hwmgr, low); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return clk; > } > > @@ -429,9 +429,9 @@ static uint32_t pp_dpm_get_mclk(void *handle, bool > low) > pr_info("%s was not implemented.\n", __func__); > return 0; > } > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > clk = hwmgr->hwmgr_func->get_mclk(hwmgr, low); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return clk; > } > > @@ -446,9 +446,9 @@ static void pp_dpm_powergate_vce(void *handle, > bool gate) > pr_info("%s was not implemented.\n", __func__); > return; > } > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > hwmgr->hwmgr_func->powergate_vce(hwmgr, gate); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > } > > static void pp_dpm_powergate_uvd(void *handle, bool gate) @@ -462,9 > +462,9 @@ static void pp_dpm_powergate_uvd(void *handle, bool gate) > pr_info("%s was not implemented.\n", __func__); > return; > } > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > hwmgr->hwmgr_func->powergate_uvd(hwmgr, gate); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > } > > static int pp_dpm_dispatch_tasks(void *handle, enum amd_pp_task > task_id, @@ -476,9 +476,9 @@ static int pp_dpm_dispatch_tasks(void > *handle, enum amd_pp_task task_id, > if (!hwmgr || !hwmgr->pm_en) > return -EINVAL; > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > ret = hwmgr_handle_task(hwmgr, task_id, user_state); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > > return ret; > } > @@ -492,7 +492,7 @@ static enum amd_pm_state_type > pp_dpm_get_current_power_state(void *handle) > if (!hwmgr || !hwmgr->pm_en || !hwmgr->current_ps) > return -EINVAL; > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > > state = hwmgr->current_ps; > > @@ -513,7 +513,7 @@ static enum amd_pm_state_type > pp_dpm_get_current_power_state(void *handle) > pm_type = POWER_STATE_TYPE_DEFAULT; > break; > } > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > > return pm_type; > } > @@ -529,9 +529,9 @@ static void pp_dpm_set_fan_control_mode(void > *handle, uint32_t mode) > pr_info("%s was not implemented.\n", __func__); > return; > } > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > hwmgr->hwmgr_func->set_fan_control_mode(hwmgr, mode); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > } > > static uint32_t pp_dpm_get_fan_control_mode(void *handle) @@ -546,9 > +546,9 @@ static uint32_t pp_dpm_get_fan_control_mode(void *handle) > pr_info("%s was not implemented.\n", __func__); > return 0; > } > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > mode = hwmgr->hwmgr_func->get_fan_control_mode(hwmgr); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return mode; > } > > @@ -564,9 +564,9 @@ static int pp_dpm_set_fan_speed_percent(void > *handle, uint32_t percent) > pr_info("%s was not implemented.\n", __func__); > return 0; > } > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > ret = hwmgr->hwmgr_func->set_fan_speed_percent(hwmgr, > percent); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return ret; > } > > @@ -583,9 +583,9 @@ static int pp_dpm_get_fan_speed_percent(void > *handle, uint32_t *speed) > return 0; > } > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > ret = hwmgr->hwmgr_func->get_fan_speed_percent(hwmgr, > speed); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return ret; > } > > @@ -600,9 +600,9 @@ static int pp_dpm_get_fan_speed_rpm(void *handle, > uint32_t *rpm) > if (hwmgr->hwmgr_func->get_fan_speed_rpm == NULL) > return -EINVAL; > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > ret = hwmgr->hwmgr_func->get_fan_speed_rpm(hwmgr, rpm); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return ret; > } > > @@ -617,7 +617,7 @@ static int pp_dpm_get_pp_num_states(void *handle, > if (!hwmgr || !hwmgr->pm_en ||!hwmgr->ps) > return -EINVAL; > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > > data->nums = hwmgr->num_ps; > > @@ -641,7 +641,7 @@ static int pp_dpm_get_pp_num_states(void *handle, > data->states[i] = > POWER_STATE_TYPE_DEFAULT; > } > } > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return 0; > } > > @@ -653,10 +653,10 @@ static int pp_dpm_get_pp_table(void *handle, char > **table) > if (!hwmgr || !hwmgr->pm_en ||!hwmgr->soft_pp_table) > return -EINVAL; > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > *table = (char *)hwmgr->soft_pp_table; > size = hwmgr->soft_pp_table_size; > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return size; > } > > @@ -684,7 +684,7 @@ static int pp_dpm_set_pp_table(void *handle, const > char *buf, size_t size) > if (!hwmgr || !hwmgr->pm_en) > return -EINVAL; > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > if (!hwmgr->hardcode_pp_table) { > hwmgr->hardcode_pp_table = kmemdup(hwmgr- > >soft_pp_table, > hwmgr- > >soft_pp_table_size, > @@ -706,10 +706,10 @@ static int pp_dpm_set_pp_table(void *handle, > const char *buf, size_t size) > if (ret) > goto err; > } > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return 0; > err: > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return ret; > } > > @@ -726,12 +726,12 @@ static int pp_dpm_force_clock_level(void *handle, > pr_info("%s was not implemented.\n", __func__); > return 0; > } > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > if (hwmgr->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL) > ret = hwmgr->hwmgr_func->force_clock_level(hwmgr, type, > mask); > else > ret = -EINVAL; > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return ret; > } > > @@ -748,9 +748,9 @@ static int pp_dpm_print_clock_levels(void *handle, > pr_info("%s was not implemented.\n", __func__); > return 0; > } > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > ret = hwmgr->hwmgr_func->print_clock_levels(hwmgr, type, buf); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return ret; > } > > @@ -766,9 +766,9 @@ static int pp_dpm_get_sclk_od(void *handle) > pr_info("%s was not implemented.\n", __func__); > return 0; > } > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > ret = hwmgr->hwmgr_func->get_sclk_od(hwmgr); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return ret; > } > > @@ -785,9 +785,9 @@ static int pp_dpm_set_sclk_od(void *handle, uint32_t > value) > return 0; > } > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > ret = hwmgr->hwmgr_func->set_sclk_od(hwmgr, value); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return ret; > } > > @@ -803,9 +803,9 @@ static int pp_dpm_get_mclk_od(void *handle) > pr_info("%s was not implemented.\n", __func__); > return 0; > } > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > ret = hwmgr->hwmgr_func->get_mclk_od(hwmgr); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return ret; > } > > @@ -821,9 +821,9 @@ static int pp_dpm_set_mclk_od(void *handle, > uint32_t value) > pr_info("%s was not implemented.\n", __func__); > return 0; > } > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > ret = hwmgr->hwmgr_func->set_mclk_od(hwmgr, value); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return ret; > } > > @@ -844,9 +844,9 @@ static int pp_dpm_read_sensor(void *handle, int idx, > *((uint32_t *)value) = hwmgr->pstate_mclk; > return 0; > default: > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > ret = hwmgr->hwmgr_func->read_sensor(hwmgr, idx, value, > size); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return ret; > } > } > @@ -891,10 +891,10 @@ static int pp_set_power_profile_mode(void > *handle, long *input, uint32_t size) > pr_info("%s was not implemented.\n", __func__); > return ret; > } > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > if (hwmgr->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL) > ret = hwmgr->hwmgr_func- > >set_power_profile_mode(hwmgr, input, size); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return ret; > } > > @@ -931,7 +931,7 @@ static int pp_dpm_switch_power_profile(void > *handle, > if (!(type < PP_SMC_POWER_PROFILE_CUSTOM)) > return -EINVAL; > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > > if (!en) { > hwmgr->workload_mask &= ~(1 << hwmgr- > >workload_prority[type]); @@ -947,7 +947,7 @@ static int > pp_dpm_switch_power_profile(void *handle, > > if (hwmgr->dpm_level != AMD_DPM_FORCED_LEVEL_MANUAL) > hwmgr->hwmgr_func->set_power_profile_mode(hwmgr, > &workload, 0); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > > return 0; > } > @@ -970,10 +970,10 @@ static int pp_set_power_limit(void *handle, > uint32_t limit) > if (limit > hwmgr->default_power_limit) > return -EINVAL; > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > hwmgr->hwmgr_func->set_power_limit(hwmgr, limit); > hwmgr->power_limit = limit; > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return 0; > } > > @@ -984,14 +984,14 @@ static int pp_get_power_limit(void *handle, > uint32_t *limit, bool default_limit) > if (!hwmgr || !hwmgr->pm_en ||!limit) > return -EINVAL; > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > > if (default_limit) > *limit = hwmgr->default_power_limit; > else > *limit = hwmgr->power_limit; > > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > > return 0; > } > @@ -1004,9 +1004,9 @@ static int pp_display_configuration_change(void > *handle, > if (!hwmgr || !hwmgr->pm_en) > return -EINVAL; > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > phm_store_dal_configuration_data(hwmgr, display_config); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return 0; > } > > @@ -1019,9 +1019,9 @@ static int pp_get_display_power_level(void > *handle, > if (!hwmgr || !hwmgr->pm_en ||!output) > return -EINVAL; > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > ret = phm_get_dal_power_level(hwmgr, output); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return ret; > } > > @@ -1036,7 +1036,7 @@ static int pp_get_current_clocks(void *handle, > if (!hwmgr || !hwmgr->pm_en) > return -EINVAL; > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > > phm_get_dal_power_level(hwmgr, &simple_clocks); > > @@ -1050,7 +1050,7 @@ static int pp_get_current_clocks(void *handle, > > if (ret) { > pr_info("Error in phm_get_clock_info \n"); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return -EINVAL; > } > > @@ -1070,7 +1070,7 @@ static int pp_get_current_clocks(void *handle, > clocks->max_engine_clock_in_sr = hw_clocks.max_eng_clk; > clocks->min_engine_clock_in_sr = hw_clocks.min_eng_clk; > } > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return 0; > } > > @@ -1085,9 +1085,9 @@ static int pp_get_clock_by_type(void *handle, > enum amd_pp_clock_type type, struc > if (clocks == NULL) > return -EINVAL; > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > ret = phm_get_clock_by_type(hwmgr, type, clocks); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return ret; > } > > @@ -1101,9 +1101,9 @@ static int pp_get_clock_by_type_with_latency(void > *handle, > if (!hwmgr || !hwmgr->pm_en ||!clocks) > return -EINVAL; > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > ret = phm_get_clock_by_type_with_latency(hwmgr, type, clocks); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return ret; > } > > @@ -1117,11 +1117,11 @@ static int > pp_get_clock_by_type_with_voltage(void *handle, > if (!hwmgr || !hwmgr->pm_en ||!clocks) > return -EINVAL; > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > > ret = phm_get_clock_by_type_with_voltage(hwmgr, type, clocks); > > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return ret; > } > > @@ -1134,10 +1134,10 @@ static int > pp_set_watermarks_for_clocks_ranges(void *handle, > if (!hwmgr || !hwmgr->pm_en ||!wm_with_clock_ranges) > return -EINVAL; > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > ret = phm_set_watermarks_for_clocks_ranges(hwmgr, > wm_with_clock_ranges); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > > return ret; > } > @@ -1151,9 +1151,9 @@ static int pp_display_clock_voltage_request(void > *handle, > if (!hwmgr || !hwmgr->pm_en ||!clock) > return -EINVAL; > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > ret = phm_display_clock_voltage_request(hwmgr, clock); > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > > return ret; > } > @@ -1167,12 +1167,12 @@ static int > pp_get_display_mode_validation_clocks(void *handle, > if (!hwmgr || !hwmgr->pm_en ||!clocks) > return -EINVAL; > > - mutex_lock(&hwmgr->smu_lock); > + spin_lock(&hwmgr->smu_lock); > > if (phm_cap_enabled(hwmgr->platform_descriptor.platformCaps, > PHM_PlatformCaps_DynamicPatchPowerState)) > ret = phm_get_max_high_clocks(hwmgr, clocks); > > - mutex_unlock(&hwmgr->smu_lock); > + spin_unlock(&hwmgr->smu_lock); > return ret; > } > > diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h > b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h > index b99fb8a..37d1382 100644 > --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h > +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h > @@ -709,7 +709,7 @@ struct pp_hwmgr { > uint32_t smu_version; > bool not_vf; > bool pm_en; > - struct mutex smu_lock; > + spinlock_t smu_lock; > > uint32_t pp_table_version; > void *device; > -- > 2.7.4 > > _______________________________________________ > amd-gfx mailing list > amd-gfx at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
