drm/amdgpu: Stop dividing watermarks by 1000 in powerplay

PATCH Superseded David Francis 2 participants 4 messages Nov 1, 2018 – Nov 2, 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-11-01 19:07 UTC #0 lore raw

PATCH v2 1/3 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 2b83f922ac02..59a6e0ab811a 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): [email protected]

David Francis

author

2018-11-01 19:07 UTC #2 lore raw

PATCH v2 2/3 drm/amdgpu: Fix a bad unit conversion in vega12 powerplay

The default unit in powerplay is 10kHz

To convert from 10kHz to kHz, multiply by 10

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

---
 drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

index 74bc37308dc0..b6baf817b4db 100644
@@ -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_khz = min_clocks.dcefClock * 10;
 		if (!vega12_display_clock_voltage_request(hwmgr, &clock_req)) {
 			if (data->smu_features[GNLD_DS_DCEFCLK].supported)
 				PP_ASSERT_WITH_CODE(

Signature · 6 lines

2.17.1

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

David Francis

author

2018-11-01 19:07 UTC #3 lore raw

PATCH v2 3/3 drm/amdgpu: Change powerplay clock requests to mHz

In reply to a message not in the archive

This will clean up powerplay code, as we are no longer multiplying the clocks by 1000 in DM and then dividing them by 1000 in powerplay

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 b6baf817b4db..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): [email protected]

Michel Dänzer

2018-11-02 10:25 UTC #5 lore raw

Re: [PATCH v2 3/3] drm/amdgpu: Change powerplay clock requests to mHz

Quoted 5 lines · David Francis

This will clean up powerplay code, as we are no longer multiplying the clocks by 1000 in DM and then dividing them by 1000 in powerplay
Signed-off-by David Francis <[email protected]>

In the shortlog, "mHz" means millihertz, should be "MHz" for megahertz.

Signature · 6 lines

Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx