[PATCH 2/2] drm/amdgpu: Reset the clear flag in buddy during resume

[Matthew Brost]

On Mon, Jun 23, 2025 at 01:37:35PM +0100, Matthew Auld wrote:
> +Matt B who is adding clear-on-free support in xe. I'm not sure if we might
> also see something like this.
> 

Thanks for the heads up.

> On 23/06/2025 06:52, 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.
> > 
> > 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_vram_mgr.h |  5 ++++
> >   drivers/gpu/drm/drm_buddy.c                  | 24 ++++++++++++++++++++
> >   include/drm/drm_buddy.h                      |  2 ++
> >   4 files changed, 33 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index a59f194e3360..eb67d6c97392 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->mman.vram_mgr.mm);
> >   	adev->in_suspend = false;
> >   	if (amdgpu_acpi_smart_shift_update(dev, AMDGPU_SS_DEV_D0))
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
> > index 1019c5806ec7..e9e2928fa4d1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
> > @@ -58,6 +58,11 @@ static inline bool amdgpu_vram_mgr_is_cleared(struct drm_buddy_block *block)
> >   	return drm_buddy_block_is_clear(block);
> >   }
> > +static inline void amdgpu_vram_mgr_clear_reset_blocks(struct drm_buddy *mm)
> > +{
> > +	drm_buddy_clear_reset_blocks(mm);
> 
> No lock needed?
> 
> > +}
> > +
> >   static inline struct amdgpu_vram_mgr_resource *
> >   to_amdgpu_vram_mgr_resource(struct ttm_resource *res)
> >   {
> > diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
> > index a1e652b7631d..b5e44867adf2 100644
> > --- a/drivers/gpu/drm/drm_buddy.c
> > +++ b/drivers/gpu/drm/drm_buddy.c
> > @@ -405,6 +405,30 @@ drm_get_buddy(struct drm_buddy_block *block)
> >   }
> >   EXPORT_SYMBOL(drm_get_buddy);
> > +/**
> > + * drm_buddy_clear_reset_blocks - reset cleared blocks
> > + *
> > + * @mm: DRM buddy manager
> > + *
> > + * Reset all the cleared blocks in the freelist.
> > + */
> > +void drm_buddy_clear_reset_blocks(struct drm_buddy *mm)
> > +{
> > +	unsigned int i;
> > +
> 
> This might be a good spot to also force merge freed blocks back together,
> for the ones that have the clear vs dirty mismatch. Otherwise with the below
> loop we can have two buddies that are now dirty but don't get merged back
> together? Fairly sure fini() can chuck a warning otherwise. Also a simple
> kunit test for this would be good.
> 
> > +	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 (drm_buddy_block_is_clear(block)) {
> > +				clear_reset(block);
> > +				mm->clear_avail -= drm_buddy_block_size(mm, block);
> > +			}
> > +		}
> > +	}
> > +}
> > +EXPORT_SYMBOL(drm_buddy_clear_reset_blocks);
> > +
> >   /**
> >    * drm_buddy_free_block - free a block
> >    *
> > diff --git a/include/drm/drm_buddy.h b/include/drm/drm_buddy.h
> > index 9689a7c5dd36..da569dea16b7 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_clear_reset_blocks(struct drm_buddy *mm);
> > +

I think a polarity argument here would be good. Following up if on Intel
GPUs the inverse is true - our VRAM is entirely cleared. Either way
having this function being able to flip the state either way would be
good.

Matt

> >   void drm_buddy_free_block(struct drm_buddy *mm, struct drm_buddy_block *block);
> >   void drm_buddy_free_list(struct drm_buddy *mm,
>