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

Darren Powell darren.powell at amd.com
Wed Dec 8 06:36:09 UTC 2021


 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.

 - 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

 == 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_pm.c            | 19 ++++++++---------
 drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h       | 10 ++++++++-
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     |  9 ++++----
 .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   | 21 +++++++++----------
 4 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 04992a3bc1d2..741e991baaca 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -927,17 +927,16 @@ static ssize_t amdgpu_get_pp_od_clk_voltage(struct device *dev,
 		return ret;
 	}
 
-	ret = -EOPNOTSUPP;
+	ret = -ENOENT;
 	if (adev->powerplay.pp_funcs->emit_clock_levels) {
-		ret = amdgpu_dpm_emit_clock_levels(adev, od_clocks[0], 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;
+		}
 	}
 
-	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);
-	}
-	else {
+	if (ret == -ENOENT) {
 		if (adev->powerplay.pp_funcs->print_clock_levels) {
 			size = amdgpu_dpm_print_clock_levels(adev, OD_SCLK, buf);
 			size += amdgpu_dpm_print_clock_levels(adev, OD_MCLK, buf+size);
@@ -1092,12 +1091,12 @@ static ssize_t amdgpu_get_pp_dpm_clock(struct device *dev,
 		return ret;
 	}
 
-	ret = -EOPNOTSUPP;
+	ret = -ENOENT;
 	if (adev->powerplay.pp_funcs->emit_clock_levels) {
 		ret = amdgpu_dpm_emit_clock_levels(adev, type, buf, &size);
 	}
 
-	if (ret < 0) {
+	if (ret == -ENOENT) {
 		if (adev->powerplay.pp_funcs->print_clock_levels) {
 			size = amdgpu_dpm_print_clock_levels(adev, type, buf);
 		}
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index dc1325783c10..7db6bec158d9 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -617,6 +617,11 @@ struct pptable_funcs {
 	 *                    to buffer. Star current level.
 	 *
 	 * Used for sysfs interfaces.
+	 * Return: Number of characters written to the buffer
+	 *
+	 * On Success the function will return 0 or a positive value to
+	 * record the number of characters written to the buffer. A negative
+	 * value is returned on error.
 	 */
 	int (*print_clk_levels)(struct smu_context *smu, enum smu_clk_type clk_type, char *buf);
 
@@ -626,7 +631,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/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index b0638d8308d2..6dab167928b0 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2464,16 +2464,15 @@ 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;
 
-	mutex_lock(&smu->mutex);
 
 	if (smu->ppt_funcs->emit_clk_levels) {
+		mutex_lock(&smu->mutex);
 		ret = smu->ppt_funcs->emit_clk_levels(smu, clk_type, buf, offset);
+		mutex_unlock(&smu->mutex);
 	}
-	else {
-		ret = -EOPNOTSUPP;
-	}
+	else
+		ret = -ENOENT;
 
-	mutex_unlock(&smu->mutex);
 
 	return ret;
 
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 e43c7e950a55..a9ba1cb35ddf 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