For an upcoming patch where we introduce the i915 VMA, it's ideal to have the drm_mm_node as part of the VMA struct (ie. it's pre-allocated). Part of the conversion to VMAs is to kill off obj->gtt_space. Doing this will break a bunch of code, but amongst them are 2 callers of drm_mm_create_block(), both related to stolen memory.
It also allows us to embed the drm_mm_node into the object currently which provides a nice transition over to the new code.
v2: Reordered to do before ripping out obj->gtt_offset. Some minor cleanups made available because of reordering.
CC: dri-devel@lists.freedesktop.org Signed-off-by: Ben Widawsky ben@bwidawsk.net --- drivers/gpu/drm/drm_mm.c | 16 +++++---------- drivers/gpu/drm/i915/i915_gem_gtt.c | 18 +++++++++++++---- drivers/gpu/drm/i915/i915_gem_stolen.c | 36 +++++++++++++++++++++++----------- include/drm/drm_mm.h | 9 +++++---- 4 files changed, 49 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c index 07cf99c..9e8dfbc 100644 --- a/drivers/gpu/drm/drm_mm.c +++ b/drivers/gpu/drm/drm_mm.c @@ -147,12 +147,10 @@ static void drm_mm_insert_helper(struct drm_mm_node *hole_node, } }
-struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm, - unsigned long start, - unsigned long size, - bool atomic) +int drm_mm_create_block(struct drm_mm *mm, struct drm_mm_node *node, + unsigned long start, unsigned long size) { - struct drm_mm_node *hole, *node; + struct drm_mm_node *hole; unsigned long end = start + size; unsigned long hole_start; unsigned long hole_end; @@ -161,10 +159,6 @@ struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm, if (hole_start > start || hole_end < end) continue;
- node = drm_mm_kmalloc(mm, atomic); - if (unlikely(node == NULL)) - return NULL; - node->start = start; node->size = size; node->mm = mm; @@ -184,11 +178,11 @@ struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm, node->hole_follows = 1; }
- return node; + return 0; }
WARN(1, "no hole found for block 0x%lx + 0x%lx\n", start, size); - return NULL; + return -ENOSPC; } EXPORT_SYMBOL(drm_mm_create_block);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 66929ea..5c6fc0e 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -629,14 +629,24 @@ void i915_gem_setup_global_gtt(struct drm_device *dev,
/* Mark any preallocated objects as occupied */ list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) { + int ret; DRM_DEBUG_KMS("reserving preallocated space: %x + %zx\n", obj->gtt_offset, obj->base.size);
BUG_ON(obj->gtt_space != I915_GTT_RESERVED); - obj->gtt_space = drm_mm_create_block(&dev_priv->mm.gtt_space, - obj->gtt_offset, - obj->base.size, - false); + obj->gtt_space = kzalloc(sizeof(*obj->gtt_space), GFP_KERNEL); + if (!obj->gtt_space) { + DRM_ERROR("Failed to preserve all objects\n"); + break; + } + ret = drm_mm_create_block(&dev_priv->mm.gtt_space, + obj->gtt_space, + obj->gtt_offset, + obj->base.size); + if (ret) { + DRM_DEBUG_KMS("Reservation failed\n"); + kfree(obj->gtt_space); + } obj->has_global_gtt_mapping = 1; }
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 8e02344..f9db84a 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -330,6 +330,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj; struct drm_mm_node *stolen; + int ret;
if (dev_priv->mm.stolen_base == 0) return NULL; @@ -344,11 +345,15 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, if (WARN_ON(size == 0)) return NULL;
- stolen = drm_mm_create_block(&dev_priv->mm.stolen, - stolen_offset, size, - false); - if (stolen == NULL) { + stolen = kzalloc(sizeof(*stolen), GFP_KERNEL); + if (!stolen) + return NULL; + + ret = drm_mm_create_block(&dev_priv->mm.stolen, stolen, stolen_offset, + size); + if (ret) { DRM_DEBUG_KMS("failed to allocate stolen space\n"); + kfree(stolen); return NULL; }
@@ -369,13 +374,18 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, * later. */ if (drm_mm_initialized(&dev_priv->mm.gtt_space)) { - obj->gtt_space = drm_mm_create_block(&dev_priv->mm.gtt_space, - gtt_offset, size, - false); - if (obj->gtt_space == NULL) { + obj->gtt_space = kzalloc(sizeof(*obj->gtt_space), GFP_KERNEL); + if (!obj->gtt_space) { + DRM_DEBUG_KMS("-ENOMEM stolen GTT space\n"); + goto unref_out; + } + + ret = drm_mm_create_block(&dev_priv->mm.gtt_space, + obj->gtt_space, + gtt_offset, size); + if (ret) { DRM_DEBUG_KMS("failed to allocate stolen GTT space\n"); - drm_gem_object_unreference(&obj->base); - return NULL; + goto unref_out; } } else obj->gtt_space = I915_GTT_RESERVED; @@ -385,8 +395,12 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
list_add_tail(&obj->global_list, &dev_priv->mm.bound_list); list_add_tail(&obj->mm_list, &dev_priv->mm.inactive_list); - return obj; + +unref_out: + drm_gem_object_unreference(&obj->base); + drm_mm_put_block(stolen); + return NULL; }
void diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h index 88591ef..d8b56b7 100644 --- a/include/drm/drm_mm.h +++ b/include/drm/drm_mm.h @@ -138,10 +138,10 @@ static inline unsigned long drm_mm_hole_node_end(struct drm_mm_node *hole_node) /* * Basic range manager support (drm_mm.c) */ -extern struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm, - unsigned long start, - unsigned long size, - bool atomic); +extern int drm_mm_create_block(struct drm_mm *mm, + struct drm_mm_node *node, + unsigned long start, + unsigned long size); extern struct drm_mm_node *drm_mm_get_block_generic(struct drm_mm_node *node, unsigned long size, unsigned alignment, @@ -155,6 +155,7 @@ extern struct drm_mm_node *drm_mm_get_block_range_generic( unsigned long start, unsigned long end, int atomic); + static inline struct drm_mm_node *drm_mm_get_block(struct drm_mm_node *parent, unsigned long size, unsigned alignment)
From: Chris Wilson chris@chris-wilson.co.uk
Clients like i915 needs to segregate cache domains within the GTT which can lead to small amounts of fragmentation. By allocating the uncached buffers from the bottom and the cacheable buffers from the top, we can reduce the amount of wasted space and also optimize allocation of the mappable portion of the GTT to only those buffers that require CPU access through the GTT.
v2 by Ben: Update callers in i915_gem_object_bind_to_gtt() Turn search flags and allocation flags into separate enums Make checkpatch happy where logical/easy
v3: by Ben: Rebased on create_block removal
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk CC: dri-devel@lists.freedesktop.org Signed-off-by: Ben Widawsky ben@bwidawsk.net
Conflicts: drivers/gpu/drm/drm_mm.c include/drm/drm_mm.h --- drivers/gpu/drm/drm_mm.c | 121 +++++++++++++++------------ drivers/gpu/drm/i915/i915_gem.c | 4 +- drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +- drivers/gpu/drm/i915/i915_gem_stolen.c | 5 +- include/drm/drm_mm.h | 148 ++++++++++++++++++++------------- 5 files changed, 166 insertions(+), 115 deletions(-)
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c index 9e8dfbc..4a30d55 100644 --- a/drivers/gpu/drm/drm_mm.c +++ b/drivers/gpu/drm/drm_mm.c @@ -49,7 +49,7 @@
#define MM_UNUSED_TARGET 4
-static struct drm_mm_node *drm_mm_kmalloc(struct drm_mm *mm, int atomic) +static struct drm_mm_node *drm_mm_kmalloc(struct drm_mm *mm, bool atomic) { struct drm_mm_node *child;
@@ -105,7 +105,8 @@ EXPORT_SYMBOL(drm_mm_pre_get); static void drm_mm_insert_helper(struct drm_mm_node *hole_node, struct drm_mm_node *node, unsigned long size, unsigned alignment, - unsigned long color) + unsigned long color, + enum drm_mm_allocator_flags flags) { struct drm_mm *mm = hole_node->mm; unsigned long hole_start = drm_mm_hole_node_start(hole_node); @@ -118,12 +119,22 @@ static void drm_mm_insert_helper(struct drm_mm_node *hole_node, if (mm->color_adjust) mm->color_adjust(hole_node, color, &adj_start, &adj_end);
+ if (flags & DRM_MM_CREATE_TOP) + adj_start = adj_end - size; + if (alignment) { unsigned tmp = adj_start % alignment; - if (tmp) - adj_start += alignment - tmp; + if (tmp) { + if (flags & DRM_MM_CREATE_TOP) + adj_start -= tmp; + else + adj_start += alignment - tmp; + } }
+ BUG_ON(adj_start < hole_start); + BUG_ON(adj_end > hole_end); + if (adj_start == hole_start) { hole_node->hole_follows = 0; list_del(&hole_node->hole_stack); @@ -148,7 +159,8 @@ static void drm_mm_insert_helper(struct drm_mm_node *hole_node, }
int drm_mm_create_block(struct drm_mm *mm, struct drm_mm_node *node, - unsigned long start, unsigned long size) + unsigned long start, unsigned long size, + enum drm_mm_allocator_flags flags) { struct drm_mm_node *hole; unsigned long end = start + size; @@ -190,15 +202,15 @@ struct drm_mm_node *drm_mm_get_block_generic(struct drm_mm_node *hole_node, unsigned long size, unsigned alignment, unsigned long color, - int atomic) + enum drm_mm_allocator_flags flags) { struct drm_mm_node *node;
- node = drm_mm_kmalloc(hole_node->mm, atomic); + node = drm_mm_kmalloc(hole_node->mm, flags & DRM_MM_CREATE_ATOMIC); if (unlikely(node == NULL)) return NULL;
- drm_mm_insert_helper(hole_node, node, size, alignment, color); + drm_mm_insert_helper(hole_node, node, size, alignment, color, flags);
return node; } @@ -211,32 +223,28 @@ EXPORT_SYMBOL(drm_mm_get_block_generic); */ int drm_mm_insert_node_generic(struct drm_mm *mm, struct drm_mm_node *node, unsigned long size, unsigned alignment, - unsigned long color) + unsigned long color, + enum drm_mm_allocator_flags aflags, + enum drm_mm_search_flags sflags) { struct drm_mm_node *hole_node;
hole_node = drm_mm_search_free_generic(mm, size, alignment, - color, 0); + color, sflags); if (!hole_node) return -ENOSPC;
- drm_mm_insert_helper(hole_node, node, size, alignment, color); + drm_mm_insert_helper(hole_node, node, size, alignment, color, aflags); return 0; } EXPORT_SYMBOL(drm_mm_insert_node_generic);
-int drm_mm_insert_node(struct drm_mm *mm, struct drm_mm_node *node, - unsigned long size, unsigned alignment) -{ - return drm_mm_insert_node_generic(mm, node, size, alignment, 0); -} -EXPORT_SYMBOL(drm_mm_insert_node); - static void drm_mm_insert_helper_range(struct drm_mm_node *hole_node, struct drm_mm_node *node, unsigned long size, unsigned alignment, unsigned long color, - unsigned long start, unsigned long end) + unsigned long start, unsigned long end, + enum drm_mm_search_flags flags) { struct drm_mm *mm = hole_node->mm; unsigned long hole_start = drm_mm_hole_node_start(hole_node); @@ -251,13 +259,20 @@ static void drm_mm_insert_helper_range(struct drm_mm_node *hole_node, if (adj_end > end) adj_end = end;
+ if (flags & DRM_MM_CREATE_TOP) + adj_start = adj_end - size; + if (mm->color_adjust) mm->color_adjust(hole_node, color, &adj_start, &adj_end);
if (alignment) { unsigned tmp = adj_start % alignment; - if (tmp) - adj_start += alignment - tmp; + if (tmp) { + if (flags & DRM_MM_CREATE_TOP) + adj_start -= tmp; + else + adj_start += alignment - tmp; + } }
if (adj_start == hole_start) { @@ -274,6 +289,8 @@ static void drm_mm_insert_helper_range(struct drm_mm_node *hole_node, INIT_LIST_HEAD(&node->hole_stack); list_add(&node->node_list, &hole_node->node_list);
+ BUG_ON(node->start < start); + BUG_ON(node->start < adj_start); BUG_ON(node->start + node->size > adj_end); BUG_ON(node->start + node->size > end);
@@ -284,22 +301,23 @@ static void drm_mm_insert_helper_range(struct drm_mm_node *hole_node, } }
-struct drm_mm_node *drm_mm_get_block_range_generic(struct drm_mm_node *hole_node, - unsigned long size, - unsigned alignment, - unsigned long color, - unsigned long start, - unsigned long end, - int atomic) +struct drm_mm_node * +drm_mm_get_block_range_generic(struct drm_mm_node *hole_node, + unsigned long size, + unsigned alignment, + unsigned long color, + unsigned long start, + unsigned long end, + enum drm_mm_allocator_flags flags) { struct drm_mm_node *node;
- node = drm_mm_kmalloc(hole_node->mm, atomic); + node = drm_mm_kmalloc(hole_node->mm, flags & DRM_MM_CREATE_ATOMIC); if (unlikely(node == NULL)) return NULL;
drm_mm_insert_helper_range(hole_node, node, size, alignment, color, - start, end); + start, end, flags);
return node; } @@ -312,31 +330,25 @@ EXPORT_SYMBOL(drm_mm_get_block_range_generic); */ int drm_mm_insert_node_in_range_generic(struct drm_mm *mm, struct drm_mm_node *node, unsigned long size, unsigned alignment, unsigned long color, - unsigned long start, unsigned long end) + unsigned long start, unsigned long end, + enum drm_mm_allocator_flags aflags, + enum drm_mm_search_flags sflags) { struct drm_mm_node *hole_node;
hole_node = drm_mm_search_free_in_range_generic(mm, size, alignment, color, - start, end, 0); + start, end, sflags); if (!hole_node) return -ENOSPC;
drm_mm_insert_helper_range(hole_node, node, size, alignment, color, - start, end); + start, end, aflags); return 0; } EXPORT_SYMBOL(drm_mm_insert_node_in_range_generic);
-int drm_mm_insert_node_in_range(struct drm_mm *mm, struct drm_mm_node *node, - unsigned long size, unsigned alignment, - unsigned long start, unsigned long end) -{ - return drm_mm_insert_node_in_range_generic(mm, node, size, alignment, 0, start, end); -} -EXPORT_SYMBOL(drm_mm_insert_node_in_range); - /** * Remove a memory node from the allocator. */ @@ -412,7 +424,7 @@ struct drm_mm_node *drm_mm_search_free_generic(const struct drm_mm *mm, unsigned long size, unsigned alignment, unsigned long color, - bool best_match) + enum drm_mm_search_flags flags) { struct drm_mm_node *entry; struct drm_mm_node *best; @@ -425,7 +437,8 @@ struct drm_mm_node *drm_mm_search_free_generic(const struct drm_mm *mm, best = NULL; best_size = ~0UL;
- drm_mm_for_each_hole(entry, mm, adj_start, adj_end) { + __drm_mm_for_each_hole(entry, mm, adj_start, adj_end, + flags & DRM_MM_SEARCH_BELOW) { if (mm->color_adjust) { mm->color_adjust(entry, color, &adj_start, &adj_end); if (adj_end <= adj_start) @@ -435,7 +448,7 @@ struct drm_mm_node *drm_mm_search_free_generic(const struct drm_mm *mm, if (!check_free_hole(adj_start, adj_end, size, alignment)) continue;
- if (!best_match) + if ((flags & DRM_MM_SEARCH_BEST) == 0) return entry;
if (entry->size < best_size) { @@ -448,13 +461,14 @@ struct drm_mm_node *drm_mm_search_free_generic(const struct drm_mm *mm, } EXPORT_SYMBOL(drm_mm_search_free_generic);
-struct drm_mm_node *drm_mm_search_free_in_range_generic(const struct drm_mm *mm, - unsigned long size, - unsigned alignment, - unsigned long color, - unsigned long start, - unsigned long end, - bool best_match) +struct drm_mm_node * +drm_mm_search_free_in_range_generic(const struct drm_mm *mm, + unsigned long size, + unsigned alignment, + unsigned long color, + unsigned long start, + unsigned long end, + enum drm_mm_search_flags flags) { struct drm_mm_node *entry; struct drm_mm_node *best; @@ -467,7 +481,8 @@ struct drm_mm_node *drm_mm_search_free_in_range_generic(const struct drm_mm *mm, best = NULL; best_size = ~0UL;
- drm_mm_for_each_hole(entry, mm, adj_start, adj_end) { + __drm_mm_for_each_hole(entry, mm, adj_start, adj_end, + flags & DRM_MM_SEARCH_BELOW) { if (adj_start < start) adj_start = start; if (adj_end > end) @@ -482,7 +497,7 @@ struct drm_mm_node *drm_mm_search_free_in_range_generic(const struct drm_mm *mm, if (!check_free_hole(adj_start, adj_end, size, alignment)) continue;
- if (!best_match) + if ((flags & DRM_MM_SEARCH_BEST) == 0) return entry;
if (entry->size < best_size) { diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 6cb9210..9a0470b 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3115,7 +3115,9 @@ search_free: ret = drm_mm_insert_node_in_range_generic(&dev_priv->mm.gtt_space, &obj->ggtt_space, size, alignment, - obj->cache_level, 0, gtt_max); + obj->cache_level, 0, gtt_max, + DRM_MM_CREATE_DEFAULT, + DRM_MM_SEARCH_DEFAULT); if (ret) { ret = i915_gem_evict_something(dev, size, alignment, obj->cache_level, diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 4df6159..081f77e 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -637,7 +637,8 @@ void i915_gem_setup_global_gtt(struct drm_device *dev, ret = drm_mm_create_block(&dev_priv->mm.gtt_space, &obj->ggtt_space, i915_gem_obj_ggtt_offset(obj), - obj->base.size); + obj->base.size, + DRM_MM_CREATE_DEFAULT); if (ret) DRM_DEBUG_KMS("Reservation failed\n"); obj->has_global_gtt_mapping = 1; diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index dc68b30..0d19710 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -350,7 +350,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, return NULL;
ret = drm_mm_create_block(&dev_priv->mm.stolen, stolen, stolen_offset, - size); + size, DRM_MM_CREATE_DEFAULT); if (ret) { DRM_DEBUG_KMS("failed to allocate stolen space\n"); kfree(stolen); @@ -376,7 +376,8 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, if (drm_mm_initialized(&dev_priv->mm.gtt_space)) { ret = drm_mm_create_block(&dev_priv->mm.gtt_space, &obj->ggtt_space, - gtt_offset, size); + gtt_offset, size, + DRM_MM_CREATE_DEFAULT); if (ret) { DRM_DEBUG_KMS("failed to allocate stolen GTT space\n"); goto unref_out; diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h index d8b56b7..2915c43 100644 --- a/include/drm/drm_mm.h +++ b/include/drm/drm_mm.h @@ -41,6 +41,21 @@ #include <linux/seq_file.h> #endif
+enum drm_mm_allocator_flags { + DRM_MM_CREATE_DEFAULT = 0, + DRM_MM_CREATE_ATOMIC = 1<<0, + DRM_MM_CREATE_TOP = 1<<1, +}; + +enum drm_mm_search_flags { + DRM_MM_SEARCH_DEFAULT = 0, + DRM_MM_SEARCH_BEST = 1<<0, + DRM_MM_SEARCH_BELOW = 1<<1, +}; + +#define DRM_MM_BOTTOMUP DRM_MM_CREATE_DEFAULT, DRM_MM_SEARCH_DEFAULT +#define DRM_MM_TOPDOWN DRM_MM_CREATE_TOP, DRM_MM_SEARCH_BELOW + struct drm_mm_node { struct list_head node_list; struct list_head hole_stack; @@ -135,30 +150,41 @@ static inline unsigned long drm_mm_hole_node_end(struct drm_mm_node *hole_node) 1 : 0; \ entry = list_entry(entry->hole_stack.next, struct drm_mm_node, hole_stack))
+#define __drm_mm_for_each_hole(entry, mm, hole_start, hole_end, backwards) \ + for (entry = list_entry((backwards) ? (mm)->hole_stack.prev : (mm)->hole_stack.next, struct drm_mm_node, hole_stack); \ + &entry->hole_stack != &(mm)->hole_stack ? \ + hole_start = drm_mm_hole_node_start(entry), \ + hole_end = drm_mm_hole_node_end(entry), \ + 1 : 0; \ + entry = list_entry((backwards) ? entry->hole_stack.prev : entry->hole_stack.next, struct drm_mm_node, hole_stack)) + /* * Basic range manager support (drm_mm.c) */ extern int drm_mm_create_block(struct drm_mm *mm, struct drm_mm_node *node, unsigned long start, - unsigned long size); -extern struct drm_mm_node *drm_mm_get_block_generic(struct drm_mm_node *node, - unsigned long size, - unsigned alignment, - unsigned long color, - int atomic); -extern struct drm_mm_node *drm_mm_get_block_range_generic( - struct drm_mm_node *node, - unsigned long size, - unsigned alignment, - unsigned long color, - unsigned long start, - unsigned long end, - int atomic); + unsigned long size, + enum drm_mm_allocator_flags flags); +extern struct drm_mm_node * +drm_mm_get_block_generic(struct drm_mm_node *node, + unsigned long size, + unsigned alignment, + unsigned long color, + enum drm_mm_allocator_flags flags); +extern struct drm_mm_node * +drm_mm_get_block_range_generic(struct drm_mm_node *node, + unsigned long size, + unsigned alignment, + unsigned long color, + unsigned long start, + unsigned long end, + enum drm_mm_allocator_flags flags);
-static inline struct drm_mm_node *drm_mm_get_block(struct drm_mm_node *parent, - unsigned long size, - unsigned alignment) +static inline struct drm_mm_node * +drm_mm_get_block(struct drm_mm_node *parent, + unsigned long size, + unsigned alignment) { return drm_mm_get_block_generic(parent, size, alignment, 0, 0); } @@ -166,7 +192,8 @@ static inline struct drm_mm_node *drm_mm_get_block_atomic(struct drm_mm_node *pa unsigned long size, unsigned alignment) { - return drm_mm_get_block_generic(parent, size, alignment, 0, 1); + return drm_mm_get_block_generic(parent, size, alignment, 0, + DRM_MM_CREATE_ATOMIC); } static inline struct drm_mm_node *drm_mm_get_block_range( struct drm_mm_node *parent, @@ -197,39 +224,41 @@ static inline struct drm_mm_node *drm_mm_get_block_atomic_range( unsigned long end) { return drm_mm_get_block_range_generic(parent, size, alignment, 0, - start, end, 1); + start, end, + DRM_MM_CREATE_ATOMIC); }
-extern int drm_mm_insert_node(struct drm_mm *mm, - struct drm_mm_node *node, - unsigned long size, - unsigned alignment); -extern int drm_mm_insert_node_in_range(struct drm_mm *mm, - struct drm_mm_node *node, - unsigned long size, - unsigned alignment, - unsigned long start, - unsigned long end); extern int drm_mm_insert_node_generic(struct drm_mm *mm, struct drm_mm_node *node, unsigned long size, unsigned alignment, - unsigned long color); -extern int drm_mm_insert_node_in_range_generic(struct drm_mm *mm, - struct drm_mm_node *node, - unsigned long size, - unsigned alignment, - unsigned long color, - unsigned long start, - unsigned long end); + unsigned long color, + enum drm_mm_allocator_flags aflags, + enum drm_mm_search_flags sflags); +#define drm_mm_insert_node(mm, node, size, alignment) \ + drm_mm_insert_node_generic(mm, node, size, alignment, 0, 0) +extern int +drm_mm_insert_node_in_range_generic(struct drm_mm *mm, + struct drm_mm_node *node, + unsigned long size, + unsigned alignment, + unsigned long color, + unsigned long start, + unsigned long end, + enum drm_mm_allocator_flags aflags, + enum drm_mm_search_flags sflags); +#define drm_mm_insert_node_in_range(mm, node, size, alignment, start, end) \ + drm_mm_insert_node_in_range_generic(mm, node, size, alignment, 0, start, end, 0) extern void drm_mm_put_block(struct drm_mm_node *cur); extern void drm_mm_remove_node(struct drm_mm_node *node); extern void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new); -extern struct drm_mm_node *drm_mm_search_free_generic(const struct drm_mm *mm, - unsigned long size, - unsigned alignment, - unsigned long color, - bool best_match); + +extern struct drm_mm_node * +drm_mm_search_free_generic(const struct drm_mm *mm, + unsigned long size, + unsigned alignment, + unsigned long color, + enum drm_mm_search_flags flags); extern struct drm_mm_node *drm_mm_search_free_in_range_generic( const struct drm_mm *mm, unsigned long size, @@ -237,13 +266,15 @@ extern struct drm_mm_node *drm_mm_search_free_in_range_generic( unsigned long color, unsigned long start, unsigned long end, - bool best_match); -static inline struct drm_mm_node *drm_mm_search_free(const struct drm_mm *mm, - unsigned long size, - unsigned alignment, - bool best_match) + enum drm_mm_search_flags flags); + +static inline struct drm_mm_node * +drm_mm_search_free(const struct drm_mm *mm, + unsigned long size, + unsigned alignment, + enum drm_mm_search_flags flags) { - return drm_mm_search_free_generic(mm,size, alignment, 0, best_match); + return drm_mm_search_free_generic(mm, size, alignment, 0, flags); } static inline struct drm_mm_node *drm_mm_search_free_in_range( const struct drm_mm *mm, @@ -251,18 +282,19 @@ static inline struct drm_mm_node *drm_mm_search_free_in_range( unsigned alignment, unsigned long start, unsigned long end, - bool best_match) + enum drm_mm_search_flags flags) { return drm_mm_search_free_in_range_generic(mm, size, alignment, 0, - start, end, best_match); + start, end, flags); } -static inline struct drm_mm_node *drm_mm_search_free_color(const struct drm_mm *mm, - unsigned long size, - unsigned alignment, - unsigned long color, - bool best_match) +static inline struct drm_mm_node * +drm_mm_search_free_color(const struct drm_mm *mm, + unsigned long size, + unsigned alignment, + unsigned long color, + enum drm_mm_search_flags flags) { - return drm_mm_search_free_generic(mm,size, alignment, color, best_match); + return drm_mm_search_free_generic(mm, size, alignment, color, flags); } static inline struct drm_mm_node *drm_mm_search_free_in_range_color( const struct drm_mm *mm, @@ -271,10 +303,10 @@ static inline struct drm_mm_node *drm_mm_search_free_in_range_color( unsigned long color, unsigned long start, unsigned long end, - bool best_match) + enum drm_mm_search_flags flags) { return drm_mm_search_free_in_range_generic(mm, size, alignment, color, - start, end, best_match); + start, end, flags); } extern int drm_mm_init(struct drm_mm *mm, unsigned long start,
Hi
On Wed, Jul 3, 2013 at 11:45 PM, Ben Widawsky ben@bwidawsk.net wrote:
For an upcoming patch where we introduce the i915 VMA, it's ideal to have the drm_mm_node as part of the VMA struct (ie. it's pre-allocated). Part of the conversion to VMAs is to kill off obj->gtt_space. Doing this will break a bunch of code, but amongst them are 2 callers of drm_mm_create_block(), both related to stolen memory.
It also allows us to embed the drm_mm_node into the object currently which provides a nice transition over to the new code.
v2: Reordered to do before ripping out obj->gtt_offset. Some minor cleanups made available because of reordering.
CC: dri-devel@lists.freedesktop.org Signed-off-by: Ben Widawsky ben@bwidawsk.net
drivers/gpu/drm/drm_mm.c | 16 +++++---------- drivers/gpu/drm/i915/i915_gem_gtt.c | 18 +++++++++++++---- drivers/gpu/drm/i915/i915_gem_stolen.c | 36 +++++++++++++++++++++++----------- include/drm/drm_mm.h | 9 +++++---- 4 files changed, 49 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c index 07cf99c..9e8dfbc 100644 --- a/drivers/gpu/drm/drm_mm.c +++ b/drivers/gpu/drm/drm_mm.c @@ -147,12 +147,10 @@ static void drm_mm_insert_helper(struct drm_mm_node *hole_node, } }
-struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm,
unsigned long start,
unsigned long size,
bool atomic)
+int drm_mm_create_block(struct drm_mm *mm, struct drm_mm_node *node,
unsigned long start, unsigned long size)
{
struct drm_mm_node *hole, *node;
struct drm_mm_node *hole; unsigned long end = start + size; unsigned long hole_start; unsigned long hole_end;
@@ -161,10 +159,6 @@ struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm, if (hole_start > start || hole_end < end) continue;
node = drm_mm_kmalloc(mm, atomic);
if (unlikely(node == NULL))
return NULL;
node->start = start; node->size = size; node->mm = mm;
@@ -184,11 +178,11 @@ struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm, node->hole_follows = 1; }
return node;
return 0; } WARN(1, "no hole found for block 0x%lx + 0x%lx\n", start, size);
return NULL;
return -ENOSPC;
} EXPORT_SYMBOL(drm_mm_create_block);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 66929ea..5c6fc0e 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -629,14 +629,24 @@ void i915_gem_setup_global_gtt(struct drm_device *dev,
/* Mark any preallocated objects as occupied */ list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
int ret; DRM_DEBUG_KMS("reserving preallocated space: %x + %zx\n", obj->gtt_offset, obj->base.size); BUG_ON(obj->gtt_space != I915_GTT_RESERVED);
obj->gtt_space = drm_mm_create_block(&dev_priv->mm.gtt_space,
obj->gtt_offset,
obj->base.size,
false);
obj->gtt_space = kzalloc(sizeof(*obj->gtt_space), GFP_KERNEL);
if (!obj->gtt_space) {
DRM_ERROR("Failed to preserve all objects\n");
break;
}
ret = drm_mm_create_block(&dev_priv->mm.gtt_space,
obj->gtt_space,
obj->gtt_offset,
obj->base.size);
if (ret) {
DRM_DEBUG_KMS("Reservation failed\n");
kfree(obj->gtt_space);
Are you sure you don't need: obj->gtt_space = NULL; here? I am no expert in i915 gem handling, but looking at i915_gem.c I think you might run into bugs if not.
Also, why did you add the "break;" above, but not here? I am confused.
} obj->has_global_gtt_mapping = 1; }
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 8e02344..f9db84a 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -330,6 +330,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj; struct drm_mm_node *stolen;
int ret; if (dev_priv->mm.stolen_base == 0) return NULL;
@@ -344,11 +345,15 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, if (WARN_ON(size == 0)) return NULL;
stolen = drm_mm_create_block(&dev_priv->mm.stolen,
stolen_offset, size,
false);
if (stolen == NULL) {
stolen = kzalloc(sizeof(*stolen), GFP_KERNEL);
if (!stolen)
return NULL;
ret = drm_mm_create_block(&dev_priv->mm.stolen, stolen, stolen_offset,
size);
if (ret) { DRM_DEBUG_KMS("failed to allocate stolen space\n");
kfree(stolen); return NULL; }
@@ -369,13 +374,18 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, * later. */ if (drm_mm_initialized(&dev_priv->mm.gtt_space)) {
obj->gtt_space = drm_mm_create_block(&dev_priv->mm.gtt_space,
gtt_offset, size,
false);
if (obj->gtt_space == NULL) {
obj->gtt_space = kzalloc(sizeof(*obj->gtt_space), GFP_KERNEL);
if (!obj->gtt_space) {
DRM_DEBUG_KMS("-ENOMEM stolen GTT space\n");
goto unref_out;
}
ret = drm_mm_create_block(&dev_priv->mm.gtt_space,
obj->gtt_space,
gtt_offset, size);
if (ret) { DRM_DEBUG_KMS("failed to allocate stolen GTT space\n");
drm_gem_object_unreference(&obj->base);
return NULL;
goto unref_out;
Again: kfree(obj->gtt_space); obj->gtt_space = NULL; Otherwise, if gem-cleanup calls drm_mm_put_block() on an already removed node, you end up with NULL-derefs in drm_mm.c
} } else obj->gtt_space = I915_GTT_RESERVED;
@@ -385,8 +395,12 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
list_add_tail(&obj->global_list, &dev_priv->mm.bound_list); list_add_tail(&obj->mm_list, &dev_priv->mm.inactive_list);
return obj;
+unref_out:
drm_gem_object_unreference(&obj->base);
drm_mm_put_block(stolen);
"stolen" is already cleared by drm_gem_object_unreference(). So that's a double-free here.
The drm_mm_create_block() change looks good. Cheers David
return NULL;
}
void diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h index 88591ef..d8b56b7 100644 --- a/include/drm/drm_mm.h +++ b/include/drm/drm_mm.h @@ -138,10 +138,10 @@ static inline unsigned long drm_mm_hole_node_end(struct drm_mm_node *hole_node) /*
- Basic range manager support (drm_mm.c)
*/ -extern struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm,
unsigned long start,
unsigned long size,
bool atomic);
+extern int drm_mm_create_block(struct drm_mm *mm,
struct drm_mm_node *node,
unsigned long start,
unsigned long size);
extern struct drm_mm_node *drm_mm_get_block_generic(struct drm_mm_node *node, unsigned long size, unsigned alignment, @@ -155,6 +155,7 @@ extern struct drm_mm_node *drm_mm_get_block_range_generic( unsigned long start, unsigned long end, int atomic);
static inline struct drm_mm_node *drm_mm_get_block(struct drm_mm_node *parent, unsigned long size, unsigned alignment) -- 1.8.3.2
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Jul 04, 2013 at 11:19:58AM +0200, David Herrmann wrote:
Hi
On Wed, Jul 3, 2013 at 11:45 PM, Ben Widawsky ben@bwidawsk.net wrote:
For an upcoming patch where we introduce the i915 VMA, it's ideal to have the drm_mm_node as part of the VMA struct (ie. it's pre-allocated). Part of the conversion to VMAs is to kill off obj->gtt_space. Doing this will break a bunch of code, but amongst them are 2 callers of drm_mm_create_block(), both related to stolen memory.
It also allows us to embed the drm_mm_node into the object currently which provides a nice transition over to the new code.
v2: Reordered to do before ripping out obj->gtt_offset. Some minor cleanups made available because of reordering.
CC: dri-devel@lists.freedesktop.org Signed-off-by: Ben Widawsky ben@bwidawsk.net
drivers/gpu/drm/drm_mm.c | 16 +++++---------- drivers/gpu/drm/i915/i915_gem_gtt.c | 18 +++++++++++++---- drivers/gpu/drm/i915/i915_gem_stolen.c | 36 +++++++++++++++++++++++----------- include/drm/drm_mm.h | 9 +++++---- 4 files changed, 49 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c index 07cf99c..9e8dfbc 100644 --- a/drivers/gpu/drm/drm_mm.c +++ b/drivers/gpu/drm/drm_mm.c @@ -147,12 +147,10 @@ static void drm_mm_insert_helper(struct drm_mm_node *hole_node, } }
-struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm,
unsigned long start,
unsigned long size,
bool atomic)
+int drm_mm_create_block(struct drm_mm *mm, struct drm_mm_node *node,
unsigned long start, unsigned long size)
{
struct drm_mm_node *hole, *node;
struct drm_mm_node *hole; unsigned long end = start + size; unsigned long hole_start; unsigned long hole_end;
@@ -161,10 +159,6 @@ struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm, if (hole_start > start || hole_end < end) continue;
node = drm_mm_kmalloc(mm, atomic);
if (unlikely(node == NULL))
return NULL;
node->start = start; node->size = size; node->mm = mm;
@@ -184,11 +178,11 @@ struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm, node->hole_follows = 1; }
return node;
return 0; } WARN(1, "no hole found for block 0x%lx + 0x%lx\n", start, size);
return NULL;
return -ENOSPC;
} EXPORT_SYMBOL(drm_mm_create_block);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 66929ea..5c6fc0e 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -629,14 +629,24 @@ void i915_gem_setup_global_gtt(struct drm_device *dev,
/* Mark any preallocated objects as occupied */ list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
int ret; DRM_DEBUG_KMS("reserving preallocated space: %x + %zx\n", obj->gtt_offset, obj->base.size); BUG_ON(obj->gtt_space != I915_GTT_RESERVED);
obj->gtt_space = drm_mm_create_block(&dev_priv->mm.gtt_space,
obj->gtt_offset,
obj->base.size,
false);
obj->gtt_space = kzalloc(sizeof(*obj->gtt_space), GFP_KERNEL);
if (!obj->gtt_space) {
DRM_ERROR("Failed to preserve all objects\n");
break;
}
ret = drm_mm_create_block(&dev_priv->mm.gtt_space,
obj->gtt_space,
obj->gtt_offset,
obj->base.size);
if (ret) {
DRM_DEBUG_KMS("Reservation failed\n");
kfree(obj->gtt_space);
Are you sure you don't need: obj->gtt_space = NULL; here? I am no expert in i915 gem handling, but looking at i915_gem.c I think you might run into bugs if not.
I'm too lazy to actually check, but I believe you're probably right. It's fixed in a later patch where I added the getters and use node_allocated so I don't check obj->gtt_space != NULL anymore; but it would potentially be a painful bisect point.
Thanks for catching it (and the following ones).
Also, why did you add the "break;" above, but not here? I am confused.
The thought at the time was if kzalloc fails at this point, subsequent kzallocs are really likely to fail also. drm_mm_create_block OTOH is something I won't pretend to inquire about failure recurrence. I agree it looks funny though, so I'll change the break to continue.
} obj->has_global_gtt_mapping = 1; }
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 8e02344..f9db84a 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -330,6 +330,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj; struct drm_mm_node *stolen;
int ret; if (dev_priv->mm.stolen_base == 0) return NULL;
@@ -344,11 +345,15 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, if (WARN_ON(size == 0)) return NULL;
stolen = drm_mm_create_block(&dev_priv->mm.stolen,
stolen_offset, size,
false);
if (stolen == NULL) {
stolen = kzalloc(sizeof(*stolen), GFP_KERNEL);
if (!stolen)
return NULL;
ret = drm_mm_create_block(&dev_priv->mm.stolen, stolen, stolen_offset,
size);
if (ret) { DRM_DEBUG_KMS("failed to allocate stolen space\n");
kfree(stolen); return NULL; }
@@ -369,13 +374,18 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, * later. */ if (drm_mm_initialized(&dev_priv->mm.gtt_space)) {
obj->gtt_space = drm_mm_create_block(&dev_priv->mm.gtt_space,
gtt_offset, size,
false);
if (obj->gtt_space == NULL) {
obj->gtt_space = kzalloc(sizeof(*obj->gtt_space), GFP_KERNEL);
if (!obj->gtt_space) {
DRM_DEBUG_KMS("-ENOMEM stolen GTT space\n");
goto unref_out;
}
ret = drm_mm_create_block(&dev_priv->mm.gtt_space,
obj->gtt_space,
gtt_offset, size);
if (ret) { DRM_DEBUG_KMS("failed to allocate stolen GTT space\n");
drm_gem_object_unreference(&obj->base);
return NULL;
goto unref_out;
Again: kfree(obj->gtt_space); obj->gtt_space = NULL; Otherwise, if gem-cleanup calls drm_mm_put_block() on an already removed node, you end up with NULL-derefs in drm_mm.c
} } else obj->gtt_space = I915_GTT_RESERVED;
@@ -385,8 +395,12 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
list_add_tail(&obj->global_list, &dev_priv->mm.bound_list); list_add_tail(&obj->mm_list, &dev_priv->mm.inactive_list);
return obj;
+unref_out:
drm_gem_object_unreference(&obj->base);
drm_mm_put_block(stolen);
"stolen" is already cleared by drm_gem_object_unreference(). So that's a double-free here.
The drm_mm_create_block() change looks good. Cheers David
Thanks for reviewing the i915 parts so thoroughly :D
return NULL;
}
void diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h index 88591ef..d8b56b7 100644 --- a/include/drm/drm_mm.h +++ b/include/drm/drm_mm.h @@ -138,10 +138,10 @@ static inline unsigned long drm_mm_hole_node_end(struct drm_mm_node *hole_node) /*
- Basic range manager support (drm_mm.c)
*/ -extern struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm,
unsigned long start,
unsigned long size,
bool atomic);
+extern int drm_mm_create_block(struct drm_mm *mm,
struct drm_mm_node *node,
unsigned long start,
unsigned long size);
extern struct drm_mm_node *drm_mm_get_block_generic(struct drm_mm_node *node, unsigned long size, unsigned alignment, @@ -155,6 +155,7 @@ extern struct drm_mm_node *drm_mm_get_block_range_generic( unsigned long start, unsigned long end, int atomic);
static inline struct drm_mm_node *drm_mm_get_block(struct drm_mm_node *parent, unsigned long size, unsigned alignment) -- 1.8.3.2
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi
On Wed, Jul 3, 2013 at 11:45 PM, Ben Widawsky ben@bwidawsk.net wrote:
For an upcoming patch where we introduce the i915 VMA, it's ideal to have the drm_mm_node as part of the VMA struct (ie. it's pre-allocated). Part of the conversion to VMAs is to kill off obj->gtt_space. Doing this will break a bunch of code, but amongst them are 2 callers of drm_mm_create_block(), both related to stolen memory.
It also allows us to embed the drm_mm_node into the object currently which provides a nice transition over to the new code.
v2: Reordered to do before ripping out obj->gtt_offset. Some minor cleanups made available because of reordering.
CC: dri-devel@lists.freedesktop.org Signed-off-by: Ben Widawsky ben@bwidawsk.net
drivers/gpu/drm/drm_mm.c | 16 +++++---------- drivers/gpu/drm/i915/i915_gem_gtt.c | 18 +++++++++++++---- drivers/gpu/drm/i915/i915_gem_stolen.c | 36 +++++++++++++++++++++++----------- include/drm/drm_mm.h | 9 +++++---- 4 files changed, 49 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c index 07cf99c..9e8dfbc 100644 --- a/drivers/gpu/drm/drm_mm.c +++ b/drivers/gpu/drm/drm_mm.c @@ -147,12 +147,10 @@ static void drm_mm_insert_helper(struct drm_mm_node *hole_node, } }
-struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm,
unsigned long start,
unsigned long size,
bool atomic)
+int drm_mm_create_block(struct drm_mm *mm, struct drm_mm_node *node,
unsigned long start, unsigned long size)
{
struct drm_mm_node *hole, *node;
struct drm_mm_node *hole; unsigned long end = start + size; unsigned long hole_start; unsigned long hole_end;
@@ -161,10 +159,6 @@ struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm, if (hole_start > start || hole_end < end) continue;
node = drm_mm_kmalloc(mm, atomic);
if (unlikely(node == NULL))
return NULL;
node->start = start; node->size = size; node->mm = mm;
@@ -184,11 +178,11 @@ struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm, node->hole_follows = 1; }
return node;
return 0; } WARN(1, "no hole found for block 0x%lx + 0x%lx\n", start, size);
return NULL;
return -ENOSPC;
} EXPORT_SYMBOL(drm_mm_create_block);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 66929ea..5c6fc0e 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -629,14 +629,24 @@ void i915_gem_setup_global_gtt(struct drm_device *dev,
/* Mark any preallocated objects as occupied */ list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
int ret; DRM_DEBUG_KMS("reserving preallocated space: %x + %zx\n", obj->gtt_offset, obj->base.size); BUG_ON(obj->gtt_space != I915_GTT_RESERVED);
obj->gtt_space = drm_mm_create_block(&dev_priv->mm.gtt_space,
obj->gtt_offset,
obj->base.size,
false);
obj->gtt_space = kzalloc(sizeof(*obj->gtt_space), GFP_KERNEL);
if (!obj->gtt_space) {
DRM_ERROR("Failed to preserve all objects\n");
break;
}
ret = drm_mm_create_block(&dev_priv->mm.gtt_space,
obj->gtt_space,
obj->gtt_offset,
obj->base.size);
if (ret) {
DRM_DEBUG_KMS("Reservation failed\n");
kfree(obj->gtt_space);
} obj->has_global_gtt_mapping = 1; }
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 8e02344..f9db84a 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -330,6 +330,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj; struct drm_mm_node *stolen;
int ret; if (dev_priv->mm.stolen_base == 0) return NULL;
@@ -344,11 +345,15 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, if (WARN_ON(size == 0)) return NULL;
stolen = drm_mm_create_block(&dev_priv->mm.stolen,
stolen_offset, size,
false);
if (stolen == NULL) {
stolen = kzalloc(sizeof(*stolen), GFP_KERNEL);
if (!stolen)
return NULL;
ret = drm_mm_create_block(&dev_priv->mm.stolen, stolen, stolen_offset,
size);
if (ret) { DRM_DEBUG_KMS("failed to allocate stolen space\n");
kfree(stolen); return NULL; }
@@ -369,13 +374,18 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, * later. */ if (drm_mm_initialized(&dev_priv->mm.gtt_space)) {
obj->gtt_space = drm_mm_create_block(&dev_priv->mm.gtt_space,
gtt_offset, size,
false);
if (obj->gtt_space == NULL) {
obj->gtt_space = kzalloc(sizeof(*obj->gtt_space), GFP_KERNEL);
if (!obj->gtt_space) {
DRM_DEBUG_KMS("-ENOMEM stolen GTT space\n");
goto unref_out;
}
ret = drm_mm_create_block(&dev_priv->mm.gtt_space,
obj->gtt_space,
gtt_offset, size);
if (ret) { DRM_DEBUG_KMS("failed to allocate stolen GTT space\n");
drm_gem_object_unreference(&obj->base);
return NULL;
goto unref_out; } } else obj->gtt_space = I915_GTT_RESERVED;
@@ -385,8 +395,12 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
list_add_tail(&obj->global_list, &dev_priv->mm.bound_list); list_add_tail(&obj->mm_list, &dev_priv->mm.inactive_list);
return obj;
+unref_out:
drm_gem_object_unreference(&obj->base);
drm_mm_put_block(stolen);
return NULL;
}
void diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h index 88591ef..d8b56b7 100644 --- a/include/drm/drm_mm.h +++ b/include/drm/drm_mm.h @@ -138,10 +138,10 @@ static inline unsigned long drm_mm_hole_node_end(struct drm_mm_node *hole_node) /*
- Basic range manager support (drm_mm.c)
*/ -extern struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm,
unsigned long start,
unsigned long size,
bool atomic);
+extern int drm_mm_create_block(struct drm_mm *mm,
struct drm_mm_node *node,
unsigned long start,
unsigned long size);
extern struct drm_mm_node *drm_mm_get_block_generic(struct drm_mm_node *node, unsigned long size, unsigned alignment, @@ -155,6 +155,7 @@ extern struct drm_mm_node *drm_mm_get_block_range_generic( unsigned long start, unsigned long end, int atomic);
Nitpick: This newline doesn't belong in this patch.
Cheers David
static inline struct drm_mm_node *drm_mm_get_block(struct drm_mm_node *parent, unsigned long size, unsigned alignment) -- 1.8.3.2
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
For an upcoming patch where we introduce the i915 VMA, it's ideal to have the drm_mm_node as part of the VMA struct (ie. it's pre-allocated). Part of the conversion to VMAs is to kill off obj->gtt_space. Doing this will break a bunch of code, but amongst them are 2 callers of drm_mm_create_block(), both related to stolen memory.
It also allows us to embed the drm_mm_node into the object currently which provides a nice transition over to the new code.
v2: Reordered to do before ripping out obj->gtt_offset. Some minor cleanups made available because of reordering.
v3: s/continue/break on failed stolen node allocation (David) Set obj->gtt_space on failed node allocation (David) Only unref stolen (fix double free) on failed create_stolen (David) Free node, and NULL it in failed create_stolen (David) Add back accidentally removed newline (David)
CC: dri-devel@lists.freedesktop.org CC: David Herrmann dh.herrmann@gmail.com Signed-off-by: Ben Widawsky ben@bwidawsk.net --- drivers/gpu/drm/drm_mm.c | 16 +++++---------- drivers/gpu/drm/i915/i915_gem_gtt.c | 20 ++++++++++++++---- drivers/gpu/drm/i915/i915_gem_stolen.c | 37 +++++++++++++++++++++++++--------- include/drm/drm_mm.h | 9 +++++---- 4 files changed, 53 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c index 07cf99c..9e8dfbc 100644 --- a/drivers/gpu/drm/drm_mm.c +++ b/drivers/gpu/drm/drm_mm.c @@ -147,12 +147,10 @@ static void drm_mm_insert_helper(struct drm_mm_node *hole_node, } }
-struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm, - unsigned long start, - unsigned long size, - bool atomic) +int drm_mm_create_block(struct drm_mm *mm, struct drm_mm_node *node, + unsigned long start, unsigned long size) { - struct drm_mm_node *hole, *node; + struct drm_mm_node *hole; unsigned long end = start + size; unsigned long hole_start; unsigned long hole_end; @@ -161,10 +159,6 @@ struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm, if (hole_start > start || hole_end < end) continue;
- node = drm_mm_kmalloc(mm, atomic); - if (unlikely(node == NULL)) - return NULL; - node->start = start; node->size = size; node->mm = mm; @@ -184,11 +178,11 @@ struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm, node->hole_follows = 1; }
- return node; + return 0; }
WARN(1, "no hole found for block 0x%lx + 0x%lx\n", start, size); - return NULL; + return -ENOSPC; } EXPORT_SYMBOL(drm_mm_create_block);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 66929ea..88180a5 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -629,14 +629,26 @@ void i915_gem_setup_global_gtt(struct drm_device *dev,
/* Mark any preallocated objects as occupied */ list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) { + int ret; DRM_DEBUG_KMS("reserving preallocated space: %x + %zx\n", obj->gtt_offset, obj->base.size);
BUG_ON(obj->gtt_space != I915_GTT_RESERVED); - obj->gtt_space = drm_mm_create_block(&dev_priv->mm.gtt_space, - obj->gtt_offset, - obj->base.size, - false); + obj->gtt_space = kzalloc(sizeof(*obj->gtt_space), GFP_KERNEL); + if (!obj->gtt_space) { + DRM_ERROR("Failed to preserve object at offset %x\n", + obj->gtt_offset); + continue; + } + ret = drm_mm_create_block(&dev_priv->mm.gtt_space, + obj->gtt_space, + obj->gtt_offset, + obj->base.size); + if (ret) { + DRM_DEBUG_KMS("Reservation failed\n"); + kfree(obj->gtt_space); + obj->gtt_space = NULL; + } obj->has_global_gtt_mapping = 1; }
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 8e02344..fb19d00 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -330,6 +330,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj; struct drm_mm_node *stolen; + int ret;
if (dev_priv->mm.stolen_base == 0) return NULL; @@ -344,11 +345,15 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, if (WARN_ON(size == 0)) return NULL;
- stolen = drm_mm_create_block(&dev_priv->mm.stolen, - stolen_offset, size, - false); - if (stolen == NULL) { + stolen = kzalloc(sizeof(*stolen), GFP_KERNEL); + if (!stolen) + return NULL; + + ret = drm_mm_create_block(&dev_priv->mm.stolen, stolen, stolen_offset, + size); + if (ret) { DRM_DEBUG_KMS("failed to allocate stolen space\n"); + kfree(stolen); return NULL; }
@@ -369,13 +374,18 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, * later. */ if (drm_mm_initialized(&dev_priv->mm.gtt_space)) { - obj->gtt_space = drm_mm_create_block(&dev_priv->mm.gtt_space, - gtt_offset, size, - false); - if (obj->gtt_space == NULL) { + obj->gtt_space = kzalloc(sizeof(*obj->gtt_space), GFP_KERNEL); + if (!obj->gtt_space) { + DRM_DEBUG_KMS("-ENOMEM stolen GTT space\n"); + goto unref_out; + } + + ret = drm_mm_create_block(&dev_priv->mm.gtt_space, + obj->gtt_space, + gtt_offset, size); + if (ret) { DRM_DEBUG_KMS("failed to allocate stolen GTT space\n"); - drm_gem_object_unreference(&obj->base); - return NULL; + goto free_out; } } else obj->gtt_space = I915_GTT_RESERVED; @@ -387,6 +397,13 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, list_add_tail(&obj->mm_list, &dev_priv->mm.inactive_list);
return obj; + +free_out: + kfree(obj->gtt_space); + obj->gtt_space = NULL; +unref_out: + drm_gem_object_unreference(&obj->base); + return NULL; }
void diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h index 88591ef..d8b56b7 100644 --- a/include/drm/drm_mm.h +++ b/include/drm/drm_mm.h @@ -138,10 +138,10 @@ static inline unsigned long drm_mm_hole_node_end(struct drm_mm_node *hole_node) /* * Basic range manager support (drm_mm.c) */ -extern struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm, - unsigned long start, - unsigned long size, - bool atomic); +extern int drm_mm_create_block(struct drm_mm *mm, + struct drm_mm_node *node, + unsigned long start, + unsigned long size); extern struct drm_mm_node *drm_mm_get_block_generic(struct drm_mm_node *node, unsigned long size, unsigned alignment, @@ -155,6 +155,7 @@ extern struct drm_mm_node *drm_mm_get_block_range_generic( unsigned long start, unsigned long end, int atomic); + static inline struct drm_mm_node *drm_mm_get_block(struct drm_mm_node *parent, unsigned long size, unsigned alignment)
Hi
On Thu, Jul 4, 2013 at 10:14 PM, Ben Widawsky ben@bwidawsk.net wrote:
For an upcoming patch where we introduce the i915 VMA, it's ideal to have the drm_mm_node as part of the VMA struct (ie. it's pre-allocated). Part of the conversion to VMAs is to kill off obj->gtt_space. Doing this will break a bunch of code, but amongst them are 2 callers of drm_mm_create_block(), both related to stolen memory.
It also allows us to embed the drm_mm_node into the object currently which provides a nice transition over to the new code.
v2: Reordered to do before ripping out obj->gtt_offset. Some minor cleanups made available because of reordering.
v3: s/continue/break on failed stolen node allocation (David) Set obj->gtt_space on failed node allocation (David) Only unref stolen (fix double free) on failed create_stolen (David) Free node, and NULL it in failed create_stolen (David) Add back accidentally removed newline (David)
CC: dri-devel@lists.freedesktop.org CC: David Herrmann dh.herrmann@gmail.com Signed-off-by: Ben Widawsky ben@bwidawsk.net
I already suspected that you'd embed drm_mm_node in a follow-up patch but I am not subscribed to intel-gfx so I didn't get the other patches. Looks good now.
Reviewed-by: David Herrmann dh.herrmann@gmail.com
Cheers David
drivers/gpu/drm/drm_mm.c | 16 +++++---------- drivers/gpu/drm/i915/i915_gem_gtt.c | 20 ++++++++++++++---- drivers/gpu/drm/i915/i915_gem_stolen.c | 37 +++++++++++++++++++++++++--------- include/drm/drm_mm.h | 9 +++++---- 4 files changed, 53 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c index 07cf99c..9e8dfbc 100644 --- a/drivers/gpu/drm/drm_mm.c +++ b/drivers/gpu/drm/drm_mm.c @@ -147,12 +147,10 @@ static void drm_mm_insert_helper(struct drm_mm_node *hole_node, } }
-struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm,
unsigned long start,
unsigned long size,
bool atomic)
+int drm_mm_create_block(struct drm_mm *mm, struct drm_mm_node *node,
unsigned long start, unsigned long size)
{
struct drm_mm_node *hole, *node;
struct drm_mm_node *hole; unsigned long end = start + size; unsigned long hole_start; unsigned long hole_end;
@@ -161,10 +159,6 @@ struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm, if (hole_start > start || hole_end < end) continue;
node = drm_mm_kmalloc(mm, atomic);
if (unlikely(node == NULL))
return NULL;
node->start = start; node->size = size; node->mm = mm;
@@ -184,11 +178,11 @@ struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm, node->hole_follows = 1; }
return node;
return 0; } WARN(1, "no hole found for block 0x%lx + 0x%lx\n", start, size);
return NULL;
return -ENOSPC;
} EXPORT_SYMBOL(drm_mm_create_block);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 66929ea..88180a5 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -629,14 +629,26 @@ void i915_gem_setup_global_gtt(struct drm_device *dev,
/* Mark any preallocated objects as occupied */ list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
int ret; DRM_DEBUG_KMS("reserving preallocated space: %x + %zx\n", obj->gtt_offset, obj->base.size); BUG_ON(obj->gtt_space != I915_GTT_RESERVED);
obj->gtt_space = drm_mm_create_block(&dev_priv->mm.gtt_space,
obj->gtt_offset,
obj->base.size,
false);
obj->gtt_space = kzalloc(sizeof(*obj->gtt_space), GFP_KERNEL);
if (!obj->gtt_space) {
DRM_ERROR("Failed to preserve object at offset %x\n",
obj->gtt_offset);
continue;
}
ret = drm_mm_create_block(&dev_priv->mm.gtt_space,
obj->gtt_space,
obj->gtt_offset,
obj->base.size);
if (ret) {
DRM_DEBUG_KMS("Reservation failed\n");
kfree(obj->gtt_space);
obj->gtt_space = NULL;
} obj->has_global_gtt_mapping = 1; }
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 8e02344..fb19d00 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -330,6 +330,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj; struct drm_mm_node *stolen;
int ret; if (dev_priv->mm.stolen_base == 0) return NULL;
@@ -344,11 +345,15 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, if (WARN_ON(size == 0)) return NULL;
stolen = drm_mm_create_block(&dev_priv->mm.stolen,
stolen_offset, size,
false);
if (stolen == NULL) {
stolen = kzalloc(sizeof(*stolen), GFP_KERNEL);
if (!stolen)
return NULL;
ret = drm_mm_create_block(&dev_priv->mm.stolen, stolen, stolen_offset,
size);
if (ret) { DRM_DEBUG_KMS("failed to allocate stolen space\n");
kfree(stolen); return NULL; }
@@ -369,13 +374,18 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, * later. */ if (drm_mm_initialized(&dev_priv->mm.gtt_space)) {
obj->gtt_space = drm_mm_create_block(&dev_priv->mm.gtt_space,
gtt_offset, size,
false);
if (obj->gtt_space == NULL) {
obj->gtt_space = kzalloc(sizeof(*obj->gtt_space), GFP_KERNEL);
if (!obj->gtt_space) {
DRM_DEBUG_KMS("-ENOMEM stolen GTT space\n");
goto unref_out;
}
ret = drm_mm_create_block(&dev_priv->mm.gtt_space,
obj->gtt_space,
gtt_offset, size);
if (ret) { DRM_DEBUG_KMS("failed to allocate stolen GTT space\n");
drm_gem_object_unreference(&obj->base);
return NULL;
goto free_out; } } else obj->gtt_space = I915_GTT_RESERVED;
@@ -387,6 +397,13 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, list_add_tail(&obj->mm_list, &dev_priv->mm.inactive_list);
return obj;
+free_out:
kfree(obj->gtt_space);
obj->gtt_space = NULL;
+unref_out:
drm_gem_object_unreference(&obj->base);
return NULL;
}
void diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h index 88591ef..d8b56b7 100644 --- a/include/drm/drm_mm.h +++ b/include/drm/drm_mm.h @@ -138,10 +138,10 @@ static inline unsigned long drm_mm_hole_node_end(struct drm_mm_node *hole_node) /*
- Basic range manager support (drm_mm.c)
*/ -extern struct drm_mm_node *drm_mm_create_block(struct drm_mm *mm,
unsigned long start,
unsigned long size,
bool atomic);
+extern int drm_mm_create_block(struct drm_mm *mm,
struct drm_mm_node *node,
unsigned long start,
unsigned long size);
extern struct drm_mm_node *drm_mm_get_block_generic(struct drm_mm_node *node, unsigned long size, unsigned alignment, @@ -155,6 +155,7 @@ extern struct drm_mm_node *drm_mm_get_block_range_generic( unsigned long start, unsigned long end, int atomic);
static inline struct drm_mm_node *drm_mm_get_block(struct drm_mm_node *parent, unsigned long size, unsigned alignment) -- 1.8.3.2
On Thu, Jul 04, 2013 at 10:32:27PM +0200, David Herrmann wrote:
Hi
On Thu, Jul 4, 2013 at 10:14 PM, Ben Widawsky ben@bwidawsk.net wrote:
For an upcoming patch where we introduce the i915 VMA, it's ideal to have the drm_mm_node as part of the VMA struct (ie. it's pre-allocated). Part of the conversion to VMAs is to kill off obj->gtt_space. Doing this will break a bunch of code, but amongst them are 2 callers of drm_mm_create_block(), both related to stolen memory.
It also allows us to embed the drm_mm_node into the object currently which provides a nice transition over to the new code.
v2: Reordered to do before ripping out obj->gtt_offset. Some minor cleanups made available because of reordering.
v3: s/continue/break on failed stolen node allocation (David) Set obj->gtt_space on failed node allocation (David) Only unref stolen (fix double free) on failed create_stolen (David) Free node, and NULL it in failed create_stolen (David) Add back accidentally removed newline (David)
CC: dri-devel@lists.freedesktop.org CC: David Herrmann dh.herrmann@gmail.com Signed-off-by: Ben Widawsky ben@bwidawsk.net
I already suspected that you'd embed drm_mm_node in a follow-up patch but I am not subscribed to intel-gfx so I didn't get the other patches. Looks good now.
Reviewed-by: David Herrmann dh.herrmann@gmail.com
Ok, I've discussed this a bit with Dave on irc and he's a bit unhappy with the creat_block name. After a bit of chatting with Ben I think we should go with
int drm_mm_reserve_node(struct drm_mm *mm, struct drm_mm_node *node);
The start/size/color arguments would then be passed in through the drm_mm_node argument. Ben asked whether that's not too much poking around in drm_mm_node internals, but imo those three pieces of it are part of the public interface to drm_mm users.
Also, the patch as-is conflicts a bit too badly with my current tree. So can you please apply the little bikeshed, rebase, and then rebase the other patches in this series that I haven't merged on top of this?
Thanks, Daniel
On Fri, Jul 05, 2013 at 09:25:35PM +0200, Daniel Vetter wrote:
On Thu, Jul 04, 2013 at 10:32:27PM +0200, David Herrmann wrote:
Hi
On Thu, Jul 4, 2013 at 10:14 PM, Ben Widawsky ben@bwidawsk.net wrote:
For an upcoming patch where we introduce the i915 VMA, it's ideal to have the drm_mm_node as part of the VMA struct (ie. it's pre-allocated). Part of the conversion to VMAs is to kill off obj->gtt_space. Doing this will break a bunch of code, but amongst them are 2 callers of drm_mm_create_block(), both related to stolen memory.
It also allows us to embed the drm_mm_node into the object currently which provides a nice transition over to the new code.
v2: Reordered to do before ripping out obj->gtt_offset. Some minor cleanups made available because of reordering.
v3: s/continue/break on failed stolen node allocation (David) Set obj->gtt_space on failed node allocation (David) Only unref stolen (fix double free) on failed create_stolen (David) Free node, and NULL it in failed create_stolen (David) Add back accidentally removed newline (David)
CC: dri-devel@lists.freedesktop.org CC: David Herrmann dh.herrmann@gmail.com Signed-off-by: Ben Widawsky ben@bwidawsk.net
I already suspected that you'd embed drm_mm_node in a follow-up patch but I am not subscribed to intel-gfx so I didn't get the other patches. Looks good now.
Reviewed-by: David Herrmann dh.herrmann@gmail.com
Ok, I've discussed this a bit with Dave on irc and he's a bit unhappy with the creat_block name. After a bit of chatting with Ben I think we should go with
int drm_mm_reserve_node(struct drm_mm *mm, struct drm_mm_node *node);
The start/size/color arguments would then be passed in through the drm_mm_node argument. Ben asked whether that's not too much poking around in drm_mm_node internals, but imo those three pieces of it are part of the public interface to drm_mm users.
Do you mind if I leave the patch as is, since it's reviewed, and put the rename patch on top? I have a long history of screwing up simple rebases.
Also, the patch as-is conflicts a bit too badly with my current tree. So can you please apply the little bikeshed, rebase, and then rebase the other patches in this series that I haven't merged on top of this?
Thanks, Daniel
Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
dri-devel@lists.freedesktop.org