drm/amdgpu: Stop dividing watermarks by 1000 in powerplay

PATCH Superseded David Francis 3 participants 5 messages Oct 30, 2018 – Oct 31, 2018 2 messages not in the archive

Subsystems (3)

DRM drivers Radeon and amdgpu DRM drivers AMD display core

lore

Share & tools

lore

David Francis

author

2018-10-30 19:34 UTC #0 lore raw

PATCH 1/2 drm/amdgpu: Stop dividing watermarks by 1000 in powerplay

Watermarks were being multiplied by 1000 in amdgpu_dm and divided by 1000 in powerplay. Change watermarks to units of mhz to stop doing that.

Signed-off-by David Francis <[email protected]>

---
 .../amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c  | 24 +++++---------
 .../drm/amd/display/dc/dm_services_types.h    | 16 +++++-----
 .../gpu/drm/amd/powerplay/hwmgr/smu_helper.c  | 32 +++++--------------
 3 files changed, 24 insertions(+), 48 deletions(-)

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c +8 ~~-16~~

index 9d2d6986b983..d9daa038fdb2 100644
@@ -517,14 +517,10 @@ void pp_rv_set_wm_ranges(struct pp_smu *pp,
 		else
 			wm_dce_clocks[i].wm_set_id =
 					ranges->reader_wm_sets[i].wm_inst;
-		wm_dce_clocks[i].wm_max_dcfclk_clk_in_khz =
-				ranges->reader_wm_sets[i].max_drain_clk_mhz * 1000;
-		wm_dce_clocks[i].wm_min_dcfclk_clk_in_khz =
-				ranges->reader_wm_sets[i].min_drain_clk_mhz * 1000;
-		wm_dce_clocks[i].wm_max_mem_clk_in_khz =
-				ranges->reader_wm_sets[i].max_fill_clk_mhz * 1000;
-		wm_dce_clocks[i].wm_min_mem_clk_in_khz =
-				ranges->reader_wm_sets[i].min_fill_clk_mhz * 1000;
+		wm_dce_clocks[i].wm_max_dcfclk_clk_in_mhz = ranges->reader_wm_sets[i].max_drain_clk_mhz;
+		wm_dce_clocks[i].wm_min_dcfclk_clk_in_mhz = ranges->reader_wm_sets[i].min_drain_clk_mhz;
+		wm_dce_clocks[i].wm_max_mem_clk_in_mhz = ranges->reader_wm_sets[i].max_fill_clk_mhz;
+		wm_dce_clocks[i].wm_min_mem_clk_in_mhz = ranges->reader_wm_sets[i].min_fill_clk_mhz;
 	}
 
 	for (i = 0; i < wm_with_clock_ranges.num_wm_mcif_sets; i++) {

@@ -533,14 +529,10 @@ void pp_rv_set_wm_ranges(struct pp_smu *pp,
 		else
 			wm_soc_clocks[i].wm_set_id =
 					ranges->writer_wm_sets[i].wm_inst;
-		wm_soc_clocks[i].wm_max_socclk_clk_in_khz =
-				ranges->writer_wm_sets[i].max_fill_clk_mhz * 1000;
-		wm_soc_clocks[i].wm_min_socclk_clk_in_khz =
-				ranges->writer_wm_sets[i].min_fill_clk_mhz * 1000;
-		wm_soc_clocks[i].wm_max_mem_clk_in_khz =
-				ranges->writer_wm_sets[i].max_drain_clk_mhz * 1000;
-		wm_soc_clocks[i].wm_min_mem_clk_in_khz =
-				ranges->writer_wm_sets[i].min_drain_clk_mhz * 1000;
+		wm_soc_clocks[i].wm_max_socclk_clk_in_mhz = ranges->writer_wm_sets[i].max_fill_clk_mhz;
+		wm_soc_clocks[i].wm_min_socclk_clk_in_mhz = ranges->writer_wm_sets[i].min_fill_clk_mhz;
+		wm_soc_clocks[i].wm_max_mem_clk_in_mhz = ranges->writer_wm_sets[i].max_drain_clk_mhz;
+		wm_soc_clocks[i].wm_min_mem_clk_in_mhz = ranges->writer_wm_sets[i].min_drain_clk_mhz;
 	}
 
 	pp_funcs->set_watermarks_for_clocks_ranges(pp_handle, &wm_with_clock_ranges);

drivers/gpu/drm/amd/display/dc/dm_services_types.h +8 -8

index 1af8c777b3ac..ce6fedd97df9 100644
@@ -148,18 +148,18 @@ struct dm_pp_wm_sets_with_clock_ranges {
 
 struct dm_pp_clock_range_for_dmif_wm_set_soc15 {
 	enum dm_pp_wm_set_id wm_set_id;
-	uint32_t wm_min_dcfclk_clk_in_khz;
-	uint32_t wm_max_dcfclk_clk_in_khz;
-	uint32_t wm_min_mem_clk_in_khz;
-	uint32_t wm_max_mem_clk_in_khz;
+	uint32_t wm_min_dcfclk_clk_in_mhz;
+	uint32_t wm_max_dcfclk_clk_in_mhz;
+	uint32_t wm_min_mem_clk_in_mhz;
+	uint32_t wm_max_mem_clk_in_mhz;
 };
 
 struct dm_pp_clock_range_for_mcif_wm_set_soc15 {
 	enum dm_pp_wm_set_id wm_set_id;
-	uint32_t wm_min_socclk_clk_in_khz;
-	uint32_t wm_max_socclk_clk_in_khz;
-	uint32_t wm_min_mem_clk_in_khz;
-	uint32_t wm_max_mem_clk_in_khz;
+	uint32_t wm_min_socclk_clk_in_mhz;
+	uint32_t wm_max_socclk_clk_in_mhz;
+	uint32_t wm_min_mem_clk_in_mhz;
+	uint32_t wm_max_mem_clk_in_mhz;
 };
 
 struct dm_pp_wm_sets_with_clock_ranges_soc15 {

drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c +8 ~~-24~~

index 99a33c33a32c..0fae388220fe 100644
@@ -712,42 +712,26 @@ int smu_set_watermarks_for_clocks_ranges(void *wt_table,
 
 	for (i = 0; i < wm_with_clock_ranges->num_wm_dmif_sets; i++) {
 		table->WatermarkRow[1][i].MinClock =
-			cpu_to_le16((uint16_t)
-			(wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_min_dcfclk_clk_in_khz) /
-			1000);
+			cpu_to_le16((uint16_t)wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_min_dcfclk_clk_in_mhz);
 		table->WatermarkRow[1][i].MaxClock =
-			cpu_to_le16((uint16_t)
-			(wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_max_dcfclk_clk_in_khz) /
-			1000);
+			cpu_to_le16((uint16_t)wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_max_dcfclk_clk_in_mhz);
 		table->WatermarkRow[1][i].MinUclk =
-			cpu_to_le16((uint16_t)
-			(wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_min_mem_clk_in_khz) /
-			1000);
+			cpu_to_le16((uint16_t)wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_min_mem_clk_in_mhz);
 		table->WatermarkRow[1][i].MaxUclk =
-			cpu_to_le16((uint16_t)
-			(wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_max_mem_clk_in_khz) /
-			1000);
+			cpu_to_le16((uint16_t)wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_max_mem_clk_in_mhz);
 		table->WatermarkRow[1][i].WmSetting = (uint8_t)
 				wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_set_id;
 	}
 
 	for (i = 0; i < wm_with_clock_ranges->num_wm_mcif_sets; i++) {
 		table->WatermarkRow[0][i].MinClock =
-			cpu_to_le16((uint16_t)
-			(wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_min_socclk_clk_in_khz) /
-			1000);
+			cpu_to_le16((uint16_t)wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_min_socclk_clk_in_mhz);
 		table->WatermarkRow[0][i].MaxClock =
-			cpu_to_le16((uint16_t)
-			(wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_max_socclk_clk_in_khz) /
-			1000);
+			cpu_to_le16((uint16_t)wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_max_socclk_clk_in_mhz);
 		table->WatermarkRow[0][i].MinUclk =
-			cpu_to_le16((uint16_t)
-			(wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_min_mem_clk_in_khz) /
-			1000);
+			cpu_to_le16((uint16_t)wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_min_mem_clk_in_mhz);
 		table->WatermarkRow[0][i].MaxUclk =
-			cpu_to_le16((uint16_t)
-			(wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_max_mem_clk_in_khz) /
-			1000);
+			cpu_to_le16((uint16_t)wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_max_mem_clk_in_mhz);
 		table->WatermarkRow[0][i].WmSetting = (uint8_t)
 				wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_set_id;
 	}

Signature · 6 lines

2.17.1

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): 20181030193455.32750-1-David.Francis-5C7GfCeVMHo@public.gmane.org

David Francis

author

2018-10-30 19:34 UTC #2 lore raw

PATCH 2/2 drm/amdgpu: Change pp clock requests to mHz

We were multiplying clock requests by 1000 in amdgpu_dm and then dividing them by 1000 in powerplay.

Also, the vega12 code was dividing by 10 when it should have been multiplying (to convert units of 10kHz to units of kHz).

Signed-off-by David Francis <[email protected]>

---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 6 +++---
 drivers/gpu/drm/amd/include/dm_pp_interface.h            | 2 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c        | 4 ++--
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c       | 4 ++--
 drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c       | 4 ++--
 drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c       | 4 ++--
 6 files changed, 12 insertions(+), 12 deletions(-)

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c +3 -3

index d9daa038fdb2..cfa9b7f545b8 100644
@@ -436,7 +436,7 @@ bool dm_pp_apply_clock_for_voltage_request(
 	int ret = 0;
 
 	pp_clock_request.clock_type = dc_to_pp_clock_type(clock_for_voltage_req->clk_type);
-	pp_clock_request.clock_freq_in_khz = clock_for_voltage_req->clocks_in_khz;
+	pp_clock_request.clock_freq_in_mhz = clock_for_voltage_req->clocks_in_khz / 1000;
 
 	if (!pp_clock_request.clock_type)
 		return false;

@@ -485,11 +485,11 @@ void pp_rv_set_display_requirement(struct pp_smu *pp,
 		return;
 
 	clock.clock_type = amd_pp_dcf_clock;
-	clock.clock_freq_in_khz = req->hard_min_dcefclk_mhz * 1000;
+	clock.clock_freq_in_mhz = req->hard_min_dcefclk_mhz;
 	pp_funcs->display_clock_voltage_request(pp_handle, &clock);
 
 	clock.clock_type = amd_pp_f_clock;
-	clock.clock_freq_in_khz = req->hard_min_fclk_mhz * 1000;
+	clock.clock_freq_in_mhz = req->hard_min_fclk_mhz;
 	pp_funcs->display_clock_voltage_request(pp_handle, &clock);
 }

drivers/gpu/drm/amd/include/dm_pp_interface.h +1 -1

index 1d93a0c574c9..114ddd03e238 100644
@@ -188,7 +188,7 @@ struct pp_clock_levels_with_voltage {
 
 struct pp_display_clock_request {
 	enum amd_pp_clock_type clock_type;
-	uint32_t clock_freq_in_khz;
+	uint32_t clock_freq_in_mhz;
 };
 
 #endif /* _DM_PP_INTERFACE_ */

drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c +2 -2

index dd18cb710391..d6a6a4f4ac9d 100644
@@ -57,7 +57,7 @@ static int smu10_display_clock_voltage_request(struct pp_hwmgr *hwmgr,
 {
 	struct smu10_hwmgr *smu10_data = (struct smu10_hwmgr *)(hwmgr->backend);
 	enum amd_pp_clock_type clk_type = clock_req->clock_type;
-	uint32_t clk_freq = clock_req->clock_freq_in_khz / 1000;
+	uint32_t clk_freq = clock_req->clock_freq_in_mhz;
 	PPSMC_Msg        msg;
 
 	switch (clk_type) {

@@ -203,7 +203,7 @@ static int smu10_set_clock_limit(struct pp_hwmgr *hwmgr, const void *input)
 
 	clocks.dcefClock = hwmgr->display_config->min_dcef_set_clk;
 	clock_req.clock_type = amd_pp_dcf_clock;
-	clock_req.clock_freq_in_khz = clocks.dcefClock * 10;
+	clock_req.clock_freq_in_mhz = clocks.dcefClock / 100;
 
 	PP_ASSERT_WITH_CODE(!smu10_display_clock_voltage_request(hwmgr, &clock_req),
 				"Attempt to set DCF Clock Failed!", return -EINVAL);

drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c +2 -2

index 419a1d77d661..f926a46bf256 100644
@@ -3740,7 +3740,7 @@ int vega10_display_clock_voltage_request(struct pp_hwmgr *hwmgr,
 {
 	int result = 0;
 	enum amd_pp_clock_type clk_type = clock_req->clock_type;
-	uint32_t clk_freq = clock_req->clock_freq_in_khz / 1000;
+	uint32_t clk_freq = clock_req->clock_freq_in_mhz;
 	DSPCLK_e clk_select = 0;
 	uint32_t clk_request = 0;

@@ -3825,7 +3825,7 @@ static int vega10_notify_smc_display_config_after_ps_adjustment(
 
 	if (i < dpm_table->count) {
 		clock_req.clock_type = amd_pp_dcef_clock;
-		clock_req.clock_freq_in_khz = dpm_table->dpm_levels[i].value * 10;
+		clock_req.clock_freq_in_mhz = dpm_table->dpm_levels[i].value / 100;
 		if (!vega10_display_clock_voltage_request(hwmgr, &clock_req)) {
 			smum_send_msg_to_smc_with_parameter(
 					hwmgr, PPSMC_MSG_SetMinDeepSleepDcefclk,

drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c +2 -2

index 74bc37308dc0..6478711f0b88 100644
@@ -1349,7 +1349,7 @@ int vega12_display_clock_voltage_request(struct pp_hwmgr *hwmgr,
 	int result = 0;
 	struct vega12_hwmgr *data = (struct vega12_hwmgr *)(hwmgr->backend);
 	enum amd_pp_clock_type clk_type = clock_req->clock_type;
-	uint32_t clk_freq = clock_req->clock_freq_in_khz / 1000;
+	uint32_t clk_freq = clock_req->clock_freq_in_mhz;
 	PPCLK_e clk_select = 0;
 	uint32_t clk_request = 0;

@@ -1405,7 +1405,7 @@ static int vega12_notify_smc_display_config_after_ps_adjustment(
 
 	if (data->smu_features[GNLD_DPM_DCEFCLK].supported) {
 		clock_req.clock_type = amd_pp_dcef_clock;
-		clock_req.clock_freq_in_khz = min_clocks.dcefClock/10;
+		clock_req.clock_freq_in_mhz = min_clocks.dcefClock / 100;
 		if (!vega12_display_clock_voltage_request(hwmgr, &clock_req)) {
 			if (data->smu_features[GNLD_DS_DCEFCLK].supported)
 				PP_ASSERT_WITH_CODE(

drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c +2 -2

index 57143d51e3ee..7cddc39e4cd3 100644
@@ -1991,7 +1991,7 @@ int vega20_display_clock_voltage_request(struct pp_hwmgr *hwmgr,
 	int result = 0;
 	struct vega20_hwmgr *data = (struct vega20_hwmgr *)(hwmgr->backend);
 	enum amd_pp_clock_type clk_type = clock_req->clock_type;
-	uint32_t clk_freq = clock_req->clock_freq_in_khz / 1000;
+	uint32_t clk_freq = clock_req->clock_freq_in_mhz;
 	PPCLK_e clk_select = 0;
 	uint32_t clk_request = 0;

@@ -2057,7 +2057,7 @@ static int vega20_notify_smc_display_config_after_ps_adjustment(
 
 	if (data->smu_features[GNLD_DPM_DCEFCLK].supported) {
 		clock_req.clock_type = amd_pp_dcef_clock;
-		clock_req.clock_freq_in_khz = min_clocks.dcefClock * 10;
+		clock_req.clock_freq_in_mhz = min_clocks.dcefClock / 100;
 		if (!vega20_display_clock_voltage_request(hwmgr, &clock_req)) {
 			if (data->smu_features[GNLD_DS_DCEFCLK].supported)
 				PP_ASSERT_WITH_CODE((ret = smum_send_msg_to_smc_with_parameter(

Signature · 6 lines

2.17.1

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): 20181030193455.32750-2-David.Francis-5C7GfCeVMHo@public.gmane.org

Quan, Evan

2018-10-31 04:13 UTC #4 lore raw

RE: [PATCH 2/2] drm/amdgpu: Change pp clock requests to mHz

Series is reviewed-by: Evan Quan <[email protected]>

Regards,
Evan

Quoted 187 lines

-----Original Message-----
From: amd-gfx <[email protected]> On Behalf Of David Francis
Sent: Wednesday, October 31, 2018 3:35 AM
To: [email protected]
Cc: Francis, David <[email protected]>
Subject: [PATCH 2/2] drm/amdgpu: Change pp clock requests to mHz
We were multiplying clock requests by 1000 in amdgpu_dm and then dividing them by 1000 in powerplay.
Also, the vega12 code was dividing by 10 when it should have been multiplying (to convert units of 10kHz to units of kHz).
Signed-off-by David Francis <[email protected]>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 6
+++---
 drivers/gpu/drm/amd/include/dm_pp_interface.h            | 2 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c        | 4 ++--
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c       | 4 ++--
 drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c       | 4 ++--
 drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c       | 4 ++--
 6 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c index d9daa038fdb2..cfa9b7f545b8 100644
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c +4 -3
@@ -436,7 +436,7 @@ bool dm_pp_apply_clock_for_voltage_request(
 	int ret = 0;
 
 	pp_clock_request.clock_type =
 dc_to_pp_clock_type(clock_for_voltage_req->clk_type);
-	pp_clock_request.clock_freq_in_khz = clock_for_voltage_req-
 >clocks_in_khz;
+	pp_clock_request.clock_freq_in_mhz =
+clock_for_voltage_req->clocks_in_khz / 1000;
 
 	if (!pp_clock_request.clock_type)
 		return false;
@@ -485,11 +485,11 @@ void pp_rv_set_display_requirement(struct
 pp_smu *pp,
 		return;
 
 	clock.clock_type = amd_pp_dcf_clock;
-	clock.clock_freq_in_khz = req->hard_min_dcefclk_mhz * 1000;
+	clock.clock_freq_in_mhz = req->hard_min_dcefclk_mhz;
 	pp_funcs->display_clock_voltage_request(pp_handle, &clock);
 
 	clock.clock_type = amd_pp_f_clock;
-	clock.clock_freq_in_khz = req->hard_min_fclk_mhz * 1000;
+	clock.clock_freq_in_mhz = req->hard_min_fclk_mhz;
 	pp_funcs->display_clock_voltage_request(pp_handle, &clock);  }
 
drivers/gpu/drm/amd/include/dm_pp_interface.h +1 -1
index 1d93a0c574c9..114ddd03e238 100644
@@ -188,7 +188,7 @@ struct pp_clock_levels_with_voltage {
 
 struct pp_display_clock_request {
 	enum amd_pp_clock_type clock_type;
-	uint32_t clock_freq_in_khz;
+	uint32_t clock_freq_in_mhz;
 };
 
 #endif /* _DM_PP_INTERFACE_ */
drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c +1 -1
index dd18cb710391..d6a6a4f4ac9d 100644
@@ -57,7 +57,7 @@ static int smu10_display_clock_voltage_request(struct
 pp_hwmgr *hwmgr,  {
 	struct smu10_hwmgr *smu10_data = (struct smu10_hwmgr
 *)(hwmgr->backend);
 	enum amd_pp_clock_type clk_type = clock_req->clock_type;
-	uint32_t clk_freq = clock_req->clock_freq_in_khz / 1000;
+	uint32_t clk_freq = clock_req->clock_freq_in_mhz;
 	PPSMC_Msg        msg;
 
drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c +1 -1
 	switch (clk_type) {
@@ -203,7 +203,7 @@ static int smu10_set_clock_limit(struct pp_hwmgr
 *hwmgr, const void *input)
 
 	clocks.dcefClock = hwmgr->display_config->min_dcef_set_clk;
 	clock_req.clock_type = amd_pp_dcf_clock;
-	clock_req.clock_freq_in_khz = clocks.dcefClock * 10;
+	clock_req.clock_freq_in_mhz = clocks.dcefClock / 100;
 
 
PP_ASSERT_WITH_CODE(!smu10_display_clock_voltage_request(h
wmgr, &clock_req),
"Attempt to set DCF Clock Failed!", return -
EINVAL); diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
index 419a1d77d661..f926a46bf256 100644
drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c +1 -1
@@ -3740,7 +3740,7 @@ int vega10_display_clock_voltage_request(struct
 pp_hwmgr *hwmgr,  {
 	int result = 0;
 	enum amd_pp_clock_type clk_type = clock_req->clock_type;
-	uint32_t clk_freq = clock_req->clock_freq_in_khz / 1000;
+	uint32_t clk_freq = clock_req->clock_freq_in_mhz;
 	DSPCLK_e clk_select = 0;
 	uint32_t clk_request = 0;
 
@@ -3825,7 +3825,7 @@ static int
vega10_notify_smc_display_config_after_ps_adjustment(
drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c +0 -1
 	if (i < dpm_table->count) {
 		clock_req.clock_type = amd_pp_dcef_clock;
-		clock_req.clock_freq_in_khz = dpm_table-
Quoted 1 line
dpm_levels[i].value * 10;
drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c +1 -0
+		clock_req.clock_freq_in_mhz = dpm_table-
Quoted 1 line
dpm_levels[i].value / 100;
if (!vega10_display_clock_voltage_request(hwmgr,
&clock_req)) {
drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c +0 -0
 			smum_send_msg_to_smc_with_parameter(
 					hwmgr,
PPSMC_MSG_SetMinDeepSleepDcefclk, diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c index 74bc37308dc0..6478711f0b88 100644
drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c +1 -1
@@ -1349,7 +1349,7 @@ int vega12_display_clock_voltage_request(struct
 pp_hwmgr *hwmgr,
 	int result = 0;
 	struct vega12_hwmgr *data = (struct vega12_hwmgr *)(hwmgr-
 >backend);
 	enum amd_pp_clock_type clk_type = clock_req->clock_type;
-	uint32_t clk_freq = clock_req->clock_freq_in_khz / 1000;
+	uint32_t clk_freq = clock_req->clock_freq_in_mhz;
 	PPCLK_e clk_select = 0;
drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c +0 -0
 	uint32_t clk_request = 0;
@@ -1405,7 +1405,7 @@ static int
vega12_notify_smc_display_config_after_ps_adjustment(
drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c +1 -1
 	if (data->smu_features[GNLD_DPM_DCEFCLK].supported) {
 		clock_req.clock_type = amd_pp_dcef_clock;
-		clock_req.clock_freq_in_khz = min_clocks.dcefClock/10;
+		clock_req.clock_freq_in_mhz = min_clocks.dcefClock / 100;
 		if (!vega12_display_clock_voltage_request(hwmgr,
&clock_req)) {
if (data-
Quoted 1 line
smu_features[GNLD_DS_DCEFCLK].supported)
drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c +0 -0
 				PP_ASSERT_WITH_CODE(
drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c +1 -1
index 57143d51e3ee..7cddc39e4cd3 100644
@@ -1991,7 +1991,7 @@ int vega20_display_clock_voltage_request(struct
 pp_hwmgr *hwmgr,
 	int result = 0;
 	struct vega20_hwmgr *data = (struct vega20_hwmgr *)(hwmgr-
 >backend);
 	enum amd_pp_clock_type clk_type = clock_req->clock_type;
-	uint32_t clk_freq = clock_req->clock_freq_in_khz / 1000;
+	uint32_t clk_freq = clock_req->clock_freq_in_mhz;
 	PPCLK_e clk_select = 0;
drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c +0 -0
 	uint32_t clk_request = 0;
@@ -2057,7 +2057,7 @@ static int
vega20_notify_smc_display_config_after_ps_adjustment(
drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c +1 -1
 	if (data->smu_features[GNLD_DPM_DCEFCLK].supported) {
 		clock_req.clock_type = amd_pp_dcef_clock;
-		clock_req.clock_freq_in_khz = min_clocks.dcefClock * 10;
+		clock_req.clock_freq_in_mhz = min_clocks.dcefClock / 100;
 		if (!vega20_display_clock_voltage_request(hwmgr,
&clock_req)) {
if (data-
Quoted 1 line
smu_features[GNLD_DS_DCEFCLK].supported)
PP_ASSERT_WITH_CODE((ret =
smum_send_msg_to_smc_with_parameter(
--
2.17.1

Quoted 4 lines

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Wentland, Harry

2018-10-31 18:24 UTC #5 lore raw

Re: [PATCH 2/2] drm/amdgpu: Change pp clock requests to mHz

In reply to a message not in the archive

Quoted 114 lines · David Francis

We were multiplying clock requests by 1000 in amdgpu_dm and then dividing them by 1000 in powerplay.

Also, the vega12 code was dividing by 10 when it should have been multiplying (to convert units of 10kHz to units of kHz).

Signed-off-by David Francis <[email protected]>

---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 6 +++---
 drivers/gpu/drm/amd/include/dm_pp_interface.h            | 2 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c        | 4 ++--
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c       | 4 ++--
 drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c       | 4 ++--
 drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c       | 4 ++--
 6 files changed, 12 insertions(+), 12 deletions(-)

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c +3 -3

index d9daa038fdb2..cfa9b7f545b8 100644
@@ -436,7 +436,7 @@ bool dm_pp_apply_clock_for_voltage_request(
 	int ret = 0;
 
 	pp_clock_request.clock_type = dc_to_pp_clock_type(clock_for_voltage_req->clk_type);
-	pp_clock_request.clock_freq_in_khz = clock_for_voltage_req->clocks_in_khz;
+	pp_clock_request.clock_freq_in_mhz = clock_for_voltage_req->clocks_in_khz / 1000;
 
 	if (!pp_clock_request.clock_type)
 		return false;

@@ -485,11 +485,11 @@ void pp_rv_set_display_requirement(struct pp_smu *pp,
 		return;
 
 	clock.clock_type = amd_pp_dcf_clock;
-	clock.clock_freq_in_khz = req->hard_min_dcefclk_mhz * 1000;
+	clock.clock_freq_in_mhz = req->hard_min_dcefclk_mhz;
 	pp_funcs->display_clock_voltage_request(pp_handle, &clock);
 
 	clock.clock_type = amd_pp_f_clock;
-	clock.clock_freq_in_khz = req->hard_min_fclk_mhz * 1000;
+	clock.clock_freq_in_mhz = req->hard_min_fclk_mhz;
 	pp_funcs->display_clock_voltage_request(pp_handle, &clock);
 }

drivers/gpu/drm/amd/include/dm_pp_interface.h +1 -1

index 1d93a0c574c9..114ddd03e238 100644
@@ -188,7 +188,7 @@ struct pp_clock_levels_with_voltage {
 
 struct pp_display_clock_request {
 	enum amd_pp_clock_type clock_type;
-	uint32_t clock_freq_in_khz;
+	uint32_t clock_freq_in_mhz;
 };
 
 #endif /* _DM_PP_INTERFACE_ */

drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c +2 -2

index dd18cb710391..d6a6a4f4ac9d 100644
@@ -57,7 +57,7 @@ static int smu10_display_clock_voltage_request(struct pp_hwmgr *hwmgr,
 {
 	struct smu10_hwmgr *smu10_data = (struct smu10_hwmgr *)(hwmgr->backend);
 	enum amd_pp_clock_type clk_type = clock_req->clock_type;
-	uint32_t clk_freq = clock_req->clock_freq_in_khz / 1000;
+	uint32_t clk_freq = clock_req->clock_freq_in_mhz;
 	PPSMC_Msg        msg;
 
 	switch (clk_type) {

@@ -203,7 +203,7 @@ static int smu10_set_clock_limit(struct pp_hwmgr *hwmgr, const void *input)
 
 	clocks.dcefClock = hwmgr->display_config->min_dcef_set_clk;
 	clock_req.clock_type = amd_pp_dcf_clock;
-	clock_req.clock_freq_in_khz = clocks.dcefClock * 10;
+	clock_req.clock_freq_in_mhz = clocks.dcefClock / 100;
 
 	PP_ASSERT_WITH_CODE(!smu10_display_clock_voltage_request(hwmgr, &clock_req),
 				"Attempt to set DCF Clock Failed!", return -EINVAL);

drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c +2 -2

index 419a1d77d661..f926a46bf256 100644
@@ -3740,7 +3740,7 @@ int vega10_display_clock_voltage_request(struct pp_hwmgr *hwmgr,
 {
 	int result = 0;
 	enum amd_pp_clock_type clk_type = clock_req->clock_type;
-	uint32_t clk_freq = clock_req->clock_freq_in_khz / 1000;
+	uint32_t clk_freq = clock_req->clock_freq_in_mhz;
 	DSPCLK_e clk_select = 0;
 	uint32_t clk_request = 0;

@@ -3825,7 +3825,7 @@ static int vega10_notify_smc_display_config_after_ps_adjustment(
 
 	if (i < dpm_table->count) {
 		clock_req.clock_type = amd_pp_dcef_clock;
-		clock_req.clock_freq_in_khz = dpm_table->dpm_levels[i].value * 10;
+		clock_req.clock_freq_in_mhz = dpm_table->dpm_levels[i].value / 100;
 		if (!vega10_display_clock_voltage_request(hwmgr, &clock_req)) {
 			smum_send_msg_to_smc_with_parameter(
 					hwmgr, PPSMC_MSG_SetMinDeepSleepDcefclk,

drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c +1 -1

index 74bc37308dc0..6478711f0b88 100644
@@ -1349,7 +1349,7 @@ int vega12_display_clock_voltage_request(struct pp_hwmgr *hwmgr,
 	int result = 0;
 	struct vega12_hwmgr *data = (struct vega12_hwmgr *)(hwmgr->backend);
 	enum amd_pp_clock_type clk_type = clock_req->clock_type;
-	uint32_t clk_freq = clock_req->clock_freq_in_khz / 1000;
+	uint32_t clk_freq = clock_req->clock_freq_in_mhz;
 	PPCLK_e clk_select = 0;
 	uint32_t clk_request = 0;

drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c static int vega12_notify_smc_display_config_after_ps_adjustment( · line 1408

Quoted 4 lines

drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c +0 -1

@@ -1405,7 +1405,7 @@ static int vega12_notify_smc_display_config_after_ps_adjustment(
 
 	if (data->smu_features[GNLD_DPM_DCEFCLK].supported) {
 		clock_req.clock_type = amd_pp_dcef_clock;
-		clock_req.clock_freq_in_khz = min_clocks.dcefClock/10;

Was this incorrect before? If so, it's probably better to have a separate patch to correct it (i assume changing '/' to '*') before doing the khz to mhz change.

Harry

Quoted 19 lines

drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c +1 -0

+		clock_req.clock_freq_in_mhz = min_clocks.dcefClock / 100;
 		if (!vega12_display_clock_voltage_request(hwmgr, &clock_req)) {
 			if (data->smu_features[GNLD_DS_DCEFCLK].supported)
 				PP_ASSERT_WITH_CODE(

drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c +1 -1

index 57143d51e3ee..7cddc39e4cd3 100644
@@ -1991,7 +1991,7 @@ int vega20_display_clock_voltage_request(struct pp_hwmgr *hwmgr,
 	int result = 0;
 	struct vega20_hwmgr *data = (struct vega20_hwmgr *)(hwmgr->backend);
 	enum amd_pp_clock_type clk_type = clock_req->clock_type;
-	uint32_t clk_freq = clock_req->clock_freq_in_khz / 1000;
+	uint32_t clk_freq = clock_req->clock_freq_in_mhz;
 	PPCLK_e clk_select = 0;
 	uint32_t clk_request = 0;

drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c static int vega20_notify_smc_display_config_after_ps_adjustment( · line 2063

Quoted 8 lines

drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c +1 -1

@@ -2057,7 +2057,7 @@ static int vega20_notify_smc_display_config_after_ps_adjustment(
 
 	if (data->smu_features[GNLD_DPM_DCEFCLK].supported) {
 		clock_req.clock_type = amd_pp_dcef_clock;
-		clock_req.clock_freq_in_khz = min_clocks.dcefClock * 10;
+		clock_req.clock_freq_in_mhz = min_clocks.dcefClock / 100;
 		if (!vega20_display_clock_voltage_request(hwmgr, &clock_req)) {
 			if (data->smu_features[GNLD_DS_DCEFCLK].supported)
 				PP_ASSERT_WITH_CODE((ret = smum_send_msg_to_smc_with_parameter(

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Wentland, Harry

Reviewed-by Harry Wentland

2018-10-31 18:27 UTC #6 lore raw

Re: [PATCH 2/2] drm/amdgpu: Change pp clock requests to mHz

In reply to a message not in the archive

Quoted 7 lines · David Francis

We were multiplying clock requests by 1000 in amdgpu_dm and then dividing them by 1000 in powerplay.
Also, the vega12 code was dividing by 10 when it should have been multiplying (to convert units of 10kHz to units of kHz).
Signed-off-by David Francis <[email protected]>

Reviewed-by Harry Wentland <[email protected]>

Harry

Quoted 130 lines

---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 6 +++---
 drivers/gpu/drm/amd/include/dm_pp_interface.h            | 2 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c        | 4 ++--
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c       | 4 ++--
 drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c       | 4 ++--
 drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c       | 4 ++--
 6 files changed, 12 insertions(+), 12 deletions(-)

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c +3 -3

index d9daa038fdb2..cfa9b7f545b8 100644
@@ -436,7 +436,7 @@ bool dm_pp_apply_clock_for_voltage_request(
 	int ret = 0;
 
 	pp_clock_request.clock_type = dc_to_pp_clock_type(clock_for_voltage_req->clk_type);
-	pp_clock_request.clock_freq_in_khz = clock_for_voltage_req->clocks_in_khz;
+	pp_clock_request.clock_freq_in_mhz = clock_for_voltage_req->clocks_in_khz / 1000;
 
 	if (!pp_clock_request.clock_type)
 		return false;

@@ -485,11 +485,11 @@ void pp_rv_set_display_requirement(struct pp_smu *pp,
 		return;
 
 	clock.clock_type = amd_pp_dcf_clock;
-	clock.clock_freq_in_khz = req->hard_min_dcefclk_mhz * 1000;
+	clock.clock_freq_in_mhz = req->hard_min_dcefclk_mhz;
 	pp_funcs->display_clock_voltage_request(pp_handle, &clock);
 
 	clock.clock_type = amd_pp_f_clock;
-	clock.clock_freq_in_khz = req->hard_min_fclk_mhz * 1000;
+	clock.clock_freq_in_mhz = req->hard_min_fclk_mhz;
 	pp_funcs->display_clock_voltage_request(pp_handle, &clock);
 }

drivers/gpu/drm/amd/include/dm_pp_interface.h +1 -1

index 1d93a0c574c9..114ddd03e238 100644
@@ -188,7 +188,7 @@ struct pp_clock_levels_with_voltage {
 
 struct pp_display_clock_request {
 	enum amd_pp_clock_type clock_type;
-	uint32_t clock_freq_in_khz;
+	uint32_t clock_freq_in_mhz;
 };
 
 #endif /* _DM_PP_INTERFACE_ */

drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c +2 -2

index dd18cb710391..d6a6a4f4ac9d 100644
@@ -57,7 +57,7 @@ static int smu10_display_clock_voltage_request(struct pp_hwmgr *hwmgr,
 {
 	struct smu10_hwmgr *smu10_data = (struct smu10_hwmgr *)(hwmgr->backend);
 	enum amd_pp_clock_type clk_type = clock_req->clock_type;
-	uint32_t clk_freq = clock_req->clock_freq_in_khz / 1000;
+	uint32_t clk_freq = clock_req->clock_freq_in_mhz;
 	PPSMC_Msg        msg;
 
 	switch (clk_type) {

@@ -203,7 +203,7 @@ static int smu10_set_clock_limit(struct pp_hwmgr *hwmgr, const void *input)
 
 	clocks.dcefClock = hwmgr->display_config->min_dcef_set_clk;
 	clock_req.clock_type = amd_pp_dcf_clock;
-	clock_req.clock_freq_in_khz = clocks.dcefClock * 10;
+	clock_req.clock_freq_in_mhz = clocks.dcefClock / 100;
 
 	PP_ASSERT_WITH_CODE(!smu10_display_clock_voltage_request(hwmgr, &clock_req),
 				"Attempt to set DCF Clock Failed!", return -EINVAL);

drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c +2 -2

index 419a1d77d661..f926a46bf256 100644
@@ -3740,7 +3740,7 @@ int vega10_display_clock_voltage_request(struct pp_hwmgr *hwmgr,
 {
 	int result = 0;
 	enum amd_pp_clock_type clk_type = clock_req->clock_type;
-	uint32_t clk_freq = clock_req->clock_freq_in_khz / 1000;
+	uint32_t clk_freq = clock_req->clock_freq_in_mhz;
 	DSPCLK_e clk_select = 0;
 	uint32_t clk_request = 0;

@@ -3825,7 +3825,7 @@ static int vega10_notify_smc_display_config_after_ps_adjustment(
 
 	if (i < dpm_table->count) {
 		clock_req.clock_type = amd_pp_dcef_clock;
-		clock_req.clock_freq_in_khz = dpm_table->dpm_levels[i].value * 10;
+		clock_req.clock_freq_in_mhz = dpm_table->dpm_levels[i].value / 100;
 		if (!vega10_display_clock_voltage_request(hwmgr, &clock_req)) {
 			smum_send_msg_to_smc_with_parameter(
 					hwmgr, PPSMC_MSG_SetMinDeepSleepDcefclk,

drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c +2 -2

index 74bc37308dc0..6478711f0b88 100644
@@ -1349,7 +1349,7 @@ int vega12_display_clock_voltage_request(struct pp_hwmgr *hwmgr,
 	int result = 0;
 	struct vega12_hwmgr *data = (struct vega12_hwmgr *)(hwmgr->backend);
 	enum amd_pp_clock_type clk_type = clock_req->clock_type;
-	uint32_t clk_freq = clock_req->clock_freq_in_khz / 1000;
+	uint32_t clk_freq = clock_req->clock_freq_in_mhz;
 	PPCLK_e clk_select = 0;
 	uint32_t clk_request = 0;

@@ -1405,7 +1405,7 @@ static int vega12_notify_smc_display_config_after_ps_adjustment(
 
 	if (data->smu_features[GNLD_DPM_DCEFCLK].supported) {
 		clock_req.clock_type = amd_pp_dcef_clock;
-		clock_req.clock_freq_in_khz = min_clocks.dcefClock/10;
+		clock_req.clock_freq_in_mhz = min_clocks.dcefClock / 100;
 		if (!vega12_display_clock_voltage_request(hwmgr, &clock_req)) {
 			if (data->smu_features[GNLD_DS_DCEFCLK].supported)
 				PP_ASSERT_WITH_CODE(

drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c +1 -1

index 57143d51e3ee..7cddc39e4cd3 100644
@@ -1991,7 +1991,7 @@ int vega20_display_clock_voltage_request(struct pp_hwmgr *hwmgr,
 	int result = 0;
 	struct vega20_hwmgr *data = (struct vega20_hwmgr *)(hwmgr->backend);
 	enum amd_pp_clock_type clk_type = clock_req->clock_type;
-	uint32_t clk_freq = clock_req->clock_freq_in_khz / 1000;
+	uint32_t clk_freq = clock_req->clock_freq_in_mhz;
 	PPCLK_e clk_select = 0;
 	uint32_t clk_request = 0;

drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c static int vega20_notify_smc_display_config_after_ps_adjustment( · line 2063

Quoted 8 lines

drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c +1 -1

@@ -2057,7 +2057,7 @@ static int vega20_notify_smc_display_config_after_ps_adjustment(
 
 	if (data->smu_features[GNLD_DPM_DCEFCLK].supported) {
 		clock_req.clock_type = amd_pp_dcef_clock;
-		clock_req.clock_freq_in_khz = min_clocks.dcefClock * 10;
+		clock_req.clock_freq_in_mhz = min_clocks.dcefClock / 100;
 		if (!vega20_display_clock_voltage_request(hwmgr, &clock_req)) {
 			if (data->smu_features[GNLD_DS_DCEFCLK].supported)
 				PP_ASSERT_WITH_CODE((ret = smum_send_msg_to_smc_with_parameter(

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx