DRM: Remove MM "Pre-Alloc"

Add a "best_match" argument similar to the drm_mm_search_*() helpers so we can convert TTM to use them in follow up patches. We can also inline the non-generic helpers and move them into the header to allow compile-time optimizations.

Signed-off-by David Herrmann <[email protected]>

---
 drivers/gpu/drm/drm_mm.c          | 23 ++++-------------------
 drivers/gpu/drm/drm_vma_manager.c |  4 ++--
 drivers/gpu/drm/i915/i915_gem.c   |  3 ++-
 drivers/gpu/drm/sis/sis_mm.c      |  4 ++--
 drivers/gpu/drm/via/via_mm.c      |  4 ++--
 include/drm/drm_mm.h              | 38 ++++++++++++++++++++++++++------------
 6 files changed, 38 insertions(+), 38 deletions(-)

drivers/gpu/drm/drm_mm.c +0 -0

index fe304f9..5aad736 100644

drivers/gpu/drm/drm_mm.c +3 -18

 {
 	struct drm_mm_node *hole_node;
 
 	hole_node = drm_mm_search_free_generic(mm, size, alignment,
-					       color, 0);
+					       color, best_match);
 	if (!hole_node)
 		return -ENOSPC;
 

@@ -226,13 +226,6 @@ int drm_mm_insert_node_generic(struct drm_mm *mm, struct drm_mm_node *node,
 }
 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,

@@ -313,13 +306,13 @@ 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, bool best_match)
 {
 	struct drm_mm_node *hole_node;
 
 	hole_node = drm_mm_search_free_in_range_generic(mm,
 							size, alignment, color,
-							start, end, 0);
+							start, end, best_match);
 	if (!hole_node)
 		return -ENOSPC;
 

@@ -330,14 +323,6 @@ int drm_mm_insert_node_in_range_generic(struct drm_mm *mm, struct drm_mm_node *n
 }
 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.
  */

drivers/gpu/drm/drm_vma_manager.c +2 -2

index b966cea..4285b38 100644
@@ -241,8 +241,8 @@ int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
 		goto out_unlock;
 	}
 
-	ret = drm_mm_insert_node_generic(&mgr->vm_addr_space_mm,
-					 &node->vm_node, pages, 0, 0);
+	ret = drm_mm_insert_node(&mgr->vm_addr_space_mm, &node->vm_node,
+				 pages, 0, false);
 	if (ret)
 		goto out_unlock;
 

drivers/gpu/drm/i915/i915_gem.c +2 -1

index 8673a00..6667322 100644
@@ -3092,7 +3092,8 @@ search_free:
 	ret = drm_mm_insert_node_in_range_generic(&dev_priv->mm.gtt_space,
 						  &obj->gtt_space,
 						  size, alignment,
-						  obj->cache_level, 0, gtt_max);
+						  obj->cache_level, 0, gtt_max,
+						  false);
 	if (ret) {
 		ret = i915_gem_evict_something(dev, size, alignment,
 					       obj->cache_level,

drivers/gpu/drm/sis/sis_mm.c +2 -2

index 9a43d98..85da1a4 100644
@@ -109,7 +109,7 @@ static int sis_drm_alloc(struct drm_device *dev, struct drm_file *file,
 	if (pool == AGP_TYPE) {
 		retval = drm_mm_insert_node(&dev_priv->agp_mm,
 					    &item->mm_node,
-					    mem->size, 0);
+					    mem->size, 0, false);
 		offset = item->mm_node.start;
 	} else {
 #if defined(CONFIG_FB_SIS) || defined(CONFIG_FB_SIS_MODULE)

@@ -121,7 +121,7 @@ static int sis_drm_alloc(struct drm_device *dev, struct drm_file *file,
 #else
 		retval = drm_mm_insert_node(&dev_priv->vram_mm,
 					    &item->mm_node,
-					    mem->size, 0);
+					    mem->size, 0, false);
 		offset = item->mm_node.start;
 #endif
 	}

drivers/gpu/drm/via/via_mm.c +2 -2

index 0ab93ff..10b962e 100644
@@ -140,11 +140,11 @@ int via_mem_alloc(struct drm_device *dev, void *data,
 	if (mem->type == VIA_MEM_AGP)
 		retval = drm_mm_insert_node(&dev_priv->agp_mm,
 					    &item->mm_node,
-					    tmpSize, 0);
+					    tmpSize, 0, false);
 	else
 		retval = drm_mm_insert_node(&dev_priv->vram_mm,
 					    &item->mm_node,
-					    tmpSize, 0);
+					    tmpSize, 0, false);
 	if (retval)
 		goto fail_alloc;
 

include/drm/drm_mm.h +26 -12

index b87d05e..a30c9aa 100644
@@ -186,28 +186,42 @@ static inline struct drm_mm_node *drm_mm_get_block_atomic_range(
 						start, end, 1);
 }
 
-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);
+				      unsigned long color,
+				      bool best_match);
+static inline int drm_mm_insert_node(struct drm_mm *mm,
+				     struct drm_mm_node *node,
+				     unsigned long size,
+				     unsigned alignment,
+				     bool best_match)
+{
+	return drm_mm_insert_node_generic(mm, node, size, alignment, 0,
+					  best_match);
+}
+
 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 end,
+				       bool best_match);
+static inline 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,
+					      bool best_match)
+{
+	return drm_mm_insert_node_in_range_generic(mm, node, size, alignment,
+						   0, start, end, best_match);
+}
+
 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);

Signature · 1 line

1.8.3.3

Add a "best_match" flag similar to the drm_mm_search_*() helpers so we can convert TTM to use them in follow up patches. We can also inline the non-generic helpers and move them into the header to allow compile-time optimizations.

To make calls to drm_mm_{search,insert}_node() more readable, this converts the boolean argument to a flagset. There are pending patches that add additional flags for top-down allocators and more.

v2:
- use flag parameter instead of boolean "best_match"
- convert *_search_free() helpers to also use flags argument

Signed-off-by David Herrmann <[email protected]>

---
 drivers/gpu/drm/drm_mm.c               | 37 ++++++++---------------
 drivers/gpu/drm/drm_vma_manager.c      |  4 +--
 drivers/gpu/drm/i915/i915_gem.c        |  3 +-
 drivers/gpu/drm/i915/i915_gem_stolen.c | 12 +++++---
 drivers/gpu/drm/sis/sis_mm.c           |  6 ++--
 drivers/gpu/drm/ttm/ttm_bo_manager.c   |  3 +-
 drivers/gpu/drm/via/via_mm.c           |  4 +--
 include/drm/drm_mm.h                   | 54 ++++++++++++++++++++++------------
 8 files changed, 68 insertions(+), 55 deletions(-)

drivers/gpu/drm/drm_mm.c +12 -25

index fe304f9..9a38327 100644
@@ -212,12 +212,13 @@ 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_search_flags flags)
 {
 	struct drm_mm_node *hole_node;
 
 	hole_node = drm_mm_search_free_generic(mm, size, alignment,
-					       color, 0);
+					       color, flags);
 	if (!hole_node)
 		return -ENOSPC;
 

@@ -226,13 +227,6 @@ int drm_mm_insert_node_generic(struct drm_mm *mm, struct drm_mm_node *node,
 }
 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,

@@ -313,13 +307,14 @@ 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_search_flags flags)
 {
 	struct drm_mm_node *hole_node;
 
 	hole_node = drm_mm_search_free_in_range_generic(mm,
 							size, alignment, color,
-							start, end, 0);
+							start, end, flags);
 	if (!hole_node)
 		return -ENOSPC;
 

@@ -330,14 +325,6 @@ int drm_mm_insert_node_in_range_generic(struct drm_mm *mm, struct drm_mm_node *n
 }
 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.
  */

@@ -413,7 +400,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;

@@ -436,7 +423,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))
 			return entry;
 
 		if (entry->size < best_size) {

@@ -455,7 +442,7 @@ struct drm_mm_node *drm_mm_search_free_in_range_generic(const struct drm_mm *mm,
 							unsigned long color,
 							unsigned long start,
 							unsigned long end,
-							bool best_match)
+							enum drm_mm_search_flags flags)
 {
 	struct drm_mm_node *entry;
 	struct drm_mm_node *best;

@@ -483,7 +470,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))
 			return entry;
 
 		if (entry->size < best_size) {

@@ -629,8 +616,8 @@ EXPORT_SYMBOL(drm_mm_scan_add_block);
  * corrupted.
  *
  * When the scan list is empty, the selected memory nodes can be freed. An
- * immediately following drm_mm_search_free with best_match = 0 will then return
- * the just freed block (because its at the top of the free_stack list).
+ * immediately following drm_mm_search_free with !DRM_MM_SEARCH_BEST will then
+ * return the just freed block (because its at the top of the free_stack list).
  *
  * Returns one if this block should be evicted, zero otherwise. Will always
  * return zero when no hole has been found.

drivers/gpu/drm/drm_vma_manager.c +2 -2

index b966cea..3837481 100644
@@ -241,8 +241,8 @@ int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
 		goto out_unlock;
 	}
 
-	ret = drm_mm_insert_node_generic(&mgr->vm_addr_space_mm,
-					 &node->vm_node, pages, 0, 0);
+	ret = drm_mm_insert_node(&mgr->vm_addr_space_mm, &node->vm_node,
+				 pages, 0, DRM_MM_SEARCH_DEFAULT);
 	if (ret)
 		goto out_unlock;
 

drivers/gpu/drm/i915/i915_gem.c +2 -1

index 8673a00..f3065a0 100644
@@ -3092,7 +3092,8 @@ search_free:
 	ret = drm_mm_insert_node_in_range_generic(&dev_priv->mm.gtt_space,
 						  &obj->gtt_space,
 						  size, alignment,
-						  obj->cache_level, 0, gtt_max);
+						  obj->cache_level, 0, gtt_max,
+						  DRM_MM_SEARCH_DEFAULT);
 	if (ret) {
 		ret = i915_gem_evict_something(dev, size, alignment,
 					       obj->cache_level,

drivers/gpu/drm/i915/i915_gem_stolen.c +8 -4

index 5521833..e355170 100644
@@ -115,10 +115,12 @@ static int i915_setup_compression(struct drm_device *dev, int size)
 
 	/* Try to over-allocate to reduce reallocations and fragmentation */
 	compressed_fb = drm_mm_search_free(&dev_priv->mm.stolen,
-					   size <<= 1, 4096, 0);
+					   size <<= 1, 4096,
+					   DRM_MM_SEARCH_DEFAULT);
 	if (!compressed_fb)
 		compressed_fb = drm_mm_search_free(&dev_priv->mm.stolen,
-						   size >>= 1, 4096, 0);
+						   size >>= 1, 4096,
+						   DRM_MM_SEARCH_DEFAULT);
 	if (compressed_fb)
 		compressed_fb = drm_mm_get_block(compressed_fb, size, 4096);
 	if (!compressed_fb)

@@ -130,7 +132,8 @@ static int i915_setup_compression(struct drm_device *dev, int size)
 		I915_WRITE(DPFC_CB_BASE, compressed_fb->start);
 	} else {
 		compressed_llb = drm_mm_search_free(&dev_priv->mm.stolen,
-						    4096, 4096, 0);
+						    4096, 4096,
+						    DRM_MM_SEARCH_DEFAULT);
 		if (compressed_llb)
 			compressed_llb = drm_mm_get_block(compressed_llb,
 							  4096, 4096);

@@ -328,7 +331,8 @@ i915_gem_object_create_stolen(struct drm_device *dev, u32 size)
 	if (size == 0)
 		return NULL;
 
-	stolen = drm_mm_search_free(&dev_priv->mm.stolen, size, 4096, 0);
+	stolen = drm_mm_search_free(&dev_priv->mm.stolen, size, 4096,
+				    DRM_MM_SEARCH_DEFAULT);
 	if (stolen)
 		stolen = drm_mm_get_block(stolen, size, 4096);
 	if (stolen == NULL)

drivers/gpu/drm/sis/sis_mm.c +4 -2

index 9a43d98..23a2349 100644
@@ -109,7 +109,8 @@ static int sis_drm_alloc(struct drm_device *dev, struct drm_file *file,
 	if (pool == AGP_TYPE) {
 		retval = drm_mm_insert_node(&dev_priv->agp_mm,
 					    &item->mm_node,
-					    mem->size, 0);
+					    mem->size, 0,
+					    DRM_MM_SEARCH_DEFAULT);
 		offset = item->mm_node.start;
 	} else {
 #if defined(CONFIG_FB_SIS) || defined(CONFIG_FB_SIS_MODULE)

@@ -121,7 +122,8 @@ static int sis_drm_alloc(struct drm_device *dev, struct drm_file *file,
 #else
 		retval = drm_mm_insert_node(&dev_priv->vram_mm,
 					    &item->mm_node,
-					    mem->size, 0);
+					    mem->size, 0,
+					    DRM_MM_SEARCH_DEFAULT);
 		offset = item->mm_node.start;
 #endif
 	}

drivers/gpu/drm/ttm/ttm_bo_manager.c +2 -1

index e4367f9..e4be29e 100644
@@ -69,7 +69,8 @@ static int ttm_bo_man_get_node(struct ttm_mem_type_manager *man,
 		spin_lock(&rman->lock);
 		node = drm_mm_search_free_in_range(mm,
 					mem->num_pages, mem->page_alignment,
-					placement->fpfn, lpfn, 1);
+					placement->fpfn, lpfn,
+					DRM_MM_SEARCH_BEST);
 		if (unlikely(node == NULL)) {
 			spin_unlock(&rman->lock);
 			return 0;

drivers/gpu/drm/via/via_mm.c +2 -2

index 0ab93ff..7e3ad87 100644
@@ -140,11 +140,11 @@ int via_mem_alloc(struct drm_device *dev, void *data,
 	if (mem->type == VIA_MEM_AGP)
 		retval = drm_mm_insert_node(&dev_priv->agp_mm,
 					    &item->mm_node,
-					    tmpSize, 0);
+					    tmpSize, 0, DRM_MM_SEARCH_DEFAULT);
 	else
 		retval = drm_mm_insert_node(&dev_priv->vram_mm,
 					    &item->mm_node,
-					    tmpSize, 0);
+					    tmpSize, 0, DRM_MM_SEARCH_DEFAULT);
 	if (retval)
 		goto fail_alloc;
 

include/drm/drm_mm.h +36 -18

index 98cb50e..439d1a1 100644
@@ -44,6 +44,11 @@
 #include <linux/seq_file.h>
 #endif
 
+enum drm_mm_search_flags {
+	DRM_MM_SEARCH_DEFAULT =		0,
+	DRM_MM_SEARCH_BEST =		1 << 0,
+};
+
 struct drm_mm_node {
 	struct list_head node_list;
 	struct list_head hole_stack;

@@ -189,28 +194,41 @@ static inline struct drm_mm_node *drm_mm_get_block_atomic_range(
 						start, end, 1);
 }
 
-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);
+				      unsigned long color,
+				      enum drm_mm_search_flags flags);
+static inline int drm_mm_insert_node(struct drm_mm *mm,
+				     struct drm_mm_node *node,
+				     unsigned long size,
+				     unsigned alignment,
+				     enum drm_mm_search_flags flags)
+{
+	return drm_mm_insert_node_generic(mm, node, size, alignment, 0, flags);
+}
+
 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 end,
+				       enum drm_mm_search_flags flags);
+static inline 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,
+					      enum drm_mm_search_flags flags)
+{
+	return drm_mm_insert_node_in_range_generic(mm, node, size, alignment,
+						   0, start, end, flags);
+}
+
 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);

@@ -218,7 +236,7 @@ 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);
+						      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,

@@ -226,13 +244,13 @@ extern struct drm_mm_node *drm_mm_search_free_in_range_generic(
 						unsigned long color,
 						unsigned long start,
 						unsigned long end,
-						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,
-						     bool best_match)
+						     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,

@@ -240,10 +258,10 @@ 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);
 }
 
 extern void drm_mm_init(struct drm_mm *mm,

Signature · 1 line

1.8.3.4

Quoted 9 lines · David Herrmann

Add a "best_match" flag similar to the drm_mm_search_*() helpers so we can convert TTM to use them in follow up patches. We can also inline the non-generic helpers and move them into the header to allow compile-time optimizations.

To make calls to drm_mm_{search,insert}_node() more readable, this converts the boolean argument to a flagset. There are pending patches that add additional flags for top-down allocators and more.

Quoted 5 lines

v2:
- use flag parameter instead of boolean "best_match"
- convert *_search_free() helpers to also use flags argument

Signed-off-by David Herrmann <[email protected]>

I'm not sure whether the use of an enum here is proper C style (usually bitflag arguments tend to be unsigned ints), but that's a bikeshed. So

Reviewed-by Daniel Vetter <[email protected]>

Quoted 358 lines

---
 drivers/gpu/drm/drm_mm.c               | 37 ++++++++---------------
 drivers/gpu/drm/drm_vma_manager.c      |  4 +--
 drivers/gpu/drm/i915/i915_gem.c        |  3 +-
 drivers/gpu/drm/i915/i915_gem_stolen.c | 12 +++++---
 drivers/gpu/drm/sis/sis_mm.c           |  6 ++--
 drivers/gpu/drm/ttm/ttm_bo_manager.c   |  3 +-
 drivers/gpu/drm/via/via_mm.c           |  4 +--
 include/drm/drm_mm.h                   | 54 ++++++++++++++++++++++------------
 8 files changed, 68 insertions(+), 55 deletions(-)

drivers/gpu/drm/drm_mm.c +12 -25

index fe304f9..9a38327 100644
@@ -212,12 +212,13 @@ 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_search_flags flags)
 {
      struct drm_mm_node *hole_node;
 
      hole_node = drm_mm_search_free_generic(mm, size, alignment,
-                                            color, 0);
+                                            color, flags);
      if (!hole_node)
              return -ENOSPC;
 

@@ -226,13 +227,6 @@ int drm_mm_insert_node_generic(struct drm_mm *mm, struct drm_mm_node *node,
 }
 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,

@@ -313,13 +307,14 @@ 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_search_flags flags)
 {
      struct drm_mm_node *hole_node;
 
      hole_node = drm_mm_search_free_in_range_generic(mm,
                                                      size, alignment, color,
-                                                     start, end, 0);
+                                                     start, end, flags);
      if (!hole_node)
              return -ENOSPC;
 

@@ -330,14 +325,6 @@ int drm_mm_insert_node_in_range_generic(struct drm_mm *mm, struct drm_mm_node *n
 }
 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.
  */

@@ -413,7 +400,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;

@@ -436,7 +423,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))
                      return entry;
 
              if (entry->size < best_size) {

@@ -455,7 +442,7 @@ struct drm_mm_node *drm_mm_search_free_in_range_generic(const struct drm_mm *mm,
                                                      unsigned long color,
                                                      unsigned long start,
                                                      unsigned long end,
-                                                     bool best_match)
+                                                     enum drm_mm_search_flags flags)
 {
      struct drm_mm_node *entry;
      struct drm_mm_node *best;

@@ -483,7 +470,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))
                      return entry;
 
              if (entry->size < best_size) {

@@ -629,8 +616,8 @@ EXPORT_SYMBOL(drm_mm_scan_add_block);
  * corrupted.
  *
  * When the scan list is empty, the selected memory nodes can be freed. An
- * immediately following drm_mm_search_free with best_match = 0 will then return
- * the just freed block (because its at the top of the free_stack list).
+ * immediately following drm_mm_search_free with !DRM_MM_SEARCH_BEST will then
+ * return the just freed block (because its at the top of the free_stack list).
  *
  * Returns one if this block should be evicted, zero otherwise. Will always
  * return zero when no hole has been found.

drivers/gpu/drm/drm_vma_manager.c +2 -2

index b966cea..3837481 100644
@@ -241,8 +241,8 @@ int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
              goto out_unlock;
      }
 
-     ret = drm_mm_insert_node_generic(&mgr->vm_addr_space_mm,
-                                      &node->vm_node, pages, 0, 0);
+     ret = drm_mm_insert_node(&mgr->vm_addr_space_mm, &node->vm_node,
+                              pages, 0, DRM_MM_SEARCH_DEFAULT);
      if (ret)
              goto out_unlock;
 

drivers/gpu/drm/i915/i915_gem.c +2 -1

index 8673a00..f3065a0 100644
@@ -3092,7 +3092,8 @@ search_free:
      ret = drm_mm_insert_node_in_range_generic(&dev_priv->mm.gtt_space,
                                                &obj->gtt_space,
                                                size, alignment,
-                                               obj->cache_level, 0, gtt_max);
+                                               obj->cache_level, 0, gtt_max,
+                                               DRM_MM_SEARCH_DEFAULT);
      if (ret) {
              ret = i915_gem_evict_something(dev, size, alignment,
                                             obj->cache_level,

drivers/gpu/drm/i915/i915_gem_stolen.c +8 -4

index 5521833..e355170 100644
@@ -115,10 +115,12 @@ static int i915_setup_compression(struct drm_device *dev, int size)
 
      /* Try to over-allocate to reduce reallocations and fragmentation */
      compressed_fb = drm_mm_search_free(&dev_priv->mm.stolen,
-                                        size <<= 1, 4096, 0);
+                                        size <<= 1, 4096,
+                                        DRM_MM_SEARCH_DEFAULT);
      if (!compressed_fb)
              compressed_fb = drm_mm_search_free(&dev_priv->mm.stolen,
-                                                size >>= 1, 4096, 0);
+                                                size >>= 1, 4096,
+                                                DRM_MM_SEARCH_DEFAULT);
      if (compressed_fb)
              compressed_fb = drm_mm_get_block(compressed_fb, size, 4096);
      if (!compressed_fb)

@@ -130,7 +132,8 @@ static int i915_setup_compression(struct drm_device *dev, int size)
              I915_WRITE(DPFC_CB_BASE, compressed_fb->start);
      } else {
              compressed_llb = drm_mm_search_free(&dev_priv->mm.stolen,
-                                                 4096, 4096, 0);
+                                                 4096, 4096,
+                                                 DRM_MM_SEARCH_DEFAULT);
              if (compressed_llb)
                      compressed_llb = drm_mm_get_block(compressed_llb,
                                                        4096, 4096);

@@ -328,7 +331,8 @@ i915_gem_object_create_stolen(struct drm_device *dev, u32 size)
      if (size == 0)
              return NULL;
 
-     stolen = drm_mm_search_free(&dev_priv->mm.stolen, size, 4096, 0);
+     stolen = drm_mm_search_free(&dev_priv->mm.stolen, size, 4096,
+                                 DRM_MM_SEARCH_DEFAULT);
      if (stolen)
              stolen = drm_mm_get_block(stolen, size, 4096);
      if (stolen == NULL)

drivers/gpu/drm/sis/sis_mm.c +4 -2

index 9a43d98..23a2349 100644
@@ -109,7 +109,8 @@ static int sis_drm_alloc(struct drm_device *dev, struct drm_file *file,
      if (pool == AGP_TYPE) {
              retval = drm_mm_insert_node(&dev_priv->agp_mm,
                                          &item->mm_node,
-                                         mem->size, 0);
+                                         mem->size, 0,
+                                         DRM_MM_SEARCH_DEFAULT);
              offset = item->mm_node.start;
      } else {
 #if defined(CONFIG_FB_SIS) || defined(CONFIG_FB_SIS_MODULE)

@@ -121,7 +122,8 @@ static int sis_drm_alloc(struct drm_device *dev, struct drm_file *file,
 #else
              retval = drm_mm_insert_node(&dev_priv->vram_mm,
                                          &item->mm_node,
-                                         mem->size, 0);
+                                         mem->size, 0,
+                                         DRM_MM_SEARCH_DEFAULT);
              offset = item->mm_node.start;
 #endif
      }

drivers/gpu/drm/ttm/ttm_bo_manager.c +2 -1

index e4367f9..e4be29e 100644
@@ -69,7 +69,8 @@ static int ttm_bo_man_get_node(struct ttm_mem_type_manager *man,
              spin_lock(&rman->lock);
              node = drm_mm_search_free_in_range(mm,
                                      mem->num_pages, mem->page_alignment,
-                                     placement->fpfn, lpfn, 1);
+                                     placement->fpfn, lpfn,
+                                     DRM_MM_SEARCH_BEST);
              if (unlikely(node == NULL)) {
                      spin_unlock(&rman->lock);
                      return 0;

drivers/gpu/drm/via/via_mm.c +2 -2

index 0ab93ff..7e3ad87 100644
@@ -140,11 +140,11 @@ int via_mem_alloc(struct drm_device *dev, void *data,
      if (mem->type == VIA_MEM_AGP)
              retval = drm_mm_insert_node(&dev_priv->agp_mm,
                                          &item->mm_node,
-                                         tmpSize, 0);
+                                         tmpSize, 0, DRM_MM_SEARCH_DEFAULT);
      else
              retval = drm_mm_insert_node(&dev_priv->vram_mm,
                                          &item->mm_node,
-                                         tmpSize, 0);
+                                         tmpSize, 0, DRM_MM_SEARCH_DEFAULT);
      if (retval)
              goto fail_alloc;
 

include/drm/drm_mm.h +36 -19

index 98cb50e..439d1a1 100644
@@ -44,6 +44,11 @@
 #include <linux/seq_file.h>
 #endif
 
+enum drm_mm_search_flags {
+     DRM_MM_SEARCH_DEFAULT =         0,
+     DRM_MM_SEARCH_BEST =            1 << 0,
+};
+
 struct drm_mm_node {
      struct list_head node_list;
      struct list_head hole_stack;

@@ -189,28 +194,41 @@ static inline struct drm_mm_node *drm_mm_get_block_atomic_range(
                                              start, end, 1);
 }
 
-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);
+                                   unsigned long color,
+                                   enum drm_mm_search_flags flags);
+static inline int drm_mm_insert_node(struct drm_mm *mm,
+                                  struct drm_mm_node *node,
+                                  unsigned long size,
+                                  unsigned alignment,
+                                  enum drm_mm_search_flags flags)
+{
+     return drm_mm_insert_node_generic(mm, node, size, alignment, 0, flags);
+}
+
 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 end,
+                                    enum drm_mm_search_flags flags);
+static inline 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,
+                                           enum drm_mm_search_flags flags)
+{
+     return drm_mm_insert_node_in_range_generic(mm, node, size, alignment,
+                                                0, start, end, flags);
+}
+
 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);

@@ -218,7 +236,7 @@ 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);
+                                                   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,

@@ -226,13 +244,13 @@ extern struct drm_mm_node *drm_mm_search_free_in_range_generic(
                                              unsigned long color,
                                              unsigned long start,
                                              unsigned long end,
-                                             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,
-                                                  bool best_match)
+                                                  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,

@@ -240,10 +258,10 @@ 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);
 }
 
 extern void drm_mm_init(struct drm_mm *mm,
--

Quoted 2 lines

1.8.3.4

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Quoted 16 lines · Daniel Vetter

Quoted 14 lines · David Herrmann

Add a "best_match" flag similar to the drm_mm_search_*() helpers so we can convert TTM to use them in follow up patches. We can also inline the non-generic helpers and move them into the header to allow compile-time optimizations.

To make calls to drm_mm_{search,insert}_node() more readable, this converts the boolean argument to a flagset. There are pending patches that add additional flags for top-down allocators and more.

v2:
- use flag parameter instead of boolean "best_match"
- convert *_search_free() helpers to also use flags argument

Signed-off-by David Herrmann <[email protected]>

Quoted 2 lines

I'm not sure whether the use of an enum here is proper C style (usually bitflag arguments tend to be unsigned ints), but that's a bikeshed. So

I saw it used in libxkbcommon and it's actually pretty nice because gcc supports some type-safety for it. It checks whether the passed flags are really from the enum (even if OR/AND-combined). And as it is no external API, we have no problems with the "enum resizing" once you add more flags.

Instead of calling drm_mm_pre_get() in a row, we now preallocate the node and then use the atomic insertion functions. This has the exact same semantics and there is no reason to use the racy pre-allocations.

Note that ttm_bo_man_get_node() does not run in atomic context. Nouveau already uses GFP_KERNEL alloc in nouveau/nouveau_ttm.c in nouveau_gart_manager_new(). So we can do the same in ttm_bo_man_get_node().

Signed-off-by David Herrmann <[email protected]>

---
 drivers/gpu/drm/ttm/ttm_bo_manager.c | 40 +++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 21 deletions(-)

drivers/gpu/drm/ttm/ttm_bo_manager.c +19 -21

index e4367f9..cbd2ec7 100644
@@ -61,28 +61,24 @@ static int ttm_bo_man_get_node(struct ttm_mem_type_manager *man,
 	lpfn = placement->lpfn;
 	if (!lpfn)
 		lpfn = man->size;
-	do {
-		ret = drm_mm_pre_get(mm);
-		if (unlikely(ret))
-			return ret;
 
-		spin_lock(&rman->lock);
-		node = drm_mm_search_free_in_range(mm,
-					mem->num_pages, mem->page_alignment,
-					placement->fpfn, lpfn, 1);
-		if (unlikely(node == NULL)) {
-			spin_unlock(&rman->lock);
-			return 0;
-		}
-		node = drm_mm_get_block_atomic_range(node, mem->num_pages,
-						     mem->page_alignment,
-						     placement->fpfn,
-						     lpfn);
-		spin_unlock(&rman->lock);
-	} while (node == NULL);
+	node = kzalloc(sizeof(*node), GFP_KERNEL);
+	if (!node)
+		return -ENOMEM;
+
+	spin_lock(&rman->lock);
+	ret = drm_mm_insert_node_in_range(mm, node, mem->num_pages,
+					  mem->page_alignment,
+					  placement->fpfn, lpfn, true);
+	spin_unlock(&rman->lock);
+
+	if (unlikely(ret)) {
+		kfree(node);
+	} else {
+		mem->mm_node = node;
+		mem->start = node->start;
+	}
 
-	mem->mm_node = node;
-	mem->start = node->start;
 	return 0;
 }
 

@@ -93,8 +89,10 @@ static void ttm_bo_man_put_node(struct ttm_mem_type_manager *man,
 
 	if (mem->mm_node) {
 		spin_lock(&rman->lock);
-		drm_mm_put_block(mem->mm_node);
+		drm_mm_remove_node(mem->mm_node);
 		spin_unlock(&rman->lock);
+
+		kfree(mem->mm_node);
 		mem->mm_node = NULL;
 	}
 }

Signature · 1 line

1.8.3.3

i915 is the last user of the weird search+get_block drm_mm API. Convert it to an explicit kmalloc()+insert_node(). This drops the last user of the node-cache in drm_mm. We can remove it now in a follow-up patch.

Cc Daniel Vetter <[email protected]>

Signed-off-by David Herrmann <[email protected]>

---
 drivers/gpu/drm/i915/i915_gem_stolen.c | 72 ++++++++++++++++++++++------------
 1 file changed, 47 insertions(+), 25 deletions(-)

drivers/gpu/drm/i915/i915_gem_stolen.c +2 -8

index 5521833..7a2f040 100644
@@ -112,31 +112,39 @@ static int i915_setup_compression(struct drm_device *dev, int size)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_mm_node *compressed_fb, *uninitialized_var(compressed_llb);
+	int ret;
 
-	/* Try to over-allocate to reduce reallocations and fragmentation */
-	compressed_fb = drm_mm_search_free(&dev_priv->mm.stolen,
-					   size <<= 1, 4096, 0);
-	if (!compressed_fb)
-		compressed_fb = drm_mm_search_free(&dev_priv->mm.stolen,
-						   size >>= 1, 4096, 0);
-	if (compressed_fb)
-		compressed_fb = drm_mm_get_block(compressed_fb, size, 4096);
+	compressed_fb = kzalloc(sizeof(*compressed_fb), GFP_KERNEL);
 	if (!compressed_fb)
 		goto err;

i915 is the last user of the weird search+get_block drm_mm API. Convert it to an explicit kmalloc()+insert_node(). This drops the last user of the node-cache in drm_mm. We can remove it now in a follow-up patch.

v2:
- simplify error path in i915_setup_compression()

Quoted 6 lines · David Herrmann

i915 is the last user of the weird search+get_block drm_mm API. Convert it to an explicit kmalloc()+insert_node(). This drops the last user of the node-cache in drm_mm. We can remove it now in a follow-up patch.

v2:
- simplify error path in i915_setup_compression()

You only applied it to the err path, I was expecting err_fb to do the kfree(compressed_llb) as well. Feel free to rename those to err_fb and err_llb respectively if you think that helps.

Hi

Quoted 8 lines · Chris Wilson

Quoted 6 lines · David Herrmann

i915 is the last user of the weird search+get_block drm_mm API. Convert it to an explicit kmalloc()+insert_node(). This drops the last user of the node-cache in drm_mm. We can remove it now in a follow-up patch.

v2:
- simplify error path in i915_setup_compression()

Quoted 3 lines

You only applied it to the err path, I was expecting err_fb to do the kfree(compressed_llb) as well. Feel free to rename those to err_fb and err_llb respectively if you think that helps.

I wasn't sure about that second error-path. Some kernel subsystems tend to inline error-paths that are only taken once (which is the case for compressed_llb). But if that's not the case for DRM, I will surely fix that, too.

i915 is the last user of the weird search+get_block drm_mm API. Convert it to an explicit kmalloc()+insert_node(). This drops the last user of the node-cache in drm_mm. We can remove it now in a follow-up patch.

v2:
- simplify error path in i915_setup_compression()
v3:
- simplify error path even more

Cc Daniel Vetter <[email protected]>

Cc Chris Wilson <[email protected]>

Signed-off-by David Herrmann <[email protected]>

---
 drivers/gpu/drm/i915/i915_gem_stolen.c | 78 ++++++++++++++++++++--------------
 1 file changed, 47 insertions(+), 31 deletions(-)

drivers/gpu/drm/i915/i915_gem_stolen.c +47 -31

index e355170..a3d1a12 100644
@@ -112,34 +112,36 @@ static int i915_setup_compression(struct drm_device *dev, int size)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_mm_node *compressed_fb, *uninitialized_var(compressed_llb);
+	int ret;
 
-	/* Try to over-allocate to reduce reallocations and fragmentation */
-	compressed_fb = drm_mm_search_free(&dev_priv->mm.stolen,
-					   size <<= 1, 4096,
-					   DRM_MM_SEARCH_DEFAULT);
-	if (!compressed_fb)
-		compressed_fb = drm_mm_search_free(&dev_priv->mm.stolen,
-						   size >>= 1, 4096,
-						   DRM_MM_SEARCH_DEFAULT);
-	if (compressed_fb)
-		compressed_fb = drm_mm_get_block(compressed_fb, size, 4096);
+	compressed_fb = kzalloc(sizeof(*compressed_fb), GFP_KERNEL);
 	if (!compressed_fb)
-		goto err;
+		goto err_llb;
+
+	/* Try to over-allocate to reduce reallocations and fragmentation */
+	ret = drm_mm_insert_node(&dev_priv->mm.stolen, compressed_fb,
+				 size <<= 1, 4096, DRM_MM_SEARCH_DEFAULT);
+	if (ret)
+		ret = drm_mm_insert_node(&dev_priv->mm.stolen, compressed_fb,
+					 size >>= 1, 4096,
+					 DRM_MM_SEARCH_DEFAULT);
+	if (ret)
+		goto err_llb;
 
 	if (HAS_PCH_SPLIT(dev))
 		I915_WRITE(ILK_DPFC_CB_BASE, compressed_fb->start);
 	else if (IS_GM45(dev)) {
 		I915_WRITE(DPFC_CB_BASE, compressed_fb->start);
 	} else {
-		compressed_llb = drm_mm_search_free(&dev_priv->mm.stolen,
-						    4096, 4096,
-						    DRM_MM_SEARCH_DEFAULT);
-		if (compressed_llb)
-			compressed_llb = drm_mm_get_block(compressed_llb,
-							  4096, 4096);
+		compressed_llb = kzalloc(sizeof(*compressed_llb), GFP_KERNEL);
 		if (!compressed_llb)
 			goto err_fb;
 
+		ret = drm_mm_insert_node(&dev_priv->mm.stolen, compressed_llb,
+					 4096, 4096, DRM_MM_SEARCH_DEFAULT);
+		if (ret)
+			goto err_fb;
+
 		dev_priv->fbc.compressed_llb = compressed_llb;
 
 		I915_WRITE(FBC_CFB_BASE,

@@ -157,8 +159,10 @@ static int i915_setup_compression(struct drm_device *dev, int size)
 	return 0;
 
 err_fb:
-	drm_mm_put_block(compressed_fb);
-err:
+	kfree(compressed_llb);
+	drm_mm_remove_node(compressed_fb);
+err_llb:
+	kfree(compressed_fb);
 	pr_info_once("drm: not enough stolen space for compressed buffer (need %d more bytes), disabling. Hint: you may be able to increase stolen memory size in the BIOS to avoid this.\n", size);
 	return -ENOSPC;
 }

@@ -186,11 +190,15 @@ void i915_gem_stolen_cleanup_compression(struct drm_device *dev)
 	if (dev_priv->fbc.size == 0)
 		return;
 
-	if (dev_priv->fbc.compressed_fb)
-		drm_mm_put_block(dev_priv->fbc.compressed_fb);
+	if (dev_priv->fbc.compressed_fb) {
+		drm_mm_remove_node(dev_priv->fbc.compressed_fb);
+		kfree(dev_priv->fbc.compressed_fb);
+	}
 
-	if (dev_priv->fbc.compressed_llb)
-		drm_mm_put_block(dev_priv->fbc.compressed_llb);
+	if (dev_priv->fbc.compressed_llb) {
+		drm_mm_remove_node(dev_priv->fbc.compressed_llb);
+		kfree(dev_priv->fbc.compressed_llb);
+	}
 
 	dev_priv->fbc.size = 0;
 }

@@ -323,6 +331,7 @@ i915_gem_object_create_stolen(struct drm_device *dev, u32 size)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_gem_object *obj;
 	struct drm_mm_node *stolen;
+	int ret;
 
 	if (!drm_mm_initialized(&dev_priv->mm.stolen))
 		return NULL;

@@ -331,18 +340,23 @@ i915_gem_object_create_stolen(struct drm_device *dev, u32 size)
 	if (size == 0)
 		return NULL;
 
-	stolen = drm_mm_search_free(&dev_priv->mm.stolen, size, 4096,
-				    DRM_MM_SEARCH_DEFAULT);
-	if (stolen)
-		stolen = drm_mm_get_block(stolen, size, 4096);
-	if (stolen == NULL)
+	stolen = kzalloc(sizeof(*stolen), GFP_KERNEL);
+	if (!stolen)
 		return NULL;
 
+	ret = drm_mm_insert_node(&dev_priv->mm.stolen, stolen, size,
+				 4096, DRM_MM_SEARCH_DEFAULT);
+	if (ret) {
+		kfree(stolen);
+		return NULL;
+	}
+
 	obj = _i915_gem_object_create_stolen(dev, stolen);
 	if (obj)
 		return obj;
 
-	drm_mm_put_block(stolen);
+	drm_mm_remove_node(stolen);
+	kfree(stolen);
 	return NULL;
 }
 

@@ -386,7 +400,8 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
 	obj = _i915_gem_object_create_stolen(dev, stolen);
 	if (obj == NULL) {
 		DRM_DEBUG_KMS("failed to allocate stolen object\n");
-		drm_mm_put_block(stolen);
+		drm_mm_remove_node(stolen);
+		kfree(stolen);
 		return NULL;
 	}
 

@@ -426,7 +441,8 @@ void
 i915_gem_object_release_stolen(struct drm_i915_gem_object *obj)
 {
 	if (obj->stolen) {
-		drm_mm_put_block(obj->stolen);
+		drm_mm_remove_node(obj->stolen);
+		kfree(obj->stolen);
 		obj->stolen = NULL;
 	}
 }

Signature · 1 line

1.8.3.4

We used to pre-allocate drm_mm nodes and save them in a linked list for later usage so we always have spare ones in atomic contexts. However, this is really racy if multiple threads are in an atomic context at the same time and we don't have enough spare nodes. Moreover, all remaining users run in user-context and just lock drm_mm with a spinlock. So we can easily preallocate the node, take the spinlock and insert the node.

This may have worked well with BKL in place, however, with today's infrastructure it really doesn't make any sense. Besides, most users can easily embed drm_mm_node into their objects so no allocation is needed at all.

Thus, remove the old pre-alloc API and all the helpers that it provides. Drivers have already been converted and we should not use the old API for new code, anymore.

Signed-off-by David Herrmann <[email protected]>

---
 drivers/gpu/drm/drm_mm.c | 160 ++++++-----------------------------------------
 include/drm/drm_mm.h     |  95 ----------------------------
 2 files changed, 20 insertions(+), 235 deletions(-)

drivers/gpu/drm/drm_mm.c +20 -140

index 5aad736..0946251 100644
@@ -49,58 +49,18 @@
 
 #define MM_UNUSED_TARGET 4
 
-static struct drm_mm_node *drm_mm_kmalloc(struct drm_mm *mm, int atomic)
-{
-	struct drm_mm_node *child;
-
-	if (atomic)
-		child = kzalloc(sizeof(*child), GFP_ATOMIC);
-	else
-		child = kzalloc(sizeof(*child), GFP_KERNEL);
-
-	if (unlikely(child == NULL)) {
-		spin_lock(&mm->unused_lock);
-		if (list_empty(&mm->unused_nodes))
-			child = NULL;
-		else {
-			child =
-			    list_entry(mm->unused_nodes.next,
-				       struct drm_mm_node, node_list);
-			list_del(&child->node_list);
-			--mm->num_unused;
-		}
-		spin_unlock(&mm->unused_lock);
-	}
-	return child;
-}
-
-/* drm_mm_pre_get() - pre allocate drm_mm_node structure
- * drm_mm:	memory manager struct we are pre-allocating for
- *
- * Returns 0 on success or -ENOMEM if allocation fails.
- */
-int drm_mm_pre_get(struct drm_mm *mm)
-{
-	struct drm_mm_node *node;
-
-	spin_lock(&mm->unused_lock);
-	while (mm->num_unused < MM_UNUSED_TARGET) {
-		spin_unlock(&mm->unused_lock);
-		node = kzalloc(sizeof(*node), GFP_KERNEL);
-		spin_lock(&mm->unused_lock);
-
-		if (unlikely(node == NULL)) {
-			int ret = (mm->num_unused < 2) ? -ENOMEM : 0;
-			spin_unlock(&mm->unused_lock);
-			return ret;
-		}
-		++mm->num_unused;
-		list_add_tail(&node->node_list, &mm->unused_nodes);
-	}
-	spin_unlock(&mm->unused_lock);
-	return 0;
-}
-EXPORT_SYMBOL(drm_mm_pre_get);
+static 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);
+static 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);
 
 static void drm_mm_insert_helper(struct drm_mm_node *hole_node,
 				 struct drm_mm_node *node,

@@ -187,24 +147,6 @@ int drm_mm_reserve_node(struct drm_mm *mm, struct drm_mm_node *node)
 }
 EXPORT_SYMBOL(drm_mm_reserve_node);
 
-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)
-{
-	struct drm_mm_node *node;
-
-	node = drm_mm_kmalloc(hole_node->mm, atomic);
-	if (unlikely(node == NULL))
-		return NULL;
-
-	drm_mm_insert_helper(hole_node, node, size, alignment, color);
-
-	return node;
-}
-EXPORT_SYMBOL(drm_mm_get_block_generic);
-
 /**
  * Search for free space and insert a preallocated memory node. Returns
  * -ENOSPC if no suitable free area is available. The preallocated memory node

@@ -278,27 +220,6 @@ 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 *node;
-
-	node = drm_mm_kmalloc(hole_node->mm, atomic);
-	if (unlikely(node == NULL))
-		return NULL;
-
-	drm_mm_insert_helper_range(hole_node, node, size, alignment, color,
-				   start, end);
-
-	return node;
-}
-EXPORT_SYMBOL(drm_mm_get_block_range_generic);
-
 /**
  * Search for free space and insert a preallocated memory node. Returns
  * -ENOSPC if no suitable free area is available. This is for range

@@ -357,28 +278,6 @@ void drm_mm_remove_node(struct drm_mm_node *node)
 }
 EXPORT_SYMBOL(drm_mm_remove_node);
 
-/*
- * Remove a memory node from the allocator and free the allocated struct
- * drm_mm_node. Only to be used on a struct drm_mm_node obtained by one of the
- * drm_mm_get_block functions.
- */
-void drm_mm_put_block(struct drm_mm_node *node)
-{
-
-	struct drm_mm *mm = node->mm;
-
-	drm_mm_remove_node(node);
-
-	spin_lock(&mm->unused_lock);
-	if (mm->num_unused < MM_UNUSED_TARGET) {
-		list_add(&node->node_list, &mm->unused_nodes);
-		++mm->num_unused;
-	} else
-		kfree(node);
-	spin_unlock(&mm->unused_lock);
-}
-EXPORT_SYMBOL(drm_mm_put_block);
-
 static int check_free_hole(unsigned long start, unsigned long end,
 			   unsigned long size, unsigned alignment)
 {

@@ -394,11 +293,11 @@ static int check_free_hole(unsigned long start, unsigned long end,
 	return end >= start + size;
 }
 
-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)
+static 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)
 {
 	struct drm_mm_node *entry;
 	struct drm_mm_node *best;

@@ -432,9 +331,8 @@ struct drm_mm_node *drm_mm_search_free_generic(const struct drm_mm *mm,
 
 	return best;
 }
-EXPORT_SYMBOL(drm_mm_search_free_generic);
 
-struct drm_mm_node *drm_mm_search_free_in_range_generic(const struct drm_mm *mm,
+static struct drm_mm_node *drm_mm_search_free_in_range_generic(const struct drm_mm *mm,
 							unsigned long size,
 							unsigned alignment,
 							unsigned long color,

@@ -479,7 +377,6 @@ struct drm_mm_node *drm_mm_search_free_in_range_generic(const struct drm_mm *mm,
 
 	return best;
 }
-EXPORT_SYMBOL(drm_mm_search_free_in_range_generic);
 
 /**
  * Moves an allocation. To be used with embedded struct drm_mm_node.

@@ -652,10 +549,7 @@ EXPORT_SYMBOL(drm_mm_clean);
 void drm_mm_init(struct drm_mm * mm, unsigned long start, unsigned long size)
 {
 	INIT_LIST_HEAD(&mm->hole_stack);
-	INIT_LIST_HEAD(&mm->unused_nodes);
-	mm->num_unused = 0;
 	mm->scanned_blocks = 0;
-	spin_lock_init(&mm->unused_lock);
 
 	/* Clever trick to avoid a special case in the free hole tracking. */
 	INIT_LIST_HEAD(&mm->head_node.node_list);

@@ -675,22 +569,8 @@ EXPORT_SYMBOL(drm_mm_init);
 
 void drm_mm_takedown(struct drm_mm * mm)
 {
-	struct drm_mm_node *entry, *next;
-
-	if (WARN(!list_empty(&mm->head_node.node_list),
-		 "Memory manager not clean. Delaying takedown\n")) {
-		return;
-	}
-
-	spin_lock(&mm->unused_lock);
-	list_for_each_entry_safe(entry, next, &mm->unused_nodes, node_list) {
-		list_del(&entry->node_list);
-		kfree(entry);
-		--mm->num_unused;
-	}
-	spin_unlock(&mm->unused_lock);
-
-	BUG_ON(mm->num_unused != 0);
+	WARN(!list_empty(&mm->head_node.node_list),
+	     "Memory manager not clean during takedown.\n");
 }
 EXPORT_SYMBOL(drm_mm_takedown);
 

include/drm/drm_mm.h +0 -95

index a30c9aa..4890c51 100644
@@ -62,9 +62,6 @@ struct drm_mm {
 	/* head_node.node_list is the list of all memory nodes, ordered
 	 * according to the (increasing) start address of the memory node. */
 	struct drm_mm_node head_node;
-	struct list_head unused_nodes;
-	int num_unused;
-	spinlock_t unused_lock;
 	unsigned int scan_check_range : 1;
 	unsigned scan_alignment;
 	unsigned long scan_color;

@@ -115,13 +112,6 @@ static inline unsigned long drm_mm_hole_node_end(struct drm_mm_node *hole_node)
 #define drm_mm_for_each_node(entry, mm) list_for_each_entry(entry, \
 						&(mm)->head_node.node_list, \
 						node_list)
-#define drm_mm_for_each_scanned_node_reverse(entry, n, mm) \
-	for (entry = (mm)->prev_scanned_node, \
-		next = entry ? list_entry(entry->node_list.next, \
-			struct drm_mm_node, node_list) : NULL; \
-	     entry != NULL; entry = next, \
-		next = entry ? list_entry(entry->node_list.next, \
-			struct drm_mm_node, node_list) : NULL) \
 
 /* Note that we need to unroll list_for_each_entry in order to inline
  * setting hole_start and hole_end on each iteration and keep the

@@ -139,52 +129,6 @@ static inline unsigned long drm_mm_hole_node_end(struct drm_mm_node *hole_node)
  * Basic range manager support (drm_mm.c)
  */
 extern int drm_mm_reserve_node(struct drm_mm *mm, struct drm_mm_node *node);
-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);
-
-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);
-}
-static inline struct drm_mm_node *drm_mm_get_block_atomic(struct drm_mm_node *parent,
-							  unsigned long size,
-							  unsigned alignment)
-{
-	return drm_mm_get_block_generic(parent, size, alignment, 0, 1);
-}
-static inline struct drm_mm_node *drm_mm_get_block_range(
-						struct drm_mm_node *parent,
-						unsigned long size,
-						unsigned alignment,
-						unsigned long start,
-						unsigned long end)
-{
-	return drm_mm_get_block_range_generic(parent, size, alignment, 0,
-					      start, end, 0);
-}
-static inline struct drm_mm_node *drm_mm_get_block_atomic_range(
-						struct drm_mm_node *parent,
-						unsigned long size,
-						unsigned alignment,
-						unsigned long start,
-						unsigned long end)
-{
-	return drm_mm_get_block_range_generic(parent, size, alignment, 0,
-						start, end, 1);
-}
 
 extern int drm_mm_insert_node_generic(struct drm_mm *mm,
 				      struct drm_mm_node *node,

@@ -222,52 +166,13 @@ static inline int drm_mm_insert_node_in_range(struct drm_mm *mm,
 						   0, start, end, best_match);
 }
 
-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_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);
-static inline struct drm_mm_node *drm_mm_search_free(const struct drm_mm *mm,
-						     unsigned long size,
-						     unsigned alignment,
-						     bool best_match)
-{
-	return drm_mm_search_free_generic(mm,size, alignment, 0, best_match);
-}
-static inline  struct drm_mm_node *drm_mm_search_free_in_range(
-						const struct drm_mm *mm,
-						unsigned long size,
-						unsigned alignment,
-						unsigned long start,
-						unsigned long end,
-						bool best_match)
-{
-	return drm_mm_search_free_in_range_generic(mm, size, alignment, 0,
-						   start, end, best_match);
-}
-
 extern void drm_mm_init(struct drm_mm *mm,
 			unsigned long start,
 			unsigned long size);
 extern void drm_mm_takedown(struct drm_mm *mm);
 extern int drm_mm_clean(struct drm_mm *mm);
-extern int drm_mm_pre_get(struct drm_mm *mm);
-
-static inline struct drm_mm *drm_get_mm(struct drm_mm_node *block)
-{
-	return block->mm;
-}
 
 void drm_mm_init_scan(struct drm_mm *mm,
 		      unsigned long size,

Signature · 1 line

1.8.3.3