[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.
> 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.
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);
> >>>
> >>>
