All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/8] drm/amdgpu: new resource cursor
@ 2021-03-08 13:40 Christian König
  2021-03-08 13:40 ` [PATCH 2/8] drm/amdgpu: use the new cursor in amdgpu_ttm_copy_mem_to_mem Christian König
                   ` (7 more replies)
  0 siblings, 8 replies; 18+ messages in thread
From: Christian König @ 2021-03-08 13:40 UTC (permalink / raw)
  To: Arunpravin.PaneerSelvam, amd-gfx

Allows to walk over the drm_mm nodes in a TTM resource object.

Signed-off-by: Christian König <christian.koenig@amd.com>
Acked-by: Oak Zeng <Oak.Zeng@amd.com>
Tested-by: Nirmoy Das <nirmoy.das@amd.com>
---
 .../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h    | 105 ++++++++++++++++++
 1 file changed, 105 insertions(+)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
new file mode 100644
index 000000000000..1335e098510f
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright 2020 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * Authors: Christian König
+ */
+
+#ifndef __AMDGPU_RES_CURSOR_H__
+#define __AMDGPU_RES_CURSOR_H__
+
+#include <drm/drm_mm.h>
+#include <drm/ttm/ttm_resource.h>
+
+/* state back for walking over vram_mgr and gtt_mgr allocations */
+struct amdgpu_res_cursor {
+	uint64_t		start;
+	uint64_t		size;
+	uint64_t		remaining;
+	struct drm_mm_node	*node;
+};
+
+/**
+ * amdgpu_res_first - initialize a amdgpu_res_cursor
+ *
+ * @res: TTM resource object to walk
+ * @start: Start of the range
+ * @size: Size of the range
+ * @cur: cursor object to initialize
+ *
+ * Start walking over the range of allocations between @start and @size.
+ */
+static inline void amdgpu_res_first(struct ttm_resource *res,
+				    uint64_t start, uint64_t size,
+				    struct amdgpu_res_cursor *cur)
+{
+	struct drm_mm_node *node;
+
+	if (!res || !res->mm_node) {
+		cur->start = start;
+		cur->size = size;
+		cur->remaining = size;
+		cur->node = NULL;
+		return;
+	}
+
+	BUG_ON(start + size > res->num_pages << PAGE_SHIFT);
+
+	node = res->mm_node;
+	while (start > node->size << PAGE_SHIFT)
+		start -= node++->size << PAGE_SHIFT;
+
+	cur->start = (node->start << PAGE_SHIFT) + start;
+	cur->size = (node->size << PAGE_SHIFT) - start;
+	cur->remaining = size;
+	cur->node = node;
+}
+
+/**
+ * amdgpu_res_next - advance the cursor
+ *
+ * @cur: the cursor to advance
+ * @size: number of bytes to move forward
+ *
+ * Move the cursor @size bytes forwrad, walking to the next node if necessary.
+ */
+static inline void amdgpu_res_next(struct amdgpu_res_cursor *cur, uint64_t size)
+{
+	struct drm_mm_node *node = cur->node;
+
+	BUG_ON(size > cur->remaining);
+
+	cur->remaining -= size;
+	if (!cur->remaining)
+		return;
+
+	cur->size -= size;
+	if (cur->size) {
+		cur->start += size;
+		return;
+	}
+
+	cur->node = ++node;
+	cur->start = node->start << PAGE_SHIFT;
+	cur->size = min(node->size << PAGE_SHIFT, cur->remaining);
+}
+
+#endif
-- 
2.25.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/8] drm/amdgpu: use the new cursor in amdgpu_ttm_copy_mem_to_mem
  2021-03-08 13:40 [PATCH 1/8] drm/amdgpu: new resource cursor Christian König
@ 2021-03-08 13:40 ` Christian König
  2021-03-08 13:40 ` [PATCH 3/8] drm/amdgpu: use the new cursor in amdgpu_fill_buffer Christian König
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Christian König @ 2021-03-08 13:40 UTC (permalink / raw)
  To: Arunpravin.PaneerSelvam, amd-gfx

Separate the drm_mm_node walking from the actual handling.

Signed-off-by: Christian König <christian.koenig@amd.com>
Acked-by: Oak Zeng <Oak.Zeng@amd.com>
Tested-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 87 ++++++++-----------------
 1 file changed, 26 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 37751e7e4edc..f71a9ff06748 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -56,6 +56,7 @@
 #include "amdgpu_sdma.h"
 #include "amdgpu_ras.h"
 #include "amdgpu_atomfirmware.h"
+#include "amdgpu_res_cursor.h"
 #include "bif/bif_4_1_d.h"
 
 #define AMDGPU_TTM_VRAM_MAX_DW_READ	(size_t)128
@@ -223,9 +224,8 @@ static struct drm_mm_node *amdgpu_find_mm_node(struct ttm_resource *mem,
  * amdgpu_ttm_map_buffer - Map memory into the GART windows
  * @bo: buffer object to map
  * @mem: memory object to map
- * @mm_node: drm_mm node object to map
+ * @mm_cur: range to map
  * @num_pages: number of pages to map
- * @offset: offset into @mm_node where to start
  * @window: which GART window to use
  * @ring: DMA ring to use for the copy
  * @tmz: if we should setup a TMZ enabled mapping
@@ -236,10 +236,10 @@ static struct drm_mm_node *amdgpu_find_mm_node(struct ttm_resource *mem,
  */
 static int amdgpu_ttm_map_buffer(struct ttm_buffer_object *bo,
 				 struct ttm_resource *mem,
-				 struct drm_mm_node *mm_node,
-				 unsigned num_pages, uint64_t offset,
-				 unsigned window, struct amdgpu_ring *ring,
-				 bool tmz, uint64_t *addr)
+				 struct amdgpu_res_cursor *mm_cur,
+				 unsigned num_pages, unsigned window,
+				 struct amdgpu_ring *ring, bool tmz,
+				 uint64_t *addr)
 {
 	struct amdgpu_device *adev = ring->adev;
 	struct amdgpu_job *job;
@@ -256,14 +256,15 @@ static int amdgpu_ttm_map_buffer(struct ttm_buffer_object *bo,
 
 	/* Map only what can't be accessed directly */
 	if (!tmz && mem->start != AMDGPU_BO_INVALID_OFFSET) {
-		*addr = amdgpu_mm_node_addr(bo, mm_node, mem) + offset;
+		*addr = amdgpu_ttm_domain_start(adev, mem->mem_type) +
+			mm_cur->start;
 		return 0;
 	}
 
 	*addr = adev->gmc.gart_start;
 	*addr += (u64)window * AMDGPU_GTT_MAX_TRANSFER_SIZE *
 		AMDGPU_GPU_PAGE_SIZE;
-	*addr += offset & ~PAGE_MASK;
+	*addr += mm_cur->start & ~PAGE_MASK;
 
 	num_dw = ALIGN(adev->mman.buffer_funcs->copy_num_dw, 8);
 	num_bytes = num_pages * 8;
@@ -291,17 +292,17 @@ static int amdgpu_ttm_map_buffer(struct ttm_buffer_object *bo,
 	cpu_addr = &job->ibs[0].ptr[num_dw];
 
 	if (mem->mem_type == TTM_PL_TT) {
-		dma_addr_t *dma_address;
+		dma_addr_t *dma_addr;
 
-		dma_address = &bo->ttm->dma_address[offset >> PAGE_SHIFT];
-		r = amdgpu_gart_map(adev, 0, num_pages, dma_address, flags,
+		dma_addr = &bo->ttm->dma_address[mm_cur->start >> PAGE_SHIFT];
+		r = amdgpu_gart_map(adev, 0, num_pages, dma_addr, flags,
 				    cpu_addr);
 		if (r)
 			goto error_free;
 	} else {
 		dma_addr_t dma_address;
 
-		dma_address = (mm_node->start << PAGE_SHIFT) + offset;
+		dma_address = mm_cur->start;
 		dma_address += adev->vm_manager.vram_base_offset;
 
 		for (i = 0; i < num_pages; ++i) {
@@ -353,9 +354,8 @@ int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device *adev,
 	const uint32_t GTT_MAX_BYTES = (AMDGPU_GTT_MAX_TRANSFER_SIZE *
 					AMDGPU_GPU_PAGE_SIZE);
 
-	uint64_t src_node_size, dst_node_size, src_offset, dst_offset;
 	struct amdgpu_ring *ring = adev->mman.buffer_funcs_ring;
-	struct drm_mm_node *src_mm, *dst_mm;
+	struct amdgpu_res_cursor src_mm, dst_mm;
 	struct dma_fence *fence = NULL;
 	int r = 0;
 
@@ -364,29 +364,13 @@ int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device *adev,
 		return -EINVAL;
 	}
 
-	src_offset = src->offset;
-	if (src->mem->mm_node) {
-		src_mm = amdgpu_find_mm_node(src->mem, &src_offset);
-		src_node_size = (src_mm->size << PAGE_SHIFT) - src_offset;
-	} else {
-		src_mm = NULL;
-		src_node_size = ULLONG_MAX;
-	}
-
-	dst_offset = dst->offset;
-	if (dst->mem->mm_node) {
-		dst_mm = amdgpu_find_mm_node(dst->mem, &dst_offset);
-		dst_node_size = (dst_mm->size << PAGE_SHIFT) - dst_offset;
-	} else {
-		dst_mm = NULL;
-		dst_node_size = ULLONG_MAX;
-	}
+	amdgpu_res_first(src->mem, src->offset, size, &src_mm);
+	amdgpu_res_first(dst->mem, dst->offset, size, &dst_mm);
 
 	mutex_lock(&adev->mman.gtt_window_lock);
-
-	while (size) {
-		uint32_t src_page_offset = src_offset & ~PAGE_MASK;
-		uint32_t dst_page_offset = dst_offset & ~PAGE_MASK;
+	while (src_mm.remaining) {
+		uint32_t src_page_offset = src_mm.start & ~PAGE_MASK;
+		uint32_t dst_page_offset = dst_mm.start & ~PAGE_MASK;
 		struct dma_fence *next;
 		uint32_t cur_size;
 		uint64_t from, to;
@@ -395,19 +379,19 @@ int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device *adev,
 		 * begins at an offset, then adjust the size accordingly
 		 */
 		cur_size = max(src_page_offset, dst_page_offset);
-		cur_size = min(min3(src_node_size, dst_node_size, size),
+		cur_size = min(min3(src_mm.size, dst_mm.size, size),
 			       (uint64_t)(GTT_MAX_BYTES - cur_size));
 
 		/* Map src to window 0 and dst to window 1. */
-		r = amdgpu_ttm_map_buffer(src->bo, src->mem, src_mm,
+		r = amdgpu_ttm_map_buffer(src->bo, src->mem, &src_mm,
 					  PFN_UP(cur_size + src_page_offset),
-					  src_offset, 0, ring, tmz, &from);
+					  0, ring, tmz, &from);
 		if (r)
 			goto error;
 
-		r = amdgpu_ttm_map_buffer(dst->bo, dst->mem, dst_mm,
+		r = amdgpu_ttm_map_buffer(dst->bo, dst->mem, &dst_mm,
 					  PFN_UP(cur_size + dst_page_offset),
-					  dst_offset, 1, ring, tmz, &to);
+					  1, ring, tmz, &to);
 		if (r)
 			goto error;
 
@@ -419,27 +403,8 @@ int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device *adev,
 		dma_fence_put(fence);
 		fence = next;
 
-		size -= cur_size;
-		if (!size)
-			break;
-
-		src_node_size -= cur_size;
-		if (!src_node_size) {
-			++src_mm;
-			src_node_size = src_mm->size << PAGE_SHIFT;
-			src_offset = 0;
-		} else {
-			src_offset += cur_size;
-		}
-
-		dst_node_size -= cur_size;
-		if (!dst_node_size) {
-			++dst_mm;
-			dst_node_size = dst_mm->size << PAGE_SHIFT;
-			dst_offset = 0;
-		} else {
-			dst_offset += cur_size;
-		}
+		amdgpu_res_next(&src_mm, cur_size);
+		amdgpu_res_next(&dst_mm, cur_size);
 	}
 error:
 	mutex_unlock(&adev->mman.gtt_window_lock);
-- 
2.25.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/8] drm/amdgpu: use the new cursor in amdgpu_fill_buffer
  2021-03-08 13:40 [PATCH 1/8] drm/amdgpu: new resource cursor Christian König
  2021-03-08 13:40 ` [PATCH 2/8] drm/amdgpu: use the new cursor in amdgpu_ttm_copy_mem_to_mem Christian König
@ 2021-03-08 13:40 ` Christian König
  2021-03-08 13:40 ` [PATCH 4/8] drm/amdgpu: use new cursor in amdgpu_ttm_io_mem_pfn Christian König
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Christian König @ 2021-03-08 13:40 UTC (permalink / raw)
  To: Arunpravin.PaneerSelvam, amd-gfx

Separate the drm_mm_node walking from the actual handling.

Signed-off-by: Christian König <christian.koenig@amd.com>
Acked-by: Oak Zeng <Oak.Zeng@amd.com>
Tested-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 65 ++++++-------------------
 1 file changed, 15 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index f71a9ff06748..365d5693f5f0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -178,28 +178,6 @@ static int amdgpu_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 					  filp->private_data);
 }
 
-/**
- * amdgpu_mm_node_addr - Compute the GPU relative offset of a GTT buffer.
- *
- * @bo: The bo to assign the memory to.
- * @mm_node: Memory manager node for drm allocator.
- * @mem: The region where the bo resides.
- *
- */
-static uint64_t amdgpu_mm_node_addr(struct ttm_buffer_object *bo,
-				    struct drm_mm_node *mm_node,
-				    struct ttm_resource *mem)
-{
-	uint64_t addr = 0;
-
-	if (mm_node->start != AMDGPU_BO_INVALID_OFFSET) {
-		addr = mm_node->start << PAGE_SHIFT;
-		addr += amdgpu_ttm_domain_start(amdgpu_ttm_adev(bo->bdev),
-						mem->mem_type);
-	}
-	return addr;
-}
-
 /**
  * amdgpu_find_mm_node - Helper function finds the drm_mm_node corresponding to
  * @offset. It also modifies the offset to be within the drm_mm_node returned
@@ -2081,9 +2059,9 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
 	uint32_t max_bytes = adev->mman.buffer_funcs->fill_max_bytes;
 	struct amdgpu_ring *ring = adev->mman.buffer_funcs_ring;
 
-	struct drm_mm_node *mm_node;
-	unsigned long num_pages;
+	struct amdgpu_res_cursor cursor;
 	unsigned int num_loops, num_dw;
+	uint64_t num_bytes;
 
 	struct amdgpu_job *job;
 	int r;
@@ -2099,15 +2077,13 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
 			return r;
 	}
 
-	num_pages = bo->tbo.mem.num_pages;
-	mm_node = bo->tbo.mem.mm_node;
+	num_bytes = bo->tbo.mem.num_pages << PAGE_SHIFT;
 	num_loops = 0;
-	while (num_pages) {
-		uint64_t byte_count = mm_node->size << PAGE_SHIFT;
 
-		num_loops += DIV_ROUND_UP_ULL(byte_count, max_bytes);
-		num_pages -= mm_node->size;
-		++mm_node;
+	amdgpu_res_first(&bo->tbo.mem, 0, num_bytes, &cursor);
+	while (cursor.remaining) {
+		num_loops += DIV_ROUND_UP_ULL(cursor.size, max_bytes);
+		amdgpu_res_next(&cursor, cursor.size);
 	}
 	num_dw = num_loops * adev->mman.buffer_funcs->fill_num_dw;
 
@@ -2129,27 +2105,16 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
 		}
 	}
 
-	num_pages = bo->tbo.mem.num_pages;
-	mm_node = bo->tbo.mem.mm_node;
-
-	while (num_pages) {
-		uint64_t byte_count = mm_node->size << PAGE_SHIFT;
-		uint64_t dst_addr;
+	amdgpu_res_first(&bo->tbo.mem, 0, num_bytes, &cursor);
+	while (cursor.remaining) {
+		uint32_t cur_size = min_t(uint64_t, cursor.size, max_bytes);
+		uint64_t dst_addr = cursor.start;
 
-		dst_addr = amdgpu_mm_node_addr(&bo->tbo, mm_node, &bo->tbo.mem);
-		while (byte_count) {
-			uint32_t cur_size_in_bytes = min_t(uint64_t, byte_count,
-							   max_bytes);
+		dst_addr += amdgpu_ttm_domain_start(adev, bo->tbo.mem.mem_type);
+		amdgpu_emit_fill_buffer(adev, &job->ibs[0], src_data, dst_addr,
+					cur_size);
 
-			amdgpu_emit_fill_buffer(adev, &job->ibs[0], src_data,
-						dst_addr, cur_size_in_bytes);
-
-			dst_addr += cur_size_in_bytes;
-			byte_count -= cur_size_in_bytes;
-		}
-
-		num_pages -= mm_node->size;
-		++mm_node;
+		amdgpu_res_next(&cursor, cur_size);
 	}
 
 	amdgpu_ring_pad_ib(ring, &job->ibs[0]);
-- 
2.25.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/8] drm/amdgpu: use new cursor in amdgpu_ttm_io_mem_pfn
  2021-03-08 13:40 [PATCH 1/8] drm/amdgpu: new resource cursor Christian König
  2021-03-08 13:40 ` [PATCH 2/8] drm/amdgpu: use the new cursor in amdgpu_ttm_copy_mem_to_mem Christian König
  2021-03-08 13:40 ` [PATCH 3/8] drm/amdgpu: use the new cursor in amdgpu_fill_buffer Christian König
@ 2021-03-08 13:40 ` Christian König
  2021-03-08 13:40 ` [PATCH 5/8] drm/amdgpu: use the new cursor in amdgpu_ttm_access_memory Christian König
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Christian König @ 2021-03-08 13:40 UTC (permalink / raw)
  To: Arunpravin.PaneerSelvam, amd-gfx

Separate the drm_mm_node walking from the actual handling.

Signed-off-by: Christian König <christian.koenig@amd.com>
Acked-by: Oak Zeng <Oak.Zeng@amd.com>
Tested-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 365d5693f5f0..517611b709fa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -631,12 +631,10 @@ static unsigned long amdgpu_ttm_io_mem_pfn(struct ttm_buffer_object *bo,
 					   unsigned long page_offset)
 {
 	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
-	uint64_t offset = (page_offset << PAGE_SHIFT);
-	struct drm_mm_node *mm;
+	struct amdgpu_res_cursor cursor;
 
-	mm = amdgpu_find_mm_node(&bo->mem, &offset);
-	offset += adev->gmc.aper_base;
-	return mm->start + (offset >> PAGE_SHIFT);
+	amdgpu_res_first(&bo->mem, (u64)page_offset << PAGE_SHIFT, 0, &cursor);
+	return (adev->gmc.aper_base + cursor.start) >> PAGE_SHIFT;
 }
 
 /**
-- 
2.25.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/8] drm/amdgpu: use the new cursor in amdgpu_ttm_access_memory
  2021-03-08 13:40 [PATCH 1/8] drm/amdgpu: new resource cursor Christian König
                   ` (2 preceding siblings ...)
  2021-03-08 13:40 ` [PATCH 4/8] drm/amdgpu: use new cursor in amdgpu_ttm_io_mem_pfn Christian König
@ 2021-03-08 13:40 ` Christian König
  2021-03-19 21:23   ` Felix Kuehling
  2021-03-08 13:40 ` [PATCH 6/8] drm/amdgpu: use new cursor in amdgpu_mem_visible Christian König
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Christian König @ 2021-03-08 13:40 UTC (permalink / raw)
  To: Arunpravin.PaneerSelvam, amd-gfx

Separate the drm_mm_node walking from the actual handling.

Signed-off-by: Christian König <christian.koenig@amd.com>
Acked-by: Oak Zeng <Oak.Zeng@amd.com>
Tested-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 67 +++++++------------------
 1 file changed, 18 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 517611b709fa..2cbe4ace591f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -178,26 +178,6 @@ static int amdgpu_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 					  filp->private_data);
 }
 
-/**
- * amdgpu_find_mm_node - Helper function finds the drm_mm_node corresponding to
- * @offset. It also modifies the offset to be within the drm_mm_node returned
- *
- * @mem: The region where the bo resides.
- * @offset: The offset that drm_mm_node is used for finding.
- *
- */
-static struct drm_mm_node *amdgpu_find_mm_node(struct ttm_resource *mem,
-					       uint64_t *offset)
-{
-	struct drm_mm_node *mm_node = mem->mm_node;
-
-	while (*offset >= (mm_node->size << PAGE_SHIFT)) {
-		*offset -= (mm_node->size << PAGE_SHIFT);
-		++mm_node;
-	}
-	return mm_node;
-}
-
 /**
  * amdgpu_ttm_map_buffer - Map memory into the GART windows
  * @bo: buffer object to map
@@ -1478,41 +1458,36 @@ static bool amdgpu_ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
  * access for debugging purposes.
  */
 static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo,
-				    unsigned long offset,
-				    void *buf, int len, int write)
+				    unsigned long offset, void *buf, int len,
+				    int write)
 {
 	struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo);
 	struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev);
-	struct drm_mm_node *nodes;
+	struct amdgpu_res_cursor cursor;
+	unsigned long flags;
 	uint32_t value = 0;
 	int ret = 0;
-	uint64_t pos;
-	unsigned long flags;
 
 	if (bo->mem.mem_type != TTM_PL_VRAM)
 		return -EIO;
 
-	pos = offset;
-	nodes = amdgpu_find_mm_node(&abo->tbo.mem, &pos);
-	pos += (nodes->start << PAGE_SHIFT);
-
-	while (len && pos < adev->gmc.mc_vram_size) {
-		uint64_t aligned_pos = pos & ~(uint64_t)3;
-		uint64_t bytes = 4 - (pos & 3);
-		uint32_t shift = (pos & 3) * 8;
+	amdgpu_res_first(&bo->mem, offset, len, &cursor);
+	while (cursor.remaining) {
+		uint64_t aligned_pos = cursor.start & ~(uint64_t)3;
+		uint64_t bytes = 4 - (cursor.start & 3);
+		uint32_t shift = (cursor.start & 3) * 8;
 		uint32_t mask = 0xffffffff << shift;
 
-		if (len < bytes) {
-			mask &= 0xffffffff >> (bytes - len) * 8;
-			bytes = len;
+		if (cursor.size < bytes) {
+			mask &= 0xffffffff >> (bytes - cursor.size) * 8;
+			bytes = cursor.size;
 		}
 
 		if (mask != 0xffffffff) {
 			spin_lock_irqsave(&adev->mmio_idx_lock, flags);
 			WREG32_NO_KIQ(mmMM_INDEX, ((uint32_t)aligned_pos) | 0x80000000);
 			WREG32_NO_KIQ(mmMM_INDEX_HI, aligned_pos >> 31);
-			if (!write || mask != 0xffffffff)
-				value = RREG32_NO_KIQ(mmMM_DATA);
+			value = RREG32_NO_KIQ(mmMM_DATA);
 			if (write) {
 				value &= ~mask;
 				value |= (*(uint32_t *)buf << shift) & mask;
@@ -1524,21 +1499,15 @@ static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo,
 				memcpy(buf, &value, bytes);
 			}
 		} else {
-			bytes = (nodes->start + nodes->size) << PAGE_SHIFT;
-			bytes = min(bytes - pos, (uint64_t)len & ~0x3ull);
-
-			amdgpu_device_vram_access(adev, pos, (uint32_t *)buf,
-						  bytes, write);
+			bytes = cursor.size & 0x3ull;
+			amdgpu_device_vram_access(adev, cursor.start,
+						  (uint32_t *)buf, bytes,
+						  write);
 		}
 
 		ret += bytes;
 		buf = (uint8_t *)buf + bytes;
-		pos += bytes;
-		len -= bytes;
-		if (pos >= (nodes->start + nodes->size) << PAGE_SHIFT) {
-			++nodes;
-			pos = (nodes->start << PAGE_SHIFT);
-		}
+		amdgpu_res_next(&cursor, bytes);
 	}
 
 	return ret;
-- 
2.25.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 6/8] drm/amdgpu: use new cursor in amdgpu_mem_visible
  2021-03-08 13:40 [PATCH 1/8] drm/amdgpu: new resource cursor Christian König
                   ` (3 preceding siblings ...)
  2021-03-08 13:40 ` [PATCH 5/8] drm/amdgpu: use the new cursor in amdgpu_ttm_access_memory Christian König
@ 2021-03-08 13:40 ` Christian König
  2021-03-08 13:40 ` [PATCH 7/8] drm/amdgpu: use the new cursor in amdgpu_ttm_bo_eviction_valuable Christian König
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Christian König @ 2021-03-08 13:40 UTC (permalink / raw)
  To: Arunpravin.PaneerSelvam, amd-gfx

Separate the drm_mm_node walking from the actual handling.

Signed-off-by: Christian König <christian.koenig@amd.com>
Acked-by: Oak Zeng <Oak.Zeng@amd.com>
Tested-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 2cbe4ace591f..d469ba5fef2c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -441,7 +441,8 @@ static int amdgpu_move_blit(struct ttm_buffer_object *bo,
 static bool amdgpu_mem_visible(struct amdgpu_device *adev,
 			       struct ttm_resource *mem)
 {
-	struct drm_mm_node *nodes = mem->mm_node;
+	uint64_t mem_size = (u64)mem->num_pages << PAGE_SHIFT;
+	struct amdgpu_res_cursor cursor;
 
 	if (mem->mem_type == TTM_PL_SYSTEM ||
 	    mem->mem_type == TTM_PL_TT)
@@ -449,12 +450,13 @@ static bool amdgpu_mem_visible(struct amdgpu_device *adev,
 	if (mem->mem_type != TTM_PL_VRAM)
 		return false;
 
+	amdgpu_res_first(mem, 0, mem_size, &cursor);
+
 	/* ttm_resource_ioremap only supports contiguous memory */
-	if (nodes->size != mem->num_pages)
+	if (cursor.size != mem_size)
 		return false;
 
-	return ((nodes->start + nodes->size) << PAGE_SHIFT)
-		<= adev->gmc.visible_vram_size;
+	return cursor.start + cursor.size <= adev->gmc.visible_vram_size;
 }
 
 /*
-- 
2.25.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 7/8] drm/amdgpu: use the new cursor in amdgpu_ttm_bo_eviction_valuable
  2021-03-08 13:40 [PATCH 1/8] drm/amdgpu: new resource cursor Christian König
                   ` (4 preceding siblings ...)
  2021-03-08 13:40 ` [PATCH 6/8] drm/amdgpu: use new cursor in amdgpu_mem_visible Christian König
@ 2021-03-08 13:40 ` Christian König
  2021-03-08 13:40 ` [PATCH 8/8] drm/amdgpu: use the new cursor in the VM code Christian König
  2021-03-12 10:52 ` [PATCH 1/8] drm/amdgpu: new resource cursor Christian König
  7 siblings, 0 replies; 18+ messages in thread
From: Christian König @ 2021-03-08 13:40 UTC (permalink / raw)
  To: Arunpravin.PaneerSelvam, amd-gfx

Separate the drm_mm_node walking from the actual handling.

Signed-off-by: Christian König <christian.koenig@amd.com>
Acked-by: Oak Zeng <Oak.Zeng@amd.com>
Tested-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index d469ba5fef2c..5d88d1850781 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1398,7 +1398,7 @@ static bool amdgpu_ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
 					    const struct ttm_place *place)
 {
 	unsigned long num_pages = bo->mem.num_pages;
-	struct drm_mm_node *node = bo->mem.mm_node;
+	struct amdgpu_res_cursor cursor;
 	struct dma_resv_list *flist;
 	struct dma_fence *f;
 	int i;
@@ -1430,13 +1430,15 @@ static bool amdgpu_ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
 
 	case TTM_PL_VRAM:
 		/* Check each drm MM node individually */
-		while (num_pages) {
-			if (place->fpfn < (node->start + node->size) &&
-			    !(place->lpfn && place->lpfn <= node->start))
+		amdgpu_res_first(&bo->mem, 0, (u64)num_pages << PAGE_SHIFT,
+				 &cursor);
+		while (cursor.remaining) {
+			if (place->fpfn < PFN_DOWN(cursor.start + cursor.size)
+			    && !(place->lpfn &&
+				 place->lpfn <= PFN_DOWN(cursor.start)))
 				return true;
 
-			num_pages -= node->size;
-			++node;
+			amdgpu_res_next(&cursor, cursor.size);
 		}
 		return false;
 
-- 
2.25.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 8/8] drm/amdgpu: use the new cursor in the VM code
  2021-03-08 13:40 [PATCH 1/8] drm/amdgpu: new resource cursor Christian König
                   ` (5 preceding siblings ...)
  2021-03-08 13:40 ` [PATCH 7/8] drm/amdgpu: use the new cursor in amdgpu_ttm_bo_eviction_valuable Christian König
@ 2021-03-08 13:40 ` Christian König
  2021-03-12 10:52 ` [PATCH 1/8] drm/amdgpu: new resource cursor Christian König
  7 siblings, 0 replies; 18+ messages in thread
From: Christian König @ 2021-03-08 13:40 UTC (permalink / raw)
  To: Arunpravin.PaneerSelvam, amd-gfx

Separate the drm_mm_node walking from the actual handling.

Signed-off-by: Christian König <christian.koenig@amd.com>
Acked-by: Oak Zeng <Oak.Zeng@amd.com>
Tested-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 54 +++++++++-----------------
 1 file changed, 18 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 9d19078246c8..bf9638bd0ddd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -37,6 +37,7 @@
 #include "amdgpu_gmc.h"
 #include "amdgpu_xgmi.h"
 #include "amdgpu_dma_buf.h"
+#include "amdgpu_res_cursor.h"
 
 /**
  * DOC: GPUVM
@@ -1582,7 +1583,7 @@ static int amdgpu_vm_update_ptes(struct amdgpu_vm_update_params *params,
  * @last: last mapped entry
  * @flags: flags for the entries
  * @offset: offset into nodes and pages_addr
- * @nodes: array of drm_mm_nodes with the MC addresses
+ * @res: ttm_resource to map
  * @pages_addr: DMA addresses to use for mapping
  * @fence: optional resulting fence
  *
@@ -1597,13 +1598,13 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
 				       bool unlocked, struct dma_resv *resv,
 				       uint64_t start, uint64_t last,
 				       uint64_t flags, uint64_t offset,
-				       struct drm_mm_node *nodes,
+				       struct ttm_resource *res,
 				       dma_addr_t *pages_addr,
 				       struct dma_fence **fence)
 {
 	struct amdgpu_vm_update_params params;
+	struct amdgpu_res_cursor cursor;
 	enum amdgpu_sync_mode sync_mode;
-	uint64_t pfn;
 	int r;
 
 	memset(&params, 0, sizeof(params));
@@ -1621,14 +1622,6 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
 	else
 		sync_mode = AMDGPU_SYNC_EXPLICIT;
 
-	pfn = offset >> PAGE_SHIFT;
-	if (nodes) {
-		while (pfn >= nodes->size) {
-			pfn -= nodes->size;
-			++nodes;
-		}
-	}
-
 	amdgpu_vm_eviction_lock(vm);
 	if (vm->evicting) {
 		r = -EBUSY;
@@ -1647,23 +1640,17 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
 	if (r)
 		goto error_unlock;
 
-	do {
+	amdgpu_res_first(res, offset, (last - start + 1) * AMDGPU_GPU_PAGE_SIZE,
+			 &cursor);
+	while (cursor.remaining) {
 		uint64_t tmp, num_entries, addr;
 
-
-		num_entries = last - start + 1;
-		if (nodes) {
-			addr = nodes->start << PAGE_SHIFT;
-			num_entries = min((nodes->size - pfn) *
-				AMDGPU_GPU_PAGES_IN_CPU_PAGE, num_entries);
-		} else {
-			addr = 0;
-		}
-
+		num_entries = cursor.size >> AMDGPU_GPU_PAGE_SHIFT;
 		if (pages_addr) {
 			bool contiguous = true;
 
 			if (num_entries > AMDGPU_GPU_PAGES_IN_CPU_PAGE) {
+				uint64_t pfn = cursor.start >> PAGE_SHIFT;
 				uint64_t count;
 
 				contiguous = pages_addr[pfn + 1] ==
@@ -1683,16 +1670,18 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
 			}
 
 			if (!contiguous) {
-				addr = pfn << PAGE_SHIFT;
+				addr = cursor.start;
 				params.pages_addr = pages_addr;
 			} else {
-				addr = pages_addr[pfn];
+				addr = pages_addr[cursor.start >> PAGE_SHIFT];
 				params.pages_addr = NULL;
 			}
 
 		} else if (flags & (AMDGPU_PTE_VALID | AMDGPU_PTE_PRT)) {
-			addr += bo_adev->vm_manager.vram_base_offset;
-			addr += pfn << PAGE_SHIFT;
+			addr = bo_adev->vm_manager.vram_base_offset +
+				cursor.start;
+		} else {
+			addr = 0;
 		}
 
 		tmp = start + num_entries;
@@ -1700,14 +1689,9 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
 		if (r)
 			goto error_unlock;
 
-		pfn += num_entries / AMDGPU_GPU_PAGES_IN_CPU_PAGE;
-		if (nodes && nodes->size == pfn) {
-			pfn = 0;
-			++nodes;
-		}
+		amdgpu_res_next(&cursor, num_entries * AMDGPU_GPU_PAGE_SIZE);
 		start = tmp;
-
-	} while (unlikely(start != last + 1));
+	};
 
 	r = vm->update_funcs->commit(&params, fence);
 
@@ -1736,7 +1720,6 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
 	struct amdgpu_bo_va_mapping *mapping;
 	dma_addr_t *pages_addr = NULL;
 	struct ttm_resource *mem;
-	struct drm_mm_node *nodes;
 	struct dma_fence **last_update;
 	struct dma_resv *resv;
 	uint64_t flags;
@@ -1745,7 +1728,6 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
 
 	if (clear || !bo) {
 		mem = NULL;
-		nodes = NULL;
 		resv = vm->root.base.bo->tbo.base.resv;
 	} else {
 		struct drm_gem_object *obj = &bo->tbo.base;
@@ -1809,7 +1791,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
 		r = amdgpu_vm_bo_update_mapping(adev, bo_adev, vm, false, false,
 						resv, mapping->start,
 						mapping->last, update_flags,
-						mapping->offset, nodes,
+						mapping->offset, mem,
 						pages_addr, last_update);
 		if (r)
 			return r;
-- 
2.25.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/8] drm/amdgpu: new resource cursor
  2021-03-08 13:40 [PATCH 1/8] drm/amdgpu: new resource cursor Christian König
                   ` (6 preceding siblings ...)
  2021-03-08 13:40 ` [PATCH 8/8] drm/amdgpu: use the new cursor in the VM code Christian König
@ 2021-03-12 10:52 ` Christian König
  2021-03-12 11:11   ` Paneer Selvam, Arunpravin
  7 siblings, 1 reply; 18+ messages in thread
From: Christian König @ 2021-03-12 10:52 UTC (permalink / raw)
  To: Arunpravin.PaneerSelvam, amd-gfx

Any more comments on this set here or otherwise I'm going to push it 
with just Oaks ack.

Thanks,
Christian.

Am 08.03.21 um 14:40 schrieb Christian König:
> Allows to walk over the drm_mm nodes in a TTM resource object.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Acked-by: Oak Zeng <Oak.Zeng@amd.com>
> Tested-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   .../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h    | 105 ++++++++++++++++++
>   1 file changed, 105 insertions(+)
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> new file mode 100644
> index 000000000000..1335e098510f
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> @@ -0,0 +1,105 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * Copyright 2020 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * Authors: Christian König
> + */
> +
> +#ifndef __AMDGPU_RES_CURSOR_H__
> +#define __AMDGPU_RES_CURSOR_H__
> +
> +#include <drm/drm_mm.h>
> +#include <drm/ttm/ttm_resource.h>
> +
> +/* state back for walking over vram_mgr and gtt_mgr allocations */
> +struct amdgpu_res_cursor {
> +	uint64_t		start;
> +	uint64_t		size;
> +	uint64_t		remaining;
> +	struct drm_mm_node	*node;
> +};
> +
> +/**
> + * amdgpu_res_first - initialize a amdgpu_res_cursor
> + *
> + * @res: TTM resource object to walk
> + * @start: Start of the range
> + * @size: Size of the range
> + * @cur: cursor object to initialize
> + *
> + * Start walking over the range of allocations between @start and @size.
> + */
> +static inline void amdgpu_res_first(struct ttm_resource *res,
> +				    uint64_t start, uint64_t size,
> +				    struct amdgpu_res_cursor *cur)
> +{
> +	struct drm_mm_node *node;
> +
> +	if (!res || !res->mm_node) {
> +		cur->start = start;
> +		cur->size = size;
> +		cur->remaining = size;
> +		cur->node = NULL;
> +		return;
> +	}
> +
> +	BUG_ON(start + size > res->num_pages << PAGE_SHIFT);
> +
> +	node = res->mm_node;
> +	while (start > node->size << PAGE_SHIFT)
> +		start -= node++->size << PAGE_SHIFT;
> +
> +	cur->start = (node->start << PAGE_SHIFT) + start;
> +	cur->size = (node->size << PAGE_SHIFT) - start;
> +	cur->remaining = size;
> +	cur->node = node;
> +}
> +
> +/**
> + * amdgpu_res_next - advance the cursor
> + *
> + * @cur: the cursor to advance
> + * @size: number of bytes to move forward
> + *
> + * Move the cursor @size bytes forwrad, walking to the next node if necessary.
> + */
> +static inline void amdgpu_res_next(struct amdgpu_res_cursor *cur, uint64_t size)
> +{
> +	struct drm_mm_node *node = cur->node;
> +
> +	BUG_ON(size > cur->remaining);
> +
> +	cur->remaining -= size;
> +	if (!cur->remaining)
> +		return;
> +
> +	cur->size -= size;
> +	if (cur->size) {
> +		cur->start += size;
> +		return;
> +	}
> +
> +	cur->node = ++node;
> +	cur->start = node->start << PAGE_SHIFT;
> +	cur->size = min(node->size << PAGE_SHIFT, cur->remaining);
> +}
> +
> +#endif

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 1/8] drm/amdgpu: new resource cursor
  2021-03-12 10:52 ` [PATCH 1/8] drm/amdgpu: new resource cursor Christian König
@ 2021-03-12 11:11   ` Paneer Selvam, Arunpravin
  2021-03-12 12:16     ` Christian König
  0 siblings, 1 reply; 18+ messages in thread
From: Paneer Selvam, Arunpravin @ 2021-03-12 11:11 UTC (permalink / raw)
  To: Christian König, amd-gfx

[AMD Public Use]

Hi Christian,
Reviewed the changes, it looks good to me.

Reviewed-by: Arunpravin <Arunpravin.PaneerSelvam@amd.com>

Thanks,
Arun
-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@gmail.com> 
Sent: Friday, March 12, 2021 4:22 PM
To: Paneer Selvam, Arunpravin <Arunpravin.PaneerSelvam@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/8] drm/amdgpu: new resource cursor

Any more comments on this set here or otherwise I'm going to push it with just Oaks ack.

Thanks,
Christian.

Am 08.03.21 um 14:40 schrieb Christian König:
> Allows to walk over the drm_mm nodes in a TTM resource object.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Acked-by: Oak Zeng <Oak.Zeng@amd.com>
> Tested-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   .../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h    | 105 ++++++++++++++++++
>   1 file changed, 105 insertions(+)
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> new file mode 100644
> index 000000000000..1335e098510f
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> @@ -0,0 +1,105 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * Copyright 2020 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person 
> +obtaining a
> + * copy of this software and associated documentation files (the 
> +"Software"),
> + * to deal in the Software without restriction, including without 
> +limitation
> + * the rights to use, copy, modify, merge, publish, distribute, 
> +sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom 
> +the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be 
> +included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
> +SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, 
> +DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR 
> +OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE 
> +OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * Authors: Christian König
> + */
> +
> +#ifndef __AMDGPU_RES_CURSOR_H__
> +#define __AMDGPU_RES_CURSOR_H__
> +
> +#include <drm/drm_mm.h>
> +#include <drm/ttm/ttm_resource.h>
> +
> +/* state back for walking over vram_mgr and gtt_mgr allocations */ 
> +struct amdgpu_res_cursor {
> +	uint64_t		start;
> +	uint64_t		size;
> +	uint64_t		remaining;
> +	struct drm_mm_node	*node;
> +};
> +
> +/**
> + * amdgpu_res_first - initialize a amdgpu_res_cursor
> + *
> + * @res: TTM resource object to walk
> + * @start: Start of the range
> + * @size: Size of the range
> + * @cur: cursor object to initialize
> + *
> + * Start walking over the range of allocations between @start and @size.
> + */
> +static inline void amdgpu_res_first(struct ttm_resource *res,
> +				    uint64_t start, uint64_t size,
> +				    struct amdgpu_res_cursor *cur) {
> +	struct drm_mm_node *node;
> +
> +	if (!res || !res->mm_node) {
> +		cur->start = start;
> +		cur->size = size;
> +		cur->remaining = size;
> +		cur->node = NULL;
> +		return;
> +	}
> +
> +	BUG_ON(start + size > res->num_pages << PAGE_SHIFT);
> +
> +	node = res->mm_node;
> +	while (start > node->size << PAGE_SHIFT)
> +		start -= node++->size << PAGE_SHIFT;
> +
> +	cur->start = (node->start << PAGE_SHIFT) + start;
> +	cur->size = (node->size << PAGE_SHIFT) - start;
> +	cur->remaining = size;
> +	cur->node = node;
> +}
> +
> +/**
> + * amdgpu_res_next - advance the cursor
> + *
> + * @cur: the cursor to advance
> + * @size: number of bytes to move forward
> + *
> + * Move the cursor @size bytes forwrad, walking to the next node if necessary.
> + */
> +static inline void amdgpu_res_next(struct amdgpu_res_cursor *cur, 
> +uint64_t size) {
> +	struct drm_mm_node *node = cur->node;
> +
> +	BUG_ON(size > cur->remaining);
> +
> +	cur->remaining -= size;
> +	if (!cur->remaining)
> +		return;
> +
> +	cur->size -= size;
> +	if (cur->size) {
> +		cur->start += size;
> +		return;
> +	}
> +
> +	cur->node = ++node;
> +	cur->start = node->start << PAGE_SHIFT;
> +	cur->size = min(node->size << PAGE_SHIFT, cur->remaining); }
> +
> +#endif
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/8] drm/amdgpu: new resource cursor
  2021-03-12 11:11   ` Paneer Selvam, Arunpravin
@ 2021-03-12 12:16     ` Christian König
  0 siblings, 0 replies; 18+ messages in thread
From: Christian König @ 2021-03-12 12:16 UTC (permalink / raw)
  To: Paneer Selvam, Arunpravin, amd-gfx

Thanks! Going to push that stuff now.

Christian.

Am 12.03.21 um 12:11 schrieb Paneer Selvam, Arunpravin:
> [AMD Public Use]
>
> Hi Christian,
> Reviewed the changes, it looks good to me.
>
> Reviewed-by: Arunpravin <Arunpravin.PaneerSelvam@amd.com>
>
> Thanks,
> Arun
> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@gmail.com>
> Sent: Friday, March 12, 2021 4:22 PM
> To: Paneer Selvam, Arunpravin <Arunpravin.PaneerSelvam@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 1/8] drm/amdgpu: new resource cursor
>
> Any more comments on this set here or otherwise I'm going to push it with just Oaks ack.
>
> Thanks,
> Christian.
>
> Am 08.03.21 um 14:40 schrieb Christian König:
>> Allows to walk over the drm_mm nodes in a TTM resource object.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> Acked-by: Oak Zeng <Oak.Zeng@amd.com>
>> Tested-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>>    .../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h    | 105 ++++++++++++++++++
>>    1 file changed, 105 insertions(+)
>>    create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
>> new file mode 100644
>> index 000000000000..1335e098510f
>> --- /dev/null
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
>> @@ -0,0 +1,105 @@
>> +// SPDX-License-Identifier: GPL-2.0 OR MIT
>> +/*
>> + * Copyright 2020 Advanced Micro Devices, Inc.
>> + *
>> + * Permission is hereby granted, free of charge, to any person
>> +obtaining a
>> + * copy of this software and associated documentation files (the
>> +"Software"),
>> + * to deal in the Software without restriction, including without
>> +limitation
>> + * the rights to use, copy, modify, merge, publish, distribute,
>> +sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom
>> +the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be
>> +included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> +EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> +MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT
>> +SHALL
>> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM,
>> +DAMAGES OR
>> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>> +OTHERWISE,
>> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE
>> +OR
>> + * OTHER DEALINGS IN THE SOFTWARE.
>> + *
>> + * Authors: Christian König
>> + */
>> +
>> +#ifndef __AMDGPU_RES_CURSOR_H__
>> +#define __AMDGPU_RES_CURSOR_H__
>> +
>> +#include <drm/drm_mm.h>
>> +#include <drm/ttm/ttm_resource.h>
>> +
>> +/* state back for walking over vram_mgr and gtt_mgr allocations */
>> +struct amdgpu_res_cursor {
>> +	uint64_t		start;
>> +	uint64_t		size;
>> +	uint64_t		remaining;
>> +	struct drm_mm_node	*node;
>> +};
>> +
>> +/**
>> + * amdgpu_res_first - initialize a amdgpu_res_cursor
>> + *
>> + * @res: TTM resource object to walk
>> + * @start: Start of the range
>> + * @size: Size of the range
>> + * @cur: cursor object to initialize
>> + *
>> + * Start walking over the range of allocations between @start and @size.
>> + */
>> +static inline void amdgpu_res_first(struct ttm_resource *res,
>> +				    uint64_t start, uint64_t size,
>> +				    struct amdgpu_res_cursor *cur) {
>> +	struct drm_mm_node *node;
>> +
>> +	if (!res || !res->mm_node) {
>> +		cur->start = start;
>> +		cur->size = size;
>> +		cur->remaining = size;
>> +		cur->node = NULL;
>> +		return;
>> +	}
>> +
>> +	BUG_ON(start + size > res->num_pages << PAGE_SHIFT);
>> +
>> +	node = res->mm_node;
>> +	while (start > node->size << PAGE_SHIFT)
>> +		start -= node++->size << PAGE_SHIFT;
>> +
>> +	cur->start = (node->start << PAGE_SHIFT) + start;
>> +	cur->size = (node->size << PAGE_SHIFT) - start;
>> +	cur->remaining = size;
>> +	cur->node = node;
>> +}
>> +
>> +/**
>> + * amdgpu_res_next - advance the cursor
>> + *
>> + * @cur: the cursor to advance
>> + * @size: number of bytes to move forward
>> + *
>> + * Move the cursor @size bytes forwrad, walking to the next node if necessary.
>> + */
>> +static inline void amdgpu_res_next(struct amdgpu_res_cursor *cur,
>> +uint64_t size) {
>> +	struct drm_mm_node *node = cur->node;
>> +
>> +	BUG_ON(size > cur->remaining);
>> +
>> +	cur->remaining -= size;
>> +	if (!cur->remaining)
>> +		return;
>> +
>> +	cur->size -= size;
>> +	if (cur->size) {
>> +		cur->start += size;
>> +		return;
>> +	}
>> +
>> +	cur->node = ++node;
>> +	cur->start = node->start << PAGE_SHIFT;
>> +	cur->size = min(node->size << PAGE_SHIFT, cur->remaining); }
>> +
>> +#endif

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/8] drm/amdgpu: use the new cursor in amdgpu_ttm_access_memory
  2021-03-08 13:40 ` [PATCH 5/8] drm/amdgpu: use the new cursor in amdgpu_ttm_access_memory Christian König
@ 2021-03-19 21:23   ` Felix Kuehling
  2021-03-20  9:08     ` Christian König
  2021-03-22  1:19     ` Pan, Xinhui
  0 siblings, 2 replies; 18+ messages in thread
From: Felix Kuehling @ 2021-03-19 21:23 UTC (permalink / raw)
  To: Christian König, Arunpravin.PaneerSelvam, amd-gfx

This is causing a deadlock in amdgpu_ttm_access_memory during the 
PtraceAccess test in kfdtest. Unfortunately it doesn't get flagged by 
LOCKDEP. See the kernel log snippet below. I don't have a good 
explanation what's going on other than maybe some data structure corruption.

With this patch reverted the PtraceAccess test still fails, but it 
doesn't hang any more. If I also revert "use new cursor in 
amdgpu_ttm_io_mem_pfn" (which is used via amdgpu_find_mm_node in 
amdgpu_ttm_access_memory), Ptrace access starts working correctly. That 
tells me that there is some fundamental bug in the resource cursor 
implementation that's breaking several users.

Regards,
   Felix


[  129.446085] watchdog: BUG: soft lockup - CPU#8 stuck for 22s! [kfdtest:3588]
[  129.455379] Modules linked in: ip6table_filter ip6_tables iptable_filter amdgpu x86_pkg_temp_thermal drm_ttm_helper ttm iommu_v2 gpu_sched ip_tables x_tables
[  129.455428] irq event stamp: 75294000
[  129.455432] hardirqs last  enabled at (75293999): [<ffffffffa9dcfd7d>] _raw_spin_unlock_irqrestore+0x2d/0x40
[  129.455447] hardirqs last disabled at (75294000): [<ffffffffa9dbef8a>] sysvec_apic_timer_interrupt+0xa/0xa0
[  129.455457] softirqs last  enabled at (75184000): [<ffffffffaa000306>] __do_softirq+0x306/0x429
[  129.455467] softirqs last disabled at (75183995): [<ffffffffa9e00f8f>] asm_call_irq_on_stack+0xf/0x20
[  129.455477] CPU: 8 PID: 3588 Comm: kfdtest Not tainted 5.11.0-kfd-fkuehlin #194
[  129.455485] Hardware name: ASUS All Series/X99-E WS/USB 3.1, BIOS 3201 06/17/2016
[  129.455490] RIP: 0010:_raw_spin_lock_irqsave+0xb/0x50
[  129.455498] Code: d2 31 f6 e8 e7 e9 31 ff 48 89 df 58 5b e9 7d 32 32 ff 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 55 48 89 fd 53 9c <5b> fa f6 c7 02 74 05 e8 59 06 3e ff 65 ff 05 92 73 24 56 48 8d 7d
[  129.455505] RSP: 0018:ffffa3eb407f3c58 EFLAGS: 00000246
[  129.455513] RAX: ffff96466e2010a0 RBX: ffff96466e200000 RCX: 0000000000000000
[  129.455519] RDX: ffffa3eb407f3e70 RSI: 00000000019ffff0 RDI: ffff96466e2010a0
[  129.455524] RBP: ffff96466e2010a0 R08: 0000000000000000 R09: 0000000000000001
[  129.455528] R10: ffffa3eb407f3c60 R11: ffff96466e2010b8 R12: 00000000019ffff0
[  129.455533] R13: 00000000019ffff0 R14: ffffa3eb407f3e70 R15: 00000000019ffff0
[  129.455538] FS:  00007f5aad0f0740(0000) GS:ffff96467fc00000(0000) knlGS:0000000000000000
[  129.455544] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  129.455549] CR2: 0000563ea76ad0f0 CR3: 00000007c6e92005 CR4: 00000000001706e0
[  129.455554] Call Trace:
[  129.455563]  amdgpu_device_vram_access+0xc1/0x200 [amdgpu]
[  129.455820]  ? _raw_spin_unlock_irqrestore+0x2d/0x40
[  129.455834]  amdgpu_ttm_access_memory+0x29e/0x320 [amdgpu]
[  129.456063]  ttm_bo_vm_access+0x1c8/0x3a0 [ttm]
[  129.456089]  __access_remote_vm+0x289/0x390
[  129.456112]  ptrace_access_vm+0x98/0xc0
[  129.456127]  generic_ptrace_peekdata+0x31/0x80
[  129.456138]  ptrace_request+0x13b/0x5d0
[  129.456155]  arch_ptrace+0x24f/0x2f0
[  129.456165]  __x64_sys_ptrace+0xc9/0x140
[  129.456177]  do_syscall_64+0x2d/0x40
[  129.456185]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  129.456194] RIP: 0033:0x7f5aab861a3f
[  129.456199] Code: 48 89 44 24 18 48 8d 44 24 30 c7 44 24 10 18 00 00 00 8b 70 08 48 8b 50 10 48 89 44 24 20 4c 0f 43 50 18 b8 65 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 41 48 85 c0 78 06 41 83 f8 02 76 1e 48 8b 4c
[  129.456205] RSP: 002b:00007ffd27b68750 EFLAGS: 00000293 ORIG_RAX: 0000000000000065
[  129.456214] RAX: ffffffffffffffda RBX: 0000000000000001 RCX: 00007f5aab861a3f
[  129.456219] RDX: 00007f5aab3ffff0 RSI: 0000000000000dfa RDI: 0000000000000002
[  129.456224] RBP: 00007ffd27b68870 R08: 0000000000000001 R09: 0000000000000000
[  129.456228] R10: 00007ffd27b68758 R11: 0000000000000293 R12: 0000563ea764e2aa
[  129.456233] R13: 0000000000000000 R14: 0000000000000021 R15: 0000000000000000

On 2021-03-08 8:40 a.m., Christian König wrote:
> Separate the drm_mm_node walking from the actual handling.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Acked-by: Oak Zeng <Oak.Zeng@amd.com>
> Tested-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 67 +++++++------------------
>   1 file changed, 18 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 517611b709fa..2cbe4ace591f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -178,26 +178,6 @@ static int amdgpu_verify_access(struct ttm_buffer_object *bo, struct file *filp)
>   					  filp->private_data);
>   }
>   
> -/**
> - * amdgpu_find_mm_node - Helper function finds the drm_mm_node corresponding to
> - * @offset. It also modifies the offset to be within the drm_mm_node returned
> - *
> - * @mem: The region where the bo resides.
> - * @offset: The offset that drm_mm_node is used for finding.
> - *
> - */
> -static struct drm_mm_node *amdgpu_find_mm_node(struct ttm_resource *mem,
> -					       uint64_t *offset)
> -{
> -	struct drm_mm_node *mm_node = mem->mm_node;
> -
> -	while (*offset >= (mm_node->size << PAGE_SHIFT)) {
> -		*offset -= (mm_node->size << PAGE_SHIFT);
> -		++mm_node;
> -	}
> -	return mm_node;
> -}
> -
>   /**
>    * amdgpu_ttm_map_buffer - Map memory into the GART windows
>    * @bo: buffer object to map
> @@ -1478,41 +1458,36 @@ static bool amdgpu_ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
>    * access for debugging purposes.
>    */
>   static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo,
> -				    unsigned long offset,
> -				    void *buf, int len, int write)
> +				    unsigned long offset, void *buf, int len,
> +				    int write)
>   {
>   	struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo);
>   	struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev);
> -	struct drm_mm_node *nodes;
> +	struct amdgpu_res_cursor cursor;
> +	unsigned long flags;
>   	uint32_t value = 0;
>   	int ret = 0;
> -	uint64_t pos;
> -	unsigned long flags;
>   
>   	if (bo->mem.mem_type != TTM_PL_VRAM)
>   		return -EIO;
>   
> -	pos = offset;
> -	nodes = amdgpu_find_mm_node(&abo->tbo.mem, &pos);
> -	pos += (nodes->start << PAGE_SHIFT);
> -
> -	while (len && pos < adev->gmc.mc_vram_size) {
> -		uint64_t aligned_pos = pos & ~(uint64_t)3;
> -		uint64_t bytes = 4 - (pos & 3);
> -		uint32_t shift = (pos & 3) * 8;
> +	amdgpu_res_first(&bo->mem, offset, len, &cursor);
> +	while (cursor.remaining) {
> +		uint64_t aligned_pos = cursor.start & ~(uint64_t)3;
> +		uint64_t bytes = 4 - (cursor.start & 3);
> +		uint32_t shift = (cursor.start & 3) * 8;
>   		uint32_t mask = 0xffffffff << shift;
>   
> -		if (len < bytes) {
> -			mask &= 0xffffffff >> (bytes - len) * 8;
> -			bytes = len;
> +		if (cursor.size < bytes) {
> +			mask &= 0xffffffff >> (bytes - cursor.size) * 8;
> +			bytes = cursor.size;
>   		}
>   
>   		if (mask != 0xffffffff) {
>   			spin_lock_irqsave(&adev->mmio_idx_lock, flags);
>   			WREG32_NO_KIQ(mmMM_INDEX, ((uint32_t)aligned_pos) | 0x80000000);
>   			WREG32_NO_KIQ(mmMM_INDEX_HI, aligned_pos >> 31);
> -			if (!write || mask != 0xffffffff)
> -				value = RREG32_NO_KIQ(mmMM_DATA);
> +			value = RREG32_NO_KIQ(mmMM_DATA);
>   			if (write) {
>   				value &= ~mask;
>   				value |= (*(uint32_t *)buf << shift) & mask;
> @@ -1524,21 +1499,15 @@ static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo,
>   				memcpy(buf, &value, bytes);
>   			}
>   		} else {
> -			bytes = (nodes->start + nodes->size) << PAGE_SHIFT;
> -			bytes = min(bytes - pos, (uint64_t)len & ~0x3ull);
> -
> -			amdgpu_device_vram_access(adev, pos, (uint32_t *)buf,
> -						  bytes, write);
> +			bytes = cursor.size & 0x3ull;
> +			amdgpu_device_vram_access(adev, cursor.start,
> +						  (uint32_t *)buf, bytes,
> +						  write);
>   		}
>   
>   		ret += bytes;
>   		buf = (uint8_t *)buf + bytes;
> -		pos += bytes;
> -		len -= bytes;
> -		if (pos >= (nodes->start + nodes->size) << PAGE_SHIFT) {
> -			++nodes;
> -			pos = (nodes->start << PAGE_SHIFT);
> -		}
> +		amdgpu_res_next(&cursor, bytes);
>   	}
>   
>   	return ret;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/8] drm/amdgpu: use the new cursor in amdgpu_ttm_access_memory
  2021-03-19 21:23   ` Felix Kuehling
@ 2021-03-20  9:08     ` Christian König
  2021-03-22  0:47       ` Pan, Xinhui
  2021-03-22  1:19     ` Pan, Xinhui
  1 sibling, 1 reply; 18+ messages in thread
From: Christian König @ 2021-03-20  9:08 UTC (permalink / raw)
  To: Felix Kuehling, Arunpravin.PaneerSelvam, amd-gfx

Yeah, Nirmoy already stumbled over the correct fix.

The size check is off by one. Patch to fix this should be pushed on Monday.

Regards,
Christian.

Am 19.03.21 um 22:23 schrieb Felix Kuehling:
> This is causing a deadlock in amdgpu_ttm_access_memory during the 
> PtraceAccess test in kfdtest. Unfortunately it doesn't get flagged by 
> LOCKDEP. See the kernel log snippet below. I don't have a good 
> explanation what's going on other than maybe some data structure 
> corruption.
>
> With this patch reverted the PtraceAccess test still fails, but it 
> doesn't hang any more. If I also revert "use new cursor in 
> amdgpu_ttm_io_mem_pfn" (which is used via amdgpu_find_mm_node in 
> amdgpu_ttm_access_memory), Ptrace access starts working correctly. 
> That tells me that there is some fundamental bug in the resource 
> cursor implementation that's breaking several users.
>
> Regards,
>   Felix
>
>
> [  129.446085] watchdog: BUG: soft lockup - CPU#8 stuck for 22s! 
> [kfdtest:3588]
> [  129.455379] Modules linked in: ip6table_filter ip6_tables 
> iptable_filter amdgpu x86_pkg_temp_thermal drm_ttm_helper ttm iommu_v2 
> gpu_sched ip_tables x_tables
> [  129.455428] irq event stamp: 75294000
> [  129.455432] hardirqs last  enabled at (75293999): 
> [<ffffffffa9dcfd7d>] _raw_spin_unlock_irqrestore+0x2d/0x40
> [  129.455447] hardirqs last disabled at (75294000): 
> [<ffffffffa9dbef8a>] sysvec_apic_timer_interrupt+0xa/0xa0
> [  129.455457] softirqs last  enabled at (75184000): 
> [<ffffffffaa000306>] __do_softirq+0x306/0x429
> [  129.455467] softirqs last disabled at (75183995): 
> [<ffffffffa9e00f8f>] asm_call_irq_on_stack+0xf/0x20
> [  129.455477] CPU: 8 PID: 3588 Comm: kfdtest Not tainted 
> 5.11.0-kfd-fkuehlin #194
> [  129.455485] Hardware name: ASUS All Series/X99-E WS/USB 3.1, BIOS 
> 3201 06/17/2016
> [  129.455490] RIP: 0010:_raw_spin_lock_irqsave+0xb/0x50
> [  129.455498] Code: d2 31 f6 e8 e7 e9 31 ff 48 89 df 58 5b e9 7d 32 
> 32 ff 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 55 48 89 
> fd 53 9c <5b> fa f6 c7 02 74 05 e8 59 06 3e ff 65 ff 05 92 73 24 56 48 
> 8d 7d
> [  129.455505] RSP: 0018:ffffa3eb407f3c58 EFLAGS: 00000246
> [  129.455513] RAX: ffff96466e2010a0 RBX: ffff96466e200000 RCX: 
> 0000000000000000
> [  129.455519] RDX: ffffa3eb407f3e70 RSI: 00000000019ffff0 RDI: 
> ffff96466e2010a0
> [  129.455524] RBP: ffff96466e2010a0 R08: 0000000000000000 R09: 
> 0000000000000001
> [  129.455528] R10: ffffa3eb407f3c60 R11: ffff96466e2010b8 R12: 
> 00000000019ffff0
> [  129.455533] R13: 00000000019ffff0 R14: ffffa3eb407f3e70 R15: 
> 00000000019ffff0
> [  129.455538] FS:  00007f5aad0f0740(0000) GS:ffff96467fc00000(0000) 
> knlGS:0000000000000000
> [  129.455544] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  129.455549] CR2: 0000563ea76ad0f0 CR3: 00000007c6e92005 CR4: 
> 00000000001706e0
> [  129.455554] Call Trace:
> [  129.455563]  amdgpu_device_vram_access+0xc1/0x200 [amdgpu]
> [  129.455820]  ? _raw_spin_unlock_irqrestore+0x2d/0x40
> [  129.455834]  amdgpu_ttm_access_memory+0x29e/0x320 [amdgpu]
> [  129.456063]  ttm_bo_vm_access+0x1c8/0x3a0 [ttm]
> [  129.456089]  __access_remote_vm+0x289/0x390
> [  129.456112]  ptrace_access_vm+0x98/0xc0
> [  129.456127]  generic_ptrace_peekdata+0x31/0x80
> [  129.456138]  ptrace_request+0x13b/0x5d0
> [  129.456155]  arch_ptrace+0x24f/0x2f0
> [  129.456165]  __x64_sys_ptrace+0xc9/0x140
> [  129.456177]  do_syscall_64+0x2d/0x40
> [  129.456185]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [  129.456194] RIP: 0033:0x7f5aab861a3f
> [  129.456199] Code: 48 89 44 24 18 48 8d 44 24 30 c7 44 24 10 18 00 
> 00 00 8b 70 08 48 8b 50 10 48 89 44 24 20 4c 0f 43 50 18 b8 65 00 00 
> 00 0f 05 <48> 3d 00 f0 ff ff 77 41 48 85 c0 78 06 41 83 f8 02 76 1e 48 
> 8b 4c
> [  129.456205] RSP: 002b:00007ffd27b68750 EFLAGS: 00000293 ORIG_RAX: 
> 0000000000000065
> [  129.456214] RAX: ffffffffffffffda RBX: 0000000000000001 RCX: 
> 00007f5aab861a3f
> [  129.456219] RDX: 00007f5aab3ffff0 RSI: 0000000000000dfa RDI: 
> 0000000000000002
> [  129.456224] RBP: 00007ffd27b68870 R08: 0000000000000001 R09: 
> 0000000000000000
> [  129.456228] R10: 00007ffd27b68758 R11: 0000000000000293 R12: 
> 0000563ea764e2aa
> [  129.456233] R13: 0000000000000000 R14: 0000000000000021 R15: 
> 0000000000000000
>
> On 2021-03-08 8:40 a.m., Christian König wrote:
>> Separate the drm_mm_node walking from the actual handling.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> Acked-by: Oak Zeng <Oak.Zeng@amd.com>
>> Tested-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 67 +++++++------------------
>>   1 file changed, 18 insertions(+), 49 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index 517611b709fa..2cbe4ace591f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -178,26 +178,6 @@ static int amdgpu_verify_access(struct 
>> ttm_buffer_object *bo, struct file *filp)
>>                         filp->private_data);
>>   }
>>   -/**
>> - * amdgpu_find_mm_node - Helper function finds the drm_mm_node 
>> corresponding to
>> - * @offset. It also modifies the offset to be within the drm_mm_node 
>> returned
>> - *
>> - * @mem: The region where the bo resides.
>> - * @offset: The offset that drm_mm_node is used for finding.
>> - *
>> - */
>> -static struct drm_mm_node *amdgpu_find_mm_node(struct ttm_resource 
>> *mem,
>> -                           uint64_t *offset)
>> -{
>> -    struct drm_mm_node *mm_node = mem->mm_node;
>> -
>> -    while (*offset >= (mm_node->size << PAGE_SHIFT)) {
>> -        *offset -= (mm_node->size << PAGE_SHIFT);
>> -        ++mm_node;
>> -    }
>> -    return mm_node;
>> -}
>> -
>>   /**
>>    * amdgpu_ttm_map_buffer - Map memory into the GART windows
>>    * @bo: buffer object to map
>> @@ -1478,41 +1458,36 @@ static bool 
>> amdgpu_ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
>>    * access for debugging purposes.
>>    */
>>   static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo,
>> -                    unsigned long offset,
>> -                    void *buf, int len, int write)
>> +                    unsigned long offset, void *buf, int len,
>> +                    int write)
>>   {
>>       struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo);
>>       struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev);
>> -    struct drm_mm_node *nodes;
>> +    struct amdgpu_res_cursor cursor;
>> +    unsigned long flags;
>>       uint32_t value = 0;
>>       int ret = 0;
>> -    uint64_t pos;
>> -    unsigned long flags;
>>         if (bo->mem.mem_type != TTM_PL_VRAM)
>>           return -EIO;
>>   -    pos = offset;
>> -    nodes = amdgpu_find_mm_node(&abo->tbo.mem, &pos);
>> -    pos += (nodes->start << PAGE_SHIFT);
>> -
>> -    while (len && pos < adev->gmc.mc_vram_size) {
>> -        uint64_t aligned_pos = pos & ~(uint64_t)3;
>> -        uint64_t bytes = 4 - (pos & 3);
>> -        uint32_t shift = (pos & 3) * 8;
>> +    amdgpu_res_first(&bo->mem, offset, len, &cursor);
>> +    while (cursor.remaining) {
>> +        uint64_t aligned_pos = cursor.start & ~(uint64_t)3;
>> +        uint64_t bytes = 4 - (cursor.start & 3);
>> +        uint32_t shift = (cursor.start & 3) * 8;
>>           uint32_t mask = 0xffffffff << shift;
>>   -        if (len < bytes) {
>> -            mask &= 0xffffffff >> (bytes - len) * 8;
>> -            bytes = len;
>> +        if (cursor.size < bytes) {
>> +            mask &= 0xffffffff >> (bytes - cursor.size) * 8;
>> +            bytes = cursor.size;
>>           }
>>             if (mask != 0xffffffff) {
>>               spin_lock_irqsave(&adev->mmio_idx_lock, flags);
>>               WREG32_NO_KIQ(mmMM_INDEX, ((uint32_t)aligned_pos) | 
>> 0x80000000);
>>               WREG32_NO_KIQ(mmMM_INDEX_HI, aligned_pos >> 31);
>> -            if (!write || mask != 0xffffffff)
>> -                value = RREG32_NO_KIQ(mmMM_DATA);
>> +            value = RREG32_NO_KIQ(mmMM_DATA);
>>               if (write) {
>>                   value &= ~mask;
>>                   value |= (*(uint32_t *)buf << shift) & mask;
>> @@ -1524,21 +1499,15 @@ static int amdgpu_ttm_access_memory(struct 
>> ttm_buffer_object *bo,
>>                   memcpy(buf, &value, bytes);
>>               }
>>           } else {
>> -            bytes = (nodes->start + nodes->size) << PAGE_SHIFT;
>> -            bytes = min(bytes - pos, (uint64_t)len & ~0x3ull);
>> -
>> -            amdgpu_device_vram_access(adev, pos, (uint32_t *)buf,
>> -                          bytes, write);
>> +            bytes = cursor.size & 0x3ull;
>> +            amdgpu_device_vram_access(adev, cursor.start,
>> +                          (uint32_t *)buf, bytes,
>> +                          write);
>>           }
>>             ret += bytes;
>>           buf = (uint8_t *)buf + bytes;
>> -        pos += bytes;
>> -        len -= bytes;
>> -        if (pos >= (nodes->start + nodes->size) << PAGE_SHIFT) {
>> -            ++nodes;
>> -            pos = (nodes->start << PAGE_SHIFT);
>> -        }
>> +        amdgpu_res_next(&cursor, bytes);
>>       }
>>         return ret;

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 5/8] drm/amdgpu: use the new cursor in amdgpu_ttm_access_memory
  2021-03-20  9:08     ` Christian König
@ 2021-03-22  0:47       ` Pan, Xinhui
  0 siblings, 0 replies; 18+ messages in thread
From: Pan, Xinhui @ 2021-03-22  0:47 UTC (permalink / raw)
  To: Christian König, Kuehling, Felix, Paneer Selvam, Arunpravin,
	amd-gfx

[AMD Official Use Only - Internal Distribution Only]

No, the patch from Nirmoy did not fully fix this issue. I will send another fix patch later.


-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Christian K?nig
Sent: 2021年3月20日 17:08
To: Kuehling, Felix <Felix.Kuehling@amd.com>; Paneer Selvam, Arunpravin <Arunpravin.PaneerSelvam@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 5/8] drm/amdgpu: use the new cursor in amdgpu_ttm_access_memory

Yeah, Nirmoy already stumbled over the correct fix.

The size check is off by one. Patch to fix this should be pushed on Monday.

Regards,
Christian.

Am 19.03.21 um 22:23 schrieb Felix Kuehling:
> This is causing a deadlock in amdgpu_ttm_access_memory during the
> PtraceAccess test in kfdtest. Unfortunately it doesn't get flagged by
> LOCKDEP. See the kernel log snippet below. I don't have a good
> explanation what's going on other than maybe some data structure
> corruption.
>
> With this patch reverted the PtraceAccess test still fails, but it
> doesn't hang any more. If I also revert "use new cursor in
> amdgpu_ttm_io_mem_pfn" (which is used via amdgpu_find_mm_node in
> amdgpu_ttm_access_memory), Ptrace access starts working correctly.
> That tells me that there is some fundamental bug in the resource
> cursor implementation that's breaking several users.
>
> Regards,
>   Felix
>
>
> [  129.446085] watchdog: BUG: soft lockup - CPU#8 stuck for 22s!
> [kfdtest:3588]
> [  129.455379] Modules linked in: ip6table_filter ip6_tables
> iptable_filter amdgpu x86_pkg_temp_thermal drm_ttm_helper ttm iommu_v2
> gpu_sched ip_tables x_tables [  129.455428] irq event stamp: 75294000
> [  129.455432] hardirqs last  enabled at (75293999):
> [<ffffffffa9dcfd7d>] _raw_spin_unlock_irqrestore+0x2d/0x40
> [  129.455447] hardirqs last disabled at (75294000):
> [<ffffffffa9dbef8a>] sysvec_apic_timer_interrupt+0xa/0xa0
> [  129.455457] softirqs last  enabled at (75184000):
> [<ffffffffaa000306>] __do_softirq+0x306/0x429 [  129.455467] softirqs
> last disabled at (75183995):
> [<ffffffffa9e00f8f>] asm_call_irq_on_stack+0xf/0x20 [  129.455477]
> CPU: 8 PID: 3588 Comm: kfdtest Not tainted 5.11.0-kfd-fkuehlin #194 [
> 129.455485] Hardware name: ASUS All Series/X99-E WS/USB 3.1, BIOS
> 3201 06/17/2016
> [  129.455490] RIP: 0010:_raw_spin_lock_irqsave+0xb/0x50
> [  129.455498] Code: d2 31 f6 e8 e7 e9 31 ff 48 89 df 58 5b e9 7d 32
> 32 ff 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 55 48 89
> fd 53 9c <5b> fa f6 c7 02 74 05 e8 59 06 3e ff 65 ff 05 92 73 24 56 48
> 8d 7d [  129.455505] RSP: 0018:ffffa3eb407f3c58 EFLAGS: 00000246 [
> 129.455513] RAX: ffff96466e2010a0 RBX: ffff96466e200000 RCX:
> 0000000000000000
> [  129.455519] RDX: ffffa3eb407f3e70 RSI: 00000000019ffff0 RDI:
> ffff96466e2010a0
> [  129.455524] RBP: ffff96466e2010a0 R08: 0000000000000000 R09:
> 0000000000000001
> [  129.455528] R10: ffffa3eb407f3c60 R11: ffff96466e2010b8 R12:
> 00000000019ffff0
> [  129.455533] R13: 00000000019ffff0 R14: ffffa3eb407f3e70 R15:
> 00000000019ffff0
> [  129.455538] FS:  00007f5aad0f0740(0000) GS:ffff96467fc00000(0000)
> knlGS:0000000000000000
> [  129.455544] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [
> 129.455549] CR2: 0000563ea76ad0f0 CR3: 00000007c6e92005 CR4:
> 00000000001706e0
> [  129.455554] Call Trace:
> [  129.455563]  amdgpu_device_vram_access+0xc1/0x200 [amdgpu] [
> 129.455820]  ? _raw_spin_unlock_irqrestore+0x2d/0x40
> [  129.455834]  amdgpu_ttm_access_memory+0x29e/0x320 [amdgpu] [
> 129.456063]  ttm_bo_vm_access+0x1c8/0x3a0 [ttm] [  129.456089]
> __access_remote_vm+0x289/0x390 [  129.456112]
> ptrace_access_vm+0x98/0xc0 [  129.456127]
> generic_ptrace_peekdata+0x31/0x80 [  129.456138]
> ptrace_request+0x13b/0x5d0 [  129.456155]  arch_ptrace+0x24f/0x2f0 [
> 129.456165]  __x64_sys_ptrace+0xc9/0x140 [  129.456177]
> do_syscall_64+0x2d/0x40 [  129.456185]
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [  129.456194] RIP: 0033:0x7f5aab861a3f [  129.456199] Code: 48 89 44
> 24 18 48 8d 44 24 30 c7 44 24 10 18 00
> 00 00 8b 70 08 48 8b 50 10 48 89 44 24 20 4c 0f 43 50 18 b8 65 00 00
> 00 0f 05 <48> 3d 00 f0 ff ff 77 41 48 85 c0 78 06 41 83 f8 02 76 1e 48
> 8b 4c [  129.456205] RSP: 002b:00007ffd27b68750 EFLAGS: 00000293
> ORIG_RAX:
> 0000000000000065
> [  129.456214] RAX: ffffffffffffffda RBX: 0000000000000001 RCX:
> 00007f5aab861a3f
> [  129.456219] RDX: 00007f5aab3ffff0 RSI: 0000000000000dfa RDI:
> 0000000000000002
> [  129.456224] RBP: 00007ffd27b68870 R08: 0000000000000001 R09:
> 0000000000000000
> [  129.456228] R10: 00007ffd27b68758 R11: 0000000000000293 R12:
> 0000563ea764e2aa
> [  129.456233] R13: 0000000000000000 R14: 0000000000000021 R15:
> 0000000000000000
>
> On 2021-03-08 8:40 a.m., Christian König wrote:
>> Separate the drm_mm_node walking from the actual handling.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> Acked-by: Oak Zeng <Oak.Zeng@amd.com>
>> Tested-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 67
>> +++++++------------------
>>   1 file changed, 18 insertions(+), 49 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index 517611b709fa..2cbe4ace591f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -178,26 +178,6 @@ static int amdgpu_verify_access(struct
>> ttm_buffer_object *bo, struct file *filp)
>>                         filp->private_data);
>>   }
>>   -/**
>> - * amdgpu_find_mm_node - Helper function finds the drm_mm_node
>> corresponding to
>> - * @offset. It also modifies the offset to be within the drm_mm_node
>> returned
>> - *
>> - * @mem: The region where the bo resides.
>> - * @offset: The offset that drm_mm_node is used for finding.
>> - *
>> - */
>> -static struct drm_mm_node *amdgpu_find_mm_node(struct ttm_resource
>> *mem,
>> -                           uint64_t *offset) -{
>> -    struct drm_mm_node *mm_node = mem->mm_node;
>> -
>> -    while (*offset >= (mm_node->size << PAGE_SHIFT)) {
>> -        *offset -= (mm_node->size << PAGE_SHIFT);
>> -        ++mm_node;
>> -    }
>> -    return mm_node;
>> -}
>> -
>>   /**
>>    * amdgpu_ttm_map_buffer - Map memory into the GART windows
>>    * @bo: buffer object to map
>> @@ -1478,41 +1458,36 @@ static bool
>> amdgpu_ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
>>    * access for debugging purposes.
>>    */
>>   static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo,
>> -                    unsigned long offset,
>> -                    void *buf, int len, int write)
>> +                    unsigned long offset, void *buf, int len,
>> +                    int write)
>>   {
>>       struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo);
>>       struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev);
>> -    struct drm_mm_node *nodes;
>> +    struct amdgpu_res_cursor cursor;
>> +    unsigned long flags;
>>       uint32_t value = 0;
>>       int ret = 0;
>> -    uint64_t pos;
>> -    unsigned long flags;
>>         if (bo->mem.mem_type != TTM_PL_VRAM)
>>           return -EIO;
>>   -    pos = offset;
>> -    nodes = amdgpu_find_mm_node(&abo->tbo.mem, &pos);
>> -    pos += (nodes->start << PAGE_SHIFT);
>> -
>> -    while (len && pos < adev->gmc.mc_vram_size) {
>> -        uint64_t aligned_pos = pos & ~(uint64_t)3;
>> -        uint64_t bytes = 4 - (pos & 3);
>> -        uint32_t shift = (pos & 3) * 8;
>> +    amdgpu_res_first(&bo->mem, offset, len, &cursor);
>> +    while (cursor.remaining) {
>> +        uint64_t aligned_pos = cursor.start & ~(uint64_t)3;
>> +        uint64_t bytes = 4 - (cursor.start & 3);
>> +        uint32_t shift = (cursor.start & 3) * 8;
>>           uint32_t mask = 0xffffffff << shift;
>>   -        if (len < bytes) {
>> -            mask &= 0xffffffff >> (bytes - len) * 8;
>> -            bytes = len;
>> +        if (cursor.size < bytes) {
>> +            mask &= 0xffffffff >> (bytes - cursor.size) * 8;
>> +            bytes = cursor.size;
>>           }
>>             if (mask != 0xffffffff) {
>>               spin_lock_irqsave(&adev->mmio_idx_lock, flags);
>>               WREG32_NO_KIQ(mmMM_INDEX, ((uint32_t)aligned_pos) |
>> 0x80000000);
>>               WREG32_NO_KIQ(mmMM_INDEX_HI, aligned_pos >> 31);
>> -            if (!write || mask != 0xffffffff)
>> -                value = RREG32_NO_KIQ(mmMM_DATA);
>> +            value = RREG32_NO_KIQ(mmMM_DATA);
>>               if (write) {
>>                   value &= ~mask;
>>                   value |= (*(uint32_t *)buf << shift) & mask; @@
>> -1524,21 +1499,15 @@ static int amdgpu_ttm_access_memory(struct
>> ttm_buffer_object *bo,
>>                   memcpy(buf, &value, bytes);
>>               }
>>           } else {
>> -            bytes = (nodes->start + nodes->size) << PAGE_SHIFT;
>> -            bytes = min(bytes - pos, (uint64_t)len & ~0x3ull);
>> -
>> -            amdgpu_device_vram_access(adev, pos, (uint32_t *)buf,
>> -                          bytes, write);
>> +            bytes = cursor.size & 0x3ull;
>> +            amdgpu_device_vram_access(adev, cursor.start,
>> +                          (uint32_t *)buf, bytes,
>> +                          write);
>>           }
>>             ret += bytes;
>>           buf = (uint8_t *)buf + bytes;
>> -        pos += bytes;
>> -        len -= bytes;
>> -        if (pos >= (nodes->start + nodes->size) << PAGE_SHIFT) {
>> -            ++nodes;
>> -            pos = (nodes->start << PAGE_SHIFT);
>> -        }
>> +        amdgpu_res_next(&cursor, bytes);
>>       }
>>         return ret;

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cxinhui.pan%40amd.com%7C9ee7a8bef8ea40e37dac08d8eb7faf73%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637518280926169326%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=ncIBwfPtsvw379ihW47cbchHwBRArQV%2B5GB8wpY5ylc%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 5/8] drm/amdgpu: use the new cursor in amdgpu_ttm_access_memory
  2021-03-19 21:23   ` Felix Kuehling
  2021-03-20  9:08     ` Christian König
@ 2021-03-22  1:19     ` Pan, Xinhui
  1 sibling, 0 replies; 18+ messages in thread
From: Pan, Xinhui @ 2021-03-22  1:19 UTC (permalink / raw)
  To: Kuehling, Felix, Christian König, Paneer Selvam, Arunpravin,
	amd-gfx

[AMD Official Use Only - Internal Distribution Only]

Because this is not a deadlock of lock itself.
Just because something like
while(true) {
....
LOCKIRQ
...
UNLOCKIRQ
...
}
I think scheduler policy is voluntary.  So it never schedule out if there is no sleep function and then soft lockup showed up when interrupt enabled.

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Felix Kuehling
Sent: 2021年3月20日 5:23
To: Christian König <ckoenig.leichtzumerken@gmail.com>; Paneer Selvam, Arunpravin <Arunpravin.PaneerSelvam@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 5/8] drm/amdgpu: use the new cursor in amdgpu_ttm_access_memory

This is causing a deadlock in amdgpu_ttm_access_memory during the PtraceAccess test in kfdtest. Unfortunately it doesn't get flagged by LOCKDEP. See the kernel log snippet below. I don't have a good explanation what's going on other than maybe some data structure corruption.

With this patch reverted the PtraceAccess test still fails, but it doesn't hang any more. If I also revert "use new cursor in amdgpu_ttm_io_mem_pfn" (which is used via amdgpu_find_mm_node in amdgpu_ttm_access_memory), Ptrace access starts working correctly. That tells me that there is some fundamental bug in the resource cursor implementation that's breaking several users.

Regards,
   Felix


[  129.446085] watchdog: BUG: soft lockup - CPU#8 stuck for 22s! [kfdtest:3588] [  129.455379] Modules linked in: ip6table_filter ip6_tables iptable_filter amdgpu x86_pkg_temp_thermal drm_ttm_helper ttm iommu_v2 gpu_sched ip_tables x_tables [  129.455428] irq event stamp: 75294000 [  129.455432] hardirqs last  enabled at (75293999): [<ffffffffa9dcfd7d>] _raw_spin_unlock_irqrestore+0x2d/0x40
[  129.455447] hardirqs last disabled at (75294000): [<ffffffffa9dbef8a>] sysvec_apic_timer_interrupt+0xa/0xa0
[  129.455457] softirqs last  enabled at (75184000): [<ffffffffaa000306>] __do_softirq+0x306/0x429 [  129.455467] softirqs last disabled at (75183995): [<ffffffffa9e00f8f>] asm_call_irq_on_stack+0xf/0x20 [  129.455477] CPU: 8 PID: 3588 Comm: kfdtest Not tainted 5.11.0-kfd-fkuehlin #194 [  129.455485] Hardware name: ASUS All Series/X99-E WS/USB 3.1, BIOS 3201 06/17/2016 [  129.455490] RIP: 0010:_raw_spin_lock_irqsave+0xb/0x50
[  129.455498] Code: d2 31 f6 e8 e7 e9 31 ff 48 89 df 58 5b e9 7d 32 32 ff 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 55 48 89 fd 53 9c <5b> fa f6 c7 02 74 05 e8 59 06 3e ff 65 ff 05 92 73 24 56 48 8d 7d [  129.455505] RSP: 0018:ffffa3eb407f3c58 EFLAGS: 00000246 [  129.455513] RAX: ffff96466e2010a0 RBX: ffff96466e200000 RCX: 0000000000000000 [  129.455519] RDX: ffffa3eb407f3e70 RSI: 00000000019ffff0 RDI: ffff96466e2010a0 [  129.455524] RBP: ffff96466e2010a0 R08: 0000000000000000 R09: 0000000000000001 [  129.455528] R10: ffffa3eb407f3c60 R11: ffff96466e2010b8 R12: 00000000019ffff0 [  129.455533] R13: 00000000019ffff0 R14: ffffa3eb407f3e70 R15: 00000000019ffff0 [  129.455538] FS:  00007f5aad0f0740(0000) GS:ffff96467fc00000(0000) knlGS:0000000000000000 [  129.455544] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [  129.455549] CR2: 0000563ea76ad0f0 CR3: 00000007c6e92005 CR4: 00000000001706e0 [  129.455554] Call Trace:
[  129.455563]  amdgpu_device_vram_access+0xc1/0x200 [amdgpu] [  129.455820]  ? _raw_spin_unlock_irqrestore+0x2d/0x40
[  129.455834]  amdgpu_ttm_access_memory+0x29e/0x320 [amdgpu] [  129.456063]  ttm_bo_vm_access+0x1c8/0x3a0 [ttm] [  129.456089]  __access_remote_vm+0x289/0x390 [  129.456112]  ptrace_access_vm+0x98/0xc0 [  129.456127]  generic_ptrace_peekdata+0x31/0x80 [  129.456138]  ptrace_request+0x13b/0x5d0 [  129.456155]  arch_ptrace+0x24f/0x2f0 [  129.456165]  __x64_sys_ptrace+0xc9/0x140 [  129.456177]  do_syscall_64+0x2d/0x40 [  129.456185]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  129.456194] RIP: 0033:0x7f5aab861a3f
[  129.456199] Code: 48 89 44 24 18 48 8d 44 24 30 c7 44 24 10 18 00 00 00 8b 70 08 48 8b 50 10 48 89 44 24 20 4c 0f 43 50 18 b8 65 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 41 48 85 c0 78 06 41 83 f8 02 76 1e 48 8b 4c [  129.456205] RSP: 002b:00007ffd27b68750 EFLAGS: 00000293 ORIG_RAX: 0000000000000065 [  129.456214] RAX: ffffffffffffffda RBX: 0000000000000001 RCX: 00007f5aab861a3f [  129.456219] RDX: 00007f5aab3ffff0 RSI: 0000000000000dfa RDI: 0000000000000002 [  129.456224] RBP: 00007ffd27b68870 R08: 0000000000000001 R09: 0000000000000000 [  129.456228] R10: 00007ffd27b68758 R11: 0000000000000293 R12: 0000563ea764e2aa [  129.456233] R13: 0000000000000000 R14: 0000000000000021 R15: 0000000000000000

On 2021-03-08 8:40 a.m., Christian König wrote:
> Separate the drm_mm_node walking from the actual handling.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Acked-by: Oak Zeng <Oak.Zeng@amd.com>
> Tested-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 67 +++++++------------------
>   1 file changed, 18 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 517611b709fa..2cbe4ace591f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -178,26 +178,6 @@ static int amdgpu_verify_access(struct ttm_buffer_object *bo, struct file *filp)
>     filp->private_data);
>   }
>
> -/**
> - * amdgpu_find_mm_node - Helper function finds the drm_mm_node
> corresponding to
> - * @offset. It also modifies the offset to be within the drm_mm_node
> returned
> - *
> - * @mem: The region where the bo resides.
> - * @offset: The offset that drm_mm_node is used for finding.
> - *
> - */
> -static struct drm_mm_node *amdgpu_find_mm_node(struct ttm_resource *mem,
> -       uint64_t *offset)
> -{
> -struct drm_mm_node *mm_node = mem->mm_node;
> -
> -while (*offset >= (mm_node->size << PAGE_SHIFT)) {
> -*offset -= (mm_node->size << PAGE_SHIFT);
> -++mm_node;
> -}
> -return mm_node;
> -}
> -
>   /**
>    * amdgpu_ttm_map_buffer - Map memory into the GART windows
>    * @bo: buffer object to map
> @@ -1478,41 +1458,36 @@ static bool amdgpu_ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
>    * access for debugging purposes.
>    */
>   static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo,
> -    unsigned long offset,
> -    void *buf, int len, int write)
> +    unsigned long offset, void *buf, int len,
> +    int write)
>   {
>   struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo);
>   struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev);
> -struct drm_mm_node *nodes;
> +struct amdgpu_res_cursor cursor;
> +unsigned long flags;
>   uint32_t value = 0;
>   int ret = 0;
> -uint64_t pos;
> -unsigned long flags;
>
>   if (bo->mem.mem_type != TTM_PL_VRAM)
>   return -EIO;
>
> -pos = offset;
> -nodes = amdgpu_find_mm_node(&abo->tbo.mem, &pos);
> -pos += (nodes->start << PAGE_SHIFT);
> -
> -while (len && pos < adev->gmc.mc_vram_size) {
> -uint64_t aligned_pos = pos & ~(uint64_t)3;
> -uint64_t bytes = 4 - (pos & 3);
> -uint32_t shift = (pos & 3) * 8;
> +amdgpu_res_first(&bo->mem, offset, len, &cursor);
> +while (cursor.remaining) {
> +uint64_t aligned_pos = cursor.start & ~(uint64_t)3;
> +uint64_t bytes = 4 - (cursor.start & 3);
> +uint32_t shift = (cursor.start & 3) * 8;
>   uint32_t mask = 0xffffffff << shift;
>
> -if (len < bytes) {
> -mask &= 0xffffffff >> (bytes - len) * 8;
> -bytes = len;
> +if (cursor.size < bytes) {
> +mask &= 0xffffffff >> (bytes - cursor.size) * 8;
> +bytes = cursor.size;
>   }
>
>   if (mask != 0xffffffff) {
>   spin_lock_irqsave(&adev->mmio_idx_lock, flags);
>   WREG32_NO_KIQ(mmMM_INDEX, ((uint32_t)aligned_pos) | 0x80000000);
>   WREG32_NO_KIQ(mmMM_INDEX_HI, aligned_pos >> 31);
> -if (!write || mask != 0xffffffff)
> -value = RREG32_NO_KIQ(mmMM_DATA);
> +value = RREG32_NO_KIQ(mmMM_DATA);
>   if (write) {
>   value &= ~mask;
>   value |= (*(uint32_t *)buf << shift) & mask; @@ -1524,21
> +1499,15 @@ static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo,
>   memcpy(buf, &value, bytes);
>   }
>   } else {
> -bytes = (nodes->start + nodes->size) << PAGE_SHIFT;
> -bytes = min(bytes - pos, (uint64_t)len & ~0x3ull);
> -
> -amdgpu_device_vram_access(adev, pos, (uint32_t *)buf,
> -  bytes, write);
> +bytes = cursor.size & 0x3ull;
> +amdgpu_device_vram_access(adev, cursor.start,
> +  (uint32_t *)buf, bytes,
> +  write);
>   }
>
>   ret += bytes;
>   buf = (uint8_t *)buf + bytes;
> -pos += bytes;
> -len -= bytes;
> -if (pos >= (nodes->start + nodes->size) << PAGE_SHIFT) {
> -++nodes;
> -pos = (nodes->start << PAGE_SHIFT);
> -}
> +amdgpu_res_next(&cursor, bytes);
>   }
>
>   return ret;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cxinhui.pan%40amd.com%7C7180c50948af41b99c1708d8eb1d3b0d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637517858073213021%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=TnsKgmz3XtMvVc7xa048KST8tCK5O%2F21UATxD10HKBE%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/8] drm/amdgpu: new resource cursor
  2021-02-17 19:00 Christian König
  2021-02-17 21:18 ` Zeng, Oak
@ 2021-02-18 10:11 ` Nirmoy
  1 sibling, 0 replies; 18+ messages in thread
From: Nirmoy @ 2021-02-18 10:11 UTC (permalink / raw)
  To: Christian König, Ramesh.Errabolu, amd-gfx; +Cc: Arunpravin.PaneerSelvam

Tested on RX 580.

The series is Tested-by: Nirmoy Das <nirmoy.das@amd.com>


On 2/17/21 8:00 PM, Christian König wrote:
> Allows to walk over the drm_mm nodes in a TTM resource object.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   .../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h    | 105 ++++++++++++++++++
>   1 file changed, 105 insertions(+)
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> new file mode 100644
> index 000000000000..1335e098510f
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> @@ -0,0 +1,105 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * Copyright 2020 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * Authors: Christian König
> + */
> +
> +#ifndef __AMDGPU_RES_CURSOR_H__
> +#define __AMDGPU_RES_CURSOR_H__
> +
> +#include <drm/drm_mm.h>
> +#include <drm/ttm/ttm_resource.h>
> +
> +/* state back for walking over vram_mgr and gtt_mgr allocations */
> +struct amdgpu_res_cursor {
> +	uint64_t		start;
> +	uint64_t		size;
> +	uint64_t		remaining;
> +	struct drm_mm_node	*node;
> +};
> +
> +/**
> + * amdgpu_res_first - initialize a amdgpu_res_cursor
> + *
> + * @res: TTM resource object to walk
> + * @start: Start of the range
> + * @size: Size of the range
> + * @cur: cursor object to initialize
> + *
> + * Start walking over the range of allocations between @start and @size.
> + */
> +static inline void amdgpu_res_first(struct ttm_resource *res,
> +				    uint64_t start, uint64_t size,
> +				    struct amdgpu_res_cursor *cur)
> +{
> +	struct drm_mm_node *node;
> +
> +	if (!res || !res->mm_node) {
> +		cur->start = start;
> +		cur->size = size;
> +		cur->remaining = size;
> +		cur->node = NULL;
> +		return;
> +	}
> +
> +	BUG_ON(start + size > res->num_pages << PAGE_SHIFT);
> +
> +	node = res->mm_node;
> +	while (start > node->size << PAGE_SHIFT)
> +		start -= node++->size << PAGE_SHIFT;
> +
> +	cur->start = (node->start << PAGE_SHIFT) + start;
> +	cur->size = (node->size << PAGE_SHIFT) - start;
> +	cur->remaining = size;
> +	cur->node = node;
> +}
> +
> +/**
> + * amdgpu_res_next - advance the cursor
> + *
> + * @cur: the cursor to advance
> + * @size: number of bytes to move forward
> + *
> + * Move the cursor @size bytes forwrad, walking to the next node if necessary.
> + */
> +static inline void amdgpu_res_next(struct amdgpu_res_cursor *cur, uint64_t size)
> +{
> +	struct drm_mm_node *node = cur->node;
> +
> +	BUG_ON(size > cur->remaining);
> +
> +	cur->remaining -= size;
> +	if (!cur->remaining)
> +		return;
> +
> +	cur->size -= size;
> +	if (cur->size) {
> +		cur->start += size;
> +		return;
> +	}
> +
> +	cur->node = ++node;
> +	cur->start = node->start << PAGE_SHIFT;
> +	cur->size = min(node->size << PAGE_SHIFT, cur->remaining);
> +}
> +
> +#endif
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 1/8] drm/amdgpu: new resource cursor
  2021-02-17 19:00 Christian König
@ 2021-02-17 21:18 ` Zeng, Oak
  2021-02-18 10:11 ` Nirmoy
  1 sibling, 0 replies; 18+ messages in thread
From: Zeng, Oak @ 2021-02-17 21:18 UTC (permalink / raw)
  To: Christian König, Errabolu, Ramesh, amd-gfx; +Cc: Paneer Selvam, Arunpravin

[AMD Official Use Only - Internal Distribution Only]

Very nice cleaning! Series is Acked-by: Oak Zeng <Oak.Zeng@amd.com>

Regards,
Oak

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Christian König
> Sent: Wednesday, February 17, 2021 2:00 PM
> To: Errabolu, Ramesh <Ramesh.Errabolu@amd.com>; amd-
> gfx@lists.freedesktop.org
> Cc: Paneer Selvam, Arunpravin <Arunpravin.PaneerSelvam@amd.com>
> Subject: [PATCH 1/8] drm/amdgpu: new resource cursor
>
> Allows to walk over the drm_mm nodes in a TTM resource object.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  .../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h    | 105
> ++++++++++++++++++
>  1 file changed, 105 insertions(+)
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> new file mode 100644
> index 000000000000..1335e098510f
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> @@ -0,0 +1,105 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * Copyright 2020 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person
> +obtaining a
> + * copy of this software and associated documentation files (the
> +"Software"),
> + * to deal in the Software without restriction, including without
> +limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> +sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom
> +the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> +included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
> EVENT
> +SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM,
> +DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> +OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
> THE USE
> +OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * Authors: Christian König
> + */
> +
> +#ifndef __AMDGPU_RES_CURSOR_H__
> +#define __AMDGPU_RES_CURSOR_H__
> +
> +#include <drm/drm_mm.h>
> +#include <drm/ttm/ttm_resource.h>
> +
> +/* state back for walking over vram_mgr and gtt_mgr allocations */
> +struct amdgpu_res_cursor {
> +uint64_tstart;
> +uint64_tsize;
> +uint64_tremaining;
> +struct drm_mm_node*node;
> +};
> +
> +/**
> + * amdgpu_res_first - initialize a amdgpu_res_cursor
> + *
> + * @res: TTM resource object to walk
> + * @start: Start of the range
> + * @size: Size of the range
> + * @cur: cursor object to initialize
> + *
> + * Start walking over the range of allocations between @start and @size.
> + */
> +static inline void amdgpu_res_first(struct ttm_resource *res,
> +    uint64_t start, uint64_t size,
> +    struct amdgpu_res_cursor *cur)
> +{
> +struct drm_mm_node *node;
> +
> +if (!res || !res->mm_node) {
> +cur->start = start;
> +cur->size = size;
> +cur->remaining = size;
> +cur->node = NULL;
> +return;
> +}
> +
> +BUG_ON(start + size > res->num_pages << PAGE_SHIFT);
> +
> +node = res->mm_node;
> +while (start > node->size << PAGE_SHIFT)
> +start -= node++->size << PAGE_SHIFT;
> +
> +cur->start = (node->start << PAGE_SHIFT) + start;
> +cur->size = (node->size << PAGE_SHIFT) - start;
> +cur->remaining = size;
> +cur->node = node;
> +}
> +
> +/**
> + * amdgpu_res_next - advance the cursor
> + *
> + * @cur: the cursor to advance
> + * @size: number of bytes to move forward
> + *
> + * Move the cursor @size bytes forwrad, walking to the next node if
> necessary.
> + */
> +static inline void amdgpu_res_next(struct amdgpu_res_cursor *cur,
> +uint64_t size) {
> +struct drm_mm_node *node = cur->node;
> +
> +BUG_ON(size > cur->remaining);
> +
> +cur->remaining -= size;
> +if (!cur->remaining)
> +return;
> +
> +cur->size -= size;
> +if (cur->size) {
> +cur->start += size;
> +return;
> +}
> +
> +cur->node = ++node;
> +cur->start = node->start << PAGE_SHIFT;
> +cur->size = min(node->size << PAGE_SHIFT, cur->remaining); }
> +
> +#endif
> --
> 2.25.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.
> freedesktop.org%2Fmailman%2Flistinfo%2Famd-
> gfx&amp;data=04%7C01%7Coak.zeng%40amd.com%7C467ca2054278419a6e
> 8508d8d3764e38%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637
> 491852368143275%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAi
> LCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=uZ
> t%2B%2FIVc7%2FD2jtq%2F%2BIogn17%2FpfU16l%2F6%2Fccf5QBSjqw%3D&a
> mp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/8] drm/amdgpu: new resource cursor
@ 2021-02-17 19:00 Christian König
  2021-02-17 21:18 ` Zeng, Oak
  2021-02-18 10:11 ` Nirmoy
  0 siblings, 2 replies; 18+ messages in thread
From: Christian König @ 2021-02-17 19:00 UTC (permalink / raw)
  To: Ramesh.Errabolu, amd-gfx; +Cc: Arunpravin.PaneerSelvam

Allows to walk over the drm_mm nodes in a TTM resource object.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 .../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h    | 105 ++++++++++++++++++
 1 file changed, 105 insertions(+)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
new file mode 100644
index 000000000000..1335e098510f
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright 2020 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * Authors: Christian König
+ */
+
+#ifndef __AMDGPU_RES_CURSOR_H__
+#define __AMDGPU_RES_CURSOR_H__
+
+#include <drm/drm_mm.h>
+#include <drm/ttm/ttm_resource.h>
+
+/* state back for walking over vram_mgr and gtt_mgr allocations */
+struct amdgpu_res_cursor {
+	uint64_t		start;
+	uint64_t		size;
+	uint64_t		remaining;
+	struct drm_mm_node	*node;
+};
+
+/**
+ * amdgpu_res_first - initialize a amdgpu_res_cursor
+ *
+ * @res: TTM resource object to walk
+ * @start: Start of the range
+ * @size: Size of the range
+ * @cur: cursor object to initialize
+ *
+ * Start walking over the range of allocations between @start and @size.
+ */
+static inline void amdgpu_res_first(struct ttm_resource *res,
+				    uint64_t start, uint64_t size,
+				    struct amdgpu_res_cursor *cur)
+{
+	struct drm_mm_node *node;
+
+	if (!res || !res->mm_node) {
+		cur->start = start;
+		cur->size = size;
+		cur->remaining = size;
+		cur->node = NULL;
+		return;
+	}
+
+	BUG_ON(start + size > res->num_pages << PAGE_SHIFT);
+
+	node = res->mm_node;
+	while (start > node->size << PAGE_SHIFT)
+		start -= node++->size << PAGE_SHIFT;
+
+	cur->start = (node->start << PAGE_SHIFT) + start;
+	cur->size = (node->size << PAGE_SHIFT) - start;
+	cur->remaining = size;
+	cur->node = node;
+}
+
+/**
+ * amdgpu_res_next - advance the cursor
+ *
+ * @cur: the cursor to advance
+ * @size: number of bytes to move forward
+ *
+ * Move the cursor @size bytes forwrad, walking to the next node if necessary.
+ */
+static inline void amdgpu_res_next(struct amdgpu_res_cursor *cur, uint64_t size)
+{
+	struct drm_mm_node *node = cur->node;
+
+	BUG_ON(size > cur->remaining);
+
+	cur->remaining -= size;
+	if (!cur->remaining)
+		return;
+
+	cur->size -= size;
+	if (cur->size) {
+		cur->start += size;
+		return;
+	}
+
+	cur->node = ++node;
+	cur->start = node->start << PAGE_SHIFT;
+	cur->size = min(node->size << PAGE_SHIFT, cur->remaining);
+}
+
+#endif
-- 
2.25.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-03-22  1:19 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-08 13:40 [PATCH 1/8] drm/amdgpu: new resource cursor Christian König
2021-03-08 13:40 ` [PATCH 2/8] drm/amdgpu: use the new cursor in amdgpu_ttm_copy_mem_to_mem Christian König
2021-03-08 13:40 ` [PATCH 3/8] drm/amdgpu: use the new cursor in amdgpu_fill_buffer Christian König
2021-03-08 13:40 ` [PATCH 4/8] drm/amdgpu: use new cursor in amdgpu_ttm_io_mem_pfn Christian König
2021-03-08 13:40 ` [PATCH 5/8] drm/amdgpu: use the new cursor in amdgpu_ttm_access_memory Christian König
2021-03-19 21:23   ` Felix Kuehling
2021-03-20  9:08     ` Christian König
2021-03-22  0:47       ` Pan, Xinhui
2021-03-22  1:19     ` Pan, Xinhui
2021-03-08 13:40 ` [PATCH 6/8] drm/amdgpu: use new cursor in amdgpu_mem_visible Christian König
2021-03-08 13:40 ` [PATCH 7/8] drm/amdgpu: use the new cursor in amdgpu_ttm_bo_eviction_valuable Christian König
2021-03-08 13:40 ` [PATCH 8/8] drm/amdgpu: use the new cursor in the VM code Christian König
2021-03-12 10:52 ` [PATCH 1/8] drm/amdgpu: new resource cursor Christian König
2021-03-12 11:11   ` Paneer Selvam, Arunpravin
2021-03-12 12:16     ` Christian König
  -- strict thread matches above, loose matches on Subject: below --
2021-02-17 19:00 Christian König
2021-02-17 21:18 ` Zeng, Oak
2021-02-18 10:11 ` Nirmoy

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.