[PATCH 1/2] drm/ttm: improve idle/busy handling v4
Christian König
ckoenig.leichtzumerken at gmail.com
Tue Feb 6 12:56:27 UTC 2024
Am 06.02.24 um 13:53 schrieb Thomas Hellström:
> Hi, Christian,
>
> On Fri, 2024-01-26 at 15:09 +0100, Christian König wrote:
>> Previously we would never try to move a BO into the preferred
>> placements
>> when it ever landed in a busy placement since those were considered
>> compatible.
>>
>> Rework the whole handling and finally unify the idle and busy
>> handling.
>> ttm_bo_validate() is now responsible to try idle placement first and
>> then
>> use the busy placement if that didn't worked.
>>
>> Drawback is that we now always try the idle placement first for each
>> validation which might cause some additional CPU overhead on
>> overcommit.
>>
>> v2: fix kerneldoc warning and coding style
>> v3: take care of XE as well
>> v4: keep the ttm_bo_mem_space functionality as it is for now, only
>> add
>> new handling for ttm_bo_validate as suggested by Thomas
>>
>> Signed-off-by: Christian König <christian.koenig at amd.com>
>> Reviewed-by: Zack Rusin <zack.rusin at broadcom.com> v3
> Sending this through xe CI, will try to review asap.
Take your time. At the moment people are bombarding me with work and I
have only two hands and one head as well :(
Christian.
>
> /Thomas
>
>
>> ---
>> drivers/gpu/drm/ttm/ttm_bo.c | 231 +++++++++++++--------------
>> --
>> drivers/gpu/drm/ttm/ttm_resource.c | 16 +-
>> include/drm/ttm/ttm_resource.h | 3 +-
>> 3 files changed, 121 insertions(+), 129 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
>> b/drivers/gpu/drm/ttm/ttm_bo.c
>> index ba3f09e2d7e6..b12f435542a9 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -724,64 +724,36 @@ static int ttm_bo_add_move_fence(struct
>> ttm_buffer_object *bo,
>> return ret;
>> }
>>
>> -/*
>> - * Repeatedly evict memory from the LRU for @mem_type until we
>> create enough
>> - * space, or we've evicted everything and there isn't enough space.
>> - */
>> -static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo,
>> - const struct ttm_place *place,
>> - struct ttm_resource **mem,
>> - struct ttm_operation_ctx *ctx)
>> -{
>> - struct ttm_device *bdev = bo->bdev;
>> - struct ttm_resource_manager *man;
>> - struct ww_acquire_ctx *ticket;
>> - int ret;
>> -
>> - man = ttm_manager_type(bdev, place->mem_type);
>> - ticket = dma_resv_locking_ctx(bo->base.resv);
>> - do {
>> - ret = ttm_resource_alloc(bo, place, mem);
>> - if (likely(!ret))
>> - break;
>> - if (unlikely(ret != -ENOSPC))
>> - return ret;
>> - ret = ttm_mem_evict_first(bdev, man, place, ctx,
>> - ticket);
>> - if (unlikely(ret != 0))
>> - return ret;
>> - } while (1);
>> -
>> - return ttm_bo_add_move_fence(bo, man, *mem, ctx-
>>> no_wait_gpu);
>> -}
>> -
>> /**
>> - * ttm_bo_mem_space
>> + * ttm_bo_alloc_resource - Allocate backing store for a BO
>> *
>> - * @bo: Pointer to a struct ttm_buffer_object. the data of which
>> - * we want to allocate space for.
>> - * @placement: Proposed new placement for the buffer object.
>> - * @mem: A struct ttm_resource.
>> + * @bo: Pointer to a struct ttm_buffer_object of which we want a
>> resource for
>> + * @placement: Proposed new placement for the buffer object
>> * @ctx: if and how to sleep, lock buffers and alloc memory
>> + * @force_space: If we should evict buffers to force space
>> + * @res: The resulting struct ttm_resource.
>> *
>> - * Allocate memory space for the buffer object pointed to by @bo,
>> using
>> - * the placement flags in @placement, potentially evicting other
>> idle buffer objects.
>> - * This function may sleep while waiting for space to become
>> available.
>> + * Allocates a resource for the buffer object pointed to by @bo,
>> using the
>> + * placement flags in @placement, potentially evicting other buffer
>> objects when
>> + * @force_space is true.
>> + * This function may sleep while waiting for resources to become
>> available.
>> * Returns:
>> - * -EBUSY: No space available (only if no_wait == 1).
>> + * -EBUSY: No space available (only if no_wait == true).
>> * -ENOSPC: Could not allocate space for the buffer object, either
>> due to
>> * fragmentation or concurrent allocators.
>> * -ERESTARTSYS: An interruptible sleep was interrupted by a signal.
>> */
>> -int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>> - struct ttm_placement *placement,
>> - struct ttm_resource **mem,
>> - struct ttm_operation_ctx *ctx)
>> +static int ttm_bo_alloc_resource(struct ttm_buffer_object *bo,
>> + struct ttm_placement *placement,
>> + struct ttm_operation_ctx *ctx,
>> + bool force_space,
>> + struct ttm_resource **res)
>> {
>> struct ttm_device *bdev = bo->bdev;
>> - bool type_found = false;
>> + struct ww_acquire_ctx *ticket;
>> int i, ret;
>>
>> + ticket = dma_resv_locking_ctx(bo->base.resv);
>> ret = dma_resv_reserve_fences(bo->base.resv, 1);
>> if (unlikely(ret))
>> return ret;
>> @@ -790,98 +762,73 @@ int ttm_bo_mem_space(struct ttm_buffer_object
>> *bo,
>> const struct ttm_place *place = &placement-
>>> placement[i];
>> struct ttm_resource_manager *man;
>>
>> - if (place->flags & TTM_PL_FLAG_FALLBACK)
>> - continue;
>> -
>> man = ttm_manager_type(bdev, place->mem_type);
>> if (!man || !ttm_resource_manager_used(man))
>> continue;
>>
>> - type_found = true;
>> - ret = ttm_resource_alloc(bo, place, mem);
>> - if (ret == -ENOSPC)
>> + if (place->flags & (force_space ?
>> TTM_PL_FLAG_DESIRED :
>> + TTM_PL_FLAG_FALLBACK))
>> + continue;
>> +
>> + do {
>> + ret = ttm_resource_alloc(bo, place, res);
>> + if (unlikely(ret != -ENOSPC))
>> + return ret;
>> + if (likely(!ret) || !force_space)
>> + break;
>> +
>> + ret = ttm_mem_evict_first(bdev, man, place,
>> ctx,
>> + ticket);
>> + if (unlikely(ret == -EBUSY))
>> + break;
>> + if (unlikely(ret))
>> + return ret;
>> + } while (1);
>> + if (ret)
>> continue;
>> - if (unlikely(ret))
>> - goto error;
>>
>> - ret = ttm_bo_add_move_fence(bo, man, *mem, ctx-
>>> no_wait_gpu);
>> + ret = ttm_bo_add_move_fence(bo, man, *res, ctx-
>>> no_wait_gpu);
>> if (unlikely(ret)) {
>> - ttm_resource_free(bo, mem);
>> + ttm_resource_free(bo, res);
>> if (ret == -EBUSY)
>> continue;
>>
>> - goto error;
>> + return ret;
>> }
>> return 0;
>> }
>>
>> - for (i = 0; i < placement->num_placement; ++i) {
>> - const struct ttm_place *place = &placement-
>>> placement[i];
>> - struct ttm_resource_manager *man;
>> -
>> - if (place->flags & TTM_PL_FLAG_DESIRED)
>> - continue;
>> -
>> - man = ttm_manager_type(bdev, place->mem_type);
>> - if (!man || !ttm_resource_manager_used(man))
>> - continue;
>> -
>> - type_found = true;
>> - ret = ttm_bo_mem_force_space(bo, place, mem, ctx);
>> - if (likely(!ret))
>> - return 0;
>> -
>> - if (ret && ret != -EBUSY)
>> - goto error;
>> - }
>> -
>> - ret = -ENOSPC;
>> - if (!type_found) {
>> - pr_err(TTM_PFX "No compatible memory type found\n");
>> - ret = -EINVAL;
>> - }
>> -
>> -error:
>> - return ret;
>> + return -ENOSPC;
>> }
>> -EXPORT_SYMBOL(ttm_bo_mem_space);
>>
>> -static int ttm_bo_move_buffer(struct ttm_buffer_object *bo,
>> - struct ttm_placement *placement,
>> - struct ttm_operation_ctx *ctx)
>> +/*
>> + * ttm_bo_mem_space - Wrapper around ttm_bo_alloc_resource
>> + *
>> + * @bo: Pointer to a struct ttm_buffer_object of which we want a
>> resource for
>> + * @placement: Proposed new placement for the buffer object
>> + * @res: The resulting struct ttm_resource.
>> + * @ctx: if and how to sleep, lock buffers and alloc memory
>> + *
>> + * Tries both idle allocation and forcefully eviction of buffers.
>> See
>> + * ttm_bo_alloc_resource for details.
>> + */
>> +int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>> + struct ttm_placement *placement,
>> + struct ttm_resource **res,
>> + struct ttm_operation_ctx *ctx)
>> {
>> - struct ttm_resource *mem;
>> - struct ttm_place hop;
>> + bool force_space = false;
>> int ret;
>>
>> - dma_resv_assert_held(bo->base.resv);
>> + do {
>> + ret = ttm_bo_alloc_resource(bo, placement, ctx,
>> + force_space, res);
>> + force_space = !force_space;
>> + } while (ret == -ENOSPC && force_space);
>>
>> - /*
>> - * Determine where to move the buffer.
>> - *
>> - * If driver determines move is going to need
>> - * an extra step then it will return -EMULTIHOP
>> - * and the buffer will be moved to the temporary
>> - * stop and the driver will be called to make
>> - * the second hop.
>> - */
>> - ret = ttm_bo_mem_space(bo, placement, &mem, ctx);
>> - if (ret)
>> - return ret;
>> -bounce:
>> - ret = ttm_bo_handle_move_mem(bo, mem, false, ctx, &hop);
>> - if (ret == -EMULTIHOP) {
>> - ret = ttm_bo_bounce_temp_buffer(bo, &mem, ctx,
>> &hop);
>> - if (ret)
>> - goto out;
>> - /* try and move to final place now. */
>> - goto bounce;
>> - }
>> -out:
>> - if (ret)
>> - ttm_resource_free(bo, &mem);
>> return ret;
>> }
>> +EXPORT_SYMBOL(ttm_bo_mem_space);
>>
>> /**
>> * ttm_bo_validate
>> @@ -902,6 +849,9 @@ int ttm_bo_validate(struct ttm_buffer_object *bo,
>> struct ttm_placement *placement,
>> struct ttm_operation_ctx *ctx)
>> {
>> + struct ttm_resource *res;
>> + struct ttm_place hop;
>> + bool force_space;
>> int ret;
>>
>> dma_resv_assert_held(bo->base.resv);
>> @@ -912,20 +862,53 @@ int ttm_bo_validate(struct ttm_buffer_object
>> *bo,
>> if (!placement->num_placement)
>> return ttm_bo_pipeline_gutting(bo);
>>
>> - /* Check whether we need to move buffer. */
>> - if (bo->resource && ttm_resource_compatible(bo->resource,
>> placement))
>> - return 0;
>> + force_space = false;
>> + do {
>> + /* Check whether we need to move buffer. */
>> + if (bo->resource &&
>> + ttm_resource_compatible(bo->resource, placement,
>> + force_space))
>> + return 0;
>>
>> - /* Moving of pinned BOs is forbidden */
>> - if (bo->pin_count)
>> - return -EINVAL;
>> + /* Moving of pinned BOs is forbidden */
>> + if (bo->pin_count)
>> + return -EINVAL;
>> +
>> + /*
>> + * Determine where to move the buffer.
>> + *
>> + * If driver determines move is going to need
>> + * an extra step then it will return -EMULTIHOP
>> + * and the buffer will be moved to the temporary
>> + * stop and the driver will be called to make
>> + * the second hop.
>> + */
>> + ret = ttm_bo_alloc_resource(bo, placement, ctx,
>> force_space,
>> + &res);
>> + force_space = !force_space;
>> + if (ret == -ENOSPC)
>> + continue;
>> + if (ret)
>> + return ret;
>> +
>> +bounce:
>> + ret = ttm_bo_handle_move_mem(bo, res, false, ctx,
>> &hop);
>> + if (ret == -EMULTIHOP) {
>> + ret = ttm_bo_bounce_temp_buffer(bo, &res,
>> ctx, &hop);
>> + /* try and move to final place now. */
>> + if (!ret)
>> + goto bounce;
>> + }
>> + if (ret) {
>> + ttm_resource_free(bo, &res);
>> + return ret;
>> + }
>> +
>> + } while (ret && force_space);
>>
>> - ret = ttm_bo_move_buffer(bo, placement, ctx);
>> /* For backward compatibility with userspace */
>> if (ret == -ENOSPC)
>> return -ENOMEM;
>> - if (ret)
>> - return ret;
>>
>> /*
>> * We might need to add a TTM.
>> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
>> b/drivers/gpu/drm/ttm/ttm_resource.c
>> index fb14f7716cf8..65155f2013ca 100644
>> --- a/drivers/gpu/drm/ttm/ttm_resource.c
>> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
>> @@ -295,11 +295,13 @@ bool ttm_resource_intersects(struct ttm_device
>> *bdev,
>> *
>> * @res: the resource to check
>> * @placement: the placement to check against
>> + * @evicting: true if the caller is doing evictions
>> *
>> * Returns true if the placement is compatible.
>> */
>> bool ttm_resource_compatible(struct ttm_resource *res,
>> - struct ttm_placement *placement)
>> + struct ttm_placement *placement,
>> + bool evicting)
>> {
>> struct ttm_buffer_object *bo = res->bo;
>> struct ttm_device *bdev = bo->bdev;
>> @@ -315,14 +317,20 @@ bool ttm_resource_compatible(struct
>> ttm_resource *res,
>> if (res->mem_type != place->mem_type)
>> continue;
>>
>> + if (place->flags & (evicting ? TTM_PL_FLAG_DESIRED :
>> + TTM_PL_FLAG_FALLBACK))
>> + continue;
>> +
>> + if (place->flags & TTM_PL_FLAG_CONTIGUOUS &&
>> + !(res->placement & TTM_PL_FLAG_CONTIGUOUS))
>> + continue;
>> +
>> man = ttm_manager_type(bdev, res->mem_type);
>> if (man->func->compatible &&
>> !man->func->compatible(man, res, place, bo-
>>> base.size))
>> continue;
>>
>> - if ((!(place->flags & TTM_PL_FLAG_CONTIGUOUS) ||
>> - (res->placement & TTM_PL_FLAG_CONTIGUOUS)))
>> - return true;
>> + return true;
>> }
>> return false;
>> }
>> diff --git a/include/drm/ttm/ttm_resource.h
>> b/include/drm/ttm/ttm_resource.h
>> index 1afa13f0c22b..7561023db43d 100644
>> --- a/include/drm/ttm/ttm_resource.h
>> +++ b/include/drm/ttm/ttm_resource.h
>> @@ -366,7 +366,8 @@ bool ttm_resource_intersects(struct ttm_device
>> *bdev,
>> const struct ttm_place *place,
>> size_t size);
>> bool ttm_resource_compatible(struct ttm_resource *res,
>> - struct ttm_placement *placement);
>> + struct ttm_placement *placement,
>> + bool evicting);
>> void ttm_resource_set_bo(struct ttm_resource *res,
>> struct ttm_buffer_object *bo);
>>
More information about the Nouveau
mailing list