Re: [PATCH 1/4] drm/mm: add "best_match" flag to drm_mm_insert_node()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

On Mon, Aug 5, 2013 at 9:46 AM, Daniel Vetter <daniel@xxxxxxxx> 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@xxxxxxxxx>
>
> 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@xxxxxxxx>
>> ---
>>  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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux