On Fri, 30 Apr 2021 at 10:25, Christian König
<ckoenig.leichtzumerken@xxxxxxxxx> 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@xxxxxxx>
> ---
> 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@xxxxxxxxx>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
