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

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

 



[AMD Official Use Only]

It seems this patch should be combined with patch1. Rather than as a separate patch.

BR
Evan
> -----Original Message-----
> From: Powell, Darren <Darren.Powell@xxxxxxx>
> Sent: Saturday, January 22, 2022 11:47 AM
> To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Powell, Darren <Darren.Powell@xxxxxxx>
> Subject: [PATCH 3/3] amdgpu/pm: Add Error Handling to emit_clocks stack
> 
>  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@xxxxxxx>
> ---
>  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




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

  Powered by Linux