- Added a handler in DRM buddy manager to reset the cleared flag for the blocks in the freelist.

- This is necessary because, upon resuming, the VRAM becomes cluttered with BIOS data, yet the VRAM backend manager believes that everything has been cleared.

v2: - Add lock before accessing drm_buddy_clear_reset_blocks()(Matthew Auld) - Force merge the two dirty blocks.(Matthew Auld) - Add a new unit test case for this issue.(Matthew Auld) - Having this function being able to flip the state either way would be good. (Matthew Brost)

Signed-off-by: Arunpravin Paneer Selvam Arunpravin.PaneerSelvam@amd.com Suggested-by: Christian König christian.koenig@amd.com Cc: stable@vger.kernel.org Fixes: a68c7eaa7a8f ("drm/amdgpu: Enable clear page functionality") Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3812 --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 + drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 17 ++++ drivers/gpu/drm/drm_buddy.c | 90 +++++++++++++++++--- include/drm/drm_buddy.h | 2 + 5 files changed, 99 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index a59f194e3360..b89e46f29b51 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -5193,6 +5193,8 @@ int amdgpu_device_resume(struct drm_device *dev, bool notify_clients) dev->dev->power.disable_depth--; #endif } + + amdgpu_vram_mgr_clear_reset_blocks(adev); adev->in_suspend = false;

if (amdgpu_acpi_smart_shift_update(dev, AMDGPU_SS_DEV_D0)) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h index 208b7d1d8a27..450e4bf093b7 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h @@ -154,6 +154,7 @@ int amdgpu_vram_mgr_reserve_range(struct amdgpu_vram_mgr *mgr, uint64_t start, uint64_t size); int amdgpu_vram_mgr_query_page_status(struct amdgpu_vram_mgr *mgr, uint64_t start); +void amdgpu_vram_mgr_clear_reset_blocks(struct amdgpu_device *adev);

bool amdgpu_res_cpu_visible(struct amdgpu_device *adev, struct ttm_resource *res); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c index abdc52b0895a..665656fbc948 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c @@ -782,6 +782,23 @@ uint64_t amdgpu_vram_mgr_vis_usage(struct amdgpu_vram_mgr *mgr) return atomic64_read(&mgr->vis_usage); }

+/** + * amdgpu_vram_mgr_clear_reset_blocks - reset clear blocks + * + * @adev: amdgpu device pointer + * + * Reset the cleared drm buddy blocks. + */ +void amdgpu_vram_mgr_clear_reset_blocks(struct amdgpu_device *adev) +{ + struct amdgpu_vram_mgr *mgr = &adev->mman.vram_mgr; + struct drm_buddy *mm = &mgr->mm; + + mutex_lock(&mgr->lock); + drm_buddy_reset_clear_state(mm, false); + mutex_unlock(&mgr->lock); +} + /** * amdgpu_vram_mgr_intersects - test each drm buddy block for intersection * diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c index a1e652b7631d..436f7e4ee202 100644 --- a/drivers/gpu/drm/drm_buddy.c +++ b/drivers/gpu/drm/drm_buddy.c @@ -12,6 +12,9 @@

#include <drm/drm_buddy.h>

+#define FORCE_MERGE BIT(0) +#define RESET_CLEAR BIT(1) + static struct kmem_cache *slab_blocks;

static struct drm_buddy_block *drm_block_alloc(struct drm_buddy *mm, @@ -60,6 +63,16 @@ static void list_insert_sorted(struct drm_buddy *mm, __list_add(&block->link, node->link.prev, &node->link); }

+static bool is_force_merge_enabled(unsigned int flags) +{ + return flags & FORCE_MERGE; +} + +static bool is_reset_clear_enabled(unsigned int flags) +{ + return flags & RESET_CLEAR; +} + static void clear_reset(struct drm_buddy_block *block) { block->header &= ~DRM_BUDDY_HEADER_CLEAR; @@ -122,7 +135,7 @@ __get_buddy(struct drm_buddy_block *block)

static unsigned int __drm_buddy_free(struct drm_buddy *mm, struct drm_buddy_block *block, - bool force_merge) + unsigned int flags) { struct drm_buddy_block *parent; unsigned int order; @@ -135,7 +148,7 @@ static unsigned int __drm_buddy_free(struct drm_buddy *mm, if (!drm_buddy_block_is_free(buddy)) break;

- if (!force_merge) { + if (!is_force_merge_enabled(flags)) { /* * Check the block and its buddy clear state and exit * the loop if they both have the dissimilar state. @@ -149,7 +162,9 @@ static unsigned int __drm_buddy_free(struct drm_buddy *mm, }

list_del(&buddy->link); - if (force_merge && drm_buddy_block_is_clear(buddy)) + if (is_force_merge_enabled(flags) && + !is_reset_clear_enabled(flags) && + drm_buddy_block_is_clear(buddy)) mm->clear_avail -= drm_buddy_block_size(mm, buddy);

drm_block_free(mm, block); @@ -167,7 +182,8 @@ static unsigned int __drm_buddy_free(struct drm_buddy *mm, static int __force_merge(struct drm_buddy *mm, u64 start, u64 end, - unsigned int min_order) + unsigned int min_order, + unsigned int flags) { unsigned int order; int i; @@ -178,6 +194,8 @@ static int __force_merge(struct drm_buddy *mm, if (min_order > mm->max_order) return -EINVAL;

+ flags |= FORCE_MERGE; + for (i = min_order - 1; i >= 0; i--) { struct drm_buddy_block *block, *prev;

@@ -198,7 +216,8 @@ static int __force_merge(struct drm_buddy *mm, if (!drm_buddy_block_is_free(buddy)) continue;

- WARN_ON(drm_buddy_block_is_clear(block) == + WARN_ON(!is_reset_clear_enabled(flags) && + drm_buddy_block_is_clear(block) == drm_buddy_block_is_clear(buddy));

/* @@ -210,10 +229,11 @@ static int __force_merge(struct drm_buddy *mm, prev = list_prev_entry(prev, link);

list_del(&block->link); - if (drm_buddy_block_is_clear(block)) + if (!is_reset_clear_enabled(flags) && + drm_buddy_block_is_clear(block)) mm->clear_avail -= drm_buddy_block_size(mm, block);

- order = __drm_buddy_free(mm, block, true); + order = __drm_buddy_free(mm, block, flags); if (order >= min_order) return 0; } @@ -336,7 +356,7 @@ void drm_buddy_fini(struct drm_buddy *mm) for (i = 0; i < mm->n_roots; ++i) { order = ilog2(size) - ilog2(mm->chunk_size); start = drm_buddy_block_offset(mm->roots[i]); - __force_merge(mm, start, start + size, order); + __force_merge(mm, start, start + size, order, 0);

if (WARN_ON(!drm_buddy_block_is_free(mm->roots[i]))) kunit_fail_current_test("buddy_fini() root"); @@ -405,6 +425,50 @@ drm_get_buddy(struct drm_buddy_block *block) } EXPORT_SYMBOL(drm_get_buddy);

+/** + * drm_buddy_reset_clear_state - reset blocks clear state + * + * @mm: DRM buddy manager + * @is_clear: blocks clear state + * + * Reset the clear state based on @clear value for each block + * in the freelist. + */ +void drm_buddy_reset_clear_state(struct drm_buddy *mm, bool is_clear) +{ + u64 root_size, size, start; + unsigned int order; + int i; + + for (i = 0; i <= mm->max_order; ++i) { + struct drm_buddy_block *block; + + list_for_each_entry_reverse(block, &mm->free_list[i], link) { + if (is_clear != drm_buddy_block_is_clear(block)) { + if (is_clear) { + mark_cleared(block); + mm->clear_avail += drm_buddy_block_size(mm, block); + } else { + clear_reset(block); + mm->clear_avail -= drm_buddy_block_size(mm, block); + } + } + } + } + + /* Force merge the two dirty or two cleared blocks */ + size = mm->size; + for (i = 0; i < mm->n_roots; ++i) { + order = ilog2(size) - ilog2(mm->chunk_size); + start = drm_buddy_block_offset(mm->roots[i]); + __force_merge(mm, start, start + size, order, RESET_CLEAR); + + root_size = mm->chunk_size << order; + size -= root_size; + } +} +EXPORT_SYMBOL(drm_buddy_reset_clear_state); + /** * drm_buddy_free_block - free a block * @@ -419,7 +483,7 @@ void drm_buddy_free_block(struct drm_buddy *mm, if (drm_buddy_block_is_clear(block)) mm->clear_avail += drm_buddy_block_size(mm, block);

- __drm_buddy_free(mm, block, false); + __drm_buddy_free(mm, block, 0); } EXPORT_SYMBOL(drm_buddy_free_block);

@@ -566,7 +630,7 @@ __alloc_range_bias(struct drm_buddy *mm, if (buddy && (drm_buddy_block_is_free(block) && drm_buddy_block_is_free(buddy))) - __drm_buddy_free(mm, block, false); + __drm_buddy_free(mm, block, 0); return ERR_PTR(err); }

@@ -684,7 +748,7 @@ alloc_from_freelist(struct drm_buddy *mm,

err_undo: if (tmp != order) - __drm_buddy_free(mm, block, false); + __drm_buddy_free(mm, block, 0); return ERR_PTR(err); }

@@ -770,7 +834,7 @@ static int __alloc_range(struct drm_buddy *mm, if (buddy && (drm_buddy_block_is_free(block) && drm_buddy_block_is_free(buddy))) - __drm_buddy_free(mm, block, false); + __drm_buddy_free(mm, block, 0);

err_free: if (err == -ENOSPC && total_allocated_on_err) { @@ -1051,7 +1115,7 @@ int drm_buddy_alloc_blocks(struct drm_buddy *mm, if (order-- == min_order) { /* Try allocation through force merge method */ if (mm->clear_avail && - !__force_merge(mm, start, end, min_order)) { + !__force_merge(mm, start, end, min_order, 0)) { block = __drm_buddy_alloc_blocks(mm, start, end, min_order, diff --git a/include/drm/drm_buddy.h b/include/drm/drm_buddy.h index 9689a7c5dd36..8b5273d4b2d1 100644 --- a/include/drm/drm_buddy.h +++ b/include/drm/drm_buddy.h @@ -160,6 +160,8 @@ int drm_buddy_block_trim(struct drm_buddy *mm, u64 new_size, struct list_head *blocks);

+void drm_buddy_reset_clear_state(struct drm_buddy *mm, bool is_clear); + void drm_buddy_free_block(struct drm_buddy *mm, struct drm_buddy_block *block);

void drm_buddy_free_list(struct drm_buddy *mm,

On 01.07.25 21:08, Arunpravin Paneer Selvam wrote:

Set the dirty bit when the memory resource is not cleared during BO release.

v2(Christian):

• Drop the cleared flag set to false.
• Improve the amdgpu_vram_mgr_set_clear_state() function.

Signed-off-by: Arunpravin Paneer Selvam Arunpravin.PaneerSelvam@amd.com Suggested-by: Christian König christian.koenig@amd.com Cc: stable@vger.kernel.org Fixes: a68c7eaa7a8f ("drm/amdgpu: Enable clear page functionality")

---

drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 1 - drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 5 ++++- 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 9c5df35f05b7..86eb6d47dcc5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -409,7 +409,6 @@ static int amdgpu_move_blit(struct ttm_buffer_object *bo, if (r) { goto error; } else if (wipe_fence) {

• 	amdgpu_vram_mgr_set_cleared(bo->resource);
  

Mhm, that looks incorrect to me.

Why don't we consider the resource cleared after it go wiped during eviction?

Regards, Christian.

	dma_fence_put(fence);
	fence = wipe_fence;
}

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h index b256cbc2bc27..2c88d5fd87da 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h @@ -66,7 +66,10 @@ to_amdgpu_vram_mgr_resource(struct ttm_resource *res) static inline void amdgpu_vram_mgr_set_cleared(struct ttm_resource *res) {

• to_amdgpu_vram_mgr_resource(res)->flags |= DRM_BUDDY_CLEARED;

• struct amdgpu_vram_mgr_resource *ares = to_amdgpu_vram_mgr_resource(res);
• WARN_ON(ares->flags & DRM_BUDDY_CLEARED);
• ares->flags |= DRM_BUDDY_CLEARED;

} #endif