[PATCH 3/3] amdgpu/pm: Add Error Handling to emit_clocks stack

Darren Powell darren.powell at amd.com
Sat Jan 22 03:46:47 UTC 2022


 Previous implementation of print_clocks required return of bytes written
 to calling function via return value. Passing this value in by reference
 allows us now to pass back error codes up the calling stack.

 (v1)
 - Errors from get_current_clk_freq, get_dpm_level_count & get_dpm_freq
      now passed back up the stack
 - Added -EOPNOTSUPP when encountering for OD clocks
      !od_enabled
      missing od_table or od_settings
 - OD_RANGE continues to print any subset of the ODCAP settings it can find
      without reporting error for any missing
 - smu_emit_ppclk returns ENOENT if emit_clk_levels is not supported by the
      device
 - modified calling logic so fallback to print_clock_levels is only attempted
      if emit_clk_levels is not (yet) supported by the device

 (v2)
 - change return from amdgpu_dpm_emit_clock_levels if emit_clock_levels not found
 - updated documentation of pptable_func members print_clk_levels, emit_clk_levels

 == Test ==
 LOGFILE=pp_clk.test.log
 AMDGPU_PCI_ADDR=`lspci -nn | grep "VGA\|Display" | cut -d " " -f 1`
 AMDGPU_HWMON=`ls -la /sys/class/hwmon | grep $AMDGPU_PCI_ADDR | awk '{print $9}'`
 HWMON_DIR=/sys/class/hwmon/${AMDGPU_HWMON}

 lspci -nn | grep "VGA\|Display"  > $LOGFILE
 FILES="pp_od_clk_voltage
 pp_dpm_sclk
 pp_dpm_mclk
 pp_dpm_pcie
 pp_dpm_socclk
 pp_dpm_fclk
 pp_dpm_dcefclk
 pp_dpm_vclk
 pp_dpm_dclk "

 for f in $FILES
 do
   echo === $f === >> $LOGFILE
   cat $HWMON_DIR/device/$f >> $LOGFILE
 done
 cat $LOGFILE

Signed-off-by: Darren Powell <darren.powell at amd.com>
---
 drivers/gpu/drm/amd/pm/amdgpu_dpm.c           |  2 +-
 drivers/gpu/drm/amd/pm/amdgpu_pm.c            | 13 ++++++------
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     |  8 +++----
 drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h |  6 +++++-
 .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   | 21 +++++++++----------
 5 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
index e45578124928..03a690a3abb7 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
@@ -915,7 +915,7 @@ int amdgpu_dpm_emit_clock_levels(struct amdgpu_device *adev,
 	int ret = 0;
 
 	if (!pp_funcs->emit_clock_levels)
-		return 0;
+		return -ENOENT;
 
 	mutex_lock(&adev->pm.mutex);
 	ret = pp_funcs->emit_clock_levels(adev->powerplay.pp_handle,
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 97a8dcbe6eaf..a11def0ee761 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -855,13 +855,12 @@ static ssize_t amdgpu_get_pp_od_clk_voltage(struct device *dev,
 		return ret;
 	}
 
-	ret = amdgpu_dpm_emit_clock_levels(adev, od_clocks[0], buf, &size);
-	if (ret >= 0) {
-		/* Proceed with emit for other od clocks if the first call succeeds */
-		for(clk_index = 1 ; clk_index < 6 ; clk_index++)
-			amdgpu_dpm_emit_clock_levels(adev, od_clocks[clk_index], buf, &size);
+	for(clk_index = 0 ; clk_index < 6 ; clk_index++) {
+		ret = amdgpu_dpm_emit_clock_levels(adev, od_clocks[clk_index], buf, &size);
+		if (ret)
+			break;
 	}
-	else {
+	if (ret == -ENOENT) {
 		size = amdgpu_dpm_print_clock_levels(adev, OD_SCLK, buf);
 		if (size > 0) {
 			size += amdgpu_dpm_print_clock_levels(adev, OD_MCLK, buf+size);
@@ -1014,7 +1013,7 @@ static ssize_t amdgpu_get_pp_dpm_clock(struct device *dev,
 	}
 
 	ret = amdgpu_dpm_emit_clock_levels(adev, type, buf, &size);
-	if (ret < 0)
+	if (ret == -ENOENT)
 		size = amdgpu_dpm_print_clock_levels(adev, type, buf);
 
 	if (size == 0)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index d82ea7ee223f..29c101a93dc4 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2454,12 +2454,10 @@ static int smu_emit_ppclk_levels(void *handle, enum pp_clock_type type, char *bu
 	if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
 		return -EOPNOTSUPP;
 
-	if (smu->ppt_funcs->emit_clk_levels) {
+	if (smu->ppt_funcs->emit_clk_levels)
 		ret = smu->ppt_funcs->emit_clk_levels(smu, clk_type, buf, offset);
-	}
-	else {
-		ret = -EOPNOTSUPP;
-	}
+	else
+		ret = -ENOENT;
 
 	return ret;
 
diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
index 224b869eda70..1429581dca9c 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
@@ -612,6 +612,7 @@ struct pptable_funcs {
 	 *                    to buffer. Star current level.
 	 *
 	 * Used for sysfs interfaces.
+	 * Return: Number of characters written to the buffer
 	 */
 	int (*print_clk_levels)(struct smu_context *smu, enum smu_clk_type clk_type, char *buf);
 
@@ -621,7 +622,10 @@ struct pptable_funcs {
 	 *
 	 * Used for sysfs interfaces.
 	 * &buf: sysfs buffer
-	 * &offset: offset within buffer to start printing
+	 * &offset: offset within buffer to start printing, which is updated by the
+	 * function.
+	 *
+	 * Return: 0 on Success or Negative to indicate an error occurred.
 	 */
 	int (*emit_clk_levels)(struct smu_context *smu, enum smu_clk_type clk_type, char *buf, int *offset);
 
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index 50022de5337f..4bcef7d1a0d6 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1278,7 +1278,6 @@ static int navi10_emit_clk_levels(struct smu_context *smu,
 		(OverDriveTable_t *)table_context->overdrive_table;
 	struct smu_11_0_overdrive_table *od_settings = smu->od_settings;
 	uint32_t min_value, max_value;
-	int save_offset = *offset;
 
 	switch (clk_type) {
 	case SMU_GFXCLK:
@@ -1292,17 +1291,17 @@ static int navi10_emit_clk_levels(struct smu_context *smu,
 	case SMU_DCEFCLK:
 		ret = navi10_get_current_clk_freq_by_table(smu, clk_type, &cur_value);
 		if (ret)
-			return 0;
+			return ret;
 
 		ret = smu_v11_0_get_dpm_level_count(smu, clk_type, &count);
 		if (ret)
-			return 0;
+			return ret;
 
 		if (!navi10_is_support_fine_grained_dpm(smu, clk_type)) {
 			for (i = 0; i < count; i++) {
 				ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, i, &value);
 				if (ret)
-					return (*offset - save_offset);
+					return ret;
 
 				*offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n", i, value,
 						cur_value == value ? "*" : "");
@@ -1311,10 +1310,10 @@ static int navi10_emit_clk_levels(struct smu_context *smu,
 		} else {
 			ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_values[0]);
 			if (ret)
-				return 0;
+				return ret;
 			ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, count - 1, &freq_values[2]);
 			if (ret)
-				return 0;
+				return ret;
 
 			freq_values[1] = cur_value;
 			mark_index = cur_value == freq_values[0] ? 0 :
@@ -1355,7 +1354,7 @@ static int navi10_emit_clk_levels(struct smu_context *smu,
 		break;
 	case SMU_OD_SCLK:
 		if (!smu->od_enabled || !od_table || !od_settings)
-			break;
+			return -EOPNOTSUPP;
 		if (!navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_GFXCLK_LIMITS))
 			break;
 		*offset += sysfs_emit_at(buf, *offset, "OD_SCLK:\n0: %uMhz\n1: %uMhz\n",
@@ -1363,14 +1362,14 @@ static int navi10_emit_clk_levels(struct smu_context *smu,
 		break;
 	case SMU_OD_MCLK:
 		if (!smu->od_enabled || !od_table || !od_settings)
-			break;
+			return -EOPNOTSUPP;
 		if (!navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_UCLK_MAX))
 			break;
 		*offset += sysfs_emit_at(buf, *offset, "OD_MCLK:\n1: %uMHz\n", od_table->UclkFmax);
 		break;
 	case SMU_OD_VDDC_CURVE:
 		if (!smu->od_enabled || !od_table || !od_settings)
-			break;
+			return -EOPNOTSUPP;
 		if (!navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_GFXCLK_CURVE))
 			break;
 		*offset += sysfs_emit_at(buf, *offset, "OD_VDDC_CURVE:\n");
@@ -1395,7 +1394,7 @@ static int navi10_emit_clk_levels(struct smu_context *smu,
 		break;
 	case SMU_OD_RANGE:
 		if (!smu->od_enabled || !od_table || !od_settings)
-			break;
+			return -EOPNOTSUPP;
 		*offset += sysfs_emit_at(buf, *offset, "%s:\n", "OD_RANGE");
 
 		if (navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_GFXCLK_LIMITS)) {
@@ -1446,7 +1445,7 @@ static int navi10_emit_clk_levels(struct smu_context *smu,
 		break;
 	}
 
-	return (*offset - save_offset);
+	return 0;
 }
 
 static int navi10_print_clk_levels(struct smu_context *smu,
-- 
2.34.1



More information about the amd-gfx mailing list