[PATCH 2/3] drm/amdgpu: add functions to create bo at the specified position

Deucher, Alexander Alexander.Deucher at amd.com
Wed Sep 27 15:52:39 UTC 2017


> -----Original Message-----
> From: Horace Chen [mailto:horace.chen at amd.com]
> Sent: Wednesday, September 27, 2017 5:24 AM
> To: amd-gfx at lists.freedesktop.org
> Cc: Deucher, Alexander; Chen, Horace
> Subject: [PATCH 2/3] drm/amdgpu: add functions to create bo at the
> specified position
> 
>  Add two function to create and free bo at the specified position
> on the VRAM.
>  Add a new parameter to amdgpu_bo_do_create to tell the start address
> of the special bo. If the start address is located in the GPU MC
> address space, the placement will be set to the exact place according
> to the size and start address. Otherwise the start address will be
> ingored.
> 
> Signed-off-by: Horace Chen <horace.chen at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 118
> ++++++++++++++++++++++++++++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |   6 ++
>  2 files changed, 122 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 6982bae..8ea7fec 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -284,6 +284,7 @@ void amdgpu_bo_free_kernel(struct amdgpu_bo
> **bo, u64 *gpu_addr,
>  }
> 
>  static int amdgpu_bo_do_create(struct amdgpu_device *adev,
> +			       u64 start_addr,
>  			       unsigned long size, int byte_align,
>  			       bool kernel, u32 domain, u64 flags,
>  			       struct sg_table *sg,
> @@ -297,9 +298,11 @@ static int amdgpu_bo_do_create(struct
> amdgpu_device *adev,
>  	u64 initial_bytes_moved, bytes_moved;
>  	size_t acc_size;
>  	int r;
> +	int i;
> 
>  	page_align = roundup(byte_align, PAGE_SIZE) >> PAGE_SHIFT;
>  	size = ALIGN(size, PAGE_SIZE);
> +	start_addr = ALIGN(start_addr, PAGE_SIZE);
> 
>  	if (kernel) {
>  		type = ttm_bo_type_kernel;
> @@ -366,6 +369,23 @@ static int amdgpu_bo_do_create(struct
> amdgpu_device *adev,
>  	bo->tbo.bdev = &adev->mman.bdev;
>  	amdgpu_ttm_placement_from_domain(bo, domain);
> 
> +	/*
> +	 * if domain is VRAM only && start_addr in GPU address space,
> +	 * set the place to the specified place
> +	 */
> +	if (domain == AMDGPU_GEM_DOMAIN_VRAM &&
> +	    start_addr >= adev->mc.vram_start &&
> +	    (start_addr + size) < adev->mc.vram_end) {
> +		/*change it to offset */
> +		start_addr -= adev->mc.vram_start;
> +		for (i = 0; i < bo->placement.num_placement; ++i) {
> +			bo->placements[i].fpfn =
> +				start_addr >> PAGE_SHIFT;
> +			bo->placements[i].lpfn =
> +				(start_addr + size) >> PAGE_SHIFT;
> +		}
> +	}

Any reason to limit this to only VRAM?  Should be possible to handle gart as well, although I don't know that we have a particular need for that.  I guess we can always extend it if we need it.

> +
>  	initial_bytes_moved = atomic64_read(&adev->num_bytes_moved);
>  	/* Kernel allocation are uninterruptible */
>  	r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size,
> type,
> @@ -418,6 +438,100 @@ static int amdgpu_bo_do_create(struct
> amdgpu_device *adev,
>  	return r;
>  }
> 
> +/**
> + * amdgpu_bo_create_vram_specified -
> + *    create BO at the specified place on the VRAM.
> + *
> + * @adev: amdgpu device object
> + * @offset: start offset of the new BO
> + * @size: size for the new BO
> + * @byte_align: alignment for the new BO
> + * @flags: addition flags for the BO
> + * @bo_ptr: resulting BO
> + * @gpu_addr: GPU addr of the pinned BO
> + * @cpu_addr: optional CPU address mapping
> + *
> + * Allocates and pins a BO at the specified place on the VRAM.
> + *
> + * Returns 0 on success, negative error code otherwise.
> + */
> +int amdgpu_bo_create_vram_specified(struct amdgpu_device *adev,

Maybe make the name a bit more consistent?  Use restricted to mirror the pin naming and kernel so we know it's just for kernel allocations.  How about?
amdgpu_bo_create_vram_restricted_kernel() and
amdgpu_bo_free_vram_restricted_kernel()?

With that fixed:
Acked-by: Alex Deucher <alexander.deucher at amd.com>

> +		     u64 offset, unsigned long size, int byte_align,
> +		     u64 flags, struct amdgpu_bo **bo_ptr,
> +		     u64 *gpu_addr, void **cpu_addr)
> +{
> +	int r;
> +	u64 start_addr = offset + adev->mc.vram_start;
> +	/* specified memory must be in contiguous*/
> +	flags |= AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS;
> +
> +	r = amdgpu_bo_do_create(adev, start_addr, size, byte_align, true,
> +		AMDGPU_GEM_DOMAIN_VRAM, flags, NULL, NULL, 0,
> bo_ptr);
> +	if (r)
> +		return r;
> +
> +	r = amdgpu_bo_reserve(*bo_ptr, false);
> +	if (r)
> +		goto error_reserve;
> +	r = amdgpu_bo_pin_restricted(*bo_ptr,
> +		AMDGPU_GEM_DOMAIN_VRAM, offset, (offset + size),
> +		gpu_addr);
> +	if (r)
> +		goto error_pin;
> +	if (cpu_addr) {
> +		r = amdgpu_bo_kmap(*bo_ptr,
> +					cpu_addr);
> +		if (r)
> +			goto error_kmap;
> +	}
> +
> +	amdgpu_bo_unreserve(*bo_ptr);
> +
> +	return r;
> +error_kmap:
> +	amdgpu_bo_unpin(*bo_ptr);
> +error_pin:
> +	amdgpu_bo_unreserve(*bo_ptr);
> +error_reserve:
> +	amdgpu_bo_unref(bo_ptr);
> +	if (cpu_addr)
> +		*cpu_addr = NULL;
> +	if (gpu_addr)
> +		*gpu_addr = 0;
> +	return r;
> +}
> +
> +/**
> + * amdgpu_bo_free_vram_specified - free BO for specified vram
> + *
> + * @bo: amdgpu BO to free
> + * @gpu_addr : gpu address
> + * @cpu_addr : cpu address if mapped
> + *
> + * unmaps and unpin a BO for specified vram.
> + */
> +void amdgpu_bo_free_vram_specified(struct amdgpu_bo **bo, u64
> *gpu_addr,
> +			   void **cpu_addr)
> +{
> +	if (*bo == NULL)
> +		return;
> +
> +	if (likely(amdgpu_bo_reserve(*bo, true) == 0)) {
> +		if (cpu_addr)
> +			amdgpu_bo_kunmap(*bo);
> +		amdgpu_bo_unpin(*bo);
> +		amdgpu_bo_unreserve(*bo);
> +	}
> +	amdgpu_bo_unref(bo);
> +
> +	if (gpu_addr)
> +		*gpu_addr = 0;
> +
> +	if (cpu_addr)
> +		*cpu_addr = NULL;
> +}
> +
> +
>  static int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
>  				   unsigned long size, int byte_align,
>  				   struct amdgpu_bo *bo)
> @@ -427,7 +541,7 @@ static int amdgpu_bo_create_shadow(struct
> amdgpu_device *adev,
>  	if (bo->shadow)
>  		return 0;
> 
> -	r = amdgpu_bo_do_create(adev, size, byte_align, true,
> +	r = amdgpu_bo_do_create(adev, 0, size, byte_align, true,
>  				AMDGPU_GEM_DOMAIN_GTT,
>  				AMDGPU_GEM_CREATE_CPU_GTT_USWC |
>  				AMDGPU_GEM_CREATE_SHADOW,
> @@ -457,7 +571,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>  	uint64_t parent_flags = flags & ~AMDGPU_GEM_CREATE_SHADOW;
>  	int r;
> 
> -	r = amdgpu_bo_do_create(adev, size, byte_align, kernel, domain,
> +	r = amdgpu_bo_do_create(adev, 0, size, byte_align, kernel, domain,
>  				parent_flags, sg, resv, init_value, bo_ptr);
>  	if (r)
>  		return r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 39b6bf6..8742583 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -205,6 +205,12 @@ int amdgpu_bo_create_kernel(struct
> amdgpu_device *adev,
>  			    u64 *gpu_addr, void **cpu_addr);
>  void amdgpu_bo_free_kernel(struct amdgpu_bo **bo, u64 *gpu_addr,
>  			   void **cpu_addr);
> +int amdgpu_bo_create_vram_specified(struct amdgpu_device *adev,
> +		     u64 offset, unsigned long size, int byte_align,
> +		     u64 flags, struct amdgpu_bo **bo_ptr,
> +		     u64 *gpu_addr, void **cpu_addr);
> +void amdgpu_bo_free_vram_specified(struct amdgpu_bo **bo, u64
> *gpu_addr,
> +			   void **cpu_addr);
>  int amdgpu_bo_kmap(struct amdgpu_bo *bo, void **ptr);
>  void *amdgpu_bo_kptr(struct amdgpu_bo *bo);
>  void amdgpu_bo_kunmap(struct amdgpu_bo *bo);
> --
> 2.7.4



More information about the amd-gfx mailing list