[AMD Official Use Only]
> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
> Sent: Monday, January 24, 2022 1:03 PM
> To: Quan, Evan <Evan.Quan@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Chen, Guchun
> <Guchun.Chen@xxxxxxx>; Huang, Ray <Ray.Huang@xxxxxxx>
> Subject: Re: [PATCH 3/7] drm/amd/pm: drop the redundant 'supported'
> member of smu_feature structure
>
>
>
> On 1/24/2022 9:51 AM, Quan, Evan wrote:
> > [AMD Official Use Only]
> >
> >
> >
> >> -----Original Message-----
> >> From: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
> >> Sent: Friday, January 21, 2022 5:37 PM
> >> To: Quan, Evan <Evan.Quan@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> >> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Chen, Guchun
> >> <Guchun.Chen@xxxxxxx>; Huang, Ray <Ray.Huang@xxxxxxx>
> >> Subject: Re: [PATCH 3/7] drm/amd/pm: drop the redundant 'supported'
> >> member of smu_feature structure
> >>
> >>
> >>
> >> On 1/21/2022 2:56 PM, Quan, Evan wrote:
> >>> [AMD Official Use Only]
> >>>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
> >>>> Sent: Friday, January 21, 2022 4:52 PM
> >>>> To: Quan, Evan <Evan.Quan@xxxxxxx>; amd-
> gfx@xxxxxxxxxxxxxxxxxxxxx
> >>>> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Chen,
> Guchun
> >>>> <Guchun.Chen@xxxxxxx>; Huang, Ray <Ray.Huang@xxxxxxx>
> >>>> Subject: Re: [PATCH 3/7] drm/amd/pm: drop the redundant
> 'supported'
> >>>> member of smu_feature structure
> >>>>
> >>>>
> >>>>
> >>>> On 1/21/2022 1:14 PM, Evan Quan wrote:
> >>>>> As it has exactly the same value as the 'enabled' member and also
> >>>>> do the same thing.
> >>>>>
> >>>>
> >>>> I believe the original intention is different. We need to cache the
> >>>> features which are really supported by PMFW on that device on init.
> >>>> When a request comes through sysfs node for enable/disable of a
> >>>> feature, we should check against this and disallow those which are
> >>>> not
> >> supported.
> >>> [Quan, Evan] Uh, I doubt it was designed with such intention. As if
> >>> it was,
> >> there should be checks in ->get_allowed_feature_mask(e.g.
> >> navi10_get_allowed_feature_mask) whether driver tries to
> >> enable/disable different features from PMFW.
> >>> " When a request comes through sysfs node for enable/disable of a
> >> feature" If the sysfs node mentioned is "pp_features", I might be
> >> able to provide some insights why there is no such checks added when I
> made them.
> >> Considering there may be some dependency between those dpm
> >> features(e.g. gfx ulv depends on gfx dpm), we actually cannot guard
> >> user whether their settings will succeed. E.g. If PMFW supports both
> >> GFX ULV and DPM but user wants ULV only, the checks against pmfw
> >> supported features seem fine. But actually that cannot work due to
> >> the dependency between them. So, instead of applying some checks
> >> which actually cannot guard anything, we just let user take the risks.
> >>>
> >>
> >> Below is one example
> >>
> >> > - if (!smu_cmn_feature_is_supported(smu,
> >> SMU_FEATURE_FAN_CONTROL_BIT))
> >> > + if (!smu_cmn_feature_is_enabled(smu,
> >> SMU_FEATURE_FAN_CONTROL_BIT))
> >>
> >> Let's say user switched to manual mode, that time we disable fan
> >> control and move to manual mode. Next time when user requests auto
> >> mode, we check if fan control is originally supported on that platform and
> switch to auto.
> >>
> > [Quan, Evan] Logically yes. But in reality I assume this should not
> > happen. As during our post-silicon bringup, we enable those
> > features(and corresponding sysfs interfaces)support only after verified. It
> means if the feature(auto fan control) is not supported, the sysfs interface
> should be not visible to user.
>
> Well, there are multiple examples -
>
> amdgpu_asic_update_umd_stable_pstate(smu->adev,
> false);
> smu_deep_sleep_control(smu, true);
> smu_gfx_ulv_control(smu, true);
> smu_gpo_control(smu, true);
>
> This is yet another one. Some aspects are disabled while umd profiling and
> then they are enabled back when it's back to FW control. This is internal logic
> and it could be disastrous if we try to enable a feature without checking if it's
> really supported on production SKUs.
>
> >> Either way, we should cache the features which are originally
> >> supported on the platform (through a combination of PPTable features
> >> and allowed feature masks on ASICs which are applicable) and disallow
> >> anything outside of that.
> > [Quan, Evan] Although I doubt the necessity of such cache. But I do keep a
> structure member "feature->allowed" which does the similar job.
> > It may be more reasonable to expand its scope to handle the job described
> here.
> >
>
> It would be needed for both internal and external interfaces, I strongly
> suggest to keep it. The name 'supported' is more intuitive and 'allowed
> feature' concept from PMFW is not there for all ASICs, though I don't mind
> the name anyway.
[Quan, Evan] I'm fine to keep it. Please check the new patch series then.
BR
Evan
>
> Thanks,
> Lijo
>
> > BR
> > Evan
> >>
> >> Thanks,
> >> Lijo
> >>
> >>> BR
> >>> Evan
> >>>>
> >>>> Thanks,
> >>>> Lijo
> >>>>
> >>>>> Signed-off-by: Evan Quan <evan.quan@xxxxxxx>
> >>>>> Change-Id: I07c9a5ac5290cd0d88a40ce1768d393156419b5a
> >>>>> ---
> >>>>> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 1 -
> >>>>> drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 1 -
> >>>>> .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 8 ++++----
> >>>>> .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 10 +++++-----
> >>>>> .../gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 19
> ++++++++---
> >> ----
> >>>> ----
> >>>>> .../gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c | 5 +----
> >>>>> .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c | 5 +----
> >>>>> .../drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c | 3 ---
> >>>>> drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c | 17 ---------------
> --
> >>>>> drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h | 3 ---
> >>>>> 10 files changed, 19 insertions(+), 53 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> >>>>> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> >>>>> index 5ace30434e60..d3237b89f2c5 100644
> >>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> >>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> >>>>> @@ -949,7 +949,6 @@ static int smu_sw_init(void *handle)
> >>>>>
> >>>>> smu->pool_size = adev->pm.smu_prv_buffer_size;
> >>>>> smu->smu_feature.feature_num = SMU_FEATURE_MAX;
> >>>>> - bitmap_zero(smu->smu_feature.supported,
> SMU_FEATURE_MAX);
> >>>>> bitmap_zero(smu->smu_feature.enabled,
> SMU_FEATURE_MAX);
> >>>>> bitmap_zero(smu->smu_feature.allowed,
> SMU_FEATURE_MAX);
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> >>>>> b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> >>>>> index 18f24db7d202..3c0360772822 100644
> >>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> >>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> >>>>> @@ -388,7 +388,6 @@ struct smu_power_context {
> >>>>> struct smu_feature
> >>>>> {
> >>>>> uint32_t feature_num;
> >>>>> - DECLARE_BITMAP(supported, SMU_FEATURE_MAX);
> >>>>> DECLARE_BITMAP(allowed, SMU_FEATURE_MAX);
> >>>>> DECLARE_BITMAP(enabled, SMU_FEATURE_MAX);
> >>>>> };
> >>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> >>>>> b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> >>>>> index 84834c24a7e9..9fb290f9aaeb 100644
> >>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> >>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> >>>>> @@ -1625,8 +1625,8 @@ static int
> >>>>> navi10_display_config_changed(struct
> >>>> smu_context *smu)
> >>>>> int ret = 0;
> >>>>>
> >>>>> if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&
> >>>>> - smu_cmn_feature_is_supported(smu,
> >>>> SMU_FEATURE_DPM_DCEFCLK_BIT) &&
> >>>>> - smu_cmn_feature_is_supported(smu,
> >>>> SMU_FEATURE_DPM_SOCCLK_BIT)) {
> >>>>> + smu_cmn_feature_is_enabled(smu,
> >>>> SMU_FEATURE_DPM_DCEFCLK_BIT) &&
> >>>>> + smu_cmn_feature_is_enabled(smu,
> >>>> SMU_FEATURE_DPM_SOCCLK_BIT)) {
> >>>>> ret = smu_cmn_send_smc_msg_with_param(smu,
> >>>> SMU_MSG_NumOfDisplays,
> >>>>> smu-
> >display_config-
> >>>>> num_display,
> >>>>> NULL);
> >>>>> @@ -1864,13 +1864,13 @@ static int
> >>>> navi10_notify_smc_display_config(struct smu_context *smu)
> >>>>> min_clocks.dcef_clock_in_sr = smu->display_config-
> >>>>> min_dcef_deep_sleep_set_clk;
> >>>>> min_clocks.memory_clock = smu->display_config-
> >>>>> min_mem_set_clock;
> >>>>>
> >>>>> - if (smu_cmn_feature_is_supported(smu,
> >>>> SMU_FEATURE_DPM_DCEFCLK_BIT)) {
> >>>>> + if (smu_cmn_feature_is_enabled(smu,
> >>>> SMU_FEATURE_DPM_DCEFCLK_BIT)) {
> >>>>> clock_req.clock_type = amd_pp_dcef_clock;
> >>>>> clock_req.clock_freq_in_khz = min_clocks.dcef_clock
> * 10;
> >>>>>
> >>>>> ret =
> smu_v11_0_display_clock_voltage_request(smu,
> >>>> &clock_req);
> >>>>> if (!ret) {
> >>>>> - if (smu_cmn_feature_is_supported(smu,
> >>>> SMU_FEATURE_DS_DCEFCLK_BIT)) {
> >>>>> + if (smu_cmn_feature_is_enabled(smu,
> >>>> SMU_FEATURE_DS_DCEFCLK_BIT)) {
> >>>>> ret =
> >>>> smu_cmn_send_smc_msg_with_param(smu,
> >>>>>
> >>>> SMU_MSG_SetMinDeepSleepDcefclk,
> >>>>>
> >>>> min_clocks.dcef_clock_in_sr/100, diff --git
> >>>>> a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> >>>>> b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> >>>>> index 651fe748e423..d568d6137a00 100644
> >>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> >>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> >>>>> @@ -1281,8 +1281,8 @@ static int
> >>>> sienna_cichlid_display_config_changed(struct smu_context *smu)
> >>>>> int ret = 0;
> >>>>>
> >>>>> if ((smu->watermarks_bitmap & WATERMARKS_EXIST) &&
> >>>>> - smu_cmn_feature_is_supported(smu,
> >>>> SMU_FEATURE_DPM_DCEFCLK_BIT) &&
> >>>>> - smu_cmn_feature_is_supported(smu,
> >>>> SMU_FEATURE_DPM_SOCCLK_BIT)) {
> >>>>> + smu_cmn_feature_is_enabled(smu,
> >>>> SMU_FEATURE_DPM_DCEFCLK_BIT) &&
> >>>>> + smu_cmn_feature_is_enabled(smu,
> >>>> SMU_FEATURE_DPM_SOCCLK_BIT)) {
> >>>>> #if 0
> >>>>> ret = smu_cmn_send_smc_msg_with_param(smu,
> >>>> SMU_MSG_NumOfDisplays,
> >>>>> smu-
> >display_config-
> >>>>> num_display, @@ -1521,13 +1521,13 @@ static int
> >>>>> sienna_cichlid_notify_smc_display_config(struct smu_context
> >>>> *smu)
> >>>>> min_clocks.dcef_clock_in_sr = smu->display_config-
> >>>>> min_dcef_deep_sleep_set_clk;
> >>>>> min_clocks.memory_clock = smu->display_config-
> >>>>> min_mem_set_clock;
> >>>>>
> >>>>> - if (smu_cmn_feature_is_supported(smu,
> >>>> SMU_FEATURE_DPM_DCEFCLK_BIT)) {
> >>>>> + if (smu_cmn_feature_is_enabled(smu,
> >>>> SMU_FEATURE_DPM_DCEFCLK_BIT)) {
> >>>>> clock_req.clock_type = amd_pp_dcef_clock;
> >>>>> clock_req.clock_freq_in_khz = min_clocks.dcef_clock
> * 10;
> >>>>>
> >>>>> ret =
> smu_v11_0_display_clock_voltage_request(smu,
> >>>> &clock_req);
> >>>>> if (!ret) {
> >>>>> - if (smu_cmn_feature_is_supported(smu,
> >>>> SMU_FEATURE_DS_DCEFCLK_BIT)) {
> >>>>> + if (smu_cmn_feature_is_enabled(smu,
> >>>> SMU_FEATURE_DS_DCEFCLK_BIT)) {
> >>>>> ret =
> >>>> smu_cmn_send_smc_msg_with_param(smu,
> >>>>>
> >>>> SMU_MSG_SetMinDeepSleepDcefclk,
> >>>>>
> >>>> min_clocks.dcef_clock_in_sr/100, @@ -3752,7 +3752,7 @@
> >>>>> static int sienna_cichlid_gpo_control(struct smu_context *smu,
> >>>>> int ret = 0;
> >>>>>
> >>>>>
> >>>>> - if (smu_cmn_feature_is_supported(smu,
> >>>> SMU_FEATURE_DPM_GFX_GPO_BIT)) {
> >>>>> + if (smu_cmn_feature_is_enabled(smu,
> >>>> SMU_FEATURE_DPM_GFX_GPO_BIT)) {
> >>>>> ret = smu_cmn_get_smc_version(smu, NULL,
> >>>> &smu_version);
> >>>>> if (ret)
> >>>>> return ret;
> >>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> >>>>> b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> >>>>> index b58a4c2823c2..b69c2ecc8e25 100644
> >>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> >>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> >>>>> @@ -808,7 +808,6 @@ int
> smu_v11_0_system_features_control(struct
> >>>> smu_context *smu,
> >>>>> return ret;
> >>>>>
> >>>>> bitmap_zero(feature->enabled, feature->feature_num);
> >>>>> - bitmap_zero(feature->supported, feature->feature_num);
> >>>>>
> >>>>> if (en) {
> >>>>> ret = smu_cmn_get_enabled_mask(smu,
> feature_mask, 2);
> >>>> @@ -817,8
> >>>>> +816,6 @@ int smu_v11_0_system_features_control(struct
> >> smu_context
> >>>>> *smu,
> >>>>>
> >>>>> bitmap_copy(feature->enabled, (unsigned long
> >>>> *)&feature_mask,
> >>>>> feature->feature_num);
> >>>>> - bitmap_copy(feature->supported, (unsigned long
> >>>> *)&feature_mask,
> >>>>> - feature->feature_num);
> >>>>> }
> >>>>>
> >>>>> return ret;
> >>>>> @@ -1186,7 +1183,7 @@ smu_v11_0_auto_fan_control(struct
> >>>> smu_context *smu, bool auto_fan_control)
> >>>>> {
> >>>>> int ret = 0;
> >>>>>
> >>>>> - if (!smu_cmn_feature_is_supported(smu,
> >>>> SMU_FEATURE_FAN_CONTROL_BIT))
> >>>>> + if (!smu_cmn_feature_is_enabled(smu,
> >>>> SMU_FEATURE_FAN_CONTROL_BIT))
> >>>>> return 0;
> >>>>>
> >>>>> ret = smu_cmn_feature_set_enabled(smu,
> >>>> SMU_FEATURE_FAN_CONTROL_BIT,
> >>>>> auto_fan_control); @@ -1607,7 +1604,7 @@ bool
> >>>> smu_v11_0_baco_is_support(struct smu_context *smu)
> >>>>> return false;
> >>>>>
> >>>>> /* Arcturus does not support this bit mask */
> >>>>> - if (smu_cmn_feature_is_supported(smu,
> SMU_FEATURE_BACO_BIT)
> >>>> &&
> >>>>> + if (smu_cmn_feature_is_enabled(smu,
> SMU_FEATURE_BACO_BIT)
> >>>> &&
> >>>>> !smu_cmn_feature_is_enabled(smu,
> SMU_FEATURE_BACO_BIT))
> >>>>> return false;
> >>>>>
> >>>>> @@ -2150,7 +2147,7 @@ int smu_v11_0_gfx_ulv_control(struct
> >>>> smu_context *smu,
> >>>>> {
> >>>>> int ret = 0;
> >>>>>
> >>>>> - if (smu_cmn_feature_is_supported(smu,
> >>>> SMU_FEATURE_GFX_ULV_BIT))
> >>>>> + if (smu_cmn_feature_is_enabled(smu,
> >>>> SMU_FEATURE_GFX_ULV_BIT))
> >>>>> ret = smu_cmn_feature_set_enabled(smu,
> >>>> SMU_FEATURE_GFX_ULV_BIT,
> >>>>> enablement);
> >>>>>
> >>>>> return ret;
> >>>>> @@ -2162,7 +2159,7 @@ int smu_v11_0_deep_sleep_control(struct
> >>>> smu_context *smu,
> >>>>> struct amdgpu_device *adev = smu->adev;
> >>>>> int ret = 0;
> >>>>>
> >>>>> - if (smu_cmn_feature_is_supported(smu,
> >>>> SMU_FEATURE_DS_GFXCLK_BIT)) {
> >>>>> + if (smu_cmn_feature_is_enabled(smu,
> >>>> SMU_FEATURE_DS_GFXCLK_BIT)) {
> >>>>> ret = smu_cmn_feature_set_enabled(smu,
> >>>> SMU_FEATURE_DS_GFXCLK_BIT, enablement);
> >>>>> if (ret) {
> >>>>> dev_err(adev->dev, "Failed to %s GFXCLK
> DS!\n",
> >>>> enablement ?
> >>>>> "enable" : "disable"); @@ -2170,7 +2167,7 @@ int
> >>>> smu_v11_0_deep_sleep_control(struct smu_context *smu,
> >>>>> }
> >>>>> }
> >>>>>
> >>>>> - if (smu_cmn_feature_is_supported(smu,
> >>>> SMU_FEATURE_DS_UCLK_BIT)) {
> >>>>> + if (smu_cmn_feature_is_enabled(smu,
> >>>> SMU_FEATURE_DS_UCLK_BIT)) {
> >>>>> ret = smu_cmn_feature_set_enabled(smu,
> >>>> SMU_FEATURE_DS_UCLK_BIT, enablement);
> >>>>> if (ret) {
> >>>>> dev_err(adev->dev, "Failed to %s UCLK
> DS!\n",
> >>>> enablement ?
> >>>>> "enable" : "disable"); @@ -2178,7 +2175,7 @@ int
> >>>> smu_v11_0_deep_sleep_control(struct smu_context *smu,
> >>>>> }
> >>>>> }
> >>>>>
> >>>>> - if (smu_cmn_feature_is_supported(smu,
> >>>> SMU_FEATURE_DS_FCLK_BIT)) {
> >>>>> + if (smu_cmn_feature_is_enabled(smu,
> >>>> SMU_FEATURE_DS_FCLK_BIT)) {
> >>>>> ret = smu_cmn_feature_set_enabled(smu,
> >>>> SMU_FEATURE_DS_FCLK_BIT, enablement);
> >>>>> if (ret) {
> >>>>> dev_err(adev->dev, "Failed to %s FCLK
> DS!\n",
> >>>> enablement ?
> >>>>> "enable" : "disable"); @@ -2186,7 +2183,7 @@ int
> >>>> smu_v11_0_deep_sleep_control(struct smu_context *smu,
> >>>>> }
> >>>>> }
> >>>>>
> >>>>> - if (smu_cmn_feature_is_supported(smu,
> >>>> SMU_FEATURE_DS_SOCCLK_BIT)) {
> >>>>> + if (smu_cmn_feature_is_enabled(smu,
> >>>> SMU_FEATURE_DS_SOCCLK_BIT)) {
> >>>>> ret = smu_cmn_feature_set_enabled(smu,
> >>>> SMU_FEATURE_DS_SOCCLK_BIT, enablement);
> >>>>> if (ret) {
> >>>>> dev_err(adev->dev, "Failed to %s SOCCLK
> DS!\n",
> >>>> enablement ?
> >>>>> "enable" : "disable"); @@ -2194,7 +2191,7 @@ int
> >>>> smu_v11_0_deep_sleep_control(struct smu_context *smu,
> >>>>> }
> >>>>> }
> >>>>>
> >>>>> - if (smu_cmn_feature_is_supported(smu,
> >>>> SMU_FEATURE_DS_LCLK_BIT)) {
> >>>>> + if (smu_cmn_feature_is_enabled(smu,
> >>>> SMU_FEATURE_DS_LCLK_BIT)) {
> >>>>> ret = smu_cmn_feature_set_enabled(smu,
> >>>> SMU_FEATURE_DS_LCLK_BIT, enablement);
> >>>>> if (ret) {
> >>>>> dev_err(adev->dev, "Failed to %s LCLK
> DS!\n",
> >>>> enablement ?
> >>>>> "enable" : "disable"); diff --git
> >>>>> a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
> >>>>> b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
> >>>>> index b4a3c9b8b54e..e72831aa4859 100644
> >>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
> >>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
> >>>>> @@ -1960,7 +1960,6 @@ static int
> >>>> vangogh_system_features_control(struct smu_context *smu, bool en)
> >>>>> RLC_STATUS_OFF,
> NULL);
> >>>>>
> >>>>> bitmap_zero(feature->enabled, feature->feature_num);
> >>>>> - bitmap_zero(feature->supported, feature->feature_num);
> >>>>>
> >>>>> if (!en)
> >>>>> return ret;
> >>>>> @@ -1971,8 +1970,6 @@ static int
> >>>>> vangogh_system_features_control(struct smu_context *smu, bool
> en)
> >>>>>
> >>>>> bitmap_copy(feature->enabled, (unsigned long
> *)&feature_mask,
> >>>>> feature->feature_num);
> >>>>> - bitmap_copy(feature->supported, (unsigned long
> *)&feature_mask,
> >>>>> - feature->feature_num);
> >>>>>
> >>>>> return 0;
> >>>>> }
> >>>>> @@ -1989,7 +1986,7 @@ static int vangogh_post_smu_init(struct
> >>>> smu_context *smu)
> >>>>> adev->gfx.config.max_sh_per_se *
> >>>>> adev->gfx.config.max_shader_engines;
> >>>>>
> >>>>> /* allow message will be sent after enable message on
> Vangogh*/
> >>>>> - if (smu_cmn_feature_is_supported(smu,
> >>>> SMU_FEATURE_DPM_GFXCLK_BIT) &&
> >>>>> + if (smu_cmn_feature_is_enabled(smu,
> >>>> SMU_FEATURE_DPM_GFXCLK_BIT) &&
> >>>>> (adev->pg_flags &
> AMD_PG_SUPPORT_GFX_PG)) {
> >>>>> ret = smu_cmn_send_smc_msg(smu,
> >>>> SMU_MSG_EnableGfxOff, NULL);
> >>>>> if (ret) {
> >>>>> 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 1754a3720179..c5d354175675 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
> >>>>> @@ -774,7 +774,6 @@ int
> smu_v13_0_system_features_control(struct
> >>>> smu_context *smu,
> >>>>> return ret;
> >>>>>
> >>>>> bitmap_zero(feature->enabled, feature->feature_num);
> >>>>> - bitmap_zero(feature->supported, feature->feature_num);
> >>>>>
> >>>>> if (en) {
> >>>>> ret = smu_cmn_get_enabled_mask(smu,
> feature_mask, 2);
> >>>> @@ -783,8
> >>>>> +782,6 @@ int smu_v13_0_system_features_control(struct
> >> smu_context
> >>>>> *smu,
> >>>>>
> >>>>> bitmap_copy(feature->enabled, (unsigned long
> >>>> *)&feature_mask,
> >>>>> feature->feature_num);
> >>>>> - bitmap_copy(feature->supported, (unsigned long
> >>>> *)&feature_mask,
> >>>>> - feature->feature_num);
> >>>>> }
> >>>>>
> >>>>> return ret;
> >>>>> @@ -1071,7 +1068,7 @@ smu_v13_0_auto_fan_control(struct
> >>>> smu_context *smu, bool auto_fan_control)
> >>>>> {
> >>>>> int ret = 0;
> >>>>>
> >>>>> - if (!smu_cmn_feature_is_supported(smu,
> >>>> SMU_FEATURE_FAN_CONTROL_BIT))
> >>>>> + if (!smu_cmn_feature_is_enabled(smu,
> >>>> SMU_FEATURE_FAN_CONTROL_BIT))
> >>>>> return 0;
> >>>>>
> >>>>> ret = smu_cmn_feature_set_enabled(smu,
> >>>> SMU_FEATURE_FAN_CONTROL_BIT,
> >>>>> auto_fan_control); diff --git
> >>>>> a/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c
> >>>>> b/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c
> >>>>> index f425827e2361..e9172622c0c4 100644
> >>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c
> >>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c
> >>>>> @@ -204,7 +204,6 @@ static int
> >>>> yellow_carp_system_features_control(struct smu_context *smu, bool
> >>>> en)
> >>>>> ret = smu_cmn_send_smc_msg(smu,
> >>>> SMU_MSG_PrepareMp1ForUnload,
> >>>>> NULL);
> >>>>>
> >>>>> bitmap_zero(feature->enabled, feature->feature_num);
> >>>>> - bitmap_zero(feature->supported, feature->feature_num);
> >>>>>
> >>>>> if (!en)
> >>>>> return ret;
> >>>>> @@ -215,8 +214,6 @@ static int
> >>>>> yellow_carp_system_features_control(struct smu_context *smu,
> bool
> >>>>> en)
> >>>>>
> >>>>> bitmap_copy(feature->enabled, (unsigned long
> *)&feature_mask,
> >>>>> feature->feature_num);
> >>>>> - bitmap_copy(feature->supported, (unsigned long
> *)&feature_mask,
> >>>>> - feature->feature_num);
> >>>>>
> >>>>> return 0;
> >>>>> }
> >>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> >>>>> b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> >>>>> index 50164ebed1cd..49bcabe9d708 100644
> >>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> >>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> >>>>> @@ -476,23 +476,6 @@ int smu_cmn_to_asic_specific_index(struct
> >>>> smu_context *smu,
> >>>>> }
> >>>>> }
> >>>>>
> >>>>> -int smu_cmn_feature_is_supported(struct smu_context *smu,
> >>>>> - enum smu_feature_mask mask)
> >>>>> -{
> >>>>> - struct smu_feature *feature = &smu->smu_feature;
> >>>>> - int feature_id;
> >>>>> -
> >>>>> - feature_id = smu_cmn_to_asic_specific_index(smu,
> >>>>> -
> >>>> CMN2ASIC_MAPPING_FEATURE,
> >>>>> - mask);
> >>>>> - if (feature_id < 0)
> >>>>> - return 0;
> >>>>> -
> >>>>> - WARN_ON(feature_id > feature->feature_num);
> >>>>> -
> >>>>> - return test_bit(feature_id, feature->supported);
> >>>>> -}
> >>>>> -
> >>>>> int smu_cmn_feature_is_enabled(struct smu_context *smu,
> >>>>> enum smu_feature_mask mask)
> >>>>> {
> >>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> >>>>> b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> >>>>> index 4e34c18c6063..19919182260a 100644
> >>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> >>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> >>>>> @@ -48,9 +48,6 @@ int smu_cmn_to_asic_specific_index(struct
> >>>> smu_context *smu,
> >>>>> enum
> smu_cmn2asic_mapping_type type,
> >>>>> uint32_t index);
> >>>>>
> >>>>> -int smu_cmn_feature_is_supported(struct smu_context *smu,
> >>>>> - enum smu_feature_mask mask);
> >>>>> -
> >>>>> int smu_cmn_feature_is_enabled(struct smu_context *smu,
> >>>>> enum smu_feature_mask mask);
> >>>>>
> >>>>>
