Re: [PATCH 06/13] drm/ttm: flip over the range manager to self allocated nodes

On Fri, 30 Apr 2021 at 10:25, Christian König ckoenig.leichtzumerken@gmail.com wrote:

Start with the range manager to make the resource object the base class for the allocated nodes.

While at it cleanup a lot of the code around that.

Signed-off-by: Christian König christian.koenig@amd.com

drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 1 + drivers/gpu/drm/drm_gem_vram_helper.c | 2 + drivers/gpu/drm/nouveau/nouveau_ttm.c | 1 + drivers/gpu/drm/qxl/qxl_ttm.c | 1 + drivers/gpu/drm/radeon/radeon_ttm.c | 1 + drivers/gpu/drm/ttm/ttm_range_manager.c | 56 ++++++++++++++++++------- drivers/gpu/drm/ttm/ttm_resource.c | 26 ++++++++---- include/drm/ttm/ttm_bo_driver.h | 26 ------------ include/drm/ttm/ttm_range_manager.h | 43 +++++++++++++++++++ include/drm/ttm/ttm_resource.h | 3 ++ 10 files changed, 110 insertions(+), 50 deletions(-) create mode 100644 include/drm/ttm/ttm_range_manager.h

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 3fe2482a40b4..3d5863262337 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -46,6 +46,7 @@ #include <drm/ttm/ttm_bo_api.h> #include <drm/ttm/ttm_bo_driver.h> #include <drm/ttm/ttm_placement.h> +#include <drm/ttm/ttm_range_manager.h>

#include <drm/amdgpu_drm.h>

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 83e7258c7f90..17a4c5d47b6a 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -17,6 +17,8 @@ #include <drm/drm_prime.h> #include <drm/drm_simple_kms_helper.h>

+#include <drm/ttm/ttm_range_manager.h>

static const struct drm_gem_object_funcs drm_gem_vram_object_funcs;

/** diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c index b81ae90b8449..15c7627f8f58 100644 --- a/drivers/gpu/drm/nouveau/nouveau_ttm.c +++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c @@ -32,6 +32,7 @@ #include "nouveau_ttm.h"

#include <drm/drm_legacy.h> +#include <drm/ttm/ttm_range_manager.h>

#include <core/tegra.h>

diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c index 8aa87b8edb9c..19fd39d9a00c 100644 --- a/drivers/gpu/drm/qxl/qxl_ttm.c +++ b/drivers/gpu/drm/qxl/qxl_ttm.c @@ -32,6 +32,7 @@ #include <drm/ttm/ttm_bo_api.h> #include <drm/ttm/ttm_bo_driver.h> #include <drm/ttm/ttm_placement.h> +#include <drm/ttm/ttm_range_manager.h>

#include "qxl_drv.h" #include "qxl_object.h" diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index 5191e994bff7..c3d2f1fce71a 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -46,6 +46,7 @@ #include <drm/ttm/ttm_bo_api.h> #include <drm/ttm/ttm_bo_driver.h> #include <drm/ttm/ttm_placement.h> +#include <drm/ttm/ttm_range_manager.h>

#include "radeon_reg.h" #include "radeon.h" diff --git a/drivers/gpu/drm/ttm/ttm_range_manager.c b/drivers/gpu/drm/ttm/ttm_range_manager.c index b9d5da6e6a81..ce5d07ca384c 100644 --- a/drivers/gpu/drm/ttm/ttm_range_manager.c +++ b/drivers/gpu/drm/ttm/ttm_range_manager.c @@ -29,12 +29,13 @@

• Authors: Thomas Hellstrom <thellstrom-at-vmware-dot-com>

*/

-#include <drm/ttm/ttm_bo_driver.h> +#include <drm/ttm/ttm_device.h> #include <drm/ttm/ttm_placement.h> +#include <drm/ttm/ttm_range_manager.h> +#include <drm/ttm/ttm_bo_api.h> #include <drm/drm_mm.h> #include <linux/slab.h> #include <linux/spinlock.h> -#include <linux/module.h>

/*

• Currently we use a spinlock for the lock, but a mutex *may* be

@@ -60,8 +61,8 @@ static int ttm_range_man_alloc(struct ttm_resource_manager *man, struct ttm_resource *mem) { struct ttm_range_manager *rman = to_range_manager(man);

•   struct ttm_range_mgr_node *node;
    struct drm_mm *mm = &rman->mm;
  

•   struct drm_mm_node *node;
    enum drm_mm_insert_mode mode;
    unsigned long lpfn;
    int ret;
  

@@ -70,7 +71,7 @@ static int ttm_range_man_alloc(struct ttm_resource_manager *man, if (!lpfn) lpfn = man->size;

•   node = kzalloc(sizeof(*node), GFP_KERNEL);
  

•   node = kzalloc(struct_size(node, mm_nodes, 1), GFP_KERNEL);
    if (!node)
            return -ENOMEM;
  
  

@@ -78,17 +79,19 @@ static int ttm_range_man_alloc(struct ttm_resource_manager *man, if (place->flags & TTM_PL_FLAG_TOPDOWN) mode = DRM_MM_INSERT_HIGH;

•   ttm_resource_init(bo, place, &node->base);
  

•   spin_lock(&rman->lock);
  

•   ret = drm_mm_insert_node_in_range(mm, node, mem->num_pages,
  

•                                     bo->page_alignment, 0,
  

•   ret = drm_mm_insert_node_in_range(mm, &node->mm_nodes[0],
  

•                                     mem->num_pages, bo->page_alignment, 0,
                                      place->fpfn, lpfn, mode);
    spin_unlock(&rman->lock);
  
    if (unlikely(ret)) {
            kfree(node);
    } else {
  

•           mem->mm_node = node;
  

•           mem->start = node->start;
  

•           mem->mm_node = &node->mm_nodes[0];
  

•           mem->start = node->mm_nodes[0].start;
    }
  
    return ret;
  

@@ -98,15 +101,19 @@ static void ttm_range_man_free(struct ttm_resource_manager *man, struct ttm_resource *mem) { struct ttm_range_manager *rman = to_range_manager(man);

•   struct ttm_range_mgr_node *node;
  
  

•   if (mem->mm_node) {
  

•           spin_lock(&rman->lock);
  

•           drm_mm_remove_node(mem->mm_node);
  

•           spin_unlock(&rman->lock);
  

•   if (!mem->mm_node)
  

•           return;
  
  

•           kfree(mem->mm_node);
  

•           mem->mm_node = NULL;
  

•   }
  

•   node = to_ttm_range_mgr_node(mem);
  

•   spin_lock(&rman->lock);
  

•   drm_mm_remove_node(&node->mm_nodes[0]);
  

•   spin_unlock(&rman->lock);
  

•   kfree(node);
  

•   mem->mm_node = NULL;
  

}

static void ttm_range_man_debug(struct ttm_resource_manager *man, @@ -125,6 +132,17 @@ static const struct ttm_resource_manager_func ttm_range_manager_func = { .debug = ttm_range_man_debug };

+/**

• • ttm_range_man_init
• • @bdev: ttm device
• • @type: memory manager type
• • @use_tt: if the memory manager uses tt
• • @p_size: size of area to be managed in pages.
• • Initialise a generic range manager for the selected memory type.
• • The range manager is installed for this device in the type slot.
• */

int ttm_range_man_init(struct ttm_device *bdev, unsigned type, bool use_tt, unsigned long p_size) @@ -152,6 +170,14 @@ int ttm_range_man_init(struct ttm_device *bdev, } EXPORT_SYMBOL(ttm_range_man_init);

+/**

• • ttm_range_man_fini
• • @bdev: ttm device
• • @type: memory manager type
• • Remove the generic range manager from a slot and tear it down.
• */

int ttm_range_man_fini(struct ttm_device *bdev, unsigned type) { diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c index 7ebcc3e6818c..b412ae515e98 100644 --- a/drivers/gpu/drm/ttm/ttm_resource.c +++ b/drivers/gpu/drm/ttm/ttm_resource.c @@ -25,16 +25,10 @@ #include <drm/ttm/ttm_resource.h> #include <drm/ttm/ttm_bo_driver.h>

-int ttm_resource_alloc(struct ttm_buffer_object *bo,

•                  const struct ttm_place *place,
  

•                  struct ttm_resource **res_ptr)
  

+void ttm_resource_init(struct ttm_buffer_object *bo,

•                   const struct ttm_place *place,
  

•                   struct ttm_resource *res)
  

{

•   struct ttm_resource_manager *man =
  

•           ttm_manager_type(bo->bdev, place->mem_type);
  

•   struct ttm_resource *res;
  

•   int r;
  

•   res = kmalloc(sizeof(*res), GFP_KERNEL);
    res->mm_node = NULL;
    res->start = 0;
    res->num_pages = PFN_UP(bo->base.size);
  

@@ -44,6 +38,20 @@ int ttm_resource_alloc(struct ttm_buffer_object *bo, res->bus.offset = 0; res->bus.is_iomem = false; res->bus.caching = ttm_cached; +} +EXPORT_SYMBOL(ttm_resource_init);

+int ttm_resource_alloc(struct ttm_buffer_object *bo,

•                  const struct ttm_place *place,
  

•                  struct ttm_resource **res_ptr)
  

+{

•   struct ttm_resource_manager *man =
  

•           ttm_manager_type(bo->bdev, place->mem_type);
  

•   struct ttm_resource *res;
  

•   int r;
  

•   res = kmalloc(sizeof(*res), GFP_KERNEL);
  

If (!res)

•   ttm_resource_init(bo, place, res);
    r = man->func->alloc(man, bo, place, res);
    if (r) {
            kfree(res);
  

diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index ead0ef7136c8..b266971c1974 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -304,30 +304,4 @@ int ttm_bo_tt_bind(struct ttm_buffer_object *bo, struct ttm_resource *mem); */ void ttm_bo_tt_destroy(struct ttm_buffer_object *bo);

-/**

• • ttm_range_man_init
• • @bdev: ttm device
• • @type: memory manager type
• • @use_tt: if the memory manager uses tt
• • @p_size: size of area to be managed in pages.
• • Initialise a generic range manager for the selected memory type.
• • The range manager is installed for this device in the type slot.
• */

-int ttm_range_man_init(struct ttm_device *bdev,

•                  unsigned type, bool use_tt,
  

•                  unsigned long p_size);
  

-/**

• • ttm_range_man_fini
• • @bdev: ttm device
• • @type: memory manager type
• • Remove the generic range manager from a slot and tear it down.
• */

-int ttm_range_man_fini(struct ttm_device *bdev,

•                  unsigned type);
  

#endif diff --git a/include/drm/ttm/ttm_range_manager.h b/include/drm/ttm/ttm_range_manager.h new file mode 100644 index 000000000000..e02b6c8d355e --- /dev/null +++ b/include/drm/ttm/ttm_range_manager.h @@ -0,0 +1,43 @@ +/* SPDX-License-Identifier: GPL-2.0 OR MIT */

+#ifndef _TTM_RANGE_MANAGER_H_ +#define _TTM_RANGE_MANAGER_H_

+#include <drm/ttm/ttm_resource.h> +#include <drm/drm_mm.h>

+/**

• • struct ttm_range_mgr_node
• • @base: base clase we extend
• • @mm_nodes: MM nodes, usually 1
• • Extending the ttm_resource object to manage an address space allocation with
• • one or more drm_mm_nodes.
• */

+struct ttm_range_mgr_node {

•   struct ttm_resource base;
  

•   struct drm_mm_node mm_nodes[];
  

+};

+/**

• • to_ttm_range_mgr_node
• • @res: the resource to upcast
• • Upcast the ttm_resource object into a ttm_range_mgr_node object.
• */

+static inline struct ttm_range_mgr_node * +to_ttm_range_mgr_node(struct ttm_resource *res) +{

•   return container_of(res->mm_node, struct ttm_range_mgr_node,
  

•                       mm_nodes[1]);
  

Should be mm_nodes[0]?

Otherwise I think looks good, Reviewed-by: Matthew Auld matthew.auld@intel.com