[PATCH 11/11] drm/amdgpu: put the SMC into the proper state on reset/unload

[Quan, Evan]

I think it's OK to leave the S3 and BACO as they were for now.
Series is reviewed-by Evan Quan <evan.quan at amd.com>

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Alex
> Deucher
> Sent: Friday, July 26, 2019 11:22 AM
> To: amd-gfx at lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>
> Subject: [PATCH 11/11] drm/amdgpu: put the SMC into the proper state on
> reset/unload
> 
> When doing a GPU reset or unloading the driver, we need to put the SMU
> into the apprpriate state for the re-init after the reset or unload to reliably
> work.
> 
> I don't think this is necessary for BACO because the SMU actually controls the
> BACO state to it needs to be active.
> 
> For suspend (S3), the asic is put into D3 so the SMU would be powered down
> so I don't think we need to put the SMU into any special state.
> 
> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 27
> ++++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  2 ++
>  3 files changed, 30 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index e661417ba9dd..121cc5544b2b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -993,6 +993,7 @@ struct amdgpu_device {
>  	/* record last mm index being written through WREG32*/
>  	unsigned long last_mm_index;
>  	bool                            in_gpu_reset;
> +	enum pp_mp1_state               mp1_state;
>  	struct mutex  lock_reset;
>  	struct amdgpu_doorbell_index doorbell_index;
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 4425ff06ecc4..127ed01ed8fd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2174,6 +2174,21 @@ static int
> amdgpu_device_ip_suspend_phase2(struct amdgpu_device *adev)
>  			DRM_ERROR("suspend of IP block <%s> failed %d\n",
>  				  adev->ip_blocks[i].version->funcs->name,
> r);
>  		}
> +		/* handle putting the SMC in the appropriate state */
> +		if (adev->ip_blocks[i].version->type ==
> AMD_IP_BLOCK_TYPE_SMC) {
> +			if (is_support_sw_smu(adev)) {
> +				/* todo */
> +			} else if (adev->powerplay.pp_funcs &&
> +				   adev->powerplay.pp_funcs-
> >set_mp1_state) {
> +				r = adev->powerplay.pp_funcs-
> >set_mp1_state(
> +					adev->powerplay.pp_handle,
> +					adev->mp1_state);
> +				if (r) {
> +					DRM_ERROR("SMC failed to set mp1
> state %d, %d\n",
> +						  adev->mp1_state, r);
> +				}
> +			}
> +		}
>  	}
> 
>  	return 0;
> @@ -3639,6 +3654,17 @@ static bool amdgpu_device_lock_adev(struct
> amdgpu_device *adev, bool trylock)
> 
>  	atomic_inc(&adev->gpu_reset_counter);
>  	adev->in_gpu_reset = 1;
> +	switch (amdgpu_asic_reset_method(adev)) {
> +	case AMD_RESET_METHOD_MODE1:
> +		adev->mp1_state = PP_MP1_STATE_SHUTDOWN;
> +		break;
> +	case AMD_RESET_METHOD_MODE2:
> +		adev->mp1_state = PP_MP1_STATE_RESET;
> +		break;
> +	default:
> +		adev->mp1_state = PP_MP1_STATE_NONE;
> +		break;
> +	}
>  	/* Block kfd: SRIOV would do it separately */
>  	if (!amdgpu_sriov_vf(adev))
>                  amdgpu_amdkfd_pre_reset(adev); @@ -3652,6 +3678,7 @@ static
> void amdgpu_device_unlock_adev(struct amdgpu_device *adev)
>  	if (!amdgpu_sriov_vf(adev))
>                  amdgpu_amdkfd_post_reset(adev);
>  	amdgpu_vf_error_trans_all(adev);
> +	adev->mp1_state = PP_MP1_STATE_NONE;
>  	adev->in_gpu_reset = 0;
>  	mutex_unlock(&adev->lock_reset);
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 4743801357c5..800d0ceb14b4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1094,7 +1094,9 @@ amdgpu_pci_shutdown(struct pci_dev *pdev)
>  	 * unfortunately we can't detect certain
>  	 * hypervisors so just do this all the time.
>  	 */
> +	adev->mp1_state = PP_MP1_STATE_UNLOAD;
>  	amdgpu_device_ip_suspend(adev);
> +	adev->mp1_state = PP_MP1_STATE_NONE;
>  }
> 
>  static int amdgpu_pmops_suspend(struct device *dev)
> --
> 2.20.1
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx