dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: "Thomas Hellström (Intel)" <thomas_os@shipmail.org>
To: "Christian König" <ckoenig.leichtzumerken@gmail.com>,
	matthew.auld@intel.com, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2
Date: Fri, 4 Jun 2021 11:33:45 +0200	[thread overview]
Message-ID: <2763a6f7-1dcc-f2eb-7154-c433feb0b54d@shipmail.org> (raw)
In-Reply-To: <20210602100914.46246-1-christian.koenig@amd.com>

Hi, Christian,

It looks like all patches in the series have been reviewed or acked by 
Matthew,
and while still a little worried about the final outcome of embedding a
struct ttm_mem_resource, FWIW,

Acked-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

/Thomas

On 6/2/21 12:09 PM, Christian König wrote:
> To improve the handling we want the establish the resource object as base
> class for the backend allocations.
>
> v2: add missing error handling
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  4 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 54 +++++++-------
>   drivers/gpu/drm/nouveau/nouveau_bo.c       |  2 +-
>   drivers/gpu/drm/radeon/radeon_ttm.c        |  2 +-
>   drivers/gpu/drm/ttm/ttm_bo.c               | 83 ++++++++--------------
>   drivers/gpu/drm/ttm/ttm_bo_util.c          | 43 ++++++-----
>   drivers/gpu/drm/ttm/ttm_resource.c         | 31 +++++---
>   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  2 +-
>   include/drm/ttm/ttm_bo_api.h               |  1 -
>   include/drm/ttm/ttm_bo_driver.h            | 10 ++-
>   include/drm/ttm/ttm_resource.h             |  4 +-
>   11 files changed, 110 insertions(+), 126 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 03c6b63d1d54..59723c3d5826 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -362,14 +362,14 @@ int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
>   	if (cpu_addr)
>   		amdgpu_bo_kunmap(*bo_ptr);
>   
> -	ttm_resource_free(&(*bo_ptr)->tbo, (*bo_ptr)->tbo.resource);
> +	ttm_resource_free(&(*bo_ptr)->tbo, &(*bo_ptr)->tbo.resource);
>   
>   	for (i = 0; i < (*bo_ptr)->placement.num_placement; ++i) {
>   		(*bo_ptr)->placements[i].fpfn = offset >> PAGE_SHIFT;
>   		(*bo_ptr)->placements[i].lpfn = (offset + size) >> PAGE_SHIFT;
>   	}
>   	r = ttm_bo_mem_space(&(*bo_ptr)->tbo, &(*bo_ptr)->placement,
> -			     (*bo_ptr)->tbo.resource, &ctx);
> +			     &(*bo_ptr)->tbo.resource, &ctx);
>   	if (r)
>   		goto error;
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 663aa7d2e2ea..69db89261650 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -491,7 +491,7 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
>   			return r;
>   
>   		amdgpu_ttm_backend_unbind(bo->bdev, bo->ttm);
> -		ttm_resource_free(bo, bo->resource);
> +		ttm_resource_free(bo, &bo->resource);
>   		ttm_bo_assign_mem(bo, new_mem);
>   		goto out;
>   	}
> @@ -950,9 +950,9 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
>   	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
>   	struct ttm_operation_ctx ctx = { false, false };
>   	struct amdgpu_ttm_tt *gtt = (void *)bo->ttm;
> -	struct ttm_resource tmp;
>   	struct ttm_placement placement;
>   	struct ttm_place placements;
> +	struct ttm_resource *tmp;
>   	uint64_t addr, flags;
>   	int r;
>   
> @@ -962,37 +962,37 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
>   	addr = amdgpu_gmc_agp_addr(bo);
>   	if (addr != AMDGPU_BO_INVALID_OFFSET) {
>   		bo->resource->start = addr >> PAGE_SHIFT;
> -	} else {
> +		return 0;
> +	}
>   
> -		/* allocate GART space */
> -		placement.num_placement = 1;
> -		placement.placement = &placements;
> -		placement.num_busy_placement = 1;
> -		placement.busy_placement = &placements;
> -		placements.fpfn = 0;
> -		placements.lpfn = adev->gmc.gart_size >> PAGE_SHIFT;
> -		placements.mem_type = TTM_PL_TT;
> -		placements.flags = bo->resource->placement;
> -
> -		r = ttm_bo_mem_space(bo, &placement, &tmp, &ctx);
> -		if (unlikely(r))
> -			return r;
> +	/* allocate GART space */
> +	placement.num_placement = 1;
> +	placement.placement = &placements;
> +	placement.num_busy_placement = 1;
> +	placement.busy_placement = &placements;
> +	placements.fpfn = 0;
> +	placements.lpfn = adev->gmc.gart_size >> PAGE_SHIFT;
> +	placements.mem_type = TTM_PL_TT;
> +	placements.flags = bo->resource->placement;
>   
> -		/* compute PTE flags for this buffer object */
> -		flags = amdgpu_ttm_tt_pte_flags(adev, bo->ttm, &tmp);
> +	r = ttm_bo_mem_space(bo, &placement, &tmp, &ctx);
> +	if (unlikely(r))
> +		return r;
>   
> -		/* Bind pages */
> -		gtt->offset = (u64)tmp.start << PAGE_SHIFT;
> -		r = amdgpu_ttm_gart_bind(adev, bo, flags);
> -		if (unlikely(r)) {
> -			ttm_resource_free(bo, &tmp);
> -			return r;
> -		}
> +	/* compute PTE flags for this buffer object */
> +	flags = amdgpu_ttm_tt_pte_flags(adev, bo->ttm, tmp);
>   
> -		ttm_resource_free(bo, bo->resource);
> -		ttm_bo_assign_mem(bo, &tmp);
> +	/* Bind pages */
> +	gtt->offset = (u64)tmp->start << PAGE_SHIFT;
> +	r = amdgpu_ttm_gart_bind(adev, bo, flags);
> +	if (unlikely(r)) {
> +		ttm_resource_free(bo, &tmp);
> +		return r;
>   	}
>   
> +	ttm_resource_free(bo, &bo->resource);
> +	ttm_bo_assign_mem(bo, tmp);
> +
>   	return 0;
>   }
>   
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index e688ca77483d..3a0d9b3bf991 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -1009,7 +1009,7 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
>   	if (old_reg->mem_type == TTM_PL_TT &&
>   	    new_reg->mem_type == TTM_PL_SYSTEM) {
>   		nouveau_ttm_tt_unbind(bo->bdev, bo->ttm);
> -		ttm_resource_free(bo, bo->resource);
> +		ttm_resource_free(bo, &bo->resource);
>   		ttm_bo_assign_mem(bo, new_reg);
>   		goto out;
>   	}
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 2507c1741681..cdffa9b65108 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -229,7 +229,7 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
>   	if (old_mem->mem_type == TTM_PL_TT &&
>   	    new_mem->mem_type == TTM_PL_SYSTEM) {
>   		radeon_ttm_tt_unbind(bo->bdev, bo->ttm);
> -		ttm_resource_free(bo, bo->resource);
> +		ttm_resource_free(bo, &bo->resource);
>   		ttm_bo_assign_mem(bo, new_mem);
>   		goto out;
>   	}
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 5a7ab4b35b2d..4ed56520b81d 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -223,7 +223,7 @@ static void ttm_bo_cleanup_memtype_use(struct ttm_buffer_object *bo)
>   		bo->bdev->funcs->delete_mem_notify(bo);
>   
>   	ttm_bo_tt_destroy(bo);
> -	ttm_resource_free(bo, bo->resource);
> +	ttm_resource_free(bo, &bo->resource);
>   }
>   
>   static int ttm_bo_individualize_resv(struct ttm_buffer_object *bo)
> @@ -489,7 +489,7 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
>   			struct ttm_operation_ctx *ctx)
>   {
>   	struct ttm_device *bdev = bo->bdev;
> -	struct ttm_resource evict_mem;
> +	struct ttm_resource *evict_mem;
>   	struct ttm_placement placement;
>   	struct ttm_place hop;
>   	int ret = 0;
> @@ -519,7 +519,7 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
>   		goto out;
>   	}
>   
> -	ret = ttm_bo_handle_move_mem(bo, &evict_mem, true, ctx, &hop);
> +	ret = ttm_bo_handle_move_mem(bo, evict_mem, true, ctx, &hop);
>   	if (unlikely(ret)) {
>   		WARN(ret == -EMULTIHOP, "Unexpected multihop in eviction - likely driver bug\n");
>   		if (ret != -ERESTARTSYS)
> @@ -728,14 +728,15 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object *bo,
>    */
>   static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo,
>   				  const struct ttm_place *place,
> -				  struct ttm_resource *mem,
> +				  struct ttm_resource **mem,
>   				  struct ttm_operation_ctx *ctx)
>   {
>   	struct ttm_device *bdev = bo->bdev;
> -	struct ttm_resource_manager *man = ttm_manager_type(bdev, mem->mem_type);
> +	struct ttm_resource_manager *man;
>   	struct ww_acquire_ctx *ticket;
>   	int ret;
>   
> +	man = ttm_manager_type(bdev, (*mem)->mem_type);
>   	ticket = dma_resv_locking_ctx(bo->base.resv);
>   	do {
>   		ret = ttm_resource_alloc(bo, place, mem);
> @@ -749,37 +750,7 @@ static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo,
>   			return ret;
>   	} while (1);
>   
> -	return ttm_bo_add_move_fence(bo, man, mem, ctx->no_wait_gpu);
> -}
> -
> -/**
> - * ttm_bo_mem_placement - check if placement is compatible
> - * @bo: BO to find memory for
> - * @place: where to search
> - * @mem: the memory object to fill in
> - *
> - * Check if placement is compatible and fill in mem structure.
> - * Returns -EBUSY if placement won't work or negative error code.
> - * 0 when placement can be used.
> - */
> -static int ttm_bo_mem_placement(struct ttm_buffer_object *bo,
> -				const struct ttm_place *place,
> -				struct ttm_resource *mem)
> -{
> -	struct ttm_device *bdev = bo->bdev;
> -	struct ttm_resource_manager *man;
> -
> -	man = ttm_manager_type(bdev, place->mem_type);
> -	if (!man || !ttm_resource_manager_used(man))
> -		return -EBUSY;
> -
> -	mem->mem_type = place->mem_type;
> -	mem->placement = place->flags;
> -
> -	spin_lock(&bo->bdev->lru_lock);
> -	ttm_bo_move_to_lru_tail(bo, mem, NULL);
> -	spin_unlock(&bo->bdev->lru_lock);
> -	return 0;
> +	return ttm_bo_add_move_fence(bo, man, *mem, ctx->no_wait_gpu);
>   }
>   
>   /*
> @@ -792,7 +763,7 @@ static int ttm_bo_mem_placement(struct ttm_buffer_object *bo,
>    */
>   int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>   			struct ttm_placement *placement,
> -			struct ttm_resource *mem,
> +			struct ttm_resource **mem,
>   			struct ttm_operation_ctx *ctx)
>   {
>   	struct ttm_device *bdev = bo->bdev;
> @@ -807,8 +778,8 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>   		const struct ttm_place *place = &placement->placement[i];
>   		struct ttm_resource_manager *man;
>   
> -		ret = ttm_bo_mem_placement(bo, place, mem);
> -		if (ret)
> +		man = ttm_manager_type(bdev, place->mem_type);
> +		if (!man || !ttm_resource_manager_used(man))
>   			continue;
>   
>   		type_found = true;
> @@ -818,8 +789,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>   		if (unlikely(ret))
>   			goto error;
>   
> -		man = ttm_manager_type(bdev, mem->mem_type);
> -		ret = ttm_bo_add_move_fence(bo, man, mem, ctx->no_wait_gpu);
> +		ret = ttm_bo_add_move_fence(bo, man, *mem, ctx->no_wait_gpu);
>   		if (unlikely(ret)) {
>   			ttm_resource_free(bo, mem);
>   			if (ret == -EBUSY)
> @@ -832,9 +802,10 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>   
>   	for (i = 0; i < placement->num_busy_placement; ++i) {
>   		const struct ttm_place *place = &placement->busy_placement[i];
> +		struct ttm_resource_manager *man;
>   
> -		ret = ttm_bo_mem_placement(bo, place, mem);
> -		if (ret)
> +		man = ttm_manager_type(bdev, place->mem_type);
> +		if (!man || !ttm_resource_manager_used(man))
>   			continue;
>   
>   		type_found = true;
> @@ -861,12 +832,12 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>   EXPORT_SYMBOL(ttm_bo_mem_space);
>   
>   static int ttm_bo_bounce_temp_buffer(struct ttm_buffer_object *bo,
> -				     struct ttm_resource *mem,
> +				     struct ttm_resource **mem,
>   				     struct ttm_operation_ctx *ctx,
>   				     struct ttm_place *hop)
>   {
>   	struct ttm_placement hop_placement;
> -	struct ttm_resource hop_mem;
> +	struct ttm_resource *hop_mem;
>   	int ret;
>   
>   	hop_placement.num_placement = hop_placement.num_busy_placement = 1;
> @@ -877,7 +848,7 @@ static int ttm_bo_bounce_temp_buffer(struct ttm_buffer_object *bo,
>   	if (ret)
>   		return ret;
>   	/* move to the bounce domain */
> -	ret = ttm_bo_handle_move_mem(bo, &hop_mem, false, ctx, NULL);
> +	ret = ttm_bo_handle_move_mem(bo, hop_mem, false, ctx, NULL);
>   	if (ret) {
>   		ttm_resource_free(bo, &hop_mem);
>   		return ret;
> @@ -889,14 +860,12 @@ static int ttm_bo_move_buffer(struct ttm_buffer_object *bo,
>   			      struct ttm_placement *placement,
>   			      struct ttm_operation_ctx *ctx)
>   {
> +	struct ttm_resource *mem;
>   	struct ttm_place hop;
> -	struct ttm_resource mem;
>   	int ret;
>   
>   	dma_resv_assert_held(bo->base.resv);
>   
> -	memset(&hop, 0, sizeof(hop));
> -
>   	/*
>   	 * Determine where to move the buffer.
>   	 *
> @@ -910,7 +879,7 @@ static int ttm_bo_move_buffer(struct ttm_buffer_object *bo,
>   	if (ret)
>   		return ret;
>   bounce:
> -	ret = ttm_bo_handle_move_mem(bo, &mem, false, ctx, &hop);
> +	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)
> @@ -1019,7 +988,7 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
>   {
>   	static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM };
>   	bool locked;
> -	int ret = 0;
> +	int ret;
>   
>   	bo->destroy = destroy ? destroy : ttm_bo_default_destroy;
>   
> @@ -1029,8 +998,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
>   	bo->bdev = bdev;
>   	bo->type = type;
>   	bo->page_alignment = page_alignment;
> -	bo->resource = &bo->_mem;
> -	ttm_resource_alloc(bo, &sys_mem, bo->resource);
>   	bo->moving = NULL;
>   	bo->pin_count = 0;
>   	bo->sg = sg;
> @@ -1042,6 +1009,12 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
>   	}
>   	atomic_inc(&ttm_glob.bo_count);
>   
> +	ret = ttm_resource_alloc(bo, &sys_mem, &bo->resource);
> +	if (unlikely(ret)) {
> +		ttm_bo_put(bo);
> +		return ret;
> +	}
> +
>   	/*
>   	 * For ttm_bo_type_device buffers, allocate
>   	 * address space from the device.
> @@ -1170,7 +1143,7 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx,
>   	 */
>   	if (bo->resource->mem_type != TTM_PL_SYSTEM) {
>   		struct ttm_operation_ctx ctx = { false, false };
> -		struct ttm_resource evict_mem;
> +		struct ttm_resource *evict_mem;
>   		struct ttm_place place, hop;
>   
>   		memset(&place, 0, sizeof(place));
> @@ -1182,7 +1155,7 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx,
>   		if (unlikely(ret))
>   			goto out;
>   
> -		ret = ttm_bo_handle_move_mem(bo, &evict_mem, true, &ctx, &hop);
> +		ret = ttm_bo_handle_move_mem(bo, evict_mem, true, &ctx, &hop);
>   		if (unlikely(ret != 0)) {
>   			WARN(ret == -EMULTIHOP, "Unexpected multihop in swaput - likely driver bug.\n");
>   			goto out;
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index aedf02a31c70..1b326e70cb02 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -176,16 +176,17 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
>   		       struct ttm_operation_ctx *ctx,
>   		       struct ttm_resource *new_mem)
>   {
> +	struct ttm_resource *old_mem = bo->resource;
>   	struct ttm_device *bdev = bo->bdev;
> -	struct ttm_resource_manager *man = ttm_manager_type(bdev, new_mem->mem_type);
> +	struct ttm_resource_manager *man;
>   	struct ttm_tt *ttm = bo->ttm;
> -	struct ttm_resource *old_mem = bo->resource;
> -	struct ttm_resource old_copy = *old_mem;
>   	void *old_iomap;
>   	void *new_iomap;
>   	int ret;
>   	unsigned long i;
>   
> +	man = ttm_manager_type(bdev, new_mem->mem_type);
> +
>   	ret = ttm_bo_wait_ctx(bo, ctx);
>   	if (ret)
>   		return ret;
> @@ -201,7 +202,7 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
>   	 * Single TTM move. NOP.
>   	 */
>   	if (old_iomap == NULL && new_iomap == NULL)
> -		goto out2;
> +		goto out1;
>   
>   	/*
>   	 * Don't move nonexistent data. Clear destination instead.
> @@ -210,7 +211,7 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
>   	    (ttm == NULL || (!ttm_tt_is_populated(ttm) &&
>   			     !(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)))) {
>   		memset_io(new_iomap, 0, new_mem->num_pages*PAGE_SIZE);
> -		goto out2;
> +		goto out1;
>   	}
>   
>   	/*
> @@ -235,27 +236,25 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
>   			ret = ttm_copy_io_page(new_iomap, old_iomap, i);
>   		}
>   		if (ret)
> -			goto out1;
> +			break;
>   	}
>   	mb();
> -out2:
> -	old_copy = *old_mem;
> +out1:
> +	ttm_resource_iounmap(bdev, new_mem, new_iomap);
> +out:
> +	ttm_resource_iounmap(bdev, old_mem, old_iomap);
> +
> +	if (ret) {
> +		ttm_resource_free(bo, &new_mem);
> +		return ret;
> +	}
>   
> +	ttm_resource_free(bo, &bo->resource);
>   	ttm_bo_assign_mem(bo, new_mem);
>   
>   	if (!man->use_tt)
>   		ttm_bo_tt_destroy(bo);
>   
> -out1:
> -	ttm_resource_iounmap(bdev, old_mem, new_iomap);
> -out:
> -	ttm_resource_iounmap(bdev, &old_copy, old_iomap);
> -
> -	/*
> -	 * On error, keep the mm node!
> -	 */
> -	if (!ret)
> -		ttm_resource_free(bo, &old_copy);
>   	return ret;
>   }
>   EXPORT_SYMBOL(ttm_bo_move_memcpy);
> @@ -566,7 +565,7 @@ static int ttm_bo_wait_free_node(struct ttm_buffer_object *bo,
>   
>   	if (!dst_use_tt)
>   		ttm_bo_tt_destroy(bo);
> -	ttm_resource_free(bo, bo->resource);
> +	ttm_resource_free(bo, &bo->resource);
>   	return 0;
>   }
>   
> @@ -629,7 +628,7 @@ static void ttm_bo_move_pipeline_evict(struct ttm_buffer_object *bo,
>   	}
>   	spin_unlock(&from->move_lock);
>   
> -	ttm_resource_free(bo, bo->resource);
> +	ttm_resource_free(bo, &bo->resource);
>   
>   	dma_fence_put(bo->moving);
>   	bo->moving = dma_fence_get(fence);
> @@ -678,11 +677,11 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo)
>   	if (ret)
>   		ttm_bo_wait(bo, false, false);
>   
> -	ttm_resource_alloc(bo, &sys_mem, bo->resource);
> +	ret = ttm_resource_alloc(bo, &sys_mem, &bo->resource);
>   	bo->ttm = NULL;
>   
>   	dma_resv_unlock(&ghost->base._resv);
>   	ttm_bo_put(ghost);
>   
> -	return 0;
> +	return ret;
>   }
> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
> index 59e2b7157e41..65451e1bc303 100644
> --- a/drivers/gpu/drm/ttm/ttm_resource.c
> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> @@ -27,10 +27,16 @@
>   
>   int ttm_resource_alloc(struct ttm_buffer_object *bo,
>   		       const struct ttm_place *place,
> -		       struct ttm_resource *res)
> +		       struct ttm_resource **res_ptr)
>   {
>   	struct ttm_resource_manager *man =
>   		ttm_manager_type(bo->bdev, place->mem_type);
> +	struct ttm_resource *res;
> +	int r;
> +
> +	res = kmalloc(sizeof(*res), GFP_KERNEL);
> +	if (!res)
> +		return -ENOMEM;
>   
>   	res->mm_node = NULL;
>   	res->start = 0;
> @@ -41,18 +47,27 @@ int ttm_resource_alloc(struct ttm_buffer_object *bo,
>   	res->bus.offset = 0;
>   	res->bus.is_iomem = false;
>   	res->bus.caching = ttm_cached;
> +	r = man->func->alloc(man, bo, place, res);
> +	if (r) {
> +		kfree(res);
> +		return r;
> +	}
>   
> -	return man->func->alloc(man, bo, place, res);
> +	*res_ptr = res;
> +	return 0;
>   }
>   
> -void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource *res)
> +void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res)
>   {
> -	struct ttm_resource_manager *man =
> -		ttm_manager_type(bo->bdev, res->mem_type);
> +	struct ttm_resource_manager *man;
>   
> -	man->func->free(man, res);
> -	res->mm_node = NULL;
> -	res->mem_type = TTM_PL_SYSTEM;
> +	if (!*res)
> +		return;
> +
> +	man = ttm_manager_type(bo->bdev, (*res)->mem_type);
> +	man->func->free(man, *res);
> +	kfree(*res);
> +	*res = NULL;
>   }
>   EXPORT_SYMBOL(ttm_resource_free);
>   
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> index ed8563ef9a3b..bfcf31bf7e37 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> @@ -741,7 +741,7 @@ static int vmw_move(struct ttm_buffer_object *bo,
>   			goto fail;
>   
>   		vmw_ttm_unbind(bo->bdev, bo->ttm);
> -		ttm_resource_free(bo, bo->resource);
> +		ttm_resource_free(bo, &bo->resource);
>   		ttm_bo_assign_mem(bo, new_mem);
>   		return 0;
>   	} else {
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index 291a339a7e08..f681bbdbc698 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -137,7 +137,6 @@ struct ttm_buffer_object {
>   	 */
>   
>   	struct ttm_resource *resource;
> -	struct ttm_resource _mem;
>   	struct ttm_tt *ttm;
>   	bool deleted;
>   
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 1a9ba0b13622..ead0ef7136c8 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -96,7 +96,7 @@ struct ttm_lru_bulk_move {
>    */
>   int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>   		     struct ttm_placement *placement,
> -		     struct ttm_resource *mem,
> +		     struct ttm_resource **mem,
>   		     struct ttm_operation_ctx *ctx);
>   
>   /**
> @@ -188,8 +188,8 @@ ttm_bo_move_to_lru_tail_unlocked(struct ttm_buffer_object *bo)
>   static inline void ttm_bo_assign_mem(struct ttm_buffer_object *bo,
>   				     struct ttm_resource *new_mem)
>   {
> -	bo->_mem = *new_mem;
> -	new_mem->mm_node = NULL;
> +	WARN_ON(bo->resource);
> +	bo->resource = new_mem;
>   }
>   
>   /**
> @@ -202,9 +202,7 @@ static inline void ttm_bo_assign_mem(struct ttm_buffer_object *bo,
>   static inline void ttm_bo_move_null(struct ttm_buffer_object *bo,
>   				    struct ttm_resource *new_mem)
>   {
> -	struct ttm_resource *old_mem = bo->resource;
> -
> -	WARN_ON(old_mem->mm_node != NULL);
> +	ttm_resource_free(bo, &bo->resource);
>   	ttm_bo_assign_mem(bo, new_mem);
>   }
>   
> diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
> index 890b9d369519..c17c1a52070d 100644
> --- a/include/drm/ttm/ttm_resource.h
> +++ b/include/drm/ttm/ttm_resource.h
> @@ -225,8 +225,8 @@ ttm_resource_manager_cleanup(struct ttm_resource_manager *man)
>   
>   int ttm_resource_alloc(struct ttm_buffer_object *bo,
>   		       const struct ttm_place *place,
> -		       struct ttm_resource *res);
> -void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource *res);
> +		       struct ttm_resource **res);
> +void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res);
>   
>   void ttm_resource_manager_init(struct ttm_resource_manager *man,
>   			       unsigned long p_size);

  parent reply	other threads:[~2021-06-04  9:33 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-02 10:09 [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2 Christian König
2021-06-02 10:09 ` [PATCH 02/10] drm/ttm: flip over the range manager to self allocated nodes Christian König
2021-06-02 11:44   ` Thomas Hellström (Intel)
2021-06-02 12:11     ` Christian König
2021-06-02 12:33       ` Thomas Hellström (Intel)
2021-06-02 13:07         ` Christian König
2021-06-02 14:13           ` Thomas Hellström (Intel)
2021-06-02 14:17             ` Christian König
2021-06-02 15:28               ` Thomas Hellström (Intel)
2021-06-02 18:41                 ` Christian König
2021-06-02 18:52                   ` Thomas Hellström (Intel)
2021-06-02 18:53                     ` Christian König
2021-06-02 10:09 ` [PATCH 03/10] drm/ttm: flip over the sys " Christian König
2021-06-03  7:51   ` Matthew Auld
2021-06-02 10:09 ` [PATCH 04/10] drm/amdgpu: revert "drm/amdgpu: stop allocating dummy GTT nodes" Christian König
2021-06-02 10:09 ` [PATCH 05/10] drm/amdkfd: use resource cursor in svm_migrate_copy_to_vram v2 Christian König
2021-06-03  9:44   ` Matthew Auld
2021-06-02 10:09 ` [PATCH 06/10] drm/amdgpu: switch the GTT backend to self alloc Christian König
2021-06-02 10:09 ` [PATCH 07/10] drm/amdgpu: switch the VRAM " Christian König
2021-06-02 10:09 ` [PATCH 08/10] drm/nouveau: switch the TTM backends " Christian König
2021-06-02 10:09 ` [PATCH 09/10] drm/vmwgfx: " Christian König
2021-06-02 10:09 ` [PATCH 10/10] drm/ttm: flip the switch for driver allocated resources v2 Christian König
2021-06-07 10:15   ` Thomas Hellström (Intel)
2021-06-07 10:37     ` Christian König
2021-06-07 10:44       ` Thomas Hellström (Intel)
2021-06-03  8:45 ` [PATCH 01/10] drm/ttm: allocate resource object instead of embedding it v2 Matthew Auld
2021-06-04 11:54   ` Christian König
2021-06-04  9:33 ` Thomas Hellström (Intel) [this message]
2021-06-07 16:40 ` Thomas Hellström (Intel)
2021-06-07 17:06   ` Thomas Hellström (Intel)
2021-06-07 17:54     ` Christian König
2021-06-07 17:58       ` Thomas Hellström (Intel)
2021-06-07 17:59         ` Christian König
2021-06-08  5:29           ` Thomas Hellström (Intel)
2021-06-08  7:14             ` Christian König
2021-06-08  7:17               ` Thomas Hellström (Intel)
2021-06-08  7:21                 ` Christian König
2021-06-08  9:38                   ` Das, Nirmoy
2021-06-08  9:40                     ` Das, Nirmoy
2021-06-08  9:42                       ` Christian König
2021-06-08  9:48                         ` Das, Nirmoy
2021-06-07 17:10   ` Christian König
2021-06-08  6:55 ` Thomas Hellström

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2763a6f7-1dcc-f2eb-7154-c433feb0b54d@shipmail.org \
    --to=thomas_os@shipmail.org \
    --cc=ckoenig.leichtzumerken@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=matthew.auld@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).