[PATCH 1/2] drm/amd/powerplay: correct SW SMU valid mapping check

Quan, Evan Evan.Quan at amd.com
Tue Jul 16 03:00:48 UTC 2019


Ping..

> -----Original Message-----
> From: Evan Quan <evan.quan at amd.com>
> Sent: Friday, July 12, 2019 3:35 PM
> To: amd-gfx at lists.freedesktop.org
> Cc: Quan, Evan <Evan.Quan at amd.com>
> Subject: [PATCH 1/2] drm/amd/powerplay: correct SW SMU valid mapping
> check
> 
> Current implementation is not actually able to detect invalid
> message/table/workload mapping.
> 
> Change-Id: I66588f20dc2c39dfeb8aefb66757812589eab812
> Signed-off-by: Evan Quan <evan.quan at amd.com>
> ---
>  .../gpu/drm/amd/include/kgd_pp_interface.h    |  1 +
>  drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h | 15 ++--
>  drivers/gpu/drm/amd/powerplay/navi10_ppt.c    | 68 +++++++++++--------
>  drivers/gpu/drm/amd/powerplay/vega20_ppt.c    | 67 ++++++++++--------
>  4 files changed, 86 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> index 9f661bf96ed0..9733bbf9bc72 100644
> --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> @@ -141,6 +141,7 @@ enum PP_SMC_POWER_PROFILE {
>  	PP_SMC_POWER_PROFILE_VR           = 0x4,
>  	PP_SMC_POWER_PROFILE_COMPUTE      = 0x5,
>  	PP_SMC_POWER_PROFILE_CUSTOM       = 0x6,
> +	PP_SMC_POWER_PROFILE_COUNT,
>  };
> 
>  enum {
> 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 2fff4b16cb4e..fcb58012170f 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h
> @@ -43,19 +43,24 @@
>  #define SMU11_TOOL_SIZE			0x19000
> 
>  #define CLK_MAP(clk, index) \
> -	[SMU_##clk] = index
> +	[SMU_##clk] = {1, (index)}
> 
>  #define FEA_MAP(fea) \
> -	[SMU_FEATURE_##fea##_BIT] = FEATURE_##fea##_BIT
> +	[SMU_FEATURE_##fea##_BIT] = {1, FEATURE_##fea##_BIT}
> 
>  #define TAB_MAP(tab) \
> -	[SMU_TABLE_##tab] = TABLE_##tab
> +	[SMU_TABLE_##tab] = {1, TABLE_##tab}
> 
>  #define PWR_MAP(tab) \
> -	[SMU_POWER_SOURCE_##tab] = POWER_SOURCE_##tab
> +	[SMU_POWER_SOURCE_##tab] = {1, POWER_SOURCE_##tab}
> 
>  #define WORKLOAD_MAP(profile, workload) \
> -	[profile] = workload
> +	[profile] = {1, (workload)}
> +
> +struct smu_11_0_cmn2aisc_mapping {
> +	int	valid_mapping;
> +	int	map_to;
> +};
> 
>  struct smu_11_0_max_sustainable_clocks {
>  	uint32_t display_clock;
> diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> index 5ee6508f1d13..e5fa82e10535 100644
> --- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> @@ -49,9 +49,9 @@
>  	FEATURE_MASK(FEATURE_DPM_DCEFCLK_BIT))
> 
>  #define MSG_MAP(msg, index) \
> -	[SMU_MSG_##msg] = index
> +	[SMU_MSG_##msg] = {1, (index)}
> 
> -static int navi10_message_map[SMU_MSG_MAX_COUNT] = {
> +static struct smu_11_0_cmn2aisc_mapping
> +navi10_message_map[SMU_MSG_MAX_COUNT] = {
>  	MSG_MAP(TestMessage,
> 	PPSMC_MSG_TestMessage),
>  	MSG_MAP(GetSmuVersion,
> 	PPSMC_MSG_GetSmuVersion),
>  	MSG_MAP(GetDriverIfVersion,
> 	PPSMC_MSG_GetDriverIfVersion),
> @@ -118,7 +118,7 @@ static int
> navi10_message_map[SMU_MSG_MAX_COUNT] = {
>  	MSG_MAP(ArmD3,			PPSMC_MSG_ArmD3),
>  };
> 
> -static int navi10_clk_map[SMU_CLK_COUNT] = {
> +static struct smu_11_0_cmn2aisc_mapping
> navi10_clk_map[SMU_CLK_COUNT] =
> +{
>  	CLK_MAP(GFXCLK, PPCLK_GFXCLK),
>  	CLK_MAP(SCLK,	PPCLK_GFXCLK),
>  	CLK_MAP(SOCCLK, PPCLK_SOCCLK),
> @@ -133,7 +133,7 @@ static int navi10_clk_map[SMU_CLK_COUNT] = {
>  	CLK_MAP(PHYCLK, PPCLK_PHYCLK),
>  };
> 
> -static int navi10_feature_mask_map[SMU_FEATURE_COUNT] = {
> +static struct smu_11_0_cmn2aisc_mapping
> +navi10_feature_mask_map[SMU_FEATURE_COUNT] = {
>  	FEA_MAP(DPM_PREFETCHER),
>  	FEA_MAP(DPM_GFXCLK),
>  	FEA_MAP(DPM_GFX_PACE),
> @@ -178,7 +178,7 @@ static int
> navi10_feature_mask_map[SMU_FEATURE_COUNT] = {
>  	FEA_MAP(ATHUB_PG),
>  };
> 
> -static int navi10_table_map[SMU_TABLE_COUNT] = {
> +static struct smu_11_0_cmn2aisc_mapping
> +navi10_table_map[SMU_TABLE_COUNT] = {
>  	TAB_MAP(PPTABLE),
>  	TAB_MAP(WATERMARKS),
>  	TAB_MAP(AVFS),
> @@ -193,12 +193,12 @@ static int navi10_table_map[SMU_TABLE_COUNT]
> = {
>  	TAB_MAP(PACE),
>  };
> 
> -static int navi10_pwr_src_map[SMU_POWER_SOURCE_COUNT] = {
> +static struct smu_11_0_cmn2aisc_mapping
> +navi10_pwr_src_map[SMU_POWER_SOURCE_COUNT] = {
>  	PWR_MAP(AC),
>  	PWR_MAP(DC),
>  };
> 
> -static int navi10_workload_map[] = {
> +static struct smu_11_0_cmn2aisc_mapping
> +navi10_workload_map[PP_SMC_POWER_PROFILE_COUNT] = {
>  	WORKLOAD_MAP(PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT,
> 	WORKLOAD_PPLIB_DEFAULT_BIT),
>  	WORKLOAD_MAP(PP_SMC_POWER_PROFILE_FULLSCREEN3D,
> 		WORKLOAD_PPLIB_FULL_SCREEN_3D_BIT),
>  	WORKLOAD_MAP(PP_SMC_POWER_PROFILE_POWERSAVING,
> 		WORKLOAD_PPLIB_POWER_SAVING_BIT),
> @@ -210,79 +210,87 @@ static int navi10_workload_map[] = {
> 
>  static int navi10_get_smu_msg_index(struct smu_context *smc, uint32_t
> index)  {
> -	int val;
> +	struct smu_11_0_cmn2aisc_mapping mapping;
> +
>  	if (index > SMU_MSG_MAX_COUNT)
>  		return -EINVAL;
> 
> -	val = navi10_message_map[index];
> -	if (val > PPSMC_Message_Count)
> +	mapping = navi10_message_map[index];
> +	if (!(mapping.valid_mapping))
>  		return -EINVAL;
> 
> -	return val;
> +	return mapping.map_to;
>  }
> 
>  static int navi10_get_smu_clk_index(struct smu_context *smc, uint32_t
> index)  {
> -	int val;
> +	struct smu_11_0_cmn2aisc_mapping mapping;
> +
>  	if (index >= SMU_CLK_COUNT)
>  		return -EINVAL;
> 
> -	val = navi10_clk_map[index];
> -	if (val >= PPCLK_COUNT)
> +	mapping = navi10_clk_map[index];
> +	if (!(mapping.valid_mapping))
>  		return -EINVAL;
> 
> -	return val;
> +	return mapping.map_to;
>  }
> 
>  static int navi10_get_smu_feature_index(struct smu_context *smc,
> uint32_t index)  {
> -	int val;
> +	struct smu_11_0_cmn2aisc_mapping mapping;
> +
>  	if (index >= SMU_FEATURE_COUNT)
>  		return -EINVAL;
> 
> -	val = navi10_feature_mask_map[index];
> -	if (val > 64)
> +	mapping = navi10_feature_mask_map[index];
> +	if (!(mapping.valid_mapping))
>  		return -EINVAL;
> 
> -	return val;
> +	return mapping.map_to;
>  }
> 
>  static int navi10_get_smu_table_index(struct smu_context *smc, uint32_t
> index)  {
> -	int val;
> +	struct smu_11_0_cmn2aisc_mapping mapping;
> +
>  	if (index >= SMU_TABLE_COUNT)
>  		return -EINVAL;
> 
> -	val = navi10_table_map[index];
> -	if (val >= TABLE_COUNT)
> +	mapping = navi10_table_map[index];
> +	if (!(mapping.valid_mapping))
>  		return -EINVAL;
> 
> -	return val;
> +	return mapping.map_to;
>  }
> 
>  static int navi10_get_pwr_src_index(struct smu_context *smc, uint32_t
> index)  {
> -	int val;
> +	struct smu_11_0_cmn2aisc_mapping mapping;
> +
>  	if (index >= SMU_POWER_SOURCE_COUNT)
>  		return -EINVAL;
> 
> -	val = navi10_pwr_src_map[index];
> -	if (val >= POWER_SOURCE_COUNT)
> +	mapping = navi10_pwr_src_map[index];
> +	if (!(mapping.valid_mapping))
>  		return -EINVAL;
> 
> -	return val;
> +	return mapping.map_to;
>  }
> 
> 
>  static int navi10_get_workload_type(struct smu_context *smu, enum
> PP_SMC_POWER_PROFILE profile)  {
> -	int val;
> +	struct smu_11_0_cmn2aisc_mapping mapping;
> +
>  	if (profile > PP_SMC_POWER_PROFILE_CUSTOM)
>  		return -EINVAL;
> 
> -	val = navi10_workload_map[profile];
> +	mapping = navi10_workload_map[profile];
> +	if (!(mapping.valid_mapping))
> +		return -EINVAL;
> 
> -	return val;
> +	return mapping.map_to;
>  }
> 
>  static bool is_asic_secure(struct smu_context *smu) diff --git
> a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
> b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
> index d9740384a08e..4685791a65da 100644
> --- a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
> @@ -47,7 +47,7 @@
>  #define CTF_OFFSET_HBM			5
> 
>  #define MSG_MAP(msg) \
> -	[SMU_MSG_##msg] = PPSMC_MSG_##msg
> +	[SMU_MSG_##msg] = {1, PPSMC_MSG_##msg}
> 
>  #define SMC_DPM_FEATURE (FEATURE_DPM_PREFETCHER_MASK | \
>  			 FEATURE_DPM_GFXCLK_MASK | \
> @@ -59,7 +59,7 @@
>  			 FEATURE_DPM_LINK_MASK | \
>  			 FEATURE_DPM_DCEFCLK_MASK)
> 
> -static int vega20_message_map[SMU_MSG_MAX_COUNT] = {
> +static struct smu_11_0_cmn2aisc_mapping
> +vega20_message_map[SMU_MSG_MAX_COUNT] = {
>  	MSG_MAP(TestMessage),
>  	MSG_MAP(GetSmuVersion),
>  	MSG_MAP(GetDriverIfVersion),
> @@ -145,7 +145,7 @@ static int
> vega20_message_map[SMU_MSG_MAX_COUNT] = {
>  	MSG_MAP(GetAVFSVoltageByDpm),
>  };
> 
> -static int vega20_clk_map[SMU_CLK_COUNT] = {
> +static struct smu_11_0_cmn2aisc_mapping
> vega20_clk_map[SMU_CLK_COUNT] =
> +{
>  	CLK_MAP(GFXCLK, PPCLK_GFXCLK),
>  	CLK_MAP(VCLK, PPCLK_VCLK),
>  	CLK_MAP(DCLK, PPCLK_DCLK),
> @@ -159,7 +159,7 @@ static int vega20_clk_map[SMU_CLK_COUNT] = {
>  	CLK_MAP(FCLK, PPCLK_FCLK),
>  };
> 
> -static int vega20_feature_mask_map[SMU_FEATURE_COUNT] = {
> +static struct smu_11_0_cmn2aisc_mapping
> +vega20_feature_mask_map[SMU_FEATURE_COUNT] = {
>  	FEA_MAP(DPM_PREFETCHER),
>  	FEA_MAP(DPM_GFXCLK),
>  	FEA_MAP(DPM_UCLK),
> @@ -195,7 +195,7 @@ static int
> vega20_feature_mask_map[SMU_FEATURE_COUNT] = {
>  	FEA_MAP(XGMI),
>  };
> 
> -static int vega20_table_map[SMU_TABLE_COUNT] = {
> +static struct smu_11_0_cmn2aisc_mapping
> +vega20_table_map[SMU_TABLE_COUNT] = {
>  	TAB_MAP(PPTABLE),
>  	TAB_MAP(WATERMARKS),
>  	TAB_MAP(AVFS),
> @@ -208,12 +208,12 @@ static int vega20_table_map[SMU_TABLE_COUNT]
> = {
>  	TAB_MAP(OVERDRIVE),
>  };
> 
> -static int vega20_pwr_src_map[SMU_POWER_SOURCE_COUNT] = {
> +static struct smu_11_0_cmn2aisc_mapping
> +vega20_pwr_src_map[SMU_POWER_SOURCE_COUNT] = {
>  	PWR_MAP(AC),
>  	PWR_MAP(DC),
>  };
> 
> -static int vega20_workload_map[] = {
> +static struct smu_11_0_cmn2aisc_mapping
> +vega20_workload_map[PP_SMC_POWER_PROFILE_COUNT] = {
>  	WORKLOAD_MAP(PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT,
> 	WORKLOAD_DEFAULT_BIT),
>  	WORKLOAD_MAP(PP_SMC_POWER_PROFILE_FULLSCREEN3D,
> 		WORKLOAD_PPLIB_FULL_SCREEN_3D_BIT),
>  	WORKLOAD_MAP(PP_SMC_POWER_PROFILE_POWERSAVING,
> 		WORKLOAD_PPLIB_POWER_SAVING_BIT),
> @@ -225,79 +225,86 @@ static int vega20_workload_map[] = {
> 
>  static int vega20_get_smu_table_index(struct smu_context *smc, uint32_t
> index)  {
> -	int val;
> +	struct smu_11_0_cmn2aisc_mapping mapping;
> +
>  	if (index >= SMU_TABLE_COUNT)
>  		return -EINVAL;
> 
> -	val = vega20_table_map[index];
> -	if (val >= TABLE_COUNT)
> +	mapping = vega20_table_map[index];
> +	if (!(mapping.valid_mapping))
>  		return -EINVAL;
> 
> -	return val;
> +	return mapping.map_to;
>  }
> 
>  static int vega20_get_pwr_src_index(struct smu_context *smc, uint32_t
> index)  {
> -	int val;
> +	struct smu_11_0_cmn2aisc_mapping mapping;
> +
>  	if (index >= SMU_POWER_SOURCE_COUNT)
>  		return -EINVAL;
> 
> -	val = vega20_pwr_src_map[index];
> -	if (val >= POWER_SOURCE_COUNT)
> +	mapping = vega20_pwr_src_map[index];
> +	if (!(mapping.valid_mapping))
>  		return -EINVAL;
> 
> -	return val;
> +	return mapping.map_to;
>  }
> 
>  static int vega20_get_smu_feature_index(struct smu_context *smc,
> uint32_t index)  {
> -	int val;
> +	struct smu_11_0_cmn2aisc_mapping mapping;
> +
>  	if (index >= SMU_FEATURE_COUNT)
>  		return -EINVAL;
> 
> -	val = vega20_feature_mask_map[index];
> -	if (val > 64)
> +	mapping = vega20_feature_mask_map[index];
> +	if (!(mapping.valid_mapping))
>  		return -EINVAL;
> 
> -	return val;
> +	return mapping.map_to;
>  }
> 
>  static int vega20_get_smu_clk_index(struct smu_context *smc, uint32_t
> index)  {
> -	int val;
> +	struct smu_11_0_cmn2aisc_mapping mapping;
> +
>  	if (index >= SMU_CLK_COUNT)
>  		return -EINVAL;
> 
> -	val = vega20_clk_map[index];
> -	if (val >= PPCLK_COUNT)
> +	mapping = vega20_clk_map[index];
> +	if (!(mapping.valid_mapping))
>  		return -EINVAL;
> 
> -	return val;
> +	return mapping.map_to;
>  }
> 
>  static int vega20_get_smu_msg_index(struct smu_context *smc, uint32_t
> index)  {
> -	int val;
> +	struct smu_11_0_cmn2aisc_mapping mapping;
> 
>  	if (index >= SMU_MSG_MAX_COUNT)
>  		return -EINVAL;
> 
> -	val = vega20_message_map[index];
> -	if (val > PPSMC_Message_Count)
> +	mapping = vega20_message_map[index];
> +	if (!(mapping.valid_mapping))
>  		return -EINVAL;
> 
> -	return val;
> +	return mapping.map_to;
>  }
> 
>  static int vega20_get_workload_type(struct smu_context *smu, enum
> PP_SMC_POWER_PROFILE profile)  {
> -	int val;
> +	struct smu_11_0_cmn2aisc_mapping mapping;
> +
>  	if (profile > PP_SMC_POWER_PROFILE_CUSTOM)
>  		return -EINVAL;
> 
> -	val = vega20_workload_map[profile];
> +	mapping = vega20_workload_map[profile];
> +	if (!(mapping.valid_mapping))
> +		return -EINVAL;
> 
> -	return val;
> +	return mapping.map_to;
>  }
> 
>  static int vega20_tables_init(struct smu_context *smu, struct smu_table
> *tables)
> --
> 2.21.0



More information about the amd-gfx mailing list