[PATCH 1/5] drm/amd/pm: correct DPM_XGMI/VCN_DPM feature name

[PATCH 1/5] drm/amd/pm: correct DPM_XGMI/VCN_DPM feature name

[Kevin Wang]

the following feature is wrong, it will cause sysnode of pp_features show error:
1. DPM_XGMI
2. VCN_DPM

Signed-off-by: Kevin Wang <kevin1.wang@amd.com>
---
 drivers/gpu/drm/amd/pm/inc/smu_types.h           |  1 -
 .../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c    | 16 ++++++++--------
 .../gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c   |  4 ++--
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/inc/smu_types.h b/drivers/gpu/drm/amd/pm/inc/smu_types.h
index 1d3765b873df..3fdee6cf01ed 100644
--- a/drivers/gpu/drm/amd/pm/inc/smu_types.h
+++ b/drivers/gpu/drm/amd/pm/inc/smu_types.h
@@ -297,7 +297,6 @@ enum smu_clk_type {
        __SMU_DUMMY_MAP(DS_FCLK),                       	\
        __SMU_DUMMY_MAP(DS_MP1CLK),                     	\
        __SMU_DUMMY_MAP(DS_MP0CLK),                     	\
-       __SMU_DUMMY_MAP(XGMI),                          	\
        __SMU_DUMMY_MAP(XGMI_PER_LINK_PWR_DWN),          \
        __SMU_DUMMY_MAP(DPM_GFX_PACE),                  	\
        __SMU_DUMMY_MAP(MEM_VDDCI_SCALING),             	\
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
index 6ec8492f71f5..b4d98465bb36 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
@@ -163,14 +163,14 @@ static const struct cmn2asic_mapping arcturus_feature_mask_map[SMU_FEATURE_COUNT
 	FEA_MAP(DPM_SOCCLK),
 	FEA_MAP(DPM_FCLK),
 	FEA_MAP(DPM_MP0CLK),
-	ARCTURUS_FEA_MAP(SMU_FEATURE_XGMI_BIT, FEATURE_DPM_XGMI_BIT),
+	FEA_MAP(DPM_XGMI),
 	FEA_MAP(DS_GFXCLK),
 	FEA_MAP(DS_SOCCLK),
 	FEA_MAP(DS_LCLK),
 	FEA_MAP(DS_FCLK),
 	FEA_MAP(DS_UCLK),
 	FEA_MAP(GFX_ULV),
-	ARCTURUS_FEA_MAP(SMU_FEATURE_VCN_PG_BIT, FEATURE_DPM_VCN_BIT),
+	ARCTURUS_FEA_MAP(SMU_FEATURE_VCN_DPM_BIT, FEATURE_DPM_VCN_BIT),
 	FEA_MAP(RSMU_SMN_CG),
 	FEA_MAP(WAFL_CG),
 	FEA_MAP(PPT),
@@ -721,13 +721,13 @@ static int arcturus_get_current_clk_freq_by_table(struct smu_context *smu,
 			member_type = METRICS_AVERAGE_SOCCLK;
 		break;
 	case PPCLK_VCLK:
-		if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_VCN_PG_BIT))
+		if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_VCN_DPM_BIT))
 			member_type = METRICS_CURR_VCLK;
 		else
 			member_type = METRICS_AVERAGE_VCLK;
 		break;
 	case PPCLK_DCLK:
-		if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_VCN_PG_BIT))
+		if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_VCN_DPM_BIT))
 			member_type = METRICS_CURR_DCLK;
 		else
 			member_type = METRICS_AVERAGE_DCLK;
@@ -1916,16 +1916,16 @@ static int arcturus_dpm_set_vcn_enable(struct smu_context *smu, bool enable)
 	int ret = 0;
 
 	if (enable) {
-		if (!smu_cmn_feature_is_enabled(smu, SMU_FEATURE_VCN_PG_BIT)) {
-			ret = smu_cmn_feature_set_enabled(smu, SMU_FEATURE_VCN_PG_BIT, 1);
+		if (!smu_cmn_feature_is_enabled(smu, SMU_FEATURE_VCN_DPM_BIT)) {
+			ret = smu_cmn_feature_set_enabled(smu, SMU_FEATURE_VCN_DPM_BIT, 1);
 			if (ret) {
 				dev_err(smu->adev->dev, "[EnableVCNDPM] failed!\n");
 				return ret;
 			}
 		}
 	} else {
-		if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_VCN_PG_BIT)) {
-			ret = smu_cmn_feature_set_enabled(smu, SMU_FEATURE_VCN_PG_BIT, 0);
+		if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_VCN_DPM_BIT)) {
+			ret = smu_cmn_feature_set_enabled(smu, SMU_FEATURE_VCN_DPM_BIT, 0);
 			if (ret) {
 				dev_err(smu->adev->dev, "[DisableVCNDPM] failed!\n");
 				return ret;
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
index 856eeaf293b8..97cc6fb9b22b 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
@@ -156,14 +156,14 @@ static const struct cmn2asic_mapping aldebaran_feature_mask_map[SMU_FEATURE_COUN
 	ALDEBARAN_FEA_MAP(SMU_FEATURE_DPM_SOCCLK_BIT, 			FEATURE_DPM_SOCCLK_BIT),
 	ALDEBARAN_FEA_MAP(SMU_FEATURE_DPM_FCLK_BIT, 			FEATURE_DPM_FCLK_BIT),
 	ALDEBARAN_FEA_MAP(SMU_FEATURE_DPM_LCLK_BIT, 			FEATURE_DPM_LCLK_BIT),
-	ALDEBARAN_FEA_MAP(SMU_FEATURE_XGMI_BIT, 				FEATURE_DPM_XGMI_BIT),
+	ALDEBARAN_FEA_MAP(SMU_FEATURE_DPM_XGMI_BIT, 				FEATURE_DPM_XGMI_BIT),
 	ALDEBARAN_FEA_MAP(SMU_FEATURE_DS_GFXCLK_BIT, 			FEATURE_DS_GFXCLK_BIT),
 	ALDEBARAN_FEA_MAP(SMU_FEATURE_DS_SOCCLK_BIT, 			FEATURE_DS_SOCCLK_BIT),
 	ALDEBARAN_FEA_MAP(SMU_FEATURE_DS_LCLK_BIT, 				FEATURE_DS_LCLK_BIT),
 	ALDEBARAN_FEA_MAP(SMU_FEATURE_DS_FCLK_BIT, 				FEATURE_DS_FCLK_BIT),
 	ALDEBARAN_FEA_MAP(SMU_FEATURE_DS_UCLK_BIT,				FEATURE_DS_UCLK_BIT),
 	ALDEBARAN_FEA_MAP(SMU_FEATURE_GFX_SS_BIT, 				FEATURE_GFX_SS_BIT),
-	ALDEBARAN_FEA_MAP(SMU_FEATURE_VCN_PG_BIT, 				FEATURE_DPM_VCN_BIT),
+	ALDEBARAN_FEA_MAP(SMU_FEATURE_VCN_DPM_BIT, 				FEATURE_DPM_VCN_BIT),
 	ALDEBARAN_FEA_MAP(SMU_FEATURE_RSMU_SMN_CG_BIT, 			FEATURE_RSMU_SMN_CG_BIT),
 	ALDEBARAN_FEA_MAP(SMU_FEATURE_WAFL_CG_BIT, 				FEATURE_WAFL_CG_BIT),
 	ALDEBARAN_FEA_MAP(SMU_FEATURE_PPT_BIT, 					FEATURE_PPT_BIT),
-- 
2.25.1

[PATCH 2/5] drm/amd/pm: skip to load smu microcode on sriov for aldebaran

[Kevin Wang]

1. skip to load smu firmware in sriov mode for aldebaran chip
2. using vbios pptable if in sriov mode.

Signed-off-by: Kevin Wang <kevin1.wang@amd.com>
---
 .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c    | 66 ++++++++++---------
 1 file changed, 36 insertions(+), 30 deletions(-)

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 a421ba85bd6d..a0ca7e7a0903 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
@@ -85,6 +85,10 @@ int smu_v13_0_init_microcode(struct smu_context *smu)
 	const struct common_firmware_header *header;
 	struct amdgpu_firmware_info *ucode = NULL;
 
+	/* doesn't need to load smu firmware in IOV mode */
+	if (amdgpu_sriov_vf(adev))
+		return 0;
+
 	switch (adev->asic_type) {
 	case CHIP_ALDEBARAN:
 		chip_name = "aldebaran";
@@ -279,41 +283,43 @@ int smu_v13_0_setup_pptable(struct smu_context *smu)
 	void *table;
 	uint16_t version_major, version_minor;
 
+	if (!amdgpu_sriov_vf(adev)) {
+		if (amdgpu_smu_pptable_id >= 0) {
+			smu->smu_table.boot_values.pp_table_id = amdgpu_smu_pptable_id;
+			dev_info(adev->dev, "override pptable id %d\n", amdgpu_smu_pptable_id);
+		}
 
-	if (amdgpu_smu_pptable_id >= 0) {
-		smu->smu_table.boot_values.pp_table_id = amdgpu_smu_pptable_id;
-		dev_info(adev->dev, "override pptable id %d\n", amdgpu_smu_pptable_id);
-	}
-
-	hdr = (const struct smc_firmware_header_v1_0 *) adev->pm.fw->data;
-	version_major = le16_to_cpu(hdr->header.header_version_major);
-	version_minor = le16_to_cpu(hdr->header.header_version_minor);
-	if (version_major == 2 && smu->smu_table.boot_values.pp_table_id > 0) {
-		dev_info(adev->dev, "use driver provided pptable %d\n", smu->smu_table.boot_values.pp_table_id);
-		switch (version_minor) {
-		case 1:
-			ret = smu_v13_0_set_pptable_v2_1(smu, &table, &size,
-							 smu->smu_table.boot_values.pp_table_id);
-			break;
-		default:
-			ret = -EINVAL;
-			break;
+		hdr = (const struct smc_firmware_header_v1_0 *) adev->pm.fw->data;
+		version_major = le16_to_cpu(hdr->header.header_version_major);
+		version_minor = le16_to_cpu(hdr->header.header_version_minor);
+		if (version_major == 2 && smu->smu_table.boot_values.pp_table_id > 0) {
+			dev_info(adev->dev, "use driver provided pptable %d\n", smu->smu_table.boot_values.pp_table_id);
+			switch (version_minor) {
+			case 1:
+				ret = smu_v13_0_set_pptable_v2_1(smu, &table, &size,
+								 smu->smu_table.boot_values.pp_table_id);
+				break;
+			default:
+				ret = -EINVAL;
+				break;
+			}
+			if (ret)
+				return ret;
+			goto out;
 		}
-		if (ret)
-			return ret;
+	}
 
-	} else {
-		dev_info(adev->dev, "use vbios provided pptable\n");
-		index = get_index_into_master_table(atom_master_list_of_data_tables_v2_1,
-						    powerplayinfo);
+	dev_info(adev->dev, "use vbios provided pptable\n");
+	index = get_index_into_master_table(atom_master_list_of_data_tables_v2_1,
+					    powerplayinfo);
 
-		ret = amdgpu_atombios_get_data_table(adev, index, &atom_table_size, &frev, &crev,
-						     (uint8_t **)&table);
-		if (ret)
-			return ret;
-		size = atom_table_size;
-	}
+	ret = amdgpu_atombios_get_data_table(adev, index, &atom_table_size, &frev, &crev,
+					     (uint8_t **)&table);
+	if (ret)
+		return ret;
 
+	size = atom_table_size;
+out:
 	if (!smu->smu_table.power_play_table)
 		smu->smu_table.power_play_table = table;
 	if (!smu->smu_table.power_play_table_size)
-- 
2.25.1

[PATCH 3/5] drm/amd/pm: change smu msg's attribute to allow working under sriov

[Kevin Wang]

the following message is allowed in sriov mode:
1. GetEnabledSmuFeaturesLow
2. GetEnabledSmuFeaturesHigh

Signed-off-by: Kevin Wang <kevin1.wang@amd.com>
---
 drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
index 97cc6fb9b22b..fd04391364ac 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
@@ -90,8 +90,8 @@ static const struct cmn2asic_msg_mapping aldebaran_message_map[SMU_MSG_MAX_COUNT
 	MSG_MAP(GetDriverIfVersion,		     PPSMC_MSG_GetDriverIfVersion,		1),
 	MSG_MAP(EnableAllSmuFeatures,		     PPSMC_MSG_EnableAllSmuFeatures,		0),
 	MSG_MAP(DisableAllSmuFeatures,		     PPSMC_MSG_DisableAllSmuFeatures,		0),
-	MSG_MAP(GetEnabledSmuFeaturesLow,	     PPSMC_MSG_GetEnabledSmuFeaturesLow,	0),
-	MSG_MAP(GetEnabledSmuFeaturesHigh,	     PPSMC_MSG_GetEnabledSmuFeaturesHigh,	0),
+	MSG_MAP(GetEnabledSmuFeaturesLow,	     PPSMC_MSG_GetEnabledSmuFeaturesLow,	1),
+	MSG_MAP(GetEnabledSmuFeaturesHigh,	     PPSMC_MSG_GetEnabledSmuFeaturesHigh,	1),
 	MSG_MAP(SetDriverDramAddrHigh,		     PPSMC_MSG_SetDriverDramAddrHigh,		1),
 	MSG_MAP(SetDriverDramAddrLow,		     PPSMC_MSG_SetDriverDramAddrLow,		1),
 	MSG_MAP(SetToolsDramAddrHigh,		     PPSMC_MSG_SetToolsDramAddrHigh,		0),
-- 
2.25.1

[PATCH 4/5] drm/amd/pm: change return value in aldebaran_get_power_limit()

[Kevin Wang]

1. change return value to avoid smu driver probe fails when FEATURE_PPT is
not enabled.
2. if FEATURE_PPT is not enabled, set power limit value to 0.

Signed-off-by: Kevin Wang <kevin1.wang@amd.com>
---
 .../gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c    | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
index fd04391364ac..e00ac43515bb 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
@@ -1194,8 +1194,19 @@ static int aldebaran_get_power_limit(struct smu_context *smu,
 	uint32_t power_limit = 0;
 	int ret;
 
-	if (!smu_cmn_feature_is_enabled(smu, SMU_FEATURE_PPT_BIT))
-		return -EINVAL;
+	if (!smu_cmn_feature_is_enabled(smu, SMU_FEATURE_PPT_BIT)) {
+		if (current_power_limit)
+			*current_power_limit = 0;
+		if (default_power_limit)
+			*default_power_limit = 0;
+		if (max_power_limit)
+			*max_power_limit = 0;
+
+		dev_err(smu->adev->dev,
+			"the PPT feature is not enabled, set power limit to 0");
+
+		return 0;
+	}
 
 	/* Valid power data is available only from primary die.
 	 * For secondary die show the value as 0.
-- 
2.25.1

[PATCH 5/5] drm/amd/pm: change pp_dpm_sclk/mclk/fclk attribute is RO for aldebaran

[Kevin Wang]

the following clock is only support voltage DPM, change attribute to RO:
1. pp_dpm_sclk
2. pp_dpm_mclk
3. pp_dpm_fclk

Signed-off-by: Kevin Wang <kevin1.wang@amd.com>
---
 drivers/gpu/drm/amd/pm/amdgpu_pm.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 769f58d5ae1a..f894b34418df 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2094,14 +2094,19 @@ static int default_attr_update(struct amdgpu_device *adev, struct amdgpu_device_
 			*states = ATTR_STATE_UNSUPPORTED;
 	}
 
-	if (asic_type == CHIP_ARCTURUS) {
-		/* Arcturus does not support standalone mclk/socclk/fclk level setting */
+	switch (asic_type) {
+	case CHIP_ARCTURUS:
+	case CHIP_ALDEBARAN:
+		/* the Mi series card does not support standalone mclk/socclk/fclk level setting */
 		if (DEVICE_ATTR_IS(pp_dpm_mclk) ||
 		    DEVICE_ATTR_IS(pp_dpm_socclk) ||
 		    DEVICE_ATTR_IS(pp_dpm_fclk)) {
 			dev_attr->attr.mode &= ~S_IWUGO;
 			dev_attr->store = NULL;
 		}
+		break;
+	default:
+		break;
 	}
 
 	if (DEVICE_ATTR_IS(pp_dpm_dcefclk)) {
-- 
2.25.1

Re: [PATCH 2/5] drm/amd/pm: skip to load smu microcode on sriov for aldebaran

[Lazar, Lijo]

On 8/11/2021 2:03 PM, Kevin Wang wrote:
> 1. skip to load smu firmware in sriov mode for aldebaran chip
> 2. using vbios pptable if in sriov mode.
> 
> Signed-off-by: Kevin Wang <kevin1.wang@amd.com>
> ---
>   .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c    | 66 ++++++++++---------
>   1 file changed, 36 insertions(+), 30 deletions(-)
> 
> 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 a421ba85bd6d..a0ca7e7a0903 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
> @@ -85,6 +85,10 @@ int smu_v13_0_init_microcode(struct smu_context *smu)
>   	const struct common_firmware_header *header;
>   	struct amdgpu_firmware_info *ucode = NULL;
>   
> +	/* doesn't need to load smu firmware in IOV mode */
> +	if (amdgpu_sriov_vf(adev))
> +		return 0;
> +
>   	switch (adev->asic_type) {
>   	case CHIP_ALDEBARAN:
>   		chip_name = "aldebaran";
> @@ -279,41 +283,43 @@ int smu_v13_0_setup_pptable(struct smu_context *smu)
>   	void *table;
>   	uint16_t version_major, version_minor;
>   
> +	if (!amdgpu_sriov_vf(adev)) {
> +		if (amdgpu_smu_pptable_id >= 0) {
> +			smu->smu_table.boot_values.pp_table_id = amdgpu_smu_pptable_id;
> +			dev_info(adev->dev, "override pptable id %d\n", amdgpu_smu_pptable_id);
> +		}

Easier to read without goto if we keep like
	if (amdgpu_sriov_vf(dev))
		pptable_id = 0 ; // Force load from VBIOS
	else
		pptable_id = smu->smu_table.boot_values.pp_table_id;

Thanks,
Lijo

> -	if (amdgpu_smu_pptable_id >= 0) {
> -		smu->smu_table.boot_values.pp_table_id = amdgpu_smu_pptable_id;
> -		dev_info(adev->dev, "override pptable id %d\n", amdgpu_smu_pptable_id);
> -	}
> -
> -	hdr = (const struct smc_firmware_header_v1_0 *) adev->pm.fw->data;
> -	version_major = le16_to_cpu(hdr->header.header_version_major);
> -	version_minor = le16_to_cpu(hdr->header.header_version_minor);
> -	if (version_major == 2 && smu->smu_table.boot_values.pp_table_id > 0) {
> -		dev_info(adev->dev, "use driver provided pptable %d\n", smu->smu_table.boot_values.pp_table_id);
> -		switch (version_minor) {
> -		case 1:
> -			ret = smu_v13_0_set_pptable_v2_1(smu, &table, &size,
> -							 smu->smu_table.boot_values.pp_table_id);
> -			break;
> -		default:
> -			ret = -EINVAL;
> -			break;
> +		hdr = (const struct smc_firmware_header_v1_0 *) adev->pm.fw->data;
> +		version_major = le16_to_cpu(hdr->header.header_version_major);
> +		version_minor = le16_to_cpu(hdr->header.header_version_minor);
> +		if (version_major == 2 && smu->smu_table.boot_values.pp_table_id > 0) {
> +			dev_info(adev->dev, "use driver provided pptable %d\n", smu->smu_table.boot_values.pp_table_id);
> +			switch (version_minor) {
> +			case 1:
> +				ret = smu_v13_0_set_pptable_v2_1(smu, &table, &size,
> +								 smu->smu_table.boot_values.pp_table_id);
> +				break;
> +			default:
> +				ret = -EINVAL;
> +				break;
> +			}
> +			if (ret)
> +				return ret;
> +			goto out;
>   		}
> -		if (ret)
> -			return ret;
> +	}
>   
> -	} else {
> -		dev_info(adev->dev, "use vbios provided pptable\n");
> -		index = get_index_into_master_table(atom_master_list_of_data_tables_v2_1,
> -						    powerplayinfo);
> +	dev_info(adev->dev, "use vbios provided pptable\n");
> +	index = get_index_into_master_table(atom_master_list_of_data_tables_v2_1,
> +					    powerplayinfo);
>   
> -		ret = amdgpu_atombios_get_data_table(adev, index, &atom_table_size, &frev, &crev,
> -						     (uint8_t **)&table);
> -		if (ret)
> -			return ret;
> -		size = atom_table_size;
> -	}
> +	ret = amdgpu_atombios_get_data_table(adev, index, &atom_table_size, &frev, &crev,
> +					     (uint8_t **)&table);
> +	if (ret)
> +		return ret;
>   
> +	size = atom_table_size;
> +out:
>   	if (!smu->smu_table.power_play_table)
>   		smu->smu_table.power_play_table = table;
>   	if (!smu->smu_table.power_play_table_size)
> 

Re: [PATCH 4/5] drm/amd/pm: change return value in aldebaran_get_power_limit()

[Lazar, Lijo]

On 8/11/2021 2:03 PM, Kevin Wang wrote:
> 1. change return value to avoid smu driver probe fails when FEATURE_PPT is
> not enabled.
> 2. if FEATURE_PPT is not enabled, set power limit value to 0.
> 
> Signed-off-by: Kevin Wang <kevin1.wang@amd.com>
> ---
>   .../gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c    | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> index fd04391364ac..e00ac43515bb 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> @@ -1194,8 +1194,19 @@ static int aldebaran_get_power_limit(struct smu_context *smu,
>   	uint32_t power_limit = 0;
>   	int ret;
>   
> -	if (!smu_cmn_feature_is_enabled(smu, SMU_FEATURE_PPT_BIT))
> -		return -EINVAL;
> +	if (!smu_cmn_feature_is_enabled(smu, SMU_FEATURE_PPT_BIT)) {
> +		if (current_power_limit)
> +			*current_power_limit = 0;
> +		if (default_power_limit)
> +			*default_power_limit = 0;
> +		if (max_power_limit)
> +			*max_power_limit = 0;
> +
> +		dev_err(smu->adev->dev,
> +			"the PPT feature is not enabled, set power limit to 0");
> +

Maybe downgrade to dev_warn as it is not an error from driver 
perspective, it only limits the information available.

Thanks,
Lijo

> +		return 0;
> +	}
>   
>   	/* Valid power data is available only from primary die.
>   	 * For secondary die show the value as 0.
> 

Re: [PATCH 1/5] drm/amd/pm: correct DPM_XGMI/VCN_DPM feature name

[Lazar, Lijo]

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

On 8/11/2021 2:03 PM, Kevin Wang wrote:
> the following feature is wrong, it will cause sysnode of pp_features show error:
> 1. DPM_XGMI
> 2. VCN_DPM
> 
> Signed-off-by: Kevin Wang <kevin1.wang@amd.com>
> ---
>   drivers/gpu/drm/amd/pm/inc/smu_types.h           |  1 -
>   .../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c    | 16 ++++++++--------
>   .../gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c   |  4 ++--
>   3 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/inc/smu_types.h b/drivers/gpu/drm/amd/pm/inc/smu_types.h
> index 1d3765b873df..3fdee6cf01ed 100644
> --- a/drivers/gpu/drm/amd/pm/inc/smu_types.h
> +++ b/drivers/gpu/drm/amd/pm/inc/smu_types.h
> @@ -297,7 +297,6 @@ enum smu_clk_type {
>          __SMU_DUMMY_MAP(DS_FCLK),                       	\
>          __SMU_DUMMY_MAP(DS_MP1CLK),                     	\
>          __SMU_DUMMY_MAP(DS_MP0CLK),                     	\
> -       __SMU_DUMMY_MAP(XGMI),                          	\
>          __SMU_DUMMY_MAP(XGMI_PER_LINK_PWR_DWN),          \
>          __SMU_DUMMY_MAP(DPM_GFX_PACE),                  	\
>          __SMU_DUMMY_MAP(MEM_VDDCI_SCALING),             	\
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
> index 6ec8492f71f5..b4d98465bb36 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
> @@ -163,14 +163,14 @@ static const struct cmn2asic_mapping arcturus_feature_mask_map[SMU_FEATURE_COUNT
>   	FEA_MAP(DPM_SOCCLK),
>   	FEA_MAP(DPM_FCLK),
>   	FEA_MAP(DPM_MP0CLK),
> -	ARCTURUS_FEA_MAP(SMU_FEATURE_XGMI_BIT, FEATURE_DPM_XGMI_BIT),
> +	FEA_MAP(DPM_XGMI),
>   	FEA_MAP(DS_GFXCLK),
>   	FEA_MAP(DS_SOCCLK),
>   	FEA_MAP(DS_LCLK),
>   	FEA_MAP(DS_FCLK),
>   	FEA_MAP(DS_UCLK),
>   	FEA_MAP(GFX_ULV),
> -	ARCTURUS_FEA_MAP(SMU_FEATURE_VCN_PG_BIT, FEATURE_DPM_VCN_BIT),
> +	ARCTURUS_FEA_MAP(SMU_FEATURE_VCN_DPM_BIT, FEATURE_DPM_VCN_BIT),
>   	FEA_MAP(RSMU_SMN_CG),
>   	FEA_MAP(WAFL_CG),
>   	FEA_MAP(PPT),
> @@ -721,13 +721,13 @@ static int arcturus_get_current_clk_freq_by_table(struct smu_context *smu,
>   			member_type = METRICS_AVERAGE_SOCCLK;
>   		break;
>   	case PPCLK_VCLK:
> -		if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_VCN_PG_BIT))
> +		if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_VCN_DPM_BIT))
>   			member_type = METRICS_CURR_VCLK;
>   		else
>   			member_type = METRICS_AVERAGE_VCLK;
>   		break;
>   	case PPCLK_DCLK:
> -		if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_VCN_PG_BIT))
> +		if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_VCN_DPM_BIT))
>   			member_type = METRICS_CURR_DCLK;
>   		else
>   			member_type = METRICS_AVERAGE_DCLK;
> @@ -1916,16 +1916,16 @@ static int arcturus_dpm_set_vcn_enable(struct smu_context *smu, bool enable)
>   	int ret = 0;
>   
>   	if (enable) {
> -		if (!smu_cmn_feature_is_enabled(smu, SMU_FEATURE_VCN_PG_BIT)) {
> -			ret = smu_cmn_feature_set_enabled(smu, SMU_FEATURE_VCN_PG_BIT, 1);
> +		if (!smu_cmn_feature_is_enabled(smu, SMU_FEATURE_VCN_DPM_BIT)) {
> +			ret = smu_cmn_feature_set_enabled(smu, SMU_FEATURE_VCN_DPM_BIT, 1);
>   			if (ret) {
>   				dev_err(smu->adev->dev, "[EnableVCNDPM] failed!\n");
>   				return ret;
>   			}
>   		}
>   	} else {
> -		if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_VCN_PG_BIT)) {
> -			ret = smu_cmn_feature_set_enabled(smu, SMU_FEATURE_VCN_PG_BIT, 0);
> +		if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_VCN_DPM_BIT)) {
> +			ret = smu_cmn_feature_set_enabled(smu, SMU_FEATURE_VCN_DPM_BIT, 0);
>   			if (ret) {
>   				dev_err(smu->adev->dev, "[DisableVCNDPM] failed!\n");
>   				return ret;
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> index 856eeaf293b8..97cc6fb9b22b 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> @@ -156,14 +156,14 @@ static const struct cmn2asic_mapping aldebaran_feature_mask_map[SMU_FEATURE_COUN
>   	ALDEBARAN_FEA_MAP(SMU_FEATURE_DPM_SOCCLK_BIT, 			FEATURE_DPM_SOCCLK_BIT),
>   	ALDEBARAN_FEA_MAP(SMU_FEATURE_DPM_FCLK_BIT, 			FEATURE_DPM_FCLK_BIT),
>   	ALDEBARAN_FEA_MAP(SMU_FEATURE_DPM_LCLK_BIT, 			FEATURE_DPM_LCLK_BIT),
> -	ALDEBARAN_FEA_MAP(SMU_FEATURE_XGMI_BIT, 				FEATURE_DPM_XGMI_BIT),
> +	ALDEBARAN_FEA_MAP(SMU_FEATURE_DPM_XGMI_BIT, 				FEATURE_DPM_XGMI_BIT),
>   	ALDEBARAN_FEA_MAP(SMU_FEATURE_DS_GFXCLK_BIT, 			FEATURE_DS_GFXCLK_BIT),
>   	ALDEBARAN_FEA_MAP(SMU_FEATURE_DS_SOCCLK_BIT, 			FEATURE_DS_SOCCLK_BIT),
>   	ALDEBARAN_FEA_MAP(SMU_FEATURE_DS_LCLK_BIT, 				FEATURE_DS_LCLK_BIT),
>   	ALDEBARAN_FEA_MAP(SMU_FEATURE_DS_FCLK_BIT, 				FEATURE_DS_FCLK_BIT),
>   	ALDEBARAN_FEA_MAP(SMU_FEATURE_DS_UCLK_BIT,				FEATURE_DS_UCLK_BIT),
>   	ALDEBARAN_FEA_MAP(SMU_FEATURE_GFX_SS_BIT, 				FEATURE_GFX_SS_BIT),
> -	ALDEBARAN_FEA_MAP(SMU_FEATURE_VCN_PG_BIT, 				FEATURE_DPM_VCN_BIT),
> +	ALDEBARAN_FEA_MAP(SMU_FEATURE_VCN_DPM_BIT, 				FEATURE_DPM_VCN_BIT),
>   	ALDEBARAN_FEA_MAP(SMU_FEATURE_RSMU_SMN_CG_BIT, 			FEATURE_RSMU_SMN_CG_BIT),
>   	ALDEBARAN_FEA_MAP(SMU_FEATURE_WAFL_CG_BIT, 				FEATURE_WAFL_CG_BIT),
>   	ALDEBARAN_FEA_MAP(SMU_FEATURE_PPT_BIT, 					FEATURE_PPT_BIT),
> 

Re: [PATCH 3/5] drm/amd/pm: change smu msg's attribute to allow working under sriov

[Lazar, Lijo]

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

On 8/11/2021 2:03 PM, Kevin Wang wrote:
> the following message is allowed in sriov mode:
> 1. GetEnabledSmuFeaturesLow
> 2. GetEnabledSmuFeaturesHigh
> 
> Signed-off-by: Kevin Wang <kevin1.wang@amd.com>
> ---
>   drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> index 97cc6fb9b22b..fd04391364ac 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> @@ -90,8 +90,8 @@ static const struct cmn2asic_msg_mapping aldebaran_message_map[SMU_MSG_MAX_COUNT
>   	MSG_MAP(GetDriverIfVersion,		     PPSMC_MSG_GetDriverIfVersion,		1),
>   	MSG_MAP(EnableAllSmuFeatures,		     PPSMC_MSG_EnableAllSmuFeatures,		0),
>   	MSG_MAP(DisableAllSmuFeatures,		     PPSMC_MSG_DisableAllSmuFeatures,		0),
> -	MSG_MAP(GetEnabledSmuFeaturesLow,	     PPSMC_MSG_GetEnabledSmuFeaturesLow,	0),
> -	MSG_MAP(GetEnabledSmuFeaturesHigh,	     PPSMC_MSG_GetEnabledSmuFeaturesHigh,	0),
> +	MSG_MAP(GetEnabledSmuFeaturesLow,	     PPSMC_MSG_GetEnabledSmuFeaturesLow,	1),
> +	MSG_MAP(GetEnabledSmuFeaturesHigh,	     PPSMC_MSG_GetEnabledSmuFeaturesHigh,	1),
>   	MSG_MAP(SetDriverDramAddrHigh,		     PPSMC_MSG_SetDriverDramAddrHigh,		1),
>   	MSG_MAP(SetDriverDramAddrLow,		     PPSMC_MSG_SetDriverDramAddrLow,		1),
>   	MSG_MAP(SetToolsDramAddrHigh,		     PPSMC_MSG_SetToolsDramAddrHigh,		0),
> 

Re: [PATCH 5/5] drm/amd/pm: change pp_dpm_sclk/mclk/fclk attribute is RO for aldebaran

[Lazar, Lijo]

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

On 8/11/2021 2:03 PM, Kevin Wang wrote:
> the following clock is only support voltage DPM, change attribute to RO:
> 1. pp_dpm_sclk
> 2. pp_dpm_mclk
> 3. pp_dpm_fclk
> 
> Signed-off-by: Kevin Wang <kevin1.wang@amd.com>
> ---
>   drivers/gpu/drm/amd/pm/amdgpu_pm.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> index 769f58d5ae1a..f894b34418df 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> @@ -2094,14 +2094,19 @@ static int default_attr_update(struct amdgpu_device *adev, struct amdgpu_device_
>   			*states = ATTR_STATE_UNSUPPORTED;
>   	}
>   
> -	if (asic_type == CHIP_ARCTURUS) {
> -		/* Arcturus does not support standalone mclk/socclk/fclk level setting */
> +	switch (asic_type) {
> +	case CHIP_ARCTURUS:
> +	case CHIP_ALDEBARAN:
> +		/* the Mi series card does not support standalone mclk/socclk/fclk level setting */
>   		if (DEVICE_ATTR_IS(pp_dpm_mclk) ||
>   		    DEVICE_ATTR_IS(pp_dpm_socclk) ||
>   		    DEVICE_ATTR_IS(pp_dpm_fclk)) {
>   			dev_attr->attr.mode &= ~S_IWUGO;
>   			dev_attr->store = NULL;
>   		}
> +		break;
> +	default:
> +		break;
>   	}
>   
>   	if (DEVICE_ATTR_IS(pp_dpm_dcefclk)) {
> 

Re: [PATCH 4/5] drm/amd/pm: change return value in aldebaran_get_power_limit()

[Wang, Kevin(Yang)]

[-- Attachment #1: Type: text/plain, Size: 2240 bytes --]

[AMD Official Use Only]

yes, it is a coding error, I will correct it.
thanks.

Best Regards,
Kevin
________________________________
From: Lazar, Lijo <Lijo.Lazar@amd.com>
Sent: Wednesday, August 11, 2021 5:45 PM
To: Wang, Kevin(Yang) <Kevin1.Wang@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Feng, Kenneth <Kenneth.Feng@amd.com>; Min, Frank <Frank.Min@amd.com>; Zhang, Hawking <Hawking.Zhang@amd.com>
Subject: Re: [PATCH 4/5] drm/amd/pm: change return value in aldebaran_get_power_limit()



On 8/11/2021 2:03 PM, Kevin Wang wrote:
> 1. change return value to avoid smu driver probe fails when FEATURE_PPT is
> not enabled.
> 2. if FEATURE_PPT is not enabled, set power limit value to 0.
>
> Signed-off-by: Kevin Wang <kevin1.wang@amd.com>
> ---
>   .../gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c    | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> index fd04391364ac..e00ac43515bb 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> @@ -1194,8 +1194,19 @@ static int aldebaran_get_power_limit(struct smu_context *smu,
>        uint32_t power_limit = 0;
>        int ret;
>
> -     if (!smu_cmn_feature_is_enabled(smu, SMU_FEATURE_PPT_BIT))
> -             return -EINVAL;
> +     if (!smu_cmn_feature_is_enabled(smu, SMU_FEATURE_PPT_BIT)) {
> +             if (current_power_limit)
> +                     *current_power_limit = 0;
> +             if (default_power_limit)
> +                     *default_power_limit = 0;
> +             if (max_power_limit)
> +                     *max_power_limit = 0;
> +
> +             dev_err(smu->adev->dev,
> +                     "the PPT feature is not enabled, set power limit to 0");
> +

Maybe downgrade to dev_warn as it is not an error from driver
perspective, it only limits the information available.

Thanks,
Lijo

> +             return 0;
> +     }
>
>        /* Valid power data is available only from primary die.
>         * For secondary die show the value as 0.
>

[-- Attachment #2: Type: text/html, Size: 4860 bytes --]