[PATCH 00/13] shadow page table support

[PATCH 00/13] shadow page table support

[Chunming Zhou]

Since we cannot make sure VRAM is safe after gpu reset, page table backup
is neccessary, shadow page table is sense way to recovery page talbe when
gpu reset happens.
We need to allocate GTT bo as the shadow of VRAM bo when creating page table,
and make them same. After gpu reset, we will need to use SDMA to copy GTT bo
content to VRAM bo, then page table will be recoveried. 

Chunming Zhou (13):
  drm/amdgpu: irq resume should be immediately after gpu resume
  drm/amdgpu: add shadow bo support
  drm/amdgpu: set shadow flag for pd/pt bo
  drm/amdgpu: update shadow pt bo while update pt
  drm/amdgpu: update pd shadow while updating pd
  drm/amdgpu: implement amdgpu_vm_recover_page_table_from_shadow
  drm/amdgpu: link all vm clients
  drm/amdgpu: add vm_list_lock
  drm/amd: add block entity function
  drm/amdgpu: recover page tables after gpu reset
  drm/amd: wait neccessary dependency before running job
  drm/amdgpu: add vm recover pt fence
  drm/amdgpu: add backup condition for shadow page table

 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  15 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c        |   6 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |  33 +++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       |   5 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c    |  36 ++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        | 219 ++++++++++++++++++++++----
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c |  30 +++-
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |   3 +
 include/uapi/drm/amdgpu_drm.h                 |   2 +
 9 files changed, 316 insertions(+), 33 deletions(-)

-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 01/13] drm/amdgpu: irq resume should be immediately after gpu resume

[Chunming Zhou]

Change-Id: Icf64bf5964f0ef66c239ab0679d51275cc272699
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index edbb370..7fd651a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2158,6 +2158,7 @@ retry:
 		amdgpu_atombios_scratch_regs_restore(adev);
 	}
 	if (!r) {
+		amdgpu_irq_gpu_reset_resume_helper(adev);
 		r = amdgpu_ib_ring_tests(adev);
 		if (r) {
 			dev_err(adev->dev, "ib ring test failed (%d).\n", r);
@@ -2192,7 +2193,6 @@ retry:
 		/* bad news, how to tell it to userspace ? */
 		dev_info(adev->dev, "GPU reset failed\n");
 	}
-	amdgpu_irq_gpu_reset_resume_helper(adev);
 
 	return r;
 }
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 02/13] drm/amdgpu: add shadow bo support

[Chunming Zhou]

shadow bo is the shadow of a bo, which is always in GTT,
which can be used to backup the original bo.

Change-Id: Ia27d4225c47ff41d3053eb691276e29fb2d64026
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 36 +++++++++++++++++++++++++++---
 include/uapi/drm/amdgpu_drm.h              |  2 ++
 3 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 183781d..d415805 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -506,6 +506,7 @@ struct amdgpu_bo {
 	struct amdgpu_device		*adev;
 	struct drm_gem_object		gem_base;
 	struct amdgpu_bo		*parent;
+	struct amdgpu_bo		*shadow;
 
 	struct ttm_bo_kmap_obj		dma_buf_vmap;
 	struct amdgpu_mn		*mn;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index d8e69a7..e6ecf16 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -389,6 +389,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
 {
 	struct ttm_placement placement = {0};
 	struct ttm_place placements[AMDGPU_GEM_DOMAIN_MAX + 1];
+	int r;
 
 	memset(&placements, 0,
 	       (AMDGPU_GEM_DOMAIN_MAX + 1) * sizeof(struct ttm_place));
@@ -396,9 +397,32 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
 	amdgpu_ttm_placement_init(adev, &placement,
 				  placements, domain, flags);
 
-	return amdgpu_bo_create_restricted(adev, size, byte_align, kernel,
-					   domain, flags, sg, &placement,
-					   resv, bo_ptr);
+	r = amdgpu_bo_create_restricted(adev, size, byte_align, kernel,
+					domain, flags, sg, &placement,
+					resv, bo_ptr);
+	if (r)
+		return r;
+
+	if (flags & AMDGPU_GEM_CREATE_SHADOW) {
+		memset(&placements, 0,
+		       (AMDGPU_GEM_DOMAIN_MAX + 1) * sizeof(struct ttm_place));
+
+		amdgpu_ttm_placement_init(adev, &placement,
+					  placements, AMDGPU_GEM_DOMAIN_GTT,
+					  AMDGPU_GEM_CREATE_CPU_GTT_USWC);
+
+		r = amdgpu_bo_create_restricted(adev, size, byte_align, kernel,
+						AMDGPU_GEM_DOMAIN_GTT,
+						AMDGPU_GEM_CREATE_CPU_GTT_USWC,
+						NULL, &placement,
+						(*bo_ptr)->tbo.resv,
+						&(*bo_ptr)->shadow);
+		if (r)
+			amdgpu_bo_unref(bo_ptr);
+	} else
+		(*bo_ptr)->shadow = NULL;
+
+	return r;
 }
 
 int amdgpu_bo_kmap(struct amdgpu_bo *bo, void **ptr)
@@ -455,6 +479,12 @@ void amdgpu_bo_unref(struct amdgpu_bo **bo)
 
 	if ((*bo) == NULL)
 		return;
+	if ((*bo)->shadow) {
+		tbo = &((*bo)->shadow->tbo);
+		ttm_bo_unref(&tbo);
+		if (tbo == NULL)
+			(*bo)->shadow = NULL;
+	}
 
 	tbo = &((*bo)->tbo);
 	ttm_bo_unref(&tbo);
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 8df3816..da2d3e1 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -77,6 +77,8 @@
 #define AMDGPU_GEM_CREATE_CPU_GTT_USWC		(1 << 2)
 /* Flag that the memory should be in VRAM and cleared */
 #define AMDGPU_GEM_CREATE_VRAM_CLEARED		(1 << 3)
+/* Flag that create shadow bo(GTT) while allocating vram bo */
+#define AMDGPU_GEM_CREATE_SHADOW		(1 << 4)
 
 struct drm_amdgpu_gem_create_in  {
 	/** the requested memory size */
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 03/13] drm/amdgpu: set shadow flag for pd/pt bo

[Chunming Zhou]

the pd/pt shadow bo will be used to backup page table, when gpu reset
happens, we can restore the page table by them.

Change-Id: I31eeb581f203d1db0654a48745ef4e64ed40ed9b
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h    |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 16 +++++++++++++---
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index d415805..ae4608c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -875,6 +875,8 @@ struct amdgpu_ring {
 struct amdgpu_vm_pt {
 	struct amdgpu_bo_list_entry	entry;
 	uint64_t			addr;
+	struct amdgpu_bo_list_entry	entry_shadow;
+	uint64_t			addr_shadow;
 };
 
 struct amdgpu_vm {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 38c80ea..aedd1cb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1331,9 +1331,10 @@ int amdgpu_vm_bo_map(struct amdgpu_device *adev,
 	/* walk over the address space and allocate the page tables */
 	for (pt_idx = saddr; pt_idx <= eaddr; ++pt_idx) {
 		struct reservation_object *resv = vm->page_directory->tbo.resv;
-		struct amdgpu_bo_list_entry *entry;
+		struct amdgpu_bo_list_entry *entry, *entry_shadow;
 		struct amdgpu_bo *pt;
 
+		entry_shadow = &vm->page_tables[pt_idx].entry_shadow;
 		entry = &vm->page_tables[pt_idx].entry;
 		if (entry->robj)
 			continue;
@@ -1341,7 +1342,8 @@ int amdgpu_vm_bo_map(struct amdgpu_device *adev,
 		r = amdgpu_bo_create(adev, AMDGPU_VM_PTE_COUNT * 8,
 				     AMDGPU_GPU_PAGE_SIZE, true,
 				     AMDGPU_GEM_DOMAIN_VRAM,
-				     AMDGPU_GEM_CREATE_NO_CPU_ACCESS,
+				     AMDGPU_GEM_CREATE_NO_CPU_ACCESS |
+				     AMDGPU_GEM_CREATE_SHADOW,
 				     NULL, resv, &pt);
 		if (r)
 			goto error_free;
@@ -1363,6 +1365,13 @@ int amdgpu_vm_bo_map(struct amdgpu_device *adev,
 		entry->tv.shared = true;
 		entry->user_pages = NULL;
 		vm->page_tables[pt_idx].addr = 0;
+
+		entry_shadow->robj = pt->shadow;
+		entry_shadow->priority = 0;
+		entry_shadow->tv.bo = &entry_shadow->robj->tbo;
+		entry_shadow->tv.shared = true;
+		entry_shadow->user_pages = NULL;
+		vm->page_tables[pt_idx].addr_shadow = 0;
 	}
 
 	return 0;
@@ -1540,7 +1549,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 
 	r = amdgpu_bo_create(adev, pd_size, align, true,
 			     AMDGPU_GEM_DOMAIN_VRAM,
-			     AMDGPU_GEM_CREATE_NO_CPU_ACCESS,
+			     AMDGPU_GEM_CREATE_NO_CPU_ACCESS |
+			     AMDGPU_GEM_CREATE_SHADOW,
 			     NULL, NULL, &vm->page_directory);
 	if (r)
 		goto error_free_sched_entity;
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 04/13] drm/amdgpu: update shadow pt bo while update pt

[Chunming Zhou]

Change-Id: I8245cdad490d2a0b8cf4b9320e53e14db0b6add4
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index aedd1cb..e7a400d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -651,6 +651,7 @@ int amdgpu_vm_update_page_directory(struct amdgpu_device *adev,
 		if (vm->page_tables[pt_idx].addr == pt)
 			continue;
 		vm->page_tables[pt_idx].addr = pt;
+		vm->page_tables[pt_idx].addr_shadow = pt;
 
 		pde = pd_addr + pt_idx * 8;
 		if (((last_pde + 8 * count) != pde) ||
@@ -801,7 +802,7 @@ static void amdgpu_vm_update_ptes(struct amdgpu_device *adev,
 					*vm_update_params,
 				  struct amdgpu_vm *vm,
 				  uint64_t start, uint64_t end,
-				  uint64_t dst, uint32_t flags)
+				  uint64_t dst, uint32_t flags, bool shadow)
 {
 	const uint64_t mask = AMDGPU_VM_PTE_COUNT - 1;
 
@@ -815,7 +816,8 @@ static void amdgpu_vm_update_ptes(struct amdgpu_device *adev,
 	/* initialize the variables */
 	addr = start;
 	pt_idx = addr >> amdgpu_vm_block_size;
-	pt = vm->page_tables[pt_idx].entry.robj;
+	pt = shadow ? vm->page_tables[pt_idx].entry_shadow.robj :
+		vm->page_tables[pt_idx].entry.robj;
 
 	if ((addr & ~mask) == (end & ~mask))
 		nptes = end - addr;
@@ -834,7 +836,8 @@ static void amdgpu_vm_update_ptes(struct amdgpu_device *adev,
 	/* walk over the address space and update the page tables */
 	while (addr < end) {
 		pt_idx = addr >> amdgpu_vm_block_size;
-		pt = vm->page_tables[pt_idx].entry.robj;
+		pt = shadow ? vm->page_tables[pt_idx].entry_shadow.robj :
+			vm->page_tables[pt_idx].entry.robj;
 
 		if ((addr & ~mask) == (end & ~mask))
 			nptes = end - addr;
@@ -941,6 +944,8 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
 		/* two extra commands for begin/end of fragment */
 		ndw += 2 * 10;
 	}
+	/* double ndw, since need to update shadow pt bo as well */
+	ndw *= 2;
 
 	r = amdgpu_job_alloc_with_ib(adev, ndw * 4, &job);
 	if (r)
@@ -960,9 +965,12 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
 	r = reservation_object_reserve_shared(vm->page_directory->tbo.resv);
 	if (r)
 		goto error_free;
+	/* update shadow pt bo */
+	amdgpu_vm_update_ptes(adev, &vm_update_params, vm, start,
+			      last + 1, addr, flags, true);
 
 	amdgpu_vm_update_ptes(adev, &vm_update_params, vm, start,
-			      last + 1, addr, flags);
+			      last + 1, addr, flags, false);
 
 	amdgpu_ring_pad_ib(ring, vm_update_params.ib);
 	WARN_ON(vm_update_params.ib->length_dw > ndw);
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 05/13] drm/amdgpu: update pd shadow while updating pd

[Chunming Zhou]

Change-Id: Icafa90a6625ea7b5ab3e360ba0d73544cda251b0
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h    |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 68 +++++++++++++++++++++++-----------
 2 files changed, 48 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index ae4608c..4cdfdff 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -897,6 +897,7 @@ struct amdgpu_vm {
 
 	/* contains the page directory */
 	struct amdgpu_bo	*page_directory;
+	struct amdgpu_bo_list_entry	pd_entry_shadow;
 	unsigned		max_pde_used;
 	struct fence		*page_directory_fence;
 	uint64_t		last_eviction_counter;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index e7a400d..fb8a7ab 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -138,13 +138,15 @@ void amdgpu_vm_get_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	/* add the vm page table to the list */
 	for (i = 0; i <= vm->max_pde_used; ++i) {
 		struct amdgpu_bo_list_entry *entry = &vm->page_tables[i].entry;
+		struct amdgpu_bo_list_entry *entry_shadow = &vm->page_tables[i].entry_shadow;
 
-		if (!entry->robj)
+		if (!entry->robj || !entry_shadow->robj)
 			continue;
 
 		list_add(&entry->tv.head, duplicates);
+		list_add(&entry_shadow->tv.head, duplicates);
 	}
-
+	list_add(&vm->pd_entry_shadow.tv.head, duplicates);
 }
 
 /**
@@ -597,23 +599,13 @@ uint64_t amdgpu_vm_map_gart(const dma_addr_t *pages_addr, uint64_t addr)
 	return result;
 }
 
-/**
- * amdgpu_vm_update_pdes - make sure that page directory is valid
- *
- * @adev: amdgpu_device pointer
- * @vm: requested vm
- * @start: start of GPU address range
- * @end: end of GPU address range
- *
- * Allocates new page tables if necessary
- * and updates the page directory.
- * Returns 0 for success, error for failure.
- */
-int amdgpu_vm_update_page_directory(struct amdgpu_device *adev,
-				    struct amdgpu_vm *vm)
+
+static int amdgpu_vm_update_pd_or_shadow(struct amdgpu_device *adev,
+					 struct amdgpu_vm *vm, bool shadow)
 {
 	struct amdgpu_ring *ring;
-	struct amdgpu_bo *pd = vm->page_directory;
+	struct amdgpu_bo *pd = shadow ? vm->page_directory->shadow :
+		vm->page_directory;
 	uint64_t pd_addr = amdgpu_bo_gpu_offset(pd);
 	uint32_t incr = AMDGPU_VM_PTE_COUNT * 8;
 	uint64_t last_pde = ~0, last_pt = ~0;
@@ -648,10 +640,15 @@ int amdgpu_vm_update_page_directory(struct amdgpu_device *adev,
 			continue;
 
 		pt = amdgpu_bo_gpu_offset(bo);
-		if (vm->page_tables[pt_idx].addr == pt)
-			continue;
-		vm->page_tables[pt_idx].addr = pt;
-		vm->page_tables[pt_idx].addr_shadow = pt;
+		if (!shadow) {
+			if (vm->page_tables[pt_idx].addr == pt)
+				continue;
+			vm->page_tables[pt_idx].addr = pt;
+		} else {
+			if (vm->page_tables[pt_idx].addr_shadow == pt)
+				continue;
+			vm->page_tables[pt_idx].addr_shadow = pt;
+		}
 
 		pde = pd_addr + pt_idx * 8;
 		if (((last_pde + 8 * count) != pde) ||
@@ -704,6 +701,29 @@ error_free:
 }
 
 /**
+ * amdgpu_vm_update_pdes - make sure that page directory is valid
+ *
+ * @adev: amdgpu_device pointer
+ * @vm: requested vm
+ * @start: start of GPU address range
+ * @end: end of GPU address range
+ *
+ * Allocates new page tables if necessary
+ * and updates the page directory.
+ * Returns 0 for success, error for failure.
+ */
+int amdgpu_vm_update_page_directory(struct amdgpu_device *adev,
+				    struct amdgpu_vm *vm)
+{
+	int r;
+
+	r = amdgpu_vm_update_pd_or_shadow(adev, vm, true);
+	if (r)
+		return r;
+	return amdgpu_vm_update_pd_or_shadow(adev, vm, false);
+}
+
+/**
  * amdgpu_vm_frag_ptes - add fragment information to PTEs
  *
  * @adev: amdgpu_device pointer
@@ -1573,6 +1593,12 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 		goto error_free_page_directory;
 	vm->last_eviction_counter = atomic64_read(&adev->num_evictions);
 
+	vm->pd_entry_shadow.robj = vm->page_directory->shadow;
+	vm->pd_entry_shadow.priority = 0;
+	vm->pd_entry_shadow.tv.bo = &vm->page_directory->shadow->tbo;
+	vm->pd_entry_shadow.tv.shared = true;
+	vm->pd_entry_shadow.user_pages = NULL;
+
 	return 0;
 
 error_free_page_directory:
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 06/13] drm/amdgpu: implement amdgpu_vm_recover_page_table_from_shadow

[Chunming Zhou]

Change-Id: I9957e726576289448911f5fb2ff7bcb9311a1906
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h    |  2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 77 ++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 4cdfdff..1234b8e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1006,6 +1006,8 @@ int amdgpu_vm_bo_unmap(struct amdgpu_device *adev,
 		       uint64_t addr);
 void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
 		      struct amdgpu_bo_va *bo_va);
+int amdgpu_vm_recover_page_table_from_shadow(struct amdgpu_device *adev,
+					     struct amdgpu_vm *vm);
 
 /*
  * context related structures
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index fb8a7ab..4f95dc4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -700,6 +700,83 @@ error_free:
 	return r;
 }
 
+static int amdgpu_vm_recover_bo_from_shadow(struct amdgpu_device *adev,
+					    struct amdgpu_bo *bo,
+					    struct amdgpu_bo *bo_shadow,
+					    struct reservation_object *resv)
+
+{
+	struct amdgpu_ring *ring = adev->mman.buffer_funcs_ring;
+	struct fence *fence;
+	int r;
+	uint64_t vram_addr, gtt_addr;
+
+	r = amdgpu_bo_pin(bo, AMDGPU_GEM_DOMAIN_VRAM, &vram_addr);
+	if (r) {
+		DRM_ERROR("Failed to pin bo object\n");
+		goto err1;
+	}
+	r = amdgpu_bo_pin(bo_shadow, AMDGPU_GEM_DOMAIN_GTT, &gtt_addr);
+	if (r) {
+		DRM_ERROR("Failed to pin bo shadow object\n");
+		goto err2;
+	}
+
+	r = reservation_object_reserve_shared(bo->tbo.resv);
+	if (r)
+		goto err3;
+
+	r = amdgpu_copy_buffer(ring, gtt_addr, vram_addr,
+			       amdgpu_bo_size(bo), resv, &fence);
+	if (!r)
+		amdgpu_bo_fence(bo, fence, true);
+
+err3:
+	amdgpu_bo_unpin(bo_shadow);
+err2:
+	amdgpu_bo_unpin(bo);
+err1:
+
+	return r;
+}
+
+int amdgpu_vm_recover_page_table_from_shadow(struct amdgpu_device *adev,
+					     struct amdgpu_vm *vm)
+{
+	uint64_t pt_idx;
+	int r;
+
+	/* bo and shadow use same resv, so reverve one time */
+	r = amdgpu_bo_reserve(vm->page_directory, false);
+	if (unlikely(r != 0))
+		return r;
+
+	r = amdgpu_vm_recover_bo_from_shadow(adev, vm->page_directory,
+					     vm->page_directory->shadow,
+					     NULL);
+	if (r) {
+		DRM_ERROR("recover page table failed!\n");
+		goto err;
+	}
+
+	for (pt_idx = 0; pt_idx <= vm->max_pde_used; ++pt_idx) {
+		struct amdgpu_bo *bo = vm->page_tables[pt_idx].entry.robj;
+		struct amdgpu_bo *bo_shadow = vm->page_tables[pt_idx].entry_shadow.robj;
+
+		if (!bo || !bo_shadow)
+			continue;
+		r = amdgpu_vm_recover_bo_from_shadow(adev, bo, bo_shadow,
+						     NULL);
+		if (r) {
+			DRM_ERROR("recover page table failed!\n");
+			goto err;
+		}
+	}
+
+err:
+	amdgpu_bo_unreserve(vm->page_directory);
+	return r;
+}
 /**
  * amdgpu_vm_update_pdes - make sure that page directory is valid
  *
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 07/13] drm/amdgpu: link all vm clients

[Chunming Zhou]

Add vm client to list tail when creating it, move to head while submit to scheduler.

Change-Id: I0625092f918853303a5ee97ea2eac87fb790ed69
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        | 6 ++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c     | 4 ++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c     | 3 +++
 4 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 1234b8e..61fd4e2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -895,6 +895,9 @@ struct amdgpu_vm {
 	/* BO mappings freed, but not yet updated in the PT */
 	struct list_head	freed;
 
+	/* vm itself list */
+	struct list_head	list;
+
 	/* contains the page directory */
 	struct amdgpu_bo	*page_directory;
 	struct amdgpu_bo_list_entry	pd_entry_shadow;
@@ -2167,6 +2170,9 @@ struct amdgpu_device {
 	struct kfd_dev          *kfd;
 
 	struct amdgpu_virtualization virtualization;
+
+	/* link all vm clients */
+	struct list_head		vm_list;
 };
 
 bool amdgpu_device_is_px(struct drm_device *dev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 45d5227..d19838b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -835,7 +835,10 @@ static int amdgpu_cs_dependencies(struct amdgpu_device *adev,
 static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
 			    union drm_amdgpu_cs *cs)
 {
+	struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
+	struct amdgpu_vm *vm = &fpriv->vm;
 	struct amdgpu_ring *ring = p->job->ring;
+	struct amdgpu_device *adev = ring->adev;
 	struct amd_sched_entity *entity = &p->ctx->rings[ring->idx].entity;
 	struct amdgpu_job *job;
 	int r;
@@ -858,6 +861,7 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
 
 	trace_amdgpu_cs_ioctl(job);
 	amd_sched_entity_push_job(&job->base);
+	list_move(&vm->list, &adev->vm_list);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 7fd651a..552216a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1559,6 +1559,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 	spin_lock_init(&adev->gc_cac_idx_lock);
 	spin_lock_init(&adev->audio_endpt_idx_lock);
 
+	INIT_LIST_HEAD(&adev->vm_list);
+
 	adev->rmmio_base = pci_resource_start(adev->pdev, 5);
 	adev->rmmio_size = pci_resource_len(adev->pdev, 5);
 	adev->rmmio = ioremap(adev->rmmio_base, adev->rmmio_size);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 4f95dc4..35d939b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1628,6 +1628,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 	INIT_LIST_HEAD(&vm->invalidated);
 	INIT_LIST_HEAD(&vm->cleared);
 	INIT_LIST_HEAD(&vm->freed);
+	INIT_LIST_HEAD(&vm->list);
 
 	pd_size = amdgpu_vm_directory_size(adev);
 	pd_entries = amdgpu_vm_num_pdes(adev);
@@ -1675,6 +1676,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 	vm->pd_entry_shadow.tv.bo = &vm->page_directory->shadow->tbo;
 	vm->pd_entry_shadow.tv.shared = true;
 	vm->pd_entry_shadow.user_pages = NULL;
+	list_add_tail(&vm->list, &adev->vm_list);
 
 	return 0;
 
@@ -1702,6 +1704,7 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 	struct amdgpu_bo_va_mapping *mapping, *tmp;
 	int i;
 
+	list_del(&vm->list);
 	amd_sched_entity_fini(vm->entity.sched, &vm->entity);
 
 	if (!RB_EMPTY_ROOT(&vm->va)) {
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 08/13] drm/amdgpu: add vm_list_lock

[Chunming Zhou]

To lock adev->vm_list.

Change-Id: I74d309eca9c22d190dd4072c69d26fa7fdea8884
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        | 1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c     | 2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c     | 4 ++++
 4 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 61fd4e2..a2261ac 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -2173,6 +2173,7 @@ struct amdgpu_device {
 
 	/* link all vm clients */
 	struct list_head		vm_list;
+	spinlock_t			vm_list_lock;
 };
 
 bool amdgpu_device_is_px(struct drm_device *dev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index d19838b..29c10f4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -861,7 +861,9 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
 
 	trace_amdgpu_cs_ioctl(job);
 	amd_sched_entity_push_job(&job->base);
+	spin_lock(&adev->vm_list_lock);
 	list_move(&vm->list, &adev->vm_list);
+	spin_unlock(&adev->vm_list_lock);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 552216a..2412cc0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1560,6 +1560,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 	spin_lock_init(&adev->audio_endpt_idx_lock);
 
 	INIT_LIST_HEAD(&adev->vm_list);
+	spin_lock_init(&adev->vm_list_lock);
 
 	adev->rmmio_base = pci_resource_start(adev->pdev, 5);
 	adev->rmmio_size = pci_resource_len(adev->pdev, 5);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 35d939b..1cb2e71 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1676,7 +1676,9 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 	vm->pd_entry_shadow.tv.bo = &vm->page_directory->shadow->tbo;
 	vm->pd_entry_shadow.tv.shared = true;
 	vm->pd_entry_shadow.user_pages = NULL;
+	spin_lock(&adev->vm_list_lock);
 	list_add_tail(&vm->list, &adev->vm_list);
+	spin_unlock(&adev->vm_list_lock);
 
 	return 0;
 
@@ -1704,7 +1706,9 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 	struct amdgpu_bo_va_mapping *mapping, *tmp;
 	int i;
 
+	spin_lock(&adev->vm_list_lock);
 	list_del(&vm->list);
+	spin_unlock(&adev->vm_list_lock);
 	amd_sched_entity_fini(vm->entity.sched, &vm->entity);
 
 	if (!RB_EMPTY_ROOT(&vm->va)) {
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 09/13] drm/amd: add block entity function

[Chunming Zhou]

Change-Id: Ia0378640962eef362569e0bbe090aea1ca083a55
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |  3 +++
 2 files changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index f96aa82..71b5f1a 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -110,6 +110,26 @@ amd_sched_rq_select_entity(struct amd_sched_rq *rq)
 }
 
 /**
+ * block all entity of this run queue
+ *
+ * @rq		The run queue to check.
+ *
+ */
+int amd_sched_rq_block_entity(struct amd_sched_rq *rq, bool block)
+{
+	struct amd_sched_entity *entity;
+
+	spin_lock(&rq->lock);
+
+	list_for_each_entry(entity, &rq->entities, list)
+		entity->block = block;
+
+	spin_unlock(&rq->lock);
+
+	return 0;
+}
+
+/**
  * Init a context entity used by scheduler when submit to HW ring.
  *
  * @sched	The pointer to the scheduler
@@ -134,6 +154,7 @@ int amd_sched_entity_init(struct amd_gpu_scheduler *sched,
 	INIT_LIST_HEAD(&entity->list);
 	entity->rq = rq;
 	entity->sched = sched;
+	entity->block = false;
 
 	spin_lock_init(&entity->queue_lock);
 	r = kfifo_alloc(&entity->job_queue, jobs * sizeof(void *), GFP_KERNEL);
@@ -186,6 +207,9 @@ static bool amd_sched_entity_is_idle(struct amd_sched_entity *entity)
  */
 static bool amd_sched_entity_is_ready(struct amd_sched_entity *entity)
 {
+	if (entity->block)
+		return false;
+
 	if (kfifo_is_empty(&entity->job_queue))
 		return false;
 
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
index 7cbbbfb..a1c0073 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
@@ -52,6 +52,8 @@ struct amd_sched_entity {
 
 	struct fence			*dependency;
 	struct fence_cb			cb;
+
+	bool                            block;
 };
 
 /**
@@ -155,4 +157,5 @@ int amd_sched_job_init(struct amd_sched_job *job,
 		       void *owner);
 void amd_sched_hw_job_reset(struct amd_gpu_scheduler *sched);
 void amd_sched_job_recovery(struct amd_gpu_scheduler *sched);
+int amd_sched_rq_block_entity(struct amd_sched_rq *rq, bool block);
 #endif
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 10/13] drm/amdgpu: recover page tables after gpu reset

[Chunming Zhou]

Change-Id: I963598ba6eb44bc8620d70e026c0175d1a1de120
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 2412cc0..5e984cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2161,18 +2161,44 @@ retry:
 		amdgpu_atombios_scratch_regs_restore(adev);
 	}
 	if (!r) {
+		struct amdgpu_ring *buffer_ring = adev->mman.buffer_funcs_ring;
+
 		amdgpu_irq_gpu_reset_resume_helper(adev);
 		r = amdgpu_ib_ring_tests(adev);
 		if (r) {
 			dev_err(adev->dev, "ib ring test failed (%d).\n", r);
 			r = amdgpu_suspend(adev);
+			need_full_reset = true;
 			goto retry;
 		}
-
+		/**
+		 * recovery vm page tables, since we cannot depend on VRAM is no problem
+		 * after gpu full reset.
+		 */
+		if (need_full_reset && !(adev->flags & AMD_IS_APU)) {
+			struct amdgpu_vm *vm, *tmp;
+
+			DRM_INFO("recover page table from shadow\n");
+			amd_sched_rq_block_entity(
+				&buffer_ring->sched.sched_rq[AMD_SCHED_PRIORITY_NORMAL], true);
+			kthread_unpark(buffer_ring->sched.thread);
+			spin_lock(&adev->vm_list_lock);
+			list_for_each_entry_safe(vm, tmp, &adev->vm_list, list) {
+				spin_unlock(&adev->vm_list_lock);
+				amdgpu_vm_recover_page_table_from_shadow(adev, vm);
+				spin_lock(&adev->vm_list_lock);
+			}
+			spin_unlock(&adev->vm_list_lock);
+			amd_sched_rq_block_entity(
+				&buffer_ring->sched.sched_rq[AMD_SCHED_PRIORITY_NORMAL], false);
+		}
 		for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
 			struct amdgpu_ring *ring = adev->rings[i];
 			if (!ring)
 				continue;
+
+			DRM_INFO("ring:%d recover jobs\n", ring->idx);
+			kthread_park(buffer_ring->sched.thread);
 			amd_sched_job_recovery(&ring->sched);
 			kthread_unpark(ring->sched.thread);
 		}
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 11/13] drm/amd: wait neccessary dependency before running job

[Chunming Zhou]

Change-Id: Ibcc3558c2330caad1a2edb9902b3f21bd950d19f
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index 71b5f1a..a15fd88 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -436,9 +436,13 @@ void amd_sched_job_recovery(struct amd_gpu_scheduler *sched)
 
 	list_for_each_entry_safe(s_job, tmp, &sched->ring_mirror_list, node) {
 		struct amd_sched_fence *s_fence = s_job->s_fence;
-		struct fence *fence;
+		struct fence *fence, *dependency;
 
 		spin_unlock(&sched->job_list_lock);
+		while ((dependency = sched->ops->dependency(s_job))) {
+		       fence_wait(dependency, false);
+		       fence_put(dependency);
+		}
 		fence = sched->ops->run_job(s_job);
 		atomic_inc(&sched->hw_rq_count);
 		if (fence) {
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 12/13] drm/amdgpu: add vm recover pt fence

[Chunming Zhou]

Before every job runs, we must make sure which's vm is recoverred completely.

Change-Id: Ibe77a3c8f8206def280543fbb4195ad2ab9772e0
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c |  5 +++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 24 ++++++++++++++++++------
 3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a2261ac..c0e4164 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -919,6 +919,8 @@ struct amdgpu_vm {
 
 	/* client id */
 	u64                     client_id;
+
+	struct fence            *recover_pt_fence;
 };
 
 struct amdgpu_vm_id {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 6674d40..8d87a9a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -152,6 +152,10 @@ static struct fence *amdgpu_job_dependency(struct amd_sched_job *sched_job)
 		fence = amdgpu_sync_get_fence(&job->sync);
 	}
 
+	if (fence == NULL && vm && vm->recover_pt_fence &&
+	    !fence_is_signaled(vm->recover_pt_fence))
+		fence = fence_get(vm->recover_pt_fence);
+
 	return fence;
 }
 
@@ -170,6 +174,7 @@ static struct fence *amdgpu_job_run(struct amd_sched_job *sched_job)
 	BUG_ON(amdgpu_sync_peek_fence(&job->sync, NULL));
 
 	trace_amdgpu_sched_run_job(job);
+
 	r = amdgpu_ib_schedule(job->ring, job->num_ibs, job->ibs,
 			       job->sync.last_vm_update, job, &fence);
 	if (r)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 1cb2e71..1305dc1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -703,11 +703,11 @@ error_free:
 static int amdgpu_vm_recover_bo_from_shadow(struct amdgpu_device *adev,
 					    struct amdgpu_bo *bo,
 					    struct amdgpu_bo *bo_shadow,
-					    struct reservation_object *resv)
+					    struct reservation_object *resv,
+					    struct fence **fence)
 
 {
 	struct amdgpu_ring *ring = adev->mman.buffer_funcs_ring;
-	struct fence *fence;
 	int r;
 	uint64_t vram_addr, gtt_addr;
 
@@ -727,9 +727,9 @@ static int amdgpu_vm_recover_bo_from_shadow(struct amdgpu_device *adev,
 		goto err3;
 
 	r = amdgpu_copy_buffer(ring, gtt_addr, vram_addr,
-			       amdgpu_bo_size(bo), resv, &fence);
+			       amdgpu_bo_size(bo), resv, fence);
 	if (!r)
-		amdgpu_bo_fence(bo, fence, true);
+		amdgpu_bo_fence(bo, *fence, true);
 
 err3:
 	amdgpu_bo_unpin(bo_shadow);
@@ -743,6 +743,7 @@ err1:
 int amdgpu_vm_recover_page_table_from_shadow(struct amdgpu_device *adev,
 					     struct amdgpu_vm *vm)
 {
+	struct fence *fence;
 	uint64_t pt_idx;
 	int r;
 
@@ -753,11 +754,14 @@ int amdgpu_vm_recover_page_table_from_shadow(struct amdgpu_device *adev,
 
 	r = amdgpu_vm_recover_bo_from_shadow(adev, vm->page_directory,
 					     vm->page_directory->shadow,
-					     NULL);
+					     NULL, &fence);
 	if (r) {
 		DRM_ERROR("recover page table failed!\n");
 		goto err;
 	}
+	fence_put(vm->recover_pt_fence);
+	vm->recover_pt_fence = fence_get(fence);
+	fence_put(fence);
 
 	for (pt_idx = 0; pt_idx <= vm->max_pde_used; ++pt_idx) {
 		struct amdgpu_bo *bo = vm->page_tables[pt_idx].entry.robj;
@@ -766,15 +770,21 @@ int amdgpu_vm_recover_page_table_from_shadow(struct amdgpu_device *adev,
 		if (!bo || !bo_shadow)
 			continue;
 		r = amdgpu_vm_recover_bo_from_shadow(adev, bo, bo_shadow,
-						     NULL);
+						     NULL, &fence);
 		if (r) {
 			DRM_ERROR("recover page table failed!\n");
 			goto err;
 		}
+		fence_put(vm->recover_pt_fence);
+		vm->recover_pt_fence = fence_get(fence);
+		fence_put(fence);
 	}
 
 err:
 	amdgpu_bo_unreserve(vm->page_directory);
+	if (vm->recover_pt_fence)
+		r = fence_wait(vm->recover_pt_fence, false);
+
 	return r;
 }
 /**
@@ -1629,6 +1639,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 	INIT_LIST_HEAD(&vm->cleared);
 	INIT_LIST_HEAD(&vm->freed);
 	INIT_LIST_HEAD(&vm->list);
+	vm->recover_pt_fence = NULL;
 
 	pd_size = amdgpu_vm_directory_size(adev);
 	pd_entries = amdgpu_vm_num_pdes(adev);
@@ -1730,6 +1741,7 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 
 	amdgpu_bo_unref(&vm->page_directory);
 	fence_put(vm->page_directory_fence);
+	fence_put(vm->recover_pt_fence);
 }
 
 /**
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 13/13] drm/amdgpu: add backup condition for shadow page table

[Chunming Zhou]

Change-Id: I5a8c0f4c1e9b65d2310ccb0f669b478884072a11
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 71 +++++++++++++++++++++++-----------
 1 file changed, 48 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 1305dc1..0e3f116 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -112,6 +112,14 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
 	list_add(&entry->tv.head, validated);
 }
 
+static bool amdgpu_vm_need_backup(struct amdgpu_device *adev)
+{
+	if (adev->flags & AMD_IS_APU)
+		return false;
+
+	return amdgpu_lockup_timeout > 0 ? true : false;
+}
+
 /**
  * amdgpu_vm_get_bos - add the vm BOs to a duplicates list
  *
@@ -140,13 +148,18 @@ void amdgpu_vm_get_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 		struct amdgpu_bo_list_entry *entry = &vm->page_tables[i].entry;
 		struct amdgpu_bo_list_entry *entry_shadow = &vm->page_tables[i].entry_shadow;
 
-		if (!entry->robj || !entry_shadow->robj)
+		if (!entry->robj)
+			continue;
+
+		if (amdgpu_vm_need_backup(adev) && !entry_shadow->robj)
 			continue;
 
 		list_add(&entry->tv.head, duplicates);
-		list_add(&entry_shadow->tv.head, duplicates);
+		if (amdgpu_vm_need_backup(adev))
+			list_add(&entry_shadow->tv.head, duplicates);
 	}
-	list_add(&vm->pd_entry_shadow.tv.head, duplicates);
+	if (amdgpu_vm_need_backup(adev))
+		list_add(&vm->pd_entry_shadow.tv.head, duplicates);
 }
 
 /**
@@ -747,6 +760,8 @@ int amdgpu_vm_recover_page_table_from_shadow(struct amdgpu_device *adev,
 	uint64_t pt_idx;
 	int r;
 
+	if (!amdgpu_vm_need_backup(adev))
+		return 0;
 	/* bo and shadow use same resv, so reverve one time */
 	r = amdgpu_bo_reserve(vm->page_directory, false);
 	if (unlikely(r != 0))
@@ -804,9 +819,12 @@ int amdgpu_vm_update_page_directory(struct amdgpu_device *adev,
 {
 	int r;
 
-	r = amdgpu_vm_update_pd_or_shadow(adev, vm, true);
-	if (r)
-		return r;
+	if (amdgpu_vm_need_backup(adev)) {
+		r = amdgpu_vm_update_pd_or_shadow(adev, vm, true);
+		if (r)
+			return r;
+	}
+
 	return amdgpu_vm_update_pd_or_shadow(adev, vm, false);
 }
 
@@ -1072,10 +1090,11 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
 	r = reservation_object_reserve_shared(vm->page_directory->tbo.resv);
 	if (r)
 		goto error_free;
-	/* update shadow pt bo */
-	amdgpu_vm_update_ptes(adev, &vm_update_params, vm, start,
-			      last + 1, addr, flags, true);
-
+	if (amdgpu_vm_need_backup(adev)) {
+		/* update shadow pt bo */
+		amdgpu_vm_update_ptes(adev, &vm_update_params, vm, start,
+				      last + 1, addr, flags, true);
+	}
 	amdgpu_vm_update_ptes(adev, &vm_update_params, vm, start,
 			      last + 1, addr, flags, false);
 
@@ -1458,7 +1477,8 @@ int amdgpu_vm_bo_map(struct amdgpu_device *adev,
 				     AMDGPU_GPU_PAGE_SIZE, true,
 				     AMDGPU_GEM_DOMAIN_VRAM,
 				     AMDGPU_GEM_CREATE_NO_CPU_ACCESS |
-				     AMDGPU_GEM_CREATE_SHADOW,
+				     (amdgpu_vm_need_backup(adev) ?
+				      AMDGPU_GEM_CREATE_SHADOW : 0),
 				     NULL, resv, &pt);
 		if (r)
 			goto error_free;
@@ -1481,12 +1501,14 @@ int amdgpu_vm_bo_map(struct amdgpu_device *adev,
 		entry->user_pages = NULL;
 		vm->page_tables[pt_idx].addr = 0;
 
-		entry_shadow->robj = pt->shadow;
-		entry_shadow->priority = 0;
-		entry_shadow->tv.bo = &entry_shadow->robj->tbo;
-		entry_shadow->tv.shared = true;
-		entry_shadow->user_pages = NULL;
-		vm->page_tables[pt_idx].addr_shadow = 0;
+		if (amdgpu_vm_need_backup(adev)) {
+			entry_shadow->robj = pt->shadow;
+			entry_shadow->priority = 0;
+			entry_shadow->tv.bo = &entry_shadow->robj->tbo;
+			entry_shadow->tv.shared = true;
+			entry_shadow->user_pages = NULL;
+			vm->page_tables[pt_idx].addr_shadow = 0;
+		}
 	}
 
 	return 0;
@@ -1667,7 +1689,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 	r = amdgpu_bo_create(adev, pd_size, align, true,
 			     AMDGPU_GEM_DOMAIN_VRAM,
 			     AMDGPU_GEM_CREATE_NO_CPU_ACCESS |
-			     AMDGPU_GEM_CREATE_SHADOW,
+			     (amdgpu_vm_need_backup(adev) ?
+			      AMDGPU_GEM_CREATE_SHADOW : 0),
 			     NULL, NULL, &vm->page_directory);
 	if (r)
 		goto error_free_sched_entity;
@@ -1682,11 +1705,13 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 		goto error_free_page_directory;
 	vm->last_eviction_counter = atomic64_read(&adev->num_evictions);
 
-	vm->pd_entry_shadow.robj = vm->page_directory->shadow;
-	vm->pd_entry_shadow.priority = 0;
-	vm->pd_entry_shadow.tv.bo = &vm->page_directory->shadow->tbo;
-	vm->pd_entry_shadow.tv.shared = true;
-	vm->pd_entry_shadow.user_pages = NULL;
+	if (amdgpu_vm_need_backup(adev)) {
+		vm->pd_entry_shadow.robj = vm->page_directory->shadow;
+		vm->pd_entry_shadow.priority = 0;
+		vm->pd_entry_shadow.tv.bo = &vm->page_directory->shadow->tbo;
+		vm->pd_entry_shadow.tv.shared = true;
+		vm->pd_entry_shadow.user_pages = NULL;
+	}
 	spin_lock(&adev->vm_list_lock);
 	list_add_tail(&vm->list, &adev->vm_list);
 	spin_unlock(&adev->vm_list_lock);
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 00/13] shadow page table support

[Christian König]

Patch #1 looks like a fix to me, please add my rb and push ASAP.

You should separate such fixes from bugger sets, send out individually 
and maybe ping Alex to review and add them to his fixes branch.

Patch #2:
> +		amdgpu_ttm_placement_init(adev, &placement,
> +					  placements, AMDGPU_GEM_DOMAIN_GTT,
> +					  AMDGPU_GEM_CREATE_CPU_GTT_USWC);
> +
> +		r = amdgpu_bo_create_restricted(adev, size, byte_align, kernel,
> +						AMDGPU_GEM_DOMAIN_GTT,
> +						AMDGPU_GEM_CREATE_CPU_GTT_USWC,
> +						NULL, &placement,
> +						(*bo_ptr)->tbo.resv,
> +						&(*bo_ptr)->shadow);
> +		if (r)
> +			amdgpu_bo_unref(bo_ptr);

I think we should use the same reservation object as the parent BO here 
and add something like "(*bo_ptr)->shadow->parent = 
amdgpu_bo_ref(*bo_ptr);".

This way we can be sure that the shadow isn't freed before the BO.

> +	} else
> +		(*bo_ptr)->shadow = NULL;
 From the coding style when an "if" has {} the else part needs {} as 
well. But I think BOs are zero allocated anyway, so we can probably 
completely drop that.

> +		if (tbo == NULL)
Well that check is totally nonsense in the originally code as well. We 
should probably remove it.

Additional to that I would add something to amdgpu_cs_list_validate() to 
always validate the shadow when the real BO is validated.

Patch #3:
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index d415805..ae4608c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -875,6 +875,8 @@ struct amdgpu_ring {
>   struct amdgpu_vm_pt {
>   	struct amdgpu_bo_list_entry	entry;
>   	uint64_t			addr;
> +	struct amdgpu_bo_list_entry	entry_shadow;
> +	uint64_t			addr_shadow;
>   };
This becomes unnecessary when we use the same reservation object for the 
shadow and validate during CS and amdgpu_gem_va_update_vm().

Should reduce the CPU overhead of this significantly, cause otherwise we 
need to reserve and validate another BO for each page table.

For the remaining patches I would still use the approach of the common 
backup procedure as we discussed (with inverting the order for the VM 
BOs). Doubling all the page table updates clearly doesn't sound like a 
good idea to me.

Regards,
Christian.

Am 02.08.2016 um 09:48 schrieb Chunming Zhou:
> Since we cannot make sure VRAM is safe after gpu reset, page table backup
> is neccessary, shadow page table is sense way to recovery page talbe when
> gpu reset happens.
> We need to allocate GTT bo as the shadow of VRAM bo when creating page table,
> and make them same. After gpu reset, we will need to use SDMA to copy GTT bo
> content to VRAM bo, then page table will be recoveried.
>
> Chunming Zhou (13):
>    drm/amdgpu: irq resume should be immediately after gpu resume
>    drm/amdgpu: add shadow bo support
>    drm/amdgpu: set shadow flag for pd/pt bo
>    drm/amdgpu: update shadow pt bo while update pt
>    drm/amdgpu: update pd shadow while updating pd
>    drm/amdgpu: implement amdgpu_vm_recover_page_table_from_shadow
>    drm/amdgpu: link all vm clients
>    drm/amdgpu: add vm_list_lock
>    drm/amd: add block entity function
>    drm/amdgpu: recover page tables after gpu reset
>    drm/amd: wait neccessary dependency before running job
>    drm/amdgpu: add vm recover pt fence
>    drm/amdgpu: add backup condition for shadow page table
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  15 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c        |   6 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |  33 +++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       |   5 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c    |  36 ++++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        | 219 ++++++++++++++++++++++----
>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.c |  30 +++-
>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |   3 +
>   include/uapi/drm/amdgpu_drm.h                 |   2 +
>   9 files changed, 316 insertions(+), 33 deletions(-)
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 00/13] shadow page table support

[zhoucm1]

On 2016年08月03日 21:39, Christian König wrote:
> Doubling all the page table updates clearly doesn't sound like a good 
> idea to me.
Could you tell me why it isn't a good idea?
I though the overhead is the least, and we don't need to sync between bo 
and its shadow, aren't we?

Regards,
David Zhou
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 00/13] shadow page table support

[Christian König]

Am 04.08.2016 um 08:05 schrieb zhoucm1:
>
>
> On 2016年08月03日 21:39, Christian König wrote:
>> Doubling all the page table updates clearly doesn't sound like a good 
>> idea to me.
> Could you tell me why it isn't a good idea?
> I though the overhead is the least, and we don't need to sync between 
> bo and its shadow, aren't we?

Yeah, but you also double the CPU overhead generating the update 
commands. And for large updates you need to backup the whole page table 
BO anyway, so the copy overhead between system memory and VRAM shouldn't 
matter so much.

I would rather go into the direction that we switch shadow and real BO 
like we discussed previously.

This way you would made all updates on the shadow first and then copy 
all changes page tables to their VRAM location after scheduling all the 
updates.

Regards,
Christian.

>
> Regards,
> David Zhou


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 00/13] shadow page table support

[zhoucm1]

On 2016年08月04日 17:52, Christian König wrote:
> Am 04.08.2016 um 08:05 schrieb zhoucm1:
>>
>>
>> On 2016年08月03日 21:39, Christian König wrote:
>>> Doubling all the page table updates clearly doesn't sound like a 
>>> good idea to me.
>> Could you tell me why it isn't a good idea?
>> I though the overhead is the least, and we don't need to sync between 
>> bo and its shadow, aren't we?
>
> Yeah, but you also double the CPU overhead generating the update 
> commands. And for large updates you need to backup the whole page 
> table BO anyway, so the copy overhead between system memory and VRAM 
> shouldn't matter so much.
>
> I would rather go into the direction that we switch shadow and real BO 
> like we discussed previously.
>
> This way you would made all updates on the shadow first and then copy 
> all changes page tables to their VRAM location after scheduling all 
> the updates.
I have implemented your suggestion, and tested just now. The big problem 
is coming, the performance dropped very much, from 68fps to 50fps with 
heaven on Fiji.
The root cause is coping shadow bo to pt bo need to take pd reservation, 
which kills your improvement of sync for vm page table udpating.

I checked doubling update pt, which fps is 64.5fps, dropped 3.5fps from 
68fps.

With thinking more, if we select doubling update, then updating shadow 
jobs don't need to align with normal pt jobs, since shadow jobs contain 
the content of updating, just let them run in alone entity (shadow 
entity) with normal run_queue, after gpu reset, we wait for all shadow 
jobs finished, and then recover page table from shadow, this way, we 
will completely avoid the shadow jobs effect.

What do you think of it?

Regards,
David Zhou

>
> Regards,
> Christian.
>
>>
>> Regards,
>> David Zhou
>
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 00/13] shadow page table support

[Christian König]

Am 05.08.2016 um 04:43 schrieb zhoucm1:
>
>
> On 2016年08月04日 17:52, Christian König wrote:
>> Am 04.08.2016 um 08:05 schrieb zhoucm1:
>>>
>>>
>>> On 2016年08月03日 21:39, Christian König wrote:
>>>> Doubling all the page table updates clearly doesn't sound like a 
>>>> good idea to me.
>>> Could you tell me why it isn't a good idea?
>>> I though the overhead is the least, and we don't need to sync 
>>> between bo and its shadow, aren't we?
>>
>> Yeah, but you also double the CPU overhead generating the update 
>> commands. And for large updates you need to backup the whole page 
>> table BO anyway, so the copy overhead between system memory and VRAM 
>> shouldn't matter so much.
>>
>> I would rather go into the direction that we switch shadow and real 
>> BO like we discussed previously.
>>
>> This way you would made all updates on the shadow first and then copy 
>> all changes page tables to their VRAM location after scheduling all 
>> the updates.
> I have implemented your suggestion, and tested just now. The big 
> problem is coming, the performance dropped very much, from 68fps to 
> 50fps with heaven on Fiji.
> The root cause is coping shadow bo to pt bo need to take pd 
> reservation, which kills your improvement of sync for vm page table 
> udpating.

Mhm, that is indeed a bit problematic. I don't know of hand either how 
to fix this.

>
> I checked doubling update pt, which fps is 64.5fps, dropped 3.5fps 
> from 68fps.
>
> With thinking more, if we select doubling update, then updating shadow 
> jobs don't need to align with normal pt jobs, since shadow jobs 
> contain the content of updating, just let them run in alone entity 
> (shadow entity) with normal run_queue, after gpu reset, we wait for 
> all shadow jobs finished, and then recover page table from shadow, 
> this way, we will completely avoid the shadow jobs effect.
>
> What do you think of it?

This way you need to enable the scheduler again before you can recover 
anything, which is clearly not such a good idea.

Ok, let's do this with the double pt update for now. From your numbers 
that looks like the option with the least impact and we can work on that 
later on if we really find the double CPU overhead to be a problem.

Regards,
Christian.

>
> Regards,
> David Zhou
>
>>
>> Regards,
>> Christian.
>>
>>>
>>> Regards,
>>> David Zhou
>>
>>
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 00/13] shadow page table support

[zhoucm1]

On 2016年08月05日 16:56, Christian König wrote:
> Am 05.08.2016 um 04:43 schrieb zhoucm1:
>>
>>
>> On 2016年08月04日 17:52, Christian König wrote:
>>> Am 04.08.2016 um 08:05 schrieb zhoucm1:
>>>>
>>>>
>>>> On 2016年08月03日 21:39, Christian König wrote:
>>>>> Doubling all the page table updates clearly doesn't sound like a 
>>>>> good idea to me.
>>>> Could you tell me why it isn't a good idea?
>>>> I though the overhead is the least, and we don't need to sync 
>>>> between bo and its shadow, aren't we?
>>>
>>> Yeah, but you also double the CPU overhead generating the update 
>>> commands. And for large updates you need to backup the whole page 
>>> table BO anyway, so the copy overhead between system memory and VRAM 
>>> shouldn't matter so much.
>>>
>>> I would rather go into the direction that we switch shadow and real 
>>> BO like we discussed previously.
>>>
>>> This way you would made all updates on the shadow first and then 
>>> copy all changes page tables to their VRAM location after scheduling 
>>> all the updates.
>> I have implemented your suggestion, and tested just now. The big 
>> problem is coming, the performance dropped very much, from 68fps to 
>> 50fps with heaven on Fiji.
>> The root cause is coping shadow bo to pt bo need to take pd 
>> reservation, which kills your improvement of sync for vm page table 
>> udpating.
>
> Mhm, that is indeed a bit problematic. I don't know of hand either how 
> to fix this.
>
>>
>> I checked doubling update pt, which fps is 64.5fps, dropped 3.5fps 
>> from 68fps.
>>
>> With thinking more, if we select doubling update, then updating 
>> shadow jobs don't need to align with normal pt jobs, since shadow 
>> jobs contain the content of updating, just let them run in alone 
>> entity (shadow entity) with normal run_queue, after gpu reset, we 
>> wait for all shadow jobs finished, and then recover page table from 
>> shadow, this way, we will completely avoid the shadow jobs effect.
>>
>> What do you think of it?
>
> This way you need to enable the scheduler again before you can recover 
> anything, which is clearly not such a good idea.
I just completed this solution, and it works very fine as well, there 
isn't any performance dropped.
Whatever we select any of the three solutions, we cannot avoid to enable 
scheduler, since recovery page table jobs have to use scheduler.
Should I send which solution patch to review?

Regards,
David Zhou
>
> Ok, let's do this with the double pt update for now. From your numbers 
> that looks like the option with the least impact and we can work on 
> that later on if we really find the double CPU overhead to be a problem.
>
> Regards,
> Christian.
>
>>
>> Regards,
>> David Zhou
>>
>>>
>>> Regards,
>>> Christian.
>>>
>>>>
>>>> Regards,
>>>> David Zhou
>>>
>>>
>>
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 00/13] shadow page table support

[Christian König]

Am 05.08.2016 um 11:12 schrieb zhoucm1:
>
>
> On 2016年08月05日 16:56, Christian König wrote:
>> Am 05.08.2016 um 04:43 schrieb zhoucm1:
>>>
>>>
>>> On 2016年08月04日 17:52, Christian König wrote:
>>>> Am 04.08.2016 um 08:05 schrieb zhoucm1:
>>>>>
>>>>>
>>>>> On 2016年08月03日 21:39, Christian König wrote:
>>>>>> Doubling all the page table updates clearly doesn't sound like a 
>>>>>> good idea to me.
>>>>> Could you tell me why it isn't a good idea?
>>>>> I though the overhead is the least, and we don't need to sync 
>>>>> between bo and its shadow, aren't we?
>>>>
>>>> Yeah, but you also double the CPU overhead generating the update 
>>>> commands. And for large updates you need to backup the whole page 
>>>> table BO anyway, so the copy overhead between system memory and 
>>>> VRAM shouldn't matter so much.
>>>>
>>>> I would rather go into the direction that we switch shadow and real 
>>>> BO like we discussed previously.
>>>>
>>>> This way you would made all updates on the shadow first and then 
>>>> copy all changes page tables to their VRAM location after 
>>>> scheduling all the updates.
>>> I have implemented your suggestion, and tested just now. The big 
>>> problem is coming, the performance dropped very much, from 68fps to 
>>> 50fps with heaven on Fiji.
>>> The root cause is coping shadow bo to pt bo need to take pd 
>>> reservation, which kills your improvement of sync for vm page table 
>>> udpating.
>>
>> Mhm, that is indeed a bit problematic. I don't know of hand either 
>> how to fix this.
>>
>>>
>>> I checked doubling update pt, which fps is 64.5fps, dropped 3.5fps 
>>> from 68fps.
>>>
>>> With thinking more, if we select doubling update, then updating 
>>> shadow jobs don't need to align with normal pt jobs, since shadow 
>>> jobs contain the content of updating, just let them run in alone 
>>> entity (shadow entity) with normal run_queue, after gpu reset, we 
>>> wait for all shadow jobs finished, and then recover page table from 
>>> shadow, this way, we will completely avoid the shadow jobs effect.
>>>
>>> What do you think of it?
>>
>> This way you need to enable the scheduler again before you can 
>> recover anything, which is clearly not such a good idea.
> I just completed this solution, and it works very fine as well, there 
> isn't any performance dropped.

Well that sounds good.

> Whatever we select any of the three solutions, we cannot avoid to 
> enable scheduler, since recovery page table jobs have to use scheduler.
Why? I though we agreed to be able to send them to the rings directly?

> Should I send which solution patch to review?

Yeah, let's discuss on the code directly.

Regards,
Christian.

>
> Regards,
> David Zhou
>>
>> Ok, let's do this with the double pt update for now. From your 
>> numbers that looks like the option with the least impact and we can 
>> work on that later on if we really find the double CPU overhead to be 
>> a problem.
>>
>> Regards,
>> Christian.
>>
>>>
>>> Regards,
>>> David Zhou
>>>
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>>
>>>>> Regards,
>>>>> David Zhou
>>>>
>>>>
>>>
>>
>
> _______________________________________________
> 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

Re: [PATCH 00/13] shadow page table support

[zhoucm1]

On 2016年08月05日 16:56, Christian König wrote:
> Am 05.08.2016 um 04:43 schrieb zhoucm1:
>>
>>
>> On 2016年08月04日 17:52, Christian König wrote:
>>> Am 04.08.2016 um 08:05 schrieb zhoucm1:
>>>>
>>>>
>>>> On 2016年08月03日 21:39, Christian König wrote:
>>>>> Doubling all the page table updates clearly doesn't sound like a 
>>>>> good idea to me.
>>>> Could you tell me why it isn't a good idea?
>>>> I though the overhead is the least, and we don't need to sync 
>>>> between bo and its shadow, aren't we?
>>>
>>> Yeah, but you also double the CPU overhead generating the update 
>>> commands. And for large updates you need to backup the whole page 
>>> table BO anyway, so the copy overhead between system memory and VRAM 
>>> shouldn't matter so much.
>>>
>>> I would rather go into the direction that we switch shadow and real 
>>> BO like we discussed previously.
>>>
>>> This way you would made all updates on the shadow first and then 
>>> copy all changes page tables to their VRAM location after scheduling 
>>> all the updates.
>> I have implemented your suggestion, and tested just now. The big 
>> problem is coming, the performance dropped very much, from 68fps to 
>> 50fps with heaven on Fiji.
>> The root cause is coping shadow bo to pt bo need to take pd 
>> reservation, which kills your improvement of sync for vm page table 
>> udpating.
>
> Mhm, that is indeed a bit problematic. I don't know of hand either how 
> to fix this.
>
>>
>> I checked doubling update pt, which fps is 64.5fps, dropped 3.5fps 
>> from 68fps.
>>
>> With thinking more, if we select doubling update, then updating 
>> shadow jobs don't need to align with normal pt jobs, since shadow 
>> jobs contain the content of updating, just let them run in alone 
>> entity (shadow entity) with normal run_queue, after gpu reset, we 
>> wait for all shadow jobs finished, and then recover page table from 
>> shadow, this way, we will completely avoid the shadow jobs effect.
>>
>> What do you think of it?
>
> This way you need to enable the scheduler again before you can recover 
> anything, which is clearly not such a good idea.
I just completed this solution, and it works very fine as well, there 
isn't any performance dropped.
Whatever we select any of the three solutions, we cannot avoid to enable 
scheduler, since recovery page table jobs have to use scheduler.
Should I send which solution patch to review?

Regards,
David Zhou
>
> Ok, let's do this with the double pt update for now. From your numbers 
> that looks like the option with the least impact and we can work on 
> that later on if we really find the double CPU overhead to be a problem.
>
> Regards,
> Christian.
>
>>
>> Regards,
>> David Zhou
>>
>>>
>>> Regards,
>>> Christian.
>>>
>>>>
>>>> Regards,
>>>> David Zhou
>>>
>>>
>>
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 00/13] shadow page table support

[Chunming Zhou]

Since we cannot make sure VRAM is safe after gpu reset, page table backup
is neccessary, shadow page table is sense way to recovery page talbe when
gpu reset happens.
We need to allocate GTT bo as the shadow of VRAM bo when creating page table,
and make them same. After gpu reset, we will need to use SDMA to copy GTT bo
content to VRAM bo, then page table will be recoveried.

TODO: gart table should be saved as well, will generate a sperate patch set.

Chunming Zhou (13):
  drm/amdgpu: irq resume should be immediately after gpu resume
  drm/amdgpu: add shadow bo support
  drm/amdgpu: set shadow flag for pd/pt bo
  drm/amdgpu: update shadow pt bo while update pt
  drm/amdgpu: update pd shadow while updating pd
  drm/amdgpu: implement amdgpu_vm_recover_page_table_from_shadow
  drm/amdgpu: link all vm clients
  drm/amdgpu: add vm_list_lock
  drm/amd: add block entity function
  drm/amdgpu: recover page tables after gpu reset
  drm/amd: wait neccessary dependency before running job
  drm/amdgpu: add vm recover pt fence
  drm/amdgpu: add backup condition for shadow page table

 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  15 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c        |   6 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |  33 +++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c       |   5 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c    |  36 ++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        | 219 ++++++++++++++++++++++----
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c |  30 +++-
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |   3 +
 include/uapi/drm/amdgpu_drm.h                 |   2 +
 9 files changed, 316 insertions(+), 33 deletions(-)

-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 01/13] drm/amdgpu: irq resume should be immediately after gpu resume

[Chunming Zhou]

Change-Id: Icf64bf5964f0ef66c239ab0679d51275cc272699
Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index eab931a..449ea00 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2143,6 +2143,7 @@ retry:
 		amdgpu_atombios_scratch_regs_restore(adev);
 	}
 	if (!r) {
+		amdgpu_irq_gpu_reset_resume_helper(adev);
 		r = amdgpu_ib_ring_tests(adev);
 		if (r) {
 			dev_err(adev->dev, "ib ring test failed (%d).\n", r);
@@ -2177,7 +2178,6 @@ retry:
 		/* bad news, how to tell it to userspace ? */
 		dev_info(adev->dev, "GPU reset failed\n");
 	}
-	amdgpu_irq_gpu_reset_resume_helper(adev);
 
 	return r;
 }
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx