[PATCH v3 0/3] Implement lazy unmapping for GEM VRAM buffers

[PATCH v3 0/3] Implement lazy unmapping for GEM VRAM buffers

[Thomas Zimmermann]

(was: ast, mgag200: Map console BO while it's being displayed)

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 of the patch set, this problem is being solved by lazyly unmapping
the buffer as suggested by Gerd. Unmapping with drm_gem_vram_kunmap() only
changes a reference counter. VRAM helpers only 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().

v3:
	* implement lazy unmapping
v2:
      	* fixed comment typos

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

Thomas Zimmermann (3):
  drm/vram: Add kmap ref-counting to GEM VRAM objects
  drm/vram: Add infrastructure for move_notify()
  drm/vram: Implement lazy unmapping for GEM VRAM buffers

 drivers/gpu/drm/drm_gem_vram_helper.c | 112 +++++++++++++++++++++-----
 drivers/gpu/drm/drm_vram_mm_helper.c  |  12 +++
 include/drm/drm_gem_vram_helper.h     |  23 ++++++
 include/drm/drm_vram_mm_helper.h      |   4 +
 4 files changed, 130 insertions(+), 21 deletions(-)

--
2.23.0

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

[PATCH v3 1/3] 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.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 74 ++++++++++++++++++++-------
 include/drm/drm_gem_vram_helper.h     | 19 +++++++
 2 files changed, 75 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..6c7912092876 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -26,7 +26,11 @@ 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);
+	mutex_destroy(&gbo->kmap_lock);
 }
 
 static void drm_gem_vram_destroy(struct drm_gem_vram_object *gbo)
@@ -100,6 +104,8 @@ static int drm_gem_vram_init(struct drm_device *dev,
 	if (ret)
 		goto err_drm_gem_object_release;
 
+	mutex_init(&gbo->kmap_lock);
+
 	return 0;
 
 err_drm_gem_object_release:
@@ -283,6 +289,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 +338,44 @@ 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 = mutex_lock_interruptible(&gbo->kmap_lock);
 	if (ret)
 		return ERR_PTR(ret);
+	virtual = drm_gem_vram_kmap_locked(gbo, map, is_iomem);
+	mutex_unlock(&gbo->kmap_lock);
 
-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)
+{
+	mutex_lock(&gbo->kmap_lock);
+	drm_gem_vram_kunmap_locked(gbo);
+	mutex_unlock(&gbo->kmap_lock);
+}
 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..8c08bc87b788 100644
--- a/include/drm/drm_gem_vram_helper.h
+++ b/include/drm/drm_gem_vram_helper.h
@@ -34,11 +34,30 @@ 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_lock: Protects the kmap address and use count
+	 */
+	struct mutex kmap_lock;
+
+	/**
+	 * @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 v3 2/3] 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 v3 3/3] 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.

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

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index 6c7912092876..973c703534d4 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -352,18 +352,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().
+	 */
 }
 
 /**
@@ -489,6 +488,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
  *
@@ -498,7 +529,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 8c08bc87b788..e5ef0e4ab2e4 100644
--- a/include/drm/drm_gem_vram_helper.h
+++ b/include/drm/drm_gem_vram_helper.h
@@ -117,6 +117,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 v3 1/3] drm/vram: Add kmap ref-counting to GEM VRAM objects

[Daniel Vetter]

On Fri, Sep 06, 2019 at 10:52:12AM +0200, Thomas Zimmermann wrote:
> 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.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Davidlohr Bueso <dave@stgolabs.net>
> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  drivers/gpu/drm/drm_gem_vram_helper.c | 74 ++++++++++++++++++++-------
>  include/drm/drm_gem_vram_helper.h     | 19 +++++++
>  2 files changed, 75 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..6c7912092876 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -26,7 +26,11 @@ 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);
> +	mutex_destroy(&gbo->kmap_lock);
>  }
>  
>  static void drm_gem_vram_destroy(struct drm_gem_vram_object *gbo)
> @@ -100,6 +104,8 @@ static int drm_gem_vram_init(struct drm_device *dev,
>  	if (ret)
>  		goto err_drm_gem_object_release;
>  
> +	mutex_init(&gbo->kmap_lock);
> +
>  	return 0;
>  
>  err_drm_gem_object_release:
> @@ -283,6 +289,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 +338,44 @@ 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 = mutex_lock_interruptible(&gbo->kmap_lock);
>  	if (ret)
>  		return ERR_PTR(ret);
> +	virtual = drm_gem_vram_kmap_locked(gbo, map, is_iomem);
> +	mutex_unlock(&gbo->kmap_lock);
>  
> -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)
> +{
> +	mutex_lock(&gbo->kmap_lock);
> +	drm_gem_vram_kunmap_locked(gbo);
> +	mutex_unlock(&gbo->kmap_lock);
> +}
>  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..8c08bc87b788 100644
> --- a/include/drm/drm_gem_vram_helper.h
> +++ b/include/drm/drm_gem_vram_helper.h
> @@ -34,11 +34,30 @@ 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_lock: Protects the kmap address and use count
> +	 */
> +	struct mutex kmap_lock;

Isn't the locking here a bit racy: The ttm side is protected by the
dma_resv ww_mutex, but you have your own lock here. So if you race  kmap
on one side (from fbdev) with a modeset that evicts stuff (from ttm)
things go boom.

I think simpler to just reuse the ttm bo lock (reserve/unreserve). It's
atm still a bit special, but Christian König has plans to make
reserve/unreserve really nothing more than dma_resv_lock/unlock.
-Daniel

> +
> +	/**
> +	 * @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
> 

-- 
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 v3 2/3] drm/vram: Add infrastructure for move_notify()

[Daniel Vetter]

On Fri, Sep 06, 2019 at 10:52:13AM +0200, Thomas Zimmermann wrote:
> 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);

Is this indirection really worth it? We have a grand total of 1 driver
which isn't using gem (vmwgfx), and I don't think that one will ever
switch over to vram helpers.

I'd fold that all in. Helpers don't need to be flexible enough to support
every possible use-case (that's just the job of the core), they can be
opinionated to get cleaner code for most cases.

For this case here if you fold this in you can unexport 3 EXPORT_SYMBOLs
(4 with this patch here), which I think is a neat simplification of the
complexity exposed to driver writers. Just put the necessary declarations
into a drm_vram_helper_internal.h so that drivers don't get at it by
accident (like the other drm*internal.h helpers we have).
-Daniel

>  };
>  
>  /**
> -- 
> 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 v3 3/3] drm/vram: Implement lazy unmapping for GEM VRAM buffers

[Gerd Hoffmann]

> +void drm_gem_vram_bo_driver_move_notify(struct ttm_buffer_object *bo,
> +					bool evict,
> +					struct ttm_mem_reg *new_mem)
> +{
[ ... ]
> +	if (!kmap->virtual)
> +		return;
> +	ttm_bo_kunmap(kmap);
> +	kmap->virtual = NULL;
> +}

I think ttm_buffer_object_destroy() needs "if (kmap->virtual)
ttm_bo_kunmap()" too, due to the lazy unmap you can land there
with an active mapping.

cheers,
  Gerd

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

Re: [PATCH v3 2/3] drm/vram: Add infrastructure for move_notify()

[Thomas Zimmermann]

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

Hi

Am 06.09.19 um 11:28 schrieb Daniel Vetter:
> On Fri, Sep 06, 2019 at 10:52:13AM +0200, Thomas Zimmermann wrote:
>> 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);
> 
> Is this indirection really worth it? We have a grand total of 1 driver
> which isn't using gem (vmwgfx), and I don't think that one will ever
> switch over to vram helpers.
> 
> I'd fold that all in. Helpers don't need to be flexible enough to support
> every possible use-case (that's just the job of the core), they can be
> opinionated to get cleaner code for most cases.
> 

The original idea was to make this as flexible as possible; probably
more flexible than necessary. I wouldn't mind merging everything into
one file, but please not in this patch set, can we keep it for now and I
send you a cleanup next?

Best regards
Thomas

> For this case here if you fold this in you can unexport 3 EXPORT_SYMBOLs
> (4 with this patch here), which I think is a neat simplification of the
> complexity exposed to driver writers. Just put the necessary declarations
> into a drm_vram_helper_internal.h so that drivers don't get at it by
> accident (like the other drm*internal.h helpers we have).
> -Daniel
> 
>>  };
>>  
>>  /**
>> -- 
>> 2.23.0
>>
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

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

Re: [PATCH v3 3/3] drm/vram: Implement lazy unmapping for GEM VRAM buffers

[Thomas Zimmermann]

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

Hi

Am 06.09.19 um 11:39 schrieb Gerd Hoffmann:
>> +void drm_gem_vram_bo_driver_move_notify(struct ttm_buffer_object *bo,
>> +					bool evict,
>> +					struct ttm_mem_reg *new_mem)
>> +{
> [ ... ]
>> +	if (!kmap->virtual)
>> +		return;
>> +	ttm_bo_kunmap(kmap);
>> +	kmap->virtual = NULL;
>> +}
> 
> I think ttm_buffer_object_destroy() needs "if (kmap->virtual)
> ttm_bo_kunmap()" too, due to the lazy unmap you can land there
> with an active mapping.

Hmm, from my understanding, that final call to move_notify() is done by
ttm_bo_cleanup_memtype_use(), which is called from within ttm_bo_put().

If we want to add a final kunmap, There's also the release_notify()
callback, which is probably the right place to put it.

Best regards
Thomas

> 
> cheers,
>   Gerd
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

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

Re: [PATCH v3 3/3] drm/vram: Implement lazy unmapping for GEM VRAM buffers

[Gerd Hoffmann]

On Fri, Sep 06, 2019 at 12:37:47PM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 06.09.19 um 11:39 schrieb Gerd Hoffmann:
> >> +void drm_gem_vram_bo_driver_move_notify(struct ttm_buffer_object *bo,
> >> +					bool evict,
> >> +					struct ttm_mem_reg *new_mem)
> >> +{
> > [ ... ]
> >> +	if (!kmap->virtual)
> >> +		return;
> >> +	ttm_bo_kunmap(kmap);
> >> +	kmap->virtual = NULL;
> >> +}
> > 
> > I think ttm_buffer_object_destroy() needs "if (kmap->virtual)
> > ttm_bo_kunmap()" too, due to the lazy unmap you can land there
> > with an active mapping.
> 
> Hmm, from my understanding, that final call to move_notify() is done by
> ttm_bo_cleanup_memtype_use(), which is called from within ttm_bo_put().

Ah, good, sounds like this should work indeed.
Maybe add WARN_ON(kmap->virtual), just to be sure we don't overlooked something.

cheers,
  Gerd

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

Re: [PATCH v3 2/3] drm/vram: Add infrastructure for move_notify()

[Daniel Vetter]

On Fri, Sep 6, 2019 at 12:24 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 06.09.19 um 11:28 schrieb Daniel Vetter:
> > On Fri, Sep 06, 2019 at 10:52:13AM +0200, Thomas Zimmermann wrote:
> >> 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);
> >
> > Is this indirection really worth it? We have a grand total of 1 driver
> > which isn't using gem (vmwgfx), and I don't think that one will ever
> > switch over to vram helpers.
> >
> > I'd fold that all in. Helpers don't need to be flexible enough to support
> > every possible use-case (that's just the job of the core), they can be
> > opinionated to get cleaner code for most cases.
> >
>
> The original idea was to make this as flexible as possible; probably
> more flexible than necessary. I wouldn't mind merging everything into
> one file, but please not in this patch set, can we keep it for now and I
> send you a cleanup next?

Oh sure, I just wondered about why this flexibility exists and
realized there's not really a user for it. And pondering this more I
didn't come up with a use-case where it might reasonably be needed,
and you don't want to roll your own complete, and couldn't come up
with anything. Aside from the locking question on patch 1 I think this
looks like a really tidy solution for the fbdev mapping issue, happy
to ack once we've figured out the locking thing.
-Daniel

>
> Best regards
> Thomas
>
> > For this case here if you fold this in you can unexport 3 EXPORT_SYMBOLs
> > (4 with this patch here), which I think is a neat simplification of the
> > complexity exposed to driver writers. Just put the necessary declarations
> > into a drm_vram_helper_internal.h so that drivers don't get at it by
> > accident (like the other drm*internal.h helpers we have).
> > -Daniel
> >
> >>  };
> >>
> >>  /**
> >> --
> >> 2.23.0
> >>
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
> HRB 21284 (AG Nürnberg)
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 2/3] drm/vram: Add infrastructure for move_notify()

[Thomas Zimmermann]

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

Hi

Am 06.09.19 um 15:05 schrieb Daniel Vetter:
> On Fri, Sep 6, 2019 at 12:24 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Hi
>>
>> Am 06.09.19 um 11:28 schrieb Daniel Vetter:
>>> On Fri, Sep 06, 2019 at 10:52:13AM +0200, Thomas Zimmermann wrote:
>>>> 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);
>>>
>>> Is this indirection really worth it? We have a grand total of 1 driver
>>> which isn't using gem (vmwgfx), and I don't think that one will ever
>>> switch over to vram helpers.
>>>
>>> I'd fold that all in. Helpers don't need to be flexible enough to support
>>> every possible use-case (that's just the job of the core), they can be
>>> opinionated to get cleaner code for most cases.
>>>
>>
>> The original idea was to make this as flexible as possible; probably
>> more flexible than necessary. I wouldn't mind merging everything into
>> one file, but please not in this patch set, can we keep it for now and I
>> send you a cleanup next?
> 
> Oh sure, I just wondered about why this flexibility exists and
> realized there's not really a user for it. And pondering this more I
> didn't come up with a use-case where it might reasonably be needed,
> and you don't want to roll your own complete, and couldn't come up
> with anything. Aside from the locking question on patch 1 I think this
> looks like a really tidy solution for the fbdev mapping issue, happy
> to ack once we've figured out the locking thing.

Great. There's a v4 of the patch set that should resolve the locking
problem.

Best regards
Thomas

> -Daniel
> 
>>
>> Best regards
>> Thomas
>>
>>> For this case here if you fold this in you can unexport 3 EXPORT_SYMBOLs
>>> (4 with this patch here), which I think is a neat simplification of the
>>> complexity exposed to driver writers. Just put the necessary declarations
>>> into a drm_vram_helper_internal.h so that drivers don't get at it by
>>> accident (like the other drm*internal.h helpers we have).
>>> -Daniel
>>>
>>>>  };
>>>>
>>>>  /**
>>>> --
>>>> 2.23.0
>>>>
>>>
>>
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
>> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
>> HRB 21284 (AG Nürnberg)
>>
> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

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