Unmappable VRAM patchset V3

Unmappable VRAM patchset V3

[Jerome Glisse]

Thomas i think i addressed your concern here, the ttm_bo_validate
didn't needed a new argument or i did not understand what was
necessary beside no_wait. In this patchset we check the value
of callback in case of EBUSY (call set_need_resched) or ERESTARTSYS
we return VM_FAULT_NOPAGE.

For the design question of moving the io address determination
into the driver i believe separate aperture is a good example
of where this is needed, i also think nvidia hw can remap dynamicly
part of the aperture such feature can be use with the new interface
this patchset introduce.

Thomas any more concern ?

Thanks for reviewing this :)

Cheers,
Jerome


------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

[PATCH 1/9] drm/ttm: add ttm_fault callback to allow driver to handle bo placement

[Jerome Glisse]

On fault the driver is given the opportunity to perform any operation
it sees fit in order to place the buffer into a CPU visible area of
memory. This patch doesn't break TTM users, nouveau, vmwgfx and radeon
should keep working properly. Future patch will take advantage of this
infrastructure and remove the old path from TTM once driver are
converted.

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c      |    3 +-
 drivers/gpu/drm/ttm/ttm_bo_util.c |   92 +++++++++++++++++--------------------
 drivers/gpu/drm/ttm/ttm_bo_vm.c   |   48 +++++++++----------
 include/drm/ttm/ttm_bo_api.h      |    1 +
 include/drm/ttm/ttm_bo_driver.h   |   30 ++++++++++++
 5 files changed, 97 insertions(+), 77 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index c7320ce..28f3fcf 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1581,7 +1581,8 @@ void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo)
 
 	if (!bdev->dev_mapping)
 		return;
-
+	if (bdev->driver->io_mem_free)
+		bdev->driver->io_mem_free(bdev, &bo->mem);
 	unmap_mapping_range(bdev->dev_mapping, offset, holelen, 1);
 }
 EXPORT_SYMBOL(ttm_bo_unmap_virtual);
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 3f72fe1..10c5fc6 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -84,26 +84,36 @@ int ttm_mem_reg_ioremap(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem,
 			void **virtual)
 {
 	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
-	unsigned long bus_offset;
-	unsigned long bus_size;
-	unsigned long bus_base;
+	struct ttm_bus_placement pl;
 	int ret;
 	void *addr;
 
 	*virtual = NULL;
-	ret = ttm_bo_pci_offset(bdev, mem, &bus_base, &bus_offset, &bus_size);
-	if (ret || bus_size == 0)
-		return ret;
+	if (bdev->driver->io_mem_reserve) {
+		ret = bdev->driver->io_mem_reserve(bdev, mem, &pl);
+		if (unlikely(ret != 0)) {
+			return ret;
+		}
+	} else {
+		ret = ttm_bo_pci_offset(bdev, mem, &pl.base, &pl.offset, &pl.size);
+		if (unlikely(ret != 0) || pl.size == 0) {
+			return ret;
+		}
+		pl.is_iomem = (pl.size != 0);
+	}
 
 	if (!(man->flags & TTM_MEMTYPE_FLAG_NEEDS_IOREMAP))
-		addr = (void *)(((u8 *) man->io_addr) + bus_offset);
+		addr = (void *)(pl.base + pl.offset);
 	else {
 		if (mem->placement & TTM_PL_FLAG_WC)
-			addr = ioremap_wc(bus_base + bus_offset, bus_size);
+			addr = ioremap_wc(pl.base + pl.offset, pl.size);
 		else
-			addr = ioremap_nocache(bus_base + bus_offset, bus_size);
-		if (!addr)
+			addr = ioremap_nocache(pl.base + pl.offset, pl.size);
+		if (!addr) {
+			if (bdev->driver->io_mem_free)
+				bdev->driver->io_mem_free(bdev, mem);
 			return -ENOMEM;
+		}
 	}
 	*virtual = addr;
 	return 0;
@@ -118,6 +128,8 @@ void ttm_mem_reg_iounmap(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem,
 
 	if (virtual && (man->flags & TTM_MEMTYPE_FLAG_NEEDS_IOREMAP))
 		iounmap(virtual);
+	if (bdev->driver->io_mem_free)
+		bdev->driver->io_mem_free(bdev, mem);
 }
 
 static int ttm_copy_io_page(void *dst, void *src, unsigned long page)
@@ -440,13 +452,12 @@ int ttm_bo_kmap(struct ttm_buffer_object *bo,
 		unsigned long start_page, unsigned long num_pages,
 		struct ttm_bo_kmap_obj *map)
 {
+	struct ttm_bus_placement pl;
 	int ret;
-	unsigned long bus_base;
-	unsigned long bus_offset;
-	unsigned long bus_size;
 
 	BUG_ON(!list_empty(&bo->swap));
 	map->virtual = NULL;
+	map->bo = bo;
 	if (num_pages > bo->num_pages)
 		return -EINVAL;
 	if (start_page > bo->num_pages)
@@ -455,16 +466,24 @@ int ttm_bo_kmap(struct ttm_buffer_object *bo,
 	if (num_pages > 1 && !DRM_SUSER(DRM_CURPROC))
 		return -EPERM;
 #endif
-	ret = ttm_bo_pci_offset(bo->bdev, &bo->mem, &bus_base,
-				&bus_offset, &bus_size);
-	if (ret)
-		return ret;
-	if (bus_size == 0) {
+	if (bo->bdev->driver->io_mem_reserve) {
+		ret = bo->bdev->driver->io_mem_reserve(bo->bdev, &bo->mem, &pl);
+		if (unlikely(ret != 0)) {
+			return ret;
+		}
+	} else {
+		ret = ttm_bo_pci_offset(bo->bdev, &bo->mem, &pl.base, &pl.offset, &pl.size);
+		if (unlikely(ret != 0)) {
+			return ret;
+		}
+		pl.is_iomem = (pl.size != 0);
+	}
+	if (!pl.is_iomem) {
 		return ttm_bo_kmap_ttm(bo, start_page, num_pages, map);
 	} else {
-		bus_offset += start_page << PAGE_SHIFT;
-		bus_size = num_pages << PAGE_SHIFT;
-		return ttm_bo_ioremap(bo, bus_base, bus_offset, bus_size, map);
+		pl.offset += start_page << PAGE_SHIFT;
+		pl.size = num_pages << PAGE_SHIFT;
+		return ttm_bo_ioremap(bo, pl.base, pl.offset, pl.size, map);
 	}
 }
 EXPORT_SYMBOL(ttm_bo_kmap);
@@ -476,6 +495,8 @@ void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map)
 	switch (map->bo_kmap_type) {
 	case ttm_bo_map_iomap:
 		iounmap(map->virtual);
+		if (map->bo->bdev->driver->io_mem_free)
+			map->bo->bdev->driver->io_mem_free(map->bo->bdev, &map->bo->mem);
 		break;
 	case ttm_bo_map_vmap:
 		vunmap(map->virtual);
@@ -493,35 +514,6 @@ void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map)
 }
 EXPORT_SYMBOL(ttm_bo_kunmap);
 
-int ttm_bo_pfn_prot(struct ttm_buffer_object *bo,
-		    unsigned long dst_offset,
-		    unsigned long *pfn, pgprot_t *prot)
-{
-	struct ttm_mem_reg *mem = &bo->mem;
-	struct ttm_bo_device *bdev = bo->bdev;
-	unsigned long bus_offset;
-	unsigned long bus_size;
-	unsigned long bus_base;
-	int ret;
-	ret = ttm_bo_pci_offset(bdev, mem, &bus_base, &bus_offset,
-			&bus_size);
-	if (ret)
-		return -EINVAL;
-	if (bus_size != 0)
-		*pfn = (bus_base + bus_offset + dst_offset) >> PAGE_SHIFT;
-	else
-		if (!bo->ttm)
-			return -EINVAL;
-		else
-			*pfn = page_to_pfn(ttm_tt_get_page(bo->ttm,
-							   dst_offset >>
-							   PAGE_SHIFT));
-	*prot = (mem->placement & TTM_PL_FLAG_CACHED) ?
-		PAGE_KERNEL : ttm_io_prot(mem->placement, PAGE_KERNEL);
-
-	return 0;
-}
-
 int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
 			      void *sync_obj,
 			      void *sync_obj_arg,
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 668dbe8..9446b26 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -74,9 +74,7 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	struct ttm_buffer_object *bo = (struct ttm_buffer_object *)
 	    vma->vm_private_data;
 	struct ttm_bo_device *bdev = bo->bdev;
-	unsigned long bus_base;
-	unsigned long bus_offset;
-	unsigned long bus_size;
+	struct ttm_bus_placement pl;
 	unsigned long page_offset;
 	unsigned long page_last;
 	unsigned long pfn;
@@ -84,7 +82,6 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	struct page *page;
 	int ret;
 	int i;
-	bool is_iomem;
 	unsigned long address = (unsigned long)vmf->virtual_address;
 	int retval = VM_FAULT_NOPAGE;
 
@@ -104,11 +101,25 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	if (bdev->driver->fault_reserve_notify)
 		bdev->driver->fault_reserve_notify(bo);
 
+	if (bdev->driver->fault_reserve) {
+		ret = bdev->driver->fault_reserve(bo, &pl);
+		if (unlikely(ret != 0)) {
+			retval = VM_FAULT_SIGBUS;
+			goto out_unlock;
+		}
+	} else {
+		ret = ttm_bo_pci_offset(bdev, &bo->mem, &pl.base, &pl.offset, &pl.size);
+		if (unlikely(ret != 0)) {
+			retval = VM_FAULT_SIGBUS;
+			goto out_unlock;
+		}
+		pl.is_iomem = (pl.size != 0);
+	}
+
 	/*
 	 * Wait for buffer data in transit, due to a pipelined
 	 * move.
 	 */
-
 	spin_lock(&bo->lock);
 	if (test_bit(TTM_BO_PRIV_FLAG_MOVING, &bo->priv_flags)) {
 		ret = ttm_bo_wait(bo, false, true, false);
@@ -122,20 +133,10 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 		spin_unlock(&bo->lock);
 
 
-	ret = ttm_bo_pci_offset(bdev, &bo->mem, &bus_base, &bus_offset,
-				&bus_size);
-	if (unlikely(ret != 0)) {
-		retval = VM_FAULT_SIGBUS;
-		goto out_unlock;
-	}
-
-	is_iomem = (bus_size != 0);
-
 	page_offset = ((address - vma->vm_start) >> PAGE_SHIFT) +
 	    bo->vm_node->start - vma->vm_pgoff;
 	page_last = ((vma->vm_end - vma->vm_start) >> PAGE_SHIFT) +
 	    bo->vm_node->start - vma->vm_pgoff;
-
 	if (unlikely(page_offset >= bo->num_pages)) {
 		retval = VM_FAULT_SIGBUS;
 		goto out_unlock;
@@ -154,8 +155,7 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	 * vma->vm_page_prot when the object changes caching policy, with
 	 * the correct locks held.
 	 */
-
-	if (is_iomem) {
+	if (pl.is_iomem) {
 		vma->vm_page_prot = ttm_io_prot(bo->mem.placement,
 						vma->vm_page_prot);
 	} else {
@@ -169,12 +169,9 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	 * Speculatively prefault a number of pages. Only error on
 	 * first page.
 	 */
-
 	for (i = 0; i < TTM_BO_VM_NUM_PREFAULT; ++i) {
-
-		if (is_iomem)
-			pfn = ((bus_base + bus_offset) >> PAGE_SHIFT) +
-			    page_offset;
+		if (pl.is_iomem)
+			pfn = ((pl.base + pl.offset) >> PAGE_SHIFT) + page_offset;
 		else {
 			page = ttm_tt_get_page(ttm, page_offset);
 			if (unlikely(!page && i == 0)) {
@@ -191,14 +188,12 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 		 * Somebody beat us to this PTE or prefaulting to
 		 * an already populated PTE, or prefaulting error.
 		 */
-
 		if (unlikely((ret == -EBUSY) || (ret != 0 && i > 0)))
 			break;
 		else if (unlikely(ret != 0)) {
 			retval =
 			    (ret == -ENOMEM) ? VM_FAULT_OOM : VM_FAULT_SIGBUS;
 			goto out_unlock;
-
 		}
 
 		address += PAGE_SIZE;
@@ -221,9 +216,10 @@ static void ttm_bo_vm_open(struct vm_area_struct *vma)
 
 static void ttm_bo_vm_close(struct vm_area_struct *vma)
 {
-	struct ttm_buffer_object *bo =
-	    (struct ttm_buffer_object *)vma->vm_private_data;
+	struct ttm_buffer_object *bo = (struct ttm_buffer_object *)vma->vm_private_data;
 
+	if (bo->bdev->driver->io_mem_free)
+		bo->bdev->driver->io_mem_free(bo->bdev, &bo->mem);
 	ttm_bo_unref(&bo);
 	vma->vm_private_data = NULL;
 }
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 81eb9f4..1b38c67 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -274,6 +274,7 @@ struct ttm_bo_kmap_obj {
 		ttm_bo_map_kmap         = 3,
 		ttm_bo_map_premapped    = 4 | TTM_BO_MAP_IOMEM_MASK,
 	} bo_kmap_type;
+	struct ttm_buffer_object *bo;
 };
 
 /**
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 4c4e0f8..923fecf 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -240,6 +240,23 @@ struct ttm_mem_type_manager {
 };
 
 /**
+ * struct ttm_bus_placement
+ *
+ * @base:		bus base address
+ * @is_iomem:		is this io memory ?
+ * @size:		size in byte
+ * @offset:		offset from the base address
+ *
+ * Structure indicating the bus placement of an object.
+ */
+struct ttm_bus_placement {
+	unsigned long	base;
+	unsigned long	size;
+	unsigned long	offset;
+	bool		is_iomem;
+};
+
+/**
  * struct ttm_bo_driver
  *
  * @create_ttm_backend_entry: Callback to create a struct ttm_backend.
@@ -358,6 +375,19 @@ struct ttm_bo_driver {
 	 * notify the driver that we're about to swap out this bo
 	 */
 	void (*swap_notify) (struct ttm_buffer_object *bo);
+
+	/**
+	 * Driver callback on bo fault, driver is responsible to fill the
+	 * bus placement and has the opportunity to move the buffer into
+	 * visible space.
+	 */
+	int (*fault_reserve)(struct ttm_buffer_object *bo, struct ttm_bus_placement *pl);
+	/**
+	 * Driver callback on when mapping io memory (for bo_move_memcpy for instance).
+	 * TTM will take care to call io_mem_free whenever the mapping is not use anymore.
+	 */
+	int (*io_mem_reserve)(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem, struct ttm_bus_placement *pl);
+	void (*io_mem_free)(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem);
 };
 
 /**
-- 
1.6.6


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

[PATCH 1/9] drm/ttm: ttm_fault callback to allow driver to handle bo placement V2

[Jerome Glisse]

On fault the driver is given the opportunity to perform any operation
it sees fit in order to place the buffer into a CPU visible area of
memory. This patch doesn't break TTM users, nouveau, vmwgfx and radeon
should keep working properly. Future patch will take advantage of this
infrastructure and remove the old path from TTM once driver are
converted.

V2 return VM_FAULT_NOPAGE if callback return -EBUSY or -ERESTARTSYS

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c      |    3 +-
 drivers/gpu/drm/ttm/ttm_bo_util.c |   92 +++++++++++++++++--------------------
 drivers/gpu/drm/ttm/ttm_bo_vm.c   |   56 ++++++++++++----------
 include/drm/ttm/ttm_bo_api.h      |    1 +
 include/drm/ttm/ttm_bo_driver.h   |   30 ++++++++++++
 5 files changed, 105 insertions(+), 77 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index c7320ce..28f3fcf 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1581,7 +1581,8 @@ void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo)
 
 	if (!bdev->dev_mapping)
 		return;
-
+	if (bdev->driver->io_mem_free)
+		bdev->driver->io_mem_free(bdev, &bo->mem);
 	unmap_mapping_range(bdev->dev_mapping, offset, holelen, 1);
 }
 EXPORT_SYMBOL(ttm_bo_unmap_virtual);
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 3f72fe1..10c5fc6 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -84,26 +84,36 @@ int ttm_mem_reg_ioremap(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem,
 			void **virtual)
 {
 	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
-	unsigned long bus_offset;
-	unsigned long bus_size;
-	unsigned long bus_base;
+	struct ttm_bus_placement pl;
 	int ret;
 	void *addr;
 
 	*virtual = NULL;
-	ret = ttm_bo_pci_offset(bdev, mem, &bus_base, &bus_offset, &bus_size);
-	if (ret || bus_size == 0)
-		return ret;
+	if (bdev->driver->io_mem_reserve) {
+		ret = bdev->driver->io_mem_reserve(bdev, mem, &pl);
+		if (unlikely(ret != 0)) {
+			return ret;
+		}
+	} else {
+		ret = ttm_bo_pci_offset(bdev, mem, &pl.base, &pl.offset, &pl.size);
+		if (unlikely(ret != 0) || pl.size == 0) {
+			return ret;
+		}
+		pl.is_iomem = (pl.size != 0);
+	}
 
 	if (!(man->flags & TTM_MEMTYPE_FLAG_NEEDS_IOREMAP))
-		addr = (void *)(((u8 *) man->io_addr) + bus_offset);
+		addr = (void *)(pl.base + pl.offset);
 	else {
 		if (mem->placement & TTM_PL_FLAG_WC)
-			addr = ioremap_wc(bus_base + bus_offset, bus_size);
+			addr = ioremap_wc(pl.base + pl.offset, pl.size);
 		else
-			addr = ioremap_nocache(bus_base + bus_offset, bus_size);
-		if (!addr)
+			addr = ioremap_nocache(pl.base + pl.offset, pl.size);
+		if (!addr) {
+			if (bdev->driver->io_mem_free)
+				bdev->driver->io_mem_free(bdev, mem);
 			return -ENOMEM;
+		}
 	}
 	*virtual = addr;
 	return 0;
@@ -118,6 +128,8 @@ void ttm_mem_reg_iounmap(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem,
 
 	if (virtual && (man->flags & TTM_MEMTYPE_FLAG_NEEDS_IOREMAP))
 		iounmap(virtual);
+	if (bdev->driver->io_mem_free)
+		bdev->driver->io_mem_free(bdev, mem);
 }
 
 static int ttm_copy_io_page(void *dst, void *src, unsigned long page)
@@ -440,13 +452,12 @@ int ttm_bo_kmap(struct ttm_buffer_object *bo,
 		unsigned long start_page, unsigned long num_pages,
 		struct ttm_bo_kmap_obj *map)
 {
+	struct ttm_bus_placement pl;
 	int ret;
-	unsigned long bus_base;
-	unsigned long bus_offset;
-	unsigned long bus_size;
 
 	BUG_ON(!list_empty(&bo->swap));
 	map->virtual = NULL;
+	map->bo = bo;
 	if (num_pages > bo->num_pages)
 		return -EINVAL;
 	if (start_page > bo->num_pages)
@@ -455,16 +466,24 @@ int ttm_bo_kmap(struct ttm_buffer_object *bo,
 	if (num_pages > 1 && !DRM_SUSER(DRM_CURPROC))
 		return -EPERM;
 #endif
-	ret = ttm_bo_pci_offset(bo->bdev, &bo->mem, &bus_base,
-				&bus_offset, &bus_size);
-	if (ret)
-		return ret;
-	if (bus_size == 0) {
+	if (bo->bdev->driver->io_mem_reserve) {
+		ret = bo->bdev->driver->io_mem_reserve(bo->bdev, &bo->mem, &pl);
+		if (unlikely(ret != 0)) {
+			return ret;
+		}
+	} else {
+		ret = ttm_bo_pci_offset(bo->bdev, &bo->mem, &pl.base, &pl.offset, &pl.size);
+		if (unlikely(ret != 0)) {
+			return ret;
+		}
+		pl.is_iomem = (pl.size != 0);
+	}
+	if (!pl.is_iomem) {
 		return ttm_bo_kmap_ttm(bo, start_page, num_pages, map);
 	} else {
-		bus_offset += start_page << PAGE_SHIFT;
-		bus_size = num_pages << PAGE_SHIFT;
-		return ttm_bo_ioremap(bo, bus_base, bus_offset, bus_size, map);
+		pl.offset += start_page << PAGE_SHIFT;
+		pl.size = num_pages << PAGE_SHIFT;
+		return ttm_bo_ioremap(bo, pl.base, pl.offset, pl.size, map);
 	}
 }
 EXPORT_SYMBOL(ttm_bo_kmap);
@@ -476,6 +495,8 @@ void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map)
 	switch (map->bo_kmap_type) {
 	case ttm_bo_map_iomap:
 		iounmap(map->virtual);
+		if (map->bo->bdev->driver->io_mem_free)
+			map->bo->bdev->driver->io_mem_free(map->bo->bdev, &map->bo->mem);
 		break;
 	case ttm_bo_map_vmap:
 		vunmap(map->virtual);
@@ -493,35 +514,6 @@ void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map)
 }
 EXPORT_SYMBOL(ttm_bo_kunmap);
 
-int ttm_bo_pfn_prot(struct ttm_buffer_object *bo,
-		    unsigned long dst_offset,
-		    unsigned long *pfn, pgprot_t *prot)
-{
-	struct ttm_mem_reg *mem = &bo->mem;
-	struct ttm_bo_device *bdev = bo->bdev;
-	unsigned long bus_offset;
-	unsigned long bus_size;
-	unsigned long bus_base;
-	int ret;
-	ret = ttm_bo_pci_offset(bdev, mem, &bus_base, &bus_offset,
-			&bus_size);
-	if (ret)
-		return -EINVAL;
-	if (bus_size != 0)
-		*pfn = (bus_base + bus_offset + dst_offset) >> PAGE_SHIFT;
-	else
-		if (!bo->ttm)
-			return -EINVAL;
-		else
-			*pfn = page_to_pfn(ttm_tt_get_page(bo->ttm,
-							   dst_offset >>
-							   PAGE_SHIFT));
-	*prot = (mem->placement & TTM_PL_FLAG_CACHED) ?
-		PAGE_KERNEL : ttm_io_prot(mem->placement, PAGE_KERNEL);
-
-	return 0;
-}
-
 int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
 			      void *sync_obj,
 			      void *sync_obj_arg,
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 668dbe8..fe4ac95 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -74,9 +74,7 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	struct ttm_buffer_object *bo = (struct ttm_buffer_object *)
 	    vma->vm_private_data;
 	struct ttm_bo_device *bdev = bo->bdev;
-	unsigned long bus_base;
-	unsigned long bus_offset;
-	unsigned long bus_size;
+	struct ttm_bus_placement pl;
 	unsigned long page_offset;
 	unsigned long page_last;
 	unsigned long pfn;
@@ -84,7 +82,6 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	struct page *page;
 	int ret;
 	int i;
-	bool is_iomem;
 	unsigned long address = (unsigned long)vmf->virtual_address;
 	int retval = VM_FAULT_NOPAGE;
 
@@ -104,11 +101,33 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	if (bdev->driver->fault_reserve_notify)
 		bdev->driver->fault_reserve_notify(bo);
 
+	if (bdev->driver->fault_reserve) {
+		ret = bdev->driver->fault_reserve(bo, &pl);
+		switch (ret) {
+		case 0:
+			break;
+		case -EBUSY:
+			set_need_resched();
+		case -ERESTARTSYS:
+			retval = VM_FAULT_NOPAGE;
+			goto out_unlock;
+		default:
+			retval = VM_FAULT_SIGBUS;
+			goto out_unlock;
+		}
+	} else {
+		ret = ttm_bo_pci_offset(bdev, &bo->mem, &pl.base, &pl.offset, &pl.size);
+		if (unlikely(ret != 0)) {
+			retval = VM_FAULT_SIGBUS;
+			goto out_unlock;
+		}
+		pl.is_iomem = (pl.size != 0);
+	}
+
 	/*
 	 * Wait for buffer data in transit, due to a pipelined
 	 * move.
 	 */
-
 	spin_lock(&bo->lock);
 	if (test_bit(TTM_BO_PRIV_FLAG_MOVING, &bo->priv_flags)) {
 		ret = ttm_bo_wait(bo, false, true, false);
@@ -122,20 +141,10 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 		spin_unlock(&bo->lock);
 
 
-	ret = ttm_bo_pci_offset(bdev, &bo->mem, &bus_base, &bus_offset,
-				&bus_size);
-	if (unlikely(ret != 0)) {
-		retval = VM_FAULT_SIGBUS;
-		goto out_unlock;
-	}
-
-	is_iomem = (bus_size != 0);
-
 	page_offset = ((address - vma->vm_start) >> PAGE_SHIFT) +
 	    bo->vm_node->start - vma->vm_pgoff;
 	page_last = ((vma->vm_end - vma->vm_start) >> PAGE_SHIFT) +
 	    bo->vm_node->start - vma->vm_pgoff;
-
 	if (unlikely(page_offset >= bo->num_pages)) {
 		retval = VM_FAULT_SIGBUS;
 		goto out_unlock;
@@ -154,8 +163,7 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	 * vma->vm_page_prot when the object changes caching policy, with
 	 * the correct locks held.
 	 */
-
-	if (is_iomem) {
+	if (pl.is_iomem) {
 		vma->vm_page_prot = ttm_io_prot(bo->mem.placement,
 						vma->vm_page_prot);
 	} else {
@@ -169,12 +177,9 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	 * Speculatively prefault a number of pages. Only error on
 	 * first page.
 	 */
-
 	for (i = 0; i < TTM_BO_VM_NUM_PREFAULT; ++i) {
-
-		if (is_iomem)
-			pfn = ((bus_base + bus_offset) >> PAGE_SHIFT) +
-			    page_offset;
+		if (pl.is_iomem)
+			pfn = ((pl.base + pl.offset) >> PAGE_SHIFT) + page_offset;
 		else {
 			page = ttm_tt_get_page(ttm, page_offset);
 			if (unlikely(!page && i == 0)) {
@@ -191,14 +196,12 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 		 * Somebody beat us to this PTE or prefaulting to
 		 * an already populated PTE, or prefaulting error.
 		 */
-
 		if (unlikely((ret == -EBUSY) || (ret != 0 && i > 0)))
 			break;
 		else if (unlikely(ret != 0)) {
 			retval =
 			    (ret == -ENOMEM) ? VM_FAULT_OOM : VM_FAULT_SIGBUS;
 			goto out_unlock;
-
 		}
 
 		address += PAGE_SIZE;
@@ -221,9 +224,10 @@ static void ttm_bo_vm_open(struct vm_area_struct *vma)
 
 static void ttm_bo_vm_close(struct vm_area_struct *vma)
 {
-	struct ttm_buffer_object *bo =
-	    (struct ttm_buffer_object *)vma->vm_private_data;
+	struct ttm_buffer_object *bo = (struct ttm_buffer_object *)vma->vm_private_data;
 
+	if (bo->bdev->driver->io_mem_free)
+		bo->bdev->driver->io_mem_free(bo->bdev, &bo->mem);
 	ttm_bo_unref(&bo);
 	vma->vm_private_data = NULL;
 }
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 81eb9f4..1b38c67 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -274,6 +274,7 @@ struct ttm_bo_kmap_obj {
 		ttm_bo_map_kmap         = 3,
 		ttm_bo_map_premapped    = 4 | TTM_BO_MAP_IOMEM_MASK,
 	} bo_kmap_type;
+	struct ttm_buffer_object *bo;
 };
 
 /**
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 4c4e0f8..923fecf 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -240,6 +240,23 @@ struct ttm_mem_type_manager {
 };
 
 /**
+ * struct ttm_bus_placement
+ *
+ * @base:		bus base address
+ * @is_iomem:		is this io memory ?
+ * @size:		size in byte
+ * @offset:		offset from the base address
+ *
+ * Structure indicating the bus placement of an object.
+ */
+struct ttm_bus_placement {
+	unsigned long	base;
+	unsigned long	size;
+	unsigned long	offset;
+	bool		is_iomem;
+};
+
+/**
  * struct ttm_bo_driver
  *
  * @create_ttm_backend_entry: Callback to create a struct ttm_backend.
@@ -358,6 +375,19 @@ struct ttm_bo_driver {
 	 * notify the driver that we're about to swap out this bo
 	 */
 	void (*swap_notify) (struct ttm_buffer_object *bo);
+
+	/**
+	 * Driver callback on bo fault, driver is responsible to fill the
+	 * bus placement and has the opportunity to move the buffer into
+	 * visible space.
+	 */
+	int (*fault_reserve)(struct ttm_buffer_object *bo, struct ttm_bus_placement *pl);
+	/**
+	 * Driver callback on when mapping io memory (for bo_move_memcpy for instance).
+	 * TTM will take care to call io_mem_free whenever the mapping is not use anymore.
+	 */
+	int (*io_mem_reserve)(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem, struct ttm_bus_placement *pl);
+	void (*io_mem_free)(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem);
 };
 
 /**
-- 
1.6.6


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

[PATCH 2/9] drm/radeon/kms: add support for new fault callback

[Jerome Glisse]

This add the support for the new fault callback and also the
infrastructure for supporting unmappable VRAM.

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon_ttm.c |   99 ++++++++++++++++++++++++++++++++++-
 1 files changed, 98 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 1157e0f..b8c6567 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -434,10 +434,104 @@ static int radeon_bo_move(struct ttm_buffer_object *bo,
 memcpy:
 		r = ttm_bo_move_memcpy(bo, evict, no_wait, new_mem);
 	}
-
 	return r;
 }
 
+static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem, struct ttm_bus_placement *pl)
+{
+	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
+	struct radeon_device *rdev = radeon_get_rdev(bdev);
+
+	pl->offset = mem->mm_node->start << PAGE_SHIFT;
+	pl->size = mem->num_pages << PAGE_SHIFT;
+	pl->base = 0;
+	pl->is_iomem = false;
+	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
+		return -EINVAL;
+	switch (mem->mem_type) {
+	case TTM_PL_SYSTEM:
+		/* system memory */
+		return 0;
+	case TTM_PL_TT:
+#if __OS_HAS_AGP
+		if (rdev->flags & RADEON_IS_AGP) {
+			/* RADEON_IS_AGP is set only if AGP is active */
+			pl->base = rdev->mc.agp_base;
+			pl->is_iomem = true;
+		}
+#endif
+		return 0;
+	case TTM_PL_VRAM:
+		/* check if it's visible */
+		if ((pl->offset + pl->size) > rdev->mc.visible_vram_size)
+			return -EINVAL;
+		pl->base = rdev->mc.aper_base;
+		pl->is_iomem = true;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static void radeon_ttm_io_mem_free(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
+{
+	/* hopefully will be usefull soon */
+}
+
+static int radeon_ttm_fault_callback(struct ttm_buffer_object *bo, struct ttm_bus_placement *pl)
+{
+	struct ttm_mem_type_manager *man = &bo->bdev->man[bo->mem.mem_type];
+	struct radeon_bo *rbo;
+	struct radeon_device *rdev;
+	int r;
+
+	pl->offset = bo->mem.mm_node->start << PAGE_SHIFT;
+	pl->size = bo->mem.num_pages << PAGE_SHIFT;
+	pl->base = 0;
+	pl->is_iomem = false;
+	if (!radeon_ttm_bo_is_radeon_bo(bo))
+		/* FIXME should we return 0 ? we don't know about this BO */
+		return -EINVAL;
+	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
+		return -EINVAL;
+	rbo = container_of(bo, struct radeon_bo, tbo);
+	rdev = rbo->rdev;
+	switch (bo->mem.mem_type) {
+	case TTM_PL_SYSTEM:
+		/* System memory */
+		return 0;
+	case TTM_PL_TT:
+#if __OS_HAS_AGP
+		if (rdev->flags & RADEON_IS_AGP) {
+			/* RADEON_IS_AGP is set only if AGP is active */
+			pl->base = rdev->mc.agp_base;
+			pl->is_iomem = true;
+		}
+#endif
+		return 0;
+	case TTM_PL_VRAM:
+		if ((pl->offset + pl->size) > rdev->mc.visible_vram_size) {
+			/* hurrah the memory is not visible ! */
+			radeon_ttm_placement_from_domain(rbo, RADEON_GEM_DOMAIN_VRAM);
+			rbo->placement.lpfn = rdev->mc.visible_vram_size >> PAGE_SHIFT;
+			r = ttm_bo_validate(bo, &rbo->placement, false, false);
+			if (unlikely(r != 0))
+				return r;
+			pl->offset = bo->mem.mm_node->start << PAGE_SHIFT;
+			/* this should not happen */
+			if ((pl->offset + pl->size) > rdev->mc.visible_vram_size)
+				return -EINVAL;
+		}
+		pl->base = rdev->mc.aper_base;
+		pl->is_iomem = true;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
 static int radeon_sync_obj_wait(void *sync_obj, void *sync_arg,
 				bool lazy, bool interruptible)
 {
@@ -478,6 +572,9 @@ static struct ttm_bo_driver radeon_bo_driver = {
 	.sync_obj_ref = &radeon_sync_obj_ref,
 	.move_notify = &radeon_bo_move_notify,
 	.fault_reserve_notify = &radeon_bo_fault_reserve_notify,
+	.fault_reserve = &radeon_ttm_fault_callback,
+	.io_mem_reserve = &radeon_ttm_io_mem_reserve,
+	.io_mem_free = &radeon_ttm_io_mem_free,
 };
 
 int radeon_ttm_init(struct radeon_device *rdev)
-- 
1.6.6


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

[PATCH 2/9] drm/radeon/kms: add support for new fault callback V2

[Jerome Glisse]

This add the support for the new fault callback and also the
infrastructure for supporting unmappable VRAM.

V2 validate BO with no_wait = true

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon_ttm.c |   99 ++++++++++++++++++++++++++++++++++-
 1 files changed, 98 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 1157e0f..2ceb5f1 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -434,10 +434,104 @@ static int radeon_bo_move(struct ttm_buffer_object *bo,
 memcpy:
 		r = ttm_bo_move_memcpy(bo, evict, no_wait, new_mem);
 	}
-
 	return r;
 }
 
+static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem, struct ttm_bus_placement *pl)
+{
+	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
+	struct radeon_device *rdev = radeon_get_rdev(bdev);
+
+	pl->offset = mem->mm_node->start << PAGE_SHIFT;
+	pl->size = mem->num_pages << PAGE_SHIFT;
+	pl->base = 0;
+	pl->is_iomem = false;
+	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
+		return -EINVAL;
+	switch (mem->mem_type) {
+	case TTM_PL_SYSTEM:
+		/* system memory */
+		return 0;
+	case TTM_PL_TT:
+#if __OS_HAS_AGP
+		if (rdev->flags & RADEON_IS_AGP) {
+			/* RADEON_IS_AGP is set only if AGP is active */
+			pl->base = rdev->mc.agp_base;
+			pl->is_iomem = true;
+		}
+#endif
+		return 0;
+	case TTM_PL_VRAM:
+		/* check if it's visible */
+		if ((pl->offset + pl->size) > rdev->mc.visible_vram_size)
+			return -EINVAL;
+		pl->base = rdev->mc.aper_base;
+		pl->is_iomem = true;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static void radeon_ttm_io_mem_free(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
+{
+	/* hopefully will be usefull soon */
+}
+
+static int radeon_ttm_fault_callback(struct ttm_buffer_object *bo, struct ttm_bus_placement *pl)
+{
+	struct ttm_mem_type_manager *man = &bo->bdev->man[bo->mem.mem_type];
+	struct radeon_bo *rbo;
+	struct radeon_device *rdev;
+	int r;
+
+	pl->offset = bo->mem.mm_node->start << PAGE_SHIFT;
+	pl->size = bo->mem.num_pages << PAGE_SHIFT;
+	pl->base = 0;
+	pl->is_iomem = false;
+	if (!radeon_ttm_bo_is_radeon_bo(bo))
+		/* FIXME should we return 0 ? we don't know about this BO */
+		return -EINVAL;
+	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
+		return -EINVAL;
+	rbo = container_of(bo, struct radeon_bo, tbo);
+	rdev = rbo->rdev;
+	switch (bo->mem.mem_type) {
+	case TTM_PL_SYSTEM:
+		/* System memory */
+		return 0;
+	case TTM_PL_TT:
+#if __OS_HAS_AGP
+		if (rdev->flags & RADEON_IS_AGP) {
+			/* RADEON_IS_AGP is set only if AGP is active */
+			pl->base = rdev->mc.agp_base;
+			pl->is_iomem = true;
+		}
+#endif
+		return 0;
+	case TTM_PL_VRAM:
+		if ((pl->offset + pl->size) > rdev->mc.visible_vram_size) {
+			/* hurrah the memory is not visible ! */
+			radeon_ttm_placement_from_domain(rbo, RADEON_GEM_DOMAIN_VRAM);
+			rbo->placement.lpfn = rdev->mc.visible_vram_size >> PAGE_SHIFT;
+			r = ttm_bo_validate(bo, &rbo->placement, false, true);
+			if (unlikely(r != 0))
+				return r;
+			pl->offset = bo->mem.mm_node->start << PAGE_SHIFT;
+			/* this should not happen */
+			if ((pl->offset + pl->size) > rdev->mc.visible_vram_size)
+				return -EINVAL;
+		}
+		pl->base = rdev->mc.aper_base;
+		pl->is_iomem = true;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
 static int radeon_sync_obj_wait(void *sync_obj, void *sync_arg,
 				bool lazy, bool interruptible)
 {
@@ -478,6 +572,9 @@ static struct ttm_bo_driver radeon_bo_driver = {
 	.sync_obj_ref = &radeon_sync_obj_ref,
 	.move_notify = &radeon_bo_move_notify,
 	.fault_reserve_notify = &radeon_bo_fault_reserve_notify,
+	.fault_reserve = &radeon_ttm_fault_callback,
+	.io_mem_reserve = &radeon_ttm_io_mem_reserve,
+	.io_mem_free = &radeon_ttm_io_mem_free,
 };
 
 int radeon_ttm_init(struct radeon_device *rdev)
-- 
1.6.6


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

[PATCH 3/9] drm/nouveau/kms: add support for new TTM fault callback

[Jerome Glisse]

This add the support for the new fault callback, does change anything
from driver point of view, thought it should allow nouveau to add
support for unmappable VRAM.

Improvement: store the aperture base in a variable so that we don't
call a function to get it on each fault.

Patch hasn't been tested on any hw.

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/nouveau/nouveau_bo.c |   44 ++++++++++++++++++++++++++++++++++
 1 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index db0ed4c..0c62139 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -757,6 +757,47 @@ nouveau_bo_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 	return 0;
 }
 
+static int
+nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem, struct ttm_bus_placement *pl)
+{
+	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
+	struct drm_nouveau_private *dev_priv = nouveau_bdev(bdev);
+	struct drm_device *dev = dev_priv->dev;
+
+	pl->offset = mem->mm_node->start << PAGE_SHIFT;
+	pl->size = mem->num_pages << PAGE_SHIFT;
+	pl->base = 0;
+	pl->is_iomem = false;
+	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
+		return -EINVAL;
+	switch (mem->mem_type) {
+	case TTM_PL_SYSTEM:
+		/* System memory */
+		return 0;
+	case TTM_PL_TT:
+#if __OS_HAS_AGP
+		if (dev_priv->gart_info.type == NOUVEAU_GART_AGP) {
+			pl->base = dev_priv->gart_info.aper_base;
+			pl->is_iomem = true;
+		}
+#endif
+		return 0;
+	case TTM_PL_VRAM:
+		pl->base = drm_get_resource_start(dev, 1);
+		pl->is_iomem = true;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int
+nouveau_ttm_fault(struct ttm_buffer_object *bo, struct ttm_bus_placement *pl)
+{
+	return nouveau_ttm_io_mem_reserve(bo->bdev, &bo->mem, pl);
+}
+
 struct ttm_bo_driver nouveau_bo_driver = {
 	.create_ttm_backend_entry = nouveau_bo_create_ttm_backend_entry,
 	.invalidate_caches = nouveau_bo_invalidate_caches,
@@ -769,5 +810,8 @@ struct ttm_bo_driver nouveau_bo_driver = {
 	.sync_obj_flush = nouveau_fence_flush,
 	.sync_obj_unref = nouveau_fence_unref,
 	.sync_obj_ref = nouveau_fence_ref,
+	.fault_reserve = &nouveau_ttm_fault,
+	.io_mem_reserve = &nouveau_ttm_io_mem_reserve,
+	.io_mem_free = NULL,
 };
 
-- 
1.6.6


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

[PATCH 4/9] drm/vmwgfx: add support for new TTM fault callback

[Jerome Glisse]

This add the support for the new fault callback, does change anything
from driver point of view.

Improvement: store the aperture base in a variable so that we don't
call a function to get it on each fault.

Patch hasn't been tested.

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c |   35 +++++++++++++++++++++++++++++++-
 1 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
index 825ebe3..c284f52 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
@@ -193,6 +193,36 @@ static void vmw_swap_notify(struct ttm_buffer_object *bo)
 	vmw_dmabuf_gmr_unbind(bo);
 }
 
+static int vmw_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem, struct ttm_bus_placement *pl)
+{
+	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
+	struct vmw_private *dev_priv = container_of(bdev, struct vmw_private, bdev);
+
+	pl->is_iomem = false;
+	pl->offset = mem->mm_node->start << PAGE_SHIFT;
+	pl->size = mem->num_pages << PAGE_SHIFT;
+	pl->base = 0;
+	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
+		return -EINVAL;
+	switch (mem->mem_type) {
+	case TTM_PL_SYSTEM:
+		/* System memory */
+		return 0;
+	case TTM_PL_VRAM:
+		pl->base = dev_priv->vram_start;
+		pl->is_iomem = true;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int vmw_ttm_fault(struct ttm_buffer_object *bo, struct ttm_bus_placement *pl)
+{
+	return vmw_ttm_io_mem_reserve(bo->bdev, &bo->mem, pl);
+}
+
 /**
  * FIXME: We're using the old vmware polling method to sync.
  * Do this with fences instead.
@@ -248,5 +278,8 @@ struct ttm_bo_driver vmw_bo_driver = {
 	.sync_obj_unref = vmw_sync_obj_unref,
 	.sync_obj_ref = vmw_sync_obj_ref,
 	.move_notify = vmw_move_notify,
-	.swap_notify = vmw_swap_notify
+	.swap_notify = vmw_swap_notify,
+	.fault_reserve = vmw_ttm_fault,
+	.io_mem_reserve = &vmw_ttm_io_mem_reserve,
+	.io_mem_free = NULL,
 };
-- 
1.6.6


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

[PATCH 5/9] drm/radeon/kms: don't initialize TTM io memory manager field

[Jerome Glisse]

This isn't needed anymore with the new TTM fault callback

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon_ttm.c |   13 +------------
 1 files changed, 1 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 2ceb5f1..08296e0 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -161,22 +161,14 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 					  (unsigned)type);
 				return -EINVAL;
 			}
-			man->io_offset = rdev->mc.agp_base;
-			man->io_size = rdev->mc.gtt_size;
-			man->io_addr = NULL;
 			if (!rdev->ddev->agp->cant_use_aperture)
 				man->flags = TTM_MEMTYPE_FLAG_NEEDS_IOREMAP |
 					     TTM_MEMTYPE_FLAG_MAPPABLE;
 			man->available_caching = TTM_PL_FLAG_UNCACHED |
 						 TTM_PL_FLAG_WC;
 			man->default_caching = TTM_PL_FLAG_WC;
-		} else
-#endif
-		{
-			man->io_offset = 0;
-			man->io_size = 0;
-			man->io_addr = NULL;
 		}
+#endif
 		break;
 	case TTM_PL_VRAM:
 		/* "On-card" video ram */
@@ -186,9 +178,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 			     TTM_MEMTYPE_FLAG_MAPPABLE;
 		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
 		man->default_caching = TTM_PL_FLAG_WC;
-		man->io_addr = NULL;
-		man->io_offset = rdev->mc.aper_base;
-		man->io_size = rdev->mc.aper_size;
 		break;
 	default:
 		DRM_ERROR("Unsupported memory type %u\n", (unsigned)type);
-- 
1.6.6


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

[PATCH 6/9] drm/nouveau/kms: don't initialize TTM io memory manager field

[Jerome Glisse]

This isn't needed anymore with the new TTM fault callback

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/nouveau/nouveau_bo.c |   11 -----------
 1 files changed, 0 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 0c62139..fd6ae04 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -391,13 +391,6 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 		man->available_caching = TTM_PL_FLAG_UNCACHED |
 					 TTM_PL_FLAG_WC;
 		man->default_caching = TTM_PL_FLAG_WC;
-
-		man->io_addr = NULL;
-		man->io_offset = drm_get_resource_start(dev, 1);
-		man->io_size = drm_get_resource_len(dev, 1);
-		if (man->io_size > nouveau_mem_fb_amount(dev))
-			man->io_size = nouveau_mem_fb_amount(dev);
-
 		man->gpu_offset = dev_priv->vm_vram_base;
 		break;
 	case TTM_PL_TT:
@@ -419,10 +412,6 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 				 dev_priv->gart_info.type);
 			return -EINVAL;
 		}
-
-		man->io_offset  = dev_priv->gart_info.aper_base;
-		man->io_size    = dev_priv->gart_info.aper_size;
-		man->io_addr   = NULL;
 		man->gpu_offset = dev_priv->vm_gart_base;
 		break;
 	default:
-- 
1.6.6


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

[PATCH 7/9] drm/vmwgfx: don't initialize TTM io memory manager field

[Jerome Glisse]

This isn't needed anymore with the new TTM fault callback

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
index c284f52..f5d2519 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
@@ -137,9 +137,6 @@ int vmw_invalidate_caches(struct ttm_bo_device *bdev, uint32_t flags)
 int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 		      struct ttm_mem_type_manager *man)
 {
-	struct vmw_private *dev_priv =
-	    container_of(bdev, struct vmw_private, bdev);
-
 	switch (type) {
 	case TTM_PL_SYSTEM:
 		/* System memory */
@@ -151,11 +148,8 @@ int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	case TTM_PL_VRAM:
 		/* "On-card" video ram */
 		man->gpu_offset = 0;
-		man->io_offset = dev_priv->vram_start;
-		man->io_size = dev_priv->vram_size;
 		man->flags = TTM_MEMTYPE_FLAG_FIXED |
 		    TTM_MEMTYPE_FLAG_NEEDS_IOREMAP | TTM_MEMTYPE_FLAG_MAPPABLE;
-		man->io_addr = NULL;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_WC;
 		break;
-- 
1.6.6


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

[PATCH 8/9] drm/ttm: remove io_ field from TTM

[Jerome Glisse]

All TTM driver have been converted to new io_mem_reserve/free
interface which allow driver to choose and return proper io
base, offset to core TTM for ioremapping if necessary. This
patch remove what is now deadcode.

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c      |   22 ----------------------
 drivers/gpu/drm/ttm/ttm_bo_util.c |   30 +++++++-----------------------
 drivers/gpu/drm/ttm/ttm_bo_vm.c   |   17 ++++-------------
 include/drm/ttm/ttm_bo_driver.h   |   10 ----------
 4 files changed, 11 insertions(+), 68 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 28f3fcf..8a2d534 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -79,8 +79,6 @@ static void ttm_mem_type_debug(struct ttm_bo_device *bdev, int mem_type)
 	printk(KERN_ERR TTM_PFX "    use_type: %d\n", man->use_type);
 	printk(KERN_ERR TTM_PFX "    flags: 0x%08X\n", man->flags);
 	printk(KERN_ERR TTM_PFX "    gpu_offset: 0x%08lX\n", man->gpu_offset);
-	printk(KERN_ERR TTM_PFX "    io_offset: 0x%08lX\n", man->io_offset);
-	printk(KERN_ERR TTM_PFX "    io_size: %ld\n", man->io_size);
 	printk(KERN_ERR TTM_PFX "    size: %llu\n", man->size);
 	printk(KERN_ERR TTM_PFX "    available_caching: 0x%08X\n",
 		man->available_caching);
@@ -1553,26 +1551,6 @@ bool ttm_mem_reg_is_pci(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
 	return true;
 }
 
-int ttm_bo_pci_offset(struct ttm_bo_device *bdev,
-		      struct ttm_mem_reg *mem,
-		      unsigned long *bus_base,
-		      unsigned long *bus_offset, unsigned long *bus_size)
-{
-	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
-
-	*bus_size = 0;
-	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
-		return -EINVAL;
-
-	if (ttm_mem_reg_is_pci(bdev, mem)) {
-		*bus_offset = mem->mm_node->start << PAGE_SHIFT;
-		*bus_size = mem->num_pages << PAGE_SHIFT;
-		*bus_base = man->io_offset;
-	}
-
-	return 0;
-}
-
 void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo)
 {
 	struct ttm_bo_device *bdev = bo->bdev;
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 10c5fc6..47a37f6 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -89,17 +89,9 @@ int ttm_mem_reg_ioremap(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem,
 	void *addr;
 
 	*virtual = NULL;
-	if (bdev->driver->io_mem_reserve) {
-		ret = bdev->driver->io_mem_reserve(bdev, mem, &pl);
-		if (unlikely(ret != 0)) {
-			return ret;
-		}
-	} else {
-		ret = ttm_bo_pci_offset(bdev, mem, &pl.base, &pl.offset, &pl.size);
-		if (unlikely(ret != 0) || pl.size == 0) {
-			return ret;
-		}
-		pl.is_iomem = (pl.size != 0);
+	ret = bdev->driver->io_mem_reserve(bdev, mem, &pl);
+	if (unlikely(ret != 0)) {
+		return ret;
 	}
 
 	if (!(man->flags & TTM_MEMTYPE_FLAG_NEEDS_IOREMAP))
@@ -391,7 +383,7 @@ static int ttm_bo_ioremap(struct ttm_buffer_object *bo,
 
 	if (!(man->flags & TTM_MEMTYPE_FLAG_NEEDS_IOREMAP)) {
 		map->bo_kmap_type = ttm_bo_map_premapped;
-		map->virtual = (void *)(((u8 *) man->io_addr) + bus_offset);
+		map->virtual = (void *)(bus_base + bus_offset);
 	} else {
 		map->bo_kmap_type = ttm_bo_map_iomap;
 		if (mem->placement & TTM_PL_FLAG_WC)
@@ -466,17 +458,9 @@ int ttm_bo_kmap(struct ttm_buffer_object *bo,
 	if (num_pages > 1 && !DRM_SUSER(DRM_CURPROC))
 		return -EPERM;
 #endif
-	if (bo->bdev->driver->io_mem_reserve) {
-		ret = bo->bdev->driver->io_mem_reserve(bo->bdev, &bo->mem, &pl);
-		if (unlikely(ret != 0)) {
-			return ret;
-		}
-	} else {
-		ret = ttm_bo_pci_offset(bo->bdev, &bo->mem, &pl.base, &pl.offset, &pl.size);
-		if (unlikely(ret != 0)) {
-			return ret;
-		}
-		pl.is_iomem = (pl.size != 0);
+	ret = bo->bdev->driver->io_mem_reserve(bo->bdev, &bo->mem, &pl);
+	if (unlikely(ret != 0)) {
+		return ret;
 	}
 	if (!pl.is_iomem) {
 		return ttm_bo_kmap_ttm(bo, start_page, num_pages, map);
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 9446b26..7e9e621 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -101,19 +101,10 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	if (bdev->driver->fault_reserve_notify)
 		bdev->driver->fault_reserve_notify(bo);
 
-	if (bdev->driver->fault_reserve) {
-		ret = bdev->driver->fault_reserve(bo, &pl);
-		if (unlikely(ret != 0)) {
-			retval = VM_FAULT_SIGBUS;
-			goto out_unlock;
-		}
-	} else {
-		ret = ttm_bo_pci_offset(bdev, &bo->mem, &pl.base, &pl.offset, &pl.size);
-		if (unlikely(ret != 0)) {
-			retval = VM_FAULT_SIGBUS;
-			goto out_unlock;
-		}
-		pl.is_iomem = (pl.size != 0);
+	ret = bdev->driver->fault_reserve(bo, &pl);
+	if (unlikely(ret != 0)) {
+		retval = VM_FAULT_SIGBUS;
+		goto out_unlock;
 	}
 
 	/*
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 923fecf..ae8c2a7 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -190,13 +190,6 @@ struct ttm_tt {
  * managed by this memory type.
  * @gpu_offset: If used, the GPU offset of the first managed page of
  * fixed memory or the first managed location in an aperture.
- * @io_offset: The io_offset of the first managed page of IO memory or
- * the first managed location in an aperture. For TTM_MEMTYPE_FLAG_CMA
- * memory, this should be set to NULL.
- * @io_size: The size of a managed IO region (fixed memory or aperture).
- * @io_addr: Virtual kernel address if the io region is pre-mapped. For
- * TTM_MEMTYPE_FLAG_NEEDS_IOREMAP there is no pre-mapped io map and
- * @io_addr should be set to NULL.
  * @size: Size of the managed region.
  * @available_caching: A mask of available caching types, TTM_PL_FLAG_XX,
  * as defined in ttm_placement_common.h
@@ -222,9 +215,6 @@ struct ttm_mem_type_manager {
 	bool use_type;
 	uint32_t flags;
 	unsigned long gpu_offset;
-	unsigned long io_offset;
-	unsigned long io_size;
-	void *io_addr;
 	uint64_t size;
 	uint32_t available_caching;
 	uint32_t default_caching;
-- 
1.6.6


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

[PATCH 8/9] drm/ttm: remove io_ field from TTM V2

[Jerome Glisse]

All TTM driver have been converted to new io_mem_reserve/free
interface which allow driver to choose and return proper io
base, offset to core TTM for ioremapping if necessary. This
patch remove what is now deadcode.

V2 adapt to match with change in first patch of the patchset

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c      |   22 ----------------------
 drivers/gpu/drm/ttm/ttm_bo_util.c |   30 +++++++-----------------------
 drivers/gpu/drm/ttm/ttm_bo_vm.c   |   33 ++++++++++++---------------------
 include/drm/ttm/ttm_bo_driver.h   |   10 ----------
 4 files changed, 19 insertions(+), 76 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 28f3fcf..8a2d534 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -79,8 +79,6 @@ static void ttm_mem_type_debug(struct ttm_bo_device *bdev, int mem_type)
 	printk(KERN_ERR TTM_PFX "    use_type: %d\n", man->use_type);
 	printk(KERN_ERR TTM_PFX "    flags: 0x%08X\n", man->flags);
 	printk(KERN_ERR TTM_PFX "    gpu_offset: 0x%08lX\n", man->gpu_offset);
-	printk(KERN_ERR TTM_PFX "    io_offset: 0x%08lX\n", man->io_offset);
-	printk(KERN_ERR TTM_PFX "    io_size: %ld\n", man->io_size);
 	printk(KERN_ERR TTM_PFX "    size: %llu\n", man->size);
 	printk(KERN_ERR TTM_PFX "    available_caching: 0x%08X\n",
 		man->available_caching);
@@ -1553,26 +1551,6 @@ bool ttm_mem_reg_is_pci(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
 	return true;
 }
 
-int ttm_bo_pci_offset(struct ttm_bo_device *bdev,
-		      struct ttm_mem_reg *mem,
-		      unsigned long *bus_base,
-		      unsigned long *bus_offset, unsigned long *bus_size)
-{
-	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
-
-	*bus_size = 0;
-	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
-		return -EINVAL;
-
-	if (ttm_mem_reg_is_pci(bdev, mem)) {
-		*bus_offset = mem->mm_node->start << PAGE_SHIFT;
-		*bus_size = mem->num_pages << PAGE_SHIFT;
-		*bus_base = man->io_offset;
-	}
-
-	return 0;
-}
-
 void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo)
 {
 	struct ttm_bo_device *bdev = bo->bdev;
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 10c5fc6..47a37f6 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -89,17 +89,9 @@ int ttm_mem_reg_ioremap(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem,
 	void *addr;
 
 	*virtual = NULL;
-	if (bdev->driver->io_mem_reserve) {
-		ret = bdev->driver->io_mem_reserve(bdev, mem, &pl);
-		if (unlikely(ret != 0)) {
-			return ret;
-		}
-	} else {
-		ret = ttm_bo_pci_offset(bdev, mem, &pl.base, &pl.offset, &pl.size);
-		if (unlikely(ret != 0) || pl.size == 0) {
-			return ret;
-		}
-		pl.is_iomem = (pl.size != 0);
+	ret = bdev->driver->io_mem_reserve(bdev, mem, &pl);
+	if (unlikely(ret != 0)) {
+		return ret;
 	}
 
 	if (!(man->flags & TTM_MEMTYPE_FLAG_NEEDS_IOREMAP))
@@ -391,7 +383,7 @@ static int ttm_bo_ioremap(struct ttm_buffer_object *bo,
 
 	if (!(man->flags & TTM_MEMTYPE_FLAG_NEEDS_IOREMAP)) {
 		map->bo_kmap_type = ttm_bo_map_premapped;
-		map->virtual = (void *)(((u8 *) man->io_addr) + bus_offset);
+		map->virtual = (void *)(bus_base + bus_offset);
 	} else {
 		map->bo_kmap_type = ttm_bo_map_iomap;
 		if (mem->placement & TTM_PL_FLAG_WC)
@@ -466,17 +458,9 @@ int ttm_bo_kmap(struct ttm_buffer_object *bo,
 	if (num_pages > 1 && !DRM_SUSER(DRM_CURPROC))
 		return -EPERM;
 #endif
-	if (bo->bdev->driver->io_mem_reserve) {
-		ret = bo->bdev->driver->io_mem_reserve(bo->bdev, &bo->mem, &pl);
-		if (unlikely(ret != 0)) {
-			return ret;
-		}
-	} else {
-		ret = ttm_bo_pci_offset(bo->bdev, &bo->mem, &pl.base, &pl.offset, &pl.size);
-		if (unlikely(ret != 0)) {
-			return ret;
-		}
-		pl.is_iomem = (pl.size != 0);
+	ret = bo->bdev->driver->io_mem_reserve(bo->bdev, &bo->mem, &pl);
+	if (unlikely(ret != 0)) {
+		return ret;
 	}
 	if (!pl.is_iomem) {
 		return ttm_bo_kmap_ttm(bo, start_page, num_pages, map);
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index fe4ac95..7afe239 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -101,27 +101,18 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	if (bdev->driver->fault_reserve_notify)
 		bdev->driver->fault_reserve_notify(bo);
 
-	if (bdev->driver->fault_reserve) {
-		ret = bdev->driver->fault_reserve(bo, &pl);
-		switch (ret) {
-		case 0:
-			break;
-		case -EBUSY:
-			set_need_resched();
-		case -ERESTARTSYS:
-			retval = VM_FAULT_NOPAGE;
-			goto out_unlock;
-		default:
-			retval = VM_FAULT_SIGBUS;
-			goto out_unlock;
-		}
-	} else {
-		ret = ttm_bo_pci_offset(bdev, &bo->mem, &pl.base, &pl.offset, &pl.size);
-		if (unlikely(ret != 0)) {
-			retval = VM_FAULT_SIGBUS;
-			goto out_unlock;
-		}
-		pl.is_iomem = (pl.size != 0);
+	ret = bdev->driver->fault_reserve(bo, &pl);
+	switch (ret) {
+	case 0:
+		break;
+	case -EBUSY:
+		set_need_resched();
+	case -ERESTARTSYS:
+		retval = VM_FAULT_NOPAGE;
+		goto out_unlock;
+	default:
+		retval = VM_FAULT_SIGBUS;
+		goto out_unlock;
 	}
 
 	/*
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 923fecf..ae8c2a7 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -190,13 +190,6 @@ struct ttm_tt {
  * managed by this memory type.
  * @gpu_offset: If used, the GPU offset of the first managed page of
  * fixed memory or the first managed location in an aperture.
- * @io_offset: The io_offset of the first managed page of IO memory or
- * the first managed location in an aperture. For TTM_MEMTYPE_FLAG_CMA
- * memory, this should be set to NULL.
- * @io_size: The size of a managed IO region (fixed memory or aperture).
- * @io_addr: Virtual kernel address if the io region is pre-mapped. For
- * TTM_MEMTYPE_FLAG_NEEDS_IOREMAP there is no pre-mapped io map and
- * @io_addr should be set to NULL.
  * @size: Size of the managed region.
  * @available_caching: A mask of available caching types, TTM_PL_FLAG_XX,
  * as defined in ttm_placement_common.h
@@ -222,9 +215,6 @@ struct ttm_mem_type_manager {
 	bool use_type;
 	uint32_t flags;
 	unsigned long gpu_offset;
-	unsigned long io_offset;
-	unsigned long io_size;
-	void *io_addr;
 	uint64_t size;
 	uint32_t available_caching;
 	uint32_t default_caching;
-- 
1.6.6


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

[PATCH 9/9] drm/radeon/kms: enable use of unmappable VRAM

[Jerome Glisse]

This patch enable the use of unmappable VRAM thanks to
previous TTM infrastructure change.

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/evergreen.c |    5 -----
 drivers/gpu/drm/radeon/r100.c      |    5 -----
 drivers/gpu/drm/radeon/r600.c      |    5 -----
 drivers/gpu/drm/radeon/rv770.c     |    5 -----
 4 files changed, 0 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index bd2e7aa..abab06e 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -475,11 +475,6 @@ int evergreen_mc_init(struct radeon_device *rdev)
 	rdev->mc.mc_vram_size = RREG32(CONFIG_MEMSIZE) * 1024 * 1024;
 	rdev->mc.real_vram_size = RREG32(CONFIG_MEMSIZE) * 1024 * 1024;
 	rdev->mc.visible_vram_size = rdev->mc.aper_size;
-	/* FIXME remove this once we support unmappable VRAM */
-	if (rdev->mc.mc_vram_size > rdev->mc.aper_size) {
-		rdev->mc.mc_vram_size = rdev->mc.aper_size;
-		rdev->mc.real_vram_size = rdev->mc.aper_size;
-	}
 	r600_vram_gtt_location(rdev, &rdev->mc);
 	/* FIXME: we should enforce default clock in case GPU is not in
 	 * default setup
diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index 03f3b2f..2095ab2 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -1991,11 +1991,6 @@ void r100_vram_init_sizes(struct radeon_device *rdev)
 		else
 			rdev->mc.mc_vram_size = rdev->mc.real_vram_size;
 	}
-	/* FIXME remove this once we support unmappable VRAM */
-	if (rdev->mc.mc_vram_size > rdev->mc.aper_size) {
-		rdev->mc.mc_vram_size = rdev->mc.aper_size;
-		rdev->mc.real_vram_size = rdev->mc.aper_size;
-	}
 }
 
 void r100_vga_set_state(struct radeon_device *rdev, bool state)
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index ad1b0c7..9b5a0b1 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -727,11 +727,6 @@ int r600_mc_init(struct radeon_device *rdev)
 	rdev->mc.mc_vram_size = RREG32(CONFIG_MEMSIZE);
 	rdev->mc.real_vram_size = RREG32(CONFIG_MEMSIZE);
 	rdev->mc.visible_vram_size = rdev->mc.aper_size;
-	/* FIXME remove this once we support unmappable VRAM */
-	if (rdev->mc.mc_vram_size > rdev->mc.aper_size) {
-		rdev->mc.mc_vram_size = rdev->mc.aper_size;
-		rdev->mc.real_vram_size = rdev->mc.aper_size;
-	}
 	r600_vram_gtt_location(rdev, &rdev->mc);
 	/* FIXME: we should enforce default clock in case GPU is not in
 	 * default setup
diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c
index 81c8739..66b22d0 100644
--- a/drivers/gpu/drm/radeon/rv770.c
+++ b/drivers/gpu/drm/radeon/rv770.c
@@ -821,11 +821,6 @@ int rv770_mc_init(struct radeon_device *rdev)
 	rdev->mc.mc_vram_size = RREG32(CONFIG_MEMSIZE);
 	rdev->mc.real_vram_size = RREG32(CONFIG_MEMSIZE);
 	rdev->mc.visible_vram_size = rdev->mc.aper_size;
-	/* FIXME remove this once we support unmappable VRAM */
-	if (rdev->mc.mc_vram_size > rdev->mc.aper_size) {
-		rdev->mc.mc_vram_size = rdev->mc.aper_size;
-		rdev->mc.real_vram_size = rdev->mc.aper_size;
-	}
 	r600_vram_gtt_location(rdev, &rdev->mc);
 	/* FIXME: we should enforce default clock in case GPU is not in
 	 * default setup
-- 
1.6.6


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

Re: [PATCH 1/9] drm/ttm: ttm_fault callback to allow driver to handle bo placement V2

[Thomas Hellstrom]

Jerome Glisse wrote:
> On fault the driver is given the opportunity to perform any operation
> it sees fit in order to place the buffer into a CPU visible area of
> memory. This patch doesn't break TTM users, nouveau, vmwgfx and radeon
> should keep working properly. Future patch will take advantage of this
> infrastructure and remove the old path from TTM once driver are
> converted.
>
> V2 return VM_FAULT_NOPAGE if callback return -EBUSY or -ERESTARTSYS
>
> Signed-off-by: Jerome Glisse <jglisse@redhat.com>
>   


Jerome, See inline comments below and


1) Please make sure the patch passes checkpatch.pl without warnings or 
errors before submitting. This patch has 14 warnings.
2) There are a number of line spacing changes (empty newline changes) 
and line break changes. Please remove them from the patch.



> ---
>  drivers/gpu/drm/ttm/ttm_bo.c      |    3 +-
>  drivers/gpu/drm/ttm/ttm_bo_util.c |   92 +++++++++++++++++--------------------
>  drivers/gpu/drm/ttm/ttm_bo_vm.c   |   56 ++++++++++++----------
>  include/drm/ttm/ttm_bo_api.h      |    1 +
>  include/drm/ttm/ttm_bo_driver.h   |   30 ++++++++++++
>  5 files changed, 105 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index c7320ce..28f3fcf 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1581,7 +1581,8 @@ void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo)
>  
>  	if (!bdev->dev_mapping)
>  		return;
> -
> +	if (bdev->driver->io_mem_free)
> +		bdev->driver->io_mem_free(bdev, &bo->mem);
>  	unmap_mapping_range(bdev->dev_mapping, offset, holelen, 1);
>  }
>   

The user-space mappings must be taken down *before* the IO space is 
released, but please also see at the bottom for a design change for this.


>  EXPORT_SYMBOL(ttm_bo_unmap_virtual);
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 3f72fe1..10c5fc6 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -84,26 +84,36 @@ int ttm_mem_reg_ioremap(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem,
>  			void **virtual)
>  {
>  	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
> -	unsigned long bus_offset;
> -	unsigned long bus_size;
> -	unsigned long bus_base;
> +	struct ttm_bus_placement pl;
>  	int ret;
>  	void *addr;
>  
>  	*virtual = NULL;
> -	ret = ttm_bo_pci_offset(bdev, mem, &bus_base, &bus_offset, &bus_size);
> -	if (ret || bus_size == 0)
> -		return ret;
> +	if (bdev->driver->io_mem_reserve) {
> +		ret = bdev->driver->io_mem_reserve(bdev, mem, &pl);
> +		if (unlikely(ret != 0)) {
> +			return ret;
> +		}
> +	} else {
> +		ret = ttm_bo_pci_offset(bdev, mem, &pl.base, &pl.offset, &pl.size);
> +		if (unlikely(ret != 0) || pl.size == 0) {
> +			return ret;
> +		}
>   
> +		pl.is_iomem = (pl.size != 0);
>   

I fail to see how pl.size can be == 0 here?

> +	}
>  
>  	if (!(man->flags & TTM_MEMTYPE_FLAG_NEEDS_IOREMAP))
> -		addr = (void *)(((u8 *) man->io_addr) + bus_offset);
> +		addr = (void *)(pl.base + pl.offset);
>   

This is incorrect. man->io_addr is the virtual kernel address of an 
existing iomap covering the whole aperture, if the driver has choosen to 
set up one. This is not practical on 32-bit systems, due to the limited 
vmalloc / ioremap space but may well be on 64-bit system.

>  	else {
>  		if (mem->placement & TTM_PL_FLAG_WC)
> -			addr = ioremap_wc(bus_base + bus_offset, bus_size);
> +			addr = ioremap_wc(pl.base + pl.offset, pl.size);
>  		else
> -			addr = ioremap_nocache(bus_base + bus_offset, bus_size);
> -		if (!addr)
> +			addr = ioremap_nocache(pl.base + pl.offset, pl.size);
> +		if (!addr) {
> +			if (bdev->driver->io_mem_free)
> +				bdev->driver->io_mem_free(bdev, mem);
>  			return -ENOMEM;
> +		}
>  	}
>  	*virtual = addr;
>  	return 0;
> @@ -118,6 +128,8 @@ void ttm_mem_reg_iounmap(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem,
>  
>  	if (virtual && (man->flags & TTM_MEMTYPE_FLAG_NEEDS_IOREMAP))
>  		iounmap(virtual);
> +	if (bdev->driver->io_mem_free)
> +		bdev->driver->io_mem_free(bdev, mem);
>  }
>  
>  static int ttm_copy_io_page(void *dst, void *src, unsigned long page)
> @@ -440,13 +452,12 @@ int ttm_bo_kmap(struct ttm_buffer_object *bo,
>  		unsigned long start_page, unsigned long num_pages,
>  		struct ttm_bo_kmap_obj *map)
>  {
> +	struct ttm_bus_placement pl;
>  	int ret;
> -	unsigned long bus_base;
> -	unsigned long bus_offset;
> -	unsigned long bus_size;
>  
>  	BUG_ON(!list_empty(&bo->swap));
>  	map->virtual = NULL;
> +	map->bo = bo;
>  	if (num_pages > bo->num_pages)
>  		return -EINVAL;
>  	if (start_page > bo->num_pages)
> @@ -455,16 +466,24 @@ int ttm_bo_kmap(struct ttm_buffer_object *bo,
>  	if (num_pages > 1 && !DRM_SUSER(DRM_CURPROC))
>  		return -EPERM;
>  #endif
> -	ret = ttm_bo_pci_offset(bo->bdev, &bo->mem, &bus_base,
> -				&bus_offset, &bus_size);
> -	if (ret)
> -		return ret;
> -	if (bus_size == 0) {
> +	if (bo->bdev->driver->io_mem_reserve) {
> +		ret = bo->bdev->driver->io_mem_reserve(bo->bdev, &bo->mem, &pl);
> +		if (unlikely(ret != 0)) {
> +			return ret;
> +		}
> +	} else {
> +		ret = ttm_bo_pci_offset(bo->bdev, &bo->mem, &pl.base, &pl.offset, &pl.size);
> +		if (unlikely(ret != 0)) {
> +			return ret;
> +		}
> +		pl.is_iomem = (pl.size != 0);
> +	}
> +	if (!pl.is_iomem) {
>  		return ttm_bo_kmap_ttm(bo, start_page, num_pages, map);
>  	} else {
> -		bus_offset += start_page << PAGE_SHIFT;
> -		bus_size = num_pages << PAGE_SHIFT;
> -		return ttm_bo_ioremap(bo, bus_base, bus_offset, bus_size, map);
> +		pl.offset += start_page << PAGE_SHIFT;
> +		pl.size = num_pages << PAGE_SHIFT;
> +		return ttm_bo_ioremap(bo, pl.base, pl.offset, pl.size, map);
>  	}
>  }
>  EXPORT_SYMBOL(ttm_bo_kmap);
> @@ -476,6 +495,8 @@ void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map)
>  	switch (map->bo_kmap_type) {
>  	case ttm_bo_map_iomap:
>  		iounmap(map->virtual);
> +		if (map->bo->bdev->driver->io_mem_free)
> +			map->bo->bdev->driver->io_mem_free(map->bo->bdev, &map->bo->mem);
>   

Hmm.
What will happen if, say 10 multiple threads ttm_bo_kmap() and 
ttm_bo_kunmap() a single NO_EVICT bo racing with eachother? If I don't 
misunderstand things, that would cause a considerable mess?


>  		break;
>  	case ttm_bo_map_vmap:
>  		vunmap(map->virtual);
> @@ -493,35 +514,6 @@ void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map)
>  }
>  EXPORT_SYMBOL(ttm_bo_kunmap);
>   



>  
> -int ttm_bo_pfn_prot(struct ttm_buffer_object *bo,
> -		    unsigned long dst_offset,
> -		    unsigned long *pfn, pgprot_t *prot)
> -{
> -	struct ttm_mem_reg *mem = &bo->mem;
> -	struct ttm_bo_device *bdev = bo->bdev;
> -	unsigned long bus_offset;
> -	unsigned long bus_size;
> -	unsigned long bus_base;
> -	int ret;
> -	ret = ttm_bo_pci_offset(bdev, mem, &bus_base, &bus_offset,
> -			&bus_size);
> -	if (ret)
> -		return -EINVAL;
> -	if (bus_size != 0)
> -		*pfn = (bus_base + bus_offset + dst_offset) >> PAGE_SHIFT;
> -	else
> -		if (!bo->ttm)
> -			return -EINVAL;
> -		else
> -			*pfn = page_to_pfn(ttm_tt_get_page(bo->ttm,
> -							   dst_offset >>
> -							   PAGE_SHIFT));
> -	*prot = (mem->placement & TTM_PL_FLAG_CACHED) ?
> -		PAGE_KERNEL : ttm_io_prot(mem->placement, PAGE_KERNEL);
> -
> -	return 0;
> -}
> -
>  int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
>  			      void *sync_obj,
>  			      void *sync_obj_arg,
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> index 668dbe8..fe4ac95 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> @@ -74,9 +74,7 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>  	struct ttm_buffer_object *bo = (struct ttm_buffer_object *)
>  	    vma->vm_private_data;
>  	struct ttm_bo_device *bdev = bo->bdev;
> -	unsigned long bus_base;
> -	unsigned long bus_offset;
> -	unsigned long bus_size;
> +	struct ttm_bus_placement pl;
>  	unsigned long page_offset;
>  	unsigned long page_last;
>  	unsigned long pfn;
> @@ -84,7 +82,6 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>  	struct page *page;
>  	int ret;
>  	int i;
> -	bool is_iomem;
>  	unsigned long address = (unsigned long)vmf->virtual_address;
>  	int retval = VM_FAULT_NOPAGE;
>  
> @@ -104,11 +101,33 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>  	if (bdev->driver->fault_reserve_notify)
>  		bdev->driver->fault_reserve_notify(bo);
>  
> +	if (bdev->driver->fault_reserve) {
> +		ret = bdev->driver->fault_reserve(bo, &pl);
> +		switch (ret) {
> +		case 0:
> +			break;
> +		case -EBUSY:
> +			set_need_resched();
> +		case -ERESTARTSYS:
> +			retval = VM_FAULT_NOPAGE;
> +			goto out_unlock;
> +		default:
> +			retval = VM_FAULT_SIGBUS;
> +			goto out_unlock;
> +		}
> +	} else {
> +		ret = ttm_bo_pci_offset(bdev, &bo->mem, &pl.base, &pl.offset, &pl.size);
> +		if (unlikely(ret != 0)) {
> +			retval = VM_FAULT_SIGBUS;
> +			goto out_unlock;
> +		}
> +		pl.is_iomem = (pl.size != 0);
> +	}
> +
>  	/*
>  	 * Wait for buffer data in transit, due to a pipelined
>  	 * move.
>  	 */
> -
>   

>  	spin_lock(&bo->lock);
>  	if (test_bit(TTM_BO_PRIV_FLAG_MOVING, &bo->priv_flags)) {
>  		ret = ttm_bo_wait(bo, false, true, false);
> @@ -122,20 +141,10 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>  		spin_unlock(&bo->lock);
>  
>  
> -	ret = ttm_bo_pci_offset(bdev, &bo->mem, &bus_base, &bus_offset,
> -				&bus_size);
> -	if (unlikely(ret != 0)) {
> -		retval = VM_FAULT_SIGBUS;
> -		goto out_unlock;
> -	}
> -
> -	is_iomem = (bus_size != 0);
> -
>  	page_offset = ((address - vma->vm_start) >> PAGE_SHIFT) +
>  	    bo->vm_node->start - vma->vm_pgoff;
>  	page_last = ((vma->vm_end - vma->vm_start) >> PAGE_SHIFT) +
>  	    bo->vm_node->start - vma->vm_pgoff;
> -
>   

>  	if (unlikely(page_offset >= bo->num_pages)) {
>  		retval = VM_FAULT_SIGBUS;
>  		goto out_unlock;
> @@ -154,8 +163,7 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>  	 * vma->vm_page_prot when the object changes caching policy, with
>  	 * the correct locks held.
>  	 */
> -
> -	if (is_iomem) {
> +	if (pl.is_iomem) {
>  		vma->vm_page_prot = ttm_io_prot(bo->mem.placement,
>  						vma->vm_page_prot);
>   

>  	} else {
> @@ -169,12 +177,9 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>  	 * Speculatively prefault a number of pages. Only error on
>  	 * first page.
>  	 */
> -
>  	for (i = 0; i < TTM_BO_VM_NUM_PREFAULT; ++i) {
> -
>   

> -		if (is_iomem)
> -			pfn = ((bus_base + bus_offset) >> PAGE_SHIFT) +
> -			    page_offset;
> +		if (pl.is_iomem)
> +			pfn = ((pl.base + pl.offset) >> PAGE_SHIFT) + page_offset;
>  		else {
>  			page = ttm_tt_get_page(ttm, page_offset);
>  			if (unlikely(!page && i == 0)) {
> @@ -191,14 +196,12 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>  		 * Somebody beat us to this PTE or prefaulting to
>  		 * an already populated PTE, or prefaulting error.
>  		 */
> -
>   

>  		if (unlikely((ret == -EBUSY) || (ret != 0 && i > 0)))
>  			break;
>  		else if (unlikely(ret != 0)) {
>  			retval =
>  			    (ret == -ENOMEM) ? VM_FAULT_OOM : VM_FAULT_SIGBUS;
>  			goto out_unlock;
> -
>   

>  		}
>  
>  		address += PAGE_SIZE;
> @@ -221,9 +224,10 @@ static void ttm_bo_vm_open(struct vm_area_struct *vma)
>  
>  static void ttm_bo_vm_close(struct vm_area_struct *vma)
>  {
> -	struct ttm_buffer_object *bo =
> -	    (struct ttm_buffer_object *)vma->vm_private_data;
> +	struct ttm_buffer_object *bo = (struct ttm_buffer_object *)vma->vm_private_data
>   

>  
> +	if (bo->bdev->driver->io_mem_free)
> +		bo->bdev->driver->io_mem_free(bo->bdev, &bo->mem);
>   

Ugh. This doesn't work. A single bo may have any number of VMAs open. 
Closing one of them should not remove the bo from mappable state. Only 
the last vm_close should. If needed, call ttm_bo_unmap_virtual just 
before the bo's vm_node is freed in ttm_bo_release(), or refcount VMAs 
on the bo. See additional comments below.

>  	ttm_bo_unref(&bo);
>  	vma->vm_private_data = NULL;
>  }
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index 81eb9f4..1b38c67 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -274,6 +274,7 @@ struct ttm_bo_kmap_obj {
>  		ttm_bo_map_kmap         = 3,
>  		ttm_bo_map_premapped    = 4 | TTM_BO_MAP_IOMEM_MASK,
>  	} bo_kmap_type;
> +	struct ttm_buffer_object *bo;
>  };
>  
>  /**
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 4c4e0f8..923fecf 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -240,6 +240,23 @@ struct ttm_mem_type_manager {
>  };
>  
>  /**
> + * struct ttm_bus_placement
> + *
> + * @base:		bus base address
> + * @is_iomem:		is this io memory ?
> + * @size:		size in byte
> + * @offset:		offset from the base address
> + *
> + * Structure indicating the bus placement of an object.
> + */
> +struct ttm_bus_placement {
> +	unsigned long	base;
> +	unsigned long	size;
> +	unsigned long	offset;
> +	bool		is_iomem;
> +};
> +
> +/**
>   * struct ttm_bo_driver
>   *
>   * @create_ttm_backend_entry: Callback to create a struct ttm_backend.
> @@ -358,6 +375,19 @@ struct ttm_bo_driver {
>  	 * notify the driver that we're about to swap out this bo
>  	 */
>  	void (*swap_notify) (struct ttm_buffer_object *bo);
> +
> +	/**
> +	 * Driver callback on bo fault, driver is responsible to fill the
> +	 * bus placement and has the opportunity to move the buffer into
> +	 * visible space.
> +	 */
> +	int (*fault_reserve)(struct ttm_buffer_object *bo, struct ttm_bus_placement *pl);
> +	/**
> +	 * Driver callback on when mapping io memory (for bo_move_memcpy for instance).
> +	 * TTM will take care to call io_mem_free whenever the mapping is not use anymore.
> +	 */
> +	int (*io_mem_reserve)(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem, struct ttm_bus_placement *pl);
> +	void (*io_mem_free)(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem);
>  };
>  
>  /**
>   

Some additional comments:

I think io_mem_reserve() and io_mem_free() should carry out the 
equivalent of 'pin_mappable' and 'unpin_mappable' and additionally 
refcounted to avoid different processes racing. To make this consistent 
with the rest of TTM one would need to impose the restriction that these 
functions must be called only if the bo is in the reserved state or if 
it's pinned with NO_EVICT. That would mean that each struct ttm_mem_reg 
or whatever struct is passed to these functions would need to carry 
driver-specific info that can hold relevant refcounts and perhaps other 
information. Each io_mem_reserve() must be paired with exactly one 
io_mem_free().

Furthermore, For fault_reserve(), the only thing we need to guarantee is 
that the bo is mappable while there are user-space mappings to the bo. 
We can't pair fault_reserve() with io_mem_free() which is currently 
done. Instead we need a fault_unreserve() which should be called from 
ttm_bo_unmap_virtual().

That means also that any 'mappable space manager' that evicts a bo from 
mappable state needs to reserve the bo to be evicted with the no_wait 
flag set, and then call ttm_bo_unmap_virtual() on it. If the 'mappable 
space manager' is just a separate part of a TTM memory region, that's 
already taken care of.

Thanks,
Thomas


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

Re: Unmappable VRAM patchset V3

[Thomas Hellstrom]

Jerome Glisse wrote:
> Thomas i think i addressed your concern here, the ttm_bo_validate
> didn't needed a new argument or i did not understand what was
> necessary beside no_wait. In this patchset we check the value
> of callback in case of EBUSY (call set_need_resched) or ERESTARTSYS
> we return VM_FAULT_NOPAGE.
>   
Well, if we from the fault callback call any function that might call 
ttm_bo_reserve or ttm_bo_reserve_locked, we must make sure that we never 
wait, but return -EBUSY all the way back to the fault function. Such a 
case may be ttm_bo_validate that calls ttm_bo_evict_first, or something 
causing a swapout... ttm_bo_validate currently doesn't have that 
functionality, because @no_wait just means don't wait for GPU.

> For the design question of moving the io address determination
> into the driver i believe separate aperture is a good example
> of where this is needed, i also think nvidia hw can remap dynamicly
> part of the aperture such feature can be use with the new interface
> this patchset introduce.
>
> Thomas any more concern ?
>   
 I need to have a deeper look into how the vm functions are used. I'll 
get back when I've reviewed.

> Thanks for reviewing this :)
>
> Cheers,
> Jerome
>
>   

/Thomas


------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

Re: Unmappable VRAM patchset V3

[Jerome Glisse]

On Mon, Feb 22, 2010 at 06:30:24PM +0100, Thomas Hellstrom wrote:
> Jerome Glisse wrote:
> > Thomas i think i addressed your concern here, the ttm_bo_validate
> > didn't needed a new argument or i did not understand what was
> > necessary beside no_wait. In this patchset we check the value
> > of callback in case of EBUSY (call set_need_resched) or ERESTARTSYS
> > we return VM_FAULT_NOPAGE.
> >   
> Well, if we from the fault callback call any function that might call 
> ttm_bo_reserve or ttm_bo_reserve_locked, we must make sure that we never 
> wait, but return -EBUSY all the way back to the fault function. Such a 
> case may be ttm_bo_validate that calls ttm_bo_evict_first, or something 
> causing a swapout... ttm_bo_validate currently doesn't have that 
> functionality, because @no_wait just means don't wait for GPU.

What do you mean by never wait ? Is this GPU wait ? or CPU wait ie don't
use mutex or kernel code path that might sleep ?

After a new review i don't think we ever wait for the GPU with the current
patch and as far as i can tell we will return EBUSY or ERESTART all the
way up.

Cheers,
Jerome

------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

Re: Unmappable VRAM patchset V3

[Jerome Glisse]

On Mon, Feb 22, 2010 at 08:58:28PM +0100, Thomas Hellstrom wrote:
> Jerome Glisse wrote:
> >On Mon, Feb 22, 2010 at 06:30:24PM +0100, Thomas Hellstrom wrote:
> >>Jerome Glisse wrote:
> >>>Thomas i think i addressed your concern here, the ttm_bo_validate
> >>>didn't needed a new argument or i did not understand what was
> >>>necessary beside no_wait. In this patchset we check the value
> >>>of callback in case of EBUSY (call set_need_resched) or ERESTARTSYS
> >>>we return VM_FAULT_NOPAGE.
> >>Well, if we from the fault callback call any function that might
> >>call ttm_bo_reserve or ttm_bo_reserve_locked, we must make sure
> >>that we never wait, but return -EBUSY all the way back to the
> >>fault function. Such a case may be ttm_bo_validate that calls
> >>ttm_bo_evict_first, or something causing a swapout...
> >>ttm_bo_validate currently doesn't have that functionality,
> >>because @no_wait just means don't wait for GPU.
> >
> >What do you mean by never wait ? Is this GPU wait ? or CPU wait ie don't
> >use mutex or kernel code path that might sleep ?
> 
> I mean waiting while reserving a bo. (If another thread has the bo
> reserved).
> 
> >After a new review i don't think we ever wait for the GPU with the current
> >patch and as far as i can tell we will return EBUSY or ERESTART all the
> >way up.
> >
> >Cheers,
> >Jerome
> 
> If there is *no* code path trying to reserve a bo or create a
> user-space visible object from within the fault handler, it should
> be ok.
> 
> /Thomas
> 
> 

Did a new review again here is the call chain :
ttm_bo_move_buffer
  ttm_bo_mem_space
    ttm_bo_mem_force_space
      ttm_mem_evict_first
        ttm_bo_reserve_locked (no_wait = true)
        ttm_bo_evict
          ttm_bo_mem_space
          ttm_bo_handle_move_mem
            (ttm_bo_unmap_virtual)
            ttm_bo_add_ttm
              ttm_tt_create
  ttm_bo_handle_move_mem
    (ttm_bo_unmap_virtual)
    ttm_bo_add_ttm
      ttm_tt_create
the no_wait argument is passed all the way down and whenever
there is an error or EBUSY or ERESTARTSYS it's then returned
all the way up. I am bit unconfortable with the ttm_bo_unmap_virtual
i wonder if it can cause issue. I have been looking for
information on the fault callback and what one can do or one
shouldn't do but haven't found much, do you have any pointer
beside reading core code ?

Cheers,
Jerome

------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

Re: Unmappable VRAM patchset V3

[Thomas Hellstrom]

Jerome Glisse wrote:
> On Mon, Feb 22, 2010 at 08:58:28PM +0100, Thomas Hellstrom wrote:
>   
>> Jerome Glisse wrote:
>>     
>>> On Mon, Feb 22, 2010 at 06:30:24PM +0100, Thomas Hellstrom wrote:
>>>       
>>>> Jerome Glisse wrote:
>>>>         
>>>>> Thomas i think i addressed your concern here, the ttm_bo_validate
>>>>> didn't needed a new argument or i did not understand what was
>>>>> necessary beside no_wait. In this patchset we check the value
>>>>> of callback in case of EBUSY (call set_need_resched) or ERESTARTSYS
>>>>> we return VM_FAULT_NOPAGE.
>>>>>           
>>>> Well, if we from the fault callback call any function that might
>>>> call ttm_bo_reserve or ttm_bo_reserve_locked, we must make sure
>>>> that we never wait, but return -EBUSY all the way back to the
>>>> fault function. Such a case may be ttm_bo_validate that calls
>>>> ttm_bo_evict_first, or something causing a swapout...
>>>> ttm_bo_validate currently doesn't have that functionality,
>>>> because @no_wait just means don't wait for GPU.
>>>>         
>>> What do you mean by never wait ? Is this GPU wait ? or CPU wait ie don't
>>> use mutex or kernel code path that might sleep ?
>>>       
>> I mean waiting while reserving a bo. (If another thread has the bo
>> reserved).
>>
>>     
>>> After a new review i don't think we ever wait for the GPU with the current
>>> patch and as far as i can tell we will return EBUSY or ERESTART all the
>>> way up.
>>>
>>> Cheers,
>>> Jerome
>>>       
>> If there is *no* code path trying to reserve a bo or create a
>> user-space visible object from within the fault handler, it should
>> be ok.
>>
>> /Thomas
>>
>>
>>     
>
> Did a new review again here is the call chain :
> ttm_bo_move_buffer
>   ttm_bo_mem_space
>     ttm_bo_mem_force_space
>       ttm_mem_evict_first
>         ttm_bo_reserve_locked (no_wait = true)
>   

Here ttm_mem_evict_fist may wait for unreserve IIRC (the -EBUSY return 
from ttm_bo_reserve_locked) is not propagated back.


>         ttm_bo_evict
>           ttm_bo_mem_space
>           ttm_bo_handle_move_mem
>             (ttm_bo_unmap_virtual)
>             ttm_bo_add_ttm
>               ttm_tt_create
>   ttm_bo_handle_move_mem
>     (ttm_bo_unmap_virtual)
>     ttm_bo_add_ttm
>       ttm_tt_create
> the no_wait argument is passed all the way down and whenever
> there is an error or EBUSY or ERESTARTSYS it's then returned
> all the way up. I am bit unconfortable with the ttm_bo_unmap_virtual
> i wonder if it can cause issue. I have been looking for
> information on the fault callback and what one can do or one
> shouldn't do but haven't found much, do you have any pointer
> beside reading core code ?
>   

No, but I think it should be safe.

/Thomas



> Cheers,
> Jerome
>   


------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

Re: Unmappable VRAM patchset V3

[Jerome Glisse]

On Tue, Feb 23, 2010 at 02:05:50PM +0100, Thomas Hellstrom wrote:
> Jerome Glisse wrote:
> >On Mon, Feb 22, 2010 at 08:58:28PM +0100, Thomas Hellstrom wrote:
> >>Jerome Glisse wrote:
> >>>On Mon, Feb 22, 2010 at 06:30:24PM +0100, Thomas Hellstrom wrote:
> >>>>Jerome Glisse wrote:
> >>>>>Thomas i think i addressed your concern here, the ttm_bo_validate
> >>>>>didn't needed a new argument or i did not understand what was
> >>>>>necessary beside no_wait. In this patchset we check the value
> >>>>>of callback in case of EBUSY (call set_need_resched) or ERESTARTSYS
> >>>>>we return VM_FAULT_NOPAGE.
> >>>>Well, if we from the fault callback call any function that might
> >>>>call ttm_bo_reserve or ttm_bo_reserve_locked, we must make sure
> >>>>that we never wait, but return -EBUSY all the way back to the
> >>>>fault function. Such a case may be ttm_bo_validate that calls
> >>>>ttm_bo_evict_first, or something causing a swapout...
> >>>>ttm_bo_validate currently doesn't have that functionality,
> >>>>because @no_wait just means don't wait for GPU.
> >>>What do you mean by never wait ? Is this GPU wait ? or CPU wait ie don't
> >>>use mutex or kernel code path that might sleep ?
> >>I mean waiting while reserving a bo. (If another thread has the bo
> >>reserved).
> >>
> >>>After a new review i don't think we ever wait for the GPU with the current
> >>>patch and as far as i can tell we will return EBUSY or ERESTART all the
> >>>way up.
> >>>
> >>>Cheers,
> >>>Jerome
> >>If there is *no* code path trying to reserve a bo or create a
> >>user-space visible object from within the fault handler, it should
> >>be ok.
> >>
> >>/Thomas
> >>
> >>
> >
> >Did a new review again here is the call chain :
> >ttm_bo_move_buffer
> >  ttm_bo_mem_space
> >    ttm_bo_mem_force_space
> >      ttm_mem_evict_first
> >        ttm_bo_reserve_locked (no_wait = true)
> 
> Here ttm_mem_evict_fist may wait for unreserve IIRC (the -EBUSY
> return from ttm_bo_reserve_locked) is not propagated back.

The code is not straightforward but if no_wait is true the
-EBUSY of ttm_bo_reserve_locked will be propagated back.

> 
> 
> >        ttm_bo_evict
> >          ttm_bo_mem_space
> >          ttm_bo_handle_move_mem
> >            (ttm_bo_unmap_virtual)
> >            ttm_bo_add_ttm
> >              ttm_tt_create
> >  ttm_bo_handle_move_mem
> >    (ttm_bo_unmap_virtual)
> >    ttm_bo_add_ttm
> >      ttm_tt_create
> >the no_wait argument is passed all the way down and whenever
> >there is an error or EBUSY or ERESTARTSYS it's then returned
> >all the way up. I am bit unconfortable with the ttm_bo_unmap_virtual
> >i wonder if it can cause issue. I have been looking for
> >information on the fault callback and what one can do or one
> >shouldn't do but haven't found much, do you have any pointer
> >beside reading core code ?
> 
> No, but I think it should be safe.
> 
> /Thomas

Cheers,
Jerome

------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

Re: Unmappable VRAM patchset V3

[Thomas Hellstrom]

Jerome Glisse wrote:
> On Tue, Feb 23, 2010 at 02:05:50PM +0100, Thomas Hellstrom wrote:
>   
>> Jerome Glisse wrote:
>>     
>>> On Mon, Feb 22, 2010 at 08:58:28PM +0100, Thomas Hellstrom wrote:
>>>       
>>>> Jerome Glisse wrote:
>>>>         
>>>>> On Mon, Feb 22, 2010 at 06:30:24PM +0100, Thomas Hellstrom wrote:
>>>>>           
>>>>>> Jerome Glisse wrote:
>>>>>>             
>>>>>>> Thomas i think i addressed your concern here, the ttm_bo_validate
>>>>>>> didn't needed a new argument or i did not understand what was
>>>>>>> necessary beside no_wait. In this patchset we check the value
>>>>>>> of callback in case of EBUSY (call set_need_resched) or ERESTARTSYS
>>>>>>> we return VM_FAULT_NOPAGE.
>>>>>>>               
>>>>>> Well, if we from the fault callback call any function that might
>>>>>> call ttm_bo_reserve or ttm_bo_reserve_locked, we must make sure
>>>>>> that we never wait, but return -EBUSY all the way back to the
>>>>>> fault function. Such a case may be ttm_bo_validate that calls
>>>>>> ttm_bo_evict_first, or something causing a swapout...
>>>>>> ttm_bo_validate currently doesn't have that functionality,
>>>>>> because @no_wait just means don't wait for GPU.
>>>>>>             
>>>>> What do you mean by never wait ? Is this GPU wait ? or CPU wait ie don't
>>>>> use mutex or kernel code path that might sleep ?
>>>>>           
>>>> I mean waiting while reserving a bo. (If another thread has the bo
>>>> reserved).
>>>>
>>>>         
>>>>> After a new review i don't think we ever wait for the GPU with the current
>>>>> patch and as far as i can tell we will return EBUSY or ERESTART all the
>>>>> way up.
>>>>>
>>>>> Cheers,
>>>>> Jerome
>>>>>           
>>>> If there is *no* code path trying to reserve a bo or create a
>>>> user-space visible object from within the fault handler, it should
>>>> be ok.
>>>>
>>>> /Thomas
>>>>
>>>>
>>>>         
>>> Did a new review again here is the call chain :
>>> ttm_bo_move_buffer
>>>  ttm_bo_mem_space
>>>    ttm_bo_mem_force_space
>>>      ttm_mem_evict_first
>>>        ttm_bo_reserve_locked (no_wait = true)
>>>       
>> Here ttm_mem_evict_fist may wait for unreserve IIRC (the -EBUSY
>> return from ttm_bo_reserve_locked) is not propagated back.
>>     
>
> The code is not straightforward but if no_wait is true the
> -EBUSY of ttm_bo_reserve_locked will be propagated back.
>   

The point is that we don't want to set no_wait to true, because it's OK 
to wait for GPU. We want to add an extra argument no_wait_reserve.

/Thomas


------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

Re: Unmappable VRAM patchset V3

[Jerome Glisse]

On Wed, Feb 24, 2010 at 01:37:42PM +0100, Thomas Hellstrom wrote:
> Jerome Glisse wrote:
> >On Tue, Feb 23, 2010 at 02:05:50PM +0100, Thomas Hellstrom wrote:
> >>Jerome Glisse wrote:
> >>>On Mon, Feb 22, 2010 at 08:58:28PM +0100, Thomas Hellstrom wrote:
> >>>>Jerome Glisse wrote:
> >>>>>On Mon, Feb 22, 2010 at 06:30:24PM +0100, Thomas Hellstrom wrote:
> >>>>>>Jerome Glisse wrote:
> >>>>>>>Thomas i think i addressed your concern here, the ttm_bo_validate
> >>>>>>>didn't needed a new argument or i did not understand what was
> >>>>>>>necessary beside no_wait. In this patchset we check the value
> >>>>>>>of callback in case of EBUSY (call set_need_resched) or ERESTARTSYS
> >>>>>>>we return VM_FAULT_NOPAGE.
> >>>>>>Well, if we from the fault callback call any function that might
> >>>>>>call ttm_bo_reserve or ttm_bo_reserve_locked, we must make sure
> >>>>>>that we never wait, but return -EBUSY all the way back to the
> >>>>>>fault function. Such a case may be ttm_bo_validate that calls
> >>>>>>ttm_bo_evict_first, or something causing a swapout...
> >>>>>>ttm_bo_validate currently doesn't have that functionality,
> >>>>>>because @no_wait just means don't wait for GPU.
> >>>>>What do you mean by never wait ? Is this GPU wait ? or CPU wait ie don't
> >>>>>use mutex or kernel code path that might sleep ?
> >>>>I mean waiting while reserving a bo. (If another thread has the bo
> >>>>reserved).
> >>>>
> >>>>>After a new review i don't think we ever wait for the GPU with the current
> >>>>>patch and as far as i can tell we will return EBUSY or ERESTART all the
> >>>>>way up.
> >>>>>
> >>>>>Cheers,
> >>>>>Jerome
> >>>>If there is *no* code path trying to reserve a bo or create a
> >>>>user-space visible object from within the fault handler, it should
> >>>>be ok.
> >>>>
> >>>>/Thomas
> >>>>
> >>>>
> >>>Did a new review again here is the call chain :
> >>>ttm_bo_move_buffer
> >>> ttm_bo_mem_space
> >>>   ttm_bo_mem_force_space
> >>>     ttm_mem_evict_first
> >>>       ttm_bo_reserve_locked (no_wait = true)
> >>Here ttm_mem_evict_fist may wait for unreserve IIRC (the -EBUSY
> >>return from ttm_bo_reserve_locked) is not propagated back.
> >
> >The code is not straightforward but if no_wait is true the
> >-EBUSY of ttm_bo_reserve_locked will be propagated back.
> 
> The point is that we don't want to set no_wait to true, because it's
> OK to wait for GPU. We want to add an extra argument
> no_wait_reserve.
> 
> /Thomas
> 

My patchset doesn't change the code there, no_wait will be true
only if it's true as argument of ttm_bo_validate from the ttm
fault callback in the driver.

Cheers,
Jerome

------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

Re: Unmappable VRAM patchset V3

[Thomas Hellstrom]

Jerome Glisse wrote:
> On Wed, Feb 24, 2010 at 01:37:42PM +0100, Thomas Hellstrom wrote:
>   
>> Jerome Glisse wrote:
>>     
>>> On Tue, Feb 23, 2010 at 02:05:50PM +0100, Thomas Hellstrom wrote:
>>>       
>>>> Jerome Glisse wrote:
>>>>         
>>>>> On Mon, Feb 22, 2010 at 08:58:28PM +0100, Thomas Hellstrom wrote:
>>>>>           
>>>>>> Jerome Glisse wrote:
>>>>>>             
>>>>>>> On Mon, Feb 22, 2010 at 06:30:24PM +0100, Thomas Hellstrom wrote:
>>>>>>>               
>>>>>>>> Jerome Glisse wrote:
>>>>>>>>                 
>>>>>>>>> Thomas i think i addressed your concern here, the ttm_bo_validate
>>>>>>>>> didn't needed a new argument or i did not understand what was
>>>>>>>>> necessary beside no_wait. In this patchset we check the value
>>>>>>>>> of callback in case of EBUSY (call set_need_resched) or ERESTARTSYS
>>>>>>>>> we return VM_FAULT_NOPAGE.
>>>>>>>>>                   
>>>>>>>> Well, if we from the fault callback call any function that might
>>>>>>>> call ttm_bo_reserve or ttm_bo_reserve_locked, we must make sure
>>>>>>>> that we never wait, but return -EBUSY all the way back to the
>>>>>>>> fault function. Such a case may be ttm_bo_validate that calls
>>>>>>>> ttm_bo_evict_first, or something causing a swapout...
>>>>>>>> ttm_bo_validate currently doesn't have that functionality,
>>>>>>>> because @no_wait just means don't wait for GPU.
>>>>>>>>                 
>>>>>>> What do you mean by never wait ? Is this GPU wait ? or CPU wait ie don't
>>>>>>> use mutex or kernel code path that might sleep ?
>>>>>>>               
>>>>>> I mean waiting while reserving a bo. (If another thread has the bo
>>>>>> reserved).
>>>>>>
>>>>>>             
>>>>>>> After a new review i don't think we ever wait for the GPU with the current
>>>>>>> patch and as far as i can tell we will return EBUSY or ERESTART all the
>>>>>>> way up.
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Jerome
>>>>>>>               
>>>>>> If there is *no* code path trying to reserve a bo or create a
>>>>>> user-space visible object from within the fault handler, it should
>>>>>> be ok.
>>>>>>
>>>>>> /Thomas
>>>>>>
>>>>>>
>>>>>>             
>>>>> Did a new review again here is the call chain :
>>>>> ttm_bo_move_buffer
>>>>> ttm_bo_mem_space
>>>>>   ttm_bo_mem_force_space
>>>>>     ttm_mem_evict_first
>>>>>       ttm_bo_reserve_locked (no_wait = true)
>>>>>           
>>>> Here ttm_mem_evict_fist may wait for unreserve IIRC (the -EBUSY
>>>> return from ttm_bo_reserve_locked) is not propagated back.
>>>>         
>>> The code is not straightforward but if no_wait is true the
>>> -EBUSY of ttm_bo_reserve_locked will be propagated back.
>>>       
>> The point is that we don't want to set no_wait to true, because it's
>> OK to wait for GPU. We want to add an extra argument
>> no_wait_reserve.
>>
>> /Thomas
>>
>>     
>
> My patchset doesn't change the code there, no_wait will be true
> only if it's true as argument of ttm_bo_validate from the ttm
> fault callback in the driver.
>
>   

But you did call ttm_bo_move_buffer from the fault callback?
 Given the above, it's illegal to call it with no_wait set to false, and 
undesirable to call it with no_wait set to true.

/Thomas


> Cheers,
> Jerome
>   


------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

Re: Unmappable VRAM patchset V3

[Jerome Glisse]

On Wed, Feb 24, 2010 at 06:04:57PM +0100, Thomas Hellstrom wrote:
> Jerome Glisse wrote:
> >On Wed, Feb 24, 2010 at 01:37:42PM +0100, Thomas Hellstrom wrote:
> >>Jerome Glisse wrote:
> >>>On Tue, Feb 23, 2010 at 02:05:50PM +0100, Thomas Hellstrom wrote:
> >>>>Jerome Glisse wrote:
> >>>>>On Mon, Feb 22, 2010 at 08:58:28PM +0100, Thomas Hellstrom wrote:
> >>>>>>Jerome Glisse wrote:
> >>>>>>>On Mon, Feb 22, 2010 at 06:30:24PM +0100, Thomas Hellstrom wrote:
> >>>>>>>>Jerome Glisse wrote:
> >>>>>>>>>Thomas i think i addressed your concern here, the ttm_bo_validate
> >>>>>>>>>didn't needed a new argument or i did not understand what was
> >>>>>>>>>necessary beside no_wait. In this patchset we check the value
> >>>>>>>>>of callback in case of EBUSY (call set_need_resched) or ERESTARTSYS
> >>>>>>>>>we return VM_FAULT_NOPAGE.
> >>>>>>>>Well, if we from the fault callback call any function that might
> >>>>>>>>call ttm_bo_reserve or ttm_bo_reserve_locked, we must make sure
> >>>>>>>>that we never wait, but return -EBUSY all the way back to the
> >>>>>>>>fault function. Such a case may be ttm_bo_validate that calls
> >>>>>>>>ttm_bo_evict_first, or something causing a swapout...
> >>>>>>>>ttm_bo_validate currently doesn't have that functionality,
> >>>>>>>>because @no_wait just means don't wait for GPU.
> >>>>>>>What do you mean by never wait ? Is this GPU wait ? or CPU wait ie don't
> >>>>>>>use mutex or kernel code path that might sleep ?
> >>>>>>I mean waiting while reserving a bo. (If another thread has the bo
> >>>>>>reserved).
> >>>>>>
> >>>>>>>After a new review i don't think we ever wait for the GPU with the current
> >>>>>>>patch and as far as i can tell we will return EBUSY or ERESTART all the
> >>>>>>>way up.
> >>>>>>>
> >>>>>>>Cheers,
> >>>>>>>Jerome
> >>>>>>If there is *no* code path trying to reserve a bo or create a
> >>>>>>user-space visible object from within the fault handler, it should
> >>>>>>be ok.
> >>>>>>
> >>>>>>/Thomas
> >>>>>>
> >>>>>>
> >>>>>Did a new review again here is the call chain :
> >>>>>ttm_bo_move_buffer
> >>>>>ttm_bo_mem_space
> >>>>>  ttm_bo_mem_force_space
> >>>>>    ttm_mem_evict_first
> >>>>>      ttm_bo_reserve_locked (no_wait = true)
> >>>>Here ttm_mem_evict_fist may wait for unreserve IIRC (the -EBUSY
> >>>>return from ttm_bo_reserve_locked) is not propagated back.
> >>>The code is not straightforward but if no_wait is true the
> >>>-EBUSY of ttm_bo_reserve_locked will be propagated back.
> >>The point is that we don't want to set no_wait to true, because it's
> >>OK to wait for GPU. We want to add an extra argument
> >>no_wait_reserve.
> >>
> >>/Thomas
> >>
> >
> >My patchset doesn't change the code there, no_wait will be true
> >only if it's true as argument of ttm_bo_validate from the ttm
> >fault callback in the driver.
> >
> 
> But you did call ttm_bo_move_buffer from the fault callback?
> Given the above, it's illegal to call it with no_wait set to false,
> and undesirable to call it with no_wait set to true.
> 
> /Thomas
> 

I only call ttm_bo_validate from the fault callback, to validate
the bo into visible vram. I don't see why it would be undesirable
to call it with no_wait = true ?

Correct me if i am wrong but what we want is to try to move bo
without waiting at all on referencing other bo to avoid deadlock,
thus calling ttm_bo_validate with no_wait=true will return to
us with -EBUSY or -ERESTARTSYS if a bo we need to reserve to
move it away is already reserved, in this case we return to
the ttm fault handler which will return VM_FAULT_NOPAGE

The no_wait argument of ttm_bo_validate is pretty much only
use as an argument for reserving other bo in case we need
to evict or as an argument to ttm_bo_wait if we have to
wait for the bo. I can understand that we would like to
permit waiting on the bo we are faulting on, this is the
only place where i can understand having a different value
for validating.

Cheers,
Jerome

------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--