All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amd/powerplay: support custom power profile setting
@ 2019-12-23  8:05 Evan Quan
  2019-12-23 19:34 ` Alex Deucher
  0 siblings, 1 reply; 3+ messages in thread
From: Evan Quan @ 2019-12-23  8:05 UTC (permalink / raw)
  To: amd-gfx; +Cc: Evan Quan

Support custom power profile mode settings on Arcturus.

Change-Id: Id14f9a1cced41433b7487f447c452f8852964891
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
 drivers/gpu/drm/amd/powerplay/arcturus_ppt.c  | 132 +++++++++++++++++-
 .../powerplay/inc/smu11_driver_if_arcturus.h  |   6 +-
 2 files changed, 128 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index 9610b9b8a54c..043ac2ab0496 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -179,6 +179,7 @@ static struct smu_11_0_cmn2aisc_mapping arcturus_table_map[SMU_TABLE_COUNT] = {
 	TAB_MAP(DRIVER_SMU_CONFIG),
 	TAB_MAP(OVERDRIVE),
 	TAB_MAP(I2C_COMMANDS),
+	TAB_MAP(ACTIVITY_MONITOR_COEFF),
 };
 
 static struct smu_11_0_cmn2aisc_mapping arcturus_pwr_src_map[SMU_POWER_SOURCE_COUNT] = {
@@ -302,6 +303,10 @@ static int arcturus_tables_init(struct smu_context *smu, struct smu_table *table
 	SMU_TABLE_INIT(tables, SMU_TABLE_I2C_COMMANDS, sizeof(SwI2cRequest_t),
 			       PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM);
 
+	SMU_TABLE_INIT(tables, SMU_TABLE_ACTIVITY_MONITOR_COEFF,
+		       sizeof(DpmActivityMonitorCoeffInt_t), PAGE_SIZE,
+		       AMDGPU_GEM_DOMAIN_VRAM);
+
 	smu_table->metrics_table = kzalloc(sizeof(SmuMetrics_t), GFP_KERNEL);
 	if (!smu_table->metrics_table)
 		return -ENOMEM;
@@ -1314,6 +1319,7 @@ static int arcturus_get_power_limit(struct smu_context *smu,
 static int arcturus_get_power_profile_mode(struct smu_context *smu,
 					   char *buf)
 {
+	DpmActivityMonitorCoeffInt_t activity_monitor;
 	static const char *profile_name[] = {
 					"BOOTUP_DEFAULT",
 					"3D_FULL_SCREEN",
@@ -1323,14 +1329,35 @@ static int arcturus_get_power_profile_mode(struct smu_context *smu,
 					"COMPUTE",
 					"CUSTOM"};
 	static const char *title[] = {
-			"PROFILE_INDEX(NAME)"};
+			"PROFILE_INDEX(NAME)",
+			"CLOCK_TYPE(NAME)",
+			"FPS",
+			"UseRlcBusy",
+			"MinActiveFreqType",
+			"MinActiveFreq",
+			"BoosterFreqType",
+			"BoosterFreq",
+			"PD_Data_limit_c",
+			"PD_Data_error_coeff",
+			"PD_Data_error_rate_coeff"};
 	uint32_t i, size = 0;
 	int16_t workload_type = 0;
+	int result = 0;
+	uint32_t smu_version;
 
-	if (!smu->pm_enabled || !buf)
+	if (!buf)
 		return -EINVAL;
 
-	size += sprintf(buf + size, "%16s\n",
+	result = smu_get_smc_version(smu, NULL, &smu_version);
+	if (result)
+		return result;
+
+	if (smu_version >= 0x360d00)
+		size += sprintf(buf + size, "%16s %s %s %s %s %s %s %s %s %s %s\n",
+			title[0], title[1], title[2], title[3], title[4], title[5],
+			title[6], title[7], title[8], title[9], title[10]);
+	else
+		size += sprintf(buf + size, "%16s\n",
 			title[0]);
 
 	for (i = 0; i <= PP_SMC_POWER_PROFILE_CUSTOM; i++) {
@@ -1342,8 +1369,50 @@ static int arcturus_get_power_profile_mode(struct smu_context *smu,
 		if (workload_type < 0)
 			continue;
 
+		if (smu_version >= 0x360d00) {
+			result = smu_update_table(smu,
+						  SMU_TABLE_ACTIVITY_MONITOR_COEFF,
+						  workload_type,
+						  (void *)(&activity_monitor),
+						  false);
+			if (result) {
+				pr_err("[%s] Failed to get activity monitor!", __func__);
+				return result;
+			}
+		}
+
 		size += sprintf(buf + size, "%2d %14s%s\n",
 			i, profile_name[i], (i == smu->power_profile_mode) ? "*" : " ");
+
+		if (smu_version >= 0x360d00) {
+			size += sprintf(buf + size, "%19s %d(%13s) %7d %7d %7d %7d %7d %7d %7d %7d %7d\n",
+				" ",
+				0,
+				"GFXCLK",
+				activity_monitor.Gfx_FPS,
+				activity_monitor.Gfx_UseRlcBusy,
+				activity_monitor.Gfx_MinActiveFreqType,
+				activity_monitor.Gfx_MinActiveFreq,
+				activity_monitor.Gfx_BoosterFreqType,
+				activity_monitor.Gfx_BoosterFreq,
+				activity_monitor.Gfx_PD_Data_limit_c,
+				activity_monitor.Gfx_PD_Data_error_coeff,
+				activity_monitor.Gfx_PD_Data_error_rate_coeff);
+
+			size += sprintf(buf + size, "%19s %d(%13s) %7d %7d %7d %7d %7d %7d %7d %7d %7d\n",
+				" ",
+				1,
+				"UCLK",
+				activity_monitor.Mem_FPS,
+				activity_monitor.Mem_UseRlcBusy,
+				activity_monitor.Mem_MinActiveFreqType,
+				activity_monitor.Mem_MinActiveFreq,
+				activity_monitor.Mem_BoosterFreqType,
+				activity_monitor.Mem_BoosterFreq,
+				activity_monitor.Mem_PD_Data_limit_c,
+				activity_monitor.Mem_PD_Data_error_coeff,
+				activity_monitor.Mem_PD_Data_error_rate_coeff);
+		}
 	}
 
 	return size;
@@ -1353,18 +1422,69 @@ static int arcturus_set_power_profile_mode(struct smu_context *smu,
 					   long *input,
 					   uint32_t size)
 {
+	DpmActivityMonitorCoeffInt_t activity_monitor;
 	int workload_type = 0;
 	uint32_t profile_mode = input[size];
 	int ret = 0;
-
-	if (!smu->pm_enabled)
-		return -EINVAL;
+	uint32_t smu_version;
 
 	if (profile_mode > PP_SMC_POWER_PROFILE_CUSTOM) {
 		pr_err("Invalid power profile mode %d\n", profile_mode);
 		return -EINVAL;
 	}
 
+	ret = smu_get_smc_version(smu, NULL, &smu_version);
+	if (ret)
+		return ret;
+
+	if ((profile_mode == PP_SMC_POWER_PROFILE_CUSTOM) &&
+	     (smu_version >=0x360d00)) {
+		ret = smu_update_table(smu,
+				       SMU_TABLE_ACTIVITY_MONITOR_COEFF,
+				       WORKLOAD_PPLIB_CUSTOM_BIT,
+				       (void *)(&activity_monitor),
+				       false);
+		if (ret) {
+			pr_err("[%s] Failed to get activity monitor!", __func__);
+			return ret;
+		}
+
+		switch (input[0]) {
+		case 0: /* Gfxclk */
+			activity_monitor.Gfx_FPS = input[1];
+			activity_monitor.Gfx_UseRlcBusy = input[2];
+			activity_monitor.Gfx_MinActiveFreqType = input[3];
+			activity_monitor.Gfx_MinActiveFreq = input[4];
+			activity_monitor.Gfx_BoosterFreqType = input[5];
+			activity_monitor.Gfx_BoosterFreq = input[6];
+			activity_monitor.Gfx_PD_Data_limit_c = input[7];
+			activity_monitor.Gfx_PD_Data_error_coeff = input[8];
+			activity_monitor.Gfx_PD_Data_error_rate_coeff = input[9];
+			break;
+		case 1: /* Uclk */
+			activity_monitor.Mem_FPS = input[1];
+			activity_monitor.Mem_UseRlcBusy = input[2];
+			activity_monitor.Mem_MinActiveFreqType = input[3];
+			activity_monitor.Mem_MinActiveFreq = input[4];
+			activity_monitor.Mem_BoosterFreqType = input[5];
+			activity_monitor.Mem_BoosterFreq = input[6];
+			activity_monitor.Mem_PD_Data_limit_c = input[7];
+			activity_monitor.Mem_PD_Data_error_coeff = input[8];
+			activity_monitor.Mem_PD_Data_error_rate_coeff = input[9];
+			break;
+		}
+
+		ret = smu_update_table(smu,
+				       SMU_TABLE_ACTIVITY_MONITOR_COEFF,
+				       WORKLOAD_PPLIB_CUSTOM_BIT,
+				       (void *)(&activity_monitor),
+				       true);
+		if (ret) {
+			pr_err("[%s] Failed to set activity monitor!", __func__);
+			return ret;
+		}
+	}
+
 	/*
 	 * Conv PP_SMC_POWER_PROFILE* to WORKLOAD_PPLIB_*_BIT
 	 * Not all profile modes are supported on arcturus.
diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if_arcturus.h b/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if_arcturus.h
index a886f0644d24..910226ec512e 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if_arcturus.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if_arcturus.h
@@ -823,7 +823,6 @@ typedef struct {
   uint32_t MmHubPadding[8]; // SMU internal use
 } AvfsFuseOverride_t;
 
-/* NOT CURRENTLY USED
 typedef struct {
   uint8_t   Gfx_ActiveHystLimit;
   uint8_t   Gfx_IdleHystLimit;
@@ -866,7 +865,6 @@ typedef struct {
 
   uint32_t  MmHubPadding[8]; // SMU internal use
 } DpmActivityMonitorCoeffInt_t;
-*/
 
 // These defines are used with the following messages:
 // SMC_MSG_TransferTableDram2Smu
@@ -878,11 +876,11 @@ typedef struct {
 #define TABLE_PMSTATUSLOG             4
 #define TABLE_SMU_METRICS             5
 #define TABLE_DRIVER_SMU_CONFIG       6
-//#define TABLE_ACTIVITY_MONITOR_COEFF  7
 #define TABLE_OVERDRIVE               7
 #define TABLE_WAFL_XGMI_TOPOLOGY      8
 #define TABLE_I2C_COMMANDS            9
-#define TABLE_COUNT                   10
+#define TABLE_ACTIVITY_MONITOR_COEFF  10
+#define TABLE_COUNT                   11
 
 // These defines are used with the SMC_MSG_SetUclkFastSwitch message.
 typedef enum {
-- 
2.24.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/amd/powerplay: support custom power profile setting
  2019-12-23  8:05 [PATCH] drm/amd/powerplay: support custom power profile setting Evan Quan
@ 2019-12-23 19:34 ` Alex Deucher
  2019-12-24  3:06   ` Quan, Evan
  0 siblings, 1 reply; 3+ messages in thread
From: Alex Deucher @ 2019-12-23 19:34 UTC (permalink / raw)
  To: Evan Quan; +Cc: amd-gfx list

On Mon, Dec 23, 2019 at 3:05 AM Evan Quan <evan.quan@amd.com> wrote:
>
> Support custom power profile mode settings on Arcturus.
>
> Change-Id: Id14f9a1cced41433b7487f447c452f8852964891
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> ---
>  drivers/gpu/drm/amd/powerplay/arcturus_ppt.c  | 132 +++++++++++++++++-
>  .../powerplay/inc/smu11_driver_if_arcturus.h  |   6 +-
>  2 files changed, 128 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> index 9610b9b8a54c..043ac2ab0496 100644
> --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> @@ -179,6 +179,7 @@ static struct smu_11_0_cmn2aisc_mapping arcturus_table_map[SMU_TABLE_COUNT] = {
>         TAB_MAP(DRIVER_SMU_CONFIG),
>         TAB_MAP(OVERDRIVE),
>         TAB_MAP(I2C_COMMANDS),
> +       TAB_MAP(ACTIVITY_MONITOR_COEFF),
>  };
>
>  static struct smu_11_0_cmn2aisc_mapping arcturus_pwr_src_map[SMU_POWER_SOURCE_COUNT] = {
> @@ -302,6 +303,10 @@ static int arcturus_tables_init(struct smu_context *smu, struct smu_table *table
>         SMU_TABLE_INIT(tables, SMU_TABLE_I2C_COMMANDS, sizeof(SwI2cRequest_t),
>                                PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM);
>
> +       SMU_TABLE_INIT(tables, SMU_TABLE_ACTIVITY_MONITOR_COEFF,
> +                      sizeof(DpmActivityMonitorCoeffInt_t), PAGE_SIZE,
> +                      AMDGPU_GEM_DOMAIN_VRAM);
> +

Is freeing this handled properly?  Assuming that is ok, the patch is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

>         smu_table->metrics_table = kzalloc(sizeof(SmuMetrics_t), GFP_KERNEL);
>         if (!smu_table->metrics_table)
>                 return -ENOMEM;
> @@ -1314,6 +1319,7 @@ static int arcturus_get_power_limit(struct smu_context *smu,
>  static int arcturus_get_power_profile_mode(struct smu_context *smu,
>                                            char *buf)
>  {
> +       DpmActivityMonitorCoeffInt_t activity_monitor;
>         static const char *profile_name[] = {
>                                         "BOOTUP_DEFAULT",
>                                         "3D_FULL_SCREEN",
> @@ -1323,14 +1329,35 @@ static int arcturus_get_power_profile_mode(struct smu_context *smu,
>                                         "COMPUTE",
>                                         "CUSTOM"};
>         static const char *title[] = {
> -                       "PROFILE_INDEX(NAME)"};
> +                       "PROFILE_INDEX(NAME)",
> +                       "CLOCK_TYPE(NAME)",
> +                       "FPS",
> +                       "UseRlcBusy",
> +                       "MinActiveFreqType",
> +                       "MinActiveFreq",
> +                       "BoosterFreqType",
> +                       "BoosterFreq",
> +                       "PD_Data_limit_c",
> +                       "PD_Data_error_coeff",
> +                       "PD_Data_error_rate_coeff"};
>         uint32_t i, size = 0;
>         int16_t workload_type = 0;
> +       int result = 0;
> +       uint32_t smu_version;
>
> -       if (!smu->pm_enabled || !buf)
> +       if (!buf)
>                 return -EINVAL;
>
> -       size += sprintf(buf + size, "%16s\n",
> +       result = smu_get_smc_version(smu, NULL, &smu_version);
> +       if (result)
> +               return result;
> +
> +       if (smu_version >= 0x360d00)
> +               size += sprintf(buf + size, "%16s %s %s %s %s %s %s %s %s %s %s\n",
> +                       title[0], title[1], title[2], title[3], title[4], title[5],
> +                       title[6], title[7], title[8], title[9], title[10]);
> +       else
> +               size += sprintf(buf + size, "%16s\n",
>                         title[0]);
>
>         for (i = 0; i <= PP_SMC_POWER_PROFILE_CUSTOM; i++) {
> @@ -1342,8 +1369,50 @@ static int arcturus_get_power_profile_mode(struct smu_context *smu,
>                 if (workload_type < 0)
>                         continue;
>
> +               if (smu_version >= 0x360d00) {
> +                       result = smu_update_table(smu,
> +                                                 SMU_TABLE_ACTIVITY_MONITOR_COEFF,
> +                                                 workload_type,
> +                                                 (void *)(&activity_monitor),
> +                                                 false);
> +                       if (result) {
> +                               pr_err("[%s] Failed to get activity monitor!", __func__);
> +                               return result;
> +                       }
> +               }
> +
>                 size += sprintf(buf + size, "%2d %14s%s\n",
>                         i, profile_name[i], (i == smu->power_profile_mode) ? "*" : " ");
> +
> +               if (smu_version >= 0x360d00) {
> +                       size += sprintf(buf + size, "%19s %d(%13s) %7d %7d %7d %7d %7d %7d %7d %7d %7d\n",
> +                               " ",
> +                               0,
> +                               "GFXCLK",
> +                               activity_monitor.Gfx_FPS,
> +                               activity_monitor.Gfx_UseRlcBusy,
> +                               activity_monitor.Gfx_MinActiveFreqType,
> +                               activity_monitor.Gfx_MinActiveFreq,
> +                               activity_monitor.Gfx_BoosterFreqType,
> +                               activity_monitor.Gfx_BoosterFreq,
> +                               activity_monitor.Gfx_PD_Data_limit_c,
> +                               activity_monitor.Gfx_PD_Data_error_coeff,
> +                               activity_monitor.Gfx_PD_Data_error_rate_coeff);
> +
> +                       size += sprintf(buf + size, "%19s %d(%13s) %7d %7d %7d %7d %7d %7d %7d %7d %7d\n",
> +                               " ",
> +                               1,
> +                               "UCLK",
> +                               activity_monitor.Mem_FPS,
> +                               activity_monitor.Mem_UseRlcBusy,
> +                               activity_monitor.Mem_MinActiveFreqType,
> +                               activity_monitor.Mem_MinActiveFreq,
> +                               activity_monitor.Mem_BoosterFreqType,
> +                               activity_monitor.Mem_BoosterFreq,
> +                               activity_monitor.Mem_PD_Data_limit_c,
> +                               activity_monitor.Mem_PD_Data_error_coeff,
> +                               activity_monitor.Mem_PD_Data_error_rate_coeff);
> +               }
>         }
>
>         return size;
> @@ -1353,18 +1422,69 @@ static int arcturus_set_power_profile_mode(struct smu_context *smu,
>                                            long *input,
>                                            uint32_t size)
>  {
> +       DpmActivityMonitorCoeffInt_t activity_monitor;
>         int workload_type = 0;
>         uint32_t profile_mode = input[size];
>         int ret = 0;
> -
> -       if (!smu->pm_enabled)
> -               return -EINVAL;
> +       uint32_t smu_version;
>
>         if (profile_mode > PP_SMC_POWER_PROFILE_CUSTOM) {
>                 pr_err("Invalid power profile mode %d\n", profile_mode);
>                 return -EINVAL;
>         }
>
> +       ret = smu_get_smc_version(smu, NULL, &smu_version);
> +       if (ret)
> +               return ret;
> +
> +       if ((profile_mode == PP_SMC_POWER_PROFILE_CUSTOM) &&
> +            (smu_version >=0x360d00)) {
> +               ret = smu_update_table(smu,
> +                                      SMU_TABLE_ACTIVITY_MONITOR_COEFF,
> +                                      WORKLOAD_PPLIB_CUSTOM_BIT,
> +                                      (void *)(&activity_monitor),
> +                                      false);
> +               if (ret) {
> +                       pr_err("[%s] Failed to get activity monitor!", __func__);
> +                       return ret;
> +               }
> +
> +               switch (input[0]) {
> +               case 0: /* Gfxclk */
> +                       activity_monitor.Gfx_FPS = input[1];
> +                       activity_monitor.Gfx_UseRlcBusy = input[2];
> +                       activity_monitor.Gfx_MinActiveFreqType = input[3];
> +                       activity_monitor.Gfx_MinActiveFreq = input[4];
> +                       activity_monitor.Gfx_BoosterFreqType = input[5];
> +                       activity_monitor.Gfx_BoosterFreq = input[6];
> +                       activity_monitor.Gfx_PD_Data_limit_c = input[7];
> +                       activity_monitor.Gfx_PD_Data_error_coeff = input[8];
> +                       activity_monitor.Gfx_PD_Data_error_rate_coeff = input[9];
> +                       break;
> +               case 1: /* Uclk */
> +                       activity_monitor.Mem_FPS = input[1];
> +                       activity_monitor.Mem_UseRlcBusy = input[2];
> +                       activity_monitor.Mem_MinActiveFreqType = input[3];
> +                       activity_monitor.Mem_MinActiveFreq = input[4];
> +                       activity_monitor.Mem_BoosterFreqType = input[5];
> +                       activity_monitor.Mem_BoosterFreq = input[6];
> +                       activity_monitor.Mem_PD_Data_limit_c = input[7];
> +                       activity_monitor.Mem_PD_Data_error_coeff = input[8];
> +                       activity_monitor.Mem_PD_Data_error_rate_coeff = input[9];
> +                       break;
> +               }
> +
> +               ret = smu_update_table(smu,
> +                                      SMU_TABLE_ACTIVITY_MONITOR_COEFF,
> +                                      WORKLOAD_PPLIB_CUSTOM_BIT,
> +                                      (void *)(&activity_monitor),
> +                                      true);
> +               if (ret) {
> +                       pr_err("[%s] Failed to set activity monitor!", __func__);
> +                       return ret;
> +               }
> +       }
> +
>         /*
>          * Conv PP_SMC_POWER_PROFILE* to WORKLOAD_PPLIB_*_BIT
>          * Not all profile modes are supported on arcturus.
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if_arcturus.h b/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if_arcturus.h
> index a886f0644d24..910226ec512e 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if_arcturus.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if_arcturus.h
> @@ -823,7 +823,6 @@ typedef struct {
>    uint32_t MmHubPadding[8]; // SMU internal use
>  } AvfsFuseOverride_t;
>
> -/* NOT CURRENTLY USED
>  typedef struct {
>    uint8_t   Gfx_ActiveHystLimit;
>    uint8_t   Gfx_IdleHystLimit;
> @@ -866,7 +865,6 @@ typedef struct {
>
>    uint32_t  MmHubPadding[8]; // SMU internal use
>  } DpmActivityMonitorCoeffInt_t;
> -*/
>
>  // These defines are used with the following messages:
>  // SMC_MSG_TransferTableDram2Smu
> @@ -878,11 +876,11 @@ typedef struct {
>  #define TABLE_PMSTATUSLOG             4
>  #define TABLE_SMU_METRICS             5
>  #define TABLE_DRIVER_SMU_CONFIG       6
> -//#define TABLE_ACTIVITY_MONITOR_COEFF  7
>  #define TABLE_OVERDRIVE               7
>  #define TABLE_WAFL_XGMI_TOPOLOGY      8
>  #define TABLE_I2C_COMMANDS            9
> -#define TABLE_COUNT                   10
> +#define TABLE_ACTIVITY_MONITOR_COEFF  10
> +#define TABLE_COUNT                   11
>
>  // These defines are used with the SMC_MSG_SetUclkFastSwitch message.
>  typedef enum {
> --
> 2.24.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH] drm/amd/powerplay: support custom power profile setting
  2019-12-23 19:34 ` Alex Deucher
@ 2019-12-24  3:06   ` Quan, Evan
  0 siblings, 0 replies; 3+ messages in thread
From: Quan, Evan @ 2019-12-24  3:06 UTC (permalink / raw)
  To: Alex Deucher; +Cc: amd-gfx list



> -----Original Message-----
> From: Alex Deucher <alexdeucher@gmail.com>
> Sent: Tuesday, December 24, 2019 3:35 AM
> To: Quan, Evan <Evan.Quan@amd.com>
> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>
> Subject: Re: [PATCH] drm/amd/powerplay: support custom power profile
> setting
> 
> On Mon, Dec 23, 2019 at 3:05 AM Evan Quan <evan.quan@amd.com> wrote:
> >
> > Support custom power profile mode settings on Arcturus.
> >
> > Change-Id: Id14f9a1cced41433b7487f447c452f8852964891
> > Signed-off-by: Evan Quan <evan.quan@amd.com>
> > ---
> >  drivers/gpu/drm/amd/powerplay/arcturus_ppt.c  | 132
> +++++++++++++++++-
> >  .../powerplay/inc/smu11_driver_if_arcturus.h  |   6 +-
> >  2 files changed, 128 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> > b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> > index 9610b9b8a54c..043ac2ab0496 100644
> > --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> > +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> > @@ -179,6 +179,7 @@ static struct smu_11_0_cmn2aisc_mapping
> arcturus_table_map[SMU_TABLE_COUNT] = {
> >         TAB_MAP(DRIVER_SMU_CONFIG),
> >         TAB_MAP(OVERDRIVE),
> >         TAB_MAP(I2C_COMMANDS),
> > +       TAB_MAP(ACTIVITY_MONITOR_COEFF),
> >  };
> >
> >  static struct smu_11_0_cmn2aisc_mapping
> > arcturus_pwr_src_map[SMU_POWER_SOURCE_COUNT] = { @@ -302,6
> +303,10 @@ static int arcturus_tables_init(struct smu_context *smu, struct
> smu_table *table
> >         SMU_TABLE_INIT(tables, SMU_TABLE_I2C_COMMANDS,
> sizeof(SwI2cRequest_t),
> >                                PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM);
> >
> > +       SMU_TABLE_INIT(tables, SMU_TABLE_ACTIVITY_MONITOR_COEFF,
> > +                      sizeof(DpmActivityMonitorCoeffInt_t), PAGE_SIZE,
> > +                      AMDGPU_GEM_DOMAIN_VRAM);
> > +
> 
> Is freeing this handled properly?  Assuming that is ok, the patch is:
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
[Quan, Evan] Yes, that is well handled. smu_fini_fb_allocations() is responsible for reclaiming all the tables.
> 
> >         smu_table->metrics_table = kzalloc(sizeof(SmuMetrics_t), GFP_KERNEL);
> >         if (!smu_table->metrics_table)
> >                 return -ENOMEM;
> > @@ -1314,6 +1319,7 @@ static int arcturus_get_power_limit(struct
> > smu_context *smu,  static int arcturus_get_power_profile_mode(struct
> smu_context *smu,
> >                                            char *buf)  {
> > +       DpmActivityMonitorCoeffInt_t activity_monitor;
> >         static const char *profile_name[] = {
> >                                         "BOOTUP_DEFAULT",
> >                                         "3D_FULL_SCREEN", @@ -1323,14
> > +1329,35 @@ static int arcturus_get_power_profile_mode(struct
> smu_context *smu,
> >                                         "COMPUTE",
> >                                         "CUSTOM"};
> >         static const char *title[] = {
> > -                       "PROFILE_INDEX(NAME)"};
> > +                       "PROFILE_INDEX(NAME)",
> > +                       "CLOCK_TYPE(NAME)",
> > +                       "FPS",
> > +                       "UseRlcBusy",
> > +                       "MinActiveFreqType",
> > +                       "MinActiveFreq",
> > +                       "BoosterFreqType",
> > +                       "BoosterFreq",
> > +                       "PD_Data_limit_c",
> > +                       "PD_Data_error_coeff",
> > +                       "PD_Data_error_rate_coeff"};
> >         uint32_t i, size = 0;
> >         int16_t workload_type = 0;
> > +       int result = 0;
> > +       uint32_t smu_version;
> >
> > -       if (!smu->pm_enabled || !buf)
> > +       if (!buf)
> >                 return -EINVAL;
> >
> > -       size += sprintf(buf + size, "%16s\n",
> > +       result = smu_get_smc_version(smu, NULL, &smu_version);
> > +       if (result)
> > +               return result;
> > +
> > +       if (smu_version >= 0x360d00)
> > +               size += sprintf(buf + size, "%16s %s %s %s %s %s %s %s %s %s %s\n",
> > +                       title[0], title[1], title[2], title[3], title[4], title[5],
> > +                       title[6], title[7], title[8], title[9], title[10]);
> > +       else
> > +               size += sprintf(buf + size, "%16s\n",
> >                         title[0]);
> >
> >         for (i = 0; i <= PP_SMC_POWER_PROFILE_CUSTOM; i++) { @@
> > -1342,8 +1369,50 @@ static int arcturus_get_power_profile_mode(struct
> smu_context *smu,
> >                 if (workload_type < 0)
> >                         continue;
> >
> > +               if (smu_version >= 0x360d00) {
> > +                       result = smu_update_table(smu,
> > +                                                 SMU_TABLE_ACTIVITY_MONITOR_COEFF,
> > +                                                 workload_type,
> > +                                                 (void *)(&activity_monitor),
> > +                                                 false);
> > +                       if (result) {
> > +                               pr_err("[%s] Failed to get activity monitor!", __func__);
> > +                               return result;
> > +                       }
> > +               }
> > +
> >                 size += sprintf(buf + size, "%2d %14s%s\n",
> >                         i, profile_name[i], (i ==
> > smu->power_profile_mode) ? "*" : " ");
> > +
> > +               if (smu_version >= 0x360d00) {
> > +                       size += sprintf(buf + size,
> "%19s %d(%13s) %7d %7d %7d %7d %7d %7d %7d %7d %7d\n",
> > +                               " ",
> > +                               0,
> > +                               "GFXCLK",
> > +                               activity_monitor.Gfx_FPS,
> > +                               activity_monitor.Gfx_UseRlcBusy,
> > +                               activity_monitor.Gfx_MinActiveFreqType,
> > +                               activity_monitor.Gfx_MinActiveFreq,
> > +                               activity_monitor.Gfx_BoosterFreqType,
> > +                               activity_monitor.Gfx_BoosterFreq,
> > +                               activity_monitor.Gfx_PD_Data_limit_c,
> > +                               activity_monitor.Gfx_PD_Data_error_coeff,
> > +
> > + activity_monitor.Gfx_PD_Data_error_rate_coeff);
> > +
> > +                       size += sprintf(buf + size,
> "%19s %d(%13s) %7d %7d %7d %7d %7d %7d %7d %7d %7d\n",
> > +                               " ",
> > +                               1,
> > +                               "UCLK",
> > +                               activity_monitor.Mem_FPS,
> > +                               activity_monitor.Mem_UseRlcBusy,
> > +                               activity_monitor.Mem_MinActiveFreqType,
> > +                               activity_monitor.Mem_MinActiveFreq,
> > +                               activity_monitor.Mem_BoosterFreqType,
> > +                               activity_monitor.Mem_BoosterFreq,
> > +                               activity_monitor.Mem_PD_Data_limit_c,
> > +                               activity_monitor.Mem_PD_Data_error_coeff,
> > +                               activity_monitor.Mem_PD_Data_error_rate_coeff);
> > +               }
> >         }
> >
> >         return size;
> > @@ -1353,18 +1422,69 @@ static int
> arcturus_set_power_profile_mode(struct smu_context *smu,
> >                                            long *input,
> >                                            uint32_t size)  {
> > +       DpmActivityMonitorCoeffInt_t activity_monitor;
> >         int workload_type = 0;
> >         uint32_t profile_mode = input[size];
> >         int ret = 0;
> > -
> > -       if (!smu->pm_enabled)
> > -               return -EINVAL;
> > +       uint32_t smu_version;
> >
> >         if (profile_mode > PP_SMC_POWER_PROFILE_CUSTOM) {
> >                 pr_err("Invalid power profile mode %d\n", profile_mode);
> >                 return -EINVAL;
> >         }
> >
> > +       ret = smu_get_smc_version(smu, NULL, &smu_version);
> > +       if (ret)
> > +               return ret;
> > +
> > +       if ((profile_mode == PP_SMC_POWER_PROFILE_CUSTOM) &&
> > +            (smu_version >=0x360d00)) {
> > +               ret = smu_update_table(smu,
> > +                                      SMU_TABLE_ACTIVITY_MONITOR_COEFF,
> > +                                      WORKLOAD_PPLIB_CUSTOM_BIT,
> > +                                      (void *)(&activity_monitor),
> > +                                      false);
> > +               if (ret) {
> > +                       pr_err("[%s] Failed to get activity monitor!", __func__);
> > +                       return ret;
> > +               }
> > +
> > +               switch (input[0]) {
> > +               case 0: /* Gfxclk */
> > +                       activity_monitor.Gfx_FPS = input[1];
> > +                       activity_monitor.Gfx_UseRlcBusy = input[2];
> > +                       activity_monitor.Gfx_MinActiveFreqType = input[3];
> > +                       activity_monitor.Gfx_MinActiveFreq = input[4];
> > +                       activity_monitor.Gfx_BoosterFreqType = input[5];
> > +                       activity_monitor.Gfx_BoosterFreq = input[6];
> > +                       activity_monitor.Gfx_PD_Data_limit_c = input[7];
> > +                       activity_monitor.Gfx_PD_Data_error_coeff = input[8];
> > +                       activity_monitor.Gfx_PD_Data_error_rate_coeff = input[9];
> > +                       break;
> > +               case 1: /* Uclk */
> > +                       activity_monitor.Mem_FPS = input[1];
> > +                       activity_monitor.Mem_UseRlcBusy = input[2];
> > +                       activity_monitor.Mem_MinActiveFreqType = input[3];
> > +                       activity_monitor.Mem_MinActiveFreq = input[4];
> > +                       activity_monitor.Mem_BoosterFreqType = input[5];
> > +                       activity_monitor.Mem_BoosterFreq = input[6];
> > +                       activity_monitor.Mem_PD_Data_limit_c = input[7];
> > +                       activity_monitor.Mem_PD_Data_error_coeff = input[8];
> > +                       activity_monitor.Mem_PD_Data_error_rate_coeff = input[9];
> > +                       break;
> > +               }
> > +
> > +               ret = smu_update_table(smu,
> > +                                      SMU_TABLE_ACTIVITY_MONITOR_COEFF,
> > +                                      WORKLOAD_PPLIB_CUSTOM_BIT,
> > +                                      (void *)(&activity_monitor),
> > +                                      true);
> > +               if (ret) {
> > +                       pr_err("[%s] Failed to set activity monitor!", __func__);
> > +                       return ret;
> > +               }
> > +       }
> > +
> >         /*
> >          * Conv PP_SMC_POWER_PROFILE* to WORKLOAD_PPLIB_*_BIT
> >          * Not all profile modes are supported on arcturus.
> > diff --git
> > a/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if_arcturus.h
> > b/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if_arcturus.h
> > index a886f0644d24..910226ec512e 100644
> > --- a/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if_arcturus.h
> > +++ b/drivers/gpu/drm/amd/powerplay/inc/smu11_driver_if_arcturus.h
> > @@ -823,7 +823,6 @@ typedef struct {
> >    uint32_t MmHubPadding[8]; // SMU internal use  }
> > AvfsFuseOverride_t;
> >
> > -/* NOT CURRENTLY USED
> >  typedef struct {
> >    uint8_t   Gfx_ActiveHystLimit;
> >    uint8_t   Gfx_IdleHystLimit;
> > @@ -866,7 +865,6 @@ typedef struct {
> >
> >    uint32_t  MmHubPadding[8]; // SMU internal use  }
> > DpmActivityMonitorCoeffInt_t; -*/
> >
> >  // These defines are used with the following messages:
> >  // SMC_MSG_TransferTableDram2Smu
> > @@ -878,11 +876,11 @@ typedef struct {
> >  #define TABLE_PMSTATUSLOG             4
> >  #define TABLE_SMU_METRICS             5
> >  #define TABLE_DRIVER_SMU_CONFIG       6
> > -//#define TABLE_ACTIVITY_MONITOR_COEFF  7
> >  #define TABLE_OVERDRIVE               7
> >  #define TABLE_WAFL_XGMI_TOPOLOGY      8
> >  #define TABLE_I2C_COMMANDS            9
> > -#define TABLE_COUNT                   10
> > +#define TABLE_ACTIVITY_MONITOR_COEFF  10
> > +#define TABLE_COUNT                   11
> >
> >  // These defines are used with the SMC_MSG_SetUclkFastSwitch message.
> >  typedef enum {
> > --
> > 2.24.0
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> > s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-
> gfx&amp;data=02%7C01%7Cev
> >
> an.quan%40amd.com%7Cc291ae2fc8ad4b4c095208d787df37bb%7C3dd8961fe
> 4884e6
> >
> 08e11a82d994e183d%7C0%7C0%7C637127265093035498&amp;sdata=hqgAJP
> %2F9vIG
> > H8%2F2D6XgpyqtVXkMDeXpeK44aN8k9Xw4%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-12-24  3:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-23  8:05 [PATCH] drm/amd/powerplay: support custom power profile setting Evan Quan
2019-12-23 19:34 ` Alex Deucher
2019-12-24  3:06   ` Quan, Evan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.