We are calling the eviction_valuable driver callback at eviction time to determine whether we actually can evict a buffer object. The upcoming i915 TTM backend needs the same functionality for swapout, and that might actually be beneficial to other drivers as well.
Add an eviction_valuable call also in the swapout path. Try to keep the current behaviour for all drivers by returning true if the buffer object is already in the TTM_PL_SYSTEM placement. We change behaviour for the case where a buffer object is in a TT backed placement when swapped out, in which case the drivers normal eviction_valuable path is run.
Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Christian König christian.koenig@amd.com Signed-off-by: Thomas Hellström thomas.hellstrom@linux.intel.com Acked-by: Christian König christian.koenig@amd.com --- v3: - Don't export ttm_tt_unpopulate - Fix confusion reading the locked pointer instead of the value pointed to in ttm_bo_evict_swapout_allowable (Reported by Maarten Lankhorst) v5: - Use memset() rather than = {} (Suggested by Christian König) - Remove check for ttm_tt_is_populated in the swapout code in the hope it will be fixed elsewhere (Suggested by Christian König) v7: - Re-add the check for ttm_tt_is_populated in the swapout code. --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 +++ drivers/gpu/drm/ttm/ttm_bo.c | 46 ++++++++++++++++--------- 2 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 80437b6ba5f3..5116065748a5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1328,6 +1328,10 @@ static bool amdgpu_ttm_bo_eviction_valuable(struct ttm_buffer_object *bo, struct dma_fence *f; int i;
+ /* Swapout? */ + if (bo->mem.mem_type == TTM_PL_SYSTEM) + return true; + if (bo->type == ttm_bo_type_kernel && !amdgpu_vm_evictable(ttm_to_amdgpu_bo(bo))) return false; diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index be0406466460..98c41bf6a07d 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -536,6 +536,10 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo, bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo, const struct ttm_place *place) { + dma_resv_assert_held(bo->base.resv); + if (bo->mem.mem_type == TTM_PL_SYSTEM) + return true; + /* Don't evict this BO if it's outside of the * requested placement range */ @@ -558,7 +562,9 @@ EXPORT_SYMBOL(ttm_bo_eviction_valuable); * b. Otherwise, trylock it. */ static bool ttm_bo_evict_swapout_allowable(struct ttm_buffer_object *bo, - struct ttm_operation_ctx *ctx, bool *locked, bool *busy) + struct ttm_operation_ctx *ctx, + const struct ttm_place *place, + bool *locked, bool *busy) { bool ret = false;
@@ -576,6 +582,14 @@ static bool ttm_bo_evict_swapout_allowable(struct ttm_buffer_object *bo, *busy = !ret; }
+ if (ret && place && !bo->bdev->funcs->eviction_valuable(bo, place)) { + ret = false; + if (*locked) { + dma_resv_unlock(bo->base.resv); + *locked = false; + } + } + return ret; }
@@ -630,20 +644,14 @@ int ttm_mem_evict_first(struct ttm_device *bdev, list_for_each_entry(bo, &man->lru[i], lru) { bool busy;
- if (!ttm_bo_evict_swapout_allowable(bo, ctx, &locked, - &busy)) { + if (!ttm_bo_evict_swapout_allowable(bo, ctx, place, + &locked, &busy)) { if (busy && !busy_bo && ticket != dma_resv_locking_ctx(bo->base.resv)) busy_bo = bo; continue; }
- if (place && !bdev->funcs->eviction_valuable(bo, - place)) { - if (locked) - dma_resv_unlock(bo->base.resv); - continue; - } if (!ttm_bo_get_unless_zero(bo)) { if (locked) dma_resv_unlock(bo->base.resv); @@ -1140,10 +1148,19 @@ EXPORT_SYMBOL(ttm_bo_wait); int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx, gfp_t gfp_flags) { + struct ttm_place place; bool locked; int ret;
- if (!ttm_bo_evict_swapout_allowable(bo, ctx, &locked, NULL)) + /* + * While the bo may already reside in SYSTEM placement, set + * SYSTEM as new placement to cover also the move further below. + * The driver may use the fact that we're moving from SYSTEM + * as an indication that we're about to swap out. + */ + memset(&place, 0, sizeof(place)); + place.mem_type = TTM_PL_SYSTEM; + if (!ttm_bo_evict_swapout_allowable(bo, ctx, &place, &locked, NULL)) return -EBUSY;
if (!ttm_bo_get_unless_zero(bo)) { @@ -1168,13 +1185,9 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx, if (bo->mem.mem_type != TTM_PL_SYSTEM) { struct ttm_operation_ctx ctx = { false, false }; struct ttm_resource evict_mem; - struct ttm_place place, hop; + struct ttm_place hop;
- memset(&place, 0, sizeof(place)); memset(&hop, 0, sizeof(hop)); - - place.mem_type = TTM_PL_SYSTEM; - ret = ttm_resource_alloc(bo, &place, &evict_mem); if (unlikely(ret)) goto out; @@ -1202,7 +1215,8 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx, if (bo->bdev->funcs->swap_notify) bo->bdev->funcs->swap_notify(bo);
- ret = ttm_tt_swapout(bo->bdev, bo->ttm, gfp_flags); + if (ttm_tt_is_populated(bo->ttm)) + ret = ttm_tt_swapout(bo->bdev, bo->ttm, gfp_flags); out:
/*