PRT support for amdgpu

PRT support for amdgpu

[Christian König]

Hi Dave and Bas,

Hi Dave and Bas,

the following set of patches is a proposal for adding support for partial resident textures (PRT) to the amdgpu kernel module.

The basic idea behind PRT support is that you turn of VM fault reporting and only map parts of a texture into your virtual address space.

When a shader now tries to sample from a not present page it gets a notification instead of a VM fault and can react gracefully by switch to a different LOD for example.

On our current available hardware generation you can unfortunately only turn of VM faults globally, but on future generation you can do this on a per page basis. So my proposal is to have a consistent interface over all generations with a per mapping PRT flag, but enable/disable it globally on current hardware when the first/last mapping is made/destroyed.

An open problem with the proposal is that we don't know when or if we want to add the userspace implementation into radeonsi.

So price question could you guys use this for radv as well? Or is it sufficient to just write an unit test for it?

Best regards,
Christian.

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

[PATCH 1/6] drm/amdgpu: add support for BO_VAs without BO

[Christian König]

From: Christian König <christian.koenig@amd.com>

For PRT support we need mappings which aren't backed by any memory.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 8e6030d..87eae9b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1117,7 +1117,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
 	struct fence *exclusive;
 	int r;
 
-	if (clear) {
+	if (clear || !bo_va->bo) {
 		mem = NULL;
 		nodes = NULL;
 		exclusive = NULL;
@@ -1134,9 +1134,15 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
 		exclusive = reservation_object_get_excl(bo_va->bo->tbo.resv);
 	}
 
-	flags = amdgpu_ttm_tt_pte_flags(adev, bo_va->bo->tbo.ttm, mem);
-	gtt_flags = (amdgpu_ttm_is_bound(bo_va->bo->tbo.ttm) &&
-		adev == amdgpu_ttm_adev(bo_va->bo->tbo.bdev)) ? flags : 0;
+	if (bo_va->bo) {
+		flags = amdgpu_ttm_tt_pte_flags(adev, bo_va->bo->tbo.ttm, mem);
+		gtt_flags = (amdgpu_ttm_is_bound(bo_va->bo->tbo.ttm) &&
+			adev == amdgpu_ttm_adev(bo_va->bo->tbo.bdev)) ?
+			flags : 0;
+	} else {
+		flags = 0x0;
+		gtt_flags = ~0x0;
+	}
 
 	spin_lock(&vm->status_lock);
 	if (!list_empty(&bo_va->vm_status))
@@ -1271,7 +1277,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
 	INIT_LIST_HEAD(&bo_va->invalids);
 	INIT_LIST_HEAD(&bo_va->vm_status);
 
-	list_add_tail(&bo_va->bo_list, &bo->va);
+	if (bo)
+		list_add_tail(&bo_va->bo_list, &bo->va);
 
 	return bo_va;
 }
@@ -1309,7 +1316,8 @@ int amdgpu_vm_bo_map(struct amdgpu_device *adev,
 
 	/* make sure object fit at this offset */
 	eaddr = saddr + size - 1;
-	if ((saddr >= eaddr) || (offset + size > amdgpu_bo_size(bo_va->bo)))
+	if (bo_va->bo && (saddr >= eaddr ||
+			  (offset + size > amdgpu_bo_size(bo_va->bo))))
 		return -EINVAL;
 
 	last_pfn = eaddr / AMDGPU_GPU_PAGE_SIZE;
-- 
2.5.0

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

[PATCH 2/6] drm/amdgpu: add basic PRT support

[Christian König]

From: Christian König <christian.koenig@amd.com>

Future hardware generations can handle PRT flags on a per page basis,
but current hardware can only turn it on globally.

Add the basic handling for both, a global callback to enable/disable
triggered by setting a per mapping flag.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h    |   2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 101 +++++++++++++++++++++++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |   6 ++
 3 files changed, 98 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 402a895..34a971a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -296,6 +296,8 @@ struct amdgpu_gart_funcs {
 			   uint32_t gpu_page_idx, /* pte/pde to update */
 			   uint64_t addr, /* addr to write into pte/pde */
 			   uint32_t flags); /* access flags */
+	/* enable/disable PRT support */
+	void (*set_prt)(struct amdgpu_device *adev, bool enable);
 };
 
 /* provided by the ih block */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 87eae9b..5d0afca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -69,6 +69,12 @@ struct amdgpu_pte_update_params {
 	bool shadow;
 };
 
+/* Helper to disable partial resident texture feature from a fence callback */
+struct amdgpu_prt_cb {
+	struct amdgpu_device *adev;
+	struct fence_cb cb;
+};
+
 /**
  * amdgpu_vm_num_pde - return the number of page directory entries
  *
@@ -989,11 +995,8 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
 		goto error_free;
 
 	amdgpu_bo_fence(vm->page_directory, f, true);
-	if (fence) {
-		fence_put(*fence);
-		*fence = fence_get(f);
-	}
-	fence_put(f);
+	fence_put(*fence);
+	*fence = f;
 	return 0;
 
 error_free:
@@ -1177,6 +1180,61 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
 }
 
 /**
+ * amdgpu_vm_update_prt_state - update the global PRT state
+ */
+static void amdgpu_vm_update_prt_state(struct amdgpu_device *adev)
+{
+	unsigned long flags;
+	bool enable;
+
+	spin_lock_irqsave(&adev->vm_manager.prt_lock, flags);
+	enable = !!atomic_read(&adev->vm_manager.num_prt_mappings);
+	adev->gart.gart_funcs->set_prt(adev, enable);
+	spin_unlock_irqrestore(&adev->vm_manager.prt_lock, flags);
+}
+
+/**
+ * amdgpu_vm_prt - callback for updating the PRT status
+ */
+static void amdgpu_vm_prt_cb(struct fence *fence, struct fence_cb *_cb)
+{
+	struct amdgpu_prt_cb *cb = container_of(_cb, struct amdgpu_prt_cb, cb);
+
+	amdgpu_vm_update_prt_state(cb->adev);
+	kfree(cb);
+}
+
+/**
+ * amdgpu_vm_free_mapping - free a mapping
+ *
+ * @adev: amdgpu_device pointer
+ * @vm: requested vm
+ * @mapping: mapping to be freed
+ * @fence: fence of the unmap operation
+ *
+ * Free a mapping and make sure we decrease the PRT usage count if applicable.
+ */
+static void amdgpu_vm_free_mapping(struct amdgpu_device *adev,
+				   struct amdgpu_vm *vm,
+				   struct amdgpu_bo_va_mapping *mapping,
+				   struct fence *fence)
+{
+	if ((mapping->flags & AMDGPU_PTE_PRT) &&
+	    atomic_dec_return(&adev->vm_manager.num_prt_mappings) == 0) {
+		struct amdgpu_prt_cb *cb = kmalloc(sizeof(struct amdgpu_prt_cb),
+						   GFP_KERNEL);
+
+		cb->adev = adev;
+		if (!fence || fence_add_callback(fence, &cb->cb,
+						 amdgpu_vm_prt_cb)) {
+			amdgpu_vm_update_prt_state(adev);
+			kfree(cb);
+		}
+	}
+	kfree(mapping);
+}
+
+/**
  * amdgpu_vm_clear_freed - clear freed BOs in the PT
  *
  * @adev: amdgpu_device pointer
@@ -1191,6 +1249,7 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
 			  struct amdgpu_vm *vm)
 {
 	struct amdgpu_bo_va_mapping *mapping;
+	struct fence *fence = NULL;
 	int r;
 
 	while (!list_empty(&vm->freed)) {
@@ -1199,12 +1258,15 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
 		list_del(&mapping->list);
 
 		r = amdgpu_vm_bo_split_mapping(adev, NULL, 0, NULL, vm, mapping,
-					       0, 0, NULL);
-		kfree(mapping);
-		if (r)
+					       0, 0, &fence);
+		amdgpu_vm_free_mapping(adev, vm, mapping, fence);
+		if (r) {
+			fence_put(fence);
 			return r;
+		}
 
 	}
+	fence_put(fence);
 	return 0;
 
 }
@@ -1314,6 +1376,15 @@ int amdgpu_vm_bo_map(struct amdgpu_device *adev,
 	    size == 0 || size & AMDGPU_GPU_PAGE_MASK)
 		return -EINVAL;
 
+	if (flags & AMDGPU_PTE_PRT) {
+		/* Check if we have PRT hardware support */
+		if (!adev->gart.gart_funcs->set_prt)
+			return -EINVAL;
+
+		if (atomic_inc_return(&adev->vm_manager.num_prt_mappings) == 1)
+			amdgpu_vm_update_prt_state(adev);
+	}
+
 	/* make sure object fit at this offset */
 	eaddr = saddr + size - 1;
 	if (bo_va->bo && (saddr >= eaddr ||
@@ -1400,7 +1471,7 @@ int amdgpu_vm_bo_map(struct amdgpu_device *adev,
 	list_del(&mapping->list);
 	interval_tree_remove(&mapping->it, &vm->va);
 	trace_amdgpu_vm_bo_unmap(bo_va, mapping);
-	kfree(mapping);
+	amdgpu_vm_free_mapping(adev, vm, mapping, NULL);
 
 error:
 	return r;
@@ -1452,7 +1523,8 @@ int amdgpu_vm_bo_unmap(struct amdgpu_device *adev,
 	if (valid)
 		list_add(&mapping->list, &vm->freed);
 	else
-		kfree(mapping);
+		amdgpu_vm_free_mapping(adev, vm, mapping,
+				       bo_va->last_pt_update);
 
 	return 0;
 }
@@ -1488,7 +1560,8 @@ void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
 	list_for_each_entry_safe(mapping, next, &bo_va->invalids, list) {
 		list_del(&mapping->list);
 		interval_tree_remove(&mapping->it, &vm->va);
-		kfree(mapping);
+		amdgpu_vm_free_mapping(adev, vm, mapping,
+				       bo_va->last_pt_update);
 	}
 
 	fence_put(bo_va->last_pt_update);
@@ -1625,9 +1698,13 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 		kfree(mapping);
 	}
 	list_for_each_entry_safe(mapping, tmp, &vm->freed, list) {
+		if (mapping->flags & AMDGPU_PTE_PRT)
+			continue;
+
 		list_del(&mapping->list);
 		kfree(mapping);
 	}
+	amdgpu_vm_clear_freed(adev, vm);
 
 	for (i = 0; i < amdgpu_vm_num_pdes(adev); i++) {
 		struct amdgpu_bo *pt = vm->page_tables[i].bo;
@@ -1672,6 +1749,8 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev)
 
 	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);
+	atomic_set(&adev->vm_manager.num_prt_mappings, 0);
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 1f99715..4d26e9b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -65,6 +65,8 @@ struct amdgpu_bo_list_entry;
 
 #define AMDGPU_PTE_FRAG(x)	((x & 0x1f) << 7)
 
+#define AMDGPU_PTE_PRT		(1UL << 63)
+
 /* How to programm VM fault handling */
 #define AMDGPU_VM_FAULT_STOP_NEVER	0
 #define AMDGPU_VM_FAULT_STOP_FIRST	1
@@ -159,6 +161,10 @@ struct amdgpu_vm_manager {
 	atomic_t				vm_pte_next_ring;
 	/* client id counter */
 	atomic64_t				client_counter;
+
+	/* partial resident texture handling */
+	spinlock_t				prt_lock;
+	atomic_t				num_prt_mappings;
 };
 
 void amdgpu_vm_manager_init(struct amdgpu_device *adev);
-- 
2.5.0

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

[PATCH 3/6] drm/amdgpu: IOCTL interface for PRT support v3

[Christian König]

From: Junwei Zhang <Jerry.Zhang@amd.com>

Till GFX8 we can only enable PRT support globally, but with the next hardware
generation we can do this on a per page basis.

Keep the interface consistent by adding PRT mappings and enable
support globally on current hardware when the first mapping is made.

v2: disable PRT support delayed and on all error paths
v3: PRT and other permissions are mutal exclusive,
    PRT mappings don't need a BO.

Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 62 ++++++++++++++++++++-------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 10 ++++++
 include/uapi/drm/amdgpu_drm.h           |  2 ++
 4 files changed, 51 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 34a971a..99ca5e8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -703,6 +703,7 @@ void amdgpu_ctx_mgr_fini(struct amdgpu_ctx_mgr *mgr);
 
 struct amdgpu_fpriv {
 	struct amdgpu_vm	vm;
+	struct amdgpu_bo_va	*prt_va;
 	struct mutex		bo_list_lock;
 	struct idr		bo_list_handles;
 	struct amdgpu_ctx_mgr	ctx_mgr;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 9dd41e7..af430c2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -540,6 +540,12 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
 int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 			  struct drm_file *filp)
 {
+	const uint32_t valid_flags = AMDGPU_VM_DELAY_UPDATE |
+		AMDGPU_VM_PAGE_READABLE | AMDGPU_VM_PAGE_WRITEABLE |
+		AMDGPU_VM_PAGE_EXECUTABLE;
+	const uint32_t prt_flags = AMDGPU_VM_DELAY_UPDATE |
+		AMDGPU_VM_PAGE_PRT;
+
 	struct drm_amdgpu_gem_va *args = data;
 	struct drm_gem_object *gobj;
 	struct amdgpu_device *adev = dev->dev_private;
@@ -550,7 +556,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 	struct ttm_validate_buffer tv;
 	struct ww_acquire_ctx ticket;
 	struct list_head list;
-	uint32_t invalid_flags, va_flags = 0;
+	uint32_t va_flags = 0;
 	int r = 0;
 
 	if (!adev->vm_manager.enabled)
@@ -564,11 +570,9 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 		return -EINVAL;
 	}
 
-	invalid_flags = ~(AMDGPU_VM_DELAY_UPDATE | AMDGPU_VM_PAGE_READABLE |
-			AMDGPU_VM_PAGE_WRITEABLE | AMDGPU_VM_PAGE_EXECUTABLE);
-	if ((args->flags & invalid_flags)) {
-		dev_err(&dev->pdev->dev, "invalid flags 0x%08X vs 0x%08X\n",
-			args->flags, invalid_flags);
+	if ((args->flags & ~valid_flags) && (args->flags & ~prt_flags)) {
+		dev_err(&dev->pdev->dev, "invalid flags combination 0x%08X\n",
+			args->flags);
 		return -EINVAL;
 	}
 
@@ -582,28 +586,34 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 		return -EINVAL;
 	}
 
-	gobj = drm_gem_object_lookup(filp, args->handle);
-	if (gobj == NULL)
-		return -ENOENT;
-	abo = gem_to_amdgpu_bo(gobj);
 	INIT_LIST_HEAD(&list);
-	tv.bo = &abo->tbo;
-	tv.shared = false;
-	list_add(&tv.head, &list);
+	if (!(args->flags & AMDGPU_VM_PAGE_PRT)) {
+		gobj = drm_gem_object_lookup(filp, args->handle);
+		if (gobj == NULL)
+			return -ENOENT;
+		abo = gem_to_amdgpu_bo(gobj);
+		tv.bo = &abo->tbo;
+		tv.shared = false;
+		list_add(&tv.head, &list);
+	} else {
+		gobj = NULL;
+		abo = NULL;
+	}
 
 	amdgpu_vm_get_pd_bo(&fpriv->vm, &list, &vm_pd);
 
 	r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL);
-	if (r) {
-		drm_gem_object_unreference_unlocked(gobj);
-		return r;
-	}
+	if (r)
+		goto error_unref;
 
-	bo_va = amdgpu_vm_bo_find(&fpriv->vm, abo);
-	if (!bo_va) {
-		ttm_eu_backoff_reservation(&ticket, &list);
-		drm_gem_object_unreference_unlocked(gobj);
-		return -ENOENT;
+	if (abo) {
+		bo_va = amdgpu_vm_bo_find(&fpriv->vm, abo);
+		if (!bo_va) {
+			r = -ENOENT;
+			goto error_backoff;
+		}
+	} else {
+		bo_va = fpriv->prt_va;
 	}
 
 	switch (args->operation) {
@@ -614,6 +624,8 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 			va_flags |= AMDGPU_PTE_WRITEABLE;
 		if (args->flags & AMDGPU_VM_PAGE_EXECUTABLE)
 			va_flags |= AMDGPU_PTE_EXECUTABLE;
+		if (args->flags & AMDGPU_VM_PAGE_PRT)
+			va_flags |= AMDGPU_PTE_PRT;
 		r = amdgpu_vm_bo_map(adev, bo_va, args->va_address,
 				     args->offset_in_bo, args->map_size,
 				     va_flags);
@@ -624,11 +636,13 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 	default:
 		break;
 	}
-	if (!r && !(args->flags & AMDGPU_VM_DELAY_UPDATE) &&
-	    !amdgpu_vm_debug)
+	if (!r && !(args->flags & AMDGPU_VM_DELAY_UPDATE) && !amdgpu_vm_debug)
 		amdgpu_gem_va_update_vm(adev, bo_va, &list, args->operation);
+
+error_backoff:
 	ttm_eu_backoff_reservation(&ticket, &list);
 
+error_unref:
 	drm_gem_object_unreference_unlocked(gobj);
 	return r;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 215f73b..d5f9d6a4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -656,6 +656,14 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
 		goto out_suspend;
 	}
 
+	fpriv->prt_va = amdgpu_vm_bo_add(adev, &fpriv->vm, NULL);
+	if (!fpriv->prt_va) {
+		r = -ENOMEM;
+		amdgpu_vm_fini(adev, &fpriv->vm);
+		kfree(fpriv);
+		goto out_suspend;
+	}
+
 	if (amdgpu_sriov_vf(adev)) {
 		r = amdgpu_map_static_csa(adev, &fpriv->vm);
 		if (r)
@@ -700,6 +708,8 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
 	amdgpu_uvd_free_handles(adev, file_priv);
 	amdgpu_vce_free_handles(adev, file_priv);
 
+	amdgpu_vm_bo_rmv(adev, fpriv->prt_va);
+
 	if (amdgpu_sriov_vf(adev)) {
 		/* TODO: how to handle reserve failure */
 		BUG_ON(amdgpu_bo_reserve(adev->virt.csa_obj, false));
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 2cf8df8..07e3710 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -363,6 +363,8 @@ struct drm_amdgpu_gem_op {
 #define AMDGPU_VM_PAGE_WRITEABLE	(1 << 2)
 /* executable mapping, new for VI */
 #define AMDGPU_VM_PAGE_EXECUTABLE	(1 << 3)
+/* partially resident texture */
+#define AMDGPU_VM_PAGE_PRT		(1 << 4)
 
 struct drm_amdgpu_gem_va {
 	/** GEM object handle */
-- 
2.5.0

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

[PATCH 4/6] drm/amdgpu: implement PRT for GFX6

[Christian König]

From: Christian König <christian.koenig@amd.com>

Enable/disable the handling globally for now and
print a warning when we enable it for the first time.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 63 +++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index e2b0b16..c23503e 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -398,6 +398,68 @@ static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,
 	WREG32(mmVM_CONTEXT1_CNTL, tmp);
 }
 
+ /**
+   + * gmc_v8_0_set_prt - set PRT VM fault
+   + *
+   + * @adev: amdgpu_device pointer
+   + * @enable: enable/disable VM fault handling for PRT
+   +*/
+static void gmc_v6_0_set_prt(struct amdgpu_device *adev, bool enable)
+{
+	u32 tmp;
+
+	if (enable && !adev->mc.prt_warning) {
+		dev_warn(adev->dev, "Disabling VM faults because of PRT request!\n");
+		adev->mc.prt_warning = true;
+	}
+
+	tmp = RREG32(mmVM_PRT_CNTL);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    CB_DISABLE_READ_FAULT_ON_UNMAPPED_ACCESS,
+			    enable);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    CB_DISABLE_WRITE_FAULT_ON_UNMAPPED_ACCESS,
+			    enable);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    TC_DISABLE_READ_FAULT_ON_UNMAPPED_ACCESS,
+			    enable);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    TC_DISABLE_WRITE_FAULT_ON_UNMAPPED_ACCESS,
+			    enable);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    L2_CACHE_STORE_INVALID_ENTRIES,
+			    enable);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    L1_TLB_STORE_INVALID_ENTRIES,
+			    enable);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    MASK_PDE0_FAULT, enable);
+	WREG32(mmVM_CONTEXT1_CNTL, tmp);
+
+	if (enable) {
+		uint32_t low = AMDGPU_VA_RESERVED_SIZE >> AMDGPU_GPU_PAGE_SHIFT;
+		uint32_t high = adev->vm_manager.max_pfn;
+
+		WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, low);
+		WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, low);
+		WREG32(mmVM_PRT_APERTURE2_LOW_ADDR, low);
+		WREG32(mmVM_PRT_APERTURE3_LOW_ADDR, low);
+		WREG32(mmVM_PRT_APERTURE0_HIGH_ADDR, high);
+		WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
+		WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
+		WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
+	} else {
+		WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
+		WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);
+		WREG32(mmVM_PRT_APERTURE2_LOW_ADDR, 0xfffffff);
+		WREG32(mmVM_PRT_APERTURE3_LOW_ADDR, 0xfffffff);
+		WREG32(mmVM_PRT_APERTURE0_HIGH_ADDR, 0x0);
+		WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
+		WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
+		WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
+	}
+}
+
 static int gmc_v6_0_gart_enable(struct amdgpu_device *adev)
 {
 	int r, i;
@@ -1080,6 +1142,7 @@ static const struct amd_ip_funcs gmc_v6_0_ip_funcs = {
 static const struct amdgpu_gart_funcs gmc_v6_0_gart_funcs = {
 	.flush_gpu_tlb = gmc_v6_0_gart_flush_gpu_tlb,
 	.set_pte_pde = gmc_v6_0_gart_set_pte_pde,
+	.set_prt = gmc_v6_0_set_prt,
 };
 
 static const struct amdgpu_irq_src_funcs gmc_v6_0_irq_funcs = {
-- 
2.5.0

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

[PATCH 5/6] drm/amdgpu: implement PRT for GFX7

[Christian König]

From: Christian König <christian.koenig@amd.com>

Enable/disable the handling globally for now and
print a warning when we enable it for the first time.

Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h   |  1 +
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 57 +++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 99ca5e8..d8516dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -571,6 +571,7 @@ struct amdgpu_mc {
 	uint32_t		vram_type;
 	uint32_t                srbm_soft_reset;
 	struct amdgpu_mode_mc_save save;
+	bool			prt_warning;
 };
 
 /*
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index 8d05e0c..1f4a2b3 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -481,6 +481,62 @@ static void gmc_v7_0_set_fault_enable_default(struct amdgpu_device *adev,
 }
 
 /**
+ * gmc_v7_0_set_prt - set PRT VM fault
+ *
+ * @adev: amdgpu_device pointer
+ * @enable: enable/disable VM fault handling for PRT
+ */
+static void gmc_v7_0_set_prt(struct amdgpu_device *adev, bool enable)
+{
+	uint32_t tmp;
+
+	if (enable && !adev->mc.prt_warning) {
+		dev_warn(adev->dev, "Disabling VM faults because of PRT request!\n");
+		adev->mc.prt_warning = true;
+	}
+
+	tmp = RREG32(mmVM_PRT_CNTL);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    CB_DISABLE_READ_FAULT_ON_UNMAPPED_ACCESS, enable);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    CB_DISABLE_WRITE_FAULT_ON_UNMAPPED_ACCESS, enable);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    TC_DISABLE_READ_FAULT_ON_UNMAPPED_ACCESS, enable);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    TC_DISABLE_WRITE_FAULT_ON_UNMAPPED_ACCESS, enable);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    L2_CACHE_STORE_INVALID_ENTRIES, enable);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    L1_TLB_STORE_INVALID_ENTRIES, enable);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    MASK_PDE0_FAULT, enable);
+	WREG32(mmVM_CONTEXT1_CNTL, tmp);
+
+	if (enable) {
+		uint32_t low = AMDGPU_VA_RESERVED_SIZE >> AMDGPU_GPU_PAGE_SHIFT;
+		uint32_t high = adev->vm_manager.max_pfn;
+
+		WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, low);
+		WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, low);
+		WREG32(mmVM_PRT_APERTURE2_LOW_ADDR, low);
+		WREG32(mmVM_PRT_APERTURE3_LOW_ADDR, low);
+		WREG32(mmVM_PRT_APERTURE0_HIGH_ADDR, high);
+		WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
+		WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
+		WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
+	} else {
+		WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
+		WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);
+		WREG32(mmVM_PRT_APERTURE2_LOW_ADDR, 0xfffffff);
+		WREG32(mmVM_PRT_APERTURE3_LOW_ADDR, 0xfffffff);
+		WREG32(mmVM_PRT_APERTURE0_HIGH_ADDR, 0x0);
+		WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
+		WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
+		WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
+	}
+}
+
+/**
  * gmc_v7_0_gart_enable - gart enable
  *
  * @adev: amdgpu_device pointer
@@ -1259,6 +1315,7 @@ static const struct amd_ip_funcs gmc_v7_0_ip_funcs = {
 static const struct amdgpu_gart_funcs gmc_v7_0_gart_funcs = {
 	.flush_gpu_tlb = gmc_v7_0_gart_flush_gpu_tlb,
 	.set_pte_pde = gmc_v7_0_gart_set_pte_pde,
+	.set_prt = gmc_v7_0_set_prt,
 };
 
 static const struct amdgpu_irq_src_funcs gmc_v7_0_irq_funcs = {
-- 
2.5.0

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

[PATCH 6/6] drm/amdgpu: implement PRT for GFX8

[Christian König]

From: Christian König <christian.koenig@amd.com>

Enable/disable the handling globally for now and
print a warning when we enable it for the first time.

Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 57 +++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index 7669b32..d950a03 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -595,6 +595,62 @@ static void gmc_v8_0_set_fault_enable_default(struct amdgpu_device *adev,
 }
 
 /**
+ * gmc_v8_0_set_prt - set PRT VM fault
+ *
+ * @adev: amdgpu_device pointer
+ * @enable: enable/disable VM fault handling for PRT
+*/
+static void gmc_v8_0_set_prt(struct amdgpu_device *adev, bool enable)
+{
+	u32 tmp;
+
+	if (enable && !adev->mc.prt_warning) {
+		dev_warn(adev->dev, "Disabling VM faults because of PRT request!\n");
+		adev->mc.prt_warning = true;
+	}
+
+	tmp = RREG32(mmVM_PRT_CNTL);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    CB_DISABLE_READ_FAULT_ON_UNMAPPED_ACCESS, enable);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    CB_DISABLE_WRITE_FAULT_ON_UNMAPPED_ACCESS, enable);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    TC_DISABLE_READ_FAULT_ON_UNMAPPED_ACCESS, enable);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    TC_DISABLE_WRITE_FAULT_ON_UNMAPPED_ACCESS, enable);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    L2_CACHE_STORE_INVALID_ENTRIES, enable);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    L1_TLB_STORE_INVALID_ENTRIES, enable);
+	tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+			    MASK_PDE0_FAULT, enable);
+	WREG32(mmVM_CONTEXT1_CNTL, tmp);
+
+	if (enable) {
+		uint32_t low = AMDGPU_VA_RESERVED_SIZE >> AMDGPU_GPU_PAGE_SHIFT;
+		uint32_t high = adev->vm_manager.max_pfn;
+
+		WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, low);
+		WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, low);
+		WREG32(mmVM_PRT_APERTURE2_LOW_ADDR, low);
+		WREG32(mmVM_PRT_APERTURE3_LOW_ADDR, low);
+		WREG32(mmVM_PRT_APERTURE0_HIGH_ADDR, high);
+		WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
+		WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
+		WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
+	} else {
+		WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
+		WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);
+		WREG32(mmVM_PRT_APERTURE2_LOW_ADDR, 0xfffffff);
+		WREG32(mmVM_PRT_APERTURE3_LOW_ADDR, 0xfffffff);
+		WREG32(mmVM_PRT_APERTURE0_HIGH_ADDR, 0x0);
+		WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
+		WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
+		WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
+	}
+}
+
+/**
  * gmc_v8_0_gart_enable - gart enable
  *
  * @adev: amdgpu_device pointer
@@ -1485,6 +1541,7 @@ static const struct amd_ip_funcs gmc_v8_0_ip_funcs = {
 static const struct amdgpu_gart_funcs gmc_v8_0_gart_funcs = {
 	.flush_gpu_tlb = gmc_v8_0_gart_flush_gpu_tlb,
 	.set_pte_pde = gmc_v8_0_gart_set_pte_pde,
+	.set_prt = gmc_v8_0_set_prt,
 };
 
 static const struct amdgpu_irq_src_funcs gmc_v8_0_irq_funcs = {
-- 
2.5.0

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

Re: [PATCH 4/6] drm/amdgpu: implement PRT for GFX6

[StDenis, Tom]

[-- Attachment #1.1.1: Type: text/plain, Size: 4881 bytes --]

Minor nit: the comment says v8 [😊]


The changes to the GFX6/7/8 look reasonable though only question is you read from mmVM_PRT_CNTL and then write to mmVM_CONTEXT1_CNTL .  Is that expected?


Tom


________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Christian König <deathsimple@vodafone.de>
Sent: Monday, January 30, 2017 07:57
To: bas@basnieuwenhuizen.nl; airlied@gmail.com
Cc: amd-gfx@lists.freedesktop.org
Subject: [PATCH 4/6] drm/amdgpu: implement PRT for GFX6

From: Christian König <christian.koenig@amd.com>

Enable/disable the handling globally for now and
print a warning when we enable it for the first time.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 63 +++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index e2b0b16..c23503e 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -398,6 +398,68 @@ static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,
         WREG32(mmVM_CONTEXT1_CNTL, tmp);
 }

+ /**
+   + * gmc_v8_0_set_prt - set PRT VM fault
+   + *
+   + * @adev: amdgpu_device pointer
+   + * @enable: enable/disable VM fault handling for PRT
+   +*/
+static void gmc_v6_0_set_prt(struct amdgpu_device *adev, bool enable)
+{
+       u32 tmp;
+
+       if (enable && !adev->mc.prt_warning) {
+               dev_warn(adev->dev, "Disabling VM faults because of PRT request!\n");
+               adev->mc.prt_warning = true;
+       }
+
+       tmp = RREG32(mmVM_PRT_CNTL);
+       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+                           CB_DISABLE_READ_FAULT_ON_UNMAPPED_ACCESS,
+                           enable);
+       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+                           CB_DISABLE_WRITE_FAULT_ON_UNMAPPED_ACCESS,
+                           enable);
+       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+                           TC_DISABLE_READ_FAULT_ON_UNMAPPED_ACCESS,
+                           enable);
+       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+                           TC_DISABLE_WRITE_FAULT_ON_UNMAPPED_ACCESS,
+                           enable);
+       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+                           L2_CACHE_STORE_INVALID_ENTRIES,
+                           enable);
+       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+                           L1_TLB_STORE_INVALID_ENTRIES,
+                           enable);
+       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+                           MASK_PDE0_FAULT, enable);
+       WREG32(mmVM_CONTEXT1_CNTL, tmp);
+
+       if (enable) {
+               uint32_t low = AMDGPU_VA_RESERVED_SIZE >> AMDGPU_GPU_PAGE_SHIFT;
+               uint32_t high = adev->vm_manager.max_pfn;
+
+               WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, low);
+               WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, low);
+               WREG32(mmVM_PRT_APERTURE2_LOW_ADDR, low);
+               WREG32(mmVM_PRT_APERTURE3_LOW_ADDR, low);
+               WREG32(mmVM_PRT_APERTURE0_HIGH_ADDR, high);
+               WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
+               WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
+               WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
+       } else {
+               WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
+               WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);
+               WREG32(mmVM_PRT_APERTURE2_LOW_ADDR, 0xfffffff);
+               WREG32(mmVM_PRT_APERTURE3_LOW_ADDR, 0xfffffff);
+               WREG32(mmVM_PRT_APERTURE0_HIGH_ADDR, 0x0);
+               WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
+               WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
+               WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
+       }
+}
+
 static int gmc_v6_0_gart_enable(struct amdgpu_device *adev)
 {
         int r, i;
@@ -1080,6 +1142,7 @@ static const struct amd_ip_funcs gmc_v6_0_ip_funcs = {
 static const struct amdgpu_gart_funcs gmc_v6_0_gart_funcs = {
         .flush_gpu_tlb = gmc_v6_0_gart_flush_gpu_tlb,
         .set_pte_pde = gmc_v6_0_gart_set_pte_pde,
+       .set_prt = gmc_v6_0_set_prt,
 };

 static const struct amdgpu_irq_src_funcs gmc_v6_0_irq_funcs = {
--
2.5.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
amd-gfx Info Page - lists.freedesktop.org<https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
lists.freedesktop.org
To see the collection of prior postings to the list, visit the amd-gfx Archives. Using amd-gfx: To post a message to all the list members, send email ...




[-- Attachment #1.1.2: Type: text/html, Size: 12916 bytes --]

[-- Attachment #1.2: OutlookEmoji-😊.png --]
[-- Type: image/png, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [PATCH 4/6] drm/amdgpu: implement PRT for GFX6

[Christian König]

[-- Attachment #1.1: Type: text/plain, Size: 5573 bytes --]

> The changes to the GFX6/7/8 look reasonable though only question is 
> you read from mmVM_PRT_CNTL and then write to mmVM_CONTEXT1_CNTL .  Is 
> that expected?
Not at all! Looks like a copy&paste error while modifying the original 
patch. Thanks for catching this.

Also the whole set is so far only compile tested, still need to give it 
a run on all hardware generations anyway.

Christian.

Am 30.01.2017 um 14:59 schrieb StDenis, Tom:
>
> Minor nit: the comment says v8 😊
>
>
> The changes to the GFX6/7/8 look reasonable though only question is 
> you read from mmVM_PRT_CNTL and then write to mmVM_CONTEXT1_CNTL .  Is 
> that expected?
>
>
> Tom
>
>
>
> ------------------------------------------------------------------------
> *From:* amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of 
> Christian König <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
> *Sent:* Monday, January 30, 2017 07:57
> *To:* bas-dldO88ZXqoXqqjsSq9zF6IRWq/SkRNHw@public.gmane.org; airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
> *Cc:* amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> *Subject:* [PATCH 4/6] drm/amdgpu: implement PRT for GFX6
> From: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org>
>
> Enable/disable the handling globally for now and
> print a warning when we enable it for the first time.
>
> Signed-off-by: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org>
> ---
>  drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 63 
> +++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> index e2b0b16..c23503e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> @@ -398,6 +398,68 @@ static void 
> gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,
>          WREG32(mmVM_CONTEXT1_CNTL, tmp);
>  }
>
> + /**
> +   + * gmc_v8_0_set_prt - set PRT VM fault
> +   + *
> +   + * @adev: amdgpu_device pointer
> +   + * @enable: enable/disable VM fault handling for PRT
> +   +*/
> +static void gmc_v6_0_set_prt(struct amdgpu_device *adev, bool enable)
> +{
> +       u32 tmp;
> +
> +       if (enable && !adev->mc.prt_warning) {
> +               dev_warn(adev->dev, "Disabling VM faults because of 
> PRT request!\n");
> +               adev->mc.prt_warning = true;
> +       }
> +
> +       tmp = RREG32(mmVM_PRT_CNTL);
> +       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
> + CB_DISABLE_READ_FAULT_ON_UNMAPPED_ACCESS,
> +                           enable);
> +       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
> + CB_DISABLE_WRITE_FAULT_ON_UNMAPPED_ACCESS,
> +                           enable);
> +       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
> + TC_DISABLE_READ_FAULT_ON_UNMAPPED_ACCESS,
> +                           enable);
> +       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
> + TC_DISABLE_WRITE_FAULT_ON_UNMAPPED_ACCESS,
> +                           enable);
> +       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
> + L2_CACHE_STORE_INVALID_ENTRIES,
> +                           enable);
> +       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
> + L1_TLB_STORE_INVALID_ENTRIES,
> +                           enable);
> +       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
> +                           MASK_PDE0_FAULT, enable);
> +       WREG32(mmVM_CONTEXT1_CNTL, tmp);
> +
> +       if (enable) {
> +               uint32_t low = AMDGPU_VA_RESERVED_SIZE >> 
> AMDGPU_GPU_PAGE_SHIFT;
> +               uint32_t high = adev->vm_manager.max_pfn;
> +
> +               WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, low);
> +               WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, low);
> +               WREG32(mmVM_PRT_APERTURE2_LOW_ADDR, low);
> +               WREG32(mmVM_PRT_APERTURE3_LOW_ADDR, low);
> +               WREG32(mmVM_PRT_APERTURE0_HIGH_ADDR, high);
> +               WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
> +               WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
> +               WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
> +       } else {
> +               WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
> +               WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);
> +               WREG32(mmVM_PRT_APERTURE2_LOW_ADDR, 0xfffffff);
> +               WREG32(mmVM_PRT_APERTURE3_LOW_ADDR, 0xfffffff);
> +               WREG32(mmVM_PRT_APERTURE0_HIGH_ADDR, 0x0);
> +               WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
> +               WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
> +               WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
> +       }
> +}
> +
>  static int gmc_v6_0_gart_enable(struct amdgpu_device *adev)
>  {
>          int r, i;
> @@ -1080,6 +1142,7 @@ static const struct amd_ip_funcs 
> gmc_v6_0_ip_funcs = {
>  static const struct amdgpu_gart_funcs gmc_v6_0_gart_funcs = {
>          .flush_gpu_tlb = gmc_v6_0_gart_flush_gpu_tlb,
>          .set_pte_pde = gmc_v6_0_gart_set_pte_pde,
> +       .set_prt = gmc_v6_0_set_prt,
>  };
>
>  static const struct amdgpu_irq_src_funcs gmc_v6_0_irq_funcs = {
> -- 
> 2.5.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> amd-gfx Info Page - lists.freedesktop.org 
> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
> lists.freedesktop.org
> To see the collection of prior postings to the list, visit the amd-gfx 
> Archives. Using amd-gfx: To post a message to all the list members, 
> send email ...
>
>
>


[-- Attachment #1.2.1: Type: text/html, Size: 16023 bytes --]

[-- Attachment #1.2.2: Type: image/png, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [PATCH 4/6] drm/amdgpu: implement PRT for GFX6

[StDenis, Tom]

[-- Attachment #1.1.1: Type: text/plain, Size: 6433 bytes --]

Hi Christian,


I have SI,CI,VI gear in my office if you have a unit test to try it with.


Cheers,

Tom


________________________________
From: Christian König <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
Sent: Monday, January 30, 2017 09:14
To: StDenis, Tom; bas-dldO88ZXqoXqqjsSq9zF6IRWq/SkRNHw@public.gmane.org; airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH 4/6] drm/amdgpu: implement PRT for GFX6

The changes to the GFX6/7/8 look reasonable though only question is you read from mmVM_PRT_CNTL and then write to mmVM_CONTEXT1_CNTL .  Is that expected?
Not at all! Looks like a copy&paste error while modifying the original patch. Thanks for catching this.

Also the whole set is so far only compile tested, still need to give it a run on all hardware generations anyway.

Christian.

Am 30.01.2017 um 14:59 schrieb StDenis, Tom:

Minor nit: the comment says v8 [??]


The changes to the GFX6/7/8 look reasonable though only question is you read from mmVM_PRT_CNTL and then write to mmVM_CONTEXT1_CNTL .  Is that expected?


Tom


________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org><mailto:amd-gfx-bounces@lists.freedesktop.org> on behalf of Christian König <deathsimple@vodafone.de><mailto:deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
Sent: Monday, January 30, 2017 07:57
To: bas-dldO88ZXqoXqqjsSq9zF6IRWq/SkRNHw@public.gmane.org<mailto:bas-dldO88ZXqoXqqjsSq9zF6IRWq/SkRNHw@public.gmane.org>; airlied@gmail.com<mailto:airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Subject: [PATCH 4/6] drm/amdgpu: implement PRT for GFX6

From: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org><mailto:christian.koenig@amd.com>

Enable/disable the handling globally for now and
print a warning when we enable it for the first time.

Signed-off-by: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org><mailto:christian.koenig-5C7GfCeVMHo@public.gmane.org>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 63 +++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index e2b0b16..c23503e 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -398,6 +398,68 @@ static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,
         WREG32(mmVM_CONTEXT1_CNTL, tmp);
 }

+ /**
+   + * gmc_v8_0_set_prt - set PRT VM fault
+   + *
+   + * @adev: amdgpu_device pointer
+   + * @enable: enable/disable VM fault handling for PRT
+   +*/
+static void gmc_v6_0_set_prt(struct amdgpu_device *adev, bool enable)
+{
+       u32 tmp;
+
+       if (enable && !adev->mc.prt_warning) {
+               dev_warn(adev->dev, "Disabling VM faults because of PRT request!\n");
+               adev->mc.prt_warning = true;
+       }
+
+       tmp = RREG32(mmVM_PRT_CNTL);
+       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+                           CB_DISABLE_READ_FAULT_ON_UNMAPPED_ACCESS,
+                           enable);
+       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+                           CB_DISABLE_WRITE_FAULT_ON_UNMAPPED_ACCESS,
+                           enable);
+       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+                           TC_DISABLE_READ_FAULT_ON_UNMAPPED_ACCESS,
+                           enable);
+       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+                           TC_DISABLE_WRITE_FAULT_ON_UNMAPPED_ACCESS,
+                           enable);
+       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+                           L2_CACHE_STORE_INVALID_ENTRIES,
+                           enable);
+       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+                           L1_TLB_STORE_INVALID_ENTRIES,
+                           enable);
+       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
+                           MASK_PDE0_FAULT, enable);
+       WREG32(mmVM_CONTEXT1_CNTL, tmp);
+
+       if (enable) {
+               uint32_t low = AMDGPU_VA_RESERVED_SIZE >> AMDGPU_GPU_PAGE_SHIFT;
+               uint32_t high = adev->vm_manager.max_pfn;
+
+               WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, low);
+               WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, low);
+               WREG32(mmVM_PRT_APERTURE2_LOW_ADDR, low);
+               WREG32(mmVM_PRT_APERTURE3_LOW_ADDR, low);
+               WREG32(mmVM_PRT_APERTURE0_HIGH_ADDR, high);
+               WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
+               WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
+               WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
+       } else {
+               WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
+               WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);
+               WREG32(mmVM_PRT_APERTURE2_LOW_ADDR, 0xfffffff);
+               WREG32(mmVM_PRT_APERTURE3_LOW_ADDR, 0xfffffff);
+               WREG32(mmVM_PRT_APERTURE0_HIGH_ADDR, 0x0);
+               WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
+               WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
+               WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
+       }
+}
+
 static int gmc_v6_0_gart_enable(struct amdgpu_device *adev)
 {
         int r, i;
@@ -1080,6 +1142,7 @@ static const struct amd_ip_funcs gmc_v6_0_ip_funcs = {
 static const struct amdgpu_gart_funcs gmc_v6_0_gart_funcs = {
         .flush_gpu_tlb = gmc_v6_0_gart_flush_gpu_tlb,
         .set_pte_pde = gmc_v6_0_gart_set_pte_pde,
+       .set_prt = gmc_v6_0_set_prt,
 };

 static const struct amdgpu_irq_src_funcs gmc_v6_0_irq_funcs = {
--
2.5.0

_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
amd-gfx Info Page - lists.freedesktop.org<https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
lists.freedesktop.org
To see the collection of prior postings to the list, visit the amd-gfx Archives. Using amd-gfx: To post a message to all the list members, send email ...





[-- Attachment #1.1.2: Type: text/html, Size: 14578 bytes --]

[-- Attachment #1.2: ATT00001.png --]
[-- Type: image/png, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [PATCH 4/6] drm/amdgpu: implement PRT for GFX6

[Christian König]

[-- Attachment #1.1: Type: text/plain, Size: 6275 bytes --]

Thanks, going to come back to that when I actually write the unit test.

But I will wait what Dave&Bas says to the interface.

Regards,
Christian.

Am 30.01.2017 um 15:15 schrieb StDenis, Tom:
>
> Hi Christian,
>
>
> I have SI,CI,VI gear in my office if you have a unit test to try it with.
>
>
> Cheers,
>
> Tom
>
>
>
> ------------------------------------------------------------------------
> *From:* Christian König <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
> *Sent:* Monday, January 30, 2017 09:14
> *To:* StDenis, Tom; bas-dldO88ZXqoXqqjsSq9zF6IRWq/SkRNHw@public.gmane.org; airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
> *Cc:* amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> *Subject:* Re: [PATCH 4/6] drm/amdgpu: implement PRT for GFX6
>> The changes to the GFX6/7/8 look reasonable though only question is 
>> you read from mmVM_PRT_CNTL and then write to mmVM_CONTEXT1_CNTL . 
>>  Is that expected?
> Not at all! Looks like a copy&paste error while modifying the original 
> patch. Thanks for catching this.
>
> Also the whole set is so far only compile tested, still need to give 
> it a run on all hardware generations anyway.
>
> Christian.
>
> Am 30.01.2017 um 14:59 schrieb StDenis, Tom:
>>
>> Minor nit: the comment says v8 ��
>>
>>
>> The changes to the GFX6/7/8 look reasonable though only question is 
>> you read from mmVM_PRT_CNTL and then write to mmVM_CONTEXT1_CNTL . 
>>  Is that expected?
>>
>>
>> Tom
>>
>>
>>
>> ------------------------------------------------------------------------
>> *From:* amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of 
>> Christian König <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
>> *Sent:* Monday, January 30, 2017 07:57
>> *To:* bas-dldO88ZXqoXqqjsSq9zF6IRWq/SkRNHw@public.gmane.org; airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
>> *Cc:* amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>> *Subject:* [PATCH 4/6] drm/amdgpu: implement PRT for GFX6
>> From: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org>
>>
>> Enable/disable the handling globally for now and
>> print a warning when we enable it for the first time.
>>
>> Signed-off-by: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 63 
>> +++++++++++++++++++++++++++++++++++
>>  1 file changed, 63 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> index e2b0b16..c23503e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> @@ -398,6 +398,68 @@ static void 
>> gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,
>>          WREG32(mmVM_CONTEXT1_CNTL, tmp);
>>  }
>>
>> + /**
>> +   + * gmc_v8_0_set_prt - set PRT VM fault
>> +   + *
>> +   + * @adev: amdgpu_device pointer
>> +   + * @enable: enable/disable VM fault handling for PRT
>> +   +*/
>> +static void gmc_v6_0_set_prt(struct amdgpu_device *adev, bool enable)
>> +{
>> +       u32 tmp;
>> +
>> +       if (enable && !adev->mc.prt_warning) {
>> +               dev_warn(adev->dev, "Disabling VM faults because of 
>> PRT request!\n");
>> +               adev->mc.prt_warning = true;
>> +       }
>> +
>> +       tmp = RREG32(mmVM_PRT_CNTL);
>> +       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
>> + CB_DISABLE_READ_FAULT_ON_UNMAPPED_ACCESS,
>> +                           enable);
>> +       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
>> + CB_DISABLE_WRITE_FAULT_ON_UNMAPPED_ACCESS,
>> +                           enable);
>> +       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
>> + TC_DISABLE_READ_FAULT_ON_UNMAPPED_ACCESS,
>> +                           enable);
>> +       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
>> + TC_DISABLE_WRITE_FAULT_ON_UNMAPPED_ACCESS,
>> +                           enable);
>> +       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
>> + L2_CACHE_STORE_INVALID_ENTRIES,
>> +                           enable);
>> +       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
>> + L1_TLB_STORE_INVALID_ENTRIES,
>> +                           enable);
>> +       tmp = REG_SET_FIELD(tmp, VM_PRT_CNTL,
>> +                           MASK_PDE0_FAULT, enable);
>> +       WREG32(mmVM_CONTEXT1_CNTL, tmp);
>> +
>> +       if (enable) {
>> +               uint32_t low = AMDGPU_VA_RESERVED_SIZE >> 
>> AMDGPU_GPU_PAGE_SHIFT;
>> +               uint32_t high = adev->vm_manager.max_pfn;
>> +
>> + WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, low);
>> + WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, low);
>> + WREG32(mmVM_PRT_APERTURE2_LOW_ADDR, low);
>> + WREG32(mmVM_PRT_APERTURE3_LOW_ADDR, low);
>> + WREG32(mmVM_PRT_APERTURE0_HIGH_ADDR, high);
>> + WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
>> + WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
>> + WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
>> +       } else {
>> + WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
>> + WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);
>> + WREG32(mmVM_PRT_APERTURE2_LOW_ADDR, 0xfffffff);
>> + WREG32(mmVM_PRT_APERTURE3_LOW_ADDR, 0xfffffff);
>> + WREG32(mmVM_PRT_APERTURE0_HIGH_ADDR, 0x0);
>> + WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
>> + WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
>> + WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
>> +       }
>> +}
>> +
>>  static int gmc_v6_0_gart_enable(struct amdgpu_device *adev)
>>  {
>>          int r, i;
>> @@ -1080,6 +1142,7 @@ static const struct amd_ip_funcs 
>> gmc_v6_0_ip_funcs = {
>>  static const struct amdgpu_gart_funcs gmc_v6_0_gart_funcs = {
>>          .flush_gpu_tlb = gmc_v6_0_gart_flush_gpu_tlb,
>>          .set_pte_pde = gmc_v6_0_gart_set_pte_pde,
>> +       .set_prt = gmc_v6_0_set_prt,
>>  };
>>
>>  static const struct amdgpu_irq_src_funcs gmc_v6_0_irq_funcs = {
>> -- 
>> 2.5.0
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> amd-gfx Info Page - lists.freedesktop.org 
>> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
>> lists.freedesktop.org
>> To see the collection of prior postings to the list, visit the 
>> amd-gfx Archives. Using amd-gfx: To post a message to all the list 
>> members, send email ...
>>
>>
>>
>


[-- Attachment #1.2.1: Type: text/html, Size: 19409 bytes --]

[-- Attachment #1.2.2: Type: image/png, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [PATCH 1/6] drm/amdgpu: add support for BO_VAs without BO

[Nicolai Hähnle]

On 30.01.2017 13:57, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
>
> For PRT support we need mappings which aren't backed by any memory.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 8e6030d..87eae9b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1117,7 +1117,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>  	struct fence *exclusive;
>  	int r;
>
> -	if (clear) {
> +	if (clear || !bo_va->bo) {
>  		mem = NULL;
>  		nodes = NULL;
>  		exclusive = NULL;
> @@ -1134,9 +1134,15 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>  		exclusive = reservation_object_get_excl(bo_va->bo->tbo.resv);
>  	}
>
> -	flags = amdgpu_ttm_tt_pte_flags(adev, bo_va->bo->tbo.ttm, mem);
> -	gtt_flags = (amdgpu_ttm_is_bound(bo_va->bo->tbo.ttm) &&
> -		adev == amdgpu_ttm_adev(bo_va->bo->tbo.bdev)) ? flags : 0;
> +	if (bo_va->bo) {
> +		flags = amdgpu_ttm_tt_pte_flags(adev, bo_va->bo->tbo.ttm, mem);
> +		gtt_flags = (amdgpu_ttm_is_bound(bo_va->bo->tbo.ttm) &&
> +			adev == amdgpu_ttm_adev(bo_va->bo->tbo.bdev)) ?
> +			flags : 0;
> +	} else {
> +		flags = 0x0;
> +		gtt_flags = ~0x0;
> +	}
>
>  	spin_lock(&vm->status_lock);
>  	if (!list_empty(&bo_va->vm_status))
> @@ -1271,7 +1277,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
>  	INIT_LIST_HEAD(&bo_va->invalids);
>  	INIT_LIST_HEAD(&bo_va->vm_status);
>
> -	list_add_tail(&bo_va->bo_list, &bo->va);
> +	if (bo)
> +		list_add_tail(&bo_va->bo_list, &bo->va);
>
>  	return bo_va;
>  }
> @@ -1309,7 +1316,8 @@ int amdgpu_vm_bo_map(struct amdgpu_device *adev,
>
>  	/* make sure object fit at this offset */
>  	eaddr = saddr + size - 1;
> -	if ((saddr >= eaddr) || (offset + size > amdgpu_bo_size(bo_va->bo)))
> +	if (bo_va->bo && (saddr >= eaddr ||
> +			  (offset + size > amdgpu_bo_size(bo_va->bo))))
>  		return -EINVAL;

At least the saddr >= eaddr check should probably apply apply.

Come to think of it, what if offset + size wraps around? There should 
probably be an explicit check for that.

Cheers,
Nicolai

>
>  	last_pfn = eaddr / AMDGPU_GPU_PAGE_SIZE;
>

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

Re: PRT support for amdgpu

[Nicolai Hähnle]

[ Cc John for addrlib ]

On 30.01.2017 13:57, Christian König wrote:
> An open problem with the proposal is that we don't know when or if we want to add the userspace implementation into radeonsi.
>
> So price question could you guys use this for radv as well? Or is it sufficient to just write an unit test for it?

For radeonsi, I think the question is more "when", not "if". At least 
PRT/sparse textures are necessarily immutable, so the associated Mesa 
headache should be bounded, but still...

As you know, I think the ioctl interface is good.

One thing to keep in mind is that I don't know how well PRT layout is 
supported by the open addrlib. There have been a bunch of PRT-related 
fixes in the internal copy of addrlib, but opening them has been stuck 
in review limbo for a while now...

Cheers,
Nicolai

>
> Best regards,
> Christian.
>
> _______________________________________________
> 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: PRT support for amdgpu

[Bas Nieuwenhuizen]

On Mon, Jan 30, 2017, at 13:57, Christian König wrote:
> Hi Dave and Bas,
> 
> Hi Dave and Bas,
> 
> the following set of patches is a proposal for adding support for partial
> resident textures (PRT) to the amdgpu kernel module.
> 
> The basic idea behind PRT support is that you turn of VM fault reporting
> and only map parts of a texture into your virtual address space.

If we add some backing to a range, do we need to unmap the PRT range,
split and map two PRt ranges? Or will this be handled like mmap and a
new map just overrides the earlier maps for that range?

> 
> When a shader now tries to sample from a not present page it gets a
> notification instead of a VM fault and can react gracefully by switch to
> a different LOD for example.

So to confirm this is just using texture instruction with the TFE bit
enabled, no trap handlers or such?

> 
> On our current available hardware generation you can unfortunately only
> turn of VM faults globally, but on future generation you can do this on a
> per page basis. So my proposal is to have a consistent interface over all
> generations with a per mapping PRT flag, but enable/disable it globally
> on current hardware when the first/last mapping is made/destroyed.
> 
> An open problem with the proposal is that we don't know when or if we
> want to add the userspace implementation into radeonsi.
> 
> So price question could you guys use this for radv as well? Or is it
> sufficient to just write an unit test for it?

So this API seems usable, and I think this is something we can use for
radv. However, I'm not sure how much time it takes for us to implement,
as the TFE variants are not in LLVM yet and I haven't figured out what
values the NACKs get. 

Furthermore, if addrlib is missing stuff like Nicolai suggests then that
could result in complications. I can try if I can get something working
over the weekend, but no promises.

As far as an unit test being sufficient, I assume you mean as open
source user for inclusion into the kernel? I think that'd be a question
answered better by Dave.

> 
> Best regards,
> Christian.
> 
> _______________________________________________
> 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: PRT support for amdgpu

[Alex Deucher]

On Mon, Jan 30, 2017 at 7:57 AM, Christian König
<deathsimple@vodafone.de> wrote:
> Hi Dave and Bas,
>
> Hi Dave and Bas,
>
> the following set of patches is a proposal for adding support for partial resident textures (PRT) to the amdgpu kernel module.
>
> The basic idea behind PRT support is that you turn of VM fault reporting and only map parts of a texture into your virtual address space.
>
> When a shader now tries to sample from a not present page it gets a notification instead of a VM fault and can react gracefully by switch to a different LOD for example.

Do we actually need to disable faults?  I guess the shader hw probably
requires it to get the proper result in the shader?

Alex

>
> On our current available hardware generation you can unfortunately only turn of VM faults globally, but on future generation you can do this on a per page basis. So my proposal is to have a consistent interface over all generations with a per mapping PRT flag, but enable/disable it globally on current hardware when the first/last mapping is made/destroyed.
>
> An open problem with the proposal is that we don't know when or if we want to add the userspace implementation into radeonsi.
>
> So price question could you guys use this for radv as well? Or is it sufficient to just write an unit test for it?
>
> Best regards,
> Christian.
>
> _______________________________________________
> 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: PRT support for amdgpu

[Christian König]

Am 31.01.2017 um 14:06 schrieb Bas Nieuwenhuizen:
>
> On Mon, Jan 30, 2017, at 13:57, Christian König wrote:
>> Hi Dave and Bas,
>>
>> Hi Dave and Bas,
>>
>> the following set of patches is a proposal for adding support for partial
>> resident textures (PRT) to the amdgpu kernel module.
>>
>> The basic idea behind PRT support is that you turn of VM fault reporting
>> and only map parts of a texture into your virtual address space.
> If we add some backing to a range, do we need to unmap the PRT range,
> split and map two PRt ranges? Or will this be handled like mmap and a
> new map just overrides the earlier maps for that range?

Currently the idea is to unmap the PRT range first and then map the new 
stuff. But I've already discussed internally with Nicolai a couple of 
alternatives.

The problem is that IOCTL are supposed to be transactional, e.g. they 
either fail completely or they success completely. But that is rather 
tricky when you need to split mappings like you suggested as well.

So at least for the initial implementation I would like to stick to 
manual unmap calls we can still add the ability to split mappings later 
on if we find performance problems with that approach

>> When a shader now tries to sample from a not present page it gets a
>> notification instead of a VM fault and can react gracefully by switch to
>> a different LOD for example.
> So to confirm this is just using texture instruction with the TFE bit
> enabled, no trap handlers or such?

I'm not so deeply into the shader instructions, but I think so yes.

>> On our current available hardware generation you can unfortunately only
>> turn of VM faults globally, but on future generation you can do this on a
>> per page basis. So my proposal is to have a consistent interface over all
>> generations with a per mapping PRT flag, but enable/disable it globally
>> on current hardware when the first/last mapping is made/destroyed.
>>
>> An open problem with the proposal is that we don't know when or if we
>> want to add the userspace implementation into radeonsi.
>>
>> So price question could you guys use this for radv as well? Or is it
>> sufficient to just write an unit test for it?
> So this API seems usable, and I think this is something we can use for
> radv. However, I'm not sure how much time it takes for us to implement,
> as the TFE variants are not in LLVM yet and I haven't figured out what
> values the NACKs get.

Actually this is also useful without the special NACK handling. E.g. 
when you sample from a texture part which isn't present you always get 
zero and writes are ignored.

The TFE bit and the extra signaling to for special handling in shader 
code are only optional if I'm not completely mistaken.

> Furthermore, if addrlib is missing stuff like Nicolai suggests then that
> could result in complications. I can try if I can get something working
> over the weekend, but no promises.

Not sure what concern Nicolai has about addrlib here. In general we 
should know where the different parts of a texture start (LODs, layers 
etc...) and as far as I can see that's all you need to know.

> As far as an unit test being sufficient, I assume you mean as open
> source user for inclusion into the kernel?

Yes.

> I think that'd be a question
> answered better by Dave.

Yeah, though so as well. Dave can you comment?

Thanks for the comments,
Christian.

>
>> Best regards,
>> Christian.
>>
>> _______________________________________________
>> 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


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

Re: PRT support for amdgpu

[Christian König]

Am 31.01.2017 um 17:09 schrieb Alex Deucher:
> On Mon, Jan 30, 2017 at 7:57 AM, Christian König
> <deathsimple@vodafone.de> wrote:
>> Hi Dave and Bas,
>>
>> Hi Dave and Bas,
>>
>> the following set of patches is a proposal for adding support for partial resident textures (PRT) to the amdgpu kernel module.
>>
>> The basic idea behind PRT support is that you turn of VM fault reporting and only map parts of a texture into your virtual address space.
>>
>> When a shader now tries to sample from a not present page it gets a notification instead of a VM fault and can react gracefully by switch to a different LOD for example.
> Do we actually need to disable faults?  I guess the shader hw probably
> requires it to get the proper result in the shader?

Yes, you need to disable the VM faults for the PRT range otherwise the 
interrupt ring will just be flooded with faults.

Keep in mind that faults (or rather NACKs from the MC) are considered 
normal with that approach. This is also the reason why we need to change 
the TLB rules and also keep invalid entries in there.

Regards,
Christian.

>
> Alex
>
>> On our current available hardware generation you can unfortunately only turn of VM faults globally, but on future generation you can do this on a per page basis. So my proposal is to have a consistent interface over all generations with a per mapping PRT flag, but enable/disable it globally on current hardware when the first/last mapping is made/destroyed.
>>
>> An open problem with the proposal is that we don't know when or if we want to add the userspace implementation into radeonsi.
>>
>> So price question could you guys use this for radv as well? Or is it sufficient to just write an unit test for it?
>>
>> Best regards,
>> Christian.
>>
>> _______________________________________________
>> 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


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

Re: PRT support for amdgpu

[Nicolai Hähnle]

On 31.01.2017 17:28, Christian König wrote:
> Am 31.01.2017 um 14:06 schrieb Bas Nieuwenhuizen:
>> So this API seems usable, and I think this is something we can use for
>> radv. However, I'm not sure how much time it takes for us to implement,
>> as the TFE variants are not in LLVM yet and I haven't figured out what
>> values the NACKs get.
>
> Actually this is also useful without the special NACK handling. E.g.
> when you sample from a texture part which isn't present you always get
> zero and writes are ignored.
>
> The TFE bit and the extra signaling to for special handling in shader
> code are only optional if I'm not completely mistaken.

I think it's needed for ARB_sparse_texture2 / whatever the Vulkan 
equivalent of that is. But yeah, I don't think ARB_sparse_texture needs 
TFE support in LLVM.


>> Furthermore, if addrlib is missing stuff like Nicolai suggests then that
>> could result in complications. I can try if I can get something working
>> over the weekend, but no promises.
>
> Not sure what concern Nicolai has about addrlib here. In general we
> should know where the different parts of a texture start (LODs, layers
> etc...) and as far as I can see that's all you need to know.

Well, you're right that it's probably possible to work with the open 
addrlib already.

In particular, you need address info not just for layers and levels, but 
also for blocks within each 2D image, to be able to compute 
VIRTUAL_PAGE_SIZE_X/Y, and to map them to the corresponding addresses.

There are AddrComputeSurfaceAddrFromCoord and 
AddrComputeSurfaceCoordFromAddr functions that can be tricked into 
providing the necessary info.

It may be more natural with some additional functions that aren't open yet.

Also, you might possibly want different tilings for sparse textures, but 
I don't think that's really needed for an initial implementation.

Cheers,
Nicolai


>> As far as an unit test being sufficient, I assume you mean as open
>> source user for inclusion into the kernel?
>
> Yes.
>
>> I think that'd be a question
>> answered better by Dave.
>
> Yeah, though so as well. Dave can you comment?
>
> Thanks for the comments,
> Christian.
>
>>
>>> Best regards,
>>> Christian.
>>>
>>> _______________________________________________
>>> 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
>
>
> _______________________________________________
> 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: PRT support for amdgpu

[Dave Airlie]

>> answered better by Dave.
>
>
> Yeah, though so as well. Dave can you comment?

I think we would require a fully open source user for this sort of thing,
there are way to many corner cases for us to fall down here, prematurely
pushing the API without a proven test suite running on it would be bad.

We'd want to get radeonsi or radv up and running and have a complete
run of the conformance suite to make sure the kernel API was sane,
design is good, proving the design is the hard bit.

And uggh on addrlib, just stick the whole thing on github already.

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

Re: PRT support for amdgpu

[Nicolai Hähnle]

[ ceterum censeo, + John for addrlib :P ]

On 02.02.2017 02:49, Dave Airlie wrote:
>>> answered better by Dave.
>>
>>
>> Yeah, though so as well. Dave can you comment?
>
> I think we would require a fully open source user for this sort of thing,
> there are way to many corner cases for us to fall down here, prematurely
> pushing the API without a proven test suite running on it would be bad.
>
> We'd want to get radeonsi or radv up and running and have a complete
> run of the conformance suite to make sure the kernel API was sane,
> design is good, proving the design is the hard bit.

I think we can start with just GL_ARB_sparse_buffer. That extension 
isn't as useful in comparison, but it exercises all the memory 
management bits without having to worry about texture layout 
considerations, and doing that one first seems like a reasonable 
development strategy anyway.


> And uggh on addrlib, just stick the whole thing on github already.

I wish :/

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

Re: PRT support for amdgpu

[Bas Nieuwenhuizen]

On Thu, Feb 2, 2017, at 10:18, Nicolai Hähnle wrote:
> [ ceterum censeo, + John for addrlib :P ]
> 
> On 02.02.2017 02:49, Dave Airlie wrote:
> >>> answered better by Dave.
> >>
> >>
> >> Yeah, though so as well. Dave can you comment?
> >
> > I think we would require a fully open source user for this sort of thing,
> > there are way to many corner cases for us to fall down here, prematurely
> > pushing the API without a proven test suite running on it would be bad.
> >
> > We'd want to get radeonsi or radv up and running and have a complete
> > run of the conformance suite to make sure the kernel API was sane,
> > design is good, proving the design is the hard bit.
> 
> I think we can start with just GL_ARB_sparse_buffer. That extension 
> isn't as useful in comparison, but it exercises all the memory 
> management bits without having to worry about texture layout 
> considerations, and doing that one first seems like a reasonable 
> development strategy anyway.

Yeah, I noticed that vulkan had a similar extension that can be done
pretty easily, trying to get that done before the weekend.

What would be a plan for upstreaming all this? In an earlier case (the
wait on multiple fences ioctl), AFAIU the problem for upstreaming was
that there was no open-source user.  However I then wrote a branch
(which is probably outdated by now ...), but would like to wait with
upstreaming it till I know which libdrm and kernel driver version to use
for the feature tests. As a result all three the parts (kernel, libdrm,
radv) haven't been upstreamed yet. How can we avoid having the same
problem with this feature?

> 
> 
> > And uggh on addrlib, just stick the whole thing on github already.
> 
> I wish :/
> 
> Nicolai
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: PRT support for amdgpu

[Nicolai Hähnle]

On 02.02.2017 10:29, Bas Nieuwenhuizen wrote:
> On Thu, Feb 2, 2017, at 10:18, Nicolai Hähnle wrote:
>> On 02.02.2017 02:49, Dave Airlie wrote:
>>> I think we would require a fully open source user for this sort of thing,
>>> there are way to many corner cases for us to fall down here, prematurely
>>> pushing the API without a proven test suite running on it would be bad.
>>>
>>> We'd want to get radeonsi or radv up and running and have a complete
>>> run of the conformance suite to make sure the kernel API was sane,
>>> design is good, proving the design is the hard bit.
>>
>> I think we can start with just GL_ARB_sparse_buffer. That extension
>> isn't as useful in comparison, but it exercises all the memory
>> management bits without having to worry about texture layout
>> considerations, and doing that one first seems like a reasonable
>> development strategy anyway.
>
> Yeah, I noticed that vulkan had a similar extension that can be done
> pretty easily, trying to get that done before the weekend.

That would be cool. I thought of doing this for radeonsi, but I 
seriously doubt that I'll get to it any time soon.


> What would be a plan for upstreaming all this? In an earlier case (the
> wait on multiple fences ioctl), AFAIU the problem for upstreaming was
> that there was no open-source user.  However I then wrote a branch
> (which is probably outdated by now ...), but would like to wait with
> upstreaming it till I know which libdrm and kernel driver version to use
> for the feature tests. As a result all three the parts (kernel, libdrm,
> radv) haven't been upstreamed yet. How can we avoid having the same
> problem with this feature?

Once all the parts are there, the sequence should be upstream kernel, 
upstream libdrm, upstream radv.

Maybe you can ping the corresponding threads or re-send the patches for 
review?

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

Re: PRT support for amdgpu

[Christian König]

Am 02.02.2017 um 10:33 schrieb Nicolai Hähnle:
> On 02.02.2017 10:29, Bas Nieuwenhuizen wrote:
>> On Thu, Feb 2, 2017, at 10:18, Nicolai Hähnle wrote:
>>> On 02.02.2017 02:49, Dave Airlie wrote:
>>>> I think we would require a fully open source user for this sort of 
>>>> thing,
>>>> there are way to many corner cases for us to fall down here, 
>>>> prematurely
>>>> pushing the API without a proven test suite running on it would be 
>>>> bad.
>>>>
>>>> We'd want to get radeonsi or radv up and running and have a complete
>>>> run of the conformance suite to make sure the kernel API was sane,
>>>> design is good, proving the design is the hard bit.
>>>
>>> I think we can start with just GL_ARB_sparse_buffer. That extension
>>> isn't as useful in comparison, but it exercises all the memory
>>> management bits without having to worry about texture layout
>>> considerations, and doing that one first seems like a reasonable
>>> development strategy anyway.
>>
>> Yeah, I noticed that vulkan had a similar extension that can be done
>> pretty easily, trying to get that done before the weekend.
>
> That would be cool. I thought of doing this for radeonsi, but I 
> seriously doubt that I'll get to it any time soon.

Thanks, that would be great. I'm going to send out my latest bug fixes 
for the kernel side in a few minutes.

>
>
>> What would be a plan for upstreaming all this? In an earlier case (the
>> wait on multiple fences ioctl), AFAIU the problem for upstreaming was
>> that there was no open-source user.  However I then wrote a branch
>> (which is probably outdated by now ...), but would like to wait with
>> upstreaming it till I know which libdrm and kernel driver version to use
>> for the feature tests. As a result all three the parts (kernel, libdrm,
>> radv) haven't been upstreamed yet. How can we avoid having the same
>> problem with this feature?
>
> Once all the parts are there, the sequence should be upstream kernel, 
> upstream libdrm, upstream radv.
>
> Maybe you can ping the corresponding threads or re-send the patches 
> for review?

Alex did that internally just last week, but nobody volunteered to take 
care of pushing it.

Basically everybody is just to busy to look into it.

Regards,
Christian.

>
> Nicolai


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

Re: [PATCH 1/6] drm/amdgpu: add support for BO_VAs without BO

[Christian König]

Am 30.01.2017 um 15:43 schrieb Nicolai Hähnle:
> On 30.01.2017 13:57, Christian König wrote:
>> From: Christian König <christian.koenig@amd.com>
>>
>> For PRT support we need mappings which aren't backed by any memory.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 20 ++++++++++++++------
>>  1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 8e6030d..87eae9b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1117,7 +1117,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device 
>> *adev,
>>      struct fence *exclusive;
>>      int r;
>>
>> -    if (clear) {
>> +    if (clear || !bo_va->bo) {
>>          mem = NULL;
>>          nodes = NULL;
>>          exclusive = NULL;
>> @@ -1134,9 +1134,15 @@ int amdgpu_vm_bo_update(struct amdgpu_device 
>> *adev,
>>          exclusive = reservation_object_get_excl(bo_va->bo->tbo.resv);
>>      }
>>
>> -    flags = amdgpu_ttm_tt_pte_flags(adev, bo_va->bo->tbo.ttm, mem);
>> -    gtt_flags = (amdgpu_ttm_is_bound(bo_va->bo->tbo.ttm) &&
>> -        adev == amdgpu_ttm_adev(bo_va->bo->tbo.bdev)) ? flags : 0;
>> +    if (bo_va->bo) {
>> +        flags = amdgpu_ttm_tt_pte_flags(adev, bo_va->bo->tbo.ttm, mem);
>> +        gtt_flags = (amdgpu_ttm_is_bound(bo_va->bo->tbo.ttm) &&
>> +            adev == amdgpu_ttm_adev(bo_va->bo->tbo.bdev)) ?
>> +            flags : 0;
>> +    } else {
>> +        flags = 0x0;
>> +        gtt_flags = ~0x0;
>> +    }
>>
>>      spin_lock(&vm->status_lock);
>>      if (!list_empty(&bo_va->vm_status))
>> @@ -1271,7 +1277,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct 
>> amdgpu_device *adev,
>>      INIT_LIST_HEAD(&bo_va->invalids);
>>      INIT_LIST_HEAD(&bo_va->vm_status);
>>
>> -    list_add_tail(&bo_va->bo_list, &bo->va);
>> +    if (bo)
>> +        list_add_tail(&bo_va->bo_list, &bo->va);
>>
>>      return bo_va;
>>  }
>> @@ -1309,7 +1316,8 @@ int amdgpu_vm_bo_map(struct amdgpu_device *adev,
>>
>>      /* make sure object fit at this offset */
>>      eaddr = saddr + size - 1;
>> -    if ((saddr >= eaddr) || (offset + size > 
>> amdgpu_bo_size(bo_va->bo)))
>> +    if (bo_va->bo && (saddr >= eaddr ||
>> +              (offset + size > amdgpu_bo_size(bo_va->bo))))
>>          return -EINVAL;
>
> At least the saddr >= eaddr check should probably apply apply.

Oh, yes of course.

>
> Come to think of it, what if offset + size wraps around?

Well that is exactly what the saddr >= eaddr check is good for :)

Christian.

> There should probably be an explicit check for that.
>
> Cheers,
> Nicolai
>
>>
>>      last_pfn = eaddr / AMDGPU_GPU_PAGE_SIZE;
>>
>

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

Re: [PATCH 1/6] drm/amdgpu: add support for BO_VAs without BO

[Nicolai Hähnle]

On 02.02.2017 11:26, Christian König wrote:
> Am 30.01.2017 um 15:43 schrieb Nicolai Hähnle:
>> On 30.01.2017 13:57, Christian König wrote:
>>> From: Christian König <christian.koenig@amd.com>
>>>
>>> For PRT support we need mappings which aren't backed by any memory.
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> ---
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 20 ++++++++++++++------
>>>  1 file changed, 14 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index 8e6030d..87eae9b 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -1117,7 +1117,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device
>>> *adev,
>>>      struct fence *exclusive;
>>>      int r;
>>>
>>> -    if (clear) {
>>> +    if (clear || !bo_va->bo) {
>>>          mem = NULL;
>>>          nodes = NULL;
>>>          exclusive = NULL;
>>> @@ -1134,9 +1134,15 @@ int amdgpu_vm_bo_update(struct amdgpu_device
>>> *adev,
>>>          exclusive = reservation_object_get_excl(bo_va->bo->tbo.resv);
>>>      }
>>>
>>> -    flags = amdgpu_ttm_tt_pte_flags(adev, bo_va->bo->tbo.ttm, mem);
>>> -    gtt_flags = (amdgpu_ttm_is_bound(bo_va->bo->tbo.ttm) &&
>>> -        adev == amdgpu_ttm_adev(bo_va->bo->tbo.bdev)) ? flags : 0;
>>> +    if (bo_va->bo) {
>>> +        flags = amdgpu_ttm_tt_pte_flags(adev, bo_va->bo->tbo.ttm, mem);
>>> +        gtt_flags = (amdgpu_ttm_is_bound(bo_va->bo->tbo.ttm) &&
>>> +            adev == amdgpu_ttm_adev(bo_va->bo->tbo.bdev)) ?
>>> +            flags : 0;
>>> +    } else {
>>> +        flags = 0x0;
>>> +        gtt_flags = ~0x0;
>>> +    }
>>>
>>>      spin_lock(&vm->status_lock);
>>>      if (!list_empty(&bo_va->vm_status))
>>> @@ -1271,7 +1277,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct
>>> amdgpu_device *adev,
>>>      INIT_LIST_HEAD(&bo_va->invalids);
>>>      INIT_LIST_HEAD(&bo_va->vm_status);
>>>
>>> -    list_add_tail(&bo_va->bo_list, &bo->va);
>>> +    if (bo)
>>> +        list_add_tail(&bo_va->bo_list, &bo->va);
>>>
>>>      return bo_va;
>>>  }
>>> @@ -1309,7 +1316,8 @@ int amdgpu_vm_bo_map(struct amdgpu_device *adev,
>>>
>>>      /* make sure object fit at this offset */
>>>      eaddr = saddr + size - 1;
>>> -    if ((saddr >= eaddr) || (offset + size >
>>> amdgpu_bo_size(bo_va->bo)))
>>> +    if (bo_va->bo && (saddr >= eaddr ||
>>> +              (offset + size > amdgpu_bo_size(bo_va->bo))))
>>>          return -EINVAL;
>>
>> At least the saddr >= eaddr check should probably apply apply.
>
> Oh, yes of course.
>
>>
>> Come to think of it, what if offset + size wraps around?
>
> Well that is exactly what the saddr >= eaddr check is good for :)

But eaddr doesn't take offset into account. It seems to me that offset 
can be arbitrarily large and this isn't checked anywhere. At least I 
don't see it.

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