> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Christian König
> Sent: Wednesday, May 10, 2017 2:09 PM
> To: amd-gfx@lists.freedesktop.org
> Subject: [PATCH] drm/amdgpu: fix fundamental suspend/resume issue
>
> From: Christian König <christian.koe...@amd.com>
>
> Reinitializing the VM manager during suspend/resume is a very very bad
> idea since all the VMs are still active and kicking.
>
> This can lead to random VM faults after resume when new processes
> become the same client ID assigned.
>
> Signed-off-by: Christian König <christian.koe...@amd.com>
Reviewed-by: Alex Deucher <alexander.deuc...@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 22
> +++++++++++++++++++++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 1 +
> drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 15 ++-------------
> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 15 ++-------------
> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 15 ++-------------
> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 16 ++--------------
> 6 files changed, 30 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index ed97a2e..9803392 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -790,6 +790,7 @@ void amdgpu_vm_reset_id(struct amdgpu_device
> *adev, unsigned vmhub,
> struct amdgpu_vm_id_manager *id_mgr = &adev-
> >vm_manager.id_mgr[vmhub];
> struct amdgpu_vm_id *id = &id_mgr->ids[vmid];
>
> + atomic64_set(&id->owner, 0);
> id->gds_base = 0;
> id->gds_size = 0;
> id->gws_base = 0;
> @@ -799,6 +800,26 @@ void amdgpu_vm_reset_id(struct amdgpu_device
> *adev, unsigned vmhub,
> }
>
> /**
> + * amdgpu_vm_reset_all_id - reset VMID to zero
> + *
> + * @adev: amdgpu device structure
> + *
> + * Reset VMID to force flush on next use
> + */
> +void amdgpu_vm_reset_all_ids(struct amdgpu_device *adev)
> +{
> + unsigned i, j;
> +
> + for (i = 0; i < AMDGPU_MAX_VMHUBS; ++i) {
> + struct amdgpu_vm_id_manager *id_mgr =
> + &adev->vm_manager.id_mgr[i];
> +
> + for (j = 1; j < id_mgr->num_ids; ++j)
> + amdgpu_vm_reset_id(adev, i, j);
> + }
> +}
> +
> +/**
> * amdgpu_vm_bo_find - find the bo_va for a specific vm & bo
> *
> * @vm: requested vm
> @@ -2393,7 +2414,6 @@ void amdgpu_vm_manager_init(struct
> amdgpu_device *adev)
> for (i = 0; i < AMDGPU_MAX_RINGS; ++i)
> adev->vm_manager.seqno[i] = 0;
>
> -
> atomic_set(&adev->vm_manager.vm_pte_next_ring, 0);
> atomic64_set(&adev->vm_manager.client_counter, 0);
> spin_lock_init(&adev->vm_manager.prt_lock);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index abc0bab..d62547d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -209,6 +209,7 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm,
> struct amdgpu_ring *ring,
> int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job);
> void amdgpu_vm_reset_id(struct amdgpu_device *adev, unsigned vmhub,
> unsigned vmid);
> +void amdgpu_vm_reset_all_ids(struct amdgpu_device *adev);
> int amdgpu_vm_update_directories(struct amdgpu_device *adev,
> struct amdgpu_vm *vm);
> int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> index a572979..d860939 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> @@ -950,10 +950,6 @@ static int gmc_v6_0_suspend(void *handle)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>
> - if (adev->vm_manager.enabled) {
> - gmc_v6_0_vm_fini(adev);
> - adev->vm_manager.enabled = false;
> - }
> gmc_v6_0_hw_fini(adev);
>
> return 0;
> @@ -968,16 +964,9 @@ static int gmc_v6_0_resume(void *handle)
> if (r)
> return r;
>
> - if (!adev->vm_manager.enabled) {
> - r = gmc_v6_0_vm_init(adev);
> - if (r) {
> - dev_err(adev->dev, "vm manager initialization failed
> (%d).\n", r);
> - return r;
> - }
> - adev->vm_manager.enabled = true;
> - }
> + amdgpu_vm_reset_all_ids(adev);
>
> - return r;
> + return 0;
> }
>
> static bool gmc_v6_0_is_idle(void *handle)
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index a9083a1..2750e5c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -1117,10 +1117,6 @@ static int gmc_v7_0_suspend(void *handle)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>
> - if (adev->vm_manager.enabled) {
> - gmc_v7_0_vm_fini(adev);
> - adev->vm_manager.enabled = false;
> - }
> gmc_v7_0_hw_fini(adev);
>
> return 0;
> @@ -1135,16 +1131,9 @@ static int gmc_v7_0_resume(void *handle)
> if (r)
> return r;
>
> - if (!adev->vm_manager.enabled) {
> - r = gmc_v7_0_vm_init(adev);
> - if (r) {
> - dev_err(adev->dev, "vm manager initialization failed
> (%d).\n", r);
> - return r;
> - }
> - adev->vm_manager.enabled = true;
> - }
> + amdgpu_vm_reset_all_ids(adev);
>
> - return r;
> + return 0;
> }
>
> static bool gmc_v7_0_is_idle(void *handle)
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index 4ac9978..f56b408 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -1209,10 +1209,6 @@ static int gmc_v8_0_suspend(void *handle)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>
> - if (adev->vm_manager.enabled) {
> - gmc_v8_0_vm_fini(adev);
> - adev->vm_manager.enabled = false;
> - }
> gmc_v8_0_hw_fini(adev);
>
> return 0;
> @@ -1227,16 +1223,9 @@ static int gmc_v8_0_resume(void *handle)
> if (r)
> return r;
>
> - if (!adev->vm_manager.enabled) {
> - r = gmc_v8_0_vm_init(adev);
> - if (r) {
> - dev_err(adev->dev, "vm manager initialization failed
> (%d).\n", r);
> - return r;
> - }
> - adev->vm_manager.enabled = true;
> - }
> + amdgpu_vm_reset_all_ids(adev);
>
> - return r;
> + return 0;
> }
>
> static bool gmc_v8_0_is_idle(void *handle)
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index b9f11fa..ae8fd91 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -797,10 +797,6 @@ static int gmc_v9_0_suspend(void *handle)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>
> - if (adev->vm_manager.enabled) {
> - gmc_v9_0_vm_fini(adev);
> - adev->vm_manager.enabled = false;
> - }
> gmc_v9_0_hw_fini(adev);
>
> return 0;
> @@ -815,17 +811,9 @@ static int gmc_v9_0_resume(void *handle)
> if (r)
> return r;
>
> - if (!adev->vm_manager.enabled) {
> - r = gmc_v9_0_vm_init(adev);
> - if (r) {
> - dev_err(adev->dev,
> - "vm manager initialization failed (%d).\n", r);
> - return r;
> - }
> - adev->vm_manager.enabled = true;
> - }
> + amdgpu_vm_reset_all_ids(adev);
>
> - return r;
> + return 0;
> }
>
> static bool gmc_v9_0_is_idle(void *handle)
> --
> 2.7.4
>
> _______________________________________________
> 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
