Nice cleanup.  Patch is:
Reviewed-by: Alex Deucher <alexander.deuc...@amd.com>

On Fri, Mar 9, 2018 at 5:55 AM, Zhu, Rex <rex....@amd.com> wrote:

> Hi Alex,
>
>
> The avfs btc state is only used in driver and not interact with HW.
>
> and we only need to know whether this feature is supported or not by HW.
>
> so delete the complex define of avfs btc state and simplify related code.
>
> Please review the attached patch.
>
>
> Best Regards
>
> Rex
>
> ------------------------------
> *From:* Alex Deucher <alexdeuc...@gmail.com>
> *Sent:* Thursday, March 8, 2018 1:12 AM
> *To:* Zhu, Rex
> *Cc:* amd-gfx list
> *Subject:* Re: [PATCH 1/8] drm/amd/pp: Refine code for smu7 in powerplay
>
> On Wed, Mar 7, 2018 at 5:46 AM, Rex Zhu <rex....@amd.com> wrote:
> > 1. Add avfs_support in hwmgr to avoid to visit smu backend struct in
> >    hwmgr.so do not need to include smu7_smumgr.h under hwmgr folder.
> > 2. simplify the list of enum AVFS_BTC_STATUS
> >
> > Change-Id: I04c769972deff797229339f3ccb1c442b35768a2
> > Signed-off-by: Rex Zhu <rex....@amd.com>
> > ---
> >  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c        | 14
> ++------------
> >  drivers/gpu/drm/amd/powerplay/inc/hwmgr.h               |  2 +-
> >  drivers/gpu/drm/amd/powerplay/inc/smumgr.h              | 10 +---------
> >  drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c      |  4 +++-
> >  drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c |  4 +++-
> >  5 files changed, 10 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > index d4d1d2e..da25e7f 100644
> > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> > @@ -40,7 +40,6 @@
> >
> >  #include "hwmgr.h"
> >  #include "smu7_hwmgr.h"
> > -#include "smu7_smumgr.h"
> >  #include "smu_ucode_xfer_vi.h"
> >  #include "smu7_powertune.h"
> >  #include "smu7_dyn_defaults.h"
> > @@ -1353,12 +1352,7 @@ static int smu7_enable_dpm_tasks(struct pp_hwmgr
> *hwmgr)
> >
> >  static int smu7_avfs_control(struct pp_hwmgr *hwmgr, bool enable)
> >  {
> > -       struct smu7_smumgr *smu_data = (struct smu7_smumgr
> *)(hwmgr->smu_backend);
> > -
> > -       if (smu_data == NULL)
> > -               return -EINVAL;
> > -
> > -       if (smu_data->avfs.avfs_btc_status == AVFS_BTC_NOTSUPPORTED)
> > +       if (!hwmgr->avfs_supported)
> >                 return 0;
> >
> >         if (enable) {
> > @@ -1382,13 +1376,9 @@ static int smu7_avfs_control(struct pp_hwmgr
> *hwmgr, bool enable)
> >
> >  static int smu7_update_avfs(struct pp_hwmgr *hwmgr)
> >  {
> > -       struct smu7_smumgr *smu_data = (struct smu7_smumgr
> *)(hwmgr->smu_backend);
> >         struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
> >
> > -       if (smu_data == NULL)
> > -               return -EINVAL;
> > -
> > -       if (smu_data->avfs.avfs_btc_status == AVFS_BTC_NOTSUPPORTED)
> > +       if (!hwmgr->avfs_supported)
> >                 return 0;
> >
> >         if (data->need_update_smu7_dpm_table & DPMTABLE_OD_UPDATE_VDDC)
> {
> > diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > index b151ad85..312fbc3 100644
> > --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > @@ -748,7 +748,7 @@ struct pp_hwmgr {
> >         struct pp_power_state    *uvd_ps;
> >         struct amd_pp_display_configuration display_config;
> >         uint32_t feature_mask;
> > -
> > +       bool avfs_supported;
> >         /* UMD Pstate */
> >         bool en_umd_pstate;
> >         uint32_t power_profile_mode;
> > diff --git a/drivers/gpu/drm/amd/powerplay/inc/smumgr.h
> b/drivers/gpu/drm/amd/powerplay/inc/smumgr.h
> > index 9bba0a0..f0ef02a 100644
> > --- a/drivers/gpu/drm/amd/powerplay/inc/smumgr.h
> > +++ b/drivers/gpu/drm/amd/powerplay/inc/smumgr.h
> > @@ -28,23 +28,15 @@
> >
> >  enum AVFS_BTC_STATUS {
> >         AVFS_BTC_BOOT = 0,
> > -       AVFS_BTC_BOOT_STARTEDSMU,
> >         AVFS_LOAD_VIRUS,
> >         AVFS_BTC_VIRUS_LOADED,
> >         AVFS_BTC_VIRUS_FAIL,
> >         AVFS_BTC_COMPLETED_PREVIOUSLY,
> >         AVFS_BTC_ENABLEAVFS,
> > -       AVFS_BTC_STARTED,
> >         AVFS_BTC_FAILED,
> > -       AVFS_BTC_RESTOREVFT_FAILED,
> > -       AVFS_BTC_SAVEVFT_FAILED,
> >         AVFS_BTC_DPMTABLESETUP_FAILED,
> > -       AVFS_BTC_COMPLETED_UNSAVED,
> > -       AVFS_BTC_COMPLETED_SAVED,
> > -       AVFS_BTC_COMPLETED_RESTORED,
> >         AVFS_BTC_DISABLED,
> > -       AVFS_BTC_NOTSUPPORTED,
> > -       AVFS_BTC_SMUMSG_ERROR
> > +       AVFS_BTC_NOTSUPPORTED
> >  };
>
> This isn't used to interact with the hw anywhere is it?  If so, this
> will break that.  It should probably be split out as a separate patch
> since it's not really directly related to the rest of this patch.
> With those addresses this patch is:
> Reviewed-by: Alex Deucher <alexander.deuc...@amd.com>
>
> >
> >  enum SMU_TABLE {
> > diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
> b/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
> > index 0b2b5d1..573c9be 100644
> > --- a/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
> > +++ b/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
> > @@ -360,8 +360,10 @@ static bool fiji_is_hw_avfs_present(struct pp_hwmgr
> *hwmgr)
> >
> >         if (!atomctrl_read_efuse(hwmgr->device, AVFS_EN_LSB,
> AVFS_EN_MSB,
> >                         mask, &efuse)) {
> > -               if (efuse)
> > +               if (efuse) {
> > +                       hwmgr->avfs_supported = true;
> >                         return true;
> > +               }
> >         }
> >         return false;
> >  }
> > diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c
> b/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c
> > index 632d1ca..1075ec1 100644
> > --- a/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c
> > +++ b/drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c
> > @@ -357,8 +357,10 @@ static bool polaris10_is_hw_avfs_present(struct
> pp_hwmgr *hwmgr)
> >
> >         efuse = cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC,
> ixSMU_EFUSE_0 + (49*4));
> >         efuse &= 0x00000001;
> > -       if (efuse)
> > +       if (efuse) {
> > +               hwmgr->avfs_supported = true;
> >                 return true;
> > +       }
> >
> >         return false;
> >  }
> > --
> > 1.9.1
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> amd-gfx Info Page - freedesktop.org
> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
> lists.freedesktop.org
> Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the following
> form. Use of all freedesktop.org lists is subject to our Code of ...
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to