Re: [PATCH 2/2] drm/ttm: replace context flags with bools

On Mon, Nov 02, 2020 at 01:58:08PM +0100, Christian König wrote:

The ttm_operation_ctx structure has a mixture of flags and bools. Drop the flags and replace them with bools as well.

Signed-off-by: Christian König christian.koenig@amd.com

drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 5 ++--- drivers/gpu/drm/ttm/ttm_bo.c | 2 +- drivers/gpu/drm/ttm/ttm_bo_vm.c | 3 +-- drivers/gpu/drm/ttm/ttm_memory.c | 3 ++- drivers/gpu/drm/ttm/ttm_resource.c | 2 +- include/drm/ttm/ttm_bo_api.h | 10 ++++------ 6 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 52041f48e1c9..c302a2c7982d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -518,9 +518,8 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, .no_wait_gpu = bp->no_wait_gpu, /* We opt to avoid OOM on system pages allocations */ .retry_mayfail = true,

• .resv = bp->resv,
  

• .flags = bp->type != ttm_bo_type_kernel ?
  

• 	TTM_OPT_FLAG_ALLOW_RES_EVICT : 0
  

• .allow_res_evict = bp->type != ttm_bo_type_kernel,
  

• .resv = bp->resv
  

  }; struct amdgpu_bo *bo; unsigned long page_align, size = bp->size;

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index c63b7ea1cd5d..e2a124b3affb 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -637,7 +637,7 @@ static bool ttm_bo_evict_swapout_allowable(struct ttm_buffer_object *bo,

if (bo->base.resv == ctx->resv) { dma_resv_assert_held(bo->base.resv);

• if (ctx->flags & TTM_OPT_FLAG_ALLOW_RES_EVICT)
  

• if (ctx->allow_res_evict)
  ret = true;
  

  *locked = false; if (busy)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index eeaca5d1efe3..4cf9628f38ac 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -315,8 +315,7 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf, struct ttm_operation_ctx ctx = { .interruptible = false, .no_wait_gpu = false,

• 	.flags = TTM_OPT_FLAG_FORCE_ALLOC
  

• 	.fource_alloc = true
  

  };

  ttm = bo->ttm;

diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c index f9a90bfaa3c1..5ed1fc8f2ace 100644 --- a/drivers/gpu/drm/ttm/ttm_memory.c +++ b/drivers/gpu/drm/ttm/ttm_memory.c @@ -542,7 +542,8 @@ ttm_check_under_lowerlimit(struct ttm_mem_global *glob, { int64_t available;

• if (ctx->flags & TTM_OPT_FLAG_FORCE_ALLOC)

• /* We allow over commit during suspend */

• if (ctx->force_alloc) return false;

  available = get_nr_swap_pages() + si_mem_available();

diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c index 4ebc043e2867..29cf905d97b7 100644 --- a/drivers/gpu/drm/ttm/ttm_resource.c +++ b/drivers/gpu/drm/ttm/ttm_resource.c @@ -89,7 +89,7 @@ int ttm_resource_manager_evict_all(struct ttm_bo_device *bdev, struct ttm_operation_ctx ctx = { .interruptible = false, .no_wait_gpu = false,

• .flags = TTM_OPT_FLAG_FORCE_ALLOC
  

• .fource_alloc = true
  

  }; struct ttm_bo_global *glob = &ttm_bo_glob; struct dma_fence *fence;

diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 4c7c2d574db6..6315133c69e5 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -196,6 +196,8 @@ struct ttm_bo_kmap_obj {

• @interruptible: Sleep interruptible if sleeping.
• @no_wait_gpu: Return immediately if the GPU is busy.
• @retry_mayfail: Set the __GFP_RETRY_MAYFAIL when allocation pages.

• • @allow_res_evict: Allow eviction of reserved BOs.
• • @force_alloc: Fource allocation when serving page faults.

s/Fource/Force/ and I think this would be an excellent application of the inline kerneldoc style for structs, so that you can spend a few more words on what exactly these do, and when they're supposed to be used.

I know originally we said we'll do kerneldoc last, but with all the stuff going on and details being discussed I don't think that makes much sense. We'll have forgotten it all again :-)

• @resv: Reservation object to allow reserved evictions with.
• @flags: Including the following flags

Forgot to remove this one here.

With the nits addressed:

Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch

@@ -206,16 +208,12 @@ struct ttm_operation_ctx { bool interruptible; bool no_wait_gpu; bool retry_mayfail;

• bool allow_res_evict;
• bool force_alloc; struct dma_resv *resv; uint64_t bytes_moved;

• uint32_t flags;

};

-/* Allow eviction of reserved BOs */ -#define TTM_OPT_FLAG_ALLOW_RES_EVICT 0x1 -/* when serving page fault or suspend, allow alloc anyway */ -#define TTM_OPT_FLAG_FORCE_ALLOC 0x2

/**

• ttm_bo_get - reference a struct ttm_buffer_object

-- 2.25.1

---

dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel