[PATCH 1/2] drm/amdgpu: Dirty cleared blocks on free

[Christian König]

On 23.06.25 07:52, Arunpravin Paneer Selvam wrote:
> Set the dirty bit when the memory resource is not cleared
> during BO release.
> 
> 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")
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c   | 26 ++++++++++++++------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c      |  3 ++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h |  7 ++++--
>  3 files changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 73403744331a..ea6ce53c3a44 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -1302,28 +1302,40 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object *bo)
>  	 * So when this locking here fails something is wrong with the reference
>  	 * counting.
>  	 */
> -	if (WARN_ON_ONCE(!dma_resv_trylock(&bo->base._resv)))
> +	if (WARN_ON_ONCE(!dma_resv_trylock(&bo->base._resv))) {
> +		if (bo->resource && bo->resource->mem_type == TTM_PL_VRAM)
> +			amdgpu_vram_mgr_set_clear_state(bo->resource, false);

As far as I can see this is illegal while the BO is not locked, so please drop that.

> +
>  		return;
> +	}
>  
>  	amdgpu_amdkfd_remove_all_eviction_fences(abo);
>  
> -	if (!bo->resource || bo->resource->mem_type != TTM_PL_VRAM ||
> -	    !(abo->flags & AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE) ||
> -	    adev->in_suspend || drm_dev_is_unplugged(adev_to_drm(adev)))
> +	if (!bo->resource || bo->resource->mem_type != TTM_PL_VRAM)
>  		goto out;
>  
> +	if (!(abo->flags & AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE) ||
> +	    adev->in_suspend || drm_dev_is_unplugged(adev_to_drm(adev)))
> +		goto out_clear_err;
> +
>  	r = dma_resv_reserve_fences(&bo->base._resv, 1);
>  	if (r)
> -		goto out;
> +		goto out_clear_err;
>  
>  	r = amdgpu_fill_buffer(abo, 0, &bo->base._resv, &fence, true);
>  	if (WARN_ON(r))
> -		goto out;
> +		goto out_clear_err;
>  
> -	amdgpu_vram_mgr_set_cleared(bo->resource);
> +	amdgpu_vram_mgr_set_clear_state(bo->resource, true);
>  	dma_resv_add_fence(&bo->base._resv, fence, DMA_RESV_USAGE_KERNEL);
>  	dma_fence_put(fence);
>  
> +	dma_resv_unlock(&bo->base._resv);
> +
> +	return;
> +
> +out_clear_err:
> +	amdgpu_vram_mgr_set_clear_state(bo->resource, false);

As far as I can see that is actually not a good idea. The cleared flag should not be set here in the first place.

>  out:
>  	dma_resv_unlock(&bo->base._resv);
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 9c5df35f05b7..9ec14ab900f4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -407,9 +407,10 @@ static int amdgpu_move_blit(struct ttm_buffer_object *bo,
>  		r = amdgpu_fill_buffer(abo, 0, NULL, &wipe_fence,
>  				       false);
>  		if (r) {
> +			amdgpu_vram_mgr_set_clear_state(bo->resource, false);
>  			goto error;
>  		} else if (wipe_fence) {
> -			amdgpu_vram_mgr_set_cleared(bo->resource);
> +			amdgpu_vram_mgr_set_clear_state(bo->resource, true);
>  			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..1019c5806ec7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h
> @@ -64,9 +64,12 @@ to_amdgpu_vram_mgr_resource(struct ttm_resource *res)
>  	return container_of(res, struct amdgpu_vram_mgr_resource, base);
>  }
>  
> -static inline void amdgpu_vram_mgr_set_cleared(struct ttm_resource *res)
> +static inline void amdgpu_vram_mgr_set_clear_state(struct ttm_resource *res, bool is_clear)
>  {
> -	to_amdgpu_vram_mgr_resource(res)->flags |= DRM_BUDDY_CLEARED;
> +	if (is_clear)
> +		to_amdgpu_vram_mgr_resource(res)->flags |= DRM_BUDDY_CLEARED;
> +	else
> +		to_amdgpu_vram_mgr_resource(res)->flags &= ~DRM_BUDDY_CLEARED;


Rather code this here like:

struct amdgpu_vram_mgr_resource *ares = to_amdgpu_vram_mgr_resource(res);

WARN_ON(ares->flags & DRM_BUDDY_CLEARED);
ares->flags |= DRM_BUDDY_CLEARED;

Regards,
Christian.

>  }
>  
>  #endif