From: Matthew Auld <matthew.william.auld@gmail.com>
To: "Christian König" <ckoenig.leichtzumerken@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
ML dri-devel <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH 13/13] drm/ttm: flip the switch for driver allocated resources
Date: Wed, 5 May 2021 17:44:08 +0100 [thread overview]
Message-ID: <CAM0jSHN-4FWf0_9mZ5DA075jcri2=nMP6LsZGZ2cJtm-6LKWRQ@mail.gmail.com> (raw)
In-Reply-To: <20210430092508.60710-13-christian.koenig@amd.com>
On Fri, 30 Apr 2021 at 10:25, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Instead of both driver and TTM allocating memory finalize embedding the
> ttm_resource object as base into the driver backends.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 44 ++++++--------
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +-
> .../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h | 5 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 60 +++++++++----------
> drivers/gpu/drm/drm_gem_vram_helper.c | 3 +-
> drivers/gpu/drm/nouveau/nouveau_bo.c | 8 +--
> drivers/gpu/drm/nouveau/nouveau_mem.c | 11 ++--
> drivers/gpu/drm/nouveau/nouveau_mem.h | 14 ++---
> drivers/gpu/drm/nouveau/nouveau_ttm.c | 32 +++++-----
> drivers/gpu/drm/ttm/ttm_range_manager.c | 23 +++----
> drivers/gpu/drm/ttm/ttm_resource.c | 15 +----
> drivers/gpu/drm/ttm/ttm_sys_manager.c | 12 ++--
> drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 24 ++++----
> drivers/gpu/drm/vmwgfx/vmwgfx_thp.c | 27 ++++-----
> include/drm/ttm/ttm_range_manager.h | 3 +-
> include/drm/ttm/ttm_resource.h | 43 ++++++-------
> 16 files changed, 140 insertions(+), 186 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> index 2e7dffd3614d..d6c1960abf64 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> @@ -40,8 +40,7 @@ to_gtt_mgr(struct ttm_resource_manager *man)
> static inline struct amdgpu_gtt_node *
> to_amdgpu_gtt_node(struct ttm_resource *res)
> {
> - return container_of(res->mm_node, struct amdgpu_gtt_node,
> - base.mm_nodes[0]);
> + return container_of(res, struct amdgpu_gtt_node, base.base);
> }
>
> /**
> @@ -92,13 +91,13 @@ static DEVICE_ATTR(mem_info_gtt_used, S_IRUGO,
> /**
> * amdgpu_gtt_mgr_has_gart_addr - Check if mem has address space
> *
> - * @mem: the mem object to check
> + * @res: the mem object to check
> *
> * Check if a mem object has already address space allocated.
> */
> -bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *mem)
> +bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *res)
> {
> - struct amdgpu_gtt_node *node = to_amdgpu_gtt_node(mem);
> + struct amdgpu_gtt_node *node = to_amdgpu_gtt_node(res);
>
> return drm_mm_node_allocated(&node->base.mm_nodes[0]);
> }
> @@ -116,19 +115,20 @@ bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *mem)
> static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
> struct ttm_buffer_object *tbo,
> const struct ttm_place *place,
> - struct ttm_resource *mem)
> + struct ttm_resource **res)
> {
> struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
> + uint32_t num_pages = PFN_UP(tbo->base.size);
> struct amdgpu_gtt_node *node;
> int r;
>
> spin_lock(&mgr->lock);
> - if ((tbo->resource == mem || tbo->resource->mem_type != TTM_PL_TT) &&
> - atomic64_read(&mgr->available) < mem->num_pages) {
> + if (tbo->resource && tbo->resource->mem_type != TTM_PL_TT &&
> + atomic64_read(&mgr->available) < num_pages) {
> spin_unlock(&mgr->lock);
> return -ENOSPC;
> }
> - atomic64_sub(mem->num_pages, &mgr->available);
> + atomic64_sub(num_pages, &mgr->available);
> spin_unlock(&mgr->lock);
>
> node = kzalloc(struct_size(node, base.mm_nodes, 1), GFP_KERNEL);
> @@ -144,29 +144,28 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
> spin_lock(&mgr->lock);
> r = drm_mm_insert_node_in_range(&mgr->mm,
> &node->base.mm_nodes[0],
> - mem->num_pages,
> - tbo->page_alignment, 0,
> - place->fpfn, place->lpfn,
> + num_pages, tbo->page_alignment,
> + 0, place->fpfn, place->lpfn,
> DRM_MM_INSERT_BEST);
> spin_unlock(&mgr->lock);
> if (unlikely(r))
> goto err_free;
>
> - mem->start = node->base.mm_nodes[0].start;
> + node->base.base.start = node->base.mm_nodes[0].start;
> } else {
> node->base.mm_nodes[0].start = 0;
> - node->base.mm_nodes[0].size = mem->num_pages;
> - mem->start = AMDGPU_BO_INVALID_OFFSET;
> + node->base.mm_nodes[0].size = node->base.base.num_pages;
> + node->base.base.start = AMDGPU_BO_INVALID_OFFSET;
> }
>
> - mem->mm_node = &node->base.mm_nodes[0];
> + *res = &node->base.base;
> return 0;
>
> err_free:
> kfree(node);
>
> err_out:
> - atomic64_add(mem->num_pages, &mgr->available);
> + atomic64_add(num_pages, &mgr->available);
>
> return r;
> }
> @@ -180,21 +179,16 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
> * Free the allocated GTT again.
> */
> static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man,
> - struct ttm_resource *mem)
> + struct ttm_resource *res)
> {
> + struct amdgpu_gtt_node *node = to_amdgpu_gtt_node(res);
> struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
> - struct amdgpu_gtt_node *node;
> -
> - if (!mem->mm_node)
> - return;
> -
> - node = to_amdgpu_gtt_node(mem);
>
> spin_lock(&mgr->lock);
> if (drm_mm_node_allocated(&node->base.mm_nodes[0]))
> drm_mm_remove_node(&node->base.mm_nodes[0]);
> spin_unlock(&mgr->lock);
> - atomic64_add(mem->num_pages, &mgr->available);
> + atomic64_add(res->num_pages, &mgr->available);
>
> kfree(node);
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index e669a3adac44..84142eed66df 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -1315,7 +1315,7 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object *bo)
> if (bo->base.resv == &bo->base._resv)
> amdgpu_amdkfd_remove_fence_on_pt_pd_bos(abo);
>
> - if (bo->resource->mem_type != TTM_PL_VRAM || !bo->resource->mm_node ||
> + if (bo->resource->mem_type != TTM_PL_VRAM ||
> !(abo->flags & AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE))
> return;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> index 40f2adf305bc..59e0fefb15aa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> @@ -28,6 +28,7 @@
>
> #include <drm/drm_mm.h>
> #include <drm/ttm/ttm_resource.h>
> +#include <drm/ttm/ttm_range_manager.h>
>
> /* state back for walking over vram_mgr and gtt_mgr allocations */
> struct amdgpu_res_cursor {
> @@ -53,7 +54,7 @@ static inline void amdgpu_res_first(struct ttm_resource *res,
> {
> struct drm_mm_node *node;
>
> - if (!res || !res->mm_node) {
> + if (!res) {
> cur->start = start;
> cur->size = size;
> cur->remaining = size;
> @@ -63,7 +64,7 @@ static inline void amdgpu_res_first(struct ttm_resource *res,
>
> BUG_ON(start + size > res->num_pages << PAGE_SHIFT);
>
> - node = res->mm_node;
> + node = to_ttm_range_mgr_node(res)->mm_nodes;
> while (start >= node->size << PAGE_SHIFT)
> start -= node++->size << PAGE_SHIFT;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> index d59ec07c77bb..c8feae788e79 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> @@ -215,19 +215,20 @@ static u64 amdgpu_vram_mgr_vis_size(struct amdgpu_device *adev,
> u64 amdgpu_vram_mgr_bo_visible_size(struct amdgpu_bo *bo)
> {
> struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> - struct ttm_resource *mem = bo->tbo.resource;
> - struct drm_mm_node *nodes = mem->mm_node;
> - unsigned pages = mem->num_pages;
> + struct ttm_resource *res = bo->tbo.resource;
> + unsigned pages = res->num_pages;
> + struct drm_mm_node *mm;
> u64 usage;
>
> if (amdgpu_gmc_vram_full_visible(&adev->gmc))
> return amdgpu_bo_size(bo);
>
> - if (mem->start >= adev->gmc.visible_vram_size >> PAGE_SHIFT)
> + if (res->start >= adev->gmc.visible_vram_size >> PAGE_SHIFT)
> return 0;
>
> - for (usage = 0; nodes && pages; pages -= nodes->size, nodes++)
> - usage += amdgpu_vram_mgr_vis_size(adev, nodes);
> + mm = &container_of(res, struct ttm_range_mgr_node, base)->mm_nodes[0];
> + for (usage = 0; pages; pages -= mm->size, mm++)
> + usage += amdgpu_vram_mgr_vis_size(adev, mm);
>
> return usage;
> }
> @@ -363,7 +364,7 @@ static void amdgpu_vram_mgr_virt_start(struct ttm_resource *mem,
> static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
> struct ttm_buffer_object *tbo,
> const struct ttm_place *place,
> - struct ttm_resource *mem)
> + struct ttm_resource **res)
> {
> unsigned long lpfn, num_nodes, pages_per_node, pages_left, pages;
> struct amdgpu_vram_mgr *mgr = to_vram_mgr(man);
> @@ -384,7 +385,7 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
> max_bytes -= AMDGPU_VM_RESERVED_VRAM;
>
> /* bail out quickly if there's likely not enough VRAM for this BO */
> - mem_bytes = (u64)mem->num_pages << PAGE_SHIFT;
> + mem_bytes = tbo->base.size;
> if (atomic64_add_return(mem_bytes, &mgr->usage) > max_bytes) {
> r = -ENOSPC;
> goto error_sub;
> @@ -402,7 +403,7 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
> #endif
> pages_per_node = max_t(uint32_t, pages_per_node,
> tbo->page_alignment);
> - num_nodes = DIV_ROUND_UP(mem->num_pages, pages_per_node);
> + num_nodes = DIV_ROUND_UP(PFN_UP(mem_bytes), pages_per_node);
> }
>
> node = kvmalloc(struct_size(node, mm_nodes, num_nodes),
> @@ -418,8 +419,7 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
> if (place->flags & TTM_PL_FLAG_TOPDOWN)
> mode = DRM_MM_INSERT_HIGH;
>
> - mem->start = 0;
> - pages_left = mem->num_pages;
> + pages_left = node->base.num_pages;
>
> /* Limit maximum size to 2GB due to SG table limitations */
> pages = min(pages_left, 2UL << (30 - PAGE_SHIFT));
> @@ -447,7 +447,7 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
> }
>
> vis_usage += amdgpu_vram_mgr_vis_size(adev, &node->mm_nodes[i]);
> - amdgpu_vram_mgr_virt_start(mem, &node->mm_nodes[i]);
> + amdgpu_vram_mgr_virt_start(&node->base, &node->mm_nodes[i]);
> pages_left -= pages;
> ++i;
>
> @@ -457,10 +457,10 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
> spin_unlock(&mgr->lock);
>
> if (i == 1)
> - mem->placement |= TTM_PL_FLAG_CONTIGUOUS;
> + node->base.placement |= TTM_PL_FLAG_CONTIGUOUS;
>
> atomic64_add(vis_usage, &mgr->vis_usage);
> - mem->mm_node = &node->mm_nodes[0];
> + *res = &node->base;
> return 0;
>
> error_free:
> @@ -483,28 +483,22 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
> * Free the allocated VRAM again.
> */
> static void amdgpu_vram_mgr_del(struct ttm_resource_manager *man,
> - struct ttm_resource *mem)
> + struct ttm_resource *res)
> {
> + struct ttm_range_mgr_node *node = to_ttm_range_mgr_node(res);
> struct amdgpu_vram_mgr *mgr = to_vram_mgr(man);
> struct amdgpu_device *adev = to_amdgpu_device(mgr);
> - struct ttm_range_mgr_node *node;
> uint64_t usage = 0, vis_usage = 0;
> - unsigned pages = mem->num_pages;
> - struct drm_mm_node *nodes;
> -
> - if (!mem->mm_node)
> - return;
> -
> - node = to_ttm_range_mgr_node(mem);
> - nodes = &node->mm_nodes[0];
> + unsigned i, pages = res->num_pages;
>
> spin_lock(&mgr->lock);
> - while (pages) {
> - pages -= nodes->size;
> - drm_mm_remove_node(nodes);
> - usage += nodes->size << PAGE_SHIFT;
> - vis_usage += amdgpu_vram_mgr_vis_size(adev, nodes);
> - ++nodes;
> + for (i = 0, pages = res->num_pages; pages;
No need to initialise pages twice I suppose.
With the issue Nirmoy spotted fixed,
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2021-05-05 16:44 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-30 9:24 [PATCH 01/13] drm/ttm: add ttm_sys_manager v2 Christian König
2021-04-30 9:24 ` [PATCH 02/13] drm/ttm: always initialize the full ttm_resource Christian König
2021-04-30 12:05 ` Matthew Auld
2021-04-30 12:51 ` Christian König
2021-04-30 9:24 ` [PATCH 03/13] drm/ttm: properly allocate sys resource during swapout Christian König
2021-04-30 10:22 ` Matthew Auld
2021-04-30 9:24 ` [PATCH 04/13] drm/ttm: rename bo->mem and make it a pointer Christian König
2021-04-30 9:25 ` [PATCH 05/13] drm/ttm: allocate resource object instead of embedding it Christian König
2021-04-30 11:19 ` Matthew Auld
2021-04-30 9:25 ` [PATCH 06/13] drm/ttm: flip over the range manager to self allocated nodes Christian König
2021-04-30 13:14 ` Matthew Auld
2021-05-29 15:48 ` Thomas Hellström (Intel)
2021-05-30 16:51 ` Christian König
2021-05-31 8:56 ` Thomas Hellström (Intel)
2021-04-30 9:25 ` [PATCH 07/13] drm/ttm: flip over the sys " Christian König
2021-04-30 13:16 ` Matthew Auld
2021-04-30 15:04 ` Matthew Auld
2021-05-03 11:08 ` Christian König
2021-04-30 9:25 ` [PATCH 08/13] drm/amdgpu: revert "drm/amdgpu: stop allocating dummy GTT nodes" Christian König
2021-05-05 16:48 ` Matthew Auld
2021-04-30 9:25 ` [PATCH 09/13] drm/amdgpu: switch the GTT backend to self alloc Christian König
2021-04-30 14:43 ` Matthew Auld
2021-04-30 9:25 ` [PATCH 10/13] drm/amdgpu: switch the VRAM " Christian König
2021-04-30 14:53 ` Matthew Auld
2021-04-30 9:25 ` [PATCH 11/13] drm/nouveau: switch the TTM backends " Christian König
2021-04-30 15:02 ` Matthew Auld
2021-05-03 11:14 ` Christian König
2021-05-05 16:46 ` Matthew Auld
2021-04-30 9:25 ` [PATCH 12/13] drm/vmwgfx: " Christian König
2021-05-05 16:49 ` Matthew Auld
2021-04-30 9:25 ` [PATCH 13/13] drm/ttm: flip the switch for driver allocated resources Christian König
2021-05-03 16:15 ` Nirmoy
2021-05-05 16:44 ` Matthew Auld [this message]
2021-04-30 10:09 ` [PATCH 01/13] drm/ttm: add ttm_sys_manager v2 Matthew Auld
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAM0jSHN-4FWf0_9mZ5DA075jcri2=nMP6LsZGZ2cJtm-6LKWRQ@mail.gmail.com' \
--to=matthew.william.auld@gmail.com \
--cc=ckoenig.leichtzumerken@gmail.com \
--cc=daniel.vetter@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.