[PATCH 3/5] drm/amdgpu: Extract the function of fw loading out of powerplay
Alex Deucher
alexdeucher at gmail.com
Tue Oct 9 19:27:41 UTC 2018
On Tue, Oct 9, 2018 at 8:45 AM Rex Zhu <Rex.Zhu at 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.Zhu at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
More information about the amd-gfx
mailing list