[PATCH 06/33] drm/amd/display: correctly populate dcn315 clock table

Rodrigo Siqueira Rodrigo.Siqueira at amd.com
Thu Oct 20 15:46:35 UTC 2022


From: Dmytro Laktyushkin <Dmytro.Laktyushkin at amd.com>

Fix incorrect pstate read order as well as min and max state logic.

Reviewed-by: Charlene Liu <Charlene.Liu at amd.com>
Acked-by: Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>
Signed-off-by: Dmytro Laktyushkin <Dmytro.Laktyushkin at amd.com>
---
 .../dc/clk_mgr/dcn315/dcn315_clk_mgr.c        | 71 ++++++-------------
 1 file changed, 23 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c
index 893991a0eb97..07edd9777edf 100644
--- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c
+++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c
@@ -458,19 +458,6 @@ static void dcn315_get_dpm_table_from_smu(struct clk_mgr_internal *clk_mgr,
 	dcn315_smu_transfer_dpm_table_smu_2_dram(clk_mgr);
 }
 
-static uint32_t find_max_clk_value(const uint32_t clocks[], uint32_t num_clocks)
-{
-	uint32_t max = 0;
-	int i;
-
-	for (i = 0; i < num_clocks; ++i) {
-		if (clocks[i] > max)
-			max = clocks[i];
-	}
-
-	return max;
-}
-
 static void dcn315_clk_mgr_helper_populate_bw_params(
 		struct clk_mgr_internal *clk_mgr,
 		struct integrated_info *bios_info,
@@ -478,29 +465,21 @@ static void dcn315_clk_mgr_helper_populate_bw_params(
 {
 	int i;
 	struct clk_bw_params *bw_params = clk_mgr->base.bw_params;
-	uint32_t max_pstate = 0, max_fclk = 0, min_pstate = 0;
+	uint32_t max_pstate = clock_table->NumDfPstatesEnabled - 1;
 	struct clk_limit_table_entry def_max = bw_params->clk_table.entries[bw_params->clk_table.num_entries - 1];
 
-	/* Find highest fclk pstate */
-	for (i = 0; i < clock_table->NumDfPstatesEnabled; i++) {
-		if (clock_table->DfPstateTable[i].FClk > max_fclk) {
-			max_fclk = clock_table->DfPstateTable[i].FClk;
-			max_pstate = i;
-		}
-	}
-
 	/* For 315 we want to base clock table on dcfclk, need at least one entry regardless of pmfw table */
 	for (i = 0; i < clock_table->NumDcfClkLevelsEnabled; i++) {
 		int j;
-		uint32_t min_fclk = clock_table->DfPstateTable[0].FClk;
 
-		for (j = 1; j < clock_table->NumDfPstatesEnabled; j++) {
-			if (clock_table->DfPstateTable[j].Voltage <= clock_table->SocVoltage[i]
-					&& clock_table->DfPstateTable[j].FClk < min_fclk) {
-				min_fclk = clock_table->DfPstateTable[j].FClk;
-				min_pstate = j;
-			}
+		/* DF table is sorted with clocks decreasing */
+		for (j = clock_table->NumDfPstatesEnabled - 2; j >= 0; j--) {
+			if (clock_table->DfPstateTable[j].Voltage <= clock_table->SocVoltage[i])
+				max_pstate = j;
 		}
+		/* Max DCFCLK should match up with max pstate */
+		if (i == clock_table->NumDcfClkLevelsEnabled - 1)
+			max_pstate = 0;
 
 		/* First search defaults for the clocks we don't read using closest lower or equal default dcfclk */
 		for (j = bw_params->clk_table.num_entries - 1; j > 0; j--)
@@ -511,9 +490,9 @@ static void dcn315_clk_mgr_helper_populate_bw_params(
 		bw_params->clk_table.entries[i].dtbclk_mhz = bw_params->clk_table.entries[j].dtbclk_mhz;
 
 		/* Now update clocks we do read */
-		bw_params->clk_table.entries[i].fclk_mhz = min_fclk;
-		bw_params->clk_table.entries[i].memclk_mhz = clock_table->DfPstateTable[min_pstate].MemClk;
-		bw_params->clk_table.entries[i].voltage = clock_table->DfPstateTable[min_pstate].Voltage;
+		bw_params->clk_table.entries[i].fclk_mhz = clock_table->DfPstateTable[max_pstate].FClk;
+		bw_params->clk_table.entries[i].memclk_mhz = clock_table->DfPstateTable[max_pstate].MemClk;
+		bw_params->clk_table.entries[i].voltage = clock_table->SocVoltage[i];
 		bw_params->clk_table.entries[i].dcfclk_mhz = clock_table->DcfClocks[i];
 		bw_params->clk_table.entries[i].socclk_mhz = clock_table->SocClocks[i];
 		bw_params->clk_table.entries[i].dispclk_mhz = clock_table->DispClocks[i];
@@ -521,25 +500,16 @@ static void dcn315_clk_mgr_helper_populate_bw_params(
 		bw_params->clk_table.entries[i].wck_ratio = 1;
 	}
 
-	/* Make sure to include at least one entry and highest pstate */
-	if (max_pstate != min_pstate || i == 0) {
-		bw_params->clk_table.entries[i].fclk_mhz = max_fclk;
-		bw_params->clk_table.entries[i].memclk_mhz = clock_table->DfPstateTable[max_pstate].MemClk;
-		bw_params->clk_table.entries[i].voltage = clock_table->DfPstateTable[max_pstate].Voltage;
-		bw_params->clk_table.entries[i].dcfclk_mhz = find_max_clk_value(clock_table->DcfClocks, NUM_DCFCLK_DPM_LEVELS);
+	/* Make sure to include at least one entry */
+	if (i == 0) {
+		bw_params->clk_table.entries[i].fclk_mhz = clock_table->DfPstateTable[0].FClk;
+		bw_params->clk_table.entries[i].memclk_mhz = clock_table->DfPstateTable[0].MemClk;
+		bw_params->clk_table.entries[i].voltage = clock_table->DfPstateTable[0].Voltage;
+		bw_params->clk_table.entries[i].dcfclk_mhz = clock_table->DcfClocks[0];
 		bw_params->clk_table.entries[i].wck_ratio = 1;
 		i++;
 	}
-	bw_params->clk_table.num_entries = i--;
-
-	/* Make sure all highest clocks are included*/
-	bw_params->clk_table.entries[i].socclk_mhz = find_max_clk_value(clock_table->SocClocks, NUM_SOCCLK_DPM_LEVELS);
-	bw_params->clk_table.entries[i].dispclk_mhz = find_max_clk_value(clock_table->DispClocks, NUM_DISPCLK_DPM_LEVELS);
-	bw_params->clk_table.entries[i].dppclk_mhz = find_max_clk_value(clock_table->DppClocks, NUM_DPPCLK_DPM_LEVELS);
-	ASSERT(clock_table->DcfClocks[i] == find_max_clk_value(clock_table->DcfClocks, NUM_DCFCLK_DPM_LEVELS));
-	bw_params->clk_table.entries[i].phyclk_mhz = def_max.phyclk_mhz;
-	bw_params->clk_table.entries[i].phyclk_d18_mhz = def_max.phyclk_d18_mhz;
-	bw_params->clk_table.entries[i].dtbclk_mhz = def_max.dtbclk_mhz;
+	bw_params->clk_table.num_entries = i;
 
 	/* Set any 0 clocks to max default setting. Not an issue for
 	 * power since we aren't doing switching in such case anyway
@@ -565,6 +535,11 @@ static void dcn315_clk_mgr_helper_populate_bw_params(
 		if (!bw_params->clk_table.entries[i].dtbclk_mhz)
 			bw_params->clk_table.entries[i].dtbclk_mhz = def_max.dtbclk_mhz;
 	}
+
+	/* Make sure all highest default clocks are included*/
+	ASSERT(bw_params->clk_table.entries[i-1].phyclk_mhz == def_max.phyclk_mhz);
+	ASSERT(bw_params->clk_table.entries[i-1].phyclk_d18_mhz == def_max.phyclk_d18_mhz);
+	ASSERT(bw_params->clk_table.entries[i-1].dtbclk_mhz == def_max.dtbclk_mhz);
 	ASSERT(bw_params->clk_table.entries[i-1].dcfclk_mhz);
 	bw_params->vram_type = bios_info->memory_type;
 	bw_params->num_channels = bios_info->ma_channel_number;
-- 
2.35.1



More information about the amd-gfx mailing list