[PATCH v2 2/3] drm/amdgpu: Reset the clear flag in buddy during resume
Arunpravin Paneer Selvam
arunpravin.paneerselvam at amd.com
Tue Jul 8 06:10:42 UTC 2025
Hi Matthew,
On 7/4/2025 2:22 PM, Matthew Auld wrote:
> On 01/07/2025 20:08, Arunpravin Paneer Selvam wrote:
>> - 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 at amd.com>
>> Suggested-by: Christian König <christian.koenig at amd.com>
>> Cc: stable at 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);
>> + }
>> + }
>> + }
>> + }
>
> Is it not possible to do the merge step first and then go through
> whatever is left marking at clear/dirty? If we do that then we maybe
> don't need any extra changes or flags outside of reset_clear_state? Or
> am I missing something?
Yes you are right, I will make the changes accordingly.
Thanks,
Arun.
>
>> +
>> + /* 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,
>
More information about the dri-devel
mailing list