On Tue, Jun 05, 2018 at 07:51:17PM +0800, Rex Zhu wrote:
> v3: 1. Delete the dead gfx off code in powerplay late_init.
> 2. Revert v2.
> 3. call smu to power on gfx at the begin of ip_suspend/device_fini
> 4. only power off gfx ip in the end of gfx power gate function
> v2: Delete the dead gfx off code in ip_suspend.
>
> gfx off should be controlled by GFX IP.
> Powerplay only export interface to gfx ip.
> This logic is same as uvd/vce cg/pg.
>
> Signed-off-by: Rex Zhu <Rex.Zhu at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 ++++++------
> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 4 ++++
> drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 8 --------
> drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c | 4 ++--
> 4 files changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index df6ef9c..ee87fea 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1830,6 +1830,8 @@ static int amdgpu_device_ip_fini(struct amdgpu_device *adev)
> adev->ip_blocks[i].version->funcs->name, r);
> return r;
> }
> + if (adev->powerplay.pp_funcs->set_powergating_by_smu)
> + amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, false);
> r = adev->ip_blocks[i].version->funcs->hw_fini((void *)adev);
> /* XXX handle errors */
> if (r) {
> @@ -1940,12 +1942,6 @@ int amdgpu_device_ip_suspend(struct amdgpu_device *adev)
> if (amdgpu_sriov_vf(adev))
> amdgpu_virt_request_full_gpu(adev, false);
>
> - /* ungate SMC block powergating */
> - if (adev->powerplay.pp_feature & PP_GFXOFF_MASK)
> - amdgpu_device_ip_set_powergating_state(adev,
> - AMD_IP_BLOCK_TYPE_SMC,
> - AMD_CG_STATE_UNGATE);
> -
> /* ungate SMC block first */
> r = amdgpu_device_ip_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_SMC,
> AMD_CG_STATE_UNGATE);
> @@ -1953,6 +1949,10 @@ int amdgpu_device_ip_suspend(struct amdgpu_device *adev)
> DRM_ERROR("set_clockgating_state(ungate) SMC failed %d\n", r);
> }
>
> + /* call smu to disable gfx off feature first when suspend */
> + if (adev->powerplay.pp_funcs->set_powergating_by_smu)
> + amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, false);
> +
Still has risk for s3, we would better not put PG disabling (gfxoff) behind of CG
disabling, even here is SMC clockgating. It is to avoid any MMU using when
gfx is in "off" state.
Thanks,
Ray
> for (i = adev->num_ip_blocks - 1; i >= 0; i--) {
> if (!adev->ip_blocks[i].status.valid)
> continue;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index c382969..43c8023 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -3712,6 +3712,10 @@ static int gfx_v9_0_set_powergating_state(void *handle,
>
> /* update mgcg state */
> gfx_v9_0_update_gfx_mg_power_gating(adev, enable);
> +
> + /* set gfx off through smu */
> + if (enable && adev->powerplay.pp_funcs->set_powergating_by_smu)
> + amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, true);
> break;
> default:
> break;
> diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> index fe3ed8c..8bb3a9a 100644
> --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> @@ -180,7 +180,6 @@ static int pp_late_init(void *handle)
> {
> struct amdgpu_device *adev = handle;
> struct pp_hwmgr *hwmgr = adev->powerplay.pp_handle;
> - int ret;
>
> if (hwmgr && hwmgr->pm_en) {
> mutex_lock(&hwmgr->smu_lock);
> @@ -191,13 +190,6 @@ static int pp_late_init(void *handle)
> if (adev->pm.smu_prv_buffer_size != 0)
> pp_reserve_vram_for_smu(adev);
>
> - if (hwmgr->hwmgr_func->gfx_off_control &&
> - (hwmgr->feature_mask & PP_GFXOFF_MASK)) {
> - ret = hwmgr->hwmgr_func->gfx_off_control(hwmgr, true);
> - if (ret)
> - pr_err("gfx off enabling failed!\n");
> - }
> -
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> index b9e6dfb..5abae28 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> @@ -314,7 +314,7 @@ static int smu10_disable_gfx_off(struct pp_hwmgr *hwmgr)
>
> static int smu10_disable_dpm_tasks(struct pp_hwmgr *hwmgr)
> {
> - return smu10_disable_gfx_off(hwmgr);
> + return 0;
> }
>
> static int smu10_enable_gfx_off(struct pp_hwmgr *hwmgr)
> @@ -329,7 +329,7 @@ static int smu10_enable_gfx_off(struct pp_hwmgr *hwmgr)
>
> static int smu10_enable_dpm_tasks(struct pp_hwmgr *hwmgr)
> {
> - return smu10_enable_gfx_off(hwmgr);
> + return 0;
> }
>
> static int smu10_gfx_off_control(struct pp_hwmgr *hwmgr, bool enable)
> --
> 1.9.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
