Re: [PATCH 23/66] drm/amdgpu/amdgpu_smu: convert to IP version checking

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

On 9/21/2021 11:36 PM, Alex Deucher wrote:
> Use IP versions rather than asic_type to differentiate
> IP version specific features.
> 
> v2: rebase
> 
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>   drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 94 +++++++++++++----------
>   1 file changed, 55 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index 04863a797115..5f372d353d9d 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -455,7 +455,8 @@ static int smu_get_power_num_states(void *handle,
>   
>   bool is_support_sw_smu(struct amdgpu_device *adev)
>   {
> -	if (adev->asic_type >= CHIP_ARCTURUS)
> +	if ((adev->asic_type >= CHIP_ARCTURUS) ||
> +	    (adev->ip_versions[MP1_HWIP] >= IP_VERSION(11, 0, 0)))
>   		return true;
>   
>   	return false;
> @@ -575,43 +576,47 @@ static int smu_set_funcs(struct amdgpu_device *adev)
>   	if (adev->pm.pp_feature & PP_OVERDRIVE_MASK)
>   		smu->od_enabled = true;
>   
> -	switch (adev->asic_type) {
> -	case CHIP_NAVI10:
> -	case CHIP_NAVI14:
> -	case CHIP_NAVI12:
> +	switch (adev->ip_versions[MP1_HWIP]) {
> +	case IP_VERSION(11, 0, 0):
> +	case IP_VERSION(11, 0, 5):
> +	case IP_VERSION(11, 0, 9):
>   		navi10_set_ppt_funcs(smu);
>   		break;
> -	case CHIP_ARCTURUS:
> -		adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
> -		arcturus_set_ppt_funcs(smu);
> -		/* OD is not supported on Arcturus */
> -		smu->od_enabled =false;
> -		break;
> -	case CHIP_SIENNA_CICHLID:
> -	case CHIP_NAVY_FLOUNDER:
> -	case CHIP_DIMGREY_CAVEFISH:
> -	case CHIP_BEIGE_GOBY:
> +	case IP_VERSION(11, 0, 7):
> +	case IP_VERSION(11, 0, 11):
> +	case IP_VERSION(11, 0, 12):
> +	case IP_VERSION(11, 0, 13):
>   		sienna_cichlid_set_ppt_funcs(smu);
>   		break;
> -	case CHIP_ALDEBARAN:
> -		aldebaran_set_ppt_funcs(smu);
> -		/* Enable pp_od_clk_voltage node */
> -		smu->od_enabled = true;
> -		break;
> -	case CHIP_RENOIR:
> +	case IP_VERSION(12, 0, 0):
>   		renoir_set_ppt_funcs(smu);
>   		break;
> -	case CHIP_VANGOGH:
> +	case IP_VERSION(11, 5, 0):
>   		vangogh_set_ppt_funcs(smu);
>   		break;
> -	case CHIP_YELLOW_CARP:
> +	case IP_VERSION(13, 0, 1):
>   		yellow_carp_set_ppt_funcs(smu);
>   		break;
> -	case CHIP_CYAN_SKILLFISH:
> +	case IP_VERSION(11, 0, 8):
>   		cyan_skillfish_set_ppt_funcs(smu);
>   		break;
>   	default:
> -		return -EINVAL;
> +		switch (adev->asic_type) {
> +		case CHIP_ARCTURUS:
> +			adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
> +			arcturus_set_ppt_funcs(smu);
> +			/* OD is not supported on Arcturus */
> +			smu->od_enabled =false;
> +			break;
> +		case CHIP_ALDEBARAN:
> +			aldebaran_set_ppt_funcs(smu);
> +			/* Enable pp_od_clk_voltage node */
> +			smu->od_enabled = true;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
>   	}
>   
>   	return 0;
> @@ -694,7 +699,7 @@ static int smu_late_init(void *handle)
>   		return ret;
>   	}
>   
> -	if (adev->asic_type == CHIP_YELLOW_CARP)
> +	if (adev->ip_versions[MP1_HWIP] == IP_VERSION(13, 0, 1))
>   		return 0;
>   
>   	if (!amdgpu_sriov_vf(adev) || smu->od_enabled) {
> @@ -1140,8 +1145,10 @@ static int smu_smc_hw_setup(struct smu_context *smu)
>   	if (adev->in_suspend && smu_is_dpm_running(smu)) {
>   		dev_info(adev->dev, "dpm has been enabled\n");
>   		/* this is needed specifically */
> -		if ((adev->asic_type >= CHIP_SIENNA_CICHLID) &&
> -		    (adev->asic_type <= CHIP_DIMGREY_CAVEFISH))
> +		if ((adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 7)) ||
> +		    (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 11)) ||
> +		    (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 5, 0)) ||
> +		    (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 12)))

switch...case looks better here

>   			ret = smu_system_features_control(smu, true);
>   		return ret;
>   	}
> @@ -1284,7 +1291,7 @@ static int smu_start_smc_engine(struct smu_context *smu)
>   	int ret = 0;
>   
>   	if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP) {
> -		if (adev->asic_type < CHIP_NAVI10) {
> +		if (adev->ip_versions[MP1_HWIP] < IP_VERSION(11, 0, 0)) {
>   			if (smu->ppt_funcs->load_microcode) {
>   				ret = smu->ppt_funcs->load_microcode(smu);
>   				if (ret)
> @@ -1403,8 +1410,14 @@ static int smu_disable_dpms(struct smu_context *smu)
>   	 *     properly.
>   	 */
>   	if (smu->uploading_custom_pp_table &&
> -	    (adev->asic_type >= CHIP_NAVI10) &&
> -	    (adev->asic_type <= CHIP_BEIGE_GOBY))
> +	    ((adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 0)) ||
> +	     (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 5)) ||
> +	     (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 9)) ||
> +	     (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 7)) ||
> +	     (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 11)) ||
> +	     (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 5, 0)) ||
> +	     (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 12)) ||
> +	     (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 13))))

Same - switch...case

>   		return smu_disable_all_features_with_exception(smu,
>   							       true,
>   							       SMU_FEATURE_COUNT);
> @@ -1413,9 +1426,11 @@ static int smu_disable_dpms(struct smu_context *smu)
>   	 * For Sienna_Cichlid, PMFW will handle the features disablement properly
>   	 * on BACO in. Driver involvement is unnecessary.
>   	 */
> -	if (((adev->asic_type == CHIP_SIENNA_CICHLID) ||
> -	     ((adev->asic_type >= CHIP_NAVI10) && (adev->asic_type <= CHIP_NAVI12))) &&
> -	     use_baco)
> +	if (((adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 7)) ||
> +	     (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 0)) ||
> +	     (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 5)) ||
> +	     (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 9))) &&
> +	    use_baco)

Here as well.

Thanks,
Lijo

>   		return smu_disable_all_features_with_exception(smu,
>   							       true,
>   							       SMU_FEATURE_BACO_BIT);
> @@ -1436,7 +1451,7 @@ static int smu_disable_dpms(struct smu_context *smu)
>   			dev_err(adev->dev, "Failed to disable smu features.\n");
>   	}
>   
> -	if (adev->asic_type >= CHIP_NAVI10 &&
> +	if (adev->ip_versions[MP1_HWIP] >= IP_VERSION(11, 0, 0) &&
>   	    adev->gfx.rlc.funcs->stop)
>   		adev->gfx.rlc.funcs->stop(adev);
>   
> @@ -2229,6 +2244,7 @@ int smu_get_power_limit(void *handle,
>   			enum pp_power_type pp_power_type)
>   {
>   	struct smu_context *smu = handle;
> +	struct amdgpu_device *adev = smu->adev;
>   	enum smu_ppt_limit_level limit_level;
>   	uint32_t limit_type;
>   	int ret = 0;
> @@ -2273,10 +2289,10 @@ int smu_get_power_limit(void *handle,
>   		switch (limit_level) {
>   		case SMU_PPT_LIMIT_CURRENT:
>   			if ((smu->adev->asic_type == CHIP_ALDEBARAN) ||
> -			     (smu->adev->asic_type == CHIP_SIENNA_CICHLID) ||
> -			     (smu->adev->asic_type == CHIP_NAVY_FLOUNDER) ||
> -			     (smu->adev->asic_type == CHIP_DIMGREY_CAVEFISH) ||
> -			     (smu->adev->asic_type == CHIP_BEIGE_GOBY))
> +			     (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 7)) ||
> +			     (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 11)) ||
> +			     (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 12)) ||
> +			     (adev->ip_versions[MP1_HWIP] == IP_VERSION(11, 0, 13)))
>   				ret = smu_get_asic_power_limits(smu,
>   								&smu->current_power_limit,
>   								NULL,
>