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
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];
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); }
/*
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); };
/**
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);
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
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
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@lists.freedesktop.org
-
Daniel Vetter -
Davidlohr Bueso -
Gerd Hoffmann -
Thomas Zimmermann