[PATCH V2 2/7] drm/amd/pm: unify the interface for retrieving enabled ppfeatures

Quan, Evan Evan.Quan at amd.com
Wed Jan 26 04:05:46 UTC 2022


[AMD Official Use Only]



> -----Original Message-----
> From: Alex Deucher <alexdeucher at gmail.com>
> Sent: Tuesday, January 25, 2022 11:58 PM
> To: Quan, Evan <Evan.Quan at amd.com>
> Cc: amd-gfx list <amd-gfx at lists.freedesktop.org>; Deucher, Alexander
> <Alexander.Deucher at amd.com>; Lazar, Lijo <Lijo.Lazar at amd.com>
> Subject: Re: [PATCH V2 2/7] drm/amd/pm: unify the interface for retrieving
> enabled ppfeatures
> 
> On Tue, Jan 25, 2022 at 4:00 AM Evan Quan <evan.quan at amd.com> wrote:
> >
> > Instead of having two which do the same thing.
> >
> > Signed-off-by: Evan Quan <evan.quan at amd.com>
> > Change-Id: I6302c9b5abdb999c4b7c83a0d1852181208b1c1f
> > ---
> >  .../amd/pm/swsmu/smu11/cyan_skillfish_ppt.c   |  2 +-
> >  .../gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c  |  6 +-
> > .../drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c  |  6 +-
> >  drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c        | 93 ++++++++-----------
> >  drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h        |  4 -
> >  5 files changed, 44 insertions(+), 67 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/cyan_skillfish_ppt.c
> > b/drivers/gpu/drm/amd/pm/swsmu/smu11/cyan_skillfish_ppt.c
> > index 2f57333e6071..cc080a0075ee 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/cyan_skillfish_ppt.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/cyan_skillfish_ppt.c
> > @@ -357,7 +357,7 @@ static bool cyan_skillfish_is_dpm_running(struct
> smu_context *smu)
> >         if (adev->in_suspend)
> >                 return false;
> >
> > -       ret = smu_cmn_get_enabled_32_bits_mask(smu, feature_mask, 2);
> > +       ret = smu_cmn_get_enabled_mask(smu, feature_mask, 2);
> >         if (ret)
> >                 return false;
> >
> > 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 721027917f81..b4a3c9b8b54e 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
> > @@ -507,7 +507,7 @@ static bool vangogh_is_dpm_running(struct
> smu_context *smu)
> >         if (adev->in_suspend)
> >                 return false;
> >
> > -       ret = smu_cmn_get_enabled_32_bits_mask(smu, feature_mask, 2);
> > +       ret = smu_cmn_get_enabled_mask(smu, feature_mask, 2);
> >
> >         if (ret)
> >                 return false;
> > @@ -1965,7 +1965,7 @@ static int
> vangogh_system_features_control(struct smu_context *smu, bool en)
> >         if (!en)
> >                 return ret;
> >
> > -       ret = smu_cmn_get_enabled_32_bits_mask(smu, feature_mask, 2);
> > +       ret = smu_cmn_get_enabled_mask(smu, feature_mask, 2);
> >         if (ret)
> >                 return ret;
> >
> > @@ -2182,7 +2182,7 @@ static const struct pptable_funcs
> vangogh_ppt_funcs = {
> >         .dpm_set_jpeg_enable = vangogh_dpm_set_jpeg_enable,
> >         .is_dpm_running = vangogh_is_dpm_running,
> >         .read_sensor = vangogh_read_sensor,
> > -       .get_enabled_mask = smu_cmn_get_enabled_32_bits_mask,
> > +       .get_enabled_mask = smu_cmn_get_enabled_mask,
> >         .get_pp_feature_mask = smu_cmn_get_pp_feature_mask,
> >         .set_watermarks_table = vangogh_set_watermarks_table,
> >         .set_driver_table_location =
> > smu_v11_0_set_driver_table_location,
> > 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 bd24a2632214..f425827e2361 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
> > @@ -209,7 +209,7 @@ static int
> yellow_carp_system_features_control(struct smu_context *smu, bool en)
> >         if (!en)
> >                 return ret;
> >
> > -       ret = smu_cmn_get_enabled_32_bits_mask(smu, feature_mask, 2);
> > +       ret = smu_cmn_get_enabled_mask(smu, feature_mask, 2);
> >         if (ret)
> >                 return ret;
> >
> > @@ -258,7 +258,7 @@ static bool yellow_carp_is_dpm_running(struct
> smu_context *smu)
> >         uint32_t feature_mask[2];
> >         uint64_t feature_enabled;
> >
> > -       ret = smu_cmn_get_enabled_32_bits_mask(smu, feature_mask, 2);
> > +       ret = smu_cmn_get_enabled_mask(smu, feature_mask, 2);
> >
> >         if (ret)
> >                 return false;
> > @@ -1174,7 +1174,7 @@ static const struct pptable_funcs
> yellow_carp_ppt_funcs = {
> >         .is_dpm_running = yellow_carp_is_dpm_running,
> >         .set_watermarks_table = yellow_carp_set_watermarks_table,
> >         .get_gpu_metrics = yellow_carp_get_gpu_metrics,
> > -       .get_enabled_mask = smu_cmn_get_enabled_32_bits_mask,
> > +       .get_enabled_mask = smu_cmn_get_enabled_mask,
> >         .get_pp_feature_mask = smu_cmn_get_pp_feature_mask,
> >         .set_driver_table_location = smu_v13_0_set_driver_table_location,
> >         .gfx_off_control = smu_v13_0_gfx_off_control, diff --git
> > a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> > b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> > index c3c679bf9d9f..50164ebed1cd 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> > @@ -545,67 +545,57 @@ int smu_cmn_get_enabled_mask(struct
> smu_context *smu,
> >                              uint32_t *feature_mask,
> >                              uint32_t num)  {
> > -       uint32_t feature_mask_high = 0, feature_mask_low = 0;
> >         struct smu_feature *feature = &smu->smu_feature;
> > +       struct amdgpu_device *adev = smu->adev;
> > +       uint32_t *feature_mask_high;
> > +       uint32_t *feature_mask_low;
> >         int ret = 0;
> >
> >         if (!feature_mask || num < 2)
> >                 return -EINVAL;
> >
> > -       if (bitmap_empty(feature->enabled, feature->feature_num)) {
> > -               ret = smu_cmn_send_smc_msg(smu,
> SMU_MSG_GetEnabledSmuFeaturesHigh, &feature_mask_high);
> > -               if (ret)
> > -                       return ret;
> > -
> > -               ret = smu_cmn_send_smc_msg(smu,
> SMU_MSG_GetEnabledSmuFeaturesLow, &feature_mask_low);
> > -               if (ret)
> > -                       return ret;
> > -
> > -               feature_mask[0] = feature_mask_low;
> > -               feature_mask[1] = feature_mask_high;
> > -       } else {
> > -               bitmap_copy((unsigned long *)feature_mask, feature->enabled,
> > +       if (!bitmap_empty(feature->enabled, feature->feature_num)) {
> > +               bitmap_copy((unsigned long *)feature_mask,
> > +                            feature->enabled,
> >                              feature->feature_num);
> > +               return 0;
> >         }
> >
> > -       return ret;
> > -}
> > -
> > -int smu_cmn_get_enabled_32_bits_mask(struct smu_context *smu,
> > -                                       uint32_t *feature_mask,
> > -                                       uint32_t num)
> > -{
> > -       uint32_t feature_mask_en_low = 0;
> > -       uint32_t feature_mask_en_high = 0;
> > -       struct smu_feature *feature = &smu->smu_feature;
> > -       int ret = 0;
> > -
> > -       if (!feature_mask || num < 2)
> > -               return -EINVAL;
> > -
> > -       if (bitmap_empty(feature->enabled, feature->feature_num)) {
> > -               ret = smu_cmn_send_smc_msg_with_param(smu,
> SMU_MSG_GetEnabledSmuFeatures, 0,
> > -                                                                                &feature_mask_en_low);
> > +       feature_mask_low = &feature_mask[0];
> > +       feature_mask_high = &feature_mask[1];
> >
> > +       switch (adev->asic_type) {
> > +       case CHIP_CYAN_SKILLFISH:
> > +       case CHIP_VANGOGH:
> > +       case CHIP_YELLOW_CARP:
> 
> Can you convert this to an SMU IP version check rather than an asic type
> check?
[Quan, Evan] Sure, will update these.

BR
Evan
> 
> > +               ret = smu_cmn_send_smc_msg_with_param(smu,
> > +                                                     SMU_MSG_GetEnabledSmuFeatures,
> > +                                                     0,
> > +
> > + feature_mask_low);
> >                 if (ret)
> >                         return ret;
> >
> > -               ret = smu_cmn_send_smc_msg_with_param(smu,
> SMU_MSG_GetEnabledSmuFeatures, 1,
> > -                                                                                &feature_mask_en_high);
> > -
> > +               ret = smu_cmn_send_smc_msg_with_param(smu,
> > +                                                     SMU_MSG_GetEnabledSmuFeatures,
> > +                                                     1,
> > +                                                     feature_mask_high);
> > +               break;
> > +       case CHIP_RENOIR:
> > +       /* other dGPU ASICs */
> > +       default:
> > +               ret = smu_cmn_send_smc_msg(smu,
> > +                                          SMU_MSG_GetEnabledSmuFeaturesHigh,
> > +                                          feature_mask_high);
> >                 if (ret)
> >                         return ret;
> >
> > -               feature_mask[0] = feature_mask_en_low;
> > -               feature_mask[1] = feature_mask_en_high;
> > -
> > -       } else {
> > -               bitmap_copy((unsigned long *)feature_mask, feature->enabled,
> > -                                feature->feature_num);
> > +               ret = smu_cmn_send_smc_msg(smu,
> > +                                          SMU_MSG_GetEnabledSmuFeaturesLow,
> > +                                          feature_mask_low);
> > +               break;
> >         }
> >
> >         return ret;
> > -
> >  }
> >
> >  uint64_t smu_cmn_get_indep_throttler_status(
> > @@ -710,20 +700,11 @@ size_t smu_cmn_get_pp_feature_mask(struct
> smu_context *smu,
> >         size_t size = 0;
> >         int ret = 0, i;
> >
> > -       if (!smu->is_apu ||
> > -           (smu->adev->asic_type == CHIP_RENOIR)) {
> > -               ret = smu_cmn_get_enabled_mask(smu,
> > -                                               feature_mask,
> > -                                               2);
> > -               if (ret)
> > -                       return 0;
> > -       } else {
> > -               ret = smu_cmn_get_enabled_32_bits_mask(smu,
> > -                                       feature_mask,
> > -                                       2);
> > -               if (ret)
> > -                       return 0;
> > -       }
> > +       ret = smu_cmn_get_enabled_mask(smu,
> > +                                      feature_mask,
> > +                                      2);
> > +       if (ret)
> > +               return 0;
> >
> >         size =  sysfs_emit_at(buf, size, "features high: 0x%08x low: 0x%08x\n",
> >                         feature_mask[1], feature_mask[0]); diff --git
> > a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> > b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> > index f0b4fb2a0960..4e34c18c6063 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> > @@ -61,10 +61,6 @@ int smu_cmn_get_enabled_mask(struct
> smu_context *smu,
> >                              uint32_t *feature_mask,
> >                              uint32_t num);
> >
> > -int smu_cmn_get_enabled_32_bits_mask(struct smu_context *smu,
> > -                                       uint32_t *feature_mask,
> > -                                       uint32_t num);
> > -
> >  uint64_t smu_cmn_get_indep_throttler_status(
> >                                         const unsigned long dep_status,
> >                                         const uint8_t *throttler_map);
> > --
> > 2.29.0
> >


More information about the amd-gfx mailing list