[PATCH] drm/amd/powerplay: correct smu_update_table usage

Quan, Evan Evan.Quan at amd.com
Tue Jul 16 03:01:04 UTC 2019


Ping..

> -----Original Message-----
> From: Evan Quan <evan.quan at amd.com>
> Sent: Friday, July 12, 2019 3:36 PM
> To: amd-gfx at lists.freedesktop.org
> Cc: Quan, Evan <Evan.Quan at amd.com>
> Subject: [PATCH] drm/amd/powerplay: correct smu_update_table usage
> 
> The interface was used in a confusing way. In profile mode scenario, the 2nd
> parameter of the interface was used in a different way from other scenarios.
> 
> Change-Id: Iabcebb47db8fdf242580c1059393132ee10b93e4
> Signed-off-by: Evan Quan <evan.quan at amd.com>
> ---
>  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c    |  4 ++--
>  .../gpu/drm/amd/powerplay/inc/amdgpu_smu.h    |  2 +-
>  drivers/gpu/drm/amd/powerplay/navi10_ppt.c    | 16 +++++++--------
>  drivers/gpu/drm/amd/powerplay/smu_v11_0.c     |  4 ++--
>  drivers/gpu/drm/amd/powerplay/vega20_ppt.c    | 20 +++++++++----------
>  5 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 7015fe1011e8..32f98cd4e4f2 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -299,7 +299,7 @@ int smu_common_read_sensor(struct smu_context
> *smu, enum amd_pp_sensors sensor,
>  	return ret;
>  }
> 
> -int smu_update_table(struct smu_context *smu, enum smu_table_id
> table_index,
> +int smu_update_table(struct smu_context *smu, enum smu_table_id
> +table_index, int argument,
>  		     void *table_data, bool drv2smu)
>  {
>  	struct smu_table_context *smu_table = &smu->smu_table; @@ -
> 326,7 +326,7 @@ int smu_update_table(struct smu_context *smu, enum
> smu_table_id table_index,
>  	ret = smu_send_smc_msg_with_param(smu, drv2smu ?
> 
> SMU_MSG_TransferTableDram2Smu :
> 
> SMU_MSG_TransferTableSmu2Dram,
> -					  table_id);
> +					  table_id | ((argument & 0xFFFF) <<
> 16));
>  	if (ret)
>  		return ret;
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> index c97324ef7db2..baf1ce4d1a14 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> @@ -937,7 +937,7 @@ extern int smu_feature_is_supported(struct
> smu_context *smu,  extern int smu_feature_set_supported(struct
> smu_context *smu,
>  				     enum smu_feature_mask mask, bool
> enable);
> 
> -int smu_update_table(struct smu_context *smu, uint32_t table_index,
> +int smu_update_table(struct smu_context *smu, enum smu_table_id
> +table_index, int argument,
>  		     void *table_data, bool drv2smu);
> 
>  bool is_support_sw_smu(struct amdgpu_device *adev); diff --git
> a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> index 4cb0c18b12ce..655ad6c66b21 100644
> --- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> @@ -634,7 +634,7 @@ static int
> navi10_get_current_clk_freq_by_table(struct smu_context *smu,
> 
>  	memset(&metrics, 0, sizeof(metrics));
> 
> -	ret = smu_update_table(smu, SMU_TABLE_SMU_METRICS, (void
> *)&metrics, false);
> +	ret = smu_update_table(smu, SMU_TABLE_SMU_METRICS, 0, (void
> +*)&metrics, false);
>  	if (ret)
>  		return ret;
> 
> @@ -888,7 +888,7 @@ static int navi10_get_gpu_power(struct smu_context
> *smu, uint32_t *value)
>  	if (!value)
>  		return -EINVAL;
> 
> -	ret = smu_update_table(smu, SMU_TABLE_SMU_METRICS, (void
> *)&metrics,
> +	ret = smu_update_table(smu, SMU_TABLE_SMU_METRICS, 0, (void
> +*)&metrics,
>  			       false);
>  	if (ret)
>  		return ret;
> @@ -910,7 +910,7 @@ static int navi10_get_current_activity_percent(struct
> smu_context *smu,
> 
>  	msleep(1);
> 
> -	ret = smu_update_table(smu, SMU_TABLE_SMU_METRICS,
> +	ret = smu_update_table(smu, SMU_TABLE_SMU_METRICS, 0,
>  			       (void *)&metrics, false);
>  	if (ret)
>  		return ret;
> @@ -951,7 +951,7 @@ static int navi10_get_fan_speed(struct smu_context
> *smu, uint16_t *value)
> 
>  	memset(&metrics, 0, sizeof(metrics));
> 
> -	ret = smu_update_table(smu, SMU_TABLE_SMU_METRICS,
> +	ret = smu_update_table(smu, SMU_TABLE_SMU_METRICS, 0,
>  			       (void *)&metrics, false);
>  	if (ret)
>  		return ret;
> @@ -1020,7 +1020,7 @@ static int navi10_get_power_profile_mode(struct
> smu_context *smu, char *buf)
>  			return -EINVAL;
> 
>  		result = smu_update_table(smu,
> -
> SMU_TABLE_ACTIVITY_MONITOR_COEFF | workload_type << 16,
> +
> SMU_TABLE_ACTIVITY_MONITOR_COEFF, workload_type,
>  					  (void *)(&activity_monitor), false);
>  		if (result) {
>  			pr_err("[%s] Failed to get activity monitor!",
> __func__); @@ -1093,7 +1093,7 @@ static int
> navi10_set_power_profile_mode(struct smu_context *smu, long *input, u
>  			return -EINVAL;
> 
>  		ret = smu_update_table(smu,
> -				       SMU_TABLE_ACTIVITY_MONITOR_COEFF
> | WORKLOAD_PPLIB_CUSTOM_BIT << 16,
> +				       SMU_TABLE_ACTIVITY_MONITOR_COEFF,
> WORKLOAD_PPLIB_CUSTOM_BIT,
>  				       (void *)(&activity_monitor), false);
>  		if (ret) {
>  			pr_err("[%s] Failed to get activity monitor!",
> __func__); @@ -1137,7 +1137,7 @@ static int
> navi10_set_power_profile_mode(struct smu_context *smu, long *input, u
>  		}
> 
>  		ret = smu_update_table(smu,
> -				       SMU_TABLE_ACTIVITY_MONITOR_COEFF
> | WORKLOAD_PPLIB_CUSTOM_BIT << 16,
> +				       SMU_TABLE_ACTIVITY_MONITOR_COEFF,
> WORKLOAD_PPLIB_CUSTOM_BIT,
>  				       (void *)(&activity_monitor), true);
>  		if (ret) {
>  			pr_err("[%s] Failed to set activity monitor!",
> __func__); @@ -1305,7 +1305,7 @@ static int
> navi10_thermal_get_temperature(struct smu_context *smu,
>  	if (!value)
>  		return -EINVAL;
> 
> -	ret = smu_update_table(smu, SMU_TABLE_SMU_METRICS, (void
> *)&metrics, false);
> +	ret = smu_update_table(smu, SMU_TABLE_SMU_METRICS, 0, (void
> +*)&metrics, false);
>  	if (ret)
>  		return ret;
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index 12b7f763dddd..bd9a6f80699d 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -704,7 +704,7 @@ static int smu_v11_0_write_pptable(struct
> smu_context *smu)
>  	struct smu_table_context *table_context = &smu->smu_table;
>  	int ret = 0;
> 
> -	ret = smu_update_table(smu, SMU_TABLE_PPTABLE,
> +	ret = smu_update_table(smu, SMU_TABLE_PPTABLE, 0,
>  			       table_context->driver_pptable, true);
> 
>  	return ret;
> @@ -723,7 +723,7 @@ static int smu_v11_0_write_watermarks_table(struct
> smu_context *smu)
>  	if (!table->cpu_addr)
>  		return -EINVAL;
> 
> -	ret = smu_update_table(smu, SMU_TABLE_WATERMARKS, table-
> >cpu_addr,
> +	ret = smu_update_table(smu, SMU_TABLE_WATERMARKS, 0, table-
> >cpu_addr,
>  				true);
> 
>  	return ret;
> diff --git a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
> b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
> index dbc17920b879..c0c0565b56cd 100644
> --- a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
> @@ -1699,7 +1699,7 @@ static int vega20_get_metrics_table(struct
> smu_context *smu,
>  	int ret = 0;
> 
>  	if (!smu_table->metrics_time || time_after(jiffies, smu_table-
> >metrics_time + HZ / 1000)) {
> -		ret = smu_update_table(smu, SMU_TABLE_SMU_METRICS,
> +		ret = smu_update_table(smu, SMU_TABLE_SMU_METRICS,
> 0,
>  				(void *)smu_table->metrics_table, false);
>  		if (ret) {
>  			pr_info("Failed to export SMU metrics table!\n");
> @@ -1728,7 +1728,7 @@ static int vega20_set_default_od_settings(struct
> smu_context *smu,
>  		if (!table_context->overdrive_table)
>  			return -ENOMEM;
> 
> -		ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE,
> +		ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE, 0,
>  				       table_context->overdrive_table, false);
>  		if (ret) {
>  			pr_err("Failed to export over drive table!\n"); @@ -
> 1740,7 +1740,7 @@ static int vega20_set_default_od_settings(struct
> smu_context *smu,
>  			return ret;
>  	}
> 
> -	ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE,
> +	ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE, 0,
>  			       table_context->overdrive_table, true);
>  	if (ret) {
>  		pr_err("Failed to import over drive table!\n"); @@ -1827,7
> +1827,7 @@ static int vega20_get_power_profile_mode(struct smu_context
> *smu, char *buf)
>  			return -EINVAL;
> 
>  		result = smu_update_table(smu,
> -
> SMU_TABLE_ACTIVITY_MONITOR_COEFF | workload_type << 16,
> +
> SMU_TABLE_ACTIVITY_MONITOR_COEFF, workload_type,
>  					  (void *)(&activity_monitor), false);
>  		if (result) {
>  			pr_err("[%s] Failed to get activity monitor!",
> __func__); @@ -1913,7 +1913,7 @@ static int
> vega20_set_power_profile_mode(struct smu_context *smu, long *input, u
> 
>  	if (smu->power_profile_mode ==
> PP_SMC_POWER_PROFILE_CUSTOM) {
>  		ret = smu_update_table(smu,
> -				       SMU_TABLE_ACTIVITY_MONITOR_COEFF
> | WORKLOAD_PPLIB_CUSTOM_BIT << 16,
> +				       SMU_TABLE_ACTIVITY_MONITOR_COEFF,
> WORKLOAD_PPLIB_CUSTOM_BIT,
>  				       (void *)(&activity_monitor), false);
>  		if (ret) {
>  			pr_err("[%s] Failed to get activity monitor!",
> __func__); @@ -1968,7 +1968,7 @@ static int
> vega20_set_power_profile_mode(struct smu_context *smu, long *input, u
>  		}
> 
>  		ret = smu_update_table(smu,
> -				       SMU_TABLE_ACTIVITY_MONITOR_COEFF
> | WORKLOAD_PPLIB_CUSTOM_BIT << 16,
> +				       SMU_TABLE_ACTIVITY_MONITOR_COEFF,
> WORKLOAD_PPLIB_CUSTOM_BIT,
>  				       (void *)(&activity_monitor), true);
>  		if (ret) {
>  			pr_err("[%s] Failed to set activity monitor!",
> __func__); @@ -2519,7 +2519,7 @@ static int
> vega20_update_od8_settings(struct smu_context *smu,
>  	struct smu_table_context *table_context = &smu->smu_table;
>  	int ret;
> 
> -	ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE,
> +	ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE, 0,
>  			       table_context->overdrive_table, false);
>  	if (ret) {
>  		pr_err("Failed to export over drive table!\n"); @@ -2530,7
> +2530,7 @@ static int vega20_update_od8_settings(struct smu_context
> *smu,
>  	if (ret)
>  		return ret;
> 
> -	ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE,
> +	ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE, 0,
>  			       table_context->overdrive_table, true);
>  	if (ret) {
>  		pr_err("Failed to import over drive table!\n"); @@ -2794,7
> +2794,7 @@ static int vega20_odn_edit_dpm_table(struct smu_context
> *smu,
>  		break;
> 
>  	case PP_OD_RESTORE_DEFAULT_TABLE:
> -		ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE,
> table_context->overdrive_table, false);
> +		ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE, 0,
> +table_context->overdrive_table, false);
>  		if (ret) {
>  			pr_err("Failed to export over drive table!\n");
>  			return ret;
> @@ -2803,7 +2803,7 @@ static int vega20_odn_edit_dpm_table(struct
> smu_context *smu,
>  		break;
> 
>  	case PP_OD_COMMIT_DPM_TABLE:
> -		ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE,
> table_context->overdrive_table, true);
> +		ret = smu_update_table(smu, SMU_TABLE_OVERDRIVE, 0,
> +table_context->overdrive_table, true);
>  		if (ret) {
>  			pr_err("Failed to import over drive table!\n");
>  			return ret;
> --
> 2.21.0



More information about the amd-gfx mailing list