[PATCH v6 1/6] drm/ttm: Add new callbacks to ttm res mgr

[PATCH v6 1/6] drm/ttm: Add new callbacks to ttm res mgr

[Arunpravin Paneer Selvam]

We are adding two new callbacks to ttm resource manager
function to handle intersection and compatibility of
placement and resources.

v2: move the amdgpu and ttm_range_manager changes to
    separate patches (Christian)
v3: rename "intersect" to "intersects" (Matthew)
v4: move !place check to the !res if and return false
    in ttm_resource_compatible() function (Christian)
v5: move bits of code from patch number 6 to avoid
    temporary driver breakup (Christian)

Signed-off-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c       |  9 ++--
 drivers/gpu/drm/ttm/ttm_resource.c | 77 +++++++++++++++++++++++++++++-
 include/drm/ttm/ttm_resource.h     | 40 ++++++++++++++++
 3 files changed, 119 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index c1bd006a5525..f066e8124c50 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -518,6 +518,9 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
 bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
 			      const struct ttm_place *place)
 {
+	struct ttm_resource *res = bo->resource;
+	struct ttm_device *bdev = bo->bdev;
+
 	dma_resv_assert_held(bo->base.resv);
 	if (bo->resource->mem_type == TTM_PL_SYSTEM)
 		return true;
@@ -525,11 +528,7 @@ bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
 	/* Don't evict this BO if it's outside of the
 	 * requested placement range
 	 */
-	if (place->fpfn >= (bo->resource->start + bo->resource->num_pages) ||
-	    (place->lpfn && place->lpfn <= bo->resource->start))
-		return false;
-
-	return true;
+	return ttm_resource_intersects(bdev, res, place, bo->base.size);
 }
 EXPORT_SYMBOL(ttm_bo_eviction_valuable);
 
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index 20f9adcc3235..0d1f862a582b 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -253,10 +253,84 @@ void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res)
 }
 EXPORT_SYMBOL(ttm_resource_free);
 
+/**
+ * ttm_resource_intersects - test for intersection
+ *
+ * @bdev: TTM device structure
+ * @res: The resource to test
+ * @place: The placement to test
+ * @size: How many bytes the new allocation needs.
+ *
+ * Test if @res intersects with @place and @size. Used for testing if evictions
+ * are valueable or not.
+ *
+ * Returns true if the res placement intersects with @place and @size.
+ */
+bool ttm_resource_intersects(struct ttm_device *bdev,
+			     struct ttm_resource *res,
+			     const struct ttm_place *place,
+			     size_t size)
+{
+	struct ttm_resource_manager *man;
+
+	if (!res)
+		return false;
+
+	if (!place)
+		return true;
+
+	man = ttm_manager_type(bdev, res->mem_type);
+	if (!man->func->intersects) {
+		if (place->fpfn >= (res->start + res->num_pages) ||
+		    (place->lpfn && place->lpfn <= res->start))
+			return false;
+
+		return true;
+	}
+
+	return man->func->intersects(man, res, place, size);
+}
+
+/**
+ * ttm_resource_compatible - test for compatibility
+ *
+ * @bdev: TTM device structure
+ * @res: The resource to test
+ * @place: The placement to test
+ * @size: How many bytes the new allocation needs.
+ *
+ * Test if @res compatible with @place and @size.
+ *
+ * Returns true if the res placement compatible with @place and @size.
+ */
+bool ttm_resource_compatible(struct ttm_device *bdev,
+			     struct ttm_resource *res,
+			     const struct ttm_place *place,
+			     size_t size)
+{
+	struct ttm_resource_manager *man;
+
+	if (!res || !place)
+		return false;
+
+	man = ttm_manager_type(bdev, res->mem_type);
+	if (!man->func->compatible) {
+		if (res->start < place->fpfn ||
+		    (place->lpfn && (res->start + res->num_pages) > place->lpfn))
+			return false;
+
+		return true;
+	}
+
+	return man->func->compatible(man, res, place, size);
+}
+
 static bool ttm_resource_places_compat(struct ttm_resource *res,
 				       const struct ttm_place *places,
 				       unsigned num_placement)
 {
+	struct ttm_buffer_object *bo = res->bo;
+	struct ttm_device *bdev = bo->bdev;
 	unsigned i;
 
 	if (res->placement & TTM_PL_FLAG_TEMPORARY)
@@ -265,8 +339,7 @@ static bool ttm_resource_places_compat(struct ttm_resource *res,
 	for (i = 0; i < num_placement; i++) {
 		const struct ttm_place *heap = &places[i];
 
-		if (res->start < heap->fpfn || (heap->lpfn &&
-		    (res->start + res->num_pages) > heap->lpfn))
+		if (!ttm_resource_compatible(bdev, res, heap, bo->base.size))
 			continue;
 
 		if ((res->mem_type == heap->mem_type) &&
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index ca89a48c2460..5afc6d664fde 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -88,6 +88,38 @@ struct ttm_resource_manager_func {
 	void (*free)(struct ttm_resource_manager *man,
 		     struct ttm_resource *res);
 
+	/**
+	 * struct ttm_resource_manager_func member intersects
+	 *
+	 * @man: Pointer to a memory type manager.
+	 * @res: Pointer to a struct ttm_resource to be checked.
+	 * @place: Placement to check against.
+	 * @size: Size of the check.
+	 *
+	 * Test if @res intersects with @place + @size. Used to judge if
+	 * evictions are valueable or not.
+	 */
+	bool (*intersects)(struct ttm_resource_manager *man,
+			   struct ttm_resource *res,
+			   const struct ttm_place *place,
+			   size_t size);
+
+	/**
+	 * struct ttm_resource_manager_func member compatible
+	 *
+	 * @man: Pointer to a memory type manager.
+	 * @res: Pointer to a struct ttm_resource to be checked.
+	 * @place: Placement to check against.
+	 * @size: Size of the check.
+	 *
+	 * Test if @res compatible with @place + @size. Used to check of
+	 * the need to move the backing store or not.
+	 */
+	bool (*compatible)(struct ttm_resource_manager *man,
+			   struct ttm_resource *res,
+			   const struct ttm_place *place,
+			   size_t size);
+
 	/**
 	 * struct ttm_resource_manager_func member debug
 	 *
@@ -329,6 +361,14 @@ int ttm_resource_alloc(struct ttm_buffer_object *bo,
 		       const struct ttm_place *place,
 		       struct ttm_resource **res);
 void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res);
+bool ttm_resource_intersects(struct ttm_device *bdev,
+			     struct ttm_resource *res,
+			     const struct ttm_place *place,
+			     size_t size);
+bool ttm_resource_compatible(struct ttm_device *bdev,
+			     struct ttm_resource *res,
+			     const struct ttm_place *place,
+			     size_t size);
 bool ttm_resource_compat(struct ttm_resource *res,
 			 struct ttm_placement *placement);
 void ttm_resource_set_bo(struct ttm_resource *res,

base-commit: 730c2bf4ad395acf0aa0820535fdb8ea6abe5df1
-- 
2.25.1

[PATCH v6 2/6] drm/ttm: Implement intersect/compatible functions

[Arunpravin Paneer Selvam]

Implemented a new intersect and compatible callback functions
to ttm range manager fetching start offset from drm mm range
allocator.

Signed-off-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
---
 drivers/gpu/drm/ttm/ttm_range_manager.c | 33 +++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_range_manager.c b/drivers/gpu/drm/ttm/ttm_range_manager.c
index d91666721dc6..4cfef2b3514d 100644
--- a/drivers/gpu/drm/ttm/ttm_range_manager.c
+++ b/drivers/gpu/drm/ttm/ttm_range_manager.c
@@ -113,6 +113,37 @@ static void ttm_range_man_free(struct ttm_resource_manager *man,
 	kfree(node);
 }
 
+static bool ttm_range_man_intersects(struct ttm_resource_manager *man,
+				     struct ttm_resource *res,
+				     const struct ttm_place *place,
+				     size_t size)
+{
+	struct drm_mm_node *node = &to_ttm_range_mgr_node(res)->mm_nodes[0];
+	u32 num_pages = PFN_UP(size);
+
+	/* Don't evict BOs outside of the requested placement range */
+	if (place->fpfn >= (node->start + num_pages) ||
+	    (place->lpfn && place->lpfn <= node->start))
+		return false;
+
+	return true;
+}
+
+static bool ttm_range_man_compatible(struct ttm_resource_manager *man,
+				     struct ttm_resource *res,
+				     const struct ttm_place *place,
+				     size_t size)
+{
+	struct drm_mm_node *node = &to_ttm_range_mgr_node(res)->mm_nodes[0];
+	u32 num_pages = PFN_UP(size);
+
+	if (node->start < place->fpfn ||
+	    (place->lpfn && (node->start + num_pages) > place->lpfn))
+		return false;
+
+	return true;
+}
+
 static void ttm_range_man_debug(struct ttm_resource_manager *man,
 				struct drm_printer *printer)
 {
@@ -126,6 +157,8 @@ static void ttm_range_man_debug(struct ttm_resource_manager *man,
 static const struct ttm_resource_manager_func ttm_range_manager_func = {
 	.alloc = ttm_range_man_alloc,
 	.free = ttm_range_man_free,
+	.intersects = ttm_range_man_intersects,
+	.compatible = ttm_range_man_compatible,
 	.debug = ttm_range_man_debug
 };
 
-- 
2.25.1

[PATCH v6 3/6] drm/amdgpu: Implement intersect/compatible functions

[Arunpravin Paneer Selvam]

Implemented a new intersect and compatible callback function
fetching start offset from backend drm buddy allocator.

Signed-off-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c  | 38 +++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 68 ++++++++++++++++++++
 2 files changed, 106 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 8c6b2284cf56..1f3302aebeff 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -204,6 +204,42 @@ void amdgpu_gtt_mgr_recover(struct amdgpu_gtt_mgr *mgr)
 	amdgpu_gart_invalidate_tlb(adev);
 }
 
+/**
+ * amdgpu_gtt_mgr_intersects - test for intersection
+ *
+ * @man: Our manager object
+ * @res: The resource to test
+ * @place: The place for the new allocation
+ * @size: The size of the new allocation
+ *
+ * Simplified intersection test, only interesting if we need GART or not.
+ */
+static bool amdgpu_gtt_mgr_intersects(struct ttm_resource_manager *man,
+				      struct ttm_resource *res,
+				      const struct ttm_place *place,
+				      size_t size)
+{
+	return !place->lpfn || amdgpu_gtt_mgr_has_gart_addr(res);
+}
+
+/**
+ * amdgpu_gtt_mgr_compatible - test for compatibility
+ *
+ * @man: Our manager object
+ * @res: The resource to test
+ * @place: The place for the new allocation
+ * @size: The size of the new allocation
+ *
+ * Simplified compatibility test.
+ */
+static bool amdgpu_gtt_mgr_compatible(struct ttm_resource_manager *man,
+				      struct ttm_resource *res,
+				      const struct ttm_place *place,
+				      size_t size)
+{
+	return !place->lpfn || amdgpu_gtt_mgr_has_gart_addr(res);
+}
+
 /**
  * amdgpu_gtt_mgr_debug - dump VRAM table
  *
@@ -225,6 +261,8 @@ static void amdgpu_gtt_mgr_debug(struct ttm_resource_manager *man,
 static const struct ttm_resource_manager_func amdgpu_gtt_mgr_func = {
 	.alloc = amdgpu_gtt_mgr_new,
 	.free = amdgpu_gtt_mgr_del,
+	.intersects = amdgpu_gtt_mgr_intersects,
+	.compatible = amdgpu_gtt_mgr_compatible,
 	.debug = amdgpu_gtt_mgr_debug
 };
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 28ec5f8ac1c1..d1a2619fa89f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -720,6 +720,72 @@ uint64_t amdgpu_vram_mgr_vis_usage(struct amdgpu_vram_mgr *mgr)
 	return atomic64_read(&mgr->vis_usage);
 }
 
+/**
+ * amdgpu_vram_mgr_intersects - test each drm buddy block for intersection
+ *
+ * @man: TTM memory type manager
+ * @res: The resource to test
+ * @place: The place to test against
+ * @size: Size of the new allocation
+ *
+ * Test each drm buddy block for intersection for eviction decision.
+ */
+static bool amdgpu_vram_mgr_intersects(struct ttm_resource_manager *man,
+				       struct ttm_resource *res,
+				       const struct ttm_place *place,
+				       size_t size)
+{
+	struct amdgpu_vram_mgr_resource *mgr = to_amdgpu_vram_mgr_resource(res);
+	struct drm_buddy_block *block;
+
+	/* Check each drm buddy block individually */
+	list_for_each_entry(block, &mgr->blocks, link) {
+		unsigned long fpfn =
+			amdgpu_vram_mgr_block_start(block) >> PAGE_SHIFT;
+		unsigned long lpfn = fpfn +
+			(amdgpu_vram_mgr_block_size(block) >> PAGE_SHIFT);
+
+		if (place->fpfn < lpfn &&
+		    (place->lpfn && place->lpfn > fpfn))
+			return true;
+	}
+
+	return false;
+}
+
+/**
+ * amdgpu_vram_mgr_compatible - test each drm buddy block for compatibility
+ *
+ * @man: TTM memory type manager
+ * @res: The resource to test
+ * @place: The place to test against
+ * @size: Size of the new allocation
+ *
+ * Test each drm buddy block for placement compatibility.
+ */
+static bool amdgpu_vram_mgr_compatible(struct ttm_resource_manager *man,
+				       struct ttm_resource *res,
+				       const struct ttm_place *place,
+				       size_t size)
+{
+	struct amdgpu_vram_mgr_resource *mgr = to_amdgpu_vram_mgr_resource(res);
+	struct drm_buddy_block *block;
+
+	/* Check each drm buddy block individually */
+	list_for_each_entry(block, &mgr->blocks, link) {
+		unsigned long fpfn =
+			amdgpu_vram_mgr_block_start(block) >> PAGE_SHIFT;
+		unsigned long lpfn = fpfn +
+			(amdgpu_vram_mgr_block_size(block) >> PAGE_SHIFT);
+
+		if (fpfn < place->fpfn ||
+		    (place->lpfn && lpfn > place->lpfn))
+			return false;
+	}
+
+	return true;
+}
+
 /**
  * amdgpu_vram_mgr_debug - dump VRAM table
  *
@@ -753,6 +819,8 @@ static void amdgpu_vram_mgr_debug(struct ttm_resource_manager *man,
 static const struct ttm_resource_manager_func amdgpu_vram_mgr_func = {
 	.alloc	= amdgpu_vram_mgr_new,
 	.free	= amdgpu_vram_mgr_del,
+	.intersects = amdgpu_vram_mgr_intersects,
+	.compatible = amdgpu_vram_mgr_compatible,
 	.debug	= amdgpu_vram_mgr_debug
 };
 
-- 
2.25.1

[PATCH v6 4/6] drm/i915: Implement intersect/compatible functions

[Arunpravin Paneer Selvam]

Implemented a new intersect and compatible callback function
fetching start offset from drm buddy allocator.

v3: move the bits that are specific to buddy_man (Matthew)
v4: consider the block size /range (Matthew)

Signed-off-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_ttm.c       | 41 +----------
 drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 73 +++++++++++++++++++
 2 files changed, 74 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 5a5cf332d8a5..bc9c432edffe 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -361,7 +361,6 @@ static bool i915_ttm_eviction_valuable(struct ttm_buffer_object *bo,
 				       const struct ttm_place *place)
 {
 	struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
-	struct ttm_resource *res = bo->resource;
 
 	if (!obj)
 		return false;
@@ -378,45 +377,7 @@ static bool i915_ttm_eviction_valuable(struct ttm_buffer_object *bo,
 	if (!i915_gem_object_evictable(obj))
 		return false;
 
-	switch (res->mem_type) {
-	case I915_PL_LMEM0: {
-		struct ttm_resource_manager *man =
-			ttm_manager_type(bo->bdev, res->mem_type);
-		struct i915_ttm_buddy_resource *bman_res =
-			to_ttm_buddy_resource(res);
-		struct drm_buddy *mm = bman_res->mm;
-		struct drm_buddy_block *block;
-
-		if (!place->fpfn && !place->lpfn)
-			return true;
-
-		GEM_BUG_ON(!place->lpfn);
-
-		/*
-		 * If we just want something mappable then we can quickly check
-		 * if the current victim resource is using any of the CPU
-		 * visible portion.
-		 */
-		if (!place->fpfn &&
-		    place->lpfn == i915_ttm_buddy_man_visible_size(man))
-			return bman_res->used_visible_size > 0;
-
-		/* Real range allocation */
-		list_for_each_entry(block, &bman_res->blocks, link) {
-			unsigned long fpfn =
-				drm_buddy_block_offset(block) >> PAGE_SHIFT;
-			unsigned long lpfn = fpfn +
-				(drm_buddy_block_size(mm, block) >> PAGE_SHIFT);
-
-			if (place->fpfn < lpfn && place->lpfn > fpfn)
-				return true;
-		}
-		return false;
-	} default:
-		break;
-	}
-
-	return true;
+	return ttm_bo_eviction_valuable(bo, place);
 }
 
 static void i915_ttm_evict_flags(struct ttm_buffer_object *bo,
diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
index 427de1aaab36..e19452f0e100 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
@@ -173,6 +173,77 @@ static void i915_ttm_buddy_man_free(struct ttm_resource_manager *man,
 	kfree(bman_res);
 }
 
+static bool i915_ttm_buddy_man_intersects(struct ttm_resource_manager *man,
+					  struct ttm_resource *res,
+					  const struct ttm_place *place,
+					  size_t size)
+{
+	struct i915_ttm_buddy_resource *bman_res = to_ttm_buddy_resource(res);
+	struct i915_ttm_buddy_manager *bman = to_buddy_manager(man);
+	struct drm_buddy *mm = &bman->mm;
+	struct drm_buddy_block *block;
+
+	if (!place->fpfn && !place->lpfn)
+		return true;
+
+	GEM_BUG_ON(!place->lpfn);
+
+	/*
+	 * If we just want something mappable then we can quickly check
+	 * if the current victim resource is using any of the CPU
+	 * visible portion.
+	 */
+	if (!place->fpfn &&
+	    place->lpfn == i915_ttm_buddy_man_visible_size(man))
+		return bman_res->used_visible_size > 0;
+
+	/* Check each drm buddy block individually */
+	list_for_each_entry(block, &bman_res->blocks, link) {
+		unsigned long fpfn =
+			drm_buddy_block_offset(block) >> PAGE_SHIFT;
+		unsigned long lpfn = fpfn +
+			(drm_buddy_block_size(mm, block) >> PAGE_SHIFT);
+
+		if (place->fpfn < lpfn && place->lpfn > fpfn)
+			return true;
+	}
+
+	return false;
+}
+
+static bool i915_ttm_buddy_man_compatible(struct ttm_resource_manager *man,
+					  struct ttm_resource *res,
+					  const struct ttm_place *place,
+					  size_t size)
+{
+	struct i915_ttm_buddy_resource *bman_res = to_ttm_buddy_resource(res);
+	struct i915_ttm_buddy_manager *bman = to_buddy_manager(man);
+	struct drm_buddy *mm = &bman->mm;
+	struct drm_buddy_block *block;
+
+	if (!place->fpfn && !place->lpfn)
+		return true;
+
+	GEM_BUG_ON(!place->lpfn);
+
+	if (!place->fpfn &&
+	    place->lpfn == i915_ttm_buddy_man_visible_size(man))
+		return bman_res->used_visible_size == res->num_pages;
+
+	/* Check each drm buddy block individually */
+	list_for_each_entry(block, &bman_res->blocks, link) {
+		unsigned long fpfn =
+			drm_buddy_block_offset(block) >> PAGE_SHIFT;
+		unsigned long lpfn = fpfn +
+			(drm_buddy_block_size(mm, block) >> PAGE_SHIFT);
+
+		if (fpfn < place->fpfn || lpfn > place->lpfn)
+			return false;
+	}
+
+	return true;
+}
+
 static void i915_ttm_buddy_man_debug(struct ttm_resource_manager *man,
 				     struct drm_printer *printer)
 {
@@ -200,6 +271,8 @@ static void i915_ttm_buddy_man_debug(struct ttm_resource_manager *man,
 static const struct ttm_resource_manager_func i915_ttm_buddy_manager_func = {
 	.alloc = i915_ttm_buddy_man_alloc,
 	.free = i915_ttm_buddy_man_free,
+	.intersects = i915_ttm_buddy_man_intersects,
+	.compatible = i915_ttm_buddy_man_compatible,
 	.debug = i915_ttm_buddy_man_debug,
 };
 
-- 
2.25.1

[PATCH v6 5/6] drm/nouveau: Implement intersect/compatible functions

[Arunpravin Paneer Selvam]

Implemented a new intersect and compatible callback function
fetching the start offset from struct ttm_resource.

Signed-off-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
---
 drivers/gpu/drm/nouveau/nouveau_mem.c | 29 +++++++++++++++++++++++++++
 drivers/gpu/drm/nouveau/nouveau_mem.h |  6 ++++++
 drivers/gpu/drm/nouveau/nouveau_ttm.c | 24 ++++++++++++++++++++++
 3 files changed, 59 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_mem.c b/drivers/gpu/drm/nouveau/nouveau_mem.c
index 2e517cdc24c9..76f8edefa637 100644
--- a/drivers/gpu/drm/nouveau/nouveau_mem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_mem.c
@@ -187,3 +187,32 @@ nouveau_mem_new(struct nouveau_cli *cli, u8 kind, u8 comp,
 	*res = &mem->base;
 	return 0;
 }
+
+bool
+nouveau_mem_intersects(struct ttm_resource *res,
+		       const struct ttm_place *place,
+		       size_t size)
+{
+	u32 num_pages = PFN_UP(size);
+
+	/* Don't evict BOs outside of the requested placement range */
+	if (place->fpfn >= (res->start + num_pages) ||
+	    (place->lpfn && place->lpfn <= res->start))
+		return false;
+
+	return true;
+}
+
+bool
+nouveau_mem_compatible(struct ttm_resource *res,
+		       const struct ttm_place *place,
+		       size_t size)
+{
+	u32 num_pages = PFN_UP(size);
+
+	if (res->start < place->fpfn ||
+	    (place->lpfn && (res->start + num_pages) > place->lpfn))
+		return false;
+
+	return true;
+}
diff --git a/drivers/gpu/drm/nouveau/nouveau_mem.h b/drivers/gpu/drm/nouveau/nouveau_mem.h
index 325551eba5cd..1ee6cdb9ad9b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_mem.h
+++ b/drivers/gpu/drm/nouveau/nouveau_mem.h
@@ -25,6 +25,12 @@ int nouveau_mem_new(struct nouveau_cli *, u8 kind, u8 comp,
 		    struct ttm_resource **);
 void nouveau_mem_del(struct ttm_resource_manager *man,
 		     struct ttm_resource *);
+bool nouveau_mem_intersects(struct ttm_resource *res,
+			    const struct ttm_place *place,
+			    size_t size);
+bool nouveau_mem_compatible(struct ttm_resource *res,
+			    const struct ttm_place *place,
+			    size_t size);
 int nouveau_mem_vram(struct ttm_resource *, bool contig, u8 page);
 int nouveau_mem_host(struct ttm_resource *, struct ttm_tt *);
 void nouveau_mem_fini(struct nouveau_mem *);
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index 85f1f5a0fe5d..9602c30928f2 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -42,6 +42,24 @@ nouveau_manager_del(struct ttm_resource_manager *man,
 	nouveau_mem_del(man, reg);
 }
 
+static bool
+nouveau_manager_intersects(struct ttm_resource_manager *man,
+			   struct ttm_resource *res,
+			   const struct ttm_place *place,
+			   size_t size)
+{
+	return nouveau_mem_intersects(res, place, size);
+}
+
+static bool
+nouveau_manager_compatible(struct ttm_resource_manager *man,
+			   struct ttm_resource *res,
+			   const struct ttm_place *place,
+			   size_t size)
+{
+	return nouveau_mem_compatible(res, place, size);
+}
+
 static int
 nouveau_vram_manager_new(struct ttm_resource_manager *man,
 			 struct ttm_buffer_object *bo,
@@ -73,6 +91,8 @@ nouveau_vram_manager_new(struct ttm_resource_manager *man,
 const struct ttm_resource_manager_func nouveau_vram_manager = {
 	.alloc = nouveau_vram_manager_new,
 	.free = nouveau_manager_del,
+	.intersects = nouveau_manager_intersects,
+	.compatible = nouveau_manager_compatible,
 };
 
 static int
@@ -97,6 +117,8 @@ nouveau_gart_manager_new(struct ttm_resource_manager *man,
 const struct ttm_resource_manager_func nouveau_gart_manager = {
 	.alloc = nouveau_gart_manager_new,
 	.free = nouveau_manager_del,
+	.intersects = nouveau_manager_intersects,
+	.compatible = nouveau_manager_compatible,
 };
 
 static int
@@ -130,6 +152,8 @@ nv04_gart_manager_new(struct ttm_resource_manager *man,
 const struct ttm_resource_manager_func nv04_gart_manager = {
 	.alloc = nv04_gart_manager_new,
 	.free = nouveau_manager_del,
+	.intersects = nouveau_manager_intersects,
+	.compatible = nouveau_manager_compatible,
 };
 
 static int
-- 
2.25.1

[PATCH v6 6/6] drm/ttm: Switch to using the new res callback

[Arunpravin Paneer Selvam]

Apply new intersect and compatible callback instead
of having a generic placement range verfications.

v2: Added a separate callback for compatiblilty
    checks (Christian)
v3: Cleanups and removal of workarounds

Signed-off-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 45 +++++++------------------
 drivers/gpu/drm/ttm/ttm_resource.c      | 17 ++--------
 2 files changed, 15 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 170935c294f5..7d25a10395c0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1328,11 +1328,12 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct amdgpu_device *adev, struct ttm_tt *ttm,
 static bool amdgpu_ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
 					    const struct ttm_place *place)
 {
-	unsigned long num_pages = bo->resource->num_pages;
 	struct dma_resv_iter resv_cursor;
-	struct amdgpu_res_cursor cursor;
 	struct dma_fence *f;
 
+	if (!amdgpu_bo_is_amdgpu_bo(bo))
+		return ttm_bo_eviction_valuable(bo, place);
+
 	/* Swapout? */
 	if (bo->resource->mem_type == TTM_PL_SYSTEM)
 		return true;
@@ -1351,40 +1352,20 @@ static bool amdgpu_ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
 			return false;
 	}
 
-	switch (bo->resource->mem_type) {
-	case AMDGPU_PL_PREEMPT:
-		/* Preemptible BOs don't own system resources managed by the
-		 * driver (pages, VRAM, GART space). They point to resources
-		 * owned by someone else (e.g. pageable memory in user mode
-		 * or a DMABuf). They are used in a preemptible context so we
-		 * can guarantee no deadlocks and good QoS in case of MMU
-		 * notifiers or DMABuf move notifiers from the resource owner.
-		 */
+	/* Preemptible BOs don't own system resources managed by the
+	 * driver (pages, VRAM, GART space). They point to resources
+	 * owned by someone else (e.g. pageable memory in user mode
+	 * or a DMABuf). They are used in a preemptible context so we
+	 * can guarantee no deadlocks and good QoS in case of MMU
+	 * notifiers or DMABuf move notifiers from the resource owner.
+	 */
+	if (bo->resource->mem_type == AMDGPU_PL_PREEMPT)
 		return false;
-	case TTM_PL_TT:
-		if (amdgpu_bo_is_amdgpu_bo(bo) &&
-		    amdgpu_bo_encrypted(ttm_to_amdgpu_bo(bo)))
-			return false;
-		return true;
 
-	case TTM_PL_VRAM:
-		/* Check each drm MM node individually */
-		amdgpu_res_first(bo->resource, 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;
-
-			amdgpu_res_next(&cursor, cursor.size);
-		}
+	if (bo->resource->mem_type == TTM_PL_TT &&
+	    amdgpu_bo_encrypted(ttm_to_amdgpu_bo(bo)))
 		return false;
 
-	default:
-		break;
-	}
-
 	return ttm_bo_eviction_valuable(bo, place);
 }
 
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index 0d1f862a582b..a729c32a1e48 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -276,17 +276,9 @@ bool ttm_resource_intersects(struct ttm_device *bdev,
 	if (!res)
 		return false;
 
-	if (!place)
-		return true;
-
 	man = ttm_manager_type(bdev, res->mem_type);
-	if (!man->func->intersects) {
-		if (place->fpfn >= (res->start + res->num_pages) ||
-		    (place->lpfn && place->lpfn <= res->start))
-			return false;
-
+	if (!place || !man->func->intersects)
 		return true;
-	}
 
 	return man->func->intersects(man, res, place, size);
 }
@@ -314,13 +306,8 @@ bool ttm_resource_compatible(struct ttm_device *bdev,
 		return false;
 
 	man = ttm_manager_type(bdev, res->mem_type);
-	if (!man->func->compatible) {
-		if (res->start < place->fpfn ||
-		    (place->lpfn && (res->start + res->num_pages) > place->lpfn))
-			return false;
-
+	if (!man->func->compatible)
 		return true;
-	}
 
 	return man->func->compatible(man, res, place, size);
 }
-- 
2.25.1

Re: [PATCH v6 1/6] drm/ttm: Add new callbacks to ttm res mgr

[Christian König]

Am 12.08.22 um 15:30 schrieb Arunpravin Paneer Selvam:
> We are adding two new callbacks to ttm resource manager
> function to handle intersection and compatibility of
> placement and resources.
>
> v2: move the amdgpu and ttm_range_manager changes to
>      separate patches (Christian)
> v3: rename "intersect" to "intersects" (Matthew)
> v4: move !place check to the !res if and return false
>      in ttm_resource_compatible() function (Christian)
> v5: move bits of code from patch number 6 to avoid
>      temporary driver breakup (Christian)
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>

Patch #6 could still be cleaned up more now that we have the workaround 
code in patch #1, but that not really a must have.

Reviewed-by: Christian König <christian.koenig@amd.com> for the entire 
series.

Do you already have commit rights?

Regards,
Christian.

> ---
>   drivers/gpu/drm/ttm/ttm_bo.c       |  9 ++--
>   drivers/gpu/drm/ttm/ttm_resource.c | 77 +++++++++++++++++++++++++++++-
>   include/drm/ttm/ttm_resource.h     | 40 ++++++++++++++++
>   3 files changed, 119 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index c1bd006a5525..f066e8124c50 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -518,6 +518,9 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
>   bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
>   			      const struct ttm_place *place)
>   {
> +	struct ttm_resource *res = bo->resource;
> +	struct ttm_device *bdev = bo->bdev;
> +
>   	dma_resv_assert_held(bo->base.resv);
>   	if (bo->resource->mem_type == TTM_PL_SYSTEM)
>   		return true;
> @@ -525,11 +528,7 @@ bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
>   	/* Don't evict this BO if it's outside of the
>   	 * requested placement range
>   	 */
> -	if (place->fpfn >= (bo->resource->start + bo->resource->num_pages) ||
> -	    (place->lpfn && place->lpfn <= bo->resource->start))
> -		return false;
> -
> -	return true;
> +	return ttm_resource_intersects(bdev, res, place, bo->base.size);
>   }
>   EXPORT_SYMBOL(ttm_bo_eviction_valuable);
>   
> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
> index 20f9adcc3235..0d1f862a582b 100644
> --- a/drivers/gpu/drm/ttm/ttm_resource.c
> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> @@ -253,10 +253,84 @@ void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res)
>   }
>   EXPORT_SYMBOL(ttm_resource_free);
>   
> +/**
> + * ttm_resource_intersects - test for intersection
> + *
> + * @bdev: TTM device structure
> + * @res: The resource to test
> + * @place: The placement to test
> + * @size: How many bytes the new allocation needs.
> + *
> + * Test if @res intersects with @place and @size. Used for testing if evictions
> + * are valueable or not.
> + *
> + * Returns true if the res placement intersects with @place and @size.
> + */
> +bool ttm_resource_intersects(struct ttm_device *bdev,
> +			     struct ttm_resource *res,
> +			     const struct ttm_place *place,
> +			     size_t size)
> +{
> +	struct ttm_resource_manager *man;
> +
> +	if (!res)
> +		return false;
> +
> +	if (!place)
> +		return true;
> +
> +	man = ttm_manager_type(bdev, res->mem_type);
> +	if (!man->func->intersects) {
> +		if (place->fpfn >= (res->start + res->num_pages) ||
> +		    (place->lpfn && place->lpfn <= res->start))
> +			return false;
> +
> +		return true;
> +	}
> +
> +	return man->func->intersects(man, res, place, size);
> +}
> +
> +/**
> + * ttm_resource_compatible - test for compatibility
> + *
> + * @bdev: TTM device structure
> + * @res: The resource to test
> + * @place: The placement to test
> + * @size: How many bytes the new allocation needs.
> + *
> + * Test if @res compatible with @place and @size.
> + *
> + * Returns true if the res placement compatible with @place and @size.
> + */
> +bool ttm_resource_compatible(struct ttm_device *bdev,
> +			     struct ttm_resource *res,
> +			     const struct ttm_place *place,
> +			     size_t size)
> +{
> +	struct ttm_resource_manager *man;
> +
> +	if (!res || !place)
> +		return false;
> +
> +	man = ttm_manager_type(bdev, res->mem_type);
> +	if (!man->func->compatible) {
> +		if (res->start < place->fpfn ||
> +		    (place->lpfn && (res->start + res->num_pages) > place->lpfn))
> +			return false;
> +
> +		return true;
> +	}
> +
> +	return man->func->compatible(man, res, place, size);
> +}
> +
>   static bool ttm_resource_places_compat(struct ttm_resource *res,
>   				       const struct ttm_place *places,
>   				       unsigned num_placement)
>   {
> +	struct ttm_buffer_object *bo = res->bo;
> +	struct ttm_device *bdev = bo->bdev;
>   	unsigned i;
>   
>   	if (res->placement & TTM_PL_FLAG_TEMPORARY)
> @@ -265,8 +339,7 @@ static bool ttm_resource_places_compat(struct ttm_resource *res,
>   	for (i = 0; i < num_placement; i++) {
>   		const struct ttm_place *heap = &places[i];
>   
> -		if (res->start < heap->fpfn || (heap->lpfn &&
> -		    (res->start + res->num_pages) > heap->lpfn))
> +		if (!ttm_resource_compatible(bdev, res, heap, bo->base.size))
>   			continue;
>   
>   		if ((res->mem_type == heap->mem_type) &&
> diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
> index ca89a48c2460..5afc6d664fde 100644
> --- a/include/drm/ttm/ttm_resource.h
> +++ b/include/drm/ttm/ttm_resource.h
> @@ -88,6 +88,38 @@ struct ttm_resource_manager_func {
>   	void (*free)(struct ttm_resource_manager *man,
>   		     struct ttm_resource *res);
>   
> +	/**
> +	 * struct ttm_resource_manager_func member intersects
> +	 *
> +	 * @man: Pointer to a memory type manager.
> +	 * @res: Pointer to a struct ttm_resource to be checked.
> +	 * @place: Placement to check against.
> +	 * @size: Size of the check.
> +	 *
> +	 * Test if @res intersects with @place + @size. Used to judge if
> +	 * evictions are valueable or not.
> +	 */
> +	bool (*intersects)(struct ttm_resource_manager *man,
> +			   struct ttm_resource *res,
> +			   const struct ttm_place *place,
> +			   size_t size);
> +
> +	/**
> +	 * struct ttm_resource_manager_func member compatible
> +	 *
> +	 * @man: Pointer to a memory type manager.
> +	 * @res: Pointer to a struct ttm_resource to be checked.
> +	 * @place: Placement to check against.
> +	 * @size: Size of the check.
> +	 *
> +	 * Test if @res compatible with @place + @size. Used to check of
> +	 * the need to move the backing store or not.
> +	 */
> +	bool (*compatible)(struct ttm_resource_manager *man,
> +			   struct ttm_resource *res,
> +			   const struct ttm_place *place,
> +			   size_t size);
> +
>   	/**
>   	 * struct ttm_resource_manager_func member debug
>   	 *
> @@ -329,6 +361,14 @@ int ttm_resource_alloc(struct ttm_buffer_object *bo,
>   		       const struct ttm_place *place,
>   		       struct ttm_resource **res);
>   void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res);
> +bool ttm_resource_intersects(struct ttm_device *bdev,
> +			     struct ttm_resource *res,
> +			     const struct ttm_place *place,
> +			     size_t size);
> +bool ttm_resource_compatible(struct ttm_device *bdev,
> +			     struct ttm_resource *res,
> +			     const struct ttm_place *place,
> +			     size_t size);
>   bool ttm_resource_compat(struct ttm_resource *res,
>   			 struct ttm_placement *placement);
>   void ttm_resource_set_bo(struct ttm_resource *res,
>
> base-commit: 730c2bf4ad395acf0aa0820535fdb8ea6abe5df1

Re: [PATCH v6 1/6] drm/ttm: Add new callbacks to ttm res mgr

[Arunpravin Paneer Selvam]

On 8/15/2022 4:35 PM, Christian König wrote:
> Am 12.08.22 um 15:30 schrieb Arunpravin Paneer Selvam:
>> We are adding two new callbacks to ttm resource manager
>> function to handle intersection and compatibility of
>> placement and resources.
>>
>> v2: move the amdgpu and ttm_range_manager changes to
>>      separate patches (Christian)
>> v3: rename "intersect" to "intersects" (Matthew)
>> v4: move !place check to the !res if and return false
>>      in ttm_resource_compatible() function (Christian)
>> v5: move bits of code from patch number 6 to avoid
>>      temporary driver breakup (Christian)
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> Signed-off-by: Arunpravin Paneer Selvam 
>> <Arunpravin.PaneerSelvam@amd.com>
>
> Patch #6 could still be cleaned up more now that we have the 
> workaround code in patch #1, but that not really a must have.
>
> Reviewed-by: Christian König <christian.koenig@amd.com> for the entire 
> series.
>
> Do you already have commit rights?

Hi Christian,
I applied for drm-misc commit rights, waiting for the project 
maintainers to approve my request.

Thanks,
Arun
>
> Regards,
> Christian.
>
>> ---
>>   drivers/gpu/drm/ttm/ttm_bo.c       |  9 ++--
>>   drivers/gpu/drm/ttm/ttm_resource.c | 77 +++++++++++++++++++++++++++++-
>>   include/drm/ttm/ttm_resource.h     | 40 ++++++++++++++++
>>   3 files changed, 119 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>> index c1bd006a5525..f066e8124c50 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -518,6 +518,9 @@ static int ttm_bo_evict(struct ttm_buffer_object 
>> *bo,
>>   bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
>>                     const struct ttm_place *place)
>>   {
>> +    struct ttm_resource *res = bo->resource;
>> +    struct ttm_device *bdev = bo->bdev;
>> +
>>       dma_resv_assert_held(bo->base.resv);
>>       if (bo->resource->mem_type == TTM_PL_SYSTEM)
>>           return true;
>> @@ -525,11 +528,7 @@ bool ttm_bo_eviction_valuable(struct 
>> ttm_buffer_object *bo,
>>       /* Don't evict this BO if it's outside of the
>>        * requested placement range
>>        */
>> -    if (place->fpfn >= (bo->resource->start + 
>> bo->resource->num_pages) ||
>> -        (place->lpfn && place->lpfn <= bo->resource->start))
>> -        return false;
>> -
>> -    return true;
>> +    return ttm_resource_intersects(bdev, res, place, bo->base.size);
>>   }
>>   EXPORT_SYMBOL(ttm_bo_eviction_valuable);
>>   diff --git a/drivers/gpu/drm/ttm/ttm_resource.c 
>> b/drivers/gpu/drm/ttm/ttm_resource.c
>> index 20f9adcc3235..0d1f862a582b 100644
>> --- a/drivers/gpu/drm/ttm/ttm_resource.c
>> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
>> @@ -253,10 +253,84 @@ void ttm_resource_free(struct ttm_buffer_object 
>> *bo, struct ttm_resource **res)
>>   }
>>   EXPORT_SYMBOL(ttm_resource_free);
>>   +/**
>> + * ttm_resource_intersects - test for intersection
>> + *
>> + * @bdev: TTM device structure
>> + * @res: The resource to test
>> + * @place: The placement to test
>> + * @size: How many bytes the new allocation needs.
>> + *
>> + * Test if @res intersects with @place and @size. Used for testing 
>> if evictions
>> + * are valueable or not.
>> + *
>> + * Returns true if the res placement intersects with @place and @size.
>> + */
>> +bool ttm_resource_intersects(struct ttm_device *bdev,
>> +                 struct ttm_resource *res,
>> +                 const struct ttm_place *place,
>> +                 size_t size)
>> +{
>> +    struct ttm_resource_manager *man;
>> +
>> +    if (!res)
>> +        return false;
>> +
>> +    if (!place)
>> +        return true;
>> +
>> +    man = ttm_manager_type(bdev, res->mem_type);
>> +    if (!man->func->intersects) {
>> +        if (place->fpfn >= (res->start + res->num_pages) ||
>> +            (place->lpfn && place->lpfn <= res->start))
>> +            return false;
>> +
>> +        return true;
>> +    }
>> +
>> +    return man->func->intersects(man, res, place, size);
>> +}
>> +
>> +/**
>> + * ttm_resource_compatible - test for compatibility
>> + *
>> + * @bdev: TTM device structure
>> + * @res: The resource to test
>> + * @place: The placement to test
>> + * @size: How many bytes the new allocation needs.
>> + *
>> + * Test if @res compatible with @place and @size.
>> + *
>> + * Returns true if the res placement compatible with @place and @size.
>> + */
>> +bool ttm_resource_compatible(struct ttm_device *bdev,
>> +                 struct ttm_resource *res,
>> +                 const struct ttm_place *place,
>> +                 size_t size)
>> +{
>> +    struct ttm_resource_manager *man;
>> +
>> +    if (!res || !place)
>> +        return false;
>> +
>> +    man = ttm_manager_type(bdev, res->mem_type);
>> +    if (!man->func->compatible) {
>> +        if (res->start < place->fpfn ||
>> +            (place->lpfn && (res->start + res->num_pages) > 
>> place->lpfn))
>> +            return false;
>> +
>> +        return true;
>> +    }
>> +
>> +    return man->func->compatible(man, res, place, size);
>> +}
>> +
>>   static bool ttm_resource_places_compat(struct ttm_resource *res,
>>                          const struct ttm_place *places,
>>                          unsigned num_placement)
>>   {
>> +    struct ttm_buffer_object *bo = res->bo;
>> +    struct ttm_device *bdev = bo->bdev;
>>       unsigned i;
>>         if (res->placement & TTM_PL_FLAG_TEMPORARY)
>> @@ -265,8 +339,7 @@ static bool ttm_resource_places_compat(struct 
>> ttm_resource *res,
>>       for (i = 0; i < num_placement; i++) {
>>           const struct ttm_place *heap = &places[i];
>>   -        if (res->start < heap->fpfn || (heap->lpfn &&
>> -            (res->start + res->num_pages) > heap->lpfn))
>> +        if (!ttm_resource_compatible(bdev, res, heap, bo->base.size))
>>               continue;
>>             if ((res->mem_type == heap->mem_type) &&
>> diff --git a/include/drm/ttm/ttm_resource.h 
>> b/include/drm/ttm/ttm_resource.h
>> index ca89a48c2460..5afc6d664fde 100644
>> --- a/include/drm/ttm/ttm_resource.h
>> +++ b/include/drm/ttm/ttm_resource.h
>> @@ -88,6 +88,38 @@ struct ttm_resource_manager_func {
>>       void (*free)(struct ttm_resource_manager *man,
>>                struct ttm_resource *res);
>>   +    /**
>> +     * struct ttm_resource_manager_func member intersects
>> +     *
>> +     * @man: Pointer to a memory type manager.
>> +     * @res: Pointer to a struct ttm_resource to be checked.
>> +     * @place: Placement to check against.
>> +     * @size: Size of the check.
>> +     *
>> +     * Test if @res intersects with @place + @size. Used to judge if
>> +     * evictions are valueable or not.
>> +     */
>> +    bool (*intersects)(struct ttm_resource_manager *man,
>> +               struct ttm_resource *res,
>> +               const struct ttm_place *place,
>> +               size_t size);
>> +
>> +    /**
>> +     * struct ttm_resource_manager_func member compatible
>> +     *
>> +     * @man: Pointer to a memory type manager.
>> +     * @res: Pointer to a struct ttm_resource to be checked.
>> +     * @place: Placement to check against.
>> +     * @size: Size of the check.
>> +     *
>> +     * Test if @res compatible with @place + @size. Used to check of
>> +     * the need to move the backing store or not.
>> +     */
>> +    bool (*compatible)(struct ttm_resource_manager *man,
>> +               struct ttm_resource *res,
>> +               const struct ttm_place *place,
>> +               size_t size);
>> +
>>       /**
>>        * struct ttm_resource_manager_func member debug
>>        *
>> @@ -329,6 +361,14 @@ int ttm_resource_alloc(struct ttm_buffer_object 
>> *bo,
>>                  const struct ttm_place *place,
>>                  struct ttm_resource **res);
>>   void ttm_resource_free(struct ttm_buffer_object *bo, struct 
>> ttm_resource **res);
>> +bool ttm_resource_intersects(struct ttm_device *bdev,
>> +                 struct ttm_resource *res,
>> +                 const struct ttm_place *place,
>> +                 size_t size);
>> +bool ttm_resource_compatible(struct ttm_device *bdev,
>> +                 struct ttm_resource *res,
>> +                 const struct ttm_place *place,
>> +                 size_t size);
>>   bool ttm_resource_compat(struct ttm_resource *res,
>>                struct ttm_placement *placement);
>>   void ttm_resource_set_bo(struct ttm_resource *res,
>>
>> base-commit: 730c2bf4ad395acf0aa0820535fdb8ea6abe5df1
>

Re: [PATCH v6 1/6] drm/ttm: Add new callbacks to ttm res mgr

[Daniel Vetter]

On Tue, Aug 16, 2022 at 10:33:16AM +0530, Arunpravin Paneer Selvam wrote:
> 
> 
> On 8/15/2022 4:35 PM, Christian König wrote:
> > Am 12.08.22 um 15:30 schrieb Arunpravin Paneer Selvam:
> > > We are adding two new callbacks to ttm resource manager
> > > function to handle intersection and compatibility of
> > > placement and resources.
> > > 
> > > v2: move the amdgpu and ttm_range_manager changes to
> > >      separate patches (Christian)
> > > v3: rename "intersect" to "intersects" (Matthew)
> > > v4: move !place check to the !res if and return false
> > >      in ttm_resource_compatible() function (Christian)
> > > v5: move bits of code from patch number 6 to avoid
> > >      temporary driver breakup (Christian)
> > > 
> > > Signed-off-by: Christian König <christian.koenig@amd.com>
> > > Signed-off-by: Arunpravin Paneer Selvam
> > > <Arunpravin.PaneerSelvam@amd.com>
> > 
> > Patch #6 could still be cleaned up more now that we have the workaround
> > code in patch #1, but that not really a must have.
> > 
> > Reviewed-by: Christian König <christian.koenig@amd.com> for the entire
> > series.
> > 
> > Do you already have commit rights?
> 
> Hi Christian,
> I applied for drm-misc commit rights, waiting for the project maintainers to
> approve my request.

Why do all drivers have to implement the current behaviour? Can we have a
default implementation, which gets called if nothing is set instead?

I'm a bit confused why the bloat here ...

Also please document new callbacks precisely with inline kerneldoc. I know
ttm docs aren't great yet, but they don't get better if we don't start
somewhere. I think the in-depth comments for modeset vfuncs (e.g. in
drm_modeset_helper_vtables.h) are a good standard here.
-Daniel

> 
> Thanks,
> Arun
> > 
> > Regards,
> > Christian.
> > 
> > > ---
> > >   drivers/gpu/drm/ttm/ttm_bo.c       |  9 ++--
> > >   drivers/gpu/drm/ttm/ttm_resource.c | 77 +++++++++++++++++++++++++++++-
> > >   include/drm/ttm/ttm_resource.h     | 40 ++++++++++++++++
> > >   3 files changed, 119 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> > > index c1bd006a5525..f066e8124c50 100644
> > > --- a/drivers/gpu/drm/ttm/ttm_bo.c
> > > +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> > > @@ -518,6 +518,9 @@ static int ttm_bo_evict(struct ttm_buffer_object
> > > *bo,
> > >   bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
> > >                     const struct ttm_place *place)
> > >   {
> > > +    struct ttm_resource *res = bo->resource;
> > > +    struct ttm_device *bdev = bo->bdev;
> > > +
> > >       dma_resv_assert_held(bo->base.resv);
> > >       if (bo->resource->mem_type == TTM_PL_SYSTEM)
> > >           return true;
> > > @@ -525,11 +528,7 @@ bool ttm_bo_eviction_valuable(struct
> > > ttm_buffer_object *bo,
> > >       /* Don't evict this BO if it's outside of the
> > >        * requested placement range
> > >        */
> > > -    if (place->fpfn >= (bo->resource->start +
> > > bo->resource->num_pages) ||
> > > -        (place->lpfn && place->lpfn <= bo->resource->start))
> > > -        return false;
> > > -
> > > -    return true;
> > > +    return ttm_resource_intersects(bdev, res, place, bo->base.size);
> > >   }
> > >   EXPORT_SYMBOL(ttm_bo_eviction_valuable);
> > >   diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
> > > b/drivers/gpu/drm/ttm/ttm_resource.c
> > > index 20f9adcc3235..0d1f862a582b 100644
> > > --- a/drivers/gpu/drm/ttm/ttm_resource.c
> > > +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> > > @@ -253,10 +253,84 @@ void ttm_resource_free(struct
> > > ttm_buffer_object *bo, struct ttm_resource **res)
> > >   }
> > >   EXPORT_SYMBOL(ttm_resource_free);
> > >   +/**
> > > + * ttm_resource_intersects - test for intersection
> > > + *
> > > + * @bdev: TTM device structure
> > > + * @res: The resource to test
> > > + * @place: The placement to test
> > > + * @size: How many bytes the new allocation needs.
> > > + *
> > > + * Test if @res intersects with @place and @size. Used for testing
> > > if evictions
> > > + * are valueable or not.
> > > + *
> > > + * Returns true if the res placement intersects with @place and @size.
> > > + */
> > > +bool ttm_resource_intersects(struct ttm_device *bdev,
> > > +                 struct ttm_resource *res,
> > > +                 const struct ttm_place *place,
> > > +                 size_t size)
> > > +{
> > > +    struct ttm_resource_manager *man;
> > > +
> > > +    if (!res)
> > > +        return false;
> > > +
> > > +    if (!place)
> > > +        return true;
> > > +
> > > +    man = ttm_manager_type(bdev, res->mem_type);
> > > +    if (!man->func->intersects) {
> > > +        if (place->fpfn >= (res->start + res->num_pages) ||
> > > +            (place->lpfn && place->lpfn <= res->start))
> > > +            return false;
> > > +
> > > +        return true;
> > > +    }
> > > +
> > > +    return man->func->intersects(man, res, place, size);
> > > +}
> > > +
> > > +/**
> > > + * ttm_resource_compatible - test for compatibility
> > > + *
> > > + * @bdev: TTM device structure
> > > + * @res: The resource to test
> > > + * @place: The placement to test
> > > + * @size: How many bytes the new allocation needs.
> > > + *
> > > + * Test if @res compatible with @place and @size.
> > > + *
> > > + * Returns true if the res placement compatible with @place and @size.
> > > + */
> > > +bool ttm_resource_compatible(struct ttm_device *bdev,
> > > +                 struct ttm_resource *res,
> > > +                 const struct ttm_place *place,
> > > +                 size_t size)
> > > +{
> > > +    struct ttm_resource_manager *man;
> > > +
> > > +    if (!res || !place)
> > > +        return false;
> > > +
> > > +    man = ttm_manager_type(bdev, res->mem_type);
> > > +    if (!man->func->compatible) {
> > > +        if (res->start < place->fpfn ||
> > > +            (place->lpfn && (res->start + res->num_pages) >
> > > place->lpfn))
> > > +            return false;
> > > +
> > > +        return true;
> > > +    }
> > > +
> > > +    return man->func->compatible(man, res, place, size);
> > > +}
> > > +
> > >   static bool ttm_resource_places_compat(struct ttm_resource *res,
> > >                          const struct ttm_place *places,
> > >                          unsigned num_placement)
> > >   {
> > > +    struct ttm_buffer_object *bo = res->bo;
> > > +    struct ttm_device *bdev = bo->bdev;
> > >       unsigned i;
> > >         if (res->placement & TTM_PL_FLAG_TEMPORARY)
> > > @@ -265,8 +339,7 @@ static bool ttm_resource_places_compat(struct
> > > ttm_resource *res,
> > >       for (i = 0; i < num_placement; i++) {
> > >           const struct ttm_place *heap = &places[i];
> > >   -        if (res->start < heap->fpfn || (heap->lpfn &&
> > > -            (res->start + res->num_pages) > heap->lpfn))
> > > +        if (!ttm_resource_compatible(bdev, res, heap, bo->base.size))
> > >               continue;
> > >             if ((res->mem_type == heap->mem_type) &&
> > > diff --git a/include/drm/ttm/ttm_resource.h
> > > b/include/drm/ttm/ttm_resource.h
> > > index ca89a48c2460..5afc6d664fde 100644
> > > --- a/include/drm/ttm/ttm_resource.h
> > > +++ b/include/drm/ttm/ttm_resource.h
> > > @@ -88,6 +88,38 @@ struct ttm_resource_manager_func {
> > >       void (*free)(struct ttm_resource_manager *man,
> > >                struct ttm_resource *res);
> > >   +    /**
> > > +     * struct ttm_resource_manager_func member intersects
> > > +     *
> > > +     * @man: Pointer to a memory type manager.
> > > +     * @res: Pointer to a struct ttm_resource to be checked.
> > > +     * @place: Placement to check against.
> > > +     * @size: Size of the check.
> > > +     *
> > > +     * Test if @res intersects with @place + @size. Used to judge if
> > > +     * evictions are valueable or not.
> > > +     */
> > > +    bool (*intersects)(struct ttm_resource_manager *man,
> > > +               struct ttm_resource *res,
> > > +               const struct ttm_place *place,
> > > +               size_t size);
> > > +
> > > +    /**
> > > +     * struct ttm_resource_manager_func member compatible
> > > +     *
> > > +     * @man: Pointer to a memory type manager.
> > > +     * @res: Pointer to a struct ttm_resource to be checked.
> > > +     * @place: Placement to check against.
> > > +     * @size: Size of the check.
> > > +     *
> > > +     * Test if @res compatible with @place + @size. Used to check of
> > > +     * the need to move the backing store or not.
> > > +     */
> > > +    bool (*compatible)(struct ttm_resource_manager *man,
> > > +               struct ttm_resource *res,
> > > +               const struct ttm_place *place,
> > > +               size_t size);
> > > +
> > >       /**
> > >        * struct ttm_resource_manager_func member debug
> > >        *
> > > @@ -329,6 +361,14 @@ int ttm_resource_alloc(struct ttm_buffer_object
> > > *bo,
> > >                  const struct ttm_place *place,
> > >                  struct ttm_resource **res);
> > >   void ttm_resource_free(struct ttm_buffer_object *bo, struct
> > > ttm_resource **res);
> > > +bool ttm_resource_intersects(struct ttm_device *bdev,
> > > +                 struct ttm_resource *res,
> > > +                 const struct ttm_place *place,
> > > +                 size_t size);
> > > +bool ttm_resource_compatible(struct ttm_device *bdev,
> > > +                 struct ttm_resource *res,
> > > +                 const struct ttm_place *place,
> > > +                 size_t size);
> > >   bool ttm_resource_compat(struct ttm_resource *res,
> > >                struct ttm_placement *placement);
> > >   void ttm_resource_set_bo(struct ttm_resource *res,
> > > 
> > > base-commit: 730c2bf4ad395acf0aa0820535fdb8ea6abe5df1
> > 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v6 1/6] drm/ttm: Add new callbacks to ttm res mgr

[Christian König]

Am 06.09.22 um 21:58 schrieb Daniel Vetter:
> On Tue, Aug 16, 2022 at 10:33:16AM +0530, Arunpravin Paneer Selvam wrote:
>>
>> On 8/15/2022 4:35 PM, Christian König wrote:
>>> Am 12.08.22 um 15:30 schrieb Arunpravin Paneer Selvam:
>>>> We are adding two new callbacks to ttm resource manager
>>>> function to handle intersection and compatibility of
>>>> placement and resources.
>>>>
>>>> v2: move the amdgpu and ttm_range_manager changes to
>>>>       separate patches (Christian)
>>>> v3: rename "intersect" to "intersects" (Matthew)
>>>> v4: move !place check to the !res if and return false
>>>>       in ttm_resource_compatible() function (Christian)
>>>> v5: move bits of code from patch number 6 to avoid
>>>>       temporary driver breakup (Christian)
>>>>
>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>> Signed-off-by: Arunpravin Paneer Selvam
>>>> <Arunpravin.PaneerSelvam@amd.com>
>>> Patch #6 could still be cleaned up more now that we have the workaround
>>> code in patch #1, but that not really a must have.
>>>
>>> Reviewed-by: Christian König <christian.koenig@amd.com> for the entire
>>> series.
>>>
>>> Do you already have commit rights?
>> Hi Christian,
>> I applied for drm-misc commit rights, waiting for the project maintainers to
>> approve my request.
> Why do all drivers have to implement the current behaviour? Can we have a
> default implementation, which gets called if nothing is set instead?

We do have a default implementation in the range manager which is used 
by radeon, GEM VRAM helpers, VMWGFX and amdgpu (but there only for some 
domains).

> I'm a bit confused why the bloat here ...

Drivers do have specialized implementations of the backend, e.g. VMWGFX 
have his handle backend, amdgpu the VRAM backend with special 
placements, i915 is completely special as well.

Here we only move the decision if resources intersect or are compatible 
into those specialized backends. Previously we had all this in a 
centralized callback for all backends of a driver.

See the switch in amdgpu_ttm_bo_eviction_valuable() for an example. 
Final goal is to move all this stuff into the specialized backends and 
remove this callback.

The only driver where I couldn't figure out why we have duplicated all 
this from the standard implementation is Nouveau.

> Also please document new callbacks precisely with inline kerneldoc. I know
> ttm docs aren't great yet, but they don't get better if we don't start
> somewhere. I think the in-depth comments for modeset vfuncs (e.g. in
> drm_modeset_helper_vtables.h) are a good standard here.

I thought we already did that. Please be a bit more specific.

Thanks,
Christian.

> -Daniel
>
>> Thanks,
>> Arun
>>> Regards,
>>> Christian.
>>>
>>>> ---
>>>>    drivers/gpu/drm/ttm/ttm_bo.c       |  9 ++--
>>>>    drivers/gpu/drm/ttm/ttm_resource.c | 77 +++++++++++++++++++++++++++++-
>>>>    include/drm/ttm/ttm_resource.h     | 40 ++++++++++++++++
>>>>    3 files changed, 119 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>>>> index c1bd006a5525..f066e8124c50 100644
>>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>>> @@ -518,6 +518,9 @@ static int ttm_bo_evict(struct ttm_buffer_object
>>>> *bo,
>>>>    bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
>>>>                      const struct ttm_place *place)
>>>>    {
>>>> +    struct ttm_resource *res = bo->resource;
>>>> +    struct ttm_device *bdev = bo->bdev;
>>>> +
>>>>        dma_resv_assert_held(bo->base.resv);
>>>>        if (bo->resource->mem_type == TTM_PL_SYSTEM)
>>>>            return true;
>>>> @@ -525,11 +528,7 @@ bool ttm_bo_eviction_valuable(struct
>>>> ttm_buffer_object *bo,
>>>>        /* Don't evict this BO if it's outside of the
>>>>         * requested placement range
>>>>         */
>>>> -    if (place->fpfn >= (bo->resource->start +
>>>> bo->resource->num_pages) ||
>>>> -        (place->lpfn && place->lpfn <= bo->resource->start))
>>>> -        return false;
>>>> -
>>>> -    return true;
>>>> +    return ttm_resource_intersects(bdev, res, place, bo->base.size);
>>>>    }
>>>>    EXPORT_SYMBOL(ttm_bo_eviction_valuable);
>>>>    diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
>>>> b/drivers/gpu/drm/ttm/ttm_resource.c
>>>> index 20f9adcc3235..0d1f862a582b 100644
>>>> --- a/drivers/gpu/drm/ttm/ttm_resource.c
>>>> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
>>>> @@ -253,10 +253,84 @@ void ttm_resource_free(struct
>>>> ttm_buffer_object *bo, struct ttm_resource **res)
>>>>    }
>>>>    EXPORT_SYMBOL(ttm_resource_free);
>>>>    +/**
>>>> + * ttm_resource_intersects - test for intersection
>>>> + *
>>>> + * @bdev: TTM device structure
>>>> + * @res: The resource to test
>>>> + * @place: The placement to test
>>>> + * @size: How many bytes the new allocation needs.
>>>> + *
>>>> + * Test if @res intersects with @place and @size. Used for testing
>>>> if evictions
>>>> + * are valueable or not.
>>>> + *
>>>> + * Returns true if the res placement intersects with @place and @size.
>>>> + */
>>>> +bool ttm_resource_intersects(struct ttm_device *bdev,
>>>> +                 struct ttm_resource *res,
>>>> +                 const struct ttm_place *place,
>>>> +                 size_t size)
>>>> +{
>>>> +    struct ttm_resource_manager *man;
>>>> +
>>>> +    if (!res)
>>>> +        return false;
>>>> +
>>>> +    if (!place)
>>>> +        return true;
>>>> +
>>>> +    man = ttm_manager_type(bdev, res->mem_type);
>>>> +    if (!man->func->intersects) {
>>>> +        if (place->fpfn >= (res->start + res->num_pages) ||
>>>> +            (place->lpfn && place->lpfn <= res->start))
>>>> +            return false;
>>>> +
>>>> +        return true;
>>>> +    }
>>>> +
>>>> +    return man->func->intersects(man, res, place, size);
>>>> +}
>>>> +
>>>> +/**
>>>> + * ttm_resource_compatible - test for compatibility
>>>> + *
>>>> + * @bdev: TTM device structure
>>>> + * @res: The resource to test
>>>> + * @place: The placement to test
>>>> + * @size: How many bytes the new allocation needs.
>>>> + *
>>>> + * Test if @res compatible with @place and @size.
>>>> + *
>>>> + * Returns true if the res placement compatible with @place and @size.
>>>> + */
>>>> +bool ttm_resource_compatible(struct ttm_device *bdev,
>>>> +                 struct ttm_resource *res,
>>>> +                 const struct ttm_place *place,
>>>> +                 size_t size)
>>>> +{
>>>> +    struct ttm_resource_manager *man;
>>>> +
>>>> +    if (!res || !place)
>>>> +        return false;
>>>> +
>>>> +    man = ttm_manager_type(bdev, res->mem_type);
>>>> +    if (!man->func->compatible) {
>>>> +        if (res->start < place->fpfn ||
>>>> +            (place->lpfn && (res->start + res->num_pages) >
>>>> place->lpfn))
>>>> +            return false;
>>>> +
>>>> +        return true;
>>>> +    }
>>>> +
>>>> +    return man->func->compatible(man, res, place, size);
>>>> +}
>>>> +
>>>>    static bool ttm_resource_places_compat(struct ttm_resource *res,
>>>>                           const struct ttm_place *places,
>>>>                           unsigned num_placement)
>>>>    {
>>>> +    struct ttm_buffer_object *bo = res->bo;
>>>> +    struct ttm_device *bdev = bo->bdev;
>>>>        unsigned i;
>>>>          if (res->placement & TTM_PL_FLAG_TEMPORARY)
>>>> @@ -265,8 +339,7 @@ static bool ttm_resource_places_compat(struct
>>>> ttm_resource *res,
>>>>        for (i = 0; i < num_placement; i++) {
>>>>            const struct ttm_place *heap = &places[i];
>>>>    -        if (res->start < heap->fpfn || (heap->lpfn &&
>>>> -            (res->start + res->num_pages) > heap->lpfn))
>>>> +        if (!ttm_resource_compatible(bdev, res, heap, bo->base.size))
>>>>                continue;
>>>>              if ((res->mem_type == heap->mem_type) &&
>>>> diff --git a/include/drm/ttm/ttm_resource.h
>>>> b/include/drm/ttm/ttm_resource.h
>>>> index ca89a48c2460..5afc6d664fde 100644
>>>> --- a/include/drm/ttm/ttm_resource.h
>>>> +++ b/include/drm/ttm/ttm_resource.h
>>>> @@ -88,6 +88,38 @@ struct ttm_resource_manager_func {
>>>>        void (*free)(struct ttm_resource_manager *man,
>>>>                 struct ttm_resource *res);
>>>>    +    /**
>>>> +     * struct ttm_resource_manager_func member intersects
>>>> +     *
>>>> +     * @man: Pointer to a memory type manager.
>>>> +     * @res: Pointer to a struct ttm_resource to be checked.
>>>> +     * @place: Placement to check against.
>>>> +     * @size: Size of the check.
>>>> +     *
>>>> +     * Test if @res intersects with @place + @size. Used to judge if
>>>> +     * evictions are valueable or not.
>>>> +     */
>>>> +    bool (*intersects)(struct ttm_resource_manager *man,
>>>> +               struct ttm_resource *res,
>>>> +               const struct ttm_place *place,
>>>> +               size_t size);
>>>> +
>>>> +    /**
>>>> +     * struct ttm_resource_manager_func member compatible
>>>> +     *
>>>> +     * @man: Pointer to a memory type manager.
>>>> +     * @res: Pointer to a struct ttm_resource to be checked.
>>>> +     * @place: Placement to check against.
>>>> +     * @size: Size of the check.
>>>> +     *
>>>> +     * Test if @res compatible with @place + @size. Used to check of
>>>> +     * the need to move the backing store or not.
>>>> +     */
>>>> +    bool (*compatible)(struct ttm_resource_manager *man,
>>>> +               struct ttm_resource *res,
>>>> +               const struct ttm_place *place,
>>>> +               size_t size);
>>>> +
>>>>        /**
>>>>         * struct ttm_resource_manager_func member debug
>>>>         *
>>>> @@ -329,6 +361,14 @@ int ttm_resource_alloc(struct ttm_buffer_object
>>>> *bo,
>>>>                   const struct ttm_place *place,
>>>>                   struct ttm_resource **res);
>>>>    void ttm_resource_free(struct ttm_buffer_object *bo, struct
>>>> ttm_resource **res);
>>>> +bool ttm_resource_intersects(struct ttm_device *bdev,
>>>> +                 struct ttm_resource *res,
>>>> +                 const struct ttm_place *place,
>>>> +                 size_t size);
>>>> +bool ttm_resource_compatible(struct ttm_device *bdev,
>>>> +                 struct ttm_resource *res,
>>>> +                 const struct ttm_place *place,
>>>> +                 size_t size);
>>>>    bool ttm_resource_compat(struct ttm_resource *res,
>>>>                 struct ttm_placement *placement);
>>>>    void ttm_resource_set_bo(struct ttm_resource *res,
>>>>
>>>> base-commit: 730c2bf4ad395acf0aa0820535fdb8ea6abe5df1

Re: [PATCH v6 1/6] drm/ttm: Add new callbacks to ttm res mgr

[Daniel Vetter]

On Wed, Sep 07, 2022 at 08:45:22AM +0200, Christian König wrote:
> Am 06.09.22 um 21:58 schrieb Daniel Vetter:
> > On Tue, Aug 16, 2022 at 10:33:16AM +0530, Arunpravin Paneer Selvam wrote:
> > > 
> > > On 8/15/2022 4:35 PM, Christian König wrote:
> > > > Am 12.08.22 um 15:30 schrieb Arunpravin Paneer Selvam:
> > > > > We are adding two new callbacks to ttm resource manager
> > > > > function to handle intersection and compatibility of
> > > > > placement and resources.
> > > > > 
> > > > > v2: move the amdgpu and ttm_range_manager changes to
> > > > >       separate patches (Christian)
> > > > > v3: rename "intersect" to "intersects" (Matthew)
> > > > > v4: move !place check to the !res if and return false
> > > > >       in ttm_resource_compatible() function (Christian)
> > > > > v5: move bits of code from patch number 6 to avoid
> > > > >       temporary driver breakup (Christian)
> > > > > 
> > > > > Signed-off-by: Christian König <christian.koenig@amd.com>
> > > > > Signed-off-by: Arunpravin Paneer Selvam
> > > > > <Arunpravin.PaneerSelvam@amd.com>
> > > > Patch #6 could still be cleaned up more now that we have the workaround
> > > > code in patch #1, but that not really a must have.
> > > > 
> > > > Reviewed-by: Christian König <christian.koenig@amd.com> for the entire
> > > > series.
> > > > 
> > > > Do you already have commit rights?
> > > Hi Christian,
> > > I applied for drm-misc commit rights, waiting for the project maintainers to
> > > approve my request.
> > Why do all drivers have to implement the current behaviour? Can we have a
> > default implementation, which gets called if nothing is set instead?
> 
> We do have a default implementation in the range manager which is used by
> radeon, GEM VRAM helpers, VMWGFX and amdgpu (but there only for some
> domains).
> 
> > I'm a bit confused why the bloat here ...
> 
> Drivers do have specialized implementations of the backend, e.g. VMWGFX have
> his handle backend, amdgpu the VRAM backend with special placements, i915 is
> completely special as well.
> 
> Here we only move the decision if resources intersect or are compatible into
> those specialized backends. Previously we had all this in a centralized
> callback for all backends of a driver.
> 
> See the switch in amdgpu_ttm_bo_eviction_valuable() for an example. Final
> goal is to move all this stuff into the specialized backends and remove this
> callback.
> 
> The only driver where I couldn't figure out why we have duplicated all this
> from the standard implementation is Nouveau.

Yeah I didn't read this too carefully, apologies.

> > Also please document new callbacks precisely with inline kerneldoc. I know
> > ttm docs aren't great yet, but they don't get better if we don't start
> > somewhere. I think the in-depth comments for modeset vfuncs (e.g. in
> > drm_modeset_helper_vtables.h) are a good standard here.
> 
> I thought we already did that. Please be a bit more specific.

Yeah rushed this too, but the kerneldoc isn't too great yet. It's
definitely not formatted correctly (you can't do a full function
definition in a struct unfortunately, see the examples I linked). And it
would be good to specificy what the default implementation is, that there
is one (i.e. the hook is optional) and when exactly a driver would want to
overwrite this. Atm it's a one-liner that explains exactly as much as you
can guess from the function interface anyway, that's not super userful.
-Daniel



> 
> Thanks,
> Christian.
> 
> > -Daniel
> > 
> > > Thanks,
> > > Arun
> > > > Regards,
> > > > Christian.
> > > > 
> > > > > ---
> > > > >    drivers/gpu/drm/ttm/ttm_bo.c       |  9 ++--
> > > > >    drivers/gpu/drm/ttm/ttm_resource.c | 77 +++++++++++++++++++++++++++++-
> > > > >    include/drm/ttm/ttm_resource.h     | 40 ++++++++++++++++
> > > > >    3 files changed, 119 insertions(+), 7 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> > > > > index c1bd006a5525..f066e8124c50 100644
> > > > > --- a/drivers/gpu/drm/ttm/ttm_bo.c
> > > > > +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> > > > > @@ -518,6 +518,9 @@ static int ttm_bo_evict(struct ttm_buffer_object
> > > > > *bo,
> > > > >    bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
> > > > >                      const struct ttm_place *place)
> > > > >    {
> > > > > +    struct ttm_resource *res = bo->resource;
> > > > > +    struct ttm_device *bdev = bo->bdev;
> > > > > +
> > > > >        dma_resv_assert_held(bo->base.resv);
> > > > >        if (bo->resource->mem_type == TTM_PL_SYSTEM)
> > > > >            return true;
> > > > > @@ -525,11 +528,7 @@ bool ttm_bo_eviction_valuable(struct
> > > > > ttm_buffer_object *bo,
> > > > >        /* Don't evict this BO if it's outside of the
> > > > >         * requested placement range
> > > > >         */
> > > > > -    if (place->fpfn >= (bo->resource->start +
> > > > > bo->resource->num_pages) ||
> > > > > -        (place->lpfn && place->lpfn <= bo->resource->start))
> > > > > -        return false;
> > > > > -
> > > > > -    return true;
> > > > > +    return ttm_resource_intersects(bdev, res, place, bo->base.size);
> > > > >    }
> > > > >    EXPORT_SYMBOL(ttm_bo_eviction_valuable);
> > > > >    diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
> > > > > b/drivers/gpu/drm/ttm/ttm_resource.c
> > > > > index 20f9adcc3235..0d1f862a582b 100644
> > > > > --- a/drivers/gpu/drm/ttm/ttm_resource.c
> > > > > +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> > > > > @@ -253,10 +253,84 @@ void ttm_resource_free(struct
> > > > > ttm_buffer_object *bo, struct ttm_resource **res)
> > > > >    }
> > > > >    EXPORT_SYMBOL(ttm_resource_free);
> > > > >    +/**
> > > > > + * ttm_resource_intersects - test for intersection
> > > > > + *
> > > > > + * @bdev: TTM device structure
> > > > > + * @res: The resource to test
> > > > > + * @place: The placement to test
> > > > > + * @size: How many bytes the new allocation needs.
> > > > > + *
> > > > > + * Test if @res intersects with @place and @size. Used for testing
> > > > > if evictions
> > > > > + * are valueable or not.
> > > > > + *
> > > > > + * Returns true if the res placement intersects with @place and @size.
> > > > > + */
> > > > > +bool ttm_resource_intersects(struct ttm_device *bdev,
> > > > > +                 struct ttm_resource *res,
> > > > > +                 const struct ttm_place *place,
> > > > > +                 size_t size)
> > > > > +{
> > > > > +    struct ttm_resource_manager *man;
> > > > > +
> > > > > +    if (!res)
> > > > > +        return false;
> > > > > +
> > > > > +    if (!place)
> > > > > +        return true;
> > > > > +
> > > > > +    man = ttm_manager_type(bdev, res->mem_type);
> > > > > +    if (!man->func->intersects) {
> > > > > +        if (place->fpfn >= (res->start + res->num_pages) ||
> > > > > +            (place->lpfn && place->lpfn <= res->start))
> > > > > +            return false;
> > > > > +
> > > > > +        return true;
> > > > > +    }
> > > > > +
> > > > > +    return man->func->intersects(man, res, place, size);
> > > > > +}
> > > > > +
> > > > > +/**
> > > > > + * ttm_resource_compatible - test for compatibility
> > > > > + *
> > > > > + * @bdev: TTM device structure
> > > > > + * @res: The resource to test
> > > > > + * @place: The placement to test
> > > > > + * @size: How many bytes the new allocation needs.
> > > > > + *
> > > > > + * Test if @res compatible with @place and @size.
> > > > > + *
> > > > > + * Returns true if the res placement compatible with @place and @size.
> > > > > + */
> > > > > +bool ttm_resource_compatible(struct ttm_device *bdev,
> > > > > +                 struct ttm_resource *res,
> > > > > +                 const struct ttm_place *place,
> > > > > +                 size_t size)
> > > > > +{
> > > > > +    struct ttm_resource_manager *man;
> > > > > +
> > > > > +    if (!res || !place)
> > > > > +        return false;
> > > > > +
> > > > > +    man = ttm_manager_type(bdev, res->mem_type);
> > > > > +    if (!man->func->compatible) {
> > > > > +        if (res->start < place->fpfn ||
> > > > > +            (place->lpfn && (res->start + res->num_pages) >
> > > > > place->lpfn))
> > > > > +            return false;
> > > > > +
> > > > > +        return true;
> > > > > +    }
> > > > > +
> > > > > +    return man->func->compatible(man, res, place, size);
> > > > > +}
> > > > > +
> > > > >    static bool ttm_resource_places_compat(struct ttm_resource *res,
> > > > >                           const struct ttm_place *places,
> > > > >                           unsigned num_placement)
> > > > >    {
> > > > > +    struct ttm_buffer_object *bo = res->bo;
> > > > > +    struct ttm_device *bdev = bo->bdev;
> > > > >        unsigned i;
> > > > >          if (res->placement & TTM_PL_FLAG_TEMPORARY)
> > > > > @@ -265,8 +339,7 @@ static bool ttm_resource_places_compat(struct
> > > > > ttm_resource *res,
> > > > >        for (i = 0; i < num_placement; i++) {
> > > > >            const struct ttm_place *heap = &places[i];
> > > > >    -        if (res->start < heap->fpfn || (heap->lpfn &&
> > > > > -            (res->start + res->num_pages) > heap->lpfn))
> > > > > +        if (!ttm_resource_compatible(bdev, res, heap, bo->base.size))
> > > > >                continue;
> > > > >              if ((res->mem_type == heap->mem_type) &&
> > > > > diff --git a/include/drm/ttm/ttm_resource.h
> > > > > b/include/drm/ttm/ttm_resource.h
> > > > > index ca89a48c2460..5afc6d664fde 100644
> > > > > --- a/include/drm/ttm/ttm_resource.h
> > > > > +++ b/include/drm/ttm/ttm_resource.h
> > > > > @@ -88,6 +88,38 @@ struct ttm_resource_manager_func {
> > > > >        void (*free)(struct ttm_resource_manager *man,
> > > > >                 struct ttm_resource *res);
> > > > >    +    /**
> > > > > +     * struct ttm_resource_manager_func member intersects
> > > > > +     *
> > > > > +     * @man: Pointer to a memory type manager.
> > > > > +     * @res: Pointer to a struct ttm_resource to be checked.
> > > > > +     * @place: Placement to check against.
> > > > > +     * @size: Size of the check.
> > > > > +     *
> > > > > +     * Test if @res intersects with @place + @size. Used to judge if
> > > > > +     * evictions are valueable or not.
> > > > > +     */
> > > > > +    bool (*intersects)(struct ttm_resource_manager *man,
> > > > > +               struct ttm_resource *res,
> > > > > +               const struct ttm_place *place,
> > > > > +               size_t size);
> > > > > +
> > > > > +    /**
> > > > > +     * struct ttm_resource_manager_func member compatible
> > > > > +     *
> > > > > +     * @man: Pointer to a memory type manager.
> > > > > +     * @res: Pointer to a struct ttm_resource to be checked.
> > > > > +     * @place: Placement to check against.
> > > > > +     * @size: Size of the check.
> > > > > +     *
> > > > > +     * Test if @res compatible with @place + @size. Used to check of
> > > > > +     * the need to move the backing store or not.
> > > > > +     */
> > > > > +    bool (*compatible)(struct ttm_resource_manager *man,
> > > > > +               struct ttm_resource *res,
> > > > > +               const struct ttm_place *place,
> > > > > +               size_t size);
> > > > > +
> > > > >        /**
> > > > >         * struct ttm_resource_manager_func member debug
> > > > >         *
> > > > > @@ -329,6 +361,14 @@ int ttm_resource_alloc(struct ttm_buffer_object
> > > > > *bo,
> > > > >                   const struct ttm_place *place,
> > > > >                   struct ttm_resource **res);
> > > > >    void ttm_resource_free(struct ttm_buffer_object *bo, struct
> > > > > ttm_resource **res);
> > > > > +bool ttm_resource_intersects(struct ttm_device *bdev,
> > > > > +                 struct ttm_resource *res,
> > > > > +                 const struct ttm_place *place,
> > > > > +                 size_t size);
> > > > > +bool ttm_resource_compatible(struct ttm_device *bdev,
> > > > > +                 struct ttm_resource *res,
> > > > > +                 const struct ttm_place *place,
> > > > > +                 size_t size);
> > > > >    bool ttm_resource_compat(struct ttm_resource *res,
> > > > >                 struct ttm_placement *placement);
> > > > >    void ttm_resource_set_bo(struct ttm_resource *res,
> > > > > 
> > > > > base-commit: 730c2bf4ad395acf0aa0820535fdb8ea6abe5df1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v6 1/6] drm/ttm: Add new callbacks to ttm res mgr

[Christian König]

Am 07.09.22 um 19:00 schrieb Daniel Vetter:
> [SNIP]
>>> I'm a bit confused why the bloat here ...
>> Drivers do have specialized implementations of the backend, e.g. VMWGFX have
>> his handle backend, amdgpu the VRAM backend with special placements, i915 is
>> completely special as well.
>>
>> Here we only move the decision if resources intersect or are compatible into
>> those specialized backends. Previously we had all this in a centralized
>> callback for all backends of a driver.
>>
>> See the switch in amdgpu_ttm_bo_eviction_valuable() for an example. Final
>> goal is to move all this stuff into the specialized backends and remove this
>> callback.
>>
>> The only driver where I couldn't figure out why we have duplicated all this
>> from the standard implementation is Nouveau.
> Yeah I didn't read this too carefully, apologies.
>
>>> Also please document new callbacks precisely with inline kerneldoc. I know
>>> ttm docs aren't great yet, but they don't get better if we don't start
>>> somewhere. I think the in-depth comments for modeset vfuncs (e.g. in
>>> drm_modeset_helper_vtables.h) are a good standard here.
>> I thought we already did that. Please be a bit more specific.
> Yeah rushed this too, but the kerneldoc isn't too great yet. It's
> definitely not formatted correctly (you can't do a full function
> definition in a struct unfortunately, see the examples I linked). And it
> would be good to specificy what the default implementation is, that there
> is one (i.e. the hook is optional) and when exactly a driver would want to
> overwrite this. Atm it's a one-liner that explains exactly as much as you
> can guess from the function interface anyway, that's not super userful.
> -Daniel

Arun can you take care of improving this?

Thanks,
Christian.

>
>
>
>> Thanks,
>> Christian.
>>
>>> -Daniel
>>>
>>>> Thanks,
>>>> Arun
>>>>> Regards,
>>>>> Christian.
>>>>>

Re: [PATCH v6 1/6] drm/ttm: Add new callbacks to ttm res mgr

[Arunpravin Paneer Selvam]

On 9/8/2022 11:34 AM, Christian König wrote:
> Am 07.09.22 um 19:00 schrieb Daniel Vetter:
>> [SNIP]
>>>> I'm a bit confused why the bloat here ...
>>> Drivers do have specialized implementations of the backend, e.g. 
>>> VMWGFX have
>>> his handle backend, amdgpu the VRAM backend with special placements, 
>>> i915 is
>>> completely special as well.
>>>
>>> Here we only move the decision if resources intersect or are 
>>> compatible into
>>> those specialized backends. Previously we had all this in a centralized
>>> callback for all backends of a driver.
>>>
>>> See the switch in amdgpu_ttm_bo_eviction_valuable() for an example. 
>>> Final
>>> goal is to move all this stuff into the specialized backends and 
>>> remove this
>>> callback.
>>>
>>> The only driver where I couldn't figure out why we have duplicated 
>>> all this
>>> from the standard implementation is Nouveau.
>> Yeah I didn't read this too carefully, apologies.
>>
>>>> Also please document new callbacks precisely with inline kerneldoc. 
>>>> I know
>>>> ttm docs aren't great yet, but they don't get better if we don't start
>>>> somewhere. I think the in-depth comments for modeset vfuncs (e.g. in
>>>> drm_modeset_helper_vtables.h) are a good standard here.
>>> I thought we already did that. Please be a bit more specific.
>> Yeah rushed this too, but the kerneldoc isn't too great yet. It's
>> definitely not formatted correctly (you can't do a full function
>> definition in a struct unfortunately, see the examples I linked). And it
>> would be good to specificy what the default implementation is, that 
>> there
>> is one (i.e. the hook is optional) and when exactly a driver would 
>> want to
>> overwrite this. Atm it's a one-liner that explains exactly as much as 
>> you
>> can guess from the function interface anyway, that's not super userful.
>> -Daniel
>
> Arun can you take care of improving this?

Hi Christian,
Yes, I will check on this.

Thanks,
Arun
>
> Thanks,
> Christian.
>
>>
>>
>>
>>> Thanks,
>>> Christian.
>>>
>>>> -Daniel
>>>>
>>>>> Thanks,
>>>>> Arun
>>>>>> Regards,
>>>>>> Christian.
>>>>>>
>

Re: [PATCH v6 1/6] drm/ttm: Add new callbacks to ttm res mgr

[Christian König]

Am 20.08.22 um 09:32 schrieb Arunpravin Paneer Selvam:
> We are adding two new callbacks to ttm resource manager
> function to handle intersection and compatibility of
> placement and resources.
>
> v2: move the amdgpu and ttm_range_manager changes to
>      separate patches (Christian)
> v3: rename "intersect" to "intersects" (Matthew)
> v4: move !place check to the !res if and return false
>      in ttm_resource_compatible() function (Christian)
> v5: move bits of code from patch number 6 to avoid
>      temporary driver breakup (Christian)
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
> Reviewed-by: Christian König <christian.koenig@amd.com>

Pushed to drm-misc-next.

Thanks,
Christian.

> ---
>   drivers/gpu/drm/ttm/ttm_bo.c       |  9 ++--
>   drivers/gpu/drm/ttm/ttm_resource.c | 77 +++++++++++++++++++++++++++++-
>   include/drm/ttm/ttm_resource.h     | 40 ++++++++++++++++
>   3 files changed, 119 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index c1bd006a5525..f066e8124c50 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -518,6 +518,9 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
>   bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
>   			      const struct ttm_place *place)
>   {
> +	struct ttm_resource *res = bo->resource;
> +	struct ttm_device *bdev = bo->bdev;
> +
>   	dma_resv_assert_held(bo->base.resv);
>   	if (bo->resource->mem_type == TTM_PL_SYSTEM)
>   		return true;
> @@ -525,11 +528,7 @@ bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
>   	/* Don't evict this BO if it's outside of the
>   	 * requested placement range
>   	 */
> -	if (place->fpfn >= (bo->resource->start + bo->resource->num_pages) ||
> -	    (place->lpfn && place->lpfn <= bo->resource->start))
> -		return false;
> -
> -	return true;
> +	return ttm_resource_intersects(bdev, res, place, bo->base.size);
>   }
>   EXPORT_SYMBOL(ttm_bo_eviction_valuable);
>   
> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
> index 20f9adcc3235..0d1f862a582b 100644
> --- a/drivers/gpu/drm/ttm/ttm_resource.c
> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> @@ -253,10 +253,84 @@ void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res)
>   }
>   EXPORT_SYMBOL(ttm_resource_free);
>   
> +/**
> + * ttm_resource_intersects - test for intersection
> + *
> + * @bdev: TTM device structure
> + * @res: The resource to test
> + * @place: The placement to test
> + * @size: How many bytes the new allocation needs.
> + *
> + * Test if @res intersects with @place and @size. Used for testing if evictions
> + * are valueable or not.
> + *
> + * Returns true if the res placement intersects with @place and @size.
> + */
> +bool ttm_resource_intersects(struct ttm_device *bdev,
> +			     struct ttm_resource *res,
> +			     const struct ttm_place *place,
> +			     size_t size)
> +{
> +	struct ttm_resource_manager *man;
> +
> +	if (!res)
> +		return false;
> +
> +	if (!place)
> +		return true;
> +
> +	man = ttm_manager_type(bdev, res->mem_type);
> +	if (!man->func->intersects) {
> +		if (place->fpfn >= (res->start + res->num_pages) ||
> +		    (place->lpfn && place->lpfn <= res->start))
> +			return false;
> +
> +		return true;
> +	}
> +
> +	return man->func->intersects(man, res, place, size);
> +}
> +
> +/**
> + * ttm_resource_compatible - test for compatibility
> + *
> + * @bdev: TTM device structure
> + * @res: The resource to test
> + * @place: The placement to test
> + * @size: How many bytes the new allocation needs.
> + *
> + * Test if @res compatible with @place and @size.
> + *
> + * Returns true if the res placement compatible with @place and @size.
> + */
> +bool ttm_resource_compatible(struct ttm_device *bdev,
> +			     struct ttm_resource *res,
> +			     const struct ttm_place *place,
> +			     size_t size)
> +{
> +	struct ttm_resource_manager *man;
> +
> +	if (!res || !place)
> +		return false;
> +
> +	man = ttm_manager_type(bdev, res->mem_type);
> +	if (!man->func->compatible) {
> +		if (res->start < place->fpfn ||
> +		    (place->lpfn && (res->start + res->num_pages) > place->lpfn))
> +			return false;
> +
> +		return true;
> +	}
> +
> +	return man->func->compatible(man, res, place, size);
> +}
> +
>   static bool ttm_resource_places_compat(struct ttm_resource *res,
>   				       const struct ttm_place *places,
>   				       unsigned num_placement)
>   {
> +	struct ttm_buffer_object *bo = res->bo;
> +	struct ttm_device *bdev = bo->bdev;
>   	unsigned i;
>   
>   	if (res->placement & TTM_PL_FLAG_TEMPORARY)
> @@ -265,8 +339,7 @@ static bool ttm_resource_places_compat(struct ttm_resource *res,
>   	for (i = 0; i < num_placement; i++) {
>   		const struct ttm_place *heap = &places[i];
>   
> -		if (res->start < heap->fpfn || (heap->lpfn &&
> -		    (res->start + res->num_pages) > heap->lpfn))
> +		if (!ttm_resource_compatible(bdev, res, heap, bo->base.size))
>   			continue;
>   
>   		if ((res->mem_type == heap->mem_type) &&
> diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
> index ca89a48c2460..5afc6d664fde 100644
> --- a/include/drm/ttm/ttm_resource.h
> +++ b/include/drm/ttm/ttm_resource.h
> @@ -88,6 +88,38 @@ struct ttm_resource_manager_func {
>   	void (*free)(struct ttm_resource_manager *man,
>   		     struct ttm_resource *res);
>   
> +	/**
> +	 * struct ttm_resource_manager_func member intersects
> +	 *
> +	 * @man: Pointer to a memory type manager.
> +	 * @res: Pointer to a struct ttm_resource to be checked.
> +	 * @place: Placement to check against.
> +	 * @size: Size of the check.
> +	 *
> +	 * Test if @res intersects with @place + @size. Used to judge if
> +	 * evictions are valueable or not.
> +	 */
> +	bool (*intersects)(struct ttm_resource_manager *man,
> +			   struct ttm_resource *res,
> +			   const struct ttm_place *place,
> +			   size_t size);
> +
> +	/**
> +	 * struct ttm_resource_manager_func member compatible
> +	 *
> +	 * @man: Pointer to a memory type manager.
> +	 * @res: Pointer to a struct ttm_resource to be checked.
> +	 * @place: Placement to check against.
> +	 * @size: Size of the check.
> +	 *
> +	 * Test if @res compatible with @place + @size. Used to check of
> +	 * the need to move the backing store or not.
> +	 */
> +	bool (*compatible)(struct ttm_resource_manager *man,
> +			   struct ttm_resource *res,
> +			   const struct ttm_place *place,
> +			   size_t size);
> +
>   	/**
>   	 * struct ttm_resource_manager_func member debug
>   	 *
> @@ -329,6 +361,14 @@ int ttm_resource_alloc(struct ttm_buffer_object *bo,
>   		       const struct ttm_place *place,
>   		       struct ttm_resource **res);
>   void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res);
> +bool ttm_resource_intersects(struct ttm_device *bdev,
> +			     struct ttm_resource *res,
> +			     const struct ttm_place *place,
> +			     size_t size);
> +bool ttm_resource_compatible(struct ttm_device *bdev,
> +			     struct ttm_resource *res,
> +			     const struct ttm_place *place,
> +			     size_t size);
>   bool ttm_resource_compat(struct ttm_resource *res,
>   			 struct ttm_placement *placement);
>   void ttm_resource_set_bo(struct ttm_resource *res,
>
> base-commit: 8869fa666a9e6782c3c896c1fa57d65adca23249

[PATCH v6 1/6] drm/ttm: Add new callbacks to ttm res mgr

[Arunpravin Paneer Selvam]

We are adding two new callbacks to ttm resource manager
function to handle intersection and compatibility of
placement and resources.

v2: move the amdgpu and ttm_range_manager changes to
    separate patches (Christian)
v3: rename "intersect" to "intersects" (Matthew)
v4: move !place check to the !res if and return false
    in ttm_resource_compatible() function (Christian)
v5: move bits of code from patch number 6 to avoid
    temporary driver breakup (Christian)

Signed-off-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c       |  9 ++--
 drivers/gpu/drm/ttm/ttm_resource.c | 77 +++++++++++++++++++++++++++++-
 include/drm/ttm/ttm_resource.h     | 40 ++++++++++++++++
 3 files changed, 119 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index c1bd006a5525..f066e8124c50 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -518,6 +518,9 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
 bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
 			      const struct ttm_place *place)
 {
+	struct ttm_resource *res = bo->resource;
+	struct ttm_device *bdev = bo->bdev;
+
 	dma_resv_assert_held(bo->base.resv);
 	if (bo->resource->mem_type == TTM_PL_SYSTEM)
 		return true;
@@ -525,11 +528,7 @@ bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
 	/* Don't evict this BO if it's outside of the
 	 * requested placement range
 	 */
-	if (place->fpfn >= (bo->resource->start + bo->resource->num_pages) ||
-	    (place->lpfn && place->lpfn <= bo->resource->start))
-		return false;
-
-	return true;
+	return ttm_resource_intersects(bdev, res, place, bo->base.size);
 }
 EXPORT_SYMBOL(ttm_bo_eviction_valuable);
 
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index 20f9adcc3235..0d1f862a582b 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -253,10 +253,84 @@ void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res)
 }
 EXPORT_SYMBOL(ttm_resource_free);
 
+/**
+ * ttm_resource_intersects - test for intersection
+ *
+ * @bdev: TTM device structure
+ * @res: The resource to test
+ * @place: The placement to test
+ * @size: How many bytes the new allocation needs.
+ *
+ * Test if @res intersects with @place and @size. Used for testing if evictions
+ * are valueable or not.
+ *
+ * Returns true if the res placement intersects with @place and @size.
+ */
+bool ttm_resource_intersects(struct ttm_device *bdev,
+			     struct ttm_resource *res,
+			     const struct ttm_place *place,
+			     size_t size)
+{
+	struct ttm_resource_manager *man;
+
+	if (!res)
+		return false;
+
+	if (!place)
+		return true;
+
+	man = ttm_manager_type(bdev, res->mem_type);
+	if (!man->func->intersects) {
+		if (place->fpfn >= (res->start + res->num_pages) ||
+		    (place->lpfn && place->lpfn <= res->start))
+			return false;
+
+		return true;
+	}
+
+	return man->func->intersects(man, res, place, size);
+}
+
+/**
+ * ttm_resource_compatible - test for compatibility
+ *
+ * @bdev: TTM device structure
+ * @res: The resource to test
+ * @place: The placement to test
+ * @size: How many bytes the new allocation needs.
+ *
+ * Test if @res compatible with @place and @size.
+ *
+ * Returns true if the res placement compatible with @place and @size.
+ */
+bool ttm_resource_compatible(struct ttm_device *bdev,
+			     struct ttm_resource *res,
+			     const struct ttm_place *place,
+			     size_t size)
+{
+	struct ttm_resource_manager *man;
+
+	if (!res || !place)
+		return false;
+
+	man = ttm_manager_type(bdev, res->mem_type);
+	if (!man->func->compatible) {
+		if (res->start < place->fpfn ||
+		    (place->lpfn && (res->start + res->num_pages) > place->lpfn))
+			return false;
+
+		return true;
+	}
+
+	return man->func->compatible(man, res, place, size);
+}
+
 static bool ttm_resource_places_compat(struct ttm_resource *res,
 				       const struct ttm_place *places,
 				       unsigned num_placement)
 {
+	struct ttm_buffer_object *bo = res->bo;
+	struct ttm_device *bdev = bo->bdev;
 	unsigned i;
 
 	if (res->placement & TTM_PL_FLAG_TEMPORARY)
@@ -265,8 +339,7 @@ static bool ttm_resource_places_compat(struct ttm_resource *res,
 	for (i = 0; i < num_placement; i++) {
 		const struct ttm_place *heap = &places[i];
 
-		if (res->start < heap->fpfn || (heap->lpfn &&
-		    (res->start + res->num_pages) > heap->lpfn))
+		if (!ttm_resource_compatible(bdev, res, heap, bo->base.size))
 			continue;
 
 		if ((res->mem_type == heap->mem_type) &&
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index ca89a48c2460..5afc6d664fde 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -88,6 +88,38 @@ struct ttm_resource_manager_func {
 	void (*free)(struct ttm_resource_manager *man,
 		     struct ttm_resource *res);
 
+	/**
+	 * struct ttm_resource_manager_func member intersects
+	 *
+	 * @man: Pointer to a memory type manager.
+	 * @res: Pointer to a struct ttm_resource to be checked.
+	 * @place: Placement to check against.
+	 * @size: Size of the check.
+	 *
+	 * Test if @res intersects with @place + @size. Used to judge if
+	 * evictions are valueable or not.
+	 */
+	bool (*intersects)(struct ttm_resource_manager *man,
+			   struct ttm_resource *res,
+			   const struct ttm_place *place,
+			   size_t size);
+
+	/**
+	 * struct ttm_resource_manager_func member compatible
+	 *
+	 * @man: Pointer to a memory type manager.
+	 * @res: Pointer to a struct ttm_resource to be checked.
+	 * @place: Placement to check against.
+	 * @size: Size of the check.
+	 *
+	 * Test if @res compatible with @place + @size. Used to check of
+	 * the need to move the backing store or not.
+	 */
+	bool (*compatible)(struct ttm_resource_manager *man,
+			   struct ttm_resource *res,
+			   const struct ttm_place *place,
+			   size_t size);
+
 	/**
 	 * struct ttm_resource_manager_func member debug
 	 *
@@ -329,6 +361,14 @@ int ttm_resource_alloc(struct ttm_buffer_object *bo,
 		       const struct ttm_place *place,
 		       struct ttm_resource **res);
 void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res);
+bool ttm_resource_intersects(struct ttm_device *bdev,
+			     struct ttm_resource *res,
+			     const struct ttm_place *place,
+			     size_t size);
+bool ttm_resource_compatible(struct ttm_device *bdev,
+			     struct ttm_resource *res,
+			     const struct ttm_place *place,
+			     size_t size);
 bool ttm_resource_compat(struct ttm_resource *res,
 			 struct ttm_placement *placement);
 void ttm_resource_set_bo(struct ttm_resource *res,

base-commit: 8869fa666a9e6782c3c896c1fa57d65adca23249
-- 
2.25.1