[PATCH] drm/amdgpu: correctly sign extend 48bit addresses v3
Christian König
ckoenig.leichtzumerken at gmail.com
Wed Aug 29 09:39:44 UTC 2018
Am 29.08.2018 um 04:03 schrieb Zhang, Jerry (Junwei):
> On 08/28/2018 08:17 PM, Christian König wrote:
>> Correct sign extend the GMC addresses to 48bit.
>
> Could you explain a bit more why to extend the sign?
The hardware works like this, in other words when bit 47 is set we must
extend that into bits 48-63.
> the address is uint64_t. is if failed in some case?
What do you mean?
>
> > -/* VA hole for 48bit addresses on Vega10 */
> > -#define AMDGPU_VA_HOLE_START 0x0000800000000000ULL
> > -#define AMDGPU_VA_HOLE_END 0xffff800000000000ULL
>
> BTW, the hole for 48bit is actually 47 bit left, any background for that?
Well bits start counting at zero. So the 48bit addresses have bits 0-47.
Regards,
Christian.
>
> Regards,
> Jerry
>
>>
>> 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 0d2c9f65ca13..9d9c7a9f54e4 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 5ddd4e87480b..9c3cc23488c2 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -1370,7 +1370,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 1f4b8dfc0456..8bbba0127e42 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -399,7 +399,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 {
>> @@ -629,7 +629,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;
>> @@ -1934,7 +1934,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 94fe47890adf..2ce452198017 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