RE: [PATCH] drm/amdgpu/powerplay: fix sysfs_emit/sysfs_emit_at handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[AMD Official Use Only]

Yes, I missed such conversions in powerplay. Thanks!

Reviewed-by: Lang Yu <lang.yu@xxxxxxx> 

>-----Original Message-----
>From: Deucher, Alexander <Alexander.Deucher@xxxxxxx>
>Sent: Thursday, November 4, 2021 8:59 AM
>To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
>Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Yu, Lang
><Lang.Yu@xxxxxxx>; Powell, Darren <Darren.Powell@xxxxxxx>
>Subject: [PATCH] drm/amdgpu/powerplay: fix sysfs_emit/sysfs_emit_at handling
>
>sysfs_emit and sysfs_emit_at requrie a page boundary aligned buf address. Make
>them happy!
>
>v2: fix sysfs_emit -> sysfs_emit_at missed conversions
>
>Cc: Lang Yu <lang.yu@xxxxxxx>
>Cc: Darren Powell <darren.powell@xxxxxxx>
>Fixes: 6db0c87a0a8e ("amdgpu/pm: Replace hwmgr smu usage of sprintf with
>sysfs_emit")
>Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1774
>Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
>---
> .../gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c   |  8 ++++++--
> .../gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c    | 10 +++++++---
> .../gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c    |  2 ++
> .../gpu/drm/amd/pm/powerplay/hwmgr/smu_helper.h    | 13 +++++++++++++
> .../gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c  | 12 +++++++++---
>  .../gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c  |  4
>++++  .../gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c  | 14
>++++++++++----
> 7 files changed, 51 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
>b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
>index 1de3ae77e03e..258c573acc97 100644
>--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
>+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
>@@ -1024,6 +1024,8 @@ static int smu10_print_clock_levels(struct pp_hwmgr
>*hwmgr,
> 	uint32_t min_freq, max_freq = 0;
> 	uint32_t ret = 0;
>
>+	phm_get_sysfs_buf(&buf, &size);
>+
> 	switch (type) {
> 	case PP_SCLK:
> 		smum_send_msg_to_smc(hwmgr,
>PPSMC_MSG_GetGfxclkFrequency, &now); @@ -1065,7 +1067,7 @@ static int
>smu10_print_clock_levels(struct pp_hwmgr *hwmgr,
> 			if (ret)
> 				return ret;
>
>-			size = sysfs_emit(buf, "%s:\n", "OD_SCLK");
>+			size += sysfs_emit_at(buf, size, "%s:\n", "OD_SCLK");
> 			size += sysfs_emit_at(buf, size, "0: %10uMhz\n",
> 			(data->gfx_actual_soft_min_freq > 0) ? data-
>>gfx_actual_soft_min_freq : min_freq);
> 			size += sysfs_emit_at(buf, size, "1: %10uMhz\n", @@ -
>1081,7 +1083,7 @@ static int smu10_print_clock_levels(struct pp_hwmgr
>*hwmgr,
> 			if (ret)
> 				return ret;
>
>-			size = sysfs_emit(buf, "%s:\n", "OD_RANGE");
>+			size += sysfs_emit_at(buf, size, "%s:\n", "OD_RANGE");
> 			size += sysfs_emit_at(buf, size,
>"SCLK: %7uMHz %10uMHz\n",
> 				min_freq, max_freq);
> 		}
>@@ -1456,6 +1458,8 @@ static int smu10_get_power_profile_mode(struct
>pp_hwmgr *hwmgr, char *buf)
> 	if (!buf)
> 		return -EINVAL;
>
>+	phm_get_sysfs_buf(&buf, &size);
>+
> 	size += sysfs_emit_at(buf, size, "%s %16s %s %s %s %s\n",title[0],
> 			title[1], title[2], title[3], title[4], title[5]);
>
>diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
>b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
>index e7803ce8f67a..aceebf584225 100644
>--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
>+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
>@@ -4914,6 +4914,8 @@ static int smu7_print_clock_levels(struct pp_hwmgr
>*hwmgr,
> 	int size = 0;
> 	uint32_t i, now, clock, pcie_speed;
>
>+	phm_get_sysfs_buf(&buf, &size);
>+
> 	switch (type) {
> 	case PP_SCLK:
> 		smum_send_msg_to_smc(hwmgr,
>PPSMC_MSG_API_GetSclkFrequency, &clock); @@ -4963,7 +4965,7 @@ static int
>smu7_print_clock_levels(struct pp_hwmgr *hwmgr,
> 		break;
> 	case OD_SCLK:
> 		if (hwmgr->od_enabled) {
>-			size = sysfs_emit(buf, "%s:\n", "OD_SCLK");
>+			size += sysfs_emit_at(buf, size, "%s:\n", "OD_SCLK");
> 			for (i = 0; i < odn_sclk_table->num_of_pl; i++)
> 				size += sysfs_emit_at(buf, size,
>"%d: %10uMHz %10umV\n",
> 					i, odn_sclk_table->entries[i].clock/100,
>@@ -4972,7 +4974,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr
>*hwmgr,
> 		break;
> 	case OD_MCLK:
> 		if (hwmgr->od_enabled) {
>-			size = sysfs_emit(buf, "%s:\n", "OD_MCLK");
>+			size += sysfs_emit_at(buf, size, "%s:\n", "OD_MCLK");
> 			for (i = 0; i < odn_mclk_table->num_of_pl; i++)
> 				size += sysfs_emit_at(buf, size,
>"%d: %10uMHz %10umV\n",
> 					i, odn_mclk_table->entries[i].clock/100,
>@@ -4981,7 +4983,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr
>*hwmgr,
> 		break;
> 	case OD_RANGE:
> 		if (hwmgr->od_enabled) {
>-			size = sysfs_emit(buf, "%s:\n", "OD_RANGE");
>+			size += sysfs_emit_at(buf, size, "%s:\n", "OD_RANGE");
> 			size += sysfs_emit_at(buf, size,
>"SCLK: %7uMHz %10uMHz\n",
> 				data-
>>golden_dpm_table.sclk_table.dpm_levels[0].value/100,
> 				hwmgr-
>>platform_descriptor.overdriveLimit.engineClock/100);
>@@ -5518,6 +5520,8 @@ static int smu7_get_power_profile_mode(struct
>pp_hwmgr *hwmgr, char *buf)
> 	if (!buf)
> 		return -EINVAL;
>
>+	phm_get_sysfs_buf(&buf, &size);
>+
> 	size += sysfs_emit_at(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]); diff --git
>a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
>b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
>index b94a77e4e714..8e28a8eecefc 100644
>--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
>+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
>@@ -1550,6 +1550,8 @@ static int smu8_print_clock_levels(struct pp_hwmgr
>*hwmgr,
> 	uint32_t i, now;
> 	int size = 0;
>
>+	phm_get_sysfs_buf(&buf, &size);
>+
> 	switch (type) {
> 	case PP_SCLK:
> 		now = PHM_GET_FIELD(cgs_read_ind_register(hwmgr->device,
>diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu_helper.h
>b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu_helper.h
>index ad33983a8064..2a75da1e9f03 100644
>--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu_helper.h
>+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu_helper.h
>@@ -109,6 +109,19 @@ int phm_irq_process(struct amdgpu_device *adev,
> 			   struct amdgpu_irq_src *source,
> 			   struct amdgpu_iv_entry *entry);
>
>+/*
>+ * Helper function to make sysfs_emit_at() happy. Align buf to
>+ * the current page boundary and record the offset.
>+ */
>+static inline void phm_get_sysfs_buf(char **buf, int *offset) {
>+	if (!*buf || !offset)
>+		return;
>+
>+	*offset = offset_in_page(*buf);
>+	*buf -= *offset;
>+}
>+
> int smu9_register_irq_handlers(struct pp_hwmgr *hwmgr);
>
> void *smu_atom_get_data_table(void *dev, uint32_t table, uint16_t *size, diff --
>git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
>b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
>index c152a61ddd2c..c981fc2882f0 100644
>--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
>+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
>@@ -4548,6 +4548,8 @@ static int vega10_get_ppfeature_status(struct
>pp_hwmgr *hwmgr, char *buf)
> 	int ret = 0;
> 	int size = 0;
>
>+	phm_get_sysfs_buf(&buf, &size);
>+
> 	ret = vega10_get_enabled_smc_features(hwmgr, &features_enabled);
> 	PP_ASSERT_WITH_CODE(!ret,
> 			"[EnableAllSmuFeatures] Failed to get enabled smc
>features!", @@ -4637,6 +4639,8 @@ static int vega10_print_clock_levels(struct
>pp_hwmgr *hwmgr,
>
> 	int i, now, size = 0, count = 0;
>
>+	phm_get_sysfs_buf(&buf, &size);
>+
> 	switch (type) {
> 	case PP_SCLK:
> 		if (data->registry_data.sclk_dpm_key_disabled)
>@@ -4717,7 +4721,7 @@ static int vega10_print_clock_levels(struct pp_hwmgr
>*hwmgr,
>
> 	case OD_SCLK:
> 		if (hwmgr->od_enabled) {
>-			size = sysfs_emit(buf, "%s:\n", "OD_SCLK");
>+			size += sysfs_emit_at(buf, size, "%s:\n", "OD_SCLK");
> 			podn_vdd_dep = &data-
>>odn_dpm_table.vdd_dep_on_sclk;
> 			for (i = 0; i < podn_vdd_dep->count; i++)
> 				size += sysfs_emit_at(buf, size,
>"%d: %10uMhz %10umV\n", @@ -4727,7 +4731,7 @@ static int
>vega10_print_clock_levels(struct pp_hwmgr *hwmgr,
> 		break;
> 	case OD_MCLK:
> 		if (hwmgr->od_enabled) {
>-			size = sysfs_emit(buf, "%s:\n", "OD_MCLK");
>+			size += sysfs_emit_at(buf, size, "%s:\n", "OD_MCLK");
> 			podn_vdd_dep = &data-
>>odn_dpm_table.vdd_dep_on_mclk;
> 			for (i = 0; i < podn_vdd_dep->count; i++)
> 				size += sysfs_emit_at(buf, size,
>"%d: %10uMhz %10umV\n", @@ -4737,7 +4741,7 @@ static int
>vega10_print_clock_levels(struct pp_hwmgr *hwmgr,
> 		break;
> 	case OD_RANGE:
> 		if (hwmgr->od_enabled) {
>-			size = sysfs_emit(buf, "%s:\n", "OD_RANGE");
>+			size += sysfs_emit_at(buf, size, "%s:\n", "OD_RANGE");
> 			size += sysfs_emit_at(buf, size,
>"SCLK: %7uMHz %10uMHz\n",
> 				data-
>>golden_dpm_table.gfx_table.dpm_levels[0].value/100,
> 				hwmgr-
>>platform_descriptor.overdriveLimit.engineClock/100);
>@@ -5112,6 +5116,8 @@ static int vega10_get_power_profile_mode(struct
>pp_hwmgr *hwmgr, char *buf)
> 	if (!buf)
> 		return -EINVAL;
>
>+	phm_get_sysfs_buf(&buf, &size);
>+
> 	size += sysfs_emit_at(buf, size, "%s %16s %s %s %s %s\n",title[0],
> 			title[1], title[2], title[3], title[4], title[5]);
>
>diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
>b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
>index 8558718e15a8..f7e783e1c888 100644
>--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
>+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
>@@ -2141,6 +2141,8 @@ static int vega12_get_ppfeature_status(struct
>pp_hwmgr *hwmgr, char *buf)
> 	int ret = 0;
> 	int size = 0;
>
>+	phm_get_sysfs_buf(&buf, &size);
>+
> 	ret = vega12_get_enabled_smc_features(hwmgr, &features_enabled);
> 	PP_ASSERT_WITH_CODE(!ret,
> 		"[EnableAllSmuFeatures] Failed to get enabled smc features!",
>@@ -2244,6 +2246,8 @@ static int vega12_print_clock_levels(struct pp_hwmgr
>*hwmgr,
> 	int i, now, size = 0;
> 	struct pp_clock_levels_with_latency clocks;
>
>+	phm_get_sysfs_buf(&buf, &size);
>+
> 	switch (type) {
> 	case PP_SCLK:
> 		PP_ASSERT_WITH_CODE(
>diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c
>b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c
>index 0cf39c1244b1..03e63be4ee27 100644
>--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c
>+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c
>@@ -3238,6 +3238,8 @@ static int vega20_get_ppfeature_status(struct
>pp_hwmgr *hwmgr, char *buf)
> 	int ret = 0;
> 	int size = 0;
>
>+	phm_get_sysfs_buf(&buf, &size);
>+
> 	ret = vega20_get_enabled_smc_features(hwmgr, &features_enabled);
> 	PP_ASSERT_WITH_CODE(!ret,
> 			"[EnableAllSmuFeatures] Failed to get enabled smc
>features!", @@ -3364,6 +3366,8 @@ static int vega20_print_clock_levels(struct
>pp_hwmgr *hwmgr,
> 	int ret = 0;
> 	uint32_t gen_speed, lane_width, current_gen_speed, current_lane_width;
>
>+	phm_get_sysfs_buf(&buf, &size);
>+
> 	switch (type) {
> 	case PP_SCLK:
> 		ret = vega20_get_current_clk_freq(hwmgr, PPCLK_GFXCLK,
>&now); @@ -3479,7 +3483,7 @@ static int vega20_print_clock_levels(struct
>pp_hwmgr *hwmgr,
> 	case OD_SCLK:
> 		if (od8_settings[OD8_SETTING_GFXCLK_FMIN].feature_id &&
> 		    od8_settings[OD8_SETTING_GFXCLK_FMAX].feature_id) {
>-			size = sysfs_emit(buf, "%s:\n", "OD_SCLK");
>+			size += sysfs_emit_at(buf, size, "%s:\n", "OD_SCLK");
> 			size += sysfs_emit_at(buf, size, "0: %10uMhz\n",
> 				od_table->GfxclkFmin);
> 			size += sysfs_emit_at(buf, size, "1: %10uMhz\n", @@ -
>3489,7 +3493,7 @@ static int vega20_print_clock_levels(struct pp_hwmgr
>*hwmgr,
>
> 	case OD_MCLK:
> 		if (od8_settings[OD8_SETTING_UCLK_FMAX].feature_id) {
>-			size = sysfs_emit(buf, "%s:\n", "OD_MCLK");
>+			size += sysfs_emit_at(buf, size, "%s:\n", "OD_MCLK");
> 			size += sysfs_emit_at(buf, size, "1: %10uMhz\n",
> 				od_table->UclkFmax);
> 		}
>@@ -3503,7 +3507,7 @@ static int vega20_print_clock_levels(struct pp_hwmgr
>*hwmgr,
> 		    od8_settings[OD8_SETTING_GFXCLK_VOLTAGE1].feature_id
>&&
> 		    od8_settings[OD8_SETTING_GFXCLK_VOLTAGE2].feature_id
>&&
> 		    od8_settings[OD8_SETTING_GFXCLK_VOLTAGE3].feature_id) {
>-			size = sysfs_emit(buf, "%s:\n", "OD_VDDC_CURVE");
>+			size += sysfs_emit_at(buf, size, "%s:\n",
>"OD_VDDC_CURVE");
> 			size += sysfs_emit_at(buf, size, "0: %10uMhz %10dmV\n",
> 				od_table->GfxclkFreq1,
> 				od_table->GfxclkVolt1 / VOLTAGE_SCALE); @@
>-3518,7 +3522,7 @@ static int vega20_print_clock_levels(struct pp_hwmgr
>*hwmgr,
> 		break;
>
> 	case OD_RANGE:
>-		size = sysfs_emit(buf, "%s:\n", "OD_RANGE");
>+		size += sysfs_emit_at(buf, size, "%s:\n", "OD_RANGE");
>
> 		if (od8_settings[OD8_SETTING_GFXCLK_FMIN].feature_id &&
> 		    od8_settings[OD8_SETTING_GFXCLK_FMAX].feature_id) { @@
>-4003,6 +4007,8 @@ static int vega20_get_power_profile_mode(struct
>pp_hwmgr *hwmgr, char *buf)
> 	if (!buf)
> 		return -EINVAL;
>
>+	phm_get_sysfs_buf(&buf, &size);
>+
> 	size += sysfs_emit_at(buf, size, "%16s %s %s %s %s %s %s %s %s %s %s\n",
> 			title[0], title[1], title[2], title[3], title[4], title[5],
> 			title[6], title[7], title[8], title[9], title[10]);
>--
>2.31.1




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux