On Tue, Oct 9, 2018 at 8:45 AM Rex Zhu <rex....@amd.com> wrote: > > So there is no dependence between gfx/sdma/smu. > and for Vi, after IH hw_init, driver load all the smu/gfx/sdma > fw. for AI, fw loading is controlled by PSP, after psp hw init, > we call the function to check smu fw version. > > Signed-off-by: Rex Zhu <rex....@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 30 > ++++++++++++++++++++++ > drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 11 -------- > drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 8 ------ > drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c | 20 --------------- > drivers/gpu/drm/amd/powerplay/inc/hwmgr.h | 1 - > drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c | 8 ++---- > drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c | 5 ---- > 7 files changed, 32 insertions(+), 51 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 4787571..a6766b3 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -1525,6 +1525,24 @@ static int amdgpu_device_ip_early_init(struct > amdgpu_device *adev) > return 0; > } > > +static int amdgpu_device_fw_loading(struct amdgpu_device *adev, uint32_t > index) > +{ > + int r = 0; > + > + if ((adev->asic_type < CHIP_VEGA10 > + && (adev->ip_blocks[index].version->type == > AMD_IP_BLOCK_TYPE_IH)) > + || (adev->asic_type >= CHIP_VEGA10 > + && (adev->ip_blocks[index].version->type == > AMD_IP_BLOCK_TYPE_PSP))) {
This seems kind of fragile. If we change the order again at some point, it will break. How about we check whether hw_init/resume is done or not on the blocks we care about or move the checks into the callers and only call when we need it? > + if (adev->powerplay.pp_funcs->load_firmware) { > + r = > adev->powerplay.pp_funcs->load_firmware(adev->powerplay.pp_handle); > + if (r) { > + pr_err("firmware loading failed\n"); > + return r; > + } > + } > + } > + return 0; > +} > /** > * amdgpu_device_ip_init - run init for hardware IPs > * > @@ -1595,6 +1613,9 @@ static int amdgpu_device_ip_init(struct amdgpu_device > *adev) > return r; > } > adev->ip_blocks[i].status.hw = true; > + r = amdgpu_device_fw_loading(adev, i); > + if (r) > + return r; > } > > amdgpu_xgmi_add_device(adev); > @@ -2030,6 +2051,9 @@ static int amdgpu_device_ip_reinit_early_sriov(struct > amdgpu_device *adev) > DRM_INFO("RE-INIT: %s %s\n", > block->version->funcs->name, r?"failed":"succeeded"); > if (r) > return r; > + r = amdgpu_device_fw_loading(adev, i); > + if (r) > + return r; > } > } > > @@ -2098,6 +2122,9 @@ static int amdgpu_device_ip_resume_phase1(struct > amdgpu_device *adev) > > adev->ip_blocks[i].version->funcs->name, r); > return r; > } > + r = amdgpu_device_fw_loading(adev, i); > + if (r) > + return r; > } > } > > @@ -2134,6 +2161,9 @@ static int amdgpu_device_ip_resume_phase2(struct > amdgpu_device *adev) > adev->ip_blocks[i].version->funcs->name, r); > return r; > } > + r = amdgpu_device_fw_loading(adev, i); > + if (r) > + return r; > } > > return 0; > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c > b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c > index 8439f9a..3d0f277 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c > @@ -4175,20 +4175,9 @@ static void gfx_v8_0_rlc_start(struct amdgpu_device > *adev) > > static int gfx_v8_0_rlc_resume(struct amdgpu_device *adev) > { > - int r; > - > gfx_v8_0_rlc_stop(adev); > gfx_v8_0_rlc_reset(adev); > gfx_v8_0_init_pg(adev); > - > - if (adev->powerplay.pp_funcs->load_firmware) { > - r = > adev->powerplay.pp_funcs->load_firmware(adev->powerplay.pp_handle); > - if (r) { > - pr_err("firmware loading failed\n"); > - return r; > - } > - } > - > gfx_v8_0_rlc_start(adev); > > return 0; > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c > b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c > index 0bdde7f..6fb3eda 100644 > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c > @@ -788,14 +788,6 @@ static int sdma_v3_0_start(struct amdgpu_device *adev) > { > int r; > > - if (adev->powerplay.pp_funcs->load_firmware) { > - r = > adev->powerplay.pp_funcs->load_firmware(adev->powerplay.pp_handle); > - if (r) { > - pr_err("firmware loading failed\n"); > - return r; > - } > - } > - > /* disable sdma engine before programing it */ > sdma_v3_0_ctx_switch_enable(adev, false); > sdma_v3_0_enable(adev, false); > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c > b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c > index d552af2..47ac923 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c > @@ -89,7 +89,6 @@ int hwmgr_early_init(struct pp_hwmgr *hwmgr) > hwmgr_init_default_caps(hwmgr); > hwmgr_set_user_specify_caps(hwmgr); > hwmgr->fan_ctrl_is_in_default_mode = true; > - hwmgr->reload_fw = 1; > hwmgr_init_workload_prority(hwmgr); > > switch (hwmgr->chip_family) { > @@ -209,17 +208,6 @@ int hwmgr_hw_init(struct pp_hwmgr *hwmgr) > { > int ret = 0; > > - if (!hwmgr || !hwmgr->smumgr_funcs) > - return -EINVAL; > - > - if (hwmgr->smumgr_funcs->start_smu) { > - ret = hwmgr->smumgr_funcs->start_smu(hwmgr); > - if (ret) { > - pr_err("smc start failed\n"); > - return -EINVAL; > - } > - } > - > if (!hwmgr->pm_en) > return 0; > > @@ -301,7 +289,6 @@ int hwmgr_suspend(struct pp_hwmgr *hwmgr) > if (!hwmgr || !hwmgr->pm_en) > return 0; > > - hwmgr->reload_fw = true; > phm_disable_smc_firmware_ctf(hwmgr); > ret = psm_set_boot_states(hwmgr); > if (ret) > @@ -321,13 +308,6 @@ int hwmgr_resume(struct pp_hwmgr *hwmgr) > if (!hwmgr) > return -EINVAL; > > - if (hwmgr->smumgr_funcs && hwmgr->smumgr_funcs->start_smu) { > - if (hwmgr->smumgr_funcs->start_smu(hwmgr)) { > - pr_err("smc start failed\n"); > - return -EINVAL; > - } > - } > - > if (!hwmgr->pm_en) > return 0; > > diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h > b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h > index 35f2272..e5a60aa 100644 > --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h > +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h > @@ -734,7 +734,6 @@ struct pp_hwmgr { > void *smu_backend; > const struct pp_smumgr_func *smumgr_funcs; > bool is_kicker; > - bool reload_fw; > > enum PP_DAL_POWERLEVEL dal_power_level; > struct phm_dynamic_state_info dyn_state; > diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c > b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c > index 99b4e4f..3f51d54 100644 > --- a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c > +++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c > @@ -343,9 +343,6 @@ int smu7_request_smu_load_fw(struct pp_hwmgr *hwmgr) > uint32_t fw_to_load; > int r = 0; > > - if (!hwmgr->reload_fw) > - return 0; > - > amdgpu_ucode_init_bo(hwmgr->adev); > > if (smu_data->soft_regs_start) > @@ -432,10 +429,9 @@ int smu7_request_smu_load_fw(struct pp_hwmgr *hwmgr) > smu7_send_msg_to_smc_with_parameter(hwmgr, PPSMC_MSG_LoadUcodes, > fw_to_load); > > r = smu7_check_fw_load_finish(hwmgr, fw_to_load); > - if (!r) { > - hwmgr->reload_fw = 0; > + if (!r) > return 0; > - } > + > pr_err("SMU load firmware failed\n"); > > failed: > diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c > b/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c > index abbf2f2..f836d30 100644 > --- a/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c > +++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c > @@ -661,9 +661,6 @@ static int smu8_request_smu_load_fw(struct pp_hwmgr > *hwmgr) > uint32_t fw_to_check = 0; > int ret; > > - if (!hwmgr->reload_fw) > - return 0; > - > amdgpu_ucode_init_bo(hwmgr->adev); > > smu8_smu_populate_firmware_entries(hwmgr); > @@ -719,8 +716,6 @@ static int smu8_request_smu_load_fw(struct pp_hwmgr > *hwmgr) > return ret; > } > > - hwmgr->reload_fw = 0; > - > return 0; > } > > -- > 1.9.1 > > _______________________________________________ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx