On 7/28/2022 8:57 PM, Matthew Auld wrote:
On 28/07/2022 15:33, Arunpravin Paneer Selvam wrote:
Implemented a new intersect and compatible callback function
fetching start offset from drm buddy allocator.
v2: move the bits that are specific to buddy_man (Matthew)
Signed-off-by: Christian König <christian.koenig@xxxxxxx>
Signed-off-by: Arunpravin Paneer Selvam
<Arunpravin.PaneerSelvam@xxxxxxx>
---
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 39 +-----------
drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 62 +++++++++++++++++++
2 files changed, 64 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 70e2ed4e99df..54eead15d74b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -396,43 +396,8 @@ 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;
- }
+ if (res->mem_type == I915_PL_LMEM0)
+ return ttm_bo_eviction_valuable(bo, place);
We should be able to drop the mem_type == I915_PL_LMEM0 check here I
think, and just unconditionally do:
return ttm_bo_eviction_valuable(bo, place);
okay
return true;
}
diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
index a5109548abc0..9d2a31154d58 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
@@ -178,6 +178,66 @@ static void i915_ttm_buddy_man_free(struct
ttm_resource_manager *man,
kfree(bman_res);
}
+static bool i915_ttm_buddy_man_intersect(struct
ttm_resource_manager *man,
Nit: intersects
ok
+ struct ttm_resource *res,
+ const struct ttm_place *place,
+ size_t size)
+{
+ struct i915_ttm_buddy_resource *bman_res =
to_ttm_buddy_resource(res);
+ u32 start, num_pages = PFN_UP(size);
+ struct drm_buddy_block *block;
+
+ if (!place->fpfn && !place->lpfn)
+ return true;
I place bug_on check here
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 CP
+ * 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) {
+ start = drm_buddy_block_offset(block) >> PAGE_SHIFT;
+ /* Don't evict BOs outside of the requested placement range */
+ if (place->fpfn >= (start + num_pages) ||
+ (place->lpfn && place->lpfn <= start))
+ return false;
+ }
+
+ return true;
We need to account for the block size somewhere. Also same bug in the
amdgpu patch it seems. We also need to do this the other way around
and keep checking until we find something that overlaps, for example
if the first block doesn't intersect/overlap we will incorrectly
return false here, even if one of the other blocks does intersect.
list_for_each_entry() {
fpfn = drm_buddy_block_size(mm, block) >> PAGE_SHIFT;
lpfn = fpfn + drm_buddy_block_size(mm, block) >> PAGE_SHIFT);
if (place->fpfn < lpfn && place->lpfn > fpfn)
return true;
}
return false;
yes, here the final code looks like,
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);
/* Don't evict BOs outside of the requested placement
range */
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);
+ u32 start, num_pages = PFN_UP(size);
+ struct drm_buddy_block *block;
+
+ if (!place->fpfn && !place->lpfn)
+ return true;
I place bug_on check here
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) {
+ start = drm_buddy_block_offset(block) >> PAGE_SHIFT;
+ if (start < place->fpfn ||
+ (place->lpfn && (start + num_pages) > place->lpfn))
Same here. We need to consider the block size/range.
ahh somehow missed the block size, here the final code looks like,
/* 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;
Otherwise I think looks good.
+ return false;
+ }
+
+ return true;
+}
+
static void i915_ttm_buddy_man_debug(struct ttm_resource_manager *man,
struct drm_printer *printer)
{
@@ -205,6 +265,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_intersect,
+ .compatible = i915_ttm_buddy_man_compatible,
.debug = i915_ttm_buddy_man_debug,
};
