* [PATCH 0/4] DRM: Remove MM "Pre-Alloc"
@ 2013-07-25 13:55 David Herrmann
2013-07-25 13:55 ` [PATCH 1/4] drm/mm: add "best_match" to drm_mm_insert_node() David Herrmann
` (3 more replies)
0 siblings, 4 replies; 21+ messages in thread
From: David Herrmann @ 2013-07-25 13:55 UTC (permalink / raw)
To: dri-devel; +Cc: Dave Airlie, Daniel Vetter
Hi
See patch 4/4 for a more detailed explanation of this series. I basically kill
off the whole drm_mm pre-alloc code as it really doesn't make any sense with
todays infrastructure. No drm_mm user runs in atomic context. We use pre-alloc
only to allow allocation while holding a spin-lock. But we can easily
kzalloc() the node before taking the spinlock and use the drm_mm_insert_*()
helpers directly.
This series converts the last pre-alloc users (ttm and i915-gem-stolen) to use
the already established kzalloc()+drm_mm_insert_*() helpers.
The last patch removes a bunch of old drm_mm code, so Daniel can tackle his
"drm_mm documentation" TODO list.
Cheers
David
David Herrmann (4):
drm/mm: add "best_match" to drm_mm_insert_node()
drm/ttm: replace drm_mm_pre_get() by direct alloc
drm/i915: pre-alloc instead of drm_mm search/get_block
drm/mm: remove unused API
drivers/gpu/drm/drm_mm.c | 183 +++++----------------------------
drivers/gpu/drm/drm_vma_manager.c | 4 +-
drivers/gpu/drm/i915/i915_gem.c | 3 +-
drivers/gpu/drm/i915/i915_gem_stolen.c | 72 ++++++++-----
drivers/gpu/drm/sis/sis_mm.c | 4 +-
drivers/gpu/drm/ttm/ttm_bo_manager.c | 40 ++++---
drivers/gpu/drm/via/via_mm.c | 4 +-
include/drm/drm_mm.h | 131 +++++------------------
8 files changed, 123 insertions(+), 318 deletions(-)
--
1.8.3.3
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH 1/4] drm/mm: add "best_match" to drm_mm_insert_node() 2013-07-25 13:55 [PATCH 0/4] DRM: Remove MM "Pre-Alloc" David Herrmann @ 2013-07-25 13:55 ` David Herrmann 2013-07-25 14:15 ` Daniel Vetter 2013-07-27 11:36 ` [PATCH 1/4] drm/mm: add "best_match" flag " David Herrmann 2013-07-25 13:56 ` [PATCH 2/4] drm/ttm: replace drm_mm_pre_get() by direct alloc David Herrmann ` (2 subsequent siblings) 3 siblings, 2 replies; 21+ messages in thread From: David Herrmann @ 2013-07-25 13:55 UTC (permalink / raw) To: dri-devel; +Cc: Dave Airlie, Daniel Vetter 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 <dh.herrmann@gmail.com> --- 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(-) diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c index fe304f9..5aad736 100644 --- a/drivers/gpu/drm/drm_mm.c +++ b/drivers/gpu/drm/drm_mm.c @@ -212,12 +212,12 @@ 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, bool best_match) { 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. */ diff --git a/drivers/gpu/drm/drm_vma_manager.c b/drivers/gpu/drm/drm_vma_manager.c index b966cea..4285b38 100644 --- a/drivers/gpu/drm/drm_vma_manager.c +++ b/drivers/gpu/drm/drm_vma_manager.c @@ -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; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 8673a00..6667322 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -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, diff --git a/drivers/gpu/drm/sis/sis_mm.c b/drivers/gpu/drm/sis/sis_mm.c index 9a43d98..85da1a4 100644 --- a/drivers/gpu/drm/sis/sis_mm.c +++ b/drivers/gpu/drm/sis/sis_mm.c @@ -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 } diff --git a/drivers/gpu/drm/via/via_mm.c b/drivers/gpu/drm/via/via_mm.c index 0ab93ff..10b962e 100644 --- a/drivers/gpu/drm/via/via_mm.c +++ b/drivers/gpu/drm/via/via_mm.c @@ -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; diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h index b87d05e..a30c9aa 100644 --- a/include/drm/drm_mm.h +++ b/include/drm/drm_mm.h @@ -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); -- 1.8.3.3 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] drm/mm: add "best_match" to drm_mm_insert_node() 2013-07-25 13:55 ` [PATCH 1/4] drm/mm: add "best_match" to drm_mm_insert_node() David Herrmann @ 2013-07-25 14:15 ` Daniel Vetter 2013-07-27 11:36 ` [PATCH 1/4] drm/mm: add "best_match" flag " David Herrmann 1 sibling, 0 replies; 21+ messages in thread From: Daniel Vetter @ 2013-07-25 14:15 UTC (permalink / raw) To: David Herrmann; +Cc: Dave Airlie, dri-devel, Daniel Vetter On Thu, Jul 25, 2013 at 03:55:59PM +0200, David Herrmann wrote: > 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 <dh.herrmann@gmail.com> > --- > 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(-) > > diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c > index fe304f9..5aad736 100644 > --- a/drivers/gpu/drm/drm_mm.c > +++ b/drivers/gpu/drm/drm_mm.c > @@ -212,12 +212,12 @@ 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, bool best_match) Hm, we have a patch in the queue to add a top-down allocator mode, see: http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg40556.html So if we go to the trouble of changing all callers I'd vote for a unsinged int flags parameter and #define DRM_MM_BEST_MATCH (1 << 0) Then we could add the top-down flag without another tree-wide change. Note that the above patch needs to be rebased onto your series here anyway since it still tries to cope with GFP_ATOMIC allocations and the preallocation dance. So imo it's best if your series here goes in first. Otherwise this looks good. -Daniel > { > 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. > */ > diff --git a/drivers/gpu/drm/drm_vma_manager.c b/drivers/gpu/drm/drm_vma_manager.c > index b966cea..4285b38 100644 > --- a/drivers/gpu/drm/drm_vma_manager.c > +++ b/drivers/gpu/drm/drm_vma_manager.c > @@ -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; > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 8673a00..6667322 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -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, > diff --git a/drivers/gpu/drm/sis/sis_mm.c b/drivers/gpu/drm/sis/sis_mm.c > index 9a43d98..85da1a4 100644 > --- a/drivers/gpu/drm/sis/sis_mm.c > +++ b/drivers/gpu/drm/sis/sis_mm.c > @@ -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 > } > diff --git a/drivers/gpu/drm/via/via_mm.c b/drivers/gpu/drm/via/via_mm.c > index 0ab93ff..10b962e 100644 > --- a/drivers/gpu/drm/via/via_mm.c > +++ b/drivers/gpu/drm/via/via_mm.c > @@ -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; > > diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h > index b87d05e..a30c9aa 100644 > --- a/include/drm/drm_mm.h > +++ b/include/drm/drm_mm.h > @@ -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); > -- > 1.8.3.3 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/4] drm/mm: add "best_match" flag to drm_mm_insert_node() 2013-07-25 13:55 ` [PATCH 1/4] drm/mm: add "best_match" to drm_mm_insert_node() David Herrmann 2013-07-25 14:15 ` Daniel Vetter @ 2013-07-27 11:36 ` David Herrmann 2013-08-05 7:46 ` Daniel Vetter 1 sibling, 1 reply; 21+ messages in thread From: David Herrmann @ 2013-07-27 11:36 UTC (permalink / raw) To: dri-devel; +Cc: Dave Airlie, Daniel Vetter 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 <dh.herrmann@gmail.com> --- 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(-) diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c index fe304f9..9a38327 100644 --- a/drivers/gpu/drm/drm_mm.c +++ b/drivers/gpu/drm/drm_mm.c @@ -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. diff --git a/drivers/gpu/drm/drm_vma_manager.c b/drivers/gpu/drm/drm_vma_manager.c index b966cea..3837481 100644 --- a/drivers/gpu/drm/drm_vma_manager.c +++ b/drivers/gpu/drm/drm_vma_manager.c @@ -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; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 8673a00..f3065a0 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -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, diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 5521833..e355170 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -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) diff --git a/drivers/gpu/drm/sis/sis_mm.c b/drivers/gpu/drm/sis/sis_mm.c index 9a43d98..23a2349 100644 --- a/drivers/gpu/drm/sis/sis_mm.c +++ b/drivers/gpu/drm/sis/sis_mm.c @@ -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 } diff --git a/drivers/gpu/drm/ttm/ttm_bo_manager.c b/drivers/gpu/drm/ttm/ttm_bo_manager.c index e4367f9..e4be29e 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_manager.c +++ b/drivers/gpu/drm/ttm/ttm_bo_manager.c @@ -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; diff --git a/drivers/gpu/drm/via/via_mm.c b/drivers/gpu/drm/via/via_mm.c index 0ab93ff..7e3ad87 100644 --- a/drivers/gpu/drm/via/via_mm.c +++ b/drivers/gpu/drm/via/via_mm.c @@ -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; diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h index 98cb50e..439d1a1 100644 --- a/include/drm/drm_mm.h +++ b/include/drm/drm_mm.h @@ -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, -- 1.8.3.4 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] drm/mm: add "best_match" flag to drm_mm_insert_node() 2013-07-27 11:36 ` [PATCH 1/4] drm/mm: add "best_match" flag " David Herrmann @ 2013-08-05 7:46 ` Daniel Vetter 2013-08-06 9:39 ` David Herrmann 0 siblings, 1 reply; 21+ messages in thread From: Daniel Vetter @ 2013-08-05 7:46 UTC (permalink / raw) To: David Herrmann; +Cc: Dave Airlie, dri-devel, Daniel Vetter On Sat, Jul 27, 2013 at 01:36:27PM +0200, David Herrmann wrote: > 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 <dh.herrmann@gmail.com> 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 <daniel.vetter@ffwll.ch> > --- > 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(-) > > diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c > index fe304f9..9a38327 100644 > --- a/drivers/gpu/drm/drm_mm.c > +++ b/drivers/gpu/drm/drm_mm.c > @@ -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. > diff --git a/drivers/gpu/drm/drm_vma_manager.c b/drivers/gpu/drm/drm_vma_manager.c > index b966cea..3837481 100644 > --- a/drivers/gpu/drm/drm_vma_manager.c > +++ b/drivers/gpu/drm/drm_vma_manager.c > @@ -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; > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 8673a00..f3065a0 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -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, > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c > index 5521833..e355170 100644 > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c > @@ -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) > diff --git a/drivers/gpu/drm/sis/sis_mm.c b/drivers/gpu/drm/sis/sis_mm.c > index 9a43d98..23a2349 100644 > --- a/drivers/gpu/drm/sis/sis_mm.c > +++ b/drivers/gpu/drm/sis/sis_mm.c > @@ -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 > } > diff --git a/drivers/gpu/drm/ttm/ttm_bo_manager.c b/drivers/gpu/drm/ttm/ttm_bo_manager.c > index e4367f9..e4be29e 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_manager.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_manager.c > @@ -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; > diff --git a/drivers/gpu/drm/via/via_mm.c b/drivers/gpu/drm/via/via_mm.c > index 0ab93ff..7e3ad87 100644 > --- a/drivers/gpu/drm/via/via_mm.c > +++ b/drivers/gpu/drm/via/via_mm.c > @@ -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; > > diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h > index 98cb50e..439d1a1 100644 > --- a/include/drm/drm_mm.h > +++ b/include/drm/drm_mm.h > @@ -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, > -- > 1.8.3.4 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] drm/mm: add "best_match" flag to drm_mm_insert_node() 2013-08-05 7:46 ` Daniel Vetter @ 2013-08-06 9:39 ` David Herrmann 2013-08-06 11:03 ` Daniel Vetter 0 siblings, 1 reply; 21+ messages in thread From: David Herrmann @ 2013-08-06 9:39 UTC (permalink / raw) To: Daniel Vetter; +Cc: Dave Airlie, dri-devel@lists.freedesktop.org, Daniel Vetter Hi On Mon, Aug 5, 2013 at 9:46 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Sat, Jul 27, 2013 at 01:36:27PM +0200, David Herrmann wrote: >> 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 <dh.herrmann@gmail.com> > > 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. But if anyone feels uncomfortable with it, I can change it to "unsigned int". Thanks for reviewing! David > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> --- >> 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(-) >> >> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c >> index fe304f9..9a38327 100644 >> --- a/drivers/gpu/drm/drm_mm.c >> +++ b/drivers/gpu/drm/drm_mm.c >> @@ -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. >> diff --git a/drivers/gpu/drm/drm_vma_manager.c b/drivers/gpu/drm/drm_vma_manager.c >> index b966cea..3837481 100644 >> --- a/drivers/gpu/drm/drm_vma_manager.c >> +++ b/drivers/gpu/drm/drm_vma_manager.c >> @@ -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; >> >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c >> index 8673a00..f3065a0 100644 >> --- a/drivers/gpu/drm/i915/i915_gem.c >> +++ b/drivers/gpu/drm/i915/i915_gem.c >> @@ -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, >> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c >> index 5521833..e355170 100644 >> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c >> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c >> @@ -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) >> diff --git a/drivers/gpu/drm/sis/sis_mm.c b/drivers/gpu/drm/sis/sis_mm.c >> index 9a43d98..23a2349 100644 >> --- a/drivers/gpu/drm/sis/sis_mm.c >> +++ b/drivers/gpu/drm/sis/sis_mm.c >> @@ -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 >> } >> diff --git a/drivers/gpu/drm/ttm/ttm_bo_manager.c b/drivers/gpu/drm/ttm/ttm_bo_manager.c >> index e4367f9..e4be29e 100644 >> --- a/drivers/gpu/drm/ttm/ttm_bo_manager.c >> +++ b/drivers/gpu/drm/ttm/ttm_bo_manager.c >> @@ -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; >> diff --git a/drivers/gpu/drm/via/via_mm.c b/drivers/gpu/drm/via/via_mm.c >> index 0ab93ff..7e3ad87 100644 >> --- a/drivers/gpu/drm/via/via_mm.c >> +++ b/drivers/gpu/drm/via/via_mm.c >> @@ -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; >> >> diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h >> index 98cb50e..439d1a1 100644 >> --- a/include/drm/drm_mm.h >> +++ b/include/drm/drm_mm.h >> @@ -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, >> -- >> 1.8.3.4 >> > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] drm/mm: add "best_match" flag to drm_mm_insert_node() 2013-08-06 9:39 ` David Herrmann @ 2013-08-06 11:03 ` Daniel Vetter 0 siblings, 0 replies; 21+ messages in thread From: Daniel Vetter @ 2013-08-06 11:03 UTC (permalink / raw) To: David Herrmann; +Cc: Dave Airlie, dri-devel@lists.freedesktop.org On Tue, Aug 6, 2013 at 11:39 AM, David Herrmann <dh.herrmann@gmail.com> wrote: > > On Mon, Aug 5, 2013 at 9:46 AM, Daniel Vetter <daniel@ffwll.ch> wrote: >> On Sat, Jul 27, 2013 at 01:36:27PM +0200, David Herrmann wrote: >>> 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 <dh.herrmann@gmail.com> >> >> 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. Hm, I didn't know that gcc checks enums even when they're used as flags with bitwise ops. That makes an enum indeed useful and the better option. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/4] drm/ttm: replace drm_mm_pre_get() by direct alloc 2013-07-25 13:55 [PATCH 0/4] DRM: Remove MM "Pre-Alloc" David Herrmann 2013-07-25 13:55 ` [PATCH 1/4] drm/mm: add "best_match" to drm_mm_insert_node() David Herrmann @ 2013-07-25 13:56 ` David Herrmann 2013-07-25 14:25 ` Daniel Vetter 2013-07-27 11:37 ` [PATCH v2 " David Herrmann 2013-07-25 13:56 ` [PATCH 3/4] drm/i915: pre-alloc instead of drm_mm search/get_block David Herrmann 2013-07-25 13:56 ` [PATCH 4/4] drm/mm: remove unused API David Herrmann 3 siblings, 2 replies; 21+ messages in thread From: David Herrmann @ 2013-07-25 13:56 UTC (permalink / raw) To: dri-devel; +Cc: Dave Airlie, Daniel Vetter 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 <dh.herrmann@gmail.com> --- drivers/gpu/drm/ttm/ttm_bo_manager.c | 40 +++++++++++++++++------------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo_manager.c b/drivers/gpu/drm/ttm/ttm_bo_manager.c index e4367f9..cbd2ec7 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_manager.c +++ b/drivers/gpu/drm/ttm/ttm_bo_manager.c @@ -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; } } -- 1.8.3.3 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 2/4] drm/ttm: replace drm_mm_pre_get() by direct alloc 2013-07-25 13:56 ` [PATCH 2/4] drm/ttm: replace drm_mm_pre_get() by direct alloc David Herrmann @ 2013-07-25 14:25 ` Daniel Vetter 2013-07-27 11:37 ` [PATCH v2 " David Herrmann 1 sibling, 0 replies; 21+ messages in thread From: Daniel Vetter @ 2013-07-25 14:25 UTC (permalink / raw) To: David Herrmann; +Cc: Dave Airlie, dri-devel, Daniel Vetter On Thu, Jul 25, 2013 at 03:56:00PM +0200, David Herrmann wrote: > 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 <dh.herrmann@gmail.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/ttm/ttm_bo_manager.c | 40 +++++++++++++++++------------------- > 1 file changed, 19 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_manager.c b/drivers/gpu/drm/ttm/ttm_bo_manager.c > index e4367f9..cbd2ec7 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_manager.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_manager.c > @@ -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; > } > } > -- > 1.8.3.3 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 2/4] drm/ttm: replace drm_mm_pre_get() by direct alloc 2013-07-25 13:56 ` [PATCH 2/4] drm/ttm: replace drm_mm_pre_get() by direct alloc David Herrmann 2013-07-25 14:25 ` Daniel Vetter @ 2013-07-27 11:37 ` David Herrmann 1 sibling, 0 replies; 21+ messages in thread From: David Herrmann @ 2013-07-27 11:37 UTC (permalink / raw) To: dri-devel; +Cc: Dave Airlie, Daniel Vetter 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 <dh.herrmann@gmail.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/ttm/ttm_bo_manager.c | 42 +++++++++++++++++------------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo_manager.c b/drivers/gpu/drm/ttm/ttm_bo_manager.c index e4be29e..c58eba33 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_manager.c +++ b/drivers/gpu/drm/ttm/ttm_bo_manager.c @@ -61,29 +61,25 @@ 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, - DRM_MM_SEARCH_BEST); - 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, + DRM_MM_SEARCH_BEST); + 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; } @@ -94,8 +90,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; } } -- 1.8.3.4 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/4] drm/i915: pre-alloc instead of drm_mm search/get_block 2013-07-25 13:55 [PATCH 0/4] DRM: Remove MM "Pre-Alloc" David Herrmann 2013-07-25 13:55 ` [PATCH 1/4] drm/mm: add "best_match" to drm_mm_insert_node() David Herrmann 2013-07-25 13:56 ` [PATCH 2/4] drm/ttm: replace drm_mm_pre_get() by direct alloc David Herrmann @ 2013-07-25 13:56 ` David Herrmann 2013-07-25 14:14 ` Chris Wilson 2013-07-27 11:38 ` [PATCH v2 " David Herrmann 2013-07-25 13:56 ` [PATCH 4/4] drm/mm: remove unused API David Herrmann 3 siblings, 2 replies; 21+ messages in thread From: David Herrmann @ 2013-07-25 13:56 UTC (permalink / raw) To: dri-devel; +Cc: Dave Airlie, Daniel Vetter 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 <daniel.vetter@ffwll.ch> Signed-off-by: David Herrmann <dh.herrmann@gmail.com> --- drivers/gpu/drm/i915/i915_gem_stolen.c | 72 ++++++++++++++++++++++------------ 1 file changed, 47 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 5521833..7a2f040 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -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; + /* Try to over-allocate to reduce reallocations and fragmentation */ + ret = drm_mm_insert_node(&dev_priv->mm.stolen, compressed_fb, + size <<= 1, 4096, false); + if (ret) + ret = drm_mm_insert_node(&dev_priv->mm.stolen, compressed_fb, + size >>= 1, 4096, false); + if (ret) { + kfree(compressed_fb); + goto err; + } + 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, 0); - 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, false); + if (ret) { + kfree(compressed_llb); + goto err_fb; + } + dev_priv->fbc.compressed_llb = compressed_llb; I915_WRITE(FBC_CFB_BASE, @@ -154,7 +162,8 @@ static int i915_setup_compression(struct drm_device *dev, int size) return 0; err_fb: - drm_mm_put_block(compressed_fb); + drm_mm_remove_node(compressed_fb); + kfree(compressed_fb); err: 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; @@ -183,11 +192,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; } @@ -320,6 +333,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; @@ -328,17 +342,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, 0); - 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, false); + 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; } @@ -382,7 +402,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; } @@ -422,7 +443,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; } } -- 1.8.3.3 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 3/4] drm/i915: pre-alloc instead of drm_mm search/get_block 2013-07-25 13:56 ` [PATCH 3/4] drm/i915: pre-alloc instead of drm_mm search/get_block David Herrmann @ 2013-07-25 14:14 ` Chris Wilson 2013-07-27 11:38 ` [PATCH v2 " David Herrmann 1 sibling, 0 replies; 21+ messages in thread From: Chris Wilson @ 2013-07-25 14:14 UTC (permalink / raw) To: David Herrmann; +Cc: Dave Airlie, dri-devel, Daniel Vetter On Thu, Jul 25, 2013 at 03:56:01PM +0200, David Herrmann wrote: > 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 <daniel.vetter@ffwll.ch> > Signed-off-by: David Herrmann <dh.herrmann@gmail.com> > --- > drivers/gpu/drm/i915/i915_gem_stolen.c | 72 ++++++++++++++++++++++------------ > 1 file changed, 47 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c > index 5521833..7a2f040 100644 > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c > @@ -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; > > + /* Try to over-allocate to reduce reallocations and fragmentation */ > + ret = drm_mm_insert_node(&dev_priv->mm.stolen, compressed_fb, > + size <<= 1, 4096, false); > + if (ret) > + ret = drm_mm_insert_node(&dev_priv->mm.stolen, compressed_fb, > + size >>= 1, 4096, false); > + if (ret) { > + kfree(compressed_fb); > + goto err; kfree(NULL) is legal so move the kfree(compressed_fb) below err: and we do a similar cleanup for err_fb: -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 3/4] drm/i915: pre-alloc instead of drm_mm search/get_block 2013-07-25 13:56 ` [PATCH 3/4] drm/i915: pre-alloc instead of drm_mm search/get_block David Herrmann 2013-07-25 14:14 ` Chris Wilson @ 2013-07-27 11:38 ` David Herrmann 2013-07-27 13:06 ` Chris Wilson 2013-07-27 14:21 ` [PATCH v3 " David Herrmann 1 sibling, 2 replies; 21+ messages in thread From: David Herrmann @ 2013-07-27 11:38 UTC (permalink / raw) To: dri-devel; +Cc: Dave Airlie, Daniel Vetter 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() Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: David Herrmann <dh.herrmann@gmail.com> --- drivers/gpu/drm/i915/i915_gem_stolen.c | 75 +++++++++++++++++++++------------- 1 file changed, 46 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index e355170..0044e65 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -112,34 +112,38 @@ 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; + /* 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; + 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) { + kfree(compressed_llb); + goto err_fb; + } + dev_priv->fbc.compressed_llb = compressed_llb; I915_WRITE(FBC_CFB_BASE, @@ -157,8 +161,9 @@ static int i915_setup_compression(struct drm_device *dev, int size) return 0; err_fb: - drm_mm_put_block(compressed_fb); + drm_mm_remove_node(compressed_fb); err: + 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 +191,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 +332,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 +341,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 +401,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 +442,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; } } -- 1.8.3.4 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 3/4] drm/i915: pre-alloc instead of drm_mm search/get_block 2013-07-27 11:38 ` [PATCH v2 " David Herrmann @ 2013-07-27 13:06 ` Chris Wilson 2013-07-27 13:09 ` David Herrmann 2013-07-27 14:21 ` [PATCH v3 " David Herrmann 1 sibling, 1 reply; 21+ messages in thread From: Chris Wilson @ 2013-07-27 13:06 UTC (permalink / raw) To: David Herrmann; +Cc: Dave Airlie, dri-devel, Daniel Vetter On Sat, Jul 27, 2013 at 01:38:42PM +0200, David Herrmann wrote: > 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. -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 3/4] drm/i915: pre-alloc instead of drm_mm search/get_block 2013-07-27 13:06 ` Chris Wilson @ 2013-07-27 13:09 ` David Herrmann 2013-07-27 13:15 ` Chris Wilson 0 siblings, 1 reply; 21+ messages in thread From: David Herrmann @ 2013-07-27 13:09 UTC (permalink / raw) To: Chris Wilson, David Herrmann, dri-devel@lists.freedesktop.org, Dave Airlie, Daniel Vetter Hi On Sat, Jul 27, 2013 at 3:06 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Sat, Jul 27, 2013 at 01:38:42PM +0200, David Herrmann wrote: >> 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. 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. Thanks David ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 3/4] drm/i915: pre-alloc instead of drm_mm search/get_block 2013-07-27 13:09 ` David Herrmann @ 2013-07-27 13:15 ` Chris Wilson 0 siblings, 0 replies; 21+ messages in thread From: Chris Wilson @ 2013-07-27 13:15 UTC (permalink / raw) To: David Herrmann Cc: Dave Airlie, dri-devel@lists.freedesktop.org, Daniel Vetter On Sat, Jul 27, 2013 at 03:09:48PM +0200, David Herrmann wrote: > Hi > > On Sat, Jul 27, 2013 at 3:06 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > On Sat, Jul 27, 2013 at 01:38:42PM +0200, David Herrmann wrote: > >> 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. > > 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. My concern in this case is to keep the two path similar; we use the same allocation style so we should use the same cleanup. Otherwise, the next time I look at the code, I wil wonder why they are different. :) -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 3/4] drm/i915: pre-alloc instead of drm_mm search/get_block 2013-07-27 11:38 ` [PATCH v2 " David Herrmann 2013-07-27 13:06 ` Chris Wilson @ 2013-07-27 14:21 ` David Herrmann 2013-08-05 7:49 ` Daniel Vetter 1 sibling, 1 reply; 21+ messages in thread From: David Herrmann @ 2013-07-27 14:21 UTC (permalink / raw) To: dri-devel; +Cc: Daniel Vetter, Dave Airlie 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 <daniel.vetter@ffwll.ch> Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: David Herrmann <dh.herrmann@gmail.com> --- drivers/gpu/drm/i915/i915_gem_stolen.c | 78 ++++++++++++++++++++-------------- 1 file changed, 47 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index e355170..a3d1a12 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -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; } } -- 1.8.3.4 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v3 3/4] drm/i915: pre-alloc instead of drm_mm search/get_block 2013-07-27 14:21 ` [PATCH v3 " David Herrmann @ 2013-08-05 7:49 ` Daniel Vetter 0 siblings, 0 replies; 21+ messages in thread From: Daniel Vetter @ 2013-08-05 7:49 UTC (permalink / raw) To: David Herrmann; +Cc: Dave Airlie, dri-devel, Daniel Vetter On Sat, Jul 27, 2013 at 04:21:27PM +0200, David Herrmann wrote: > 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 <daniel.vetter@ffwll.ch> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: David Herrmann <dh.herrmann@gmail.com> I think this won't conflict badly with the ongoing VMA rework in drm/i915, so Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> for merging through drm-next directly. -Daniel > --- > drivers/gpu/drm/i915/i915_gem_stolen.c | 78 ++++++++++++++++++++-------------- > 1 file changed, 47 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c > index e355170..a3d1a12 100644 > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c > @@ -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; > } > } > -- > 1.8.3.4 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 4/4] drm/mm: remove unused API 2013-07-25 13:55 [PATCH 0/4] DRM: Remove MM "Pre-Alloc" David Herrmann ` (2 preceding siblings ...) 2013-07-25 13:56 ` [PATCH 3/4] drm/i915: pre-alloc instead of drm_mm search/get_block David Herrmann @ 2013-07-25 13:56 ` David Herrmann 2013-07-25 14:27 ` Daniel Vetter 2013-07-27 11:39 ` [PATCH v2 " David Herrmann 3 siblings, 2 replies; 21+ messages in thread From: David Herrmann @ 2013-07-25 13:56 UTC (permalink / raw) To: dri-devel; +Cc: Dave Airlie, Daniel Vetter 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 <dh.herrmann@gmail.com> --- drivers/gpu/drm/drm_mm.c | 160 ++++++----------------------------------------- include/drm/drm_mm.h | 95 ---------------------------- 2 files changed, 20 insertions(+), 235 deletions(-) diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c index 5aad736..0946251 100644 --- a/drivers/gpu/drm/drm_mm.c +++ b/drivers/gpu/drm/drm_mm.c @@ -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); diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h index a30c9aa..4890c51 100644 --- a/include/drm/drm_mm.h +++ b/include/drm/drm_mm.h @@ -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, -- 1.8.3.3 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] drm/mm: remove unused API 2013-07-25 13:56 ` [PATCH 4/4] drm/mm: remove unused API David Herrmann @ 2013-07-25 14:27 ` Daniel Vetter 2013-07-27 11:39 ` [PATCH v2 " David Herrmann 1 sibling, 0 replies; 21+ messages in thread From: Daniel Vetter @ 2013-07-25 14:27 UTC (permalink / raw) To: David Herrmann; +Cc: Dave Airlie, dri-devel, Daniel Vetter On Thu, Jul 25, 2013 at 03:56:02PM +0200, David Herrmann wrote: > 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 <dh.herrmann@gmail.com> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/drm_mm.c | 160 ++++++----------------------------------------- > include/drm/drm_mm.h | 95 ---------------------------- > 2 files changed, 20 insertions(+), 235 deletions(-) > > diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c > index 5aad736..0946251 100644 > --- a/drivers/gpu/drm/drm_mm.c > +++ b/drivers/gpu/drm/drm_mm.c > @@ -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); > > diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h > index a30c9aa..4890c51 100644 > --- a/include/drm/drm_mm.h > +++ b/include/drm/drm_mm.h > @@ -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, > -- > 1.8.3.3 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 4/4] drm/mm: remove unused API 2013-07-25 13:56 ` [PATCH 4/4] drm/mm: remove unused API David Herrmann 2013-07-25 14:27 ` Daniel Vetter @ 2013-07-27 11:39 ` David Herrmann 1 sibling, 0 replies; 21+ messages in thread From: David Herrmann @ 2013-07-27 11:39 UTC (permalink / raw) To: dri-devel; +Cc: Dave Airlie, Daniel Vetter 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 <dh.herrmann@gmail.com> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/drm_mm.c | 160 ++++++----------------------------------------- include/drm/drm_mm.h | 95 ---------------------------- 2 files changed, 20 insertions(+), 235 deletions(-) diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c index 9a38327..aded1e1 100644 --- a/drivers/gpu/drm/drm_mm.c +++ b/drivers/gpu/drm/drm_mm.c @@ -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, + enum drm_mm_search_flags flags); +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, + enum drm_mm_search_flags flags); 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 @@ -279,27 +221,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 @@ -359,28 +280,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) { @@ -396,11 +295,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, - enum drm_mm_search_flags flags) +static 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) { struct drm_mm_node *entry; struct drm_mm_node *best; @@ -434,9 +333,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, @@ -481,7 +379,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. @@ -654,10 +551,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); @@ -677,22 +571,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); diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h index 439d1a1..cba6786 100644 --- a/include/drm/drm_mm.h +++ b/include/drm/drm_mm.h @@ -70,9 +70,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; @@ -123,13 +120,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 @@ -147,52 +137,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, @@ -229,52 +173,13 @@ static inline int drm_mm_insert_node_in_range(struct drm_mm *mm, 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); -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, - unsigned alignment, - unsigned long color, - unsigned long start, - unsigned long end, - 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, flags); -} -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, - enum drm_mm_search_flags flags) -{ - return drm_mm_search_free_in_range_generic(mm, size, alignment, 0, - start, end, flags); -} - 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, -- 1.8.3.4 ^ permalink raw reply related [flat|nested] 21+ messages in thread
end of thread, other threads:[~2013-08-06 11:03 UTC | newest] Thread overview: 21+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-07-25 13:55 [PATCH 0/4] DRM: Remove MM "Pre-Alloc" David Herrmann 2013-07-25 13:55 ` [PATCH 1/4] drm/mm: add "best_match" to drm_mm_insert_node() David Herrmann 2013-07-25 14:15 ` Daniel Vetter 2013-07-27 11:36 ` [PATCH 1/4] drm/mm: add "best_match" flag " David Herrmann 2013-08-05 7:46 ` Daniel Vetter 2013-08-06 9:39 ` David Herrmann 2013-08-06 11:03 ` Daniel Vetter 2013-07-25 13:56 ` [PATCH 2/4] drm/ttm: replace drm_mm_pre_get() by direct alloc David Herrmann 2013-07-25 14:25 ` Daniel Vetter 2013-07-27 11:37 ` [PATCH v2 " David Herrmann 2013-07-25 13:56 ` [PATCH 3/4] drm/i915: pre-alloc instead of drm_mm search/get_block David Herrmann 2013-07-25 14:14 ` Chris Wilson 2013-07-27 11:38 ` [PATCH v2 " David Herrmann 2013-07-27 13:06 ` Chris Wilson 2013-07-27 13:09 ` David Herrmann 2013-07-27 13:15 ` Chris Wilson 2013-07-27 14:21 ` [PATCH v3 " David Herrmann 2013-08-05 7:49 ` Daniel Vetter 2013-07-25 13:56 ` [PATCH 4/4] drm/mm: remove unused API David Herrmann 2013-07-25 14:27 ` Daniel Vetter 2013-07-27 11:39 ` [PATCH v2 " David Herrmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).