[PATCH 1/4] drm/amd/pp: Add new smu callback function

[PATCH 1/4] drm/amd/pp: Add new smu callback function

[Rex Zhu]

it is used for update dpm settings

Change-Id: Idc0362c219d84564693ca90adf9299e56cfeb6a4
Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
---
 drivers/gpu/drm/amd/powerplay/inc/hwmgr.h     | 1 +
 drivers/gpu/drm/amd/powerplay/inc/smumgr.h    | 1 +
 drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c | 8 ++++++++
 3 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
index 604a7cb..3e8959e 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
@@ -238,6 +238,7 @@ struct pp_smumgr_func {
 	int (*populate_requested_graphic_levels)(struct pp_hwmgr *hwmgr,
 			struct amd_pp_profile *request);
 	bool (*is_hw_avfs_present)(struct pp_hwmgr  *hwmgr);
+	int (*update_dpm_settings)(struct pp_hwmgr *hwmgr, void *profile_setting);
 };
 
 struct pp_hwmgr_func {
diff --git a/drivers/gpu/drm/amd/powerplay/inc/smumgr.h b/drivers/gpu/drm/amd/powerplay/inc/smumgr.h
index b1b27b2..e05a57e 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/smumgr.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/smumgr.h
@@ -134,5 +134,6 @@ extern int smum_populate_requested_graphic_levels(struct pp_hwmgr *hwmgr,
 
 extern bool smum_is_hw_avfs_present(struct pp_hwmgr *hwmgr);
 
+extern int smum_update_dpm_settings(struct pp_hwmgr *hwmgr, void *profile_setting);
 
 #endif
diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c b/drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c
index 8673884..1ce4959 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c
@@ -253,3 +253,11 @@ bool smum_is_hw_avfs_present(struct pp_hwmgr *hwmgr)
 
 	return false;
 }
+
+int smum_update_dpm_settings(struct pp_hwmgr *hwmgr, void *profile_setting)
+{
+	if (hwmgr->smumgr_funcs->update_dpm_settings)
+		return hwmgr->smumgr_funcs->update_dpm_settings(hwmgr, profile_setting);
+
+	return -EINVAL;
+}
-- 
1.9.1

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

[PATCH 2/4] drm/amd/pp: Implement update_dpm_settings on Polaris

[Rex Zhu]

Change-Id: I4533826ef6e18df125ae4445016873be3b5fe0ce
Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
---
 .../drm/amd/powerplay/smumgr/polaris10_smumgr.c    | 104 +++++++++++++++++++++
 1 file changed, 104 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c b/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c
index bfb2c85..559572d 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c
@@ -2575,6 +2575,109 @@ static int polaris10_populate_requested_graphic_levels(struct pp_hwmgr *hwmgr,
 				array_size, SMC_RAM_END);
 }
 
+uint32_t polaris10_set_field_to_u32(u32 offset, u32 original_data, u32 field, u32 size)
+{
+	u32 mask = 0;
+	u32 shift = 0;
+
+	shift = (offset % 4) << 3;
+	if (size == sizeof(uint8_t))
+		mask = 0xFF << shift;
+	else if (size == sizeof(uint16_t))
+		mask = 0xFFFF << shift;
+
+	original_data &= ~mask;
+	original_data |= (field << shift);
+	return original_data;
+}
+
+static int polaris10_update_dpm_settings(struct pp_hwmgr *hwmgr,
+				void *profile_setting)
+{
+	struct polaris10_smumgr *smu_data = (struct polaris10_smumgr *)
+			(hwmgr->smu_backend);
+	struct profile_mode_setting *setting;
+	struct SMU74_Discrete_GraphicsLevel *levels =
+			smu_data->smc_state_table.GraphicsLevel;
+	uint32_t array = smu_data->smu7_data.dpm_table_start +
+			offsetof(SMU74_Discrete_DpmTable, GraphicsLevel);
+
+	uint32_t mclk_array = smu_data->smu7_data.dpm_table_start +
+			offsetof(SMU74_Discrete_DpmTable, MemoryLevel);
+	struct SMU74_Discrete_MemoryLevel *mclk_levels =
+			smu_data->smc_state_table.MemoryLevel;
+	uint32_t i;
+	uint32_t offset, up_hyst_offset, down_hyst_offset, clk_activity_offset, tmp;
+
+	if (profile_setting == NULL)
+		return -EINVAL;
+
+	setting = (struct profile_mode_setting *)profile_setting;
+
+	if (setting->bupdate_sclk) {
+		for (i = 0; i < smu_data->smc_state_table.GraphicsDpmLevelCount; i++) {
+			if (levels[i].ActivityLevel !=
+				cpu_to_be16(setting->sclk_activity)) {
+				levels[i].ActivityLevel = cpu_to_be16(setting->sclk_activity);
+
+				clk_activity_offset = array + (sizeof(SMU74_Discrete_GraphicsLevel) * i)
+						+ offsetof(SMU74_Discrete_GraphicsLevel, ActivityLevel);
+				offset = clk_activity_offset & ~0x3;
+				tmp = PP_HOST_TO_SMC_UL(cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC, offset));
+				tmp = polaris10_set_field_to_u32(clk_activity_offset, tmp, levels[i].ActivityLevel, sizeof(uint16_t));
+				cgs_write_ind_register(hwmgr->device, CGS_IND_REG__SMC, offset, PP_HOST_TO_SMC_UL(tmp));
+
+			}
+			if (levels[i].UpHyst != setting->sclk_up_hyst ||
+				levels[i].DownHyst != setting->sclk_down_hyst) {
+				levels[i].UpHyst = setting->sclk_up_hyst;
+				levels[i].DownHyst = setting->sclk_down_hyst;
+				up_hyst_offset = array + (sizeof(SMU74_Discrete_GraphicsLevel) * i)
+						+ offsetof(SMU74_Discrete_GraphicsLevel, UpHyst);
+				down_hyst_offset = array + (sizeof(SMU74_Discrete_GraphicsLevel) * i)
+						+ offsetof(SMU74_Discrete_GraphicsLevel, DownHyst);
+				offset = up_hyst_offset & ~0x3;
+				tmp = PP_HOST_TO_SMC_UL(cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC, offset));
+				tmp = polaris10_set_field_to_u32(up_hyst_offset, tmp, levels[i].UpHyst, sizeof(uint8_t));
+				tmp = polaris10_set_field_to_u32(down_hyst_offset, tmp, levels[i].DownHyst, sizeof(uint8_t));
+				cgs_write_ind_register(hwmgr->device, CGS_IND_REG__SMC, offset, PP_HOST_TO_SMC_UL(tmp));
+			}
+		}
+	}
+
+	if (setting->bupdate_mclk) {
+		for (i = 0; i < smu_data->smc_state_table.MemoryDpmLevelCount; i++) {
+			if (mclk_levels[i].ActivityLevel !=
+				cpu_to_be16(setting->mclk_activity)) {
+				mclk_levels[i].ActivityLevel = cpu_to_be16(setting->mclk_activity);
+
+				clk_activity_offset = mclk_array + (sizeof(SMU74_Discrete_MemoryLevel) * i)
+						+ offsetof(SMU74_Discrete_MemoryLevel, ActivityLevel);
+				offset = clk_activity_offset & ~0x3;
+				tmp = PP_HOST_TO_SMC_UL(cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC, offset));
+				tmp = polaris10_set_field_to_u32(clk_activity_offset, tmp, mclk_levels[i].ActivityLevel, sizeof(uint16_t));
+				cgs_write_ind_register(hwmgr->device, CGS_IND_REG__SMC, offset, PP_HOST_TO_SMC_UL(tmp));
+
+			}
+			if (mclk_levels[i].UpHyst != setting->mclk_up_hyst ||
+				mclk_levels[i].DownHyst != setting->mclk_down_hyst) {
+				mclk_levels[i].UpHyst = setting->mclk_up_hyst;
+				mclk_levels[i].DownHyst = setting->mclk_down_hyst;
+				up_hyst_offset = mclk_array + (sizeof(SMU74_Discrete_MemoryLevel) * i)
+						+ offsetof(SMU74_Discrete_MemoryLevel, UpHyst);
+				down_hyst_offset = mclk_array + (sizeof(SMU74_Discrete_MemoryLevel) * i)
+						+ offsetof(SMU74_Discrete_MemoryLevel, DownHyst);
+				offset = up_hyst_offset & ~0x3;
+				tmp = PP_HOST_TO_SMC_UL(cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC, offset));
+				tmp = polaris10_set_field_to_u32(up_hyst_offset, tmp, mclk_levels[i].UpHyst, sizeof(uint8_t));
+				tmp = polaris10_set_field_to_u32(down_hyst_offset, tmp, mclk_levels[i].DownHyst, sizeof(uint8_t));
+				cgs_write_ind_register(hwmgr->device, CGS_IND_REG__SMC, offset, PP_HOST_TO_SMC_UL(tmp));
+			}
+		}
+	}
+	return 0;
+}
+
 const struct pp_smumgr_func polaris10_smu_funcs = {
 	.smu_init = polaris10_smu_init,
 	.smu_fini = smu7_smu_fini,
@@ -2599,4 +2702,5 @@ static int polaris10_populate_requested_graphic_levels(struct pp_hwmgr *hwmgr,
 	.is_dpm_running = polaris10_is_dpm_running,
 	.populate_requested_graphic_levels = polaris10_populate_requested_graphic_levels,
 	.is_hw_avfs_present = polaris10_is_hw_avfs_present,
+	.update_dpm_settings = polaris10_update_dpm_settings,
 };
-- 
1.9.1

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

[PATCH 3/4] drm/amd/pp: Implement get_power_profile_mode on smu7

[Rex Zhu]

User can get smu7 profile pamameters through sysfs

cat pp_power_profile_mode
NUM        MODE_NAME     SCLK_UP_HYST   SCLK_DOWN_HYST SCLK_ACTIVE_LEVEL     MCLK_UP_HYST   MCLK_DOWN_HYST MCLK_ACTIVE_LEVEL
  0   3D_FULL_SCREEN:        0              100               30                0              100               10
  1     POWER_SAVING:       10                0               30                -                -                -
  2            VIDEO:        -                -                -               10               16               31
  3               VR:        0               11               50                0              100               10
  4          COMPUTE:        0                5               30                -                -                -
  5           CUSTOM:        0                0                0                0                0                0
  6             AUTO:        -                -                -                -                -                -
  *          CURRENT:        0              100               30                0              100               10

v2: add auto mode display
Change-Id: I10e02f9e5fcd8e2b62c0cad620d9635336ea01dd
Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 107 +++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index f6236f9..9f6afd5 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -81,6 +81,21 @@
 #define PCIE_BUS_CLK                10000
 #define TCLK                        (PCIE_BUS_CLK / 10)
 
+static const struct profile_mode_setting smu7_profiling[5] =
+					{{1, 0, 100, 30, 1, 0, 100, 10},
+					 {1, 10, 0, 30, 0, 0, 0, 0},
+					 {0, 0, 0, 0, 1, 10, 16, 31},
+					 {1, 0, 11, 50, 1, 0, 100, 10},
+					 {1, 0, 5, 30, 0, 0, 0, 0},
+					};
+
+static const struct profile_mode_setting polaris11_profiling[5] =
+					{{1, 0, 100, 30, 1, 0, 100, 10},
+					 {1, 10, 0, 30, 0, 0, 0, 0},
+					 {0, 0, 0, 0, 1, 10, 16, 62},
+					 {1, 0, 11, 50, 1, 0, 100, 10},
+					 {1, 0, 5, 30, 0, 0, 0, 0},
+					};
 
 /** Values for the CG_THERMAL_CTRL::DPM_EVENT_SRC field. */
 enum DPM_EVENT_SRC {
@@ -4934,6 +4949,96 @@ static int smu7_odn_edit_dpm_table(struct pp_hwmgr *hwmgr,
 	return 0;
 }
 
+static int smu7_get_power_profile_mode(struct pp_hwmgr *hwmgr, char *buf)
+{
+	struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
+	uint32_t i, size = 0;
+	uint32_t len;
+
+	static const char *profile_name[6] = {"3D_FULL_SCREEN",
+					"POWER_SAVING",
+					"VIDEO",
+					"VR",
+					"COMPUTE",
+					"CUSTOM"};
+
+	static const char *title[8] = {"NUM",
+			"MODE_NAME",
+			"SCLK_UP_HYST",
+			"SCLK_DOWN_HYST",
+			"SCLK_ACTIVE_LEVEL",
+			"MCLK_UP_HYST",
+			"MCLK_DOWN_HYST",
+			"MCLK_ACTIVE_LEVEL"};
+
+	if (!buf)
+		return -EINVAL;
+
+	size += sprintf(buf + size, "%s %16s %16s %16s %16s %16s %16s %16s\n",
+			title[0], title[1], title[2], title[3],
+			title[4], title[5], title[6], title[7]);
+
+	len = sizeof(smu7_profiling) / sizeof(struct profile_mode_setting);
+
+	for (i = 0; i < len; i++) {
+		if (smu7_profiling[i].bupdate_sclk)
+			size += sprintf(buf + size, "%3d %16s: %8d %16d %16d ",
+			i, profile_name[i], smu7_profiling[i].sclk_up_hyst,
+			smu7_profiling[i].sclk_down_hyst,
+			smu7_profiling[i].sclk_activity);
+		else
+			size += sprintf(buf + size, "%3d %16s: %8s %16s %16s ",
+			i, profile_name[i], "-", "-", "-");
+
+		if (smu7_profiling[i].bupdate_mclk)
+			size += sprintf(buf + size, "%16d %16d %16d\n",
+			smu7_profiling[i].mclk_up_hyst,
+			smu7_profiling[i].mclk_down_hyst,
+			smu7_profiling[i].mclk_activity);
+		else
+			size += sprintf(buf + size, "%16s %16s %16s\n",
+			"-", "-", "-");
+	}
+
+	size += sprintf(buf + size, "%3d %16s: %8d %16d %16d %16d %16d %16d\n",
+			i, profile_name[i],
+			data->custom_profile_setting.sclk_up_hyst,
+			data->custom_profile_setting.sclk_down_hyst,
+			data->custom_profile_setting.sclk_activity,
+			data->custom_profile_setting.mclk_up_hyst,
+			data->custom_profile_setting.mclk_down_hyst,
+			data->custom_profile_setting.mclk_activity);
+
+	if (hwmgr->power_profile_mode == PP_SMC_POWER_PROFILE_AUTO)
+		size += sprintf(buf + size, "%3d %16s: %8d %16d %16d %16d %16d %16d\n",
+			i+1, "AUTO",
+			data->current_profile_setting.sclk_up_hyst,
+			data->current_profile_setting.sclk_down_hyst,
+			data->current_profile_setting.sclk_activity,
+			data->current_profile_setting.mclk_up_hyst,
+			data->current_profile_setting.mclk_down_hyst,
+			data->current_profile_setting.mclk_activity);
+	else
+		size += sprintf(buf + size, "%3d %16s: %8s %16s %16s %16s %16s %16s\n",
+		i+1, "AUTO", "-", "-", "-", "-", "-", "-");
+
+	size += sprintf(buf + size, "%3s %16s: %8d %16d %16d %16d %16d %16d\n",
+			"*", "CURRENT",
+			data->current_profile_setting.sclk_up_hyst,
+			data->current_profile_setting.sclk_down_hyst,
+			data->current_profile_setting.sclk_activity,
+			data->current_profile_setting.mclk_up_hyst,
+			data->current_profile_setting.mclk_down_hyst,
+			data->current_profile_setting.mclk_activity);
+
+	return size;
+}
+
+static int smu7_set_power_profile_mode(struct pp_hwmgr *hwmgr, long *input, uint32_t size)
+{
+	/* To Do */
+	return 0;
+}
 
 static const struct pp_hwmgr_func smu7_hwmgr_funcs = {
 	.backend_init = &smu7_hwmgr_backend_init,
@@ -4990,6 +5095,8 @@ static int smu7_odn_edit_dpm_table(struct pp_hwmgr *hwmgr,
 	.get_max_high_clocks = smu7_get_max_high_clocks,
 	.get_thermal_temperature_range = smu7_get_thermal_temperature_range,
 	.odn_edit_dpm_table = smu7_odn_edit_dpm_table,
+	.get_power_profile_mode = smu7_get_power_profile_mode,
+	.set_power_profile_mode = smu7_set_power_profile_mode,
 };
 
 uint8_t smu7_get_sleep_divider_id_from_clock(uint32_t clock,
-- 
1.9.1

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

[PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

[Rex Zhu]

User can set smu7 profile pamameters through sysfs

echo "0/1/2/3/4">pp_power_profile_mode
to select 3D_FULL_SCREEN/POWER_SAVING/VIDEO/VR/COMPUTE
mode.
echo "5 * * * * * * * *">pp_power_profile_mode
to config custom mode.
"5 * * * * * * * *" mean "CUSTOM enable_sclk SCLK_UP_HYST
SCLK_DOWN_HYST SCLK_ACTIVE_LEVEL enable_mclk MCLK_UP_HYST
MCLK_DOWN_HYST MCLK_ACTIVE_LEVEL"

Change-Id: Ic6d6f37363bc81ab17051285f6ace847edf725de
Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 49 +++++++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index 9f6afd5..13db75c 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -5036,7 +5036,54 @@ static int smu7_get_power_profile_mode(struct pp_hwmgr *hwmgr, char *buf)
 
 static int smu7_set_power_profile_mode(struct pp_hwmgr *hwmgr, long *input, uint32_t size)
 {
-	/* To Do */
+	struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
+	struct profile_mode_setting tmp;
+
+	hwmgr->power_profile_mode = input[size];
+
+	switch (hwmgr->power_profile_mode) {
+	case PP_SMC_POWER_PROFILE_CUSTOM:
+		if (size < 8)
+			return -EINVAL;
+
+		data->custom_profile_setting.bupdate_sclk = input[0];
+		data->custom_profile_setting.sclk_up_hyst = input[1];
+		data->custom_profile_setting.sclk_down_hyst = input[2];
+		data->custom_profile_setting.sclk_activity =  input[3];
+		data->custom_profile_setting.bupdate_mclk = input[4];
+		data->custom_profile_setting.mclk_up_hyst = input[5];
+		data->custom_profile_setting.mclk_down_hyst = input[6];
+		data->custom_profile_setting.mclk_activity =  input[7];
+		if (!smum_update_dpm_settings(hwmgr, &data->custom_profile_setting))
+			memcpy(&data->current_profile_setting, &data->custom_profile_setting, sizeof(struct profile_mode_setting));
+		break;
+	case PP_SMC_POWER_PROFILE_FULLSCREEN3D:
+	case PP_SMC_POWER_PROFILE_POWERSAVING:
+	case PP_SMC_POWER_PROFILE_VIDEO:
+	case PP_SMC_POWER_PROFILE_VR:
+	case PP_SMC_POWER_PROFILE_COMPUTE:
+		memcpy(&tmp, &smu7_profiling[hwmgr->power_profile_mode], sizeof(struct profile_mode_setting));
+		if (!smum_update_dpm_settings(hwmgr, &tmp)) {
+			if (tmp.bupdate_sclk) {
+				data->current_profile_setting.bupdate_sclk = tmp.bupdate_sclk;
+				data->current_profile_setting.sclk_up_hyst = tmp.sclk_up_hyst;
+				data->current_profile_setting.sclk_down_hyst = tmp.sclk_down_hyst;
+				data->current_profile_setting.sclk_activity = tmp.sclk_activity;
+			}
+			if (tmp.bupdate_mclk) {
+				data->current_profile_setting.bupdate_mclk = tmp.bupdate_mclk;
+				data->current_profile_setting.mclk_up_hyst = tmp.mclk_up_hyst;
+				data->current_profile_setting.mclk_down_hyst = tmp.mclk_down_hyst;
+				data->current_profile_setting.mclk_activity = tmp.mclk_activity;
+			}
+		}
+		break;
+	case PP_SMC_POWER_PROFILE_AUTO: /* TO DO auto wattman feature not implement */
+		return 0;
+	default:
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
1.9.1

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

Re: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

[Eric Huang]

We have min_sclk and min_mclk in previous power profile parameters for 
VI, which are similar with min_active_level for Vega10. How to implement 
these parameters?

Regards,
Eric

On 2018-01-24 04:37 AM, Rex Zhu wrote:
> User can set smu7 profile pamameters through sysfs
>
> echo "0/1/2/3/4">pp_power_profile_mode
> to select 3D_FULL_SCREEN/POWER_SAVING/VIDEO/VR/COMPUTE
> mode.
> echo "5 * * * * * * * *">pp_power_profile_mode
> to config custom mode.
> "5 * * * * * * * *" mean "CUSTOM enable_sclk SCLK_UP_HYST
> SCLK_DOWN_HYST SCLK_ACTIVE_LEVEL enable_mclk MCLK_UP_HYST
> MCLK_DOWN_HYST MCLK_ACTIVE_LEVEL"
>
> Change-Id: Ic6d6f37363bc81ab17051285f6ace847edf725de
> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
> ---
>   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 49 +++++++++++++++++++++++-
>   1 file changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> index 9f6afd5..13db75c 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> @@ -5036,7 +5036,54 @@ static int smu7_get_power_profile_mode(struct pp_hwmgr *hwmgr, char *buf)
>   
>   static int smu7_set_power_profile_mode(struct pp_hwmgr *hwmgr, long *input, uint32_t size)
>   {
> -	/* To Do */
> +	struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
> +	struct profile_mode_setting tmp;
> +
> +	hwmgr->power_profile_mode = input[size];
> +
> +	switch (hwmgr->power_profile_mode) {
> +	case PP_SMC_POWER_PROFILE_CUSTOM:
> +		if (size < 8)
> +			return -EINVAL;
> +
> +		data->custom_profile_setting.bupdate_sclk = input[0];
> +		data->custom_profile_setting.sclk_up_hyst = input[1];
> +		data->custom_profile_setting.sclk_down_hyst = input[2];
> +		data->custom_profile_setting.sclk_activity =  input[3];
> +		data->custom_profile_setting.bupdate_mclk = input[4];
> +		data->custom_profile_setting.mclk_up_hyst = input[5];
> +		data->custom_profile_setting.mclk_down_hyst = input[6];
> +		data->custom_profile_setting.mclk_activity =  input[7];
> +		if (!smum_update_dpm_settings(hwmgr, &data->custom_profile_setting))
> +			memcpy(&data->current_profile_setting, &data->custom_profile_setting, sizeof(struct profile_mode_setting));
> +		break;
> +	case PP_SMC_POWER_PROFILE_FULLSCREEN3D:
> +	case PP_SMC_POWER_PROFILE_POWERSAVING:
> +	case PP_SMC_POWER_PROFILE_VIDEO:
> +	case PP_SMC_POWER_PROFILE_VR:
> +	case PP_SMC_POWER_PROFILE_COMPUTE:
> +		memcpy(&tmp, &smu7_profiling[hwmgr->power_profile_mode], sizeof(struct profile_mode_setting));
> +		if (!smum_update_dpm_settings(hwmgr, &tmp)) {
> +			if (tmp.bupdate_sclk) {
> +				data->current_profile_setting.bupdate_sclk = tmp.bupdate_sclk;
> +				data->current_profile_setting.sclk_up_hyst = tmp.sclk_up_hyst;
> +				data->current_profile_setting.sclk_down_hyst = tmp.sclk_down_hyst;
> +				data->current_profile_setting.sclk_activity = tmp.sclk_activity;
> +			}
> +			if (tmp.bupdate_mclk) {
> +				data->current_profile_setting.bupdate_mclk = tmp.bupdate_mclk;
> +				data->current_profile_setting.mclk_up_hyst = tmp.mclk_up_hyst;
> +				data->current_profile_setting.mclk_down_hyst = tmp.mclk_down_hyst;
> +				data->current_profile_setting.mclk_activity = tmp.mclk_activity;
> +			}
> +		}
> +		break;
> +	case PP_SMC_POWER_PROFILE_AUTO: /* TO DO auto wattman feature not implement */
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
> +
>   	return 0;
>   }
>   

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

Re: [PATCH 2/4] drm/amd/pp: Implement update_dpm_settings on Polaris

[Eric Huang]

On 2018-01-24 04:37 AM, Rex Zhu wrote:
> Change-Id: I4533826ef6e18df125ae4445016873be3b5fe0ce
> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
> ---
>   .../drm/amd/powerplay/smumgr/polaris10_smumgr.c    | 104 +++++++++++++++++++++
>   1 file changed, 104 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c b/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c
> index bfb2c85..559572d 100644
> --- a/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c
> @@ -2575,6 +2575,109 @@ static int polaris10_populate_requested_graphic_levels(struct pp_hwmgr *hwmgr,
>   				array_size, SMC_RAM_END);
>   }
>   
> +uint32_t polaris10_set_field_to_u32(u32 offset, u32 original_data, u32 field, u32 size)
> +{
> +	u32 mask = 0;
> +	u32 shift = 0;
> +
> +	shift = (offset % 4) << 3;
> +	if (size == sizeof(uint8_t))
> +		mask = 0xFF << shift;
> +	else if (size == sizeof(uint16_t))
> +		mask = 0xFFFF << shift;
> +
> +	original_data &= ~mask;
> +	original_data |= (field << shift);
> +	return original_data;
> +}
> +
> +static int polaris10_update_dpm_settings(struct pp_hwmgr *hwmgr,
> +				void *profile_setting)
> +{
> +	struct polaris10_smumgr *smu_data = (struct polaris10_smumgr *)
> +			(hwmgr->smu_backend);
> +	struct profile_mode_setting *setting;
> +	struct SMU74_Discrete_GraphicsLevel *levels =
> +			smu_data->smc_state_table.GraphicsLevel;
> +	uint32_t array = smu_data->smu7_data.dpm_table_start +
> +			offsetof(SMU74_Discrete_DpmTable, GraphicsLevel);
> +
> +	uint32_t mclk_array = smu_data->smu7_data.dpm_table_start +
> +			offsetof(SMU74_Discrete_DpmTable, MemoryLevel);
> +	struct SMU74_Discrete_MemoryLevel *mclk_levels =
> +			smu_data->smc_state_table.MemoryLevel;
> +	uint32_t i;
> +	uint32_t offset, up_hyst_offset, down_hyst_offset, clk_activity_offset, tmp;
> +
> +	if (profile_setting == NULL)
> +		return -EINVAL;
> +
> +	setting = (struct profile_mode_setting *)profile_setting;
> +
> +	if (setting->bupdate_sclk) {
> +		for (i = 0; i < smu_data->smc_state_table.GraphicsDpmLevelCount; i++) {
> +			if (levels[i].ActivityLevel !=
> +				cpu_to_be16(setting->sclk_activity)) {
> +				levels[i].ActivityLevel = cpu_to_be16(setting->sclk_activity);
> +
> +				clk_activity_offset = array + (sizeof(SMU74_Discrete_GraphicsLevel) * i)
> +						+ offsetof(SMU74_Discrete_GraphicsLevel, ActivityLevel);
> +				offset = clk_activity_offset & ~0x3;
> +				tmp = PP_HOST_TO_SMC_UL(cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC, offset));
> +				tmp = polaris10_set_field_to_u32(clk_activity_offset, tmp, levels[i].ActivityLevel, sizeof(uint16_t));
> +				cgs_write_ind_register(hwmgr->device, CGS_IND_REG__SMC, offset, PP_HOST_TO_SMC_UL(tmp));
> +
> +			}
> +			if (levels[i].UpHyst != setting->sclk_up_hyst ||
> +				levels[i].DownHyst != setting->sclk_down_hyst) {
> +				levels[i].UpHyst = setting->sclk_up_hyst;
> +				levels[i].DownHyst = setting->sclk_down_hyst;
> +				up_hyst_offset = array + (sizeof(SMU74_Discrete_GraphicsLevel) * i)
> +						+ offsetof(SMU74_Discrete_GraphicsLevel, UpHyst);
> +				down_hyst_offset = array + (sizeof(SMU74_Discrete_GraphicsLevel) * i)
> +						+ offsetof(SMU74_Discrete_GraphicsLevel, DownHyst);
> +				offset = up_hyst_offset & ~0x3;
> +				tmp = PP_HOST_TO_SMC_UL(cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC, offset));
> +				tmp = polaris10_set_field_to_u32(up_hyst_offset, tmp, levels[i].UpHyst, sizeof(uint8_t));
> +				tmp = polaris10_set_field_to_u32(down_hyst_offset, tmp, levels[i].DownHyst, sizeof(uint8_t));
> +				cgs_write_ind_register(hwmgr->device, CGS_IND_REG__SMC, offset, PP_HOST_TO_SMC_UL(tmp));
> +			}
> +		}
> +	}
> +
> +	if (setting->bupdate_mclk) {
> +		for (i = 0; i < smu_data->smc_state_table.MemoryDpmLevelCount; i++) {
> +			if (mclk_levels[i].ActivityLevel !=
> +				cpu_to_be16(setting->mclk_activity)) {
> +				mclk_levels[i].ActivityLevel = cpu_to_be16(setting->mclk_activity);
> +
> +				clk_activity_offset = mclk_array + (sizeof(SMU74_Discrete_MemoryLevel) * i)
> +						+ offsetof(SMU74_Discrete_MemoryLevel, ActivityLevel);
> +				offset = clk_activity_offset & ~0x3;
> +				tmp = PP_HOST_TO_SMC_UL(cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC, offset));
> +				tmp = polaris10_set_field_to_u32(clk_activity_offset, tmp, mclk_levels[i].ActivityLevel, sizeof(uint16_t));
> +				cgs_write_ind_register(hwmgr->device, CGS_IND_REG__SMC, offset, PP_HOST_TO_SMC_UL(tmp));
> +
> +			}
> +			if (mclk_levels[i].UpHyst != setting->mclk_up_hyst ||
> +				mclk_levels[i].DownHyst != setting->mclk_down_hyst) {
> +				mclk_levels[i].UpHyst = setting->mclk_up_hyst;
> +				mclk_levels[i].DownHyst = setting->mclk_down_hyst;
> +				up_hyst_offset = mclk_array + (sizeof(SMU74_Discrete_MemoryLevel) * i)
> +						+ offsetof(SMU74_Discrete_MemoryLevel, UpHyst);
> +				down_hyst_offset = mclk_array + (sizeof(SMU74_Discrete_MemoryLevel) * i)
> +						+ offsetof(SMU74_Discrete_MemoryLevel, DownHyst);
> +				offset = up_hyst_offset & ~0x3;
> +				tmp = PP_HOST_TO_SMC_UL(cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC, offset));
> +				tmp = polaris10_set_field_to_u32(up_hyst_offset, tmp, mclk_levels[i].UpHyst, sizeof(uint8_t));
> +				tmp = polaris10_set_field_to_u32(down_hyst_offset, tmp, mclk_levels[i].DownHyst, sizeof(uint8_t));
> +				cgs_write_ind_register(hwmgr->device, CGS_IND_REG__SMC, offset, PP_HOST_TO_SMC_UL(tmp));
We have to call smu7_freeze/unfreeze_sclk_mclk_dpm functions 
before/after updating smc tables.

Regards,
Eric
> +			}
> +		}
> +	}
> +	return 0;
> +}
> +
>   const struct pp_smumgr_func polaris10_smu_funcs = {
>   	.smu_init = polaris10_smu_init,
>   	.smu_fini = smu7_smu_fini,
> @@ -2599,4 +2702,5 @@ static int polaris10_populate_requested_graphic_levels(struct pp_hwmgr *hwmgr,
>   	.is_dpm_running = polaris10_is_dpm_running,
>   	.populate_requested_graphic_levels = polaris10_populate_requested_graphic_levels,
>   	.is_hw_avfs_present = polaris10_is_hw_avfs_present,
> +	.update_dpm_settings = polaris10_update_dpm_settings,
>   };

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

Re: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

[Zhu, Rex]

[-- Attachment #1.1: Type: text/plain, Size: 4787 bytes --]

Hi Eric,

We have sysfs pp-dpm-sclk/mclk to set min dpm level

Best Regards
Rex
________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Eric Huang <jinhuieric.huang-5C7GfCeVMHo@public.gmane.org>
Sent: Thursday, January 25, 2018 12:04:55 AM
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

We have min_sclk and min_mclk in previous power profile parameters for
VI, which are similar with min_active_level for Vega10. How to implement
these parameters?

Regards,
Eric

On 2018-01-24 04:37 AM, Rex Zhu wrote:
> User can set smu7 profile pamameters through sysfs
>
> echo "0/1/2/3/4">pp_power_profile_mode
> to select 3D_FULL_SCREEN/POWER_SAVING/VIDEO/VR/COMPUTE
> mode.
> echo "5 * * * * * * * *">pp_power_profile_mode
> to config custom mode.
> "5 * * * * * * * *" mean "CUSTOM enable_sclk SCLK_UP_HYST
> SCLK_DOWN_HYST SCLK_ACTIVE_LEVEL enable_mclk MCLK_UP_HYST
> MCLK_DOWN_HYST MCLK_ACTIVE_LEVEL"
>
> Change-Id: Ic6d6f37363bc81ab17051285f6ace847edf725de
> Signed-off-by: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
> ---
>   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 49 +++++++++++++++++++++++-
>   1 file changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> index 9f6afd5..13db75c 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> @@ -5036,7 +5036,54 @@ static int smu7_get_power_profile_mode(struct pp_hwmgr *hwmgr, char *buf)
>
>   static int smu7_set_power_profile_mode(struct pp_hwmgr *hwmgr, long *input, uint32_t size)
>   {
> -     /* To Do */
> +     struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
> +     struct profile_mode_setting tmp;
> +
> +     hwmgr->power_profile_mode = input[size];
> +
> +     switch (hwmgr->power_profile_mode) {
> +     case PP_SMC_POWER_PROFILE_CUSTOM:
> +             if (size < 8)
> +                     return -EINVAL;
> +
> +             data->custom_profile_setting.bupdate_sclk = input[0];
> +             data->custom_profile_setting.sclk_up_hyst = input[1];
> +             data->custom_profile_setting.sclk_down_hyst = input[2];
> +             data->custom_profile_setting.sclk_activity =  input[3];
> +             data->custom_profile_setting.bupdate_mclk = input[4];
> +             data->custom_profile_setting.mclk_up_hyst = input[5];
> +             data->custom_profile_setting.mclk_down_hyst = input[6];
> +             data->custom_profile_setting.mclk_activity =  input[7];
> +             if (!smum_update_dpm_settings(hwmgr, &data->custom_profile_setting))
> +                     memcpy(&data->current_profile_setting, &data->custom_profile_setting, sizeof(struct profile_mode_setting));
> +             break;
> +     case PP_SMC_POWER_PROFILE_FULLSCREEN3D:
> +     case PP_SMC_POWER_PROFILE_POWERSAVING:
> +     case PP_SMC_POWER_PROFILE_VIDEO:
> +     case PP_SMC_POWER_PROFILE_VR:
> +     case PP_SMC_POWER_PROFILE_COMPUTE:
> +             memcpy(&tmp, &smu7_profiling[hwmgr->power_profile_mode], sizeof(struct profile_mode_setting));
> +             if (!smum_update_dpm_settings(hwmgr, &tmp)) {
> +                     if (tmp.bupdate_sclk) {
> +                             data->current_profile_setting.bupdate_sclk = tmp.bupdate_sclk;
> +                             data->current_profile_setting.sclk_up_hyst = tmp.sclk_up_hyst;
> +                             data->current_profile_setting.sclk_down_hyst = tmp.sclk_down_hyst;
> +                             data->current_profile_setting.sclk_activity = tmp.sclk_activity;
> +                     }
> +                     if (tmp.bupdate_mclk) {
> +                             data->current_profile_setting.bupdate_mclk = tmp.bupdate_mclk;
> +                             data->current_profile_setting.mclk_up_hyst = tmp.mclk_up_hyst;
> +                             data->current_profile_setting.mclk_down_hyst = tmp.mclk_down_hyst;
> +                             data->current_profile_setting.mclk_activity = tmp.mclk_activity;
> +                     }
> +             }
> +             break;
> +     case PP_SMC_POWER_PROFILE_AUTO: /* TO DO auto wattman feature not implement */
> +             return 0;
> +     default:
> +             return -EINVAL;
> +     }
> +
>        return 0;
>   }
>

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

[-- Attachment #1.2: Type: text/html, Size: 9527 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

[Eric Huang]

[-- Attachment #1.1: Type: text/plain, Size: 5095 bytes --]

Hi Rex,

So you intend to separate previous one integrated profile parameters 
into two sets implemented by two/three sysfs entries.

Regards,
Eric

On 2018-01-24 03:13 PM, Zhu, Rex wrote:
> Hi Eric,
>
> We have sysfs pp-dpm-sclk/mclk to set min dpm level
>
> Best Regards
> Rex
> ------------------------------------------------------------------------
> *From:* amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of 
> Eric Huang <jinhuieric.huang-5C7GfCeVMHo@public.gmane.org>
> *Sent:* Thursday, January 25, 2018 12:04:55 AM
> *To:* amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement 
> set_power_profile_mode on smu7
> We have min_sclk and min_mclk in previous power profile parameters for
> VI, which are similar with min_active_level for Vega10. How to implement
> these parameters?
>
> Regards,
> Eric
>
> On 2018-01-24 04:37 AM, Rex Zhu wrote:
> > User can set smu7 profile pamameters through sysfs
> >
> > echo "0/1/2/3/4">pp_power_profile_mode
> > to select 3D_FULL_SCREEN/POWER_SAVING/VIDEO/VR/COMPUTE
> > mode.
> > echo "5 * * * * * * * *">pp_power_profile_mode
> > to config custom mode.
> > "5 * * * * * * * *" mean "CUSTOM enable_sclk SCLK_UP_HYST
> > SCLK_DOWN_HYST SCLK_ACTIVE_LEVEL enable_mclk MCLK_UP_HYST
> > MCLK_DOWN_HYST MCLK_ACTIVE_LEVEL"
> >
> > Change-Id: Ic6d6f37363bc81ab17051285f6ace847edf725de
> > Signed-off-by: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
> > ---
> >   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 49 
> +++++++++++++++++++++++-
> >   1 file changed, 48 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > index 9f6afd5..13db75c 100644
> > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > @@ -5036,7 +5036,54 @@ static int smu7_get_power_profile_mode(struct 
> pp_hwmgr *hwmgr, char *buf)
> >
> >   static int smu7_set_power_profile_mode(struct pp_hwmgr *hwmgr, 
> long *input, uint32_t size)
> >   {
> > -     /* To Do */
> > +     struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
> > +     struct profile_mode_setting tmp;
> > +
> > +     hwmgr->power_profile_mode = input[size];
> > +
> > +     switch (hwmgr->power_profile_mode) {
> > +     case PP_SMC_POWER_PROFILE_CUSTOM:
> > +             if (size < 8)
> > +                     return -EINVAL;
> > +
> > + data->custom_profile_setting.bupdate_sclk = input[0];
> > + data->custom_profile_setting.sclk_up_hyst = input[1];
> > + data->custom_profile_setting.sclk_down_hyst = input[2];
> > + data->custom_profile_setting.sclk_activity =  input[3];
> > + data->custom_profile_setting.bupdate_mclk = input[4];
> > + data->custom_profile_setting.mclk_up_hyst = input[5];
> > + data->custom_profile_setting.mclk_down_hyst = input[6];
> > + data->custom_profile_setting.mclk_activity =  input[7];
> > +             if (!smum_update_dpm_settings(hwmgr, 
> &data->custom_profile_setting))
> > + memcpy(&data->current_profile_setting, 
> &data->custom_profile_setting, sizeof(struct profile_mode_setting));
> > +             break;
> > +     case PP_SMC_POWER_PROFILE_FULLSCREEN3D:
> > +     case PP_SMC_POWER_PROFILE_POWERSAVING:
> > +     case PP_SMC_POWER_PROFILE_VIDEO:
> > +     case PP_SMC_POWER_PROFILE_VR:
> > +     case PP_SMC_POWER_PROFILE_COMPUTE:
> > +             memcpy(&tmp, 
> &smu7_profiling[hwmgr->power_profile_mode], sizeof(struct 
> profile_mode_setting));
> > +             if (!smum_update_dpm_settings(hwmgr, &tmp)) {
> > +                     if (tmp.bupdate_sclk) {
> > + data->current_profile_setting.bupdate_sclk = tmp.bupdate_sclk;
> > + data->current_profile_setting.sclk_up_hyst = tmp.sclk_up_hyst;
> > + data->current_profile_setting.sclk_down_hyst = tmp.sclk_down_hyst;
> > + data->current_profile_setting.sclk_activity = tmp.sclk_activity;
> > +                     }
> > +                     if (tmp.bupdate_mclk) {
> > + data->current_profile_setting.bupdate_mclk = tmp.bupdate_mclk;
> > + data->current_profile_setting.mclk_up_hyst = tmp.mclk_up_hyst;
> > + data->current_profile_setting.mclk_down_hyst = tmp.mclk_down_hyst;
> > + data->current_profile_setting.mclk_activity = tmp.mclk_activity;
> > +                     }
> > +             }
> > +             break;
> > +     case PP_SMC_POWER_PROFILE_AUTO: /* TO DO auto wattman feature 
> not implement */
> > +             return 0;
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +
> >        return 0;
> >   }
> >
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[-- Attachment #1.2: Type: text/html, Size: 9815 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

[Zhu, Rex]

[-- Attachment #1.1: Type: text/plain, Size: 5549 bytes --]

Yes.

Best Regards
Rex
________________________________
From: Huang, JinHuiEric
Sent: Thursday, January 25, 2018 4:47:23 AM
To: Zhu, Rex; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7


Hi Rex,

So you intend to separate previous one integrated profile parameters into two sets implemented by two/three sysfs entries.

Regards,
Eric

On 2018-01-24 03:13 PM, Zhu, Rex wrote:
Hi Eric,

We have sysfs pp-dpm-sclk/mclk to set min dpm level

Best Regards
Rex
________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org><mailto:amd-gfx-bounces@lists.freedesktop.org> on behalf of Eric Huang <jinhuieric.huang-5C7GfCeVMHo@public.gmane.org><mailto:jinhuieric.huang-5C7GfCeVMHo@public.gmane.org>
Sent: Thursday, January 25, 2018 12:04:55 AM
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Subject: Re: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

We have min_sclk and min_mclk in previous power profile parameters for
VI, which are similar with min_active_level for Vega10. How to implement
these parameters?

Regards,
Eric

On 2018-01-24 04:37 AM, Rex Zhu wrote:
> User can set smu7 profile pamameters through sysfs
>
> echo "0/1/2/3/4">pp_power_profile_mode
> to select 3D_FULL_SCREEN/POWER_SAVING/VIDEO/VR/COMPUTE
> mode.
> echo "5 * * * * * * * *">pp_power_profile_mode
> to config custom mode.
> "5 * * * * * * * *" mean "CUSTOM enable_sclk SCLK_UP_HYST
> SCLK_DOWN_HYST SCLK_ACTIVE_LEVEL enable_mclk MCLK_UP_HYST
> MCLK_DOWN_HYST MCLK_ACTIVE_LEVEL"
>
> Change-Id: Ic6d6f37363bc81ab17051285f6ace847edf725de
> Signed-off-by: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org><mailto:Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
> ---
>   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 49 +++++++++++++++++++++++-
>   1 file changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> index 9f6afd5..13db75c 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> @@ -5036,7 +5036,54 @@ static int smu7_get_power_profile_mode(struct pp_hwmgr *hwmgr, char *buf)
>
>   static int smu7_set_power_profile_mode(struct pp_hwmgr *hwmgr, long *input, uint32_t size)
>   {
> -     /* To Do */
> +     struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
> +     struct profile_mode_setting tmp;
> +
> +     hwmgr->power_profile_mode = input[size];
> +
> +     switch (hwmgr->power_profile_mode) {
> +     case PP_SMC_POWER_PROFILE_CUSTOM:
> +             if (size < 8)
> +                     return -EINVAL;
> +
> +             data->custom_profile_setting.bupdate_sclk = input[0];
> +             data->custom_profile_setting.sclk_up_hyst = input[1];
> +             data->custom_profile_setting.sclk_down_hyst = input[2];
> +             data->custom_profile_setting.sclk_activity =  input[3];
> +             data->custom_profile_setting.bupdate_mclk = input[4];
> +             data->custom_profile_setting.mclk_up_hyst = input[5];
> +             data->custom_profile_setting.mclk_down_hyst = input[6];
> +             data->custom_profile_setting.mclk_activity =  input[7];
> +             if (!smum_update_dpm_settings(hwmgr, &data->custom_profile_setting))
> +                     memcpy(&data->current_profile_setting, &data->custom_profile_setting, sizeof(struct profile_mode_setting));
> +             break;
> +     case PP_SMC_POWER_PROFILE_FULLSCREEN3D:
> +     case PP_SMC_POWER_PROFILE_POWERSAVING:
> +     case PP_SMC_POWER_PROFILE_VIDEO:
> +     case PP_SMC_POWER_PROFILE_VR:
> +     case PP_SMC_POWER_PROFILE_COMPUTE:
> +             memcpy(&tmp, &smu7_profiling[hwmgr->power_profile_mode], sizeof(struct profile_mode_setting));
> +             if (!smum_update_dpm_settings(hwmgr, &tmp)) {
> +                     if (tmp.bupdate_sclk) {
> +                             data->current_profile_setting.bupdate_sclk = tmp.bupdate_sclk;
> +                             data->current_profile_setting.sclk_up_hyst = tmp.sclk_up_hyst;
> +                             data->current_profile_setting.sclk_down_hyst = tmp.sclk_down_hyst;
> +                             data->current_profile_setting.sclk_activity = tmp.sclk_activity;
> +                     }
> +                     if (tmp.bupdate_mclk) {
> +                             data->current_profile_setting.bupdate_mclk = tmp.bupdate_mclk;
> +                             data->current_profile_setting.mclk_up_hyst = tmp.mclk_up_hyst;
> +                             data->current_profile_setting.mclk_down_hyst = tmp.mclk_down_hyst;
> +                             data->current_profile_setting.mclk_activity = tmp.mclk_activity;
> +                     }
> +             }
> +             break;
> +     case PP_SMC_POWER_PROFILE_AUTO: /* TO DO auto wattman feature not implement */
> +             return 0;
> +     default:
> +             return -EINVAL;
> +     }
> +
>        return 0;
>   }
>

_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[-- Attachment #1.2: Type: text/html, Size: 10882 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

[Felix Kuehling]

Hi Rex,

As I understand it (the way power profiles currently work),
pp_dpm_sclk/mclk only apply if pp_dpm_force_performance_level is set to
"manual". Power profiles and automatic switching between profiles only
happens when pp_dpm_force_performance_level is set to "auto".

This means pp_dpm_sclk/mclk don't apply when profiles are in effect.
Also, there would be no way to set different minimum clocks for
different profiles.

I think minimum clocks should be part of the profiles.

Regards,
  Felix


On 2018-01-24 03:13 PM, Zhu, Rex wrote:
> Hi Eric,
>
> We have sysfs pp-dpm-sclk/mclk to set min dpm level
>
> Best Regards
> Rex
> ------------------------------------------------------------------------
> *From:* amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of
> Eric Huang <jinhuieric.huang@amd.com>
> *Sent:* Thursday, January 25, 2018 12:04:55 AM
> *To:* amd-gfx@lists.freedesktop.org
> *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement
> set_power_profile_mode on smu7
>  
> We have min_sclk and min_mclk in previous power profile parameters for
> VI, which are similar with min_active_level for Vega10. How to implement
> these parameters?
>
> Regards,
> Eric
>
> On 2018-01-24 04:37 AM, Rex Zhu wrote:
> > User can set smu7 profile pamameters through sysfs
> >
> > echo "0/1/2/3/4">pp_power_profile_mode
> > to select 3D_FULL_SCREEN/POWER_SAVING/VIDEO/VR/COMPUTE
> > mode.
> > echo "5 * * * * * * * *">pp_power_profile_mode
> > to config custom mode.
> > "5 * * * * * * * *" mean "CUSTOM enable_sclk SCLK_UP_HYST
> > SCLK_DOWN_HYST SCLK_ACTIVE_LEVEL enable_mclk MCLK_UP_HYST
> > MCLK_DOWN_HYST MCLK_ACTIVE_LEVEL"
> >
> > Change-Id: Ic6d6f37363bc81ab17051285f6ace847edf725de
> > Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
> > ---
> >   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 49
> +++++++++++++++++++++++-
> >   1 file changed, 48 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > index 9f6afd5..13db75c 100644
> > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > @@ -5036,7 +5036,54 @@ static int smu7_get_power_profile_mode(struct
> pp_hwmgr *hwmgr, char *buf)
> >  
> >   static int smu7_set_power_profile_mode(struct pp_hwmgr *hwmgr,
> long *input, uint32_t size)
> >   {
> > -     /* To Do */
> > +     struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
> > +     struct profile_mode_setting tmp;
> > +
> > +     hwmgr->power_profile_mode = input[size];
> > +
> > +     switch (hwmgr->power_profile_mode) {
> > +     case PP_SMC_POWER_PROFILE_CUSTOM:
> > +             if (size < 8)
> > +                     return -EINVAL;
> > +
> > +             data->custom_profile_setting.bupdate_sclk = input[0];
> > +             data->custom_profile_setting.sclk_up_hyst = input[1];
> > +             data->custom_profile_setting.sclk_down_hyst = input[2];
> > +             data->custom_profile_setting.sclk_activity =  input[3];
> > +             data->custom_profile_setting.bupdate_mclk = input[4];
> > +             data->custom_profile_setting.mclk_up_hyst = input[5];
> > +             data->custom_profile_setting.mclk_down_hyst = input[6];
> > +             data->custom_profile_setting.mclk_activity =  input[7];
> > +             if (!smum_update_dpm_settings(hwmgr,
> &data->custom_profile_setting))
> > +                     memcpy(&data->current_profile_setting,
> &data->custom_profile_setting, sizeof(struct profile_mode_setting));
> > +             break;
> > +     case PP_SMC_POWER_PROFILE_FULLSCREEN3D:
> > +     case PP_SMC_POWER_PROFILE_POWERSAVING:
> > +     case PP_SMC_POWER_PROFILE_VIDEO:
> > +     case PP_SMC_POWER_PROFILE_VR:
> > +     case PP_SMC_POWER_PROFILE_COMPUTE:
> > +             memcpy(&tmp,
> &smu7_profiling[hwmgr->power_profile_mode], sizeof(struct
> profile_mode_setting));
> > +             if (!smum_update_dpm_settings(hwmgr, &tmp)) {
> > +                     if (tmp.bupdate_sclk) {
> > +                            
> data->current_profile_setting.bupdate_sclk = tmp.bupdate_sclk;
> > +                            
> data->current_profile_setting.sclk_up_hyst = tmp.sclk_up_hyst;
> > +                            
> data->current_profile_setting.sclk_down_hyst = tmp.sclk_down_hyst;
> > +                            
> data->current_profile_setting.sclk_activity = tmp.sclk_activity;
> > +                     }
> > +                     if (tmp.bupdate_mclk) {
> > +                            
> data->current_profile_setting.bupdate_mclk = tmp.bupdate_mclk;
> > +                            
> data->current_profile_setting.mclk_up_hyst = tmp.mclk_up_hyst;
> > +                            
> data->current_profile_setting.mclk_down_hyst = tmp.mclk_down_hyst;
> > +                            
> data->current_profile_setting.mclk_activity = tmp.mclk_activity;
> > +                     }
> > +             }
> > +             break;
> > +     case PP_SMC_POWER_PROFILE_AUTO: /* TO DO auto wattman feature
> not implement */
> > +             return 0;
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +
> >        return 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

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

Re: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

[Zhu, Rex]

[-- Attachment #1.1: Type: text/plain, Size: 6588 bytes --]

Hi Felix,

The logic of gfx/computer profile setting works in such way as you said,need under “auto” state.
But in new sysfs of power profile setting, we do not check the performance
Level.

And The “manual” state is a confusing flag，when user set manual state，and then change the clk range through pp-dpm-sclk/mclk，the dpm still works automatically in new clock range, I think we can remove this flag.

So  My idea is
in the sysfs of pp dpm sclk/mclk, user can set clock range.
In the sysfs of power profile state, user can configure the parameters smu/driver exposed. They are independent.






Best Regards
Rex
________________________________
From: Kuehling, Felix
Sent: Thursday, January 25, 2018 5:41:43 AM
To: Zhu, Rex; Huang, JinHuiEric; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

Hi Rex,

As I understand it (the way power profiles currently work),
pp_dpm_sclk/mclk only apply if pp_dpm_force_performance_level is set to
"manual". Power profiles and automatic switching between profiles only
happens when pp_dpm_force_performance_level is set to "auto".

This means pp_dpm_sclk/mclk don't apply when profiles are in effect.
Also, there would be no way to set different minimum clocks for
different profiles.

I think minimum clocks should be part of the profiles.

Regards,
  Felix


On 2018-01-24 03:13 PM, Zhu, Rex wrote:
> Hi Eric,
>
> We have sysfs pp-dpm-sclk/mclk to set min dpm level
>
> Best Regards
> Rex
> ------------------------------------------------------------------------
> *From:* amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of
> Eric Huang <jinhuieric.huang-5C7GfCeVMHo@public.gmane.org>
> *Sent:* Thursday, January 25, 2018 12:04:55 AM
> *To:* amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement
> set_power_profile_mode on smu7
>
> We have min_sclk and min_mclk in previous power profile parameters for
> VI, which are similar with min_active_level for Vega10. How to implement
> these parameters?
>
> Regards,
> Eric
>
> On 2018-01-24 04:37 AM, Rex Zhu wrote:
> > User can set smu7 profile pamameters through sysfs
> >
> > echo "0/1/2/3/4">pp_power_profile_mode
> > to select 3D_FULL_SCREEN/POWER_SAVING/VIDEO/VR/COMPUTE
> > mode.
> > echo "5 * * * * * * * *">pp_power_profile_mode
> > to config custom mode.
> > "5 * * * * * * * *" mean "CUSTOM enable_sclk SCLK_UP_HYST
> > SCLK_DOWN_HYST SCLK_ACTIVE_LEVEL enable_mclk MCLK_UP_HYST
> > MCLK_DOWN_HYST MCLK_ACTIVE_LEVEL"
> >
> > Change-Id: Ic6d6f37363bc81ab17051285f6ace847edf725de
> > Signed-off-by: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
> > ---
> >   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 49
> +++++++++++++++++++++++-
> >   1 file changed, 48 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > index 9f6afd5..13db75c 100644
> > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > @@ -5036,7 +5036,54 @@ static int smu7_get_power_profile_mode(struct
> pp_hwmgr *hwmgr, char *buf)
> >
> >   static int smu7_set_power_profile_mode(struct pp_hwmgr *hwmgr,
> long *input, uint32_t size)
> >   {
> > -     /* To Do */
> > +     struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
> > +     struct profile_mode_setting tmp;
> > +
> > +     hwmgr->power_profile_mode = input[size];
> > +
> > +     switch (hwmgr->power_profile_mode) {
> > +     case PP_SMC_POWER_PROFILE_CUSTOM:
> > +             if (size < 8)
> > +                     return -EINVAL;
> > +
> > +             data->custom_profile_setting.bupdate_sclk = input[0];
> > +             data->custom_profile_setting.sclk_up_hyst = input[1];
> > +             data->custom_profile_setting.sclk_down_hyst = input[2];
> > +             data->custom_profile_setting.sclk_activity =  input[3];
> > +             data->custom_profile_setting.bupdate_mclk = input[4];
> > +             data->custom_profile_setting.mclk_up_hyst = input[5];
> > +             data->custom_profile_setting.mclk_down_hyst = input[6];
> > +             data->custom_profile_setting.mclk_activity =  input[7];
> > +             if (!smum_update_dpm_settings(hwmgr,
> &data->custom_profile_setting))
> > +                     memcpy(&data->current_profile_setting,
> &data->custom_profile_setting, sizeof(struct profile_mode_setting));
> > +             break;
> > +     case PP_SMC_POWER_PROFILE_FULLSCREEN3D:
> > +     case PP_SMC_POWER_PROFILE_POWERSAVING:
> > +     case PP_SMC_POWER_PROFILE_VIDEO:
> > +     case PP_SMC_POWER_PROFILE_VR:
> > +     case PP_SMC_POWER_PROFILE_COMPUTE:
> > +             memcpy(&tmp,
> &smu7_profiling[hwmgr->power_profile_mode], sizeof(struct
> profile_mode_setting));
> > +             if (!smum_update_dpm_settings(hwmgr, &tmp)) {
> > +                     if (tmp.bupdate_sclk) {
> > +
> data->current_profile_setting.bupdate_sclk = tmp.bupdate_sclk;
> > +
> data->current_profile_setting.sclk_up_hyst = tmp.sclk_up_hyst;
> > +
> data->current_profile_setting.sclk_down_hyst = tmp.sclk_down_hyst;
> > +
> data->current_profile_setting.sclk_activity = tmp.sclk_activity;
> > +                     }
> > +                     if (tmp.bupdate_mclk) {
> > +
> data->current_profile_setting.bupdate_mclk = tmp.bupdate_mclk;
> > +
> data->current_profile_setting.mclk_up_hyst = tmp.mclk_up_hyst;
> > +
> data->current_profile_setting.mclk_down_hyst = tmp.mclk_down_hyst;
> > +
> data->current_profile_setting.mclk_activity = tmp.mclk_activity;
> > +                     }
> > +             }
> > +             break;
> > +     case PP_SMC_POWER_PROFILE_AUTO: /* TO DO auto wattman feature
> not implement */
> > +             return 0;
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +
> >        return 0;
> >   }
> >
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[-- Attachment #1.2: Type: text/html, Size: 12314 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

[Felix Kuehling]

On 2018-01-24 06:05 PM, Zhu, Rex wrote:
> Hi Felix,
>
> The logic of gfx/computer profile setting works in such way as you
> said,need under “auto” state.
> But in new sysfs of power profile setting, we do not check the performance
> Level.
>
> And The “manual” state is a confusing flag，when user set manual
> state，and then change the clk range through pp-dpm-sclk/mclk，the dpm
> still works automatically in new clock range, I think we can remove
> this flag.

But then you're changing the semantics of how pp_dpm_sclk/mclk wok
together with pp_dpm_force_performance_level.

Sysfs interfaces should be maintained stable. I'm OK with breaking the
old power profile stuff, because it only affects KFD which isn't
upstream yet. But the old pp_dpm_sclk/mclk and
pp_dpm_force_performance_level affect code that's currently upstream and
used by existing tools.

>
> So  My idea is
> in the sysfs of pp dpm sclk/mclk, user can set clock range.
> In the sysfs of power profile state, user can configure the parameters
> smu/driver exposed. They are independent.

I disagree. the clock range is not independent of the profile. Profiles
correspond to use cases. Different use cases have different clock
requirements. For compute we want the clocks to be high. For video you
may have requirements for minimum mclks to ensure smooth playback. For
power saving you may want to limit maximum clocks, etc. So setting the
clock range independent of the profile in "auto" mode does not make
sense to me.

Regards,
  Felix

>
>
>
>
>
>
> Best Regards
> Rex
> ------------------------------------------------------------------------
> *From:* Kuehling, Felix
> *Sent:* Thursday, January 25, 2018 5:41:43 AM
> *To:* Zhu, Rex; Huang, JinHuiEric; amd-gfx@lists.freedesktop.org
> *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement
> set_power_profile_mode on smu7
>  
> Hi Rex,
>
> As I understand it (the way power profiles currently work),
> pp_dpm_sclk/mclk only apply if pp_dpm_force_performance_level is set to
> "manual". Power profiles and automatic switching between profiles only
> happens when pp_dpm_force_performance_level is set to "auto".
>
> This means pp_dpm_sclk/mclk don't apply when profiles are in effect.
> Also, there would be no way to set different minimum clocks for
> different profiles.
>
> I think minimum clocks should be part of the profiles.
>
> Regards,
>   Felix
>
>
> On 2018-01-24 03:13 PM, Zhu, Rex wrote:
> > Hi Eric,
> >
> > We have sysfs pp-dpm-sclk/mclk to set min dpm level
> >
> > Best Regards
> > Rex
> > ------------------------------------------------------------------------
> > *From:* amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of
> > Eric Huang <jinhuieric.huang@amd.com>
> > *Sent:* Thursday, January 25, 2018 12:04:55 AM
> > *To:* amd-gfx@lists.freedesktop.org
> > *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement
> > set_power_profile_mode on smu7
> >  
> > We have min_sclk and min_mclk in previous power profile parameters for
> > VI, which are similar with min_active_level for Vega10. How to implement
> > these parameters?
> >
> > Regards,
> > Eric
> >
> > On 2018-01-24 04:37 AM, Rex Zhu wrote:
> > > User can set smu7 profile pamameters through sysfs
> > >
> > > echo "0/1/2/3/4">pp_power_profile_mode
> > > to select 3D_FULL_SCREEN/POWER_SAVING/VIDEO/VR/COMPUTE
> > > mode.
> > > echo "5 * * * * * * * *">pp_power_profile_mode
> > > to config custom mode.
> > > "5 * * * * * * * *" mean "CUSTOM enable_sclk SCLK_UP_HYST
> > > SCLK_DOWN_HYST SCLK_ACTIVE_LEVEL enable_mclk MCLK_UP_HYST
> > > MCLK_DOWN_HYST MCLK_ACTIVE_LEVEL"
> > >
> > > Change-Id: Ic6d6f37363bc81ab17051285f6ace847edf725de
> > > Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
> > > ---
> > >   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 49
> > +++++++++++++++++++++++-
> > >   1 file changed, 48 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > > index 9f6afd5..13db75c 100644
> > > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > > @@ -5036,7 +5036,54 @@ static int smu7_get_power_profile_mode(struct
> > pp_hwmgr *hwmgr, char *buf)
> > >  
> > >   static int smu7_set_power_profile_mode(struct pp_hwmgr *hwmgr,
> > long *input, uint32_t size)
> > >   {
> > > -     /* To Do */
> > > +     struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
> > > +     struct profile_mode_setting tmp;
> > > +
> > > +     hwmgr->power_profile_mode = input[size];
> > > +
> > > +     switch (hwmgr->power_profile_mode) {
> > > +     case PP_SMC_POWER_PROFILE_CUSTOM:
> > > +             if (size < 8)
> > > +                     return -EINVAL;
> > > +
> > > +             data->custom_profile_setting.bupdate_sclk = input[0];
> > > +             data->custom_profile_setting.sclk_up_hyst = input[1];
> > > +             data->custom_profile_setting.sclk_down_hyst = input[2];
> > > +             data->custom_profile_setting.sclk_activity =  input[3];
> > > +             data->custom_profile_setting.bupdate_mclk = input[4];
> > > +             data->custom_profile_setting.mclk_up_hyst = input[5];
> > > +             data->custom_profile_setting.mclk_down_hyst = input[6];
> > > +             data->custom_profile_setting.mclk_activity =  input[7];
> > > +             if (!smum_update_dpm_settings(hwmgr,
> > &data->custom_profile_setting))
> > > +                     memcpy(&data->current_profile_setting,
> > &data->custom_profile_setting, sizeof(struct profile_mode_setting));
> > > +             break;
> > > +     case PP_SMC_POWER_PROFILE_FULLSCREEN3D:
> > > +     case PP_SMC_POWER_PROFILE_POWERSAVING:
> > > +     case PP_SMC_POWER_PROFILE_VIDEO:
> > > +     case PP_SMC_POWER_PROFILE_VR:
> > > +     case PP_SMC_POWER_PROFILE_COMPUTE:
> > > +             memcpy(&tmp,
> > &smu7_profiling[hwmgr->power_profile_mode], sizeof(struct
> > profile_mode_setting));
> > > +             if (!smum_update_dpm_settings(hwmgr, &tmp)) {
> > > +                     if (tmp.bupdate_sclk) {
> > > +                            
> > data->current_profile_setting.bupdate_sclk = tmp.bupdate_sclk;
> > > +                            
> > data->current_profile_setting.sclk_up_hyst = tmp.sclk_up_hyst;
> > > +                            
> > data->current_profile_setting.sclk_down_hyst = tmp.sclk_down_hyst;
> > > +                            
> > data->current_profile_setting.sclk_activity = tmp.sclk_activity;
> > > +                     }
> > > +                     if (tmp.bupdate_mclk) {
> > > +                            
> > data->current_profile_setting.bupdate_mclk = tmp.bupdate_mclk;
> > > +                            
> > data->current_profile_setting.mclk_up_hyst = tmp.mclk_up_hyst;
> > > +                            
> > data->current_profile_setting.mclk_down_hyst = tmp.mclk_down_hyst;
> > > +                            
> > data->current_profile_setting.mclk_activity = tmp.mclk_activity;
> > > +                     }
> > > +             }
> > > +             break;
> > > +     case PP_SMC_POWER_PROFILE_AUTO: /* TO DO auto wattman feature
> > not implement */
> > > +             return 0;
> > > +     default:
> > > +             return -EINVAL;
> > > +     }
> > > +
> > >        return 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
>

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

Re: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

[Alex Deucher]

On Wed, Jan 24, 2018 at 6:12 PM, Felix Kuehling <felix.kuehling@amd.com> wrote:
> On 2018-01-24 06:05 PM, Zhu, Rex wrote:
>> Hi Felix,
>>
>> The logic of gfx/computer profile setting works in such way as you
>> said,need under “auto” state.
>> But in new sysfs of power profile setting, we do not check the performance
>> Level.
>>
>> And The “manual” state is a confusing flag，when user set manual
>> state，and then change the clk range through pp-dpm-sclk/mclk，the dpm
>> still works automatically in new clock range, I think we can remove
>> this flag.
>
> But then you're changing the semantics of how pp_dpm_sclk/mclk wok
> together with pp_dpm_force_performance_level.
>
> Sysfs interfaces should be maintained stable. I'm OK with breaking the
> old power profile stuff, because it only affects KFD which isn't
> upstream yet. But the old pp_dpm_sclk/mclk and
> pp_dpm_force_performance_level affect code that's currently upstream and
> used by existing tools.
>
>>
>> So  My idea is
>> in the sysfs of pp dpm sclk/mclk, user can set clock range.
>> In the sysfs of power profile state, user can configure the parameters
>> smu/driver exposed. They are independent.
>
> I disagree. the clock range is not independent of the profile. Profiles
> correspond to use cases. Different use cases have different clock
> requirements. For compute we want the clocks to be high. For video you
> may have requirements for minimum mclks to ensure smooth playback. For
> power saving you may want to limit maximum clocks, etc. So setting the
> clock range independent of the profile in "auto" mode does not make
> sense to me.

I agree with that.  The clocks should be part of the profile, at least
as far as the automatic profile selection in the driver is concerned.
The whole point of it is so that the driver can select a profile
dynamically for a specific use case without requiring manual
interaction from the user.  For manually selecting profiles I can see
Rex's point (you may want independent knobs for testing different
parameters), but I think we should be consistent in the interface
otherwise it's confusing to users.

Alex

>
> Regards,
>   Felix
>
>>
>>
>>
>>
>>
>>
>> Best Regards
>> Rex
>> ------------------------------------------------------------------------
>> *From:* Kuehling, Felix
>> *Sent:* Thursday, January 25, 2018 5:41:43 AM
>> *To:* Zhu, Rex; Huang, JinHuiEric; amd-gfx@lists.freedesktop.org
>> *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement
>> set_power_profile_mode on smu7
>>
>> Hi Rex,
>>
>> As I understand it (the way power profiles currently work),
>> pp_dpm_sclk/mclk only apply if pp_dpm_force_performance_level is set to
>> "manual". Power profiles and automatic switching between profiles only
>> happens when pp_dpm_force_performance_level is set to "auto".
>>
>> This means pp_dpm_sclk/mclk don't apply when profiles are in effect.
>> Also, there would be no way to set different minimum clocks for
>> different profiles.
>>
>> I think minimum clocks should be part of the profiles.
>>
>> Regards,
>>   Felix
>>
>>
>> On 2018-01-24 03:13 PM, Zhu, Rex wrote:
>> > Hi Eric,
>> >
>> > We have sysfs pp-dpm-sclk/mclk to set min dpm level
>> >
>> > Best Regards
>> > Rex
>> > ------------------------------------------------------------------------
>> > *From:* amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of
>> > Eric Huang <jinhuieric.huang@amd.com>
>> > *Sent:* Thursday, January 25, 2018 12:04:55 AM
>> > *To:* amd-gfx@lists.freedesktop.org
>> > *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement
>> > set_power_profile_mode on smu7
>> >
>> > We have min_sclk and min_mclk in previous power profile parameters for
>> > VI, which are similar with min_active_level for Vega10. How to implement
>> > these parameters?
>> >
>> > Regards,
>> > Eric
>> >
>> > On 2018-01-24 04:37 AM, Rex Zhu wrote:
>> > > User can set smu7 profile pamameters through sysfs
>> > >
>> > > echo "0/1/2/3/4">pp_power_profile_mode
>> > > to select 3D_FULL_SCREEN/POWER_SAVING/VIDEO/VR/COMPUTE
>> > > mode.
>> > > echo "5 * * * * * * * *">pp_power_profile_mode
>> > > to config custom mode.
>> > > "5 * * * * * * * *" mean "CUSTOM enable_sclk SCLK_UP_HYST
>> > > SCLK_DOWN_HYST SCLK_ACTIVE_LEVEL enable_mclk MCLK_UP_HYST
>> > > MCLK_DOWN_HYST MCLK_ACTIVE_LEVEL"
>> > >
>> > > Change-Id: Ic6d6f37363bc81ab17051285f6ace847edf725de
>> > > Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
>> > > ---
>> > >   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 49
>> > +++++++++++++++++++++++-
>> > >   1 file changed, 48 insertions(+), 1 deletion(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> > b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> > > index 9f6afd5..13db75c 100644
>> > > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> > > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> > > @@ -5036,7 +5036,54 @@ static int smu7_get_power_profile_mode(struct
>> > pp_hwmgr *hwmgr, char *buf)
>> > >
>> > >   static int smu7_set_power_profile_mode(struct pp_hwmgr *hwmgr,
>> > long *input, uint32_t size)
>> > >   {
>> > > -     /* To Do */
>> > > +     struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
>> > > +     struct profile_mode_setting tmp;
>> > > +
>> > > +     hwmgr->power_profile_mode = input[size];
>> > > +
>> > > +     switch (hwmgr->power_profile_mode) {
>> > > +     case PP_SMC_POWER_PROFILE_CUSTOM:
>> > > +             if (size < 8)
>> > > +                     return -EINVAL;
>> > > +
>> > > +             data->custom_profile_setting.bupdate_sclk = input[0];
>> > > +             data->custom_profile_setting.sclk_up_hyst = input[1];
>> > > +             data->custom_profile_setting.sclk_down_hyst = input[2];
>> > > +             data->custom_profile_setting.sclk_activity =  input[3];
>> > > +             data->custom_profile_setting.bupdate_mclk = input[4];
>> > > +             data->custom_profile_setting.mclk_up_hyst = input[5];
>> > > +             data->custom_profile_setting.mclk_down_hyst = input[6];
>> > > +             data->custom_profile_setting.mclk_activity =  input[7];
>> > > +             if (!smum_update_dpm_settings(hwmgr,
>> > &data->custom_profile_setting))
>> > > +                     memcpy(&data->current_profile_setting,
>> > &data->custom_profile_setting, sizeof(struct profile_mode_setting));
>> > > +             break;
>> > > +     case PP_SMC_POWER_PROFILE_FULLSCREEN3D:
>> > > +     case PP_SMC_POWER_PROFILE_POWERSAVING:
>> > > +     case PP_SMC_POWER_PROFILE_VIDEO:
>> > > +     case PP_SMC_POWER_PROFILE_VR:
>> > > +     case PP_SMC_POWER_PROFILE_COMPUTE:
>> > > +             memcpy(&tmp,
>> > &smu7_profiling[hwmgr->power_profile_mode], sizeof(struct
>> > profile_mode_setting));
>> > > +             if (!smum_update_dpm_settings(hwmgr, &tmp)) {
>> > > +                     if (tmp.bupdate_sclk) {
>> > > +
>> > data->current_profile_setting.bupdate_sclk = tmp.bupdate_sclk;
>> > > +
>> > data->current_profile_setting.sclk_up_hyst = tmp.sclk_up_hyst;
>> > > +
>> > data->current_profile_setting.sclk_down_hyst = tmp.sclk_down_hyst;
>> > > +
>> > data->current_profile_setting.sclk_activity = tmp.sclk_activity;
>> > > +                     }
>> > > +                     if (tmp.bupdate_mclk) {
>> > > +
>> > data->current_profile_setting.bupdate_mclk = tmp.bupdate_mclk;
>> > > +
>> > data->current_profile_setting.mclk_up_hyst = tmp.mclk_up_hyst;
>> > > +
>> > data->current_profile_setting.mclk_down_hyst = tmp.mclk_down_hyst;
>> > > +
>> > data->current_profile_setting.mclk_activity = tmp.mclk_activity;
>> > > +                     }
>> > > +             }
>> > > +             break;
>> > > +     case PP_SMC_POWER_PROFILE_AUTO: /* TO DO auto wattman feature
>> > not implement */
>> > > +             return 0;
>> > > +     default:
>> > > +             return -EINVAL;
>> > > +     }
>> > > +
>> > >        return 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
>>
>
> _______________________________________________
> 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

Re: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

[Zhu, Rex]

[-- Attachment #1.1: Type: text/plain, Size: 10495 bytes --]

>But then you're changing the semantics of how pp_dpm_sclk/mclk wok
>together with pp_dpm_force_performance_level.

Rex:The two sysfs are all for clock range setting.


pp_dpm_sclk/mclk/pcie can set sclk/mclk/pcie range separately.


and pp_dpm_force_performance_level, we can support low/high/umd-pstate that set all the clocks  range.

(manual state, driver do nothing except set the flag)


No matter what state user enter in, driver can support to continue to change the clock range through pp_dpm_sclk/mclk/pcie.


so in fact, don't need manual state for sepalately set the clock range.


>Sysfs interfaces should be maintained stable. I'm OK with breaking the
>old power profile stuff, because it only affects KFD which isn't
>upstream yet.


Rex: As the old power profile interface can't support Vega.

so add new interface and we are try to support old asics with new sysfs.



>But the old pp_dpm_sclk/mclk and
>pp_dpm_force_performance_level affect code that's currently upstream and
>used by existing tools.

Rex:  Yes, we are trying to maintain the sysfs stable.
if we change the old sysfs code,  we will try to not affect existing tools.

Best Regards
Rex


________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Felix Kuehling <felix.kuehling-5C7GfCeVMHo@public.gmane.org>
Sent: Thursday, January 25, 2018 7:12 AM
To: Zhu, Rex; Huang, JinHuiEric; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

On 2018-01-24 06:05 PM, Zhu, Rex wrote:
> Hi Felix,
>
> The logic of gfx/computer profile setting works in such way as you
> said,need under “auto” state.
> But in new sysfs of power profile setting, we do not check the performance
> Level.
>
> And The “manual” state is a confusing flag，when user set manual
> state，and then change the clk range through pp-dpm-sclk/mclk，the dpm
> still works automatically in new clock range, I think we can remove
> this flag.

But then you're changing the semantics of how pp_dpm_sclk/mclk wok
together with pp_dpm_force_performance_level.

Sysfs interfaces should be maintained stable. I'm OK with breaking the
old power profile stuff, because it only affects KFD which isn't
upstream yet. But the old pp_dpm_sclk/mclk and
pp_dpm_force_performance_level affect code that's currently upstream and
used by existing tools.

>
> So  My idea is
> in the sysfs of pp dpm sclk/mclk, user can set clock range.
> In the sysfs of power profile state, user can configure the parameters
> smu/driver exposed. They are independent.

I disagree. the clock range is not independent of the profile. Profiles
correspond to use cases. Different use cases have different clock
requirements. For compute we want the clocks to be high. For video you
may have requirements for minimum mclks to ensure smooth playback. For
power saving you may want to limit maximum clocks, etc. So setting the
clock range independent of the profile in "auto" mode does not make
sense to me.

Regards,
  Felix

>
>
>
>
>
>
> Best Regards
> Rex
> ------------------------------------------------------------------------
> *From:* Kuehling, Felix
> *Sent:* Thursday, January 25, 2018 5:41:43 AM
> *To:* Zhu, Rex; Huang, JinHuiEric; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement
> set_power_profile_mode on smu7
>
> Hi Rex,
>
> As I understand it (the way power profiles currently work),
> pp_dpm_sclk/mclk only apply if pp_dpm_force_performance_level is set to
> "manual". Power profiles and automatic switching between profiles only
> happens when pp_dpm_force_performance_level is set to "auto".
>
> This means pp_dpm_sclk/mclk don't apply when profiles are in effect.
> Also, there would be no way to set different minimum clocks for
> different profiles.
>
> I think minimum clocks should be part of the profiles.
>
> Regards,
>   Felix
>
>
> On 2018-01-24 03:13 PM, Zhu, Rex wrote:
> > Hi Eric,
> >
> > We have sysfs pp-dpm-sclk/mclk to set min dpm level
> >
> > Best Regards
> > Rex
> > ------------------------------------------------------------------------
> > *From:* amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of
> > Eric Huang <jinhuieric.huang-5C7GfCeVMHo@public.gmane.org>
> > *Sent:* Thursday, January 25, 2018 12:04:55 AM
> > *To:* amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> > *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement
> > set_power_profile_mode on smu7
> >
> > We have min_sclk and min_mclk in previous power profile parameters for
> > VI, which are similar with min_active_level for Vega10. How to implement
> > these parameters?
> >
> > Regards,
> > Eric
> >
> > On 2018-01-24 04:37 AM, Rex Zhu wrote:
> > > User can set smu7 profile pamameters through sysfs
> > >
> > > echo "0/1/2/3/4">pp_power_profile_mode
> > > to select 3D_FULL_SCREEN/POWER_SAVING/VIDEO/VR/COMPUTE
> > > mode.
> > > echo "5 * * * * * * * *">pp_power_profile_mode
> > > to config custom mode.
> > > "5 * * * * * * * *" mean "CUSTOM enable_sclk SCLK_UP_HYST
> > > SCLK_DOWN_HYST SCLK_ACTIVE_LEVEL enable_mclk MCLK_UP_HYST
> > > MCLK_DOWN_HYST MCLK_ACTIVE_LEVEL"
> > >
> > > Change-Id: Ic6d6f37363bc81ab17051285f6ace847edf725de
> > > Signed-off-by: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
> > > ---
> > >   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 49
> > +++++++++++++++++++++++-
> > >   1 file changed, 48 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > > index 9f6afd5..13db75c 100644
> > > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > > @@ -5036,7 +5036,54 @@ static int smu7_get_power_profile_mode(struct
> > pp_hwmgr *hwmgr, char *buf)
> > >
> > >   static int smu7_set_power_profile_mode(struct pp_hwmgr *hwmgr,
> > long *input, uint32_t size)
> > >   {
> > > -     /* To Do */
> > > +     struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
> > > +     struct profile_mode_setting tmp;
> > > +
> > > +     hwmgr->power_profile_mode = input[size];
> > > +
> > > +     switch (hwmgr->power_profile_mode) {
> > > +     case PP_SMC_POWER_PROFILE_CUSTOM:
> > > +             if (size < 8)
> > > +                     return -EINVAL;
> > > +
> > > +             data->custom_profile_setting.bupdate_sclk = input[0];
> > > +             data->custom_profile_setting.sclk_up_hyst = input[1];
> > > +             data->custom_profile_setting.sclk_down_hyst = input[2];
> > > +             data->custom_profile_setting.sclk_activity =  input[3];
> > > +             data->custom_profile_setting.bupdate_mclk = input[4];
> > > +             data->custom_profile_setting.mclk_up_hyst = input[5];
> > > +             data->custom_profile_setting.mclk_down_hyst = input[6];
> > > +             data->custom_profile_setting.mclk_activity =  input[7];
> > > +             if (!smum_update_dpm_settings(hwmgr,
> > &data->custom_profile_setting))
> > > +                     memcpy(&data->current_profile_setting,
> > &data->custom_profile_setting, sizeof(struct profile_mode_setting));
> > > +             break;
> > > +     case PP_SMC_POWER_PROFILE_FULLSCREEN3D:
> > > +     case PP_SMC_POWER_PROFILE_POWERSAVING:
> > > +     case PP_SMC_POWER_PROFILE_VIDEO:
> > > +     case PP_SMC_POWER_PROFILE_VR:
> > > +     case PP_SMC_POWER_PROFILE_COMPUTE:
> > > +             memcpy(&tmp,
> > &smu7_profiling[hwmgr->power_profile_mode], sizeof(struct
> > profile_mode_setting));
> > > +             if (!smum_update_dpm_settings(hwmgr, &tmp)) {
> > > +                     if (tmp.bupdate_sclk) {
> > > +
> > data->current_profile_setting.bupdate_sclk = tmp.bupdate_sclk;
> > > +
> > data->current_profile_setting.sclk_up_hyst = tmp.sclk_up_hyst;
> > > +
> > data->current_profile_setting.sclk_down_hyst = tmp.sclk_down_hyst;
> > > +
> > data->current_profile_setting.sclk_activity = tmp.sclk_activity;
> > > +                     }
> > > +                     if (tmp.bupdate_mclk) {
> > > +
> > data->current_profile_setting.bupdate_mclk = tmp.bupdate_mclk;
> > > +
> > data->current_profile_setting.mclk_up_hyst = tmp.mclk_up_hyst;
> > > +
> > data->current_profile_setting.mclk_down_hyst = tmp.mclk_down_hyst;
> > > +
> > data->current_profile_setting.mclk_activity = tmp.mclk_activity;
> > > +                     }
> > > +             }
> > > +             break;
> > > +     case PP_SMC_POWER_PROFILE_AUTO: /* TO DO auto wattman feature
> > not implement */
> > > +             return 0;
> > > +     default:
> > > +             return -EINVAL;
> > > +     }
> > > +
> > >        return 0;
> > >   }
> > >
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
amd-gfx Info Page - freedesktop.org<https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
lists.freedesktop.org
Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the following form. Use of all freedesktop.org lists is subject to our Code of ...



> >
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
amd-gfx Info Page - freedesktop.org<https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
lists.freedesktop.org
Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the following form. Use of all freedesktop.org lists is subject to our Code of ...



>

_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
amd-gfx Info Page - freedesktop.org<https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
lists.freedesktop.org
Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the following form. Use of all freedesktop.org lists is subject to our Code of ...




[-- Attachment #1.2: Type: text/html, Size: 28586 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

[Felix Kuehling]

On 2018-01-24 09:28 PM, Zhu, Rex wrote:
>
> >But then you're changing the semantics of how pp_dpm_sclk/mclk wok
> >together with pp_dpm_force_performance_level.
>
> Rex:The two sysfs are all for clock range setting.
>
>
> pp_dpm_sclk/mclk/pcie can set sclk/mclk/pcie range separately.
>

Only in manual mode. If you change that, you're changing the interface
and breaking existing tools.

>
> and pp_dpm_force_performance_level, we can support low/high/umd-pstate
> that set all the clocks  range.
>
> (manual state, driver do nothing except set the flag)
>
>
> No matter what state user enter in, driver can support to continue to
> change the clock range through pp_dpm_sclk/mclk/pcie.
>
>
> so in fact, don't need manual state for sepalately set the clock range.
>

That's a change in the pp_dpm_sclk/mclk interface. Currenly these only
work in manual mode and fail or are ignored in auto mode. E.g. see this
code in smu7_hwmgr.c:

static int smu7_force_clock_level(struct pp_hwmgr *hwmgr,
                enum pp_clock_type type, uint32_t mask)
{
        struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);

        if (hwmgr->request_dpm_level & (AMD_DPM_FORCED_LEVEL_AUTO |
                                        AMD_DPM_FORCED_LEVEL_LOW |
                                        AMD_DPM_FORCED_LEVEL_HIGH))
                return -EINVAL;

Regards,
  Felix


>
> >Sysfs interfaces should be maintained stable. I'm OK with breaking the
> >old power profile stuff, because it only affects KFD which isn't
> >upstream yet. 
>
>
> Rex: As the old power profile interface can't support Vega.
>
> so add new interface and we are try to support old asics with new sysfs.
>
>
>
> >But the old pp_dpm_sclk/mclk and
> >pp_dpm_force_performance_level affect code that's currently upstream and
> >used by existing tools.
>
>
> Rex:  Yes, we are trying to maintain the sysfs stable.
> if we change the old sysfs code,  we will try to not affect existing
> tools.
>
> Best Regards
> Rex
>
>  
> ------------------------------------------------------------------------
> *From:* amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of
> Felix Kuehling <felix.kuehling@amd.com>
> *Sent:* Thursday, January 25, 2018 7:12 AM
> *To:* Zhu, Rex; Huang, JinHuiEric; amd-gfx@lists.freedesktop.org
> *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement
> set_power_profile_mode on smu7
>  
> On 2018-01-24 06:05 PM, Zhu, Rex wrote:
> > Hi Felix,
> >
> > The logic of gfx/computer profile setting works in such way as you
> > said,need under “auto” state.
> > But in new sysfs of power profile setting, we do not check the
> performance
> > Level.
> >
> > And The “manual” state is a confusing flag，when user set manual
> > state，and then change the clk range through pp-dpm-sclk/mclk，the dpm
> > still works automatically in new clock range, I think we can remove
> > this flag.
>
> But then you're changing the semantics of how pp_dpm_sclk/mclk wok
> together with pp_dpm_force_performance_level.
>
> Sysfs interfaces should be maintained stable. I'm OK with breaking the
> old power profile stuff, because it only affects KFD which isn't
> upstream yet. But the old pp_dpm_sclk/mclk and
> pp_dpm_force_performance_level affect code that's currently upstream and
> used by existing tools.
>
> >
> > So  My idea is
> > in the sysfs of pp dpm sclk/mclk, user can set clock range.
> > In the sysfs of power profile state, user can configure the parameters
> > smu/driver exposed. They are independent.
>
> I disagree. the clock range is not independent of the profile. Profiles
> correspond to use cases. Different use cases have different clock
> requirements. For compute we want the clocks to be high. For video you
> may have requirements for minimum mclks to ensure smooth playback. For
> power saving you may want to limit maximum clocks, etc. So setting the
> clock range independent of the profile in "auto" mode does not make
> sense to me.
>
> Regards,
>   Felix
>
> >
> >
> >
> >
> >
> >
> > Best Regards
> > Rex
> > ------------------------------------------------------------------------
> > *From:* Kuehling, Felix
> > *Sent:* Thursday, January 25, 2018 5:41:43 AM
> > *To:* Zhu, Rex; Huang, JinHuiEric; amd-gfx@lists.freedesktop.org
> > *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement
> > set_power_profile_mode on smu7
> >  
> > Hi Rex,
> >
> > As I understand it (the way power profiles currently work),
> > pp_dpm_sclk/mclk only apply if pp_dpm_force_performance_level is set to
> > "manual". Power profiles and automatic switching between profiles only
> > happens when pp_dpm_force_performance_level is set to "auto".
> >
> > This means pp_dpm_sclk/mclk don't apply when profiles are in effect.
> > Also, there would be no way to set different minimum clocks for
> > different profiles.
> >
> > I think minimum clocks should be part of the profiles.
> >
> > Regards,
> >   Felix
> >
> >
> > On 2018-01-24 03:13 PM, Zhu, Rex wrote:
> > > Hi Eric,
> > >
> > > We have sysfs pp-dpm-sclk/mclk to set min dpm level
> > >
> > > Best Regards
> > > Rex
> > >
> ------------------------------------------------------------------------
> > > *From:* amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of
> > > Eric Huang <jinhuieric.huang@amd.com>
> > > *Sent:* Thursday, January 25, 2018 12:04:55 AM
> > > *To:* amd-gfx@lists.freedesktop.org
> > > *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement
> > > set_power_profile_mode on smu7
> > >  
> > > We have min_sclk and min_mclk in previous power profile parameters for
> > > VI, which are similar with min_active_level for Vega10. How to
> implement
> > > these parameters?
> > >
> > > Regards,
> > > Eric
> > >
> > > On 2018-01-24 04:37 AM, Rex Zhu wrote:
> > > > User can set smu7 profile pamameters through sysfs
> > > >
> > > > echo "0/1/2/3/4">pp_power_profile_mode
> > > > to select 3D_FULL_SCREEN/POWER_SAVING/VIDEO/VR/COMPUTE
> > > > mode.
> > > > echo "5 * * * * * * * *">pp_power_profile_mode
> > > > to config custom mode.
> > > > "5 * * * * * * * *" mean "CUSTOM enable_sclk SCLK_UP_HYST
> > > > SCLK_DOWN_HYST SCLK_ACTIVE_LEVEL enable_mclk MCLK_UP_HYST
> > > > MCLK_DOWN_HYST MCLK_ACTIVE_LEVEL"
> > > >
> > > > Change-Id: Ic6d6f37363bc81ab17051285f6ace847edf725de
> > > > Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
> > > > ---
> > > >   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 49
> > > +++++++++++++++++++++++-
> > > >   1 file changed, 48 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > > b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > > > index 9f6afd5..13db75c 100644
> > > > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > > > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > > > @@ -5036,7 +5036,54 @@ static int smu7_get_power_profile_mode(struct
> > > pp_hwmgr *hwmgr, char *buf)
> > > >  
> > > >   static int smu7_set_power_profile_mode(struct pp_hwmgr *hwmgr,
> > > long *input, uint32_t size)
> > > >   {
> > > > -     /* To Do */
> > > > +     struct smu7_hwmgr *data = (struct smu7_hwmgr
> *)(hwmgr->backend);
> > > > +     struct profile_mode_setting tmp;
> > > > +
> > > > +     hwmgr->power_profile_mode = input[size];
> > > > +
> > > > +     switch (hwmgr->power_profile_mode) {
> > > > +     case PP_SMC_POWER_PROFILE_CUSTOM:
> > > > +             if (size < 8)
> > > > +                     return -EINVAL;
> > > > +
> > > > +             data->custom_profile_setting.bupdate_sclk = input[0];
> > > > +             data->custom_profile_setting.sclk_up_hyst = input[1];
> > > > +             data->custom_profile_setting.sclk_down_hyst =
> input[2];
> > > > +             data->custom_profile_setting.sclk_activity = 
> input[3];
> > > > +             data->custom_profile_setting.bupdate_mclk = input[4];
> > > > +             data->custom_profile_setting.mclk_up_hyst = input[5];
> > > > +             data->custom_profile_setting.mclk_down_hyst =
> input[6];
> > > > +             data->custom_profile_setting.mclk_activity = 
> input[7];
> > > > +             if (!smum_update_dpm_settings(hwmgr,
> > > &data->custom_profile_setting))
> > > > +                     memcpy(&data->current_profile_setting,
> > > &data->custom_profile_setting, sizeof(struct profile_mode_setting));
> > > > +             break;
> > > > +     case PP_SMC_POWER_PROFILE_FULLSCREEN3D:
> > > > +     case PP_SMC_POWER_PROFILE_POWERSAVING:
> > > > +     case PP_SMC_POWER_PROFILE_VIDEO:
> > > > +     case PP_SMC_POWER_PROFILE_VR:
> > > > +     case PP_SMC_POWER_PROFILE_COMPUTE:
> > > > +             memcpy(&tmp,
> > > &smu7_profiling[hwmgr->power_profile_mode], sizeof(struct
> > > profile_mode_setting));
> > > > +             if (!smum_update_dpm_settings(hwmgr, &tmp)) {
> > > > +                     if (tmp.bupdate_sclk) {
> > > > +                            
> > > data->current_profile_setting.bupdate_sclk = tmp.bupdate_sclk;
> > > > +                            
> > > data->current_profile_setting.sclk_up_hyst = tmp.sclk_up_hyst;
> > > > +                            
> > > data->current_profile_setting.sclk_down_hyst = tmp.sclk_down_hyst;
> > > > +                            
> > > data->current_profile_setting.sclk_activity = tmp.sclk_activity;
> > > > +                     }
> > > > +                     if (tmp.bupdate_mclk) {
> > > > +                            
> > > data->current_profile_setting.bupdate_mclk = tmp.bupdate_mclk;
> > > > +                            
> > > data->current_profile_setting.mclk_up_hyst = tmp.mclk_up_hyst;
> > > > +                            
> > > data->current_profile_setting.mclk_down_hyst = tmp.mclk_down_hyst;
> > > > +                            
> > > data->current_profile_setting.mclk_activity = tmp.mclk_activity;
> > > > +                     }
> > > > +             }
> > > > +             break;
> > > > +     case PP_SMC_POWER_PROFILE_AUTO: /* TO DO auto wattman feature
> > > not implement */
> > > > +             return 0;
> > > > +     default:
> > > > +             return -EINVAL;
> > > > +     }
> > > > +
> > > >        return 0;
> > > >   }
> > > >  
> > >
> > > _______________________________________________
> > > amd-gfx mailing list
> > > amd-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> amd-gfx Info Page - freedesktop.org
> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
> lists.freedesktop.org
> Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the
> following form. Use of all freedesktop.org lists is subject to our
> Code of ...
>
>
>
> > >
> > >
> > > _______________________________________________
> > > amd-gfx mailing list
> > > amd-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> amd-gfx Info Page - freedesktop.org
> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
> lists.freedesktop.org
> Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the
> following form. Use of all freedesktop.org lists is subject to our
> Code of ...
>
>
>
> >
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> amd-gfx Info Page - freedesktop.org
> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
> lists.freedesktop.org
> Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the
> following form. Use of all freedesktop.org lists is subject to our
> Code of ...
>
>
>

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

RE: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

[Zhu, Rex]

[-- Attachment #1: Type: text/plain, Size: 13120 bytes --]

>>That's a change in the pp_dpm_sclk/mclk interface. Currenly these only work in manual mode and fail or are ignored in auto mode. E.g. see this code in smu7_hwmgr.c:
>>static int smu7_force_clock_level(struct pp_hwmgr *hwmgr,

The attached patch can fix this issue.
Thanks.


Best Regards
Rex
-----Original Message-----
From: Kuehling, Felix 
Sent: Thursday, January 25, 2018 12:16 PM
To: Zhu, Rex; Huang, JinHuiEric; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

On 2018-01-24 09:28 PM, Zhu, Rex wrote:
>
> >But then you're changing the semantics of how pp_dpm_sclk/mclk wok 
> >together with pp_dpm_force_performance_level.
>
> Rex:The two sysfs are all for clock range setting.
>
>
> pp_dpm_sclk/mclk/pcie can set sclk/mclk/pcie range separately.
>

Only in manual mode. If you change that, you're changing the interface and breaking existing tools.

>
> and pp_dpm_force_performance_level, we can support low/high/umd-pstate 
> that set all the clocks  range.
>
> (manual state, driver do nothing except set the flag)
>
>
> No matter what state user enter in, driver can support to continue to 
> change the clock range through pp_dpm_sclk/mclk/pcie.
>
>
> so in fact, don't need manual state for sepalately set the clock range.
>

That's a change in the pp_dpm_sclk/mclk interface. Currenly these only work in manual mode and fail or are ignored in auto mode. E.g. see this code in smu7_hwmgr.c:

static int smu7_force_clock_level(struct pp_hwmgr *hwmgr,
                enum pp_clock_type type, uint32_t mask) {
        struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);

        if (hwmgr->request_dpm_level & (AMD_DPM_FORCED_LEVEL_AUTO |
                                        AMD_DPM_FORCED_LEVEL_LOW |
                                        AMD_DPM_FORCED_LEVEL_HIGH))
                return -EINVAL;

Regards,
  Felix


>
> >Sysfs interfaces should be maintained stable. I'm OK with breaking 
> >the old power profile stuff, because it only affects KFD which isn't 
> >upstream yet.
>
>
> Rex: As the old power profile interface can't support Vega.
>
> so add new interface and we are try to support old asics with new sysfs.
>
>
>
> >But the old pp_dpm_sclk/mclk and
> >pp_dpm_force_performance_level affect code that's currently upstream 
> >and used by existing tools.
>
>
> Rex:  Yes, we are trying to maintain the sysfs stable.
> if we change the old sysfs code,  we will try to not affect existing 
> tools.
>
> Best Regards
> Rex
>
>  
> ----------------------------------------------------------------------
> --
> *From:* amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of 
> Felix Kuehling <felix.kuehling@amd.com>
> *Sent:* Thursday, January 25, 2018 7:12 AM
> *To:* Zhu, Rex; Huang, JinHuiEric; amd-gfx@lists.freedesktop.org
> *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement 
> set_power_profile_mode on smu7
>  
> On 2018-01-24 06:05 PM, Zhu, Rex wrote:
> > Hi Felix,
> >
> > The logic of gfx/computer profile setting works in such way as you 
> > said,need under “auto” state.
> > But in new sysfs of power profile setting, we do not check the
> performance
> > Level.
> >
> > And The “manual” state is a confusing flag，when user set manual
> > state，and then change the clk range through pp-dpm-sclk/mclk，the dpm 
> > still works automatically in new clock range, I think we can remove 
> > this flag.
>
> But then you're changing the semantics of how pp_dpm_sclk/mclk wok 
> together with pp_dpm_force_performance_level.
>
> Sysfs interfaces should be maintained stable. I'm OK with breaking the 
> old power profile stuff, because it only affects KFD which isn't 
> upstream yet. But the old pp_dpm_sclk/mclk and 
> pp_dpm_force_performance_level affect code that's currently upstream 
> and used by existing tools.
>
> >
> > So  My idea is
> > in the sysfs of pp dpm sclk/mclk, user can set clock range.
> > In the sysfs of power profile state, user can configure the 
> > parameters smu/driver exposed. They are independent.
>
> I disagree. the clock range is not independent of the profile. 
> Profiles correspond to use cases. Different use cases have different 
> clock requirements. For compute we want the clocks to be high. For 
> video you may have requirements for minimum mclks to ensure smooth 
> playback. For power saving you may want to limit maximum clocks, etc. 
> So setting the clock range independent of the profile in "auto" mode 
> does not make sense to me.
>
> Regards,
>   Felix
>
> >
> >
> >
> >
> >
> >
> > Best Regards
> > Rex
> > --------------------------------------------------------------------
> > ----
> > *From:* Kuehling, Felix
> > *Sent:* Thursday, January 25, 2018 5:41:43 AM
> > *To:* Zhu, Rex; Huang, JinHuiEric; amd-gfx@lists.freedesktop.org
> > *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement 
> > set_power_profile_mode on smu7
> >  
> > Hi Rex,
> >
> > As I understand it (the way power profiles currently work), 
> > pp_dpm_sclk/mclk only apply if pp_dpm_force_performance_level is set 
> > to "manual". Power profiles and automatic switching between profiles 
> > only happens when pp_dpm_force_performance_level is set to "auto".
> >
> > This means pp_dpm_sclk/mclk don't apply when profiles are in effect.
> > Also, there would be no way to set different minimum clocks for 
> > different profiles.
> >
> > I think minimum clocks should be part of the profiles.
> >
> > Regards,
> >   Felix
> >
> >
> > On 2018-01-24 03:13 PM, Zhu, Rex wrote:
> > > Hi Eric,
> > >
> > > We have sysfs pp-dpm-sclk/mclk to set min dpm level
> > >
> > > Best Regards
> > > Rex
> > >
> ----------------------------------------------------------------------
> --
> > > *From:* amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf 
> > > of Eric Huang <jinhuieric.huang@amd.com>
> > > *Sent:* Thursday, January 25, 2018 12:04:55 AM
> > > *To:* amd-gfx@lists.freedesktop.org
> > > *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement 
> > > set_power_profile_mode on smu7
> > >  
> > > We have min_sclk and min_mclk in previous power profile parameters 
> > > for VI, which are similar with min_active_level for Vega10. How to
> implement
> > > these parameters?
> > >
> > > Regards,
> > > Eric
> > >
> > > On 2018-01-24 04:37 AM, Rex Zhu wrote:
> > > > User can set smu7 profile pamameters through sysfs
> > > >
> > > > echo "0/1/2/3/4">pp_power_profile_mode to select 
> > > > 3D_FULL_SCREEN/POWER_SAVING/VIDEO/VR/COMPUTE
> > > > mode.
> > > > echo "5 * * * * * * * *">pp_power_profile_mode to config custom 
> > > > mode.
> > > > "5 * * * * * * * *" mean "CUSTOM enable_sclk SCLK_UP_HYST 
> > > > SCLK_DOWN_HYST SCLK_ACTIVE_LEVEL enable_mclk MCLK_UP_HYST 
> > > > MCLK_DOWN_HYST MCLK_ACTIVE_LEVEL"
> > > >
> > > > Change-Id: Ic6d6f37363bc81ab17051285f6ace847edf725de
> > > > Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
> > > > ---
> > > >   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 49
> > > +++++++++++++++++++++++-
> > > >   1 file changed, 48 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > > b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > > > index 9f6afd5..13db75c 100644
> > > > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > > > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > > > @@ -5036,7 +5036,54 @@ static int 
> > > > smu7_get_power_profile_mode(struct
> > > pp_hwmgr *hwmgr, char *buf)
> > > >  
> > > >   static int smu7_set_power_profile_mode(struct pp_hwmgr *hwmgr,
> > > long *input, uint32_t size)
> > > >   {
> > > > -     /* To Do */
> > > > +     struct smu7_hwmgr *data = (struct smu7_hwmgr
> *)(hwmgr->backend);
> > > > +     struct profile_mode_setting tmp;
> > > > +
> > > > +     hwmgr->power_profile_mode = input[size];
> > > > +
> > > > +     switch (hwmgr->power_profile_mode) {
> > > > +     case PP_SMC_POWER_PROFILE_CUSTOM:
> > > > +             if (size < 8)
> > > > +                     return -EINVAL;
> > > > +
> > > > +             data->custom_profile_setting.bupdate_sclk = 
> > > > +input[0];
> > > > +             data->custom_profile_setting.sclk_up_hyst = 
> > > > +input[1];
> > > > +             data->custom_profile_setting.sclk_down_hyst =
> input[2];
> > > > +             data->custom_profile_setting.sclk_activity =
> input[3];
> > > > +             data->custom_profile_setting.bupdate_mclk = 
> > > > +input[4];
> > > > +             data->custom_profile_setting.mclk_up_hyst = 
> > > > +input[5];
> > > > +             data->custom_profile_setting.mclk_down_hyst =
> input[6];
> > > > +             data->custom_profile_setting.mclk_activity =
> input[7];
> > > > +             if (!smum_update_dpm_settings(hwmgr,
> > > &data->custom_profile_setting))
> > > > +                     memcpy(&data->current_profile_setting,
> > > &data->custom_profile_setting, sizeof(struct 
> > > profile_mode_setting));
> > > > +             break;
> > > > +     case PP_SMC_POWER_PROFILE_FULLSCREEN3D:
> > > > +     case PP_SMC_POWER_PROFILE_POWERSAVING:
> > > > +     case PP_SMC_POWER_PROFILE_VIDEO:
> > > > +     case PP_SMC_POWER_PROFILE_VR:
> > > > +     case PP_SMC_POWER_PROFILE_COMPUTE:
> > > > +             memcpy(&tmp,
> > > &smu7_profiling[hwmgr->power_profile_mode], sizeof(struct 
> > > profile_mode_setting));
> > > > +             if (!smum_update_dpm_settings(hwmgr, &tmp)) {
> > > > +                     if (tmp.bupdate_sclk) {
> > > > +                            
> > > data->current_profile_setting.bupdate_sclk = tmp.bupdate_sclk;
> > > > +                            
> > > data->current_profile_setting.sclk_up_hyst = tmp.sclk_up_hyst;
> > > > +                            
> > > data->current_profile_setting.sclk_down_hyst = tmp.sclk_down_hyst;
> > > > +                            
> > > data->current_profile_setting.sclk_activity = tmp.sclk_activity;
> > > > +                     }
> > > > +                     if (tmp.bupdate_mclk) {
> > > > +                            
> > > data->current_profile_setting.bupdate_mclk = tmp.bupdate_mclk;
> > > > +                            
> > > data->current_profile_setting.mclk_up_hyst = tmp.mclk_up_hyst;
> > > > +                            
> > > data->current_profile_setting.mclk_down_hyst = tmp.mclk_down_hyst;
> > > > +                            
> > > data->current_profile_setting.mclk_activity = tmp.mclk_activity;
> > > > +                     }
> > > > +             }
> > > > +             break;
> > > > +     case PP_SMC_POWER_PROFILE_AUTO: /* TO DO auto wattman 
> > > > +feature
> > > not implement */
> > > > +             return 0;
> > > > +     default:
> > > > +             return -EINVAL;
> > > > +     }
> > > > +
> > > >        return 0;
> > > >   }
> > > >  
> > >
> > > _______________________________________________
> > > amd-gfx mailing list
> > > amd-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> amd-gfx Info Page - freedesktop.org
> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
> lists.freedesktop.org
> Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the 
> following form. Use of all freedesktop.org lists is subject to our 
> Code of ...
>
>
>
> > >
> > >
> > > _______________________________________________
> > > amd-gfx mailing list
> > > amd-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> amd-gfx Info Page - freedesktop.org
> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
> lists.freedesktop.org
> Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the 
> following form. Use of all freedesktop.org lists is subject to our 
> Code of ...
>
>
>
> >
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> amd-gfx Info Page - freedesktop.org
> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
> lists.freedesktop.org
> Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the 
> following form. Use of all freedesktop.org lists is subject to our 
> Code of ...
>
>
>


[-- Attachment #2: Type: message/rfc822, Size: 29115 bytes --]

From: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
To: <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Cc: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
Subject: [PATCH 1/2] drm/amd/pp: Remove manual mode for power_dpm_force_performance_level
Date: Thu, 25 Jan 2018 19:26:53 +0800
Message-ID: <1516879614-11533-1-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>

Driver do not maintain manual mode for dpm_force_performance_level,
User can set sclk/mclk/pcie range through pp_dpm_sclk/pp_dpm_mclk/pp_dpm_pcie
directly.

In order to not break currently tools,
when set "manual" to power_dpm_force_performance_level
driver will do nothing and just return successful.

Change-Id: Iaf672b9abc7fa57b765ceb7fa2fba6ad3e80c50b
Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c             |  3 +--
 drivers/gpu/drm/amd/amdgpu/ci_dpm.c                |  5 -----
 drivers/gpu/drm/amd/include/kgd_pp_interface.h     | 15 +++++++--------
 drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c     |  4 ----
 drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c     |  1 -
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  6 ------
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c |  6 ------
 7 files changed, 8 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 1812009..66b4df0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -152,7 +152,6 @@ static ssize_t amdgpu_get_dpm_forced_performance_level(struct device *dev,
 			(level == AMD_DPM_FORCED_LEVEL_AUTO) ? "auto" :
 			(level == AMD_DPM_FORCED_LEVEL_LOW) ? "low" :
 			(level == AMD_DPM_FORCED_LEVEL_HIGH) ? "high" :
-			(level == AMD_DPM_FORCED_LEVEL_MANUAL) ? "manual" :
 			(level == AMD_DPM_FORCED_LEVEL_PROFILE_STANDARD) ? "profile_standard" :
 			(level == AMD_DPM_FORCED_LEVEL_PROFILE_MIN_SCLK) ? "profile_min_sclk" :
 			(level == AMD_DPM_FORCED_LEVEL_PROFILE_MIN_MCLK) ? "profile_min_mclk" :
@@ -186,7 +185,7 @@ static ssize_t amdgpu_set_dpm_forced_performance_level(struct device *dev,
 	} else if (strncmp("auto", buf, strlen("auto")) == 0) {
 		level = AMD_DPM_FORCED_LEVEL_AUTO;
 	} else if (strncmp("manual", buf, strlen("manual")) == 0) {
-		level = AMD_DPM_FORCED_LEVEL_MANUAL;
+		pr_info("No need to set manual mode, Just go ahead\n");
 	} else if (strncmp("profile_exit", buf, strlen("profile_exit")) == 0) {
 		level = AMD_DPM_FORCED_LEVEL_PROFILE_EXIT;
 	} else if (strncmp("profile_standard", buf, strlen("profile_standard")) == 0) {
diff --git a/drivers/gpu/drm/amd/amdgpu/ci_dpm.c b/drivers/gpu/drm/amd/amdgpu/ci_dpm.c
index ab45232..8ddc978 100644
--- a/drivers/gpu/drm/amd/amdgpu/ci_dpm.c
+++ b/drivers/gpu/drm/amd/amdgpu/ci_dpm.c
@@ -6639,11 +6639,6 @@ static int ci_dpm_force_clock_level(void *handle,
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 	struct ci_power_info *pi = ci_get_pi(adev);
 
-	if (adev->pm.dpm.forced_level & (AMD_DPM_FORCED_LEVEL_AUTO |
-				AMD_DPM_FORCED_LEVEL_LOW |
-				AMD_DPM_FORCED_LEVEL_HIGH))
-		return -EINVAL;
-
 	switch (type) {
 	case PP_SCLK:
 		if (!pi->sclk_dpm_key_disabled)
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index b9aa9f4..3fab686 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -41,14 +41,13 @@ struct amd_vce_state {
 
 enum amd_dpm_forced_level {
 	AMD_DPM_FORCED_LEVEL_AUTO = 0x1,
-	AMD_DPM_FORCED_LEVEL_MANUAL = 0x2,
-	AMD_DPM_FORCED_LEVEL_LOW = 0x4,
-	AMD_DPM_FORCED_LEVEL_HIGH = 0x8,
-	AMD_DPM_FORCED_LEVEL_PROFILE_STANDARD = 0x10,
-	AMD_DPM_FORCED_LEVEL_PROFILE_MIN_SCLK = 0x20,
-	AMD_DPM_FORCED_LEVEL_PROFILE_MIN_MCLK = 0x40,
-	AMD_DPM_FORCED_LEVEL_PROFILE_PEAK = 0x80,
-	AMD_DPM_FORCED_LEVEL_PROFILE_EXIT = 0x100,
+	AMD_DPM_FORCED_LEVEL_LOW = 0x2,
+	AMD_DPM_FORCED_LEVEL_HIGH = 0x4,
+	AMD_DPM_FORCED_LEVEL_PROFILE_STANDARD = 0x8,
+	AMD_DPM_FORCED_LEVEL_PROFILE_MIN_SCLK = 0x10,
+	AMD_DPM_FORCED_LEVEL_PROFILE_MIN_MCLK = 0x20,
+	AMD_DPM_FORCED_LEVEL_PROFILE_PEAK = 0x40,
+	AMD_DPM_FORCED_LEVEL_PROFILE_EXIT = 0x80,
 };
 
 enum amd_pm_state_type {
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
index dec8dd9..60d280c 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
@@ -1250,7 +1250,6 @@ static int cz_dpm_force_dpm_level(struct pp_hwmgr *hwmgr,
 	case AMD_DPM_FORCED_LEVEL_AUTO:
 		ret = cz_phm_unforce_dpm_levels(hwmgr);
 		break;
-	case AMD_DPM_FORCED_LEVEL_MANUAL:
 	case AMD_DPM_FORCED_LEVEL_PROFILE_EXIT:
 	default:
 		break;
@@ -1558,9 +1557,6 @@ static int cz_get_dal_power_level(struct pp_hwmgr *hwmgr,
 static int cz_force_clock_level(struct pp_hwmgr *hwmgr,
 		enum pp_clock_type type, uint32_t mask)
 {
-	if (hwmgr->dpm_level != AMD_DPM_FORCED_LEVEL_MANUAL)
-		return -EINVAL;
-
 	switch (type) {
 	case PP_SCLK:
 		smum_send_msg_to_smc_with_parameter(hwmgr,
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
index 409a56b..eddcbcd 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
@@ -605,7 +605,6 @@ static int rv_dpm_force_dpm_level(struct pp_hwmgr *hwmgr,
 						PPSMC_MSG_SetSoftMaxFclkByFreq,
 						RAVEN_UMD_PSTATE_MIN_FCLK);
 		break;
-	case AMD_DPM_FORCED_LEVEL_MANUAL:
 	case AMD_DPM_FORCED_LEVEL_PROFILE_EXIT:
 	default:
 		break;
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index 13db75c..e3a8374 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -2798,7 +2798,6 @@ static int smu7_force_dpm_level(struct pp_hwmgr *hwmgr,
 		smu7_force_clock_level(hwmgr, PP_MCLK, 1<<mclk_mask);
 		smu7_force_clock_level(hwmgr, PP_PCIE, 1<<pcie_mask);
 		break;
-	case AMD_DPM_FORCED_LEVEL_MANUAL:
 	case AMD_DPM_FORCED_LEVEL_PROFILE_EXIT:
 	default:
 		break;
@@ -4311,11 +4310,6 @@ static int smu7_force_clock_level(struct pp_hwmgr *hwmgr,
 {
 	struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
 
-	if (hwmgr->request_dpm_level & (AMD_DPM_FORCED_LEVEL_AUTO |
-					AMD_DPM_FORCED_LEVEL_LOW |
-					AMD_DPM_FORCED_LEVEL_HIGH))
-		return -EINVAL;
-
 	switch (type) {
 	case PP_SCLK:
 		if (!data->sclk_dpm_key_disabled)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
index 6b28896..828677e 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
@@ -4241,7 +4241,6 @@ static int vega10_dpm_force_dpm_level(struct pp_hwmgr *hwmgr,
 		vega10_force_clock_level(hwmgr, PP_SCLK, 1<<sclk_mask);
 		vega10_force_clock_level(hwmgr, PP_MCLK, 1<<mclk_mask);
 		break;
-	case AMD_DPM_FORCED_LEVEL_MANUAL:
 	case AMD_DPM_FORCED_LEVEL_PROFILE_EXIT:
 	default:
 		break;
@@ -4500,11 +4499,6 @@ static int vega10_force_clock_level(struct pp_hwmgr *hwmgr,
 {
 	struct vega10_hwmgr *data = (struct vega10_hwmgr *)(hwmgr->backend);
 
-	if (hwmgr->request_dpm_level & (AMD_DPM_FORCED_LEVEL_AUTO |
-				AMD_DPM_FORCED_LEVEL_LOW |
-				AMD_DPM_FORCED_LEVEL_HIGH))
-		return -EINVAL;
-
 	switch (type) {
 	case PP_SCLK:
 		data->smc_state_table.gfx_boot_level = mask ? (ffs(mask) - 1) : 0;
-- 
1.9.1

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

[-- Attachment #3: Type: text/plain, Size: 154 bytes --]

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

RE: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

[Zhu, Rex]

> I disagree. the clock range is not independent of the profile. 
> Profiles correspond to use cases. Different use cases have different 
> clock requirements. For compute we want the clocks to be high. For 
> video you may have requirements for minimum mclks to ensure smooth 
> playback. For power saving you may want to limit maximum clocks, etc. 
> So setting the clock range independent of the profile in "auto" mode 
> does not make sense to me.

>>I agree with that.  The clocks should be part of the profile, at least as far as the automatic profile selection in the driver is concerned.
>>The whole point of it is so that the driver can select a profile dynamically for a specific use case without requiring manual interaction from the user.  For manually selecting profiles I 
>>can see Rex's point (you may want independent knobs for testing different parameters), but I think we should be consistent in the interface otherwise it's confusing to users.

Rex: I also agree that clock should be part of the profile.

My point is driver supply independent knobs to user. (pp_dpm_sclk/mclk force_dpm_level for select clock range.)

And I just want to supply a new sysfs that user can configure dpm parameters(smu7: uphyst/downhyst/min_activity vega10: RLC,ent_point.etc).

And as the parameters maybe not easy to understand for users, we supply some modes(VR, COMPUTE ) as example.

So the problem maybe we should not name new sysfs as "power_profile_mode ". 

It is easy to add min sclk/mclk in the new sysfs, I just think it repeated with other sysfs.

Best Regards
Rex

-----Original Message-----
From: Alex Deucher [mailto:alexdeucher@gmail.com] 
Sent: Thursday, January 25, 2018 7:42 AM
To: Kuehling, Felix
Cc: Zhu, Rex; Huang, JinHuiEric; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

On Wed, Jan 24, 2018 at 6:12 PM, Felix Kuehling <felix.kuehling@amd.com> wrote:
> On 2018-01-24 06:05 PM, Zhu, Rex wrote:
>> Hi Felix,
>>
>> The logic of gfx/computer profile setting works in such way as you 
>> said,need under “auto” state.
>> But in new sysfs of power profile setting, we do not check the 
>> performance Level.
>>
>> And The “manual” state is a confusing flag，when user set manual
>> state，and then change the clk range through pp-dpm-sclk/mclk，the dpm 
>> still works automatically in new clock range, I think we can remove 
>> this flag.
>
> But then you're changing the semantics of how pp_dpm_sclk/mclk wok 
> together with pp_dpm_force_performance_level.
>
> Sysfs interfaces should be maintained stable. I'm OK with breaking the 
> old power profile stuff, because it only affects KFD which isn't 
> upstream yet. But the old pp_dpm_sclk/mclk and 
> pp_dpm_force_performance_level affect code that's currently upstream 
> and used by existing tools.
>
>>
>> So  My idea is
>> in the sysfs of pp dpm sclk/mclk, user can set clock range.
>> In the sysfs of power profile state, user can configure the 
>> parameters smu/driver exposed. They are independent.
>
> I disagree. the clock range is not independent of the profile. 
> Profiles correspond to use cases. Different use cases have different 
> clock requirements. For compute we want the clocks to be high. For 
> video you may have requirements for minimum mclks to ensure smooth 
> playback. For power saving you may want to limit maximum clocks, etc. 
> So setting the clock range independent of the profile in "auto" mode 
> does not make sense to me.

I agree with that.  The clocks should be part of the profile, at least as far as the automatic profile selection in the driver is concerned.
The whole point of it is so that the driver can select a profile dynamically for a specific use case without requiring manual interaction from the user.  For manually selecting profiles I can see Rex's point (you may want independent knobs for testing different parameters), but I think we should be consistent in the interface otherwise it's confusing to users.

Alex

>
> Regards,
>   Felix
>
>>
>>
>>
>>
>>
>>
>> Best Regards
>> Rex
>> ---------------------------------------------------------------------
>> ---
>> *From:* Kuehling, Felix
>> *Sent:* Thursday, January 25, 2018 5:41:43 AM
>> *To:* Zhu, Rex; Huang, JinHuiEric; amd-gfx@lists.freedesktop.org
>> *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement 
>> set_power_profile_mode on smu7
>>
>> Hi Rex,
>>
>> As I understand it (the way power profiles currently work), 
>> pp_dpm_sclk/mclk only apply if pp_dpm_force_performance_level is set 
>> to "manual". Power profiles and automatic switching between profiles 
>> only happens when pp_dpm_force_performance_level is set to "auto".
>>
>> This means pp_dpm_sclk/mclk don't apply when profiles are in effect.
>> Also, there would be no way to set different minimum clocks for 
>> different profiles.
>>
>> I think minimum clocks should be part of the profiles.
>>
>> Regards,
>>   Felix
>>
>>
>> On 2018-01-24 03:13 PM, Zhu, Rex wrote:
>> > Hi Eric,
>> >
>> > We have sysfs pp-dpm-sclk/mclk to set min dpm level
>> >
>> > Best Regards
>> > Rex
>> > -------------------------------------------------------------------
>> > -----
>> > *From:* amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf 
>> > of Eric Huang <jinhuieric.huang@amd.com>
>> > *Sent:* Thursday, January 25, 2018 12:04:55 AM
>> > *To:* amd-gfx@lists.freedesktop.org
>> > *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement 
>> > set_power_profile_mode on smu7
>> >
>> > We have min_sclk and min_mclk in previous power profile parameters 
>> > for VI, which are similar with min_active_level for Vega10. How to 
>> > implement these parameters?
>> >
>> > Regards,
>> > Eric
>> >
>> > On 2018-01-24 04:37 AM, Rex Zhu wrote:
>> > > User can set smu7 profile pamameters through sysfs
>> > >
>> > > echo "0/1/2/3/4">pp_power_profile_mode to select 
>> > > 3D_FULL_SCREEN/POWER_SAVING/VIDEO/VR/COMPUTE
>> > > mode.
>> > > echo "5 * * * * * * * *">pp_power_profile_mode to config custom 
>> > > mode.
>> > > "5 * * * * * * * *" mean "CUSTOM enable_sclk SCLK_UP_HYST 
>> > > SCLK_DOWN_HYST SCLK_ACTIVE_LEVEL enable_mclk MCLK_UP_HYST 
>> > > MCLK_DOWN_HYST MCLK_ACTIVE_LEVEL"
>> > >
>> > > Change-Id: Ic6d6f37363bc81ab17051285f6ace847edf725de
>> > > Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
>> > > ---
>> > >   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 49
>> > +++++++++++++++++++++++-
>> > >   1 file changed, 48 insertions(+), 1 deletion(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> > b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> > > index 9f6afd5..13db75c 100644
>> > > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> > > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> > > @@ -5036,7 +5036,54 @@ static int 
>> > > smu7_get_power_profile_mode(struct
>> > pp_hwmgr *hwmgr, char *buf)
>> > >
>> > >   static int smu7_set_power_profile_mode(struct pp_hwmgr *hwmgr,
>> > long *input, uint32_t size)
>> > >   {
>> > > -     /* To Do */
>> > > +     struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
>> > > +     struct profile_mode_setting tmp;
>> > > +
>> > > +     hwmgr->power_profile_mode = input[size];
>> > > +
>> > > +     switch (hwmgr->power_profile_mode) {
>> > > +     case PP_SMC_POWER_PROFILE_CUSTOM:
>> > > +             if (size < 8)
>> > > +                     return -EINVAL;
>> > > +
>> > > +             data->custom_profile_setting.bupdate_sclk = input[0];
>> > > +             data->custom_profile_setting.sclk_up_hyst = input[1];
>> > > +             data->custom_profile_setting.sclk_down_hyst = input[2];
>> > > +             data->custom_profile_setting.sclk_activity =  input[3];
>> > > +             data->custom_profile_setting.bupdate_mclk = input[4];
>> > > +             data->custom_profile_setting.mclk_up_hyst = input[5];
>> > > +             data->custom_profile_setting.mclk_down_hyst = input[6];
>> > > +             data->custom_profile_setting.mclk_activity =  input[7];
>> > > +             if (!smum_update_dpm_settings(hwmgr,
>> > &data->custom_profile_setting))
>> > > +                     memcpy(&data->current_profile_setting,
>> > &data->custom_profile_setting, sizeof(struct 
>> > profile_mode_setting));
>> > > +             break;
>> > > +     case PP_SMC_POWER_PROFILE_FULLSCREEN3D:
>> > > +     case PP_SMC_POWER_PROFILE_POWERSAVING:
>> > > +     case PP_SMC_POWER_PROFILE_VIDEO:
>> > > +     case PP_SMC_POWER_PROFILE_VR:
>> > > +     case PP_SMC_POWER_PROFILE_COMPUTE:
>> > > +             memcpy(&tmp,
>> > &smu7_profiling[hwmgr->power_profile_mode], sizeof(struct 
>> > profile_mode_setting));
>> > > +             if (!smum_update_dpm_settings(hwmgr, &tmp)) {
>> > > +                     if (tmp.bupdate_sclk) {
>> > > +
>> > data->current_profile_setting.bupdate_sclk = tmp.bupdate_sclk;
>> > > +
>> > data->current_profile_setting.sclk_up_hyst = tmp.sclk_up_hyst;
>> > > +
>> > data->current_profile_setting.sclk_down_hyst = tmp.sclk_down_hyst;
>> > > +
>> > data->current_profile_setting.sclk_activity = tmp.sclk_activity;
>> > > +                     }
>> > > +                     if (tmp.bupdate_mclk) {
>> > > +
>> > data->current_profile_setting.bupdate_mclk = tmp.bupdate_mclk;
>> > > +
>> > data->current_profile_setting.mclk_up_hyst = tmp.mclk_up_hyst;
>> > > +
>> > data->current_profile_setting.mclk_down_hyst = tmp.mclk_down_hyst;
>> > > +
>> > data->current_profile_setting.mclk_activity = tmp.mclk_activity;
>> > > +                     }
>> > > +             }
>> > > +             break;
>> > > +     case PP_SMC_POWER_PROFILE_AUTO: /* TO DO auto wattman 
>> > > + feature
>> > not implement */
>> > > +             return 0;
>> > > +     default:
>> > > +             return -EINVAL;
>> > > +     }
>> > > +
>> > >        return 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
>>
>
> _______________________________________________
> 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

Re: [PATCH 1/4] drm/amd/pp: Add new smu callback function

[Eric Huang]

Hi Rex,

Why don't you use function smum_populate_requested_graphic_levels() 
which is doing exactly the same thing as the function you add ?

And in old power profile setting function 
smu7_set_power_profile_state(), we implement this:

if (hwmgr->chip_id == CHIP_FIJI) {
         if (request->type == AMD_PP_GFX_PROFILE)
             smu7_enable_power_containment(hwmgr);
         else if (request->type == AMD_PP_COMPUTE_PROFILE)
             smu7_disable_power_containment(hwmgr);
     }

This should be merged into your new power profile setting.

Regards,
Eric

On 2018-01-24 04:37 AM, Rex Zhu wrote:
> it is used for update dpm settings
>
> Change-Id: Idc0362c219d84564693ca90adf9299e56cfeb6a4
> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
> ---
>   drivers/gpu/drm/amd/powerplay/inc/hwmgr.h     | 1 +
>   drivers/gpu/drm/amd/powerplay/inc/smumgr.h    | 1 +
>   drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c | 8 ++++++++
>   3 files changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> index 604a7cb..3e8959e 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> @@ -238,6 +238,7 @@ struct pp_smumgr_func {
>   	int (*populate_requested_graphic_levels)(struct pp_hwmgr *hwmgr,
>   			struct amd_pp_profile *request);
>   	bool (*is_hw_avfs_present)(struct pp_hwmgr  *hwmgr);
> +	int (*update_dpm_settings)(struct pp_hwmgr *hwmgr, void *profile_setting);
>   };
>   
>   struct pp_hwmgr_func {
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/smumgr.h b/drivers/gpu/drm/amd/powerplay/inc/smumgr.h
> index b1b27b2..e05a57e 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/smumgr.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/smumgr.h
> @@ -134,5 +134,6 @@ extern int smum_populate_requested_graphic_levels(struct pp_hwmgr *hwmgr,
>   
>   extern bool smum_is_hw_avfs_present(struct pp_hwmgr *hwmgr);
>   
> +extern int smum_update_dpm_settings(struct pp_hwmgr *hwmgr, void *profile_setting);
>   
>   #endif
> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c b/drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c
> index 8673884..1ce4959 100644
> --- a/drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c
> @@ -253,3 +253,11 @@ bool smum_is_hw_avfs_present(struct pp_hwmgr *hwmgr)
>   
>   	return false;
>   }
> +
> +int smum_update_dpm_settings(struct pp_hwmgr *hwmgr, void *profile_setting)
> +{
> +	if (hwmgr->smumgr_funcs->update_dpm_settings)
> +		return hwmgr->smumgr_funcs->update_dpm_settings(hwmgr, profile_setting);
> +
> +	return -EINVAL;
> +}

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

Re: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7

[Felix Kuehling]

On 2018-01-25 06:48 AM, Zhu, Rex wrote:
>>> That's a change in the pp_dpm_sclk/mclk interface. Currenly these only work in manual mode and fail or are ignored in auto mode. E.g. see this code in smu7_hwmgr.c:
>>> static int smu7_force_clock_level(struct pp_hwmgr *hwmgr,
> The attached patch can fix this issue.
> Thanks.

How do you unforce the clock levels with your patch? Currently switching
from "manual" back to "auto" will call smu7_unforce_dpm_levels. However,
if you are in "auto" mode all the time, that won't happen. So you're
still breaking the interface.

Regards,
  Felix

>
>
> Best Regards
> Rex
> -----Original Message-----
> From: Kuehling, Felix 
> Sent: Thursday, January 25, 2018 12:16 PM
> To: Zhu, Rex; Huang, JinHuiEric; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 4/4] drm/amd/pp: Implement set_power_profile_mode on smu7
>
> On 2018-01-24 09:28 PM, Zhu, Rex wrote:
>>> But then you're changing the semantics of how pp_dpm_sclk/mclk wok 
>>> together with pp_dpm_force_performance_level.
>> Rex:The two sysfs are all for clock range setting.
>>
>>
>> pp_dpm_sclk/mclk/pcie can set sclk/mclk/pcie range separately.
>>
> Only in manual mode. If you change that, you're changing the interface and breaking existing tools.
>
>> and pp_dpm_force_performance_level, we can support low/high/umd-pstate 
>> that set all the clocks  range.
>>
>> (manual state, driver do nothing except set the flag)
>>
>>
>> No matter what state user enter in, driver can support to continue to 
>> change the clock range through pp_dpm_sclk/mclk/pcie.
>>
>>
>> so in fact, don't need manual state for sepalately set the clock range.
>>
> That's a change in the pp_dpm_sclk/mclk interface. Currenly these only work in manual mode and fail or are ignored in auto mode. E.g. see this code in smu7_hwmgr.c:
>
> static int smu7_force_clock_level(struct pp_hwmgr *hwmgr,
>                 enum pp_clock_type type, uint32_t mask) {
>         struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
>
>         if (hwmgr->request_dpm_level & (AMD_DPM_FORCED_LEVEL_AUTO |
>                                         AMD_DPM_FORCED_LEVEL_LOW |
>                                         AMD_DPM_FORCED_LEVEL_HIGH))
>                 return -EINVAL;
>
> Regards,
>   Felix
>
>
>>> Sysfs interfaces should be maintained stable. I'm OK with breaking 
>>> the old power profile stuff, because it only affects KFD which isn't 
>>> upstream yet.
>>
>> Rex: As the old power profile interface can't support Vega.
>>
>> so add new interface and we are try to support old asics with new sysfs.
>>
>>
>>
>>> But the old pp_dpm_sclk/mclk and
>>> pp_dpm_force_performance_level affect code that's currently upstream 
>>> and used by existing tools.
>>
>> Rex:  Yes, we are trying to maintain the sysfs stable.
>> if we change the old sysfs code,  we will try to not affect existing 
>> tools.
>>
>> Best Regards
>> Rex
>>
>>  
>> ----------------------------------------------------------------------
>> --
>> *From:* amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of 
>> Felix Kuehling <felix.kuehling@amd.com>
>> *Sent:* Thursday, January 25, 2018 7:12 AM
>> *To:* Zhu, Rex; Huang, JinHuiEric; amd-gfx@lists.freedesktop.org
>> *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement 
>> set_power_profile_mode on smu7
>>  
>> On 2018-01-24 06:05 PM, Zhu, Rex wrote:
>>> Hi Felix,
>>>
>>> The logic of gfx/computer profile setting works in such way as you 
>>> said,need under “auto” state.
>>> But in new sysfs of power profile setting, we do not check the
>> performance
>>> Level.
>>>
>>> And The “manual” state is a confusing flag，when user set manual
>>> state，and then change the clk range through pp-dpm-sclk/mclk，the dpm 
>>> still works automatically in new clock range, I think we can remove 
>>> this flag.
>> But then you're changing the semantics of how pp_dpm_sclk/mclk wok 
>> together with pp_dpm_force_performance_level.
>>
>> Sysfs interfaces should be maintained stable. I'm OK with breaking the 
>> old power profile stuff, because it only affects KFD which isn't 
>> upstream yet. But the old pp_dpm_sclk/mclk and 
>> pp_dpm_force_performance_level affect code that's currently upstream 
>> and used by existing tools.
>>
>>> So  My idea is
>>> in the sysfs of pp dpm sclk/mclk, user can set clock range.
>>> In the sysfs of power profile state, user can configure the 
>>> parameters smu/driver exposed. They are independent.
>> I disagree. the clock range is not independent of the profile. 
>> Profiles correspond to use cases. Different use cases have different 
>> clock requirements. For compute we want the clocks to be high. For 
>> video you may have requirements for minimum mclks to ensure smooth 
>> playback. For power saving you may want to limit maximum clocks, etc. 
>> So setting the clock range independent of the profile in "auto" mode 
>> does not make sense to me.
>>
>> Regards,
>>   Felix
>>
>>>
>>>
>>>
>>>
>>>
>>> Best Regards
>>> Rex
>>> --------------------------------------------------------------------
>>> ----
>>> *From:* Kuehling, Felix
>>> *Sent:* Thursday, January 25, 2018 5:41:43 AM
>>> *To:* Zhu, Rex; Huang, JinHuiEric; amd-gfx@lists.freedesktop.org
>>> *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement 
>>> set_power_profile_mode on smu7
>>>  
>>> Hi Rex,
>>>
>>> As I understand it (the way power profiles currently work), 
>>> pp_dpm_sclk/mclk only apply if pp_dpm_force_performance_level is set 
>>> to "manual". Power profiles and automatic switching between profiles 
>>> only happens when pp_dpm_force_performance_level is set to "auto".
>>>
>>> This means pp_dpm_sclk/mclk don't apply when profiles are in effect.
>>> Also, there would be no way to set different minimum clocks for 
>>> different profiles.
>>>
>>> I think minimum clocks should be part of the profiles.
>>>
>>> Regards,
>>>   Felix
>>>
>>>
>>> On 2018-01-24 03:13 PM, Zhu, Rex wrote:
>>>> Hi Eric,
>>>>
>>>> We have sysfs pp-dpm-sclk/mclk to set min dpm level
>>>>
>>>> Best Regards
>>>> Rex
>>>>
>> ----------------------------------------------------------------------
>> --
>>>> *From:* amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf 
>>>> of Eric Huang <jinhuieric.huang@amd.com>
>>>> *Sent:* Thursday, January 25, 2018 12:04:55 AM
>>>> *To:* amd-gfx@lists.freedesktop.org
>>>> *Subject:* Re: [PATCH 4/4] drm/amd/pp: Implement 
>>>> set_power_profile_mode on smu7
>>>>  
>>>> We have min_sclk and min_mclk in previous power profile parameters 
>>>> for VI, which are similar with min_active_level for Vega10. How to
>> implement
>>>> these parameters?
>>>>
>>>> Regards,
>>>> Eric
>>>>
>>>> On 2018-01-24 04:37 AM, Rex Zhu wrote:
>>>>> User can set smu7 profile pamameters through sysfs
>>>>>
>>>>> echo "0/1/2/3/4">pp_power_profile_mode to select 
>>>>> 3D_FULL_SCREEN/POWER_SAVING/VIDEO/VR/COMPUTE
>>>>> mode.
>>>>> echo "5 * * * * * * * *">pp_power_profile_mode to config custom 
>>>>> mode.
>>>>> "5 * * * * * * * *" mean "CUSTOM enable_sclk SCLK_UP_HYST 
>>>>> SCLK_DOWN_HYST SCLK_ACTIVE_LEVEL enable_mclk MCLK_UP_HYST 
>>>>> MCLK_DOWN_HYST MCLK_ACTIVE_LEVEL"
>>>>>
>>>>> Change-Id: Ic6d6f37363bc81ab17051285f6ace847edf725de
>>>>> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
>>>>> ---
>>>>>    drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 49
>>>> +++++++++++++++++++++++-
>>>>>    1 file changed, 48 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>>>> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>>>>> index 9f6afd5..13db75c 100644
>>>>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>>>>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>>>>> @@ -5036,7 +5036,54 @@ static int 
>>>>> smu7_get_power_profile_mode(struct
>>>> pp_hwmgr *hwmgr, char *buf)
>>>>>   
>>>>>    static int smu7_set_power_profile_mode(struct pp_hwmgr *hwmgr,
>>>> long *input, uint32_t size)
>>>>>    {
>>>>> -     /* To Do */
>>>>> +     struct smu7_hwmgr *data = (struct smu7_hwmgr
>> *)(hwmgr->backend);
>>>>> +     struct profile_mode_setting tmp;
>>>>> +
>>>>> +     hwmgr->power_profile_mode = input[size];
>>>>> +
>>>>> +     switch (hwmgr->power_profile_mode) {
>>>>> +     case PP_SMC_POWER_PROFILE_CUSTOM:
>>>>> +             if (size < 8)
>>>>> +                     return -EINVAL;
>>>>> +
>>>>> +             data->custom_profile_setting.bupdate_sclk = 
>>>>> +input[0];
>>>>> +             data->custom_profile_setting.sclk_up_hyst = 
>>>>> +input[1];
>>>>> +             data->custom_profile_setting.sclk_down_hyst =
>> input[2];
>>>>> +             data->custom_profile_setting.sclk_activity =
>> input[3];
>>>>> +             data->custom_profile_setting.bupdate_mclk = 
>>>>> +input[4];
>>>>> +             data->custom_profile_setting.mclk_up_hyst = 
>>>>> +input[5];
>>>>> +             data->custom_profile_setting.mclk_down_hyst =
>> input[6];
>>>>> +             data->custom_profile_setting.mclk_activity =
>> input[7];
>>>>> +             if (!smum_update_dpm_settings(hwmgr,
>>>> &data->custom_profile_setting))
>>>>> +                     memcpy(&data->current_profile_setting,
>>>> &data->custom_profile_setting, sizeof(struct 
>>>> profile_mode_setting));
>>>>> +             break;
>>>>> +     case PP_SMC_POWER_PROFILE_FULLSCREEN3D:
>>>>> +     case PP_SMC_POWER_PROFILE_POWERSAVING:
>>>>> +     case PP_SMC_POWER_PROFILE_VIDEO:
>>>>> +     case PP_SMC_POWER_PROFILE_VR:
>>>>> +     case PP_SMC_POWER_PROFILE_COMPUTE:
>>>>> +             memcpy(&tmp,
>>>> &smu7_profiling[hwmgr->power_profile_mode], sizeof(struct 
>>>> profile_mode_setting));
>>>>> +             if (!smum_update_dpm_settings(hwmgr, &tmp)) {
>>>>> +                     if (tmp.bupdate_sclk) {
>>>>> +                            
>>>> data->current_profile_setting.bupdate_sclk = tmp.bupdate_sclk;
>>>>> +                            
>>>> data->current_profile_setting.sclk_up_hyst = tmp.sclk_up_hyst;
>>>>> +                            
>>>> data->current_profile_setting.sclk_down_hyst = tmp.sclk_down_hyst;
>>>>> +                            
>>>> data->current_profile_setting.sclk_activity = tmp.sclk_activity;
>>>>> +                     }
>>>>> +                     if (tmp.bupdate_mclk) {
>>>>> +                            
>>>> data->current_profile_setting.bupdate_mclk = tmp.bupdate_mclk;
>>>>> +                            
>>>> data->current_profile_setting.mclk_up_hyst = tmp.mclk_up_hyst;
>>>>> +                            
>>>> data->current_profile_setting.mclk_down_hyst = tmp.mclk_down_hyst;
>>>>> +                            
>>>> data->current_profile_setting.mclk_activity = tmp.mclk_activity;
>>>>> +                     }
>>>>> +             }
>>>>> +             break;
>>>>> +     case PP_SMC_POWER_PROFILE_AUTO: /* TO DO auto wattman 
>>>>> +feature
>>>> not implement */
>>>>> +             return 0;
>>>>> +     default:
>>>>> +             return -EINVAL;
>>>>> +     }
>>>>> +
>>>>>         return 0;
>>>>>    }
>>>>>   
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> amd-gfx Info Page - freedesktop.org
>> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
>> lists.freedesktop.org
>> Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the 
>> following form. Use of all freedesktop.org lists is subject to our 
>> Code of ...
>>
>>
>>
>>>>
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> amd-gfx Info Page - freedesktop.org
>> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
>> lists.freedesktop.org
>> Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the 
>> following form. Use of all freedesktop.org lists is subject to our 
>> Code of ...
>>
>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> amd-gfx Info Page - freedesktop.org
>> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
>> lists.freedesktop.org
>> Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the 
>> following form. Use of all freedesktop.org lists is subject to our 
>> Code of ...
>>
>>
>>

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

Re: [PATCH 1/4] drm/amd/pp: Add new smu callback function

[Zhu, Rex]

[-- Attachment #1.1: Type: text/plain, Size: 3559 bytes --]

For the power containment issue on Fiji.
We want to expose set power limit function.
Maybe we can adjust the power limit instand of just disable this feature.
But I am not sure this can meet kfd ‘ s requirement.

Best Regards
Rex
________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Eric Huang <jinhuieric.huang-5C7GfCeVMHo@public.gmane.org>
Sent: Friday, January 26, 2018 12:18:45 AM
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH 1/4] drm/amd/pp: Add new smu callback function

Hi Rex,

Why don't you use function smum_populate_requested_graphic_levels()
which is doing exactly the same thing as the function you add ?

And in old power profile setting function
smu7_set_power_profile_state(), we implement this:

if (hwmgr->chip_id == CHIP_FIJI) {
         if (request->type == AMD_PP_GFX_PROFILE)
             smu7_enable_power_containment(hwmgr);
         else if (request->type == AMD_PP_COMPUTE_PROFILE)
             smu7_disable_power_containment(hwmgr);
     }

This should be merged into your new power profile setting.

Regards,
Eric

On 2018-01-24 04:37 AM, Rex Zhu wrote:
> it is used for update dpm settings
>
> Change-Id: Idc0362c219d84564693ca90adf9299e56cfeb6a4
> Signed-off-by: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
> ---
>   drivers/gpu/drm/amd/powerplay/inc/hwmgr.h     | 1 +
>   drivers/gpu/drm/amd/powerplay/inc/smumgr.h    | 1 +
>   drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c | 8 ++++++++
>   3 files changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> index 604a7cb..3e8959e 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> @@ -238,6 +238,7 @@ struct pp_smumgr_func {
>        int (*populate_requested_graphic_levels)(struct pp_hwmgr *hwmgr,
>                        struct amd_pp_profile *request);
>        bool (*is_hw_avfs_present)(struct pp_hwmgr  *hwmgr);
> +     int (*update_dpm_settings)(struct pp_hwmgr *hwmgr, void *profile_setting);
>   };
>
>   struct pp_hwmgr_func {
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/smumgr.h b/drivers/gpu/drm/amd/powerplay/inc/smumgr.h
> index b1b27b2..e05a57e 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/smumgr.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/smumgr.h
> @@ -134,5 +134,6 @@ extern int smum_populate_requested_graphic_levels(struct pp_hwmgr *hwmgr,
>
>   extern bool smum_is_hw_avfs_present(struct pp_hwmgr *hwmgr);
>
> +extern int smum_update_dpm_settings(struct pp_hwmgr *hwmgr, void *profile_setting);
>
>   #endif
> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c b/drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c
> index 8673884..1ce4959 100644
> --- a/drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c
> @@ -253,3 +253,11 @@ bool smum_is_hw_avfs_present(struct pp_hwmgr *hwmgr)
>
>        return false;
>   }
> +
> +int smum_update_dpm_settings(struct pp_hwmgr *hwmgr, void *profile_setting)
> +{
> +     if (hwmgr->smumgr_funcs->update_dpm_settings)
> +             return hwmgr->smumgr_funcs->update_dpm_settings(hwmgr, profile_setting);
> +
> +     return -EINVAL;
> +}

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

[-- Attachment #1.2: Type: text/html, Size: 5457 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [PATCH 1/4] drm/amd/pp: Add new smu callback function

[Eric Huang]

[-- Attachment #1.1: Type: text/plain, Size: 4124 bytes --]

Power containment has two parts, TDC limit and power limit. Only 
changing power limit is not equal to disabling power containment.

Power containment in Fiji degrades compute performance. So we disable it 
to fix. It has been verified by compute performance test. If you want to 
replace it, it has to be verified, otherwise it will make a regression.

Regards,
Eric


On 2018-01-25 06:15 PM, Zhu, Rex wrote:
> For the power containment issue on Fiji.
> We want to expose set power limit function.
> Maybe we can adjust the power limit instand of just disable this feature.
> But I am not sure this can meet kfd ‘ s requirement.
>
> Best Regards
> Rex
> ------------------------------------------------------------------------
> *From:* amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of 
> Eric Huang <jinhuieric.huang-5C7GfCeVMHo@public.gmane.org>
> *Sent:* Friday, January 26, 2018 12:18:45 AM
> *To:* amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> *Subject:* Re: [PATCH 1/4] drm/amd/pp: Add new smu callback function
> Hi Rex,
>
> Why don't you use function smum_populate_requested_graphic_levels()
> which is doing exactly the same thing as the function you add ?
>
> And in old power profile setting function
> smu7_set_power_profile_state(), we implement this:
>
> if (hwmgr->chip_id == CHIP_FIJI) {
>          if (request->type == AMD_PP_GFX_PROFILE)
>              smu7_enable_power_containment(hwmgr);
>          else if (request->type == AMD_PP_COMPUTE_PROFILE)
>              smu7_disable_power_containment(hwmgr);
>      }
>
> This should be merged into your new power profile setting.
>
> Regards,
> Eric
>
> On 2018-01-24 04:37 AM, Rex Zhu wrote:
> > it is used for update dpm settings
> >
> > Change-Id: Idc0362c219d84564693ca90adf9299e56cfeb6a4
> > Signed-off-by: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
> > ---
> >   drivers/gpu/drm/amd/powerplay/inc/hwmgr.h     | 1 +
> >   drivers/gpu/drm/amd/powerplay/inc/smumgr.h    | 1 +
> >   drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c | 8 ++++++++
> >   3 files changed, 10 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h 
> b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > index 604a7cb..3e8959e 100644
> > --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > @@ -238,6 +238,7 @@ struct pp_smumgr_func {
> >        int (*populate_requested_graphic_levels)(struct pp_hwmgr *hwmgr,
> >                        struct amd_pp_profile *request);
> >        bool (*is_hw_avfs_present)(struct pp_hwmgr *hwmgr);
> > +     int (*update_dpm_settings)(struct pp_hwmgr *hwmgr, void 
> *profile_setting);
> >   };
> >
> >   struct pp_hwmgr_func {
> > diff --git a/drivers/gpu/drm/amd/powerplay/inc/smumgr.h 
> b/drivers/gpu/drm/amd/powerplay/inc/smumgr.h
> > index b1b27b2..e05a57e 100644
> > --- a/drivers/gpu/drm/amd/powerplay/inc/smumgr.h
> > +++ b/drivers/gpu/drm/amd/powerplay/inc/smumgr.h
> > @@ -134,5 +134,6 @@ extern int 
> smum_populate_requested_graphic_levels(struct pp_hwmgr *hwmgr,
> >
> >   extern bool smum_is_hw_avfs_present(struct pp_hwmgr *hwmgr);
> >
> > +extern int smum_update_dpm_settings(struct pp_hwmgr *hwmgr, void 
> *profile_setting);
> >
> >   #endif
> > diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c 
> b/drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c
> > index 8673884..1ce4959 100644
> > --- a/drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c
> > +++ b/drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c
> > @@ -253,3 +253,11 @@ bool smum_is_hw_avfs_present(struct pp_hwmgr 
> *hwmgr)
> >
> >        return false;
> >   }
> > +
> > +int smum_update_dpm_settings(struct pp_hwmgr *hwmgr, void 
> *profile_setting)
> > +{
> > +     if (hwmgr->smumgr_funcs->update_dpm_settings)
> > +             return hwmgr->smumgr_funcs->update_dpm_settings(hwmgr, 
> profile_setting);
> > +
> > +     return -EINVAL;
> > +}
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[-- Attachment #1.2: Type: text/html, Size: 7509 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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