[PATCH 1/6] drm/amdgpu: correctly sign extend 48bit addresses v3

Zhang, Jerry (Junwei) Jerry.Zhang at amd.com
Fri Aug 31 01:51:36 UTC 2018


Patch 1~5 are

Reviewed-by: Junwei Zhang <Jerry.Zhang at amd.com>

Patch 6 is

Acked-by: Junwei Zhang <Jerry.Zhang at amd.com>


BTW, [PATCH 4/6] drm/amdgpu: manually map the shadow BOs again

with this patch, the user cannot create a shadow bo with gart address.
anyway, I cannot image that use case either.

Regards,
Jerry

On 08/30/2018 08:14 PM, Christian König wrote:
> Correct sign extend the GMC addresses to 48bit.
>
> v2: sign extending turned out easier than thought.
> v3: clean up the defines and move them into amdgpu_gmc.h as well
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c     |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 10 ++++-----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h    | 26 ++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    |  8 +++----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c   |  6 ++---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c     |  7 +++---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h     | 13 -----------
>   9 files changed, 44 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index 8c652ecc4f9a..bc5ccfca68c5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -135,7 +135,7 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
>   			.num_queue_per_pipe = adev->gfx.mec.num_queue_per_pipe,
>   			.gpuvm_size = min(adev->vm_manager.max_pfn
>   					  << AMDGPU_GPU_PAGE_SHIFT,
> -					  AMDGPU_VA_HOLE_START),
> +					  AMDGPU_GMC_HOLE_START),
>   			.drm_render_minor = adev->ddev->render->index
>   		};
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index dd734970e167..ef2bfc04b41c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -835,7 +835,7 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p)
>   			if (chunk->chunk_id != AMDGPU_CHUNK_ID_IB)
>   				continue;
>
> -			va_start = chunk_ib->va_start & AMDGPU_VA_HOLE_MASK;
> +			va_start = chunk_ib->va_start & AMDGPU_GMC_HOLE_MASK;
>   			r = amdgpu_cs_find_mapping(p, va_start, &aobj, &m);
>   			if (r) {
>   				DRM_ERROR("IB va_start is invalid\n");
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 71792d820ae0..d30a0838851b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -572,16 +572,16 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
>   		return -EINVAL;
>   	}
>
> -	if (args->va_address >= AMDGPU_VA_HOLE_START &&
> -	    args->va_address < AMDGPU_VA_HOLE_END) {
> +	if (args->va_address >= AMDGPU_GMC_HOLE_START &&
> +	    args->va_address < AMDGPU_GMC_HOLE_END) {
>   		dev_dbg(&dev->pdev->dev,
>   			"va_address 0x%LX is in VA hole 0x%LX-0x%LX\n",
> -			args->va_address, AMDGPU_VA_HOLE_START,
> -			AMDGPU_VA_HOLE_END);
> +			args->va_address, AMDGPU_GMC_HOLE_START,
> +			AMDGPU_GMC_HOLE_END);
>   		return -EINVAL;
>   	}
>
> -	args->va_address &= AMDGPU_VA_HOLE_MASK;
> +	args->va_address &= AMDGPU_GMC_HOLE_MASK;
>
>   	if ((args->flags & ~valid_flags) && (args->flags & ~prt_flags)) {
>   		dev_dbg(&dev->pdev->dev, "invalid flags combination 0x%08X\n",
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 72fcc9338f5e..48715dd5808a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -30,6 +30,19 @@
>
>   #include "amdgpu_irq.h"
>
> +/* VA hole for 48bit addresses on Vega10 */
> +#define AMDGPU_GMC_HOLE_START	0x0000800000000000ULL
> +#define AMDGPU_GMC_HOLE_END	0xffff800000000000ULL
> +
> +/*
> + * Hardware is programmed as if the hole doesn't exists with start and end
> + * address values.
> + *
> + * This mask is used to remove the upper 16bits of the VA and so come up with
> + * the linear addr value.
> + */
> +#define AMDGPU_GMC_HOLE_MASK	0x0000ffffffffffffULL
> +
>   struct firmware;
>
>   /*
> @@ -131,6 +144,19 @@ static inline bool amdgpu_gmc_vram_full_visible(struct amdgpu_gmc *gmc)
>   	return (gmc->real_vram_size == gmc->visible_vram_size);
>   }
>
> +/**
> + * amdgpu_gmc_sign_extend - sign extend the given gmc address
> + *
> + * @addr: address to extend
> + */
> +static inline uint64_t amdgpu_gmc_sign_extend(uint64_t addr)
> +{
> +	if (addr >= AMDGPU_GMC_HOLE_START)
> +		addr |= AMDGPU_GMC_HOLE_END;
> +
> +	return addr;
> +}
> +
>   void amdgpu_gmc_get_pde_for_bo(struct amdgpu_bo *bo, int level,
>   			       uint64_t *addr, uint64_t *flags);
>   uint64_t amdgpu_gmc_pd_addr(struct amdgpu_bo *bo);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 9c4e45936ade..29ac3873eeb0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -655,11 +655,11 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
>
>   		dev_info.virtual_address_offset = AMDGPU_VA_RESERVED_SIZE;
>   		dev_info.virtual_address_max =
> -			min(vm_size, AMDGPU_VA_HOLE_START);
> +			min(vm_size, AMDGPU_GMC_HOLE_START);
>
> -		if (vm_size > AMDGPU_VA_HOLE_START) {
> -			dev_info.high_va_offset = AMDGPU_VA_HOLE_END;
> -			dev_info.high_va_max = AMDGPU_VA_HOLE_END | vm_size;
> +		if (vm_size > AMDGPU_GMC_HOLE_START) {
> +			dev_info.high_va_offset = AMDGPU_GMC_HOLE_END;
> +			dev_info.high_va_max = AMDGPU_GMC_HOLE_END | vm_size;
>   		}
>   		dev_info.virtual_address_alignment = max((int)PAGE_SIZE, AMDGPU_GPU_PAGE_SIZE);
>   		dev_info.pte_fragment_size = (1 << adev->vm_manager.fragment_size) * AMDGPU_GPU_PAGE_SIZE;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index b5f20b42439e..0cbf651a88a6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -1368,7 +1368,7 @@ u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
>   	WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_VRAM &&
>   		     !(bo->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS));
>
> -	return bo->tbo.offset;
> +	return amdgpu_gmc_sign_extend(bo->tbo.offset);
>   }
>
>   /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index 38856365580d..f2f358aa0597 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -28,9 +28,7 @@ uint64_t amdgpu_csa_vaddr(struct amdgpu_device *adev)
>   	uint64_t addr = adev->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT;
>
>   	addr -= AMDGPU_VA_RESERVED_SIZE;
> -
> -	if (addr >= AMDGPU_VA_HOLE_START)
> -		addr |= AMDGPU_VA_HOLE_END;
> +	addr = amdgpu_gmc_sign_extend(addr);
>
>   	return addr;
>   }
> @@ -73,7 +71,7 @@ void amdgpu_free_static_csa(struct amdgpu_device *adev) {
>   int amdgpu_map_static_csa(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   			  struct amdgpu_bo_va **bo_va)
>   {
> -	uint64_t csa_addr = amdgpu_csa_vaddr(adev) & AMDGPU_VA_HOLE_MASK;
> +	uint64_t csa_addr = amdgpu_csa_vaddr(adev) & AMDGPU_GMC_HOLE_MASK;
>   	struct ww_acquire_ctx ticket;
>   	struct list_head list;
>   	struct amdgpu_bo_list_entry pd;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 72f8c750e128..379903c7fddf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -403,7 +403,7 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
>   		if (level == adev->vm_manager.root_level) {
>   			ats_entries = amdgpu_vm_level_shift(adev, level);
>   			ats_entries += AMDGPU_GPU_PAGE_SHIFT;
> -			ats_entries = AMDGPU_VA_HOLE_START >> ats_entries;
> +			ats_entries = AMDGPU_GMC_HOLE_START >> ats_entries;
>   			ats_entries = min(ats_entries, entries);
>   			entries -= ats_entries;
>   		} else {
> @@ -632,7 +632,7 @@ int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
>   	eaddr = saddr + size - 1;
>
>   	if (vm->pte_support_ats)
> -		ats = saddr < AMDGPU_VA_HOLE_START;
> +		ats = saddr < AMDGPU_GMC_HOLE_START;
>
>   	saddr /= AMDGPU_GPU_PAGE_SIZE;
>   	eaddr /= AMDGPU_GPU_PAGE_SIZE;
> @@ -1929,7 +1929,8 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
>   			struct amdgpu_bo_va_mapping, list);
>   		list_del(&mapping->list);
>
> -		if (vm->pte_support_ats && mapping->start < AMDGPU_VA_HOLE_START)
> +		if (vm->pte_support_ats &&
> +		    mapping->start < AMDGPU_GMC_HOLE_START)
>   			init_pte_value = AMDGPU_PTE_DEFAULT_ATC;
>
>   		r = amdgpu_vm_bo_update_mapping(adev, NULL, NULL, vm,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 62116fa44718..ae61fa587180 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -101,19 +101,6 @@ struct amdgpu_bo_list_entry;
>   /* hardcode that limit for now */
>   #define AMDGPU_VA_RESERVED_SIZE			(1ULL << 20)
>
> -/* VA hole for 48bit addresses on Vega10 */
> -#define AMDGPU_VA_HOLE_START			0x0000800000000000ULL
> -#define AMDGPU_VA_HOLE_END			0xffff800000000000ULL
> -
> -/*
> - * Hardware is programmed as if the hole doesn't exists with start and end
> - * address values.
> - *
> - * This mask is used to remove the upper 16bits of the VA and so come up with
> - * the linear addr value.
> - */
> -#define AMDGPU_VA_HOLE_MASK			0x0000ffffffffffffULL
> -
>   /* max vmids dedicated for process */
>   #define AMDGPU_VM_MAX_RESERVED_VMID	1
>
>


More information about the amd-gfx mailing list