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 <alexdeucher@xxxxxxxxx> 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.Zhu at 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.Zhu at 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.deucher at 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 at 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 ... -------------- next part -------------- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20180309/a26c33ae/attachment-0001.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0001-drm-amd-pp-Simplified-the-avfs-btc-state-on-smu7.patch Type: text/x-patch Size: 15296 bytes Desc: 0001-drm-amd-pp-Simplified-the-avfs-btc-state-on-smu7.patch URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20180309/a26c33ae/attachment-0001.bin>