[PATCH v2 02/20] drm/gem: convert to new unified vma manager
David Herrmann
dh.herrmann at gmail.com
Thu Jul 11 03:33:24 PDT 2013
Hi
On Sun, Jul 7, 2013 at 7:17 PM, David Herrmann <dh.herrmann at gmail.com> wrote:
> Use the new vma manager instead of the old hashtable. Also convert all
> drivers to use the new convenience helpers. This drops all the
> (map_list.hash.key << PAGE_SHIFT) non-sense.
>
> Locking and access-management is exactly the same as before with an
> additional lock inside of the vma-manager, which strictly wouldn't be
> needed for gem.
>
> v2:
> - rebase on drm-next
> - init nodes via drm_vma_node_reset() in drm_gem.c
I missed the tegra driver and any staging driver. I will fix that in
v3 and reduce the series to a minimum.
Regards
David
> Signed-off-by: David Herrmann <dh.herrmann at gmail.com>
> ---
> drivers/gpu/drm/drm_gem.c | 90 ++++++------------------------
> drivers/gpu/drm/drm_gem_cma_helper.c | 9 +--
> drivers/gpu/drm/exynos/exynos_drm_gem.c | 7 ++-
> drivers/gpu/drm/gma500/gem.c | 8 +--
> drivers/gpu/drm/i915/i915_gem.c | 9 +--
> drivers/gpu/drm/omapdrm/omap_gem.c | 11 ++--
> drivers/gpu/drm/omapdrm/omap_gem_helpers.c | 49 +---------------
> drivers/gpu/drm/udl/udl_gem.c | 6 +-
> include/drm/drmP.h | 7 +--
> include/drm/drm_vma_manager.h | 1 -
> include/uapi/drm/drm.h | 2 +-
> 11 files changed, 49 insertions(+), 150 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 603f256..b5db89b 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -37,6 +37,7 @@
> #include <linux/shmem_fs.h>
> #include <linux/dma-buf.h>
> #include <drm/drmP.h>
> +#include <drm/drm_vma_manager.h>
>
> /** @file drm_gem.c
> *
> @@ -102,14 +103,9 @@ drm_gem_init(struct drm_device *dev)
> }
>
> dev->mm_private = mm;
> -
> - if (drm_ht_create(&mm->offset_hash, 12)) {
> - kfree(mm);
> - return -ENOMEM;
> - }
> -
> - drm_mm_init(&mm->offset_manager, DRM_FILE_PAGE_OFFSET_START,
> - DRM_FILE_PAGE_OFFSET_SIZE);
> + drm_vma_offset_manager_init(&mm->vma_manager,
> + DRM_FILE_PAGE_OFFSET_START,
> + DRM_FILE_PAGE_OFFSET_SIZE);
>
> return 0;
> }
> @@ -119,8 +115,7 @@ drm_gem_destroy(struct drm_device *dev)
> {
> struct drm_gem_mm *mm = dev->mm_private;
>
> - drm_mm_takedown(&mm->offset_manager);
> - drm_ht_remove(&mm->offset_hash);
> + drm_vma_offset_manager_destroy(&mm->vma_manager);
> kfree(mm);
> dev->mm_private = NULL;
> }
> @@ -141,6 +136,7 @@ int drm_gem_object_init(struct drm_device *dev,
>
> kref_init(&obj->refcount);
> atomic_set(&obj->handle_count, 0);
> + drm_vma_node_reset(&obj->vma_node);
> obj->size = size;
>
> return 0;
> @@ -162,6 +158,7 @@ int drm_gem_private_object_init(struct drm_device *dev,
>
> kref_init(&obj->refcount);
> atomic_set(&obj->handle_count, 0);
> + drm_vma_node_reset(&obj->vma_node);
> obj->size = size;
>
> return 0;
> @@ -306,12 +303,8 @@ drm_gem_free_mmap_offset(struct drm_gem_object *obj)
> {
> struct drm_device *dev = obj->dev;
> struct drm_gem_mm *mm = dev->mm_private;
> - struct drm_map_list *list = &obj->map_list;
>
> - drm_ht_remove_item(&mm->offset_hash, &list->hash);
> - drm_mm_put_block(list->file_offset_node);
> - kfree(list->map);
> - list->map = NULL;
> + drm_vma_offset_remove(&mm->vma_manager, &obj->vma_node);
> }
> EXPORT_SYMBOL(drm_gem_free_mmap_offset);
>
> @@ -331,54 +324,9 @@ drm_gem_create_mmap_offset(struct drm_gem_object *obj)
> {
> struct drm_device *dev = obj->dev;
> struct drm_gem_mm *mm = dev->mm_private;
> - struct drm_map_list *list;
> - struct drm_local_map *map;
> - int ret;
> -
> - /* Set the object up for mmap'ing */
> - list = &obj->map_list;
> - list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL);
> - if (!list->map)
> - return -ENOMEM;
> -
> - map = list->map;
> - map->type = _DRM_GEM;
> - map->size = obj->size;
> - map->handle = obj;
> -
> - /* Get a DRM GEM mmap offset allocated... */
> - list->file_offset_node = drm_mm_search_free(&mm->offset_manager,
> - obj->size / PAGE_SIZE, 0, false);
> -
> - if (!list->file_offset_node) {
> - DRM_ERROR("failed to allocate offset for bo %d\n", obj->name);
> - ret = -ENOSPC;
> - goto out_free_list;
> - }
>
> - list->file_offset_node = drm_mm_get_block(list->file_offset_node,
> - obj->size / PAGE_SIZE, 0);
> - if (!list->file_offset_node) {
> - ret = -ENOMEM;
> - goto out_free_list;
> - }
> -
> - list->hash.key = list->file_offset_node->start;
> - ret = drm_ht_insert_item(&mm->offset_hash, &list->hash);
> - if (ret) {
> - DRM_ERROR("failed to add to map hash\n");
> - goto out_free_mm;
> - }
> -
> - return 0;
> -
> -out_free_mm:
> - drm_mm_put_block(list->file_offset_node);
> -out_free_list:
> - kfree(list->map);
> - list->map = NULL;
> -
> - return ret;
> + return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node,
> + obj->size / PAGE_SIZE);
> }
> EXPORT_SYMBOL(drm_gem_create_mmap_offset);
>
> @@ -707,8 +655,8 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> struct drm_file *priv = filp->private_data;
> struct drm_device *dev = priv->minor->dev;
> struct drm_gem_mm *mm = dev->mm_private;
> - struct drm_local_map *map = NULL;
> - struct drm_hash_item *hash;
> + struct drm_gem_object *obj;
> + struct drm_vma_offset_node *node;
> int ret = 0;
>
> if (drm_device_is_unplugged(dev))
> @@ -716,21 +664,15 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>
> mutex_lock(&dev->struct_mutex);
>
> - if (drm_ht_find_item(&mm->offset_hash, vma->vm_pgoff, &hash)) {
> + node = drm_vma_offset_exact_lookup(&mm->vma_manager, vma->vm_pgoff);
> + if (!node) {
> mutex_unlock(&dev->struct_mutex);
> return drm_mmap(filp, vma);
> }
>
> - map = drm_hash_entry(hash, struct drm_map_list, hash)->map;
> - if (!map ||
> - ((map->flags & _DRM_RESTRICTED) && !capable(CAP_SYS_ADMIN))) {
> - ret = -EPERM;
> - goto out_unlock;
> - }
> -
> - ret = drm_gem_mmap_obj(map->handle, map->size, vma);
> + obj = container_of(node, struct drm_gem_object, vma_node);
> + ret = drm_gem_mmap_obj(obj, node->vm_pages, vma);
>
> -out_unlock:
> mutex_unlock(&dev->struct_mutex);
>
> return ret;
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
> index ce06397..2e0080b 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -27,10 +27,11 @@
> #include <drm/drmP.h>
> #include <drm/drm.h>
> #include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_vma_manager.h>
>
> static unsigned int get_gem_mmap_offset(struct drm_gem_object *obj)
> {
> - return (unsigned int)obj->map_list.hash.key << PAGE_SHIFT;
> + return (unsigned int)drm_vma_node_offset_addr(&obj->vma_node);
> }
>
> /*
> @@ -172,7 +173,7 @@ void drm_gem_cma_free_object(struct drm_gem_object *gem_obj)
> {
> struct drm_gem_cma_object *cma_obj;
>
> - if (gem_obj->map_list.map)
> + if (drm_vma_node_has_offset(&gem_obj->vma_node))
> drm_gem_free_mmap_offset(gem_obj);
>
> cma_obj = to_drm_gem_cma_obj(gem_obj);
> @@ -305,8 +306,8 @@ void drm_gem_cma_describe(struct drm_gem_cma_object *cma_obj, struct seq_file *m
>
> WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>
> - if (obj->map_list.map)
> - off = (uint64_t)obj->map_list.hash.key;
> + if (drm_vma_node_has_offset(&obj->vma_node))
> + off = drm_vma_node_start(&obj->vma_node);
>
> seq_printf(m, "%2d (%2d) %08llx %08Zx %p %d",
> obj->name, obj->refcount.refcount.counter,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index c3f15e7..4756513 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -10,6 +10,7 @@
> */
>
> #include <drm/drmP.h>
> +#include <drm/drm_vma_manager.h>
>
> #include <linux/shmem_fs.h>
> #include <drm/exynos_drm.h>
> @@ -152,7 +153,7 @@ out:
> exynos_drm_fini_buf(obj->dev, buf);
> exynos_gem_obj->buffer = NULL;
>
> - if (obj->map_list.map)
> + if (drm_vma_node_has_offset(&obj->vma_node))
> drm_gem_free_mmap_offset(obj);
>
> /* release file pointer to gem object. */
> @@ -702,13 +703,13 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file *file_priv,
> goto unlock;
> }
>
> - if (!obj->map_list.map) {
> + if (!drm_vma_node_has_offset(&obj->vma_node)) {
> ret = drm_gem_create_mmap_offset(obj);
> if (ret)
> goto out;
> }
>
> - *offset = (u64)obj->map_list.hash.key << PAGE_SHIFT;
> + *offset = drm_vma_node_offset_addr(&obj->vma_node);
> DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);
>
> out:
> diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
> index eefd6cc..9fd0c9f 100644
> --- a/drivers/gpu/drm/gma500/gem.c
> +++ b/drivers/gpu/drm/gma500/gem.c
> @@ -26,6 +26,7 @@
> #include <drm/drmP.h>
> #include <drm/drm.h>
> #include <drm/gma_drm.h>
> +#include <drm/drm_vma_manager.h>
> #include "psb_drv.h"
>
> int psb_gem_init_object(struct drm_gem_object *obj)
> @@ -38,7 +39,7 @@ void psb_gem_free_object(struct drm_gem_object *obj)
> struct gtt_range *gtt = container_of(obj, struct gtt_range, gem);
>
> /* Remove the list map if one is present */
> - if (obj->map_list.map)
> + if (drm_vma_node_has_offset(&obj->vma_node))
> drm_gem_free_mmap_offset(obj);
> drm_gem_object_release(obj);
>
> @@ -81,13 +82,12 @@ int psb_gem_dumb_map_gtt(struct drm_file *file, struct drm_device *dev,
> /* What validation is needed here ? */
>
> /* Make it mmapable */
> - if (!obj->map_list.map) {
> + if (!drm_vma_node_has_offset(&obj->vma_node)) {
> ret = drm_gem_create_mmap_offset(obj);
> if (ret)
> goto out;
> }
> - /* GEM should really work out the hash offsets for us */
> - *offset = (u64)obj->map_list.hash.key << PAGE_SHIFT;
> + *offset = drm_vma_node_offset_addr(&obj->vma_node);
> out:
> drm_gem_object_unreference(obj);
> unlock:
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 769f752..ca1ceb0 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -26,6 +26,7 @@
> */
>
> #include <drm/drmP.h>
> +#include <drm/drm_vma_manager.h>
> #include <drm/i915_drm.h>
> #include "i915_drv.h"
> #include "i915_trace.h"
> @@ -1427,7 +1428,7 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj)
>
> if (obj->base.dev->dev_mapping)
> unmap_mapping_range(obj->base.dev->dev_mapping,
> - (loff_t)obj->base.map_list.hash.key<<PAGE_SHIFT,
> + (loff_t)drm_vma_node_offset_addr(&obj->base.vma_node),
> obj->base.size, 1);
>
> obj->fault_mappable = false;
> @@ -1485,7 +1486,7 @@ static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj)
> struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
> int ret;
>
> - if (obj->base.map_list.map)
> + if (drm_vma_node_has_offset(&obj->base.vma_node))
> return 0;
>
> dev_priv->mm.shrinker_no_lock_stealing = true;
> @@ -1516,7 +1517,7 @@ out:
>
> static void i915_gem_object_free_mmap_offset(struct drm_i915_gem_object *obj)
> {
> - if (!obj->base.map_list.map)
> + if (!drm_vma_node_has_offset(&obj->base.vma_node))
> return;
>
> drm_gem_free_mmap_offset(&obj->base);
> @@ -1557,7 +1558,7 @@ i915_gem_mmap_gtt(struct drm_file *file,
> if (ret)
> goto out;
>
> - *offset = (u64)obj->base.map_list.hash.key << PAGE_SHIFT;
> + *offset = drm_vma_node_offset_addr(&obj->base.vma_node);
>
> out:
> drm_gem_object_unreference(&obj->base);
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
> index ebbdf41..0bcfdb9 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
> @@ -20,6 +20,7 @@
>
> #include <linux/spinlock.h>
> #include <linux/shmem_fs.h>
> +#include <drm/drm_vma_manager.h>
>
> #include "omap_drv.h"
> #include "omap_dmm_tiler.h"
> @@ -311,7 +312,7 @@ static uint64_t mmap_offset(struct drm_gem_object *obj)
>
> WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>
> - if (!obj->map_list.map) {
> + if (!drm_vma_node_has_offset(&obj->vma_node)) {
> /* Make it mmapable */
> size_t size = omap_gem_mmap_size(obj);
> int ret = _drm_gem_create_mmap_offset_size(obj, size);
> @@ -322,7 +323,7 @@ static uint64_t mmap_offset(struct drm_gem_object *obj)
> }
> }
>
> - return (uint64_t)obj->map_list.hash.key << PAGE_SHIFT;
> + return drm_vma_node_offset_addr(&obj->vma_node);
> }
>
> uint64_t omap_gem_mmap_offset(struct drm_gem_object *obj)
> @@ -1001,8 +1002,8 @@ void omap_gem_describe(struct drm_gem_object *obj, struct seq_file *m)
>
> WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>
> - if (obj->map_list.map)
> - off = (uint64_t)obj->map_list.hash.key;
> + if (drm_vma_node_has_offset(&obj->vma_node))
> + off = drm_vma_node_start(&obj->vma_node);
>
> seq_printf(m, "%08x: %2d (%2d) %08llx %08Zx (%2d) %p %4d",
> omap_obj->flags, obj->name, obj->refcount.refcount.counter,
> @@ -1309,7 +1310,7 @@ void omap_gem_free_object(struct drm_gem_object *obj)
>
> list_del(&omap_obj->mm_list);
>
> - if (obj->map_list.map)
> + if (drm_vma_node_has_offset(&obj->vma_node))
> drm_gem_free_mmap_offset(obj);
>
> /* this means the object is still pinned.. which really should
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem_helpers.c b/drivers/gpu/drm/omapdrm/omap_gem_helpers.c
> index f9eb679..dbb1575 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem_helpers.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem_helpers.c
> @@ -118,52 +118,7 @@ _drm_gem_create_mmap_offset_size(struct drm_gem_object *obj, size_t size)
> {
> struct drm_device *dev = obj->dev;
> struct drm_gem_mm *mm = dev->mm_private;
> - struct drm_map_list *list;
> - struct drm_local_map *map;
> - int ret = 0;
> -
> - /* Set the object up for mmap'ing */
> - list = &obj->map_list;
> - list->map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL);
> - if (!list->map)
> - return -ENOMEM;
> -
> - map = list->map;
> - map->type = _DRM_GEM;
> - map->size = size;
> - map->handle = obj;
> -
> - /* Get a DRM GEM mmap offset allocated... */
> - list->file_offset_node = drm_mm_search_free(&mm->offset_manager,
> - size / PAGE_SIZE, 0, 0);
> -
> - if (!list->file_offset_node) {
> - DRM_ERROR("failed to allocate offset for bo %d\n", obj->name);
> - ret = -ENOSPC;
> - goto out_free_list;
> - }
> -
> - list->file_offset_node = drm_mm_get_block(list->file_offset_node,
> - size / PAGE_SIZE, 0);
> - if (!list->file_offset_node) {
> - ret = -ENOMEM;
> - goto out_free_list;
> - }
> -
> - list->hash.key = list->file_offset_node->start;
> - ret = drm_ht_insert_item(&mm->offset_hash, &list->hash);
> - if (ret) {
> - DRM_ERROR("failed to add to map hash\n");
> - goto out_free_mm;
> - }
> -
> - return 0;
> -
> -out_free_mm:
> - drm_mm_put_block(list->file_offset_node);
> -out_free_list:
> - kfree(list->map);
> - list->map = NULL;
>
> - return ret;
> + return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node,
> + size / PAGE_SIZE);
> }
> diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
> index ef034fa..59aee8a 100644
> --- a/drivers/gpu/drm/udl/udl_gem.c
> +++ b/drivers/gpu/drm/udl/udl_gem.c
> @@ -223,7 +223,7 @@ void udl_gem_free_object(struct drm_gem_object *gem_obj)
> if (obj->pages)
> udl_gem_put_pages(obj);
>
> - if (gem_obj->map_list.map)
> + if (drm_vma_node_has_offset(&gem_obj->vma_node))
> drm_gem_free_mmap_offset(gem_obj);
> }
>
> @@ -247,13 +247,13 @@ int udl_gem_mmap(struct drm_file *file, struct drm_device *dev,
> ret = udl_gem_get_pages(gobj, GFP_KERNEL);
> if (ret)
> goto out;
> - if (!gobj->base.map_list.map) {
> + if (!drm_vma_node_has_offset(&gobj->base.vma_node)) {
> ret = drm_gem_create_mmap_offset(obj);
> if (ret)
> goto out;
> }
>
> - *offset = (u64)gobj->base.map_list.hash.key << PAGE_SHIFT;
> + *offset = drm_vma_node_offset_addr(&gobj->base.vma_node);
>
> out:
> drm_gem_object_unreference(&gobj->base);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 82670ac..6544450 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -71,6 +71,7 @@
> #include <asm/pgalloc.h>
> #include <drm/drm.h>
> #include <drm/drm_sarea.h>
> +#include <drm/drm_vma_manager.h>
>
> #include <linux/idr.h>
>
> @@ -587,7 +588,6 @@ struct drm_map_list {
> struct drm_local_map *map; /**< mapping */
> uint64_t user_token;
> struct drm_master *master;
> - struct drm_mm_node *file_offset_node; /**< fake offset */
> };
>
> /**
> @@ -622,8 +622,7 @@ struct drm_ati_pcigart_info {
> * GEM specific mm private for tracking GEM objects
> */
> struct drm_gem_mm {
> - struct drm_mm offset_manager; /**< Offset mgmt for buffer objects */
> - struct drm_open_hash offset_hash; /**< User token hash table for maps */
> + struct drm_vma_offset_manager vma_manager;
> };
>
> /**
> @@ -644,7 +643,7 @@ struct drm_gem_object {
> struct file *filp;
>
> /* Mapping info for this object */
> - struct drm_map_list map_list;
> + struct drm_vma_offset_node vma_node;
>
> /**
> * Size of the object, in bytes. Immutable over the object's
> diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
> index b519d7f..4fcc441 100644
> --- a/include/drm/drm_vma_manager.h
> +++ b/include/drm/drm_vma_manager.h
> @@ -23,7 +23,6 @@
> * OTHER DEALINGS IN THE SOFTWARE.
> */
>
> -#include <drm/drmP.h>
> #include <drm/drm_mm.h>
> #include <linux/module.h>
> #include <linux/rbtree.h>
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 238a166..272580c 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -181,7 +181,7 @@ enum drm_map_type {
> _DRM_AGP = 3, /**< AGP/GART */
> _DRM_SCATTER_GATHER = 4, /**< Scatter/gather memory for PCI DMA */
> _DRM_CONSISTENT = 5, /**< Consistent memory for PCI DMA */
> - _DRM_GEM = 6, /**< GEM object */
> + _DRM_GEM = 6, /**< GEM object (obsolete) */
> };
>
> /**
> --
> 1.8.3.2
>
More information about the dri-devel
mailing list