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