RE: [PATCH] drm/amdgpu/smu_v11: Unify and fix power limits

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

 



If smu_get_pptable_power_limit() is designed to be used internally, the second argument "lock_needed" can be dropped.
Except that, the patch is reviewed-by: Evan Quan <evan.quan@xxxxxxx>

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Matt
> Coffin
> Sent: Saturday, November 9, 2019 7:54 AM
> To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Matt Coffin <mcoffin13@xxxxxxxxx>
> Subject: [PATCH] drm/amdgpu/smu_v11: Unify and fix power limits
> 
> [Why]
> On Navi10, and presumably arcterus, updating pp_table via sysfs would
> not re-scale the maximum possible power limit one can set. On navi10,
> the SMU code ignored the power percentage overdrive setting entirely,
> and would not allow you to exceed the default power limit at all.
> 
> [How]
> Adding a function to the SMU interface to get the pptable version of the
> default power limit allows ASIC-specific code to provide the correct
> maximum-settable power limit for the current pptable.
> ---
>  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c    | 18 ++++++++-
>  drivers/gpu/drm/amd/powerplay/arcturus_ppt.c  | 22 +++++-----
>  .../gpu/drm/amd/powerplay/inc/amdgpu_smu.h    |  4 +-
>  drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h |  2 +
>  .../drm/amd/powerplay/inc/smu_v11_0_pptable.h |  2 +
>  drivers/gpu/drm/amd/powerplay/navi10_ppt.c    | 22 +++++-----
>  drivers/gpu/drm/amd/powerplay/smu_v11_0.c     | 40 +++++++++++++++++--
>  drivers/gpu/drm/amd/powerplay/vega20_ppt.c    |  1 -
>  8 files changed, 83 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 66faea66a8e9..6bf940f1edfb 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -1107,7 +1107,7 @@ static int smu_smc_table_hw_init(struct
> smu_context *smu,
>  		if (ret)
>  			return ret;
> 
> -		ret = smu_get_power_limit(smu, &smu->default_power_limit,
> true, false);
> +		ret = smu_get_power_limit(smu, &smu->default_power_limit,
> false, false);
>  		if (ret)
>  			return ret;
>  	}
> @@ -2509,3 +2509,19 @@ int smu_get_dpm_clock_table(struct smu_context
> *smu,
> 
>  	return ret;
>  }
> +
> +uint32_t smu_get_pptable_power_limit(struct smu_context *smu, bool
> lock_needed)
> +{
> +	uint32_t ret = 0;
> +
> +	if (lock_needed)
> +		mutex_lock(&smu->mutex);
> +
> +	if (smu->ppt_funcs->get_pptable_power_limit)
> +		ret = smu->ppt_funcs->get_pptable_power_limit(smu);
> +
> +	if (lock_needed)
> +		mutex_unlock(&smu->mutex);
> +
> +	return ret;
> +}
> diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> index 3099ac256bd3..ebb9c8064867 100644
> --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> @@ -1261,15 +1261,14 @@ arcturus_get_profiling_clk_mask(struct
> smu_context *smu,
> 
>  static int arcturus_get_power_limit(struct smu_context *smu,
>  				     uint32_t *limit,
> -				     bool asic_default)
> +				     bool cap)
>  {
>  	PPTable_t *pptable = smu->smu_table.driver_pptable;
>  	uint32_t asic_default_power_limit = 0;
>  	int ret = 0;
>  	int power_src;
> 
> -	if (!smu->default_power_limit ||
> -	    !smu->power_limit) {
> +	if (!smu->power_limit) {
>  		if (smu_feature_is_enabled(smu, SMU_FEATURE_PPT_BIT)) {
>  			power_src = smu_power_get_index(smu,
> SMU_POWER_SOURCE_AC);
>  			if (power_src < 0)
> @@ -1292,17 +1291,11 @@ static int arcturus_get_power_limit(struct
> smu_context *smu,
>  				pptable-
> >SocketPowerLimitAc[PPT_THROTTLER_PPT0];
>  		}
> 
> -		if (smu->od_enabled) {
> -			asic_default_power_limit *= (100 + smu-
> >smu_table.TDPODLimit);
> -			asic_default_power_limit /= 100;
> -		}
> -
> -		smu->default_power_limit = asic_default_power_limit;
>  		smu->power_limit = asic_default_power_limit;
>  	}
> 
> -	if (asic_default)
> -		*limit = smu->default_power_limit;
> +	if (cap)
> +		*limit = smu_v11_0_get_max_power_limit(smu);
>  	else
>  		*limit = smu->power_limit;
> 
> @@ -2070,6 +2063,12 @@ static void arcturus_i2c_eeprom_control_fini(struct
> i2c_adapter *control)
>  	i2c_del_adapter(control);
>  }
> 
> +static uint32_t arcterus_get_pptable_power_limit(struct smu_context *smu)
> +{
> +	PPTable_t *pptable = smu->smu_table.driver_pptable;
> +	return pptable->SocketPowerLimitAc[PPT_THROTTLER_PPT0];
> +}
> +
>  static const struct pptable_funcs arcturus_ppt_funcs = {
>  	/* translate smu index into arcturus specific index */
>  	.get_smu_msg_index = arcturus_get_smu_msg_index,
> @@ -2160,6 +2159,7 @@ static const struct pptable_funcs arcturus_ppt_funcs
> = {
>  	.get_dpm_ultimate_freq = smu_v11_0_get_dpm_ultimate_freq,
>  	.set_soft_freq_limited_range =
> smu_v11_0_set_soft_freq_limited_range,
>  	.override_pcie_parameters = smu_v11_0_override_pcie_parameters,
> +	.get_pptable_power_limit = arcterus_get_pptable_power_limit,
>  };
> 
>  void arcturus_set_ppt_funcs(struct smu_context *smu)
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> index 8120e7587585..9d0193569b05 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> @@ -261,7 +261,6 @@ struct smu_table_context
>  	struct smu_table		*tables;
>  	struct smu_table		memory_pool;
>  	uint8_t                         thermal_controller_type;
> -	uint16_t			TDPODLimit;
> 
>  	void				*overdrive_table;
>  };
> @@ -548,6 +547,7 @@ struct pptable_funcs {
>  	int (*get_dpm_ultimate_freq)(struct smu_context *smu, enum
> smu_clk_type clk_type, uint32_t *min, uint32_t *max);
>  	int (*set_soft_freq_limited_range)(struct smu_context *smu, enum
> smu_clk_type clk_type, uint32_t min, uint32_t max);
>  	int (*override_pcie_parameters)(struct smu_context *smu);
> +	uint32_t (*get_pptable_power_limit)(struct smu_context *smu);
>  };
> 
>  int smu_load_microcode(struct smu_context *smu);
> @@ -717,4 +717,6 @@ int smu_get_uclk_dpm_states(struct smu_context *smu,
>  int smu_get_dpm_clock_table(struct smu_context *smu,
>  			    struct dpm_clocks *clock_table);
> 
> +uint32_t smu_get_pptable_power_limit(struct smu_context *smu, bool
> lock_needed);
> +
>  #endif
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> index 154b57a4dbbb..0ba7a7292bea 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> @@ -252,4 +252,6 @@ int smu_v11_0_override_pcie_parameters(struct
> smu_context *smu);
> 
>  int smu_v11_0_set_default_od_settings(struct smu_context *smu, bool
> initialize, size_t overdrive_table_size);
> 
> +uint32_t smu_v11_0_get_max_power_limit(struct smu_context *smu);
> +
>  #endif
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0_pptable.h
> b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0_pptable.h
> index 86cdc3393eac..b2f96a101124 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0_pptable.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0_pptable.h
> @@ -141,7 +141,9 @@ struct smu_11_0_powerplay_table
>        struct smu_11_0_power_saving_clock_table      power_saving_clock;
>        struct smu_11_0_overdrive_table               overdrive_table;
> 
> +#ifndef SMU_11_0_PARTIAL_PPTABLE
>        PPTable_t smc_pptable;                        //PPTable_t in smu11_driver_if.h
> +#endif
>  } __attribute__((packed));
> 
>  #endif
> diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> index 4fbdf0e507f3..36cf313754e4 100644
> --- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> @@ -1633,17 +1633,22 @@ static int
> navi10_display_disable_memory_clock_switch(struct smu_context *smu,
>  	return ret;
>  }
> 
> +static uint32_t navi10_get_pptable_power_limit(struct smu_context *smu)
> +{
> +	PPTable_t *pptable = smu->smu_table.driver_pptable;
> +	return pptable->SocketPowerLimitAc[PPT_THROTTLER_PPT0];
> +}
> +
>  static int navi10_get_power_limit(struct smu_context *smu,
>  				     uint32_t *limit,
> -				     bool asic_default)
> +				     bool cap)
>  {
>  	PPTable_t *pptable = smu->smu_table.driver_pptable;
>  	uint32_t asic_default_power_limit = 0;
>  	int ret = 0;
>  	int power_src;
> 
> -	if (!smu->default_power_limit ||
> -	    !smu->power_limit) {
> +	if (!smu->power_limit) {
>  		if (smu_feature_is_enabled(smu, SMU_FEATURE_PPT_BIT)) {
>  			power_src = smu_power_get_index(smu,
> SMU_POWER_SOURCE_AC);
>  			if (power_src < 0)
> @@ -1666,17 +1671,11 @@ static int navi10_get_power_limit(struct
> smu_context *smu,
>  				pptable-
> >SocketPowerLimitAc[PPT_THROTTLER_PPT0];
>  		}
> 
> -		if (smu->od_enabled) {
> -			asic_default_power_limit *= (100 + smu-
> >smu_table.TDPODLimit);
> -			asic_default_power_limit /= 100;
> -		}
> -
> -		smu->default_power_limit = asic_default_power_limit;
>  		smu->power_limit = asic_default_power_limit;
>  	}
> 
> -	if (asic_default)
> -		*limit = smu->default_power_limit;
> +	if (cap)
> +		*limit = smu_v11_0_get_max_power_limit(smu);
>  	else
>  		*limit = smu->power_limit;
> 
> @@ -2024,6 +2023,7 @@ static const struct pptable_funcs navi10_ppt_funcs =
> {
>  	.override_pcie_parameters = smu_v11_0_override_pcie_parameters,
>  	.set_default_od_settings = navi10_set_default_od_settings,
>  	.od_edit_dpm_table = navi10_od_edit_dpm_table,
> +	.get_pptable_power_limit = navi10_get_pptable_power_limit,
>  };
> 
>  void navi10_set_ppt_funcs(struct smu_context *smu)
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index c4f67ee8840e..814d2256ff8e 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -24,6 +24,8 @@
>  #include <linux/module.h>
>  #include <linux/pci.h>
> 
> +#define SMU_11_0_PARTIAL_PPTABLE
> +
>  #include "pp_debug.h"
>  #include "amdgpu.h"
>  #include "amdgpu_smu.h"
> @@ -31,6 +33,7 @@
>  #include "atomfirmware.h"
>  #include "amdgpu_atomfirmware.h"
>  #include "smu_v11_0.h"
> +#include "smu_v11_0_pptable.h"
>  #include "soc15_common.h"
>  #include "atom.h"
>  #include "amd_pcie.h"
> @@ -1045,13 +1048,44 @@ int smu_v11_0_init_max_sustainable_clocks(struct
> smu_context *smu)
>  	return 0;
>  }
> 
> +uint32_t smu_v11_0_get_max_power_limit(struct smu_context *smu) {
> +	uint32_t od_limit, max_power_limit;
> +	struct smu_11_0_powerplay_table *powerplay_table = NULL;
> +	struct smu_table_context *table_context = &smu->smu_table;
> +	powerplay_table = table_context->power_play_table;
> +
> +	max_power_limit = smu_get_pptable_power_limit(smu, false);
> +
> +	if (!max_power_limit) {
> +		// If we couldn't get the table limit, fall back on first-read value
> +		if (!smu->default_power_limit)
> +			smu->default_power_limit = smu->power_limit;
> +		max_power_limit = smu->default_power_limit;
> +	}
> +
> +	if (smu->od_enabled) {
> +		od_limit = le32_to_cpu(powerplay_table-
> >overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
> +
> +		pr_debug("ODSETTING_POWERPERCENTAGE: %d
> (default: %d)\n", od_limit, smu->default_power_limit);
> +
> +		max_power_limit *= (100 + od_limit);
> +		max_power_limit /= 100;
> +	}
> +
> +	return max_power_limit;
> +}
> +
>  int smu_v11_0_set_power_limit(struct smu_context *smu, uint32_t n)
>  {
>  	int ret = 0;
> +	uint32_t max_power_limit;
> +
> +	max_power_limit = smu_v11_0_get_max_power_limit(smu);
> 
> -	if (n > smu->default_power_limit) {
> -		pr_err("New power limit is over the max allowed %d\n",
> -				smu->default_power_limit);
> +	if (n > max_power_limit) {
> +		pr_err("New power limit (%d) is over the max allowed %d\n",
> +				n,
> +				max_power_limit);
>  		return -EINVAL;
>  	}
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
> b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
> index 5b21386f558d..0b4892833808 100644
> --- a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
> @@ -466,7 +466,6 @@ static int vega20_store_powerplay_table(struct
> smu_context *smu)
>  	       sizeof(PPTable_t));
> 
>  	table_context->thermal_controller_type = powerplay_table-
> >ucThermalControllerType;
> -	table_context->TDPODLimit = le32_to_cpu(powerplay_table-
> >OverDrive8Table.ODSettingsMax[ATOM_VEGA20_ODSETTING_POWERPERCE
> NTAGE]);
> 
>  	return 0;
>  }
> --
> 2.23.0
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




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

  Powered by Linux