<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"><!-- P {margin-top:0;margin-bottom:0;} --></style>
</head>
<body dir="ltr">
<div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr">
<p style="margin-top:0;margin-bottom:0">Hi Hersen,</p>
<p style="margin-top:0;margin-bottom:0"><br>
</p>
<p style="margin-top:0;margin-bottom:0">Can we change "pr_info" to "pr_debug"?</p>
<p style="margin-top:0;margin-bottom:0">I am afraid there may be a bunch of "<font size="2"><span style="font-size:11pt;">was not implemented</span></font>" in kernel message on old asics.</p>
<p style="margin-top:0;margin-bottom:0"><br>
</p>
<p style="margin-top:0;margin-bottom:0">Except that, <br>
</p>
<p style="margin-top:0;margin-bottom:0">Patch is <br>
</p>
<p style="margin-top:0;margin-bottom:0"><span>Reviewed-by: Rex Zhu <Rex.Zhu@amd.com></span><br>
</p>
<p style="margin-top:0;margin-bottom:0"><br>
</p>
<p style="margin-top:0;margin-bottom:0">Best Regards</p>
<p style="margin-top:0;margin-bottom:0">Rex<br>
<br>
</p>
<div style="color: rgb(0, 0, 0);">
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font style="font-size:11pt" face="Calibri, sans-serif" color="#000000"><b>From:</b> Wu, Hersen<br>
<b>Sent:</b> Thursday, December 6, 2018 2:03 AM<br>
<b>To:</b> amd-gfx@lists.freedesktop.org; Zhu, Rex; Deucher, Alexander<br>
<b>Subject:</b> RE: [PATCH] drm/amd/powerplay: rv dal-pplib interface refactor powerplay part</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">Hello, Alex, Rex,<br>
<br>
Would you please help review the change?<br>
<br>
Thanks,<br>
Hersen<br>
<br>
<br>
<br>
<br>
<br>
[WHY] clarify dal input parameters to pplib interface, remove un-used parameters. dal knows exactly which parameters needed and their effects at pplib and smu sides.<br>
<br>
current dal sequence for dcn1_update_clock to pplib:<br>
<br>
1.smu10_display_clock_voltage_request for dcefclk 2.smu10_display_clock_voltage_request for fclk 3.phm_store_dal_configuration_data {<br>
set_min_deep_sleep_dcfclk<br>
set_active_display_count<br>
store_cc6_data --- this data never be referenced<br>
<br>
new sequence will be:<br>
<br>
1. set_display_count --- need add new pplib interface 2. set_min_deep_sleep_dcfclk -- new pplib interface 3. set_hard_min_dcfclk_by_freq 4. set_hard_min_fclk_by_freq<br>
<br>
after this code refactor, smu10_display_clock_voltage_request,<br>
phm_store_dal_configuration_data will not be needed for rv.<br>
<br>
[HOW] step 1: add new functions at pplib interface<br>
step 2: add new functions at amdgpu dm and dc<br>
<br>
Signed-off-by: hersen wu <hersenxs.wu@amd.com><br>
---<br>
drivers/gpu/drm/amd/include/kgd_pp_interface.h | 4 ++<br>
drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 76 ++++++++++++++++++++++<br>
.../gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c | 45 ++++++++++++- drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c | 36 +++++++++-<br>
.../gpu/drm/amd/powerplay/inc/hardwaremanager.h | 3 +<br>
drivers/gpu/drm/amd/powerplay/inc/hwmgr.h | 4 +-<br>
6 files changed, 162 insertions(+), 6 deletions(-)<br>
<br>
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h<br>
index 980e696..1479ea1 100644<br>
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h<br>
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h<br>
@@ -276,6 +276,10 @@ struct amd_pm_funcs {<br>
struct amd_pp_simple_clock_info *clocks);<br>
int (*notify_smu_enable_pwe)(void *handle);<br>
int (*enable_mgpu_fan_boost)(void *handle);<br>
+ int (*set_active_display_count)(void *handle, uint32_t count);<br>
+ int (*set_hard_min_dcefclk_by_freq)(void *handle, uint32_t clock);<br>
+ int (*set_hard_min_fclk_by_freq)(void *handle, uint32_t clock);<br>
+ int (*set_min_deep_sleep_dcefclk)(void *handle, uint32_t clock);<br>
};<br>
<br>
#endif<br>
diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c<br>
index d6aa1d4..53dde16 100644<br>
--- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c<br>
+++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c<br>
@@ -1332,6 +1332,78 @@ static int pp_enable_mgpu_fan_boost(void *handle)<br>
return 0;<br>
}<br>
<br>
+static int pp_set_min_deep_sleep_dcefclk(void *handle, uint32_t clock) <br>
+{<br>
+ struct pp_hwmgr *hwmgr = handle;<br>
+<br>
+ if (!hwmgr || !hwmgr->pm_en)<br>
+ return -EINVAL;<br>
+<br>
+ if (hwmgr->hwmgr_func->set_min_deep_sleep_dcefclk == NULL) {<br>
+ pr_info("%s was not implemented.\n", __func__);<br>
+ return -EINVAL;;<br>
+ }<br>
+<br>
+ mutex_lock(&hwmgr->smu_lock);<br>
+ hwmgr->hwmgr_func->set_min_deep_sleep_dcefclk(hwmgr, clock);<br>
+ mutex_unlock(&hwmgr->smu_lock);<br>
+<br>
+ return 0;<br>
+}<br>
+<br>
+static int pp_set_hard_min_dcefclk_by_freq(void *handle, uint32_t <br>
+clock) {<br>
+ struct pp_hwmgr *hwmgr = handle;<br>
+<br>
+ if (!hwmgr || !hwmgr->pm_en)<br>
+ return -EINVAL;<br>
+<br>
+ if (hwmgr->hwmgr_func->set_hard_min_dcefclk_by_freq == NULL) {<br>
+ pr_info("%s was not implemented.\n", __func__);<br>
+ return -EINVAL;;<br>
+ }<br>
+<br>
+ mutex_lock(&hwmgr->smu_lock);<br>
+ hwmgr->hwmgr_func->set_hard_min_dcefclk_by_freq(hwmgr, clock);<br>
+ mutex_unlock(&hwmgr->smu_lock);<br>
+<br>
+ return 0;<br>
+}<br>
+<br>
+static int pp_set_hard_min_fclk_by_freq(void *handle, uint32_t clock) {<br>
+ struct pp_hwmgr *hwmgr = handle;<br>
+<br>
+ if (!hwmgr || !hwmgr->pm_en)<br>
+ return -EINVAL;<br>
+<br>
+ if (hwmgr->hwmgr_func->set_hard_min_fclk_by_freq == NULL) {<br>
+ pr_info("%s was not implemented.\n", __func__);<br>
+ return -EINVAL;;<br>
+ }<br>
+<br>
+ mutex_lock(&hwmgr->smu_lock);<br>
+ hwmgr->hwmgr_func->set_hard_min_fclk_by_freq(hwmgr, clock);<br>
+ mutex_unlock(&hwmgr->smu_lock);<br>
+<br>
+ return 0;<br>
+}<br>
+<br>
+static int pp_set_active_display_count(void *handle, uint32_t count) {<br>
+ struct pp_hwmgr *hwmgr = handle;<br>
+ int ret = 0;<br>
+<br>
+ if (!hwmgr || !hwmgr->pm_en)<br>
+ return -EINVAL;<br>
+<br>
+ mutex_lock(&hwmgr->smu_lock);<br>
+ ret = phm_set_active_display_count(hwmgr, count);<br>
+ mutex_unlock(&hwmgr->smu_lock);<br>
+<br>
+ return ret;<br>
+}<br>
+<br>
static const struct amd_pm_funcs pp_dpm_funcs = {<br>
.load_firmware = pp_dpm_load_fw,<br>
.wait_for_fw_loading_complete = pp_dpm_fw_loading_complete, @@ -1378,4 +1450,8 @@ static const struct amd_pm_funcs pp_dpm_funcs = {<br>
.get_display_mode_validation_clocks = pp_get_display_mode_validation_clocks,<br>
.notify_smu_enable_pwe = pp_notify_smu_enable_pwe,<br>
.enable_mgpu_fan_boost = pp_enable_mgpu_fan_boost,<br>
+ .set_active_display_count = pp_set_active_display_count,<br>
+ .set_min_deep_sleep_dcefclk = pp_set_min_deep_sleep_dcefclk,<br>
+ .set_hard_min_dcefclk_by_freq = pp_set_hard_min_dcefclk_by_freq,<br>
+ .set_hard_min_fclk_by_freq = pp_set_hard_min_fclk_by_freq,<br>
};<br>
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c b/drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c<br>
index 85119c2..333b9b8 100644<br>
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c<br>
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c<br>
@@ -286,8 +286,8 @@ int phm_store_dal_configuration_data(struct pp_hwmgr *hwmgr,<br>
if (display_config == NULL)<br>
return -EINVAL;<br>
<br>
- if (NULL != hwmgr->hwmgr_func->set_deep_sleep_dcefclk)<br>
- hwmgr->hwmgr_func->set_deep_sleep_dcefclk(hwmgr, display_config->min_dcef_deep_sleep_set_clk);<br>
+ if (NULL != hwmgr->hwmgr_func->set_min_deep_sleep_dcefclk)<br>
+ hwmgr->hwmgr_func->set_min_deep_sleep_dcefclk(hwmgr, <br>
+display_config->min_dcef_deep_sleep_set_clk);<br>
<br>
for (index = 0; index < display_config->num_path_including_non_display; index++) {<br>
if (display_config->displays[index].controller_id != 0) @@ -478,3 +478,44 @@ int phm_disable_smc_firmware_ctf(struct pp_hwmgr *hwmgr)<br>
<br>
return hwmgr->hwmgr_func->disable_smc_firmware_ctf(hwmgr);<br>
}<br>
+<br>
+int phm_set_active_display_count(struct pp_hwmgr *hwmgr, uint32_t <br>
+count) {<br>
+ PHM_FUNC_CHECK(hwmgr);<br>
+<br>
+ if (!hwmgr->hwmgr_func->set_active_display_count)<br>
+ return -EINVAL;<br>
+<br>
+ return hwmgr->hwmgr_func->set_active_display_count(hwmgr, count); }<br>
+<br>
+int phm_set_min_deep_sleep_dcefclk(struct pp_hwmgr *hwmgr, uint32_t <br>
+clock) {<br>
+ PHM_FUNC_CHECK(hwmgr);<br>
+<br>
+ if (!hwmgr->hwmgr_func->set_min_deep_sleep_dcefclk)<br>
+ return -EINVAL;<br>
+<br>
+ return hwmgr->hwmgr_func->set_min_deep_sleep_dcefclk(hwmgr, clock); }<br>
+<br>
+int phm_set_hard_min_dcefclk_by_freq(struct pp_hwmgr *hwmgr, uint32_t <br>
+clock) {<br>
+ PHM_FUNC_CHECK(hwmgr);<br>
+<br>
+ if (!hwmgr->hwmgr_func->set_hard_min_dcefclk_by_freq)<br>
+ return -EINVAL;<br>
+<br>
+ return hwmgr->hwmgr_func->set_hard_min_dcefclk_by_freq(hwmgr, clock); <br>
+}<br>
+<br>
+int phm_set_hard_min_fclk_by_freq(struct pp_hwmgr *hwmgr, uint32_t <br>
+clock) {<br>
+ PHM_FUNC_CHECK(hwmgr);<br>
+<br>
+ if (!hwmgr->hwmgr_func->set_hard_min_fclk_by_freq)<br>
+ return -EINVAL;<br>
+<br>
+ return hwmgr->hwmgr_func->set_hard_min_fclk_by_freq(hwmgr, clock); }<br>
+<br>
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c<br>
index dd18cb7..f95c5f5 100644<br>
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c<br>
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c<br>
@@ -216,12 +216,12 @@ static inline uint32_t convert_10k_to_mhz(uint32_t clock)<br>
return (clock + 99) / 100;<br>
}<br>
<br>
-static int smu10_set_deep_sleep_dcefclk(struct pp_hwmgr *hwmgr, uint32_t clock)<br>
+static int smu10_set_min_deep_sleep_dcefclk(struct pp_hwmgr *hwmgr, <br>
+uint32_t clock)<br>
{<br>
struct smu10_hwmgr *smu10_data = (struct smu10_hwmgr *)(hwmgr->backend);<br>
<br>
if (smu10_data->need_min_deep_sleep_dcefclk &&<br>
- smu10_data->deep_sleep_dcefclk != convert_10k_to_mhz(clock)) {<br>
+ smu10_data->deep_sleep_dcefclk != convert_10k_to_mhz(clock)) {<br>
smu10_data->deep_sleep_dcefclk = convert_10k_to_mhz(clock);<br>
smum_send_msg_to_smc_with_parameter(hwmgr,<br>
PPSMC_MSG_SetMinDeepSleepDcefclk,<br>
@@ -230,6 +230,34 @@ static int smu10_set_deep_sleep_dcefclk(struct pp_hwmgr *hwmgr, uint32_t clock)<br>
return 0;<br>
}<br>
<br>
+static int smu10_set_hard_min_dcefclk_by_freq(struct pp_hwmgr *hwmgr, <br>
+uint32_t clock) {<br>
+ struct smu10_hwmgr *smu10_data = (struct smu10_hwmgr <br>
+*)(hwmgr->backend);<br>
+<br>
+ if (smu10_data->dcf_actual_hard_min_freq &&<br>
+ smu10_data->dcf_actual_hard_min_freq != convert_10k_to_mhz(clock)) {<br>
+ smu10_data->dcf_actual_hard_min_freq = convert_10k_to_mhz(clock);<br>
+ smum_send_msg_to_smc_with_parameter(hwmgr,<br>
+ PPSMC_MSG_SetHardMinDcefclkByFreq,<br>
+ smu10_data->dcf_actual_hard_min_freq);<br>
+ }<br>
+ return 0;<br>
+}<br>
+<br>
+static int smu10_set_hard_min_fclk_by_freq(struct pp_hwmgr *hwmgr, <br>
+uint32_t clock) {<br>
+ struct smu10_hwmgr *smu10_data = (struct smu10_hwmgr <br>
+*)(hwmgr->backend);<br>
+<br>
+ if (smu10_data->f_actual_hard_min_freq &&<br>
+ smu10_data->f_actual_hard_min_freq != convert_10k_to_mhz(clock)) {<br>
+ smu10_data->f_actual_hard_min_freq = convert_10k_to_mhz(clock);<br>
+ smum_send_msg_to_smc_with_parameter(hwmgr,<br>
+ PPSMC_MSG_SetHardMinFclkByFreq,<br>
+ smu10_data->f_actual_hard_min_freq);<br>
+ }<br>
+ return 0;<br>
+}<br>
+<br>
static int smu10_set_active_display_count(struct pp_hwmgr *hwmgr, uint32_t count) {<br>
struct smu10_hwmgr *smu10_data = (struct smu10_hwmgr *)(hwmgr->backend); @@ -1206,7 +1234,7 @@ static const struct pp_hwmgr_func smu10_hwmgr_funcs = {<br>
.get_max_high_clocks = smu10_get_max_high_clocks,<br>
.read_sensor = smu10_read_sensor,<br>
.set_active_display_count = smu10_set_active_display_count,<br>
- .set_deep_sleep_dcefclk = smu10_set_deep_sleep_dcefclk,<br>
+ .set_min_deep_sleep_dcefclk = smu10_set_min_deep_sleep_dcefclk,<br>
.dynamic_state_management_enable = smu10_enable_dpm_tasks,<br>
.power_off_asic = smu10_power_off_asic,<br>
.asic_setup = smu10_setup_asic_task,<br>
@@ -1217,6 +1245,8 @@ static const struct pp_hwmgr_func smu10_hwmgr_funcs = {<br>
.display_clock_voltage_request = smu10_display_clock_voltage_request,<br>
.powergate_gfx = smu10_gfx_off_control,<br>
.powergate_sdma = smu10_powergate_sdma,<br>
+ .set_hard_min_dcefclk_by_freq = smu10_set_hard_min_dcefclk_by_freq,<br>
+ .set_hard_min_fclk_by_freq = smu10_set_hard_min_fclk_by_freq,<br>
};<br>
<br>
int smu10_init_function_pointers(struct pp_hwmgr *hwmgr) diff --git a/drivers/gpu/drm/amd/powerplay/inc/hardwaremanager.h b/drivers/gpu/drm/amd/powerplay/inc/hardwaremanager.h<br>
index 54fd012..f4dab97 100644<br>
--- a/drivers/gpu/drm/amd/powerplay/inc/hardwaremanager.h<br>
+++ b/drivers/gpu/drm/amd/powerplay/inc/hardwaremanager.h<br>
@@ -463,5 +463,8 @@ extern int phm_display_clock_voltage_request(struct pp_hwmgr *hwmgr,<br>
<br>
extern int phm_get_max_high_clocks(struct pp_hwmgr *hwmgr, struct amd_pp_simple_clock_info *clocks); extern int phm_disable_smc_firmware_ctf(struct pp_hwmgr *hwmgr);<br>
+<br>
+extern int phm_set_active_display_count(struct pp_hwmgr *hwmgr, <br>
+uint32_t count);<br>
+<br>
#endif /* _HARDWARE_MANAGER_H_ */<br>
<br>
diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h<br>
index 07d180ce..19edfc9 100644<br>
--- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h<br>
+++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h<br>
@@ -309,7 +309,7 @@ struct pp_hwmgr_func {<br>
int (*avfs_control)(struct pp_hwmgr *hwmgr, bool enable);<br>
int (*disable_smc_firmware_ctf)(struct pp_hwmgr *hwmgr);<br>
int (*set_active_display_count)(struct pp_hwmgr *hwmgr, uint32_t count);<br>
- int (*set_deep_sleep_dcefclk)(struct pp_hwmgr *hwmgr, uint32_t clock);<br>
+ int (*set_min_deep_sleep_dcefclk)(struct pp_hwmgr *hwmgr, uint32_t <br>
+clock);<br>
int (*start_thermal_controller)(struct pp_hwmgr *hwmgr, struct PP_TemperatureRange *range);<br>
int (*notify_cac_buffer_info)(struct pp_hwmgr *hwmgr,<br>
uint32_t virtual_addr_low,<br>
@@ -329,6 +329,8 @@ struct pp_hwmgr_func {<br>
int (*smus_notify_pwe)(struct pp_hwmgr *hwmgr);<br>
int (*powergate_sdma)(struct pp_hwmgr *hwmgr, bool bgate);<br>
int (*enable_mgpu_fan_boost)(struct pp_hwmgr *hwmgr);<br>
+ int (*set_hard_min_dcefclk_by_freq)(struct pp_hwmgr *hwmgr, uint32_t clock);<br>
+ int (*set_hard_min_fclk_by_freq)(struct pp_hwmgr *hwmgr, uint32_t <br>
+clock);<br>
};<br>
<br>
struct pp_table_func {<br>
--<br>
2.7.4<br>
<br>
</div>
</span></font></div>
</div>
</div>
</body>
</html>