<div dir="ltr"><div>Nice cleanup. Patch is: </div>Reviewed-by: Alex Deucher <<a href="mailto:alexander.deucher@amd.com">alexander.deucher@amd.com</a>> <div><div><div class="gmail_extra"> <div class="gmail_quote">On Fri, Mar 9, 2018 at 5:55 AM, Zhu, Rex <<a href="mailto:Rex.Zhu@amd.com" target="_blank">Rex.Zhu@amd.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div dir="ltr"> <div id="m_7239844958464048745divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif" dir="ltr"> 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 <div style="color:rgb(0,0,0)"> <hr style="display:inline-block;width:98%"> <div id="m_7239844958464048745divRplyFwdMsg" dir="ltr">From: Alex Deucher <<a href="mailto:alexdeucher@gmail.com" target="_blank">alexdeucher@gmail.com</a>> 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 <div> </div> </div> <div class="m_7239844958464048745BodyFragment"> <div class="m_7239844958464048745PlainText"><div><div class="h5">On Wed, Mar 7, 2018 at 5:46 AM, Rex Zhu <<a href="mailto:Rex.Zhu@amd.com" target="_blank">Rex.Zhu@amd.com</a>> 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 <<a href="mailto:Rex.Zhu@amd.com" target="_blank">Rex.Zhu@amd.com</a>> > --- > 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 <<a href="mailto:alexander.deucher@amd.com" target="_blank">alexander.deucher@amd.com</a>> > > 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 > <a href="mailto:amd-gfx@lists.freedesktop.org" target="_blank">amd-gfx@lists.freedesktop.org</a> > <a id="m_7239844958464048745LPlnk605140" href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" target="_blank"> https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> </div></div><div style="margin-bottom:20px;overflow:auto;width:100%;text-indent:0px" id="m_7239844958464048745LPBorder_GT_15205915258620.2686861345053829"> <table style="width:90%;background-color:rgb(255,255,255);overflow:auto;padding-top:20px;padding-bottom:20px;margin-top:20px;border-top:1px dotted rgb(200,200,200);border-bottom:1px dotted rgb(200,200,200)" id="m_7239844958464048745LPContainer_15205915258590.9282736731640967" cellspacing="0"> <tbody> <tr style="border-spacing:0px" valign="top"> <td colspan="2" style="vertical-align:top;padding:0px;display:table-cell" id="m_7239844958464048745TextCell_15205915258600.9720307957613028"> <div id="m_7239844958464048745LPRemovePreviewContainer_15205915258600.5747894184804376"></div> <div style="color:rgb(0,120,215);font-weight:400;font-size:21px;font-family:"wf_segoe-ui_light","Segoe UI Light","Segoe WP Light","Segoe UI","Segoe WP",Tahoma,Arial,sans-serif;line-height:21px" id="m_7239844958464048745LPTitle_15205915258600.6420531425137407"> <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" style="text-decoration:none" id="m_7239844958464048745LPUrlAnchor_15205915258610.5529589204654419" target="_blank">amd-gfx Info Page - freedesktop.org</a></div> <div style="margin:10px 0px 16px;color:rgb(102,102,102);font-weight:400;font-family:"wf_segoe-ui_normal","Segoe UI","Segoe WP",Tahoma,Arial,sans-serif;font-size:14px;line-height:14px" id="m_7239844958464048745LPMetadata_15205915258610.39476917506447184"> <a href="http://lists.freedesktop.org" target="_blank">lists.freedesktop.org</a></div> <div style="display:block;color:rgb(102,102,102);font-weight:400;font-family:"wf_segoe-ui_normal","Segoe UI","Segoe WP",Tahoma,Arial,sans-serif;font-size:14px;line-height:20px;max-height:100px;overflow:hidden" id="m_7239844958464048745LPDescription_15205915258620.7443088027813343"> Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the following form. Use of all <a href="http://freedesktop.org" target="_blank">freedesktop.org</a> lists is subject to our Code of ...</div> </td> </tr> </tbody> </table> </div> </div> </div> </div> </div> </div> </blockquote></div> </div></div></div></div>