[PATCH 4/5] drm/amd/powerplay: move smu_feature_update_enable_state to up level
Wang, Kevin(Yang)
Kevin1.Wang at amd.com
Thu Jul 25 09:50:05 UTC 2019
i will addressed it before submit patch.
thanks.
Best Regards,
Kevin
________________________________
From: Quan, Evan <Evan.Quan at amd.com>
Sent: Thursday, July 25, 2019 5:29 PM
To: Wang, Kevin(Yang) <Kevin1.Wang at amd.com>; amd-gfx at lists.freedesktop.org <amd-gfx at lists.freedesktop.org>
Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Huang, Ray <Ray.Huang at amd.com>; Feng, Kenneth <Kenneth.Feng at amd.com>; Wang, Kevin(Yang) <Kevin1.Wang at amd.com>
Subject: RE: [PATCH 4/5] drm/amd/powerplay: move smu_feature_update_enable_state to up level
+ feature_mask = 1UL << feature_id;
Use "ULL" here. That can guard it to be 64bits long even on 32bits target.
With that fixed, reviewed-by: Evan Quan <evan.quan at amd.com>
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of
> Wang, Kevin(Yang)
> Sent: Thursday, July 25, 2019 1:11 PM
> To: amd-gfx at lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Huang, Ray
> <Ray.Huang at amd.com>; Feng, Kenneth <Kenneth.Feng at amd.com>; Wang,
> Kevin(Yang) <Kevin1.Wang at amd.com>
> Subject: [PATCH 4/5] drm/amd/powerplay: move
> smu_feature_update_enable_state to up level
>
> this function is not ip or asic related function,
> so move it to top level as public api in smu.
>
> Signed-off-by: Kevin Wang <kevin1.wang at amd.com>
> ---
> drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 40
> ++++++++++++++++++-
> .../gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 4 +-
> drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 39 ------------------
> 3 files changed, 40 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 8563f9083f4e..e881de955388 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -507,6 +507,41 @@ int smu_feature_init_dpm(struct smu_context
> *smu)
>
> return ret;
> }
> +int smu_feature_update_enable_state(struct smu_context *smu, uint64_t
> feature_mask, bool enabled)
> +{
> + uint32_t feature_low = 0, feature_high = 0;
> + int ret = 0;
> +
> + if (!smu->pm_enabled)
> + return ret;
> +
> + feature_low = (feature_mask >> 0 ) & 0xffffffff;
> + feature_high = (feature_mask >> 32) & 0xffffffff;
> +
> + if (enabled) {
> + ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_EnableSmuFeaturesLow,
> + feature_low);
> + if (ret)
> + return ret;
> + ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_EnableSmuFeaturesHigh,
> + feature_high);
> + if (ret)
> + return ret;
> +
> + } else {
> + ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_DisableSmuFeaturesLow,
> + feature_low);
> + if (ret)
> + return ret;
> + ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_DisableSmuFeaturesHigh,
> + feature_high);
> + if (ret)
> + return ret;
> +
> + }
> +
> + return ret;
> +}
>
> int smu_feature_is_enabled(struct smu_context *smu, enum
> smu_feature_mask mask)
> {
> @@ -532,6 +567,7 @@ int smu_feature_set_enabled(struct smu_context
> *smu, enum smu_feature_mask mask,
> {
> struct smu_feature *feature = &smu->smu_feature;
> int feature_id;
> + uint64_t feature_mask = 0;
> int ret = 0;
>
> feature_id = smu_feature_get_index(smu, mask);
> @@ -540,8 +576,10 @@ int smu_feature_set_enabled(struct smu_context
> *smu, enum smu_feature_mask mask,
>
> WARN_ON(feature_id > feature->feature_num);
>
> + feature_mask = 1UL << feature_id;
> +
> mutex_lock(&feature->mutex);
> - ret = smu_feature_update_enable_state(smu, feature_id, enable);
> + ret = smu_feature_update_enable_state(smu, feature_mask,
> enable);
> if (ret)
> goto failed;
>
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> index ba2385026b89..abc2644b4c07 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> @@ -479,7 +479,6 @@ struct smu_funcs
> int (*init_display_count)(struct smu_context *smu, uint32_t count);
> int (*set_allowed_mask)(struct smu_context *smu);
> int (*get_enabled_mask)(struct smu_context *smu, uint32_t
> *feature_mask, uint32_t num);
> - int (*update_feature_enable_state)(struct smu_context *smu,
> uint32_t feature_id, bool enabled);
> int (*notify_display_change)(struct smu_context *smu);
> int (*get_power_limit)(struct smu_context *smu, uint32_t *limit,
> bool def);
> int (*set_power_limit)(struct smu_context *smu, uint32_t n);
> @@ -595,8 +594,6 @@ struct smu_funcs
> ((smu)->funcs->get_enabled_mask? (smu)->funcs-
> >get_enabled_mask((smu), (mask), (num)) : 0)
> #define smu_is_dpm_running(smu) \
> ((smu)->ppt_funcs->is_dpm_running ? (smu)->ppt_funcs-
> >is_dpm_running((smu)) : 0)
> -#define smu_feature_update_enable_state(smu, feature_id, enabled) \
> - ((smu)->funcs->update_feature_enable_state? (smu)->funcs-
> >update_feature_enable_state((smu), (feature_id), (enabled)) : 0)
> #define smu_notify_display_change(smu) \
> ((smu)->funcs->notify_display_change? (smu)->funcs-
> >notify_display_change((smu)) : 0)
> #define smu_store_powerplay_table(smu) \
> @@ -804,6 +801,7 @@ enum amd_dpm_forced_level
> smu_get_performance_level(struct smu_context *smu);
> int smu_force_performance_level(struct smu_context *smu, enum
> amd_dpm_forced_level level);
> int smu_set_display_count(struct smu_context *smu, uint32_t count);
> bool smu_clk_dpm_is_enabled(struct smu_context *smu, enum
> smu_clk_type clk_type);
> +int smu_feature_update_enable_state(struct smu_context *smu, uint64_t
> feature_mask, bool enabled);
> const char *smu_get_message_name(struct smu_context *smu, enum
> smu_message_type type);
> const char *smu_get_feature_name(struct smu_context *smu, enum
> smu_feature_mask feature);
>
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index ccf6af055d03..93f3ffb8b471 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -795,44 +795,6 @@ static int smu_v11_0_init_display_count(struct
> smu_context *smu, uint32_t count)
> return ret;
> }
>
> -static int smu_v11_0_update_feature_enable_state(struct smu_context
> *smu, uint32_t feature_id, bool enabled)
> -{
> - uint32_t feature_low = 0, feature_high = 0;
> - int ret = 0;
> -
> - if (!smu->pm_enabled)
> - return ret;
> - if (feature_id >= 0 && feature_id < 31)
> - feature_low = (1 << feature_id);
> - else if (feature_id > 31 && feature_id < 63)
> - feature_high = (1 << feature_id);
> - else
> - return -EINVAL;
> -
> - if (enabled) {
> - ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_EnableSmuFeaturesLow,
> - feature_low);
> - if (ret)
> - return ret;
> - ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_EnableSmuFeaturesHigh,
> - feature_high);
> - if (ret)
> - return ret;
> -
> - } else {
> - ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_DisableSmuFeaturesLow,
> - feature_low);
> - if (ret)
> - return ret;
> - ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_DisableSmuFeaturesHigh,
> - feature_high);
> - if (ret)
> - return ret;
> -
> - }
> -
> - return ret;
> -}
>
> static int smu_v11_0_set_allowed_mask(struct smu_context *smu)
> {
> @@ -1781,7 +1743,6 @@ static const struct smu_funcs smu_v11_0_funcs = {
> .set_allowed_mask = smu_v11_0_set_allowed_mask,
> .get_enabled_mask = smu_v11_0_get_enabled_mask,
> .system_features_control = smu_v11_0_system_features_control,
> - .update_feature_enable_state =
> smu_v11_0_update_feature_enable_state,
> .notify_display_change = smu_v11_0_notify_display_change,
> .get_power_limit = smu_v11_0_get_power_limit,
> .set_power_limit = smu_v11_0_set_power_limit,
> --
> 2.22.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20190725/74344cad/attachment-0001.html>
More information about the amd-gfx
mailing list