<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> i will addressed it before submit patch.</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> thanks.</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Best Regards,</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Kevin</div> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="divRplyFwdMsg" dir="ltr">From: Quan, Evan <Evan.Quan@amd.com> Sent: Thursday, July 25, 2019 5:29 PM To: Wang, Kevin(Yang) <Kevin1.Wang@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray <Ray.Huang@amd.com>; Feng, Kenneth <Kenneth.Feng@amd.com>; Wang, Kevin(Yang) <Kevin1.Wang@amd.com> Subject: RE: [PATCH 4/5] drm/amd/powerplay: move smu_feature_update_enable_state to up level <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">+ 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@amd.com> > -----Original Message----- > From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of > Wang, Kevin(Yang) > Sent: Thursday, July 25, 2019 1:11 PM > To: amd-gfx@lists.freedesktop.org > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray > <Ray.Huang@amd.com>; Feng, Kenneth <Kenneth.Feng@amd.com>; Wang, > Kevin(Yang) <Kevin1.Wang@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@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@lists.freedesktop.org > <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> </div> </div> </body> </html>