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.paneersel...@amd.com>
> Suggested-by: Christian König <christian.koe...@amd.com>
> Cc: sta...@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
