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

[Daniel Vetter]

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 at 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 at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch