Re: [PATCH v3 2/2] drm/amd/pm: Update pci link speed for smu v13.0.6

["Lazar, Lijo" <lijo.lazar@xxxxxxx>]

On 8/14/2023 3:47 PM, Asad Kamal wrote:
> Update pcie link speed registers for smu v13.0.6 &
> populate gpu metric table with pcie link speed rather than
> gen for smu v13_0_0, smu v13_0_6 & smu v13_0_7
>
> v2:
> Update ESM register address
> Used macro to convert pcie gen to speed
>
> v3:
> Chaged macro to inline function for pcie gen to speed
>
> Signed-off-by: Asad Kamal <asad.kamal@xxxxxxx>
> ---
>   drivers/gpu/drm/amd/pm/swsmu/inc/smu_v11_0.h     |  1 -
>   drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c   |  1 -
>   .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c |  7 ++++++-
>   .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c | 16 ++++++++++++++--
>   .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c |  7 ++++++-
>   drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c           |  2 ++
>   drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h           |  8 ++++++++
>   7 files changed, 36 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_v11_0.h b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_v11_0.h
> index 1b4e0e4716ea..a0e5ad0381d6 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_v11_0.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_v11_0.h
> @@ -64,7 +64,6 @@
>   #define LINK_SPEED_MAX			3
>   
>   static const __maybe_unused uint16_t link_width[] = {0, 1, 2, 4, 8, 12, 16};
> -static const __maybe_unused uint16_t link_speed[] = {25, 50, 80, 160};
>   
>   static const
>   struct smu_temperature_range __maybe_unused smu11_thermal_policy[] = {
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
> index 895cda8e6934..6863186937f7 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
> @@ -83,7 +83,6 @@ MODULE_FIRMWARE("amdgpu/smu_13_0_10.bin");
>   #define PCIE_LC_SPEED_CNTL__LC_CURRENT_DATA_RATE__SHIFT 0xE
>   
>   static const int link_width[] = {0, 1, 2, 4, 8, 12, 16};
> -static const int link_speed[] = {25, 50, 80, 160};
>   
>   const int pmfw_decoded_link_speed[5] = {1, 2, 3, 4, 5};
>   const int pmfw_decoded_link_width[7] = {0, 1, 2, 4, 8, 12, 16};
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> index 48b03524a52d..f4e2ac4a8713 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> @@ -102,6 +102,8 @@
>   #define PP_OD_FEATURE_UCLK_FMAX				3
>   #define PP_OD_FEATURE_GFX_VF_CURVE			4
>   
> +#define LINK_SPEED_MAX					3
> +
>   static struct cmn2asic_msg_mapping smu_v13_0_0_message_map[SMU_MSG_MAX_COUNT] = {
>   	MSG_MAP(TestMessage,			PPSMC_MSG_TestMessage,                 1),
>   	MSG_MAP(GetSmuVersion,			PPSMC_MSG_GetSmuVersion,               1),
> @@ -1759,7 +1761,10 @@ static ssize_t smu_v13_0_0_get_gpu_metrics(struct smu_context *smu,
>   	gpu_metrics->current_fan_speed = metrics->AvgFanRpm;
>   
>   	gpu_metrics->pcie_link_width = metrics->PcieWidth;
> -	gpu_metrics->pcie_link_speed = metrics->PcieRate;
> +	if ((metrics->PcieRate - 1) > LINK_SPEED_MAX)
> +		gpu_metrics->pcie_link_speed = pcie_gen_to_speed(1);
> +	else
> +		gpu_metrics->pcie_link_speed = pcie_gen_to_speed(metrics->PcieRate);
>   
>   	gpu_metrics->system_clock_counter = ktime_get_boottime_ns();
>   
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> index 21275e496852..d1a9be37bece 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> @@ -80,12 +80,17 @@
>   /* possible frequency drift (1Mhz) */
>   #define EPSILON 1
>   
> -#define smnPCIE_ESM_CTRL 0x193D0
> +#define smnPCIE_ESM_CTRL 0x93D0
>   #define smnPCIE_LC_LINK_WIDTH_CNTL 0x1a340288
>   #define PCIE_LC_LINK_WIDTH_CNTL__LC_LINK_WIDTH_RD_MASK 0x00000070L
>   #define PCIE_LC_LINK_WIDTH_CNTL__LC_LINK_WIDTH_RD__SHIFT 0x4
>   #define MAX_LINK_WIDTH 6
>   
> +#define smnPCIE_LC_SPEED_CNTL                   0x1a340290
> +#define PCIE_LC_SPEED_CNTL__LC_CURRENT_DATA_RATE_MASK 0xE0
> +#define PCIE_LC_SPEED_CNTL__LC_CURRENT_DATA_RATE__SHIFT 0x5
> +#define LINK_SPEED_MAX				4
> +
>   static const struct cmn2asic_msg_mapping smu_v13_0_6_message_map[SMU_MSG_MAX_COUNT] = {
>   	MSG_MAP(TestMessage,			     PPSMC_MSG_TestMessage,			0),
>   	MSG_MAP(GetSmuVersion,			     PPSMC_MSG_GetSmuVersion,			1),
> @@ -1936,6 +1941,7 @@ smu_v13_0_6_get_current_pcie_link_width_level(struct smu_context *smu)
>   static int smu_v13_0_6_get_current_pcie_link_speed(struct smu_context *smu)
>   {
>   	struct amdgpu_device *adev = smu->adev;
> +	uint32_t speed_level;
>   	uint32_t esm_ctrl;
>   
>   	/* TODO: confirm this on real target */
> @@ -1943,7 +1949,13 @@ static int smu_v13_0_6_get_current_pcie_link_speed(struct smu_context *smu)
>   	if ((esm_ctrl >> 15) & 0x1FFFF)
>   		return (((esm_ctrl >> 8) & 0x3F) + 128);
>   
> -	return smu_v13_0_get_current_pcie_link_speed(smu);
> +	speed_level = (RREG32_PCIE(smnPCIE_LC_SPEED_CNTL) &
> +		PCIE_LC_SPEED_CNTL__LC_CURRENT_DATA_RATE_MASK)
> +		>> PCIE_LC_SPEED_CNTL__LC_CURRENT_DATA_RATE__SHIFT;
> +	if (speed_level > LINK_SPEED_MAX)
> +		speed_level = 0;
> +
> +	return pcie_gen_to_speed(speed_level + 1);
>   }
>   
>   static ssize_t smu_v13_0_6_get_gpu_metrics(struct smu_context *smu, void **table)
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> index 690f89fcbbe3..df88206aaf13 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> @@ -78,6 +78,8 @@
>   #define PP_OD_FEATURE_UCLK_FMAX				3
>   #define PP_OD_FEATURE_GFX_VF_CURVE			4
>   
> +#define LINK_SPEED_MAX					3
> +
>   static struct cmn2asic_msg_mapping smu_v13_0_7_message_map[SMU_MSG_MAX_COUNT] = {
>   	MSG_MAP(TestMessage,			PPSMC_MSG_TestMessage,                 1),
>   	MSG_MAP(GetSmuVersion,			PPSMC_MSG_GetSmuVersion,               1),
> @@ -1735,7 +1737,10 @@ static ssize_t smu_v13_0_7_get_gpu_metrics(struct smu_context *smu,
>   	gpu_metrics->current_fan_speed = metrics->AvgFanRpm;
>   
>   	gpu_metrics->pcie_link_width = metrics->PcieWidth;
> -	gpu_metrics->pcie_link_speed = metrics->PcieRate;
> +	if ((metrics->PcieRate - 1) > LINK_SPEED_MAX)
> +		gpu_metrics->pcie_link_speed = pcie_gen_to_speed(1);
> +	else
> +		gpu_metrics->pcie_link_speed = pcie_gen_to_speed(metrics->PcieRate);
>   
>   	gpu_metrics->system_clock_counter = ktime_get_boottime_ns();
>   
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> index 442d267088bc..12618a583e97 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> @@ -39,6 +39,8 @@
>   
>   #define MP1_C2PMSG_90__CONTENT_MASK                                                                    0xFFFFFFFFL
>   
> +const int link_speed[] = {25, 50, 80, 160, 320, 640};
> +
>   #undef __SMU_DUMMY_MAP
>   #define __SMU_DUMMY_MAP(type)	#type
>   static const char * const __smu_message_names[] = {
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> index d7cd358a53bd..7d0251422228 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> @@ -30,6 +30,14 @@
>   #define FDO_PWM_MODE_STATIC  1
>   #define FDO_PWM_MODE_STATIC_RPM 5
>   
> +extern const int link_speed[];
> +
> +/* Helper to Convert from PCIE Gen 1/2/3/4/5/6 to 0.1 GT/s speed units */
> +static inline int pcie_gen_to_speed(uint32_t gen)
> +{
> +	return ((gen <= 0) ? link_speed[0] : link_speed[gen - 1]);

Since parameter is unsigned, < 0 check doesn't make sense. With that 
fixed, series is -

	Reviewed-by: Lijo Lazar <lijo.lazar@xxxxxxx>

Thanks,
Lijo

> +}
> +
>   int smu_cmn_send_msg_without_waiting(struct smu_context *smu,
>   				     uint16_t msg_index,
>   				     uint32_t param);