[PATCH v4 0/4] Implement lazy unmapping for GEM VRAM buffers

[PATCH v4 0/4] Implement lazy unmapping for GEM VRAM buffers

[Thomas Zimmermann]

Generic fbdev emulation maps and unmaps the console BO for updating it's
content from the shadow buffer. If this involves an actual mapping
operation (instead of reusing an existing mapping), lots of debug messages
may be printed, such as

  x86/PAT: Overlap at 0xd0000000-0xd1000000
  x86/PAT: reserve_memtype added [mem 0xd0000000-0xd02fffff], track write-combining, req write-combining, ret write-combining
  x86/PAT: free_memtype request [mem 0xd0000000-0xd02fffff]

as reported at [1]. Drivers using VRAM helpers may also see reduced
performance as the mapping operations can create overhead.

In v3 and later of the patch set, this problem is being solved by lazily
unmapping the buffer as suggested by Gerd. Unmapping with drm_gem_vram_kunmap()
only changes a reference counter. VRAM helpers later perform the unmapping
operation when TTM evicts the buffer object from its current location. If
the buffer is never evicted, the existing mapping is reused by later calls
to drm_gem_vram_kmap().

v4:
	* lock kmap with ttm_bo_reserve()
	* acquire lock only once for vmap()
	* warn about stale mappings during buffer cleanup
v3:
      	* implement lazy unmapping
v2:
	* fixed comment typos

[1] https://lists.freedesktop.org/archives/dri-devel/2019-September/234308.html

Thomas Zimmermann (4):
  drm/vram: Add kmap ref-counting to GEM VRAM objects
  drm/vram: Acquire lock only once per call to vmap()/vunmap()
  drm/vram: Add infrastructure for move_notify()
  drm/vram: Implement lazy unmapping for GEM VRAM buffers

 drivers/gpu/drm/drm_gem_vram_helper.c | 229 ++++++++++++++++++--------
 drivers/gpu/drm/drm_vram_mm_helper.c  |  12 ++
 include/drm/drm_gem_vram_helper.h     |  18 ++
 include/drm/drm_vram_mm_helper.h      |   4 +
 4 files changed, 198 insertions(+), 65 deletions(-)

--
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4 1/4] drm/vram: Add kmap ref-counting to GEM VRAM objects

[Thomas Zimmermann]

The kmap and kunmap operations of GEM VRAM buffers can now be called
in interleaving pairs. The first call to drm_gem_vram_kmap() maps the
buffer's memory to kernel address space and the final call to
drm_gem_vram_kunmap() unmaps the memory. Intermediate calls to these
functions increment or decrement a reference counter.

This change allows for keeping buffer memory mapped for longer and
minimizes the amount of changes to TLB, page tables, etc.

v4:
	* lock in kmap()/kunmap() with ttm_bo_reserve()

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 75 ++++++++++++++++++++-------
 include/drm/drm_gem_vram_helper.h     | 14 +++++
 2 files changed, 71 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index fd751078bae1..5e86ec06644b 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -26,6 +26,9 @@ static void drm_gem_vram_cleanup(struct drm_gem_vram_object *gbo)
 	 * TTM buffer object in 'bo' has already been cleaned
 	 * up; only release the GEM object.
 	 */
+
+	WARN_ON(gbo->kmap_use_count);
+
 	drm_gem_object_release(&gbo->bo.base);
 }
 
@@ -283,6 +286,34 @@ int drm_gem_vram_unpin(struct drm_gem_vram_object *gbo)
 }
 EXPORT_SYMBOL(drm_gem_vram_unpin);
 
+static void *drm_gem_vram_kmap_locked(struct drm_gem_vram_object *gbo,
+				      bool map, bool *is_iomem)
+{
+	int ret;
+	struct ttm_bo_kmap_obj *kmap = &gbo->kmap;
+
+	if (gbo->kmap_use_count > 0)
+		goto out;
+
+	if (kmap->virtual || !map)
+		goto out;
+
+	ret = ttm_bo_kmap(&gbo->bo, 0, gbo->bo.num_pages, kmap);
+	if (ret)
+		return ERR_PTR(ret);
+
+out:
+	if (!kmap->virtual) {
+		if (is_iomem)
+			*is_iomem = false;
+		return NULL; /* not mapped; don't increment ref */
+	}
+	++gbo->kmap_use_count;
+	if (is_iomem)
+		return ttm_kmap_obj_virtual(kmap, is_iomem);
+	return kmap->virtual;
+}
+
 /**
  * drm_gem_vram_kmap() - Maps a GEM VRAM object into kernel address space
  * @gbo:	the GEM VRAM object
@@ -304,40 +335,48 @@ void *drm_gem_vram_kmap(struct drm_gem_vram_object *gbo, bool map,
 			bool *is_iomem)
 {
 	int ret;
-	struct ttm_bo_kmap_obj *kmap = &gbo->kmap;
-
-	if (kmap->virtual || !map)
-		goto out;
+	void *virtual;
 
-	ret = ttm_bo_kmap(&gbo->bo, 0, gbo->bo.num_pages, kmap);
+	ret = ttm_bo_reserve(&gbo->bo, true, false, NULL);
 	if (ret)
 		return ERR_PTR(ret);
+	virtual = drm_gem_vram_kmap_locked(gbo, map, is_iomem);
+	ttm_bo_unreserve(&gbo->bo);
 
-out:
-	if (!is_iomem)
-		return kmap->virtual;
-	if (!kmap->virtual) {
-		*is_iomem = false;
-		return NULL;
-	}
-	return ttm_kmap_obj_virtual(kmap, is_iomem);
+	return virtual;
 }
 EXPORT_SYMBOL(drm_gem_vram_kmap);
 
-/**
- * drm_gem_vram_kunmap() - Unmaps a GEM VRAM object
- * @gbo:	the GEM VRAM object
- */
-void drm_gem_vram_kunmap(struct drm_gem_vram_object *gbo)
+static void drm_gem_vram_kunmap_locked(struct drm_gem_vram_object *gbo)
 {
 	struct ttm_bo_kmap_obj *kmap = &gbo->kmap;
 
+	if (WARN_ON_ONCE(!gbo->kmap_use_count))
+		return;
+	if (--gbo->kmap_use_count > 0)
+		return;
+
 	if (!kmap->virtual)
 		return;
 
 	ttm_bo_kunmap(kmap);
 	kmap->virtual = NULL;
 }
+
+/**
+ * drm_gem_vram_kunmap() - Unmaps a GEM VRAM object
+ * @gbo:	the GEM VRAM object
+ */
+void drm_gem_vram_kunmap(struct drm_gem_vram_object *gbo)
+{
+	int ret;
+
+	ret = ttm_bo_reserve(&gbo->bo, false, false, NULL);
+	if (WARN_ONCE(ret, "ttm_bo_reserve_failed(): ret=%d\n", ret))
+		return;
+	drm_gem_vram_kunmap_locked(gbo);
+	ttm_bo_unreserve(&gbo->bo);
+}
 EXPORT_SYMBOL(drm_gem_vram_kunmap);
 
 /**
diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h
index ac217d768456..4f0e207ee097 100644
--- a/include/drm/drm_gem_vram_helper.h
+++ b/include/drm/drm_gem_vram_helper.h
@@ -34,11 +34,25 @@ struct vm_area_struct;
  * backed by VRAM. It can be used for simple framebuffer devices with
  * dedicated memory. The buffer object can be evicted to system memory if
  * video memory becomes scarce.
+ *
+ * GEM VRAM objects perform reference counting for pin and mapping
+ * operations. So a buffer object that has been pinned N times with
+ * drm_gem_vram_pin() must be unpinned N times with
+ * drm_gem_vram_unpin(). The same applies to pairs of
+ * drm_gem_vram_kmap() and drm_gem_vram_kunmap().
  */
 struct drm_gem_vram_object {
 	struct ttm_buffer_object bo;
 	struct ttm_bo_kmap_obj kmap;
 
+	/**
+	 * @kmap_use_count:
+	 *
+	 * Reference count on the virtual address.
+	 * The address are un-mapped when the count reaches zero.
+	 */
+	unsigned int kmap_use_count;
+
 	/* Supported placements are %TTM_PL_VRAM and %TTM_PL_SYSTEM */
 	struct ttm_placement placement;
 	struct ttm_place placements[2];
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4 2/4] drm/vram: Acquire lock only once per call to vmap()/vunmap()

[Thomas Zimmermann]

The implementation of vmap() is a combined pin() and kmap(). As both
functions share the same lock, we can make vmap() slightly faster by
acquiring the lock only once for both operations. Same for the inverse,
vunmap().

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 119 ++++++++++++++++----------
 1 file changed, 73 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index 5e86ec06644b..1e17f11cc7b9 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -195,30 +195,12 @@ s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo)
 }
 EXPORT_SYMBOL(drm_gem_vram_offset);
 
-/**
- * drm_gem_vram_pin() - Pins a GEM VRAM object in a region.
- * @gbo:	the GEM VRAM object
- * @pl_flag:	a bitmask of possible memory regions
- *
- * Pinning a buffer object ensures that it is not evicted from
- * a memory region. A pinned buffer object has to be unpinned before
- * it can be pinned to another region. If the pl_flag argument is 0,
- * the buffer is pinned at its current location (video RAM or system
- * memory).
- *
- * Returns:
- * 0 on success, or
- * a negative error code otherwise.
- */
-int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long pl_flag)
+static int drm_gem_vram_pin_locked(struct drm_gem_vram_object *gbo,
+				   unsigned long pl_flag)
 {
 	int i, ret;
 	struct ttm_operation_ctx ctx = { false, false };
 
-	ret = ttm_bo_reserve(&gbo->bo, true, false, NULL);
-	if (ret < 0)
-		return ret;
-
 	if (gbo->pin_count)
 		goto out;
 
@@ -230,58 +212,83 @@ int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long pl_flag)
 
 	ret = ttm_bo_validate(&gbo->bo, &gbo->placement, &ctx);
 	if (ret < 0)
-		goto err_ttm_bo_unreserve;
+		return ret;
 
 out:
 	++gbo->pin_count;
-	ttm_bo_unreserve(&gbo->bo);
 
 	return 0;
-
-err_ttm_bo_unreserve:
-	ttm_bo_unreserve(&gbo->bo);
-	return ret;
 }
-EXPORT_SYMBOL(drm_gem_vram_pin);
 
 /**
- * drm_gem_vram_unpin() - Unpins a GEM VRAM object
+ * drm_gem_vram_pin() - Pins a GEM VRAM object in a region.
  * @gbo:	the GEM VRAM object
+ * @pl_flag:	a bitmask of possible memory regions
+ *
+ * Pinning a buffer object ensures that it is not evicted from
+ * a memory region. A pinned buffer object has to be unpinned before
+ * it can be pinned to another region. If the pl_flag argument is 0,
+ * the buffer is pinned at its current location (video RAM or system
+ * memory).
  *
  * Returns:
  * 0 on success, or
  * a negative error code otherwise.
  */
-int drm_gem_vram_unpin(struct drm_gem_vram_object *gbo)
+int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long pl_flag)
 {
-	int i, ret;
-	struct ttm_operation_ctx ctx = { false, false };
+	int ret;
 
 	ret = ttm_bo_reserve(&gbo->bo, true, false, NULL);
-	if (ret < 0)
+	if (ret)
 		return ret;
+	ret = drm_gem_vram_pin_locked(gbo, pl_flag);
+	ttm_bo_unreserve(&gbo->bo);
+
+	return ret;
+}
+EXPORT_SYMBOL(drm_gem_vram_pin);
+
+static int drm_gem_vram_unpin_locked(struct drm_gem_vram_object *gbo)
+{
+	int i, ret;
+	struct ttm_operation_ctx ctx = { false, false };
 
 	if (WARN_ON_ONCE(!gbo->pin_count))
-		goto out;
+		return 0;
 
 	--gbo->pin_count;
 	if (gbo->pin_count)
-		goto out;
+		return 0;
 
 	for (i = 0; i < gbo->placement.num_placement ; ++i)
 		gbo->placements[i].flags &= ~TTM_PL_FLAG_NO_EVICT;
 
 	ret = ttm_bo_validate(&gbo->bo, &gbo->placement, &ctx);
 	if (ret < 0)
-		goto err_ttm_bo_unreserve;
-
-out:
-	ttm_bo_unreserve(&gbo->bo);
+		return ret;
 
 	return 0;
+}
 
-err_ttm_bo_unreserve:
+/**
+ * drm_gem_vram_unpin() - Unpins a GEM VRAM object
+ * @gbo:	the GEM VRAM object
+ *
+ * Returns:
+ * 0 on success, or
+ * a negative error code otherwise.
+ */
+int drm_gem_vram_unpin(struct drm_gem_vram_object *gbo)
+{
+	int ret;
+
+	ret = ttm_bo_reserve(&gbo->bo, true, false, NULL);
+	if (ret)
+		return ret;
+	ret = drm_gem_vram_unpin_locked(gbo);
 	ttm_bo_unreserve(&gbo->bo);
+
 	return ret;
 }
 EXPORT_SYMBOL(drm_gem_vram_unpin);
@@ -637,15 +644,28 @@ static void *drm_gem_vram_object_vmap(struct drm_gem_object *gem)
 	int ret;
 	void *base;
 
-	ret = drm_gem_vram_pin(gbo, 0);
+	ret = ttm_bo_reserve(&gbo->bo, true, false, NULL);
 	if (ret)
-		return NULL;
-	base = drm_gem_vram_kmap(gbo, true, NULL);
+		return ERR_PTR(ret);
+
+	ret = drm_gem_vram_pin_locked(gbo, 0);
+	if (ret)
+		goto err_ttm_bo_unreserve;
+	base = drm_gem_vram_kmap_locked(gbo, true, NULL);
 	if (IS_ERR(base)) {
-		drm_gem_vram_unpin(gbo);
-		return NULL;
+		ret = PTR_ERR(base);
+		goto err_drm_gem_vram_unpin_locked;
 	}
+
+	ttm_bo_unreserve(&gbo->bo);
+
 	return base;
+
+err_drm_gem_vram_unpin_locked:
+	drm_gem_vram_unpin_locked(gbo);
+err_ttm_bo_unreserve:
+	ttm_bo_unreserve(&gbo->bo);
+	return ERR_PTR(ret);
 }
 
 /**
@@ -658,9 +678,16 @@ static void drm_gem_vram_object_vunmap(struct drm_gem_object *gem,
 				       void *vaddr)
 {
 	struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem);
+	int ret;
 
-	drm_gem_vram_kunmap(gbo);
-	drm_gem_vram_unpin(gbo);
+	ret = ttm_bo_reserve(&gbo->bo, false, false, NULL);
+	if (WARN_ONCE(ret, "ttm_bo_reserve_failed(): ret=%d\n", ret))
+		return;
+
+	drm_gem_vram_kunmap_locked(gbo);
+	drm_gem_vram_unpin_locked(gbo);
+
+	ttm_bo_unreserve(&gbo->bo);
 }
 
 /*
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4 3/4] drm/vram: Add infrastructure for move_notify()

[Thomas Zimmermann]

This patch prepares VRAM helpers for lazy unmapping of buffer objects.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_vram_mm_helper.c | 12 ++++++++++++
 include/drm/drm_vram_mm_helper.h     |  4 ++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/drm_vram_mm_helper.c b/drivers/gpu/drm/drm_vram_mm_helper.c
index c911781d6728..31984690d5f3 100644
--- a/drivers/gpu/drm/drm_vram_mm_helper.c
+++ b/drivers/gpu/drm/drm_vram_mm_helper.c
@@ -98,6 +98,17 @@ static int bo_driver_verify_access(struct ttm_buffer_object *bo,
 	return vmm->funcs->verify_access(bo, filp);
 }
 
+static void bo_driver_move_notify(struct ttm_buffer_object *bo,
+				  bool evict,
+				  struct ttm_mem_reg *new_mem)
+{
+	struct drm_vram_mm *vmm = drm_vram_mm_of_bdev(bo->bdev);
+
+	if (!vmm->funcs || !vmm->funcs->move_notify)
+		return;
+	vmm->funcs->move_notify(bo, evict, new_mem);
+}
+
 static int bo_driver_io_mem_reserve(struct ttm_bo_device *bdev,
 				    struct ttm_mem_reg *mem)
 {
@@ -140,6 +151,7 @@ static struct ttm_bo_driver bo_driver = {
 	.eviction_valuable = ttm_bo_eviction_valuable,
 	.evict_flags = bo_driver_evict_flags,
 	.verify_access = bo_driver_verify_access,
+	.move_notify = bo_driver_move_notify,
 	.io_mem_reserve = bo_driver_io_mem_reserve,
 	.io_mem_free = bo_driver_io_mem_free,
 };
diff --git a/include/drm/drm_vram_mm_helper.h b/include/drm/drm_vram_mm_helper.h
index 2aacfb1ccfae..7fb8700f45fe 100644
--- a/include/drm/drm_vram_mm_helper.h
+++ b/include/drm/drm_vram_mm_helper.h
@@ -15,6 +15,8 @@ struct drm_device;
 	&ttm_bo_driver.evict_flags
  * @verify_access:	Provides an implementation for \
 	struct &ttm_bo_driver.verify_access
+ * @move_notify:	Provides an implementation for
+ *			struct &ttm_bo_driver.move_notify
  *
  * These callback function integrate VRAM MM with TTM buffer objects. New
  * functions can be added if necessary.
@@ -23,6 +25,8 @@ struct drm_vram_mm_funcs {
 	void (*evict_flags)(struct ttm_buffer_object *bo,
 			    struct ttm_placement *placement);
 	int (*verify_access)(struct ttm_buffer_object *bo, struct file *filp);
+	void (*move_notify)(struct ttm_buffer_object *bo, bool evict,
+			    struct ttm_mem_reg *new_mem);
 };
 
 /**
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4 4/4] drm/vram: Implement lazy unmapping for GEM VRAM buffers

[Thomas Zimmermann]

Frequent mapping and unmapping a buffer object adds overhead for
modifying the page table and creates debug output. Unmapping a buffer
is only required when the memory manager evicts the buffer from its
current location.

v4:
	* WARN_ON if buffer is still mapped during BO cleanup

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 49 ++++++++++++++++++++++-----
 include/drm/drm_gem_vram_helper.h     |  4 +++
 2 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index 1e17f11cc7b9..0990f0e03213 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -28,6 +28,7 @@ static void drm_gem_vram_cleanup(struct drm_gem_vram_object *gbo)
 	 */
 
 	WARN_ON(gbo->kmap_use_count);
+	WARN_ON(gbo->kmap.virtual);
 
 	drm_gem_object_release(&gbo->bo.base);
 }
@@ -356,18 +357,17 @@ EXPORT_SYMBOL(drm_gem_vram_kmap);
 
 static void drm_gem_vram_kunmap_locked(struct drm_gem_vram_object *gbo)
 {
-	struct ttm_bo_kmap_obj *kmap = &gbo->kmap;
-
 	if (WARN_ON_ONCE(!gbo->kmap_use_count))
 		return;
 	if (--gbo->kmap_use_count > 0)
 		return;
 
-	if (!kmap->virtual)
-		return;
-
-	ttm_bo_kunmap(kmap);
-	kmap->virtual = NULL;
+	/*
+	 * Permanently mapping and unmapping buffers adds overhead from
+	 * updating the page tables and creates debugging output. Therefore,
+	 * we delay the actual unmap operation until the BO gets evicted
+	 * from memory. See drm_gem_vram_bo_driver_move_notify().
+	 */
 }
 
 /**
@@ -497,6 +497,38 @@ int drm_gem_vram_bo_driver_verify_access(struct ttm_buffer_object *bo,
 }
 EXPORT_SYMBOL(drm_gem_vram_bo_driver_verify_access);
 
+/**
+ * drm_gem_vram_bo_driver_move_notify() -
+ *	Implements &struct ttm_bo_driver.move_notify
+ * @bo:		TTM buffer object. Refers to &struct drm_gem_vram_object.bo
+ * @evict:	True, if the BO is being evicted from graphics memory;
+ *		false otherwise.
+ * @new_mem:	New memory region, or NULL on destruction
+ */
+void drm_gem_vram_bo_driver_move_notify(struct ttm_buffer_object *bo,
+					bool evict,
+					struct ttm_mem_reg *new_mem)
+{
+	struct drm_gem_vram_object *gbo;
+	struct ttm_bo_kmap_obj *kmap;
+
+	/* TTM may pass BOs that are not GEM VRAM BOs. */
+	if (!drm_is_gem_vram(bo))
+		return;
+
+	gbo = drm_gem_vram_of_bo(bo);
+	kmap = &gbo->kmap;
+
+	if (WARN_ON_ONCE(gbo->kmap_use_count))
+		return;
+
+	if (!kmap->virtual)
+		return;
+	ttm_bo_kunmap(kmap);
+	kmap->virtual = NULL;
+}
+EXPORT_SYMBOL(drm_gem_vram_bo_driver_move_notify);
+
 /*
  * drm_gem_vram_mm_funcs - Functions for &struct drm_vram_mm
  *
@@ -506,7 +538,8 @@ EXPORT_SYMBOL(drm_gem_vram_bo_driver_verify_access);
  */
 const struct drm_vram_mm_funcs drm_gem_vram_mm_funcs = {
 	.evict_flags = drm_gem_vram_bo_driver_evict_flags,
-	.verify_access = drm_gem_vram_bo_driver_verify_access
+	.verify_access = drm_gem_vram_bo_driver_verify_access,
+	.move_notify = drm_gem_vram_bo_driver_move_notify,
 };
 EXPORT_SYMBOL(drm_gem_vram_mm_funcs);
 
diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h
index 4f0e207ee097..b47c46516466 100644
--- a/include/drm/drm_gem_vram_helper.h
+++ b/include/drm/drm_gem_vram_helper.h
@@ -112,6 +112,10 @@ int drm_gem_vram_fill_create_dumb(struct drm_file *file,
 void drm_gem_vram_bo_driver_evict_flags(struct ttm_buffer_object *bo,
 					struct ttm_placement *pl);
 
+void drm_gem_vram_bo_driver_move_notify(struct ttm_buffer_object *bo,
+					bool evict,
+					struct ttm_mem_reg *new_mem);
+
 int drm_gem_vram_bo_driver_verify_access(struct ttm_buffer_object *bo,
 					 struct file *filp);
 
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 0/4] Implement lazy unmapping for GEM VRAM buffers

[Daniel Vetter]

On Fri, Sep 06, 2019 at 02:20:52PM +0200, Thomas Zimmermann wrote:
> Generic fbdev emulation maps and unmaps the console BO for updating it's
> content from the shadow buffer. If this involves an actual mapping
> operation (instead of reusing an existing mapping), lots of debug messages
> may be printed, such as
> 
>   x86/PAT: Overlap at 0xd0000000-0xd1000000
>   x86/PAT: reserve_memtype added [mem 0xd0000000-0xd02fffff], track write-combining, req write-combining, ret write-combining
>   x86/PAT: free_memtype request [mem 0xd0000000-0xd02fffff]
> 
> as reported at [1]. Drivers using VRAM helpers may also see reduced
> performance as the mapping operations can create overhead.
> 
> In v3 and later of the patch set, this problem is being solved by lazily
> unmapping the buffer as suggested by Gerd. Unmapping with drm_gem_vram_kunmap()
> only changes a reference counter. VRAM helpers later perform the unmapping
> operation when TTM evicts the buffer object from its current location. If
> the buffer is never evicted, the existing mapping is reused by later calls
> to drm_gem_vram_kmap().
> 
> v4:
> 	* lock kmap with ttm_bo_reserve()
> 	* acquire lock only once for vmap()
> 	* warn about stale mappings during buffer cleanup
> v3:
>       	* implement lazy unmapping
> v2:
> 	* fixed comment typos
> 
> [1] https://lists.freedesktop.org/archives/dri-devel/2019-September/234308.html

On the series: Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-Daniel

> 
> Thomas Zimmermann (4):
>   drm/vram: Add kmap ref-counting to GEM VRAM objects
>   drm/vram: Acquire lock only once per call to vmap()/vunmap()
>   drm/vram: Add infrastructure for move_notify()
>   drm/vram: Implement lazy unmapping for GEM VRAM buffers
> 
>  drivers/gpu/drm/drm_gem_vram_helper.c | 229 ++++++++++++++++++--------
>  drivers/gpu/drm/drm_vram_mm_helper.c  |  12 ++
>  include/drm/drm_gem_vram_helper.h     |  18 ++
>  include/drm/drm_vram_mm_helper.h      |   4 +
>  4 files changed, 198 insertions(+), 65 deletions(-)
> 
> --
> 2.23.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 0/4] Implement lazy unmapping for GEM VRAM buffers

[Davidlohr Bueso]

On Fri, 06 Sep 2019, Thomas Zimmermann wrote:

>Generic fbdev emulation maps and unmaps the console BO for updating it's
>content from the shadow buffer. If this involves an actual mapping
>operation (instead of reusing an existing mapping), lots of debug messages
>may be printed, such as
>
>  x86/PAT: Overlap at 0xd0000000-0xd1000000
>  x86/PAT: reserve_memtype added [mem 0xd0000000-0xd02fffff], track write-combining, req write-combining, ret write-combining
>  x86/PAT: free_memtype request [mem 0xd0000000-0xd02fffff]
>
>as reported at [1]. Drivers using VRAM helpers may also see reduced
>performance as the mapping operations can create overhead.
>
>In v3 and later of the patch set, this problem is being solved by lazily
>unmapping the buffer as suggested by Gerd. Unmapping with drm_gem_vram_kunmap()
>only changes a reference counter. VRAM helpers later perform the unmapping
>operation when TTM evicts the buffer object from its current location. If
>the buffer is never evicted, the existing mapping is reused by later calls
>to drm_gem_vram_kmap().
>
>v4:
>	* lock kmap with ttm_bo_reserve()
>	* acquire lock only once for vmap()
>	* warn about stale mappings during buffer cleanup
>v3:
>      	* implement lazy unmapping
>v2:
>	* fixed comment typos
>
>[1] https://lists.freedesktop.org/archives/dri-devel/2019-September/234308.html
>
>Thomas Zimmermann (4):
>  drm/vram: Add kmap ref-counting to GEM VRAM objects
>  drm/vram: Acquire lock only once per call to vmap()/vunmap()
>  drm/vram: Add infrastructure for move_notify()
>  drm/vram: Implement lazy unmapping for GEM VRAM buffers
>
> drivers/gpu/drm/drm_gem_vram_helper.c | 229 ++++++++++++++++++--------
> drivers/gpu/drm/drm_vram_mm_helper.c  |  12 ++
> include/drm/drm_gem_vram_helper.h     |  18 ++
> include/drm/drm_vram_mm_helper.h      |   4 +
> 4 files changed, 198 insertions(+), 65 deletions(-)

Thanks for the prompt fix, feel free to add my:

Reported-and-tested-by: Davidlohr Bueso <dbueso@suse.de>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 0/4] Implement lazy unmapping for GEM VRAM buffers

[Gerd Hoffmann]

On Fri, Sep 06, 2019 at 02:20:52PM +0200, Thomas Zimmermann wrote:
> Generic fbdev emulation maps and unmaps the console BO for updating it's
> content from the shadow buffer. If this involves an actual mapping
> operation (instead of reusing an existing mapping), lots of debug messages
> may be printed, such as
> 
>   x86/PAT: Overlap at 0xd0000000-0xd1000000
>   x86/PAT: reserve_memtype added [mem 0xd0000000-0xd02fffff], track write-combining, req write-combining, ret write-combining
>   x86/PAT: free_memtype request [mem 0xd0000000-0xd02fffff]
> 
> as reported at [1]. Drivers using VRAM helpers may also see reduced
> performance as the mapping operations can create overhead.
> 
> In v3 and later of the patch set, this problem is being solved by lazily
> unmapping the buffer as suggested by Gerd. Unmapping with drm_gem_vram_kunmap()
> only changes a reference counter. VRAM helpers later perform the unmapping
> operation when TTM evicts the buffer object from its current location. If
> the buffer is never evicted, the existing mapping is reused by later calls
> to drm_gem_vram_kmap().
> 
> v4:
> 	* lock kmap with ttm_bo_reserve()
> 	* acquire lock only once for vmap()
> 	* warn about stale mappings during buffer cleanup
> v3:
>       	* implement lazy unmapping
> v2:
> 	* fixed comment typos
> 
> [1] https://lists.freedesktop.org/archives/dri-devel/2019-September/234308.html
> 
> Thomas Zimmermann (4):
>   drm/vram: Add kmap ref-counting to GEM VRAM objects
>   drm/vram: Acquire lock only once per call to vmap()/vunmap()
>   drm/vram: Add infrastructure for move_notify()
>   drm/vram: Implement lazy unmapping for GEM VRAM buffers

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

cheers,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel