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

Reply via email to