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
