* [PATCH] drm/amdgpu: correctly sign extend 48bit addresses v3
@ 2018-08-28 12:17 Christian König
[not found] ` <20180828121716.17269-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Christian König @ 2018-08-28 12:17 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
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@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
--
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: correctly sign extend 48bit addresses v3
[not found] ` <20180828121716.17269-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>
@ 2018-08-29 2:03 ` Zhang, Jerry (Junwei)
[not found] ` <5B85FF0C.7080308-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Zhang, Jerry (Junwei) @ 2018-08-29 2:03 UTC (permalink / raw)
To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
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 address is uint64_t. is if failed in some case?
> -/* 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?
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@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
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: correctly sign extend 48bit addresses v3
[not found] ` <5B85FF0C.7080308-5C7GfCeVMHo@public.gmane.org>
@ 2018-08-29 9:39 ` Christian König
[not found] ` <7fc07a91-2d11-256b-6bf1-571012841d82-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Christian König @ 2018-08-29 9:39 UTC (permalink / raw)
To: Zhang, Jerry (Junwei), amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
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@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
>>
>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: correctly sign extend 48bit addresses v3
[not found] ` <7fc07a91-2d11-256b-6bf1-571012841d82-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2018-08-30 2:43 ` Zhang, Jerry (Junwei)
[not found] ` <5B8759E8.9090408-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Zhang, Jerry (Junwei) @ 2018-08-30 2:43 UTC (permalink / raw)
To: christian.koenig-5C7GfCeVMHo,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On 08/29/2018 05:39 PM, Christian König wrote:
> 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.
Thanks. fine.
>
>> the address is uint64_t. is if failed in some case?
>
> What do you mean?
Sorry for the typo without finishing the sentence before sending.
I mean even if the address is uint64_t, still needs to extend the sign?
what I was thinking is that the int64_t needs to do this.
>
>>
>> > -/* 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.
The VA hole is going to catch the VA address out of normal range, which for vega10 is 48-bit?
if so, 0x8000_0000_0000 ULL holds from 0~46 bits, starting from 128TB, but vega10 VA is 256TB
it also could be found in old code gmc_v9.c, the mc_mask holds 48-bits address, like below:
adev->gmc.mc_mask = 0xffff_ffff_ffff ULL; /* 48 bit MC */
But the VA hole start address is 0x8000_0000_0000 ULL, then libdrm gets virtual_address_max
dev_info.virtual_address_max = min(vm_size, AMDGPU_VA_HOLE_START)
// that's 0x8000_0000_0000 ULL actually
Above all, it looks the VA hole start should be 0x1_0000_0000_0000 UL.
Regards,
Jerry
>
> 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@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
>>>
>>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: correctly sign extend 48bit addresses v3
[not found] ` <5B8759E8.9090408-5C7GfCeVMHo@public.gmane.org>
@ 2018-08-30 6:48 ` Christian König
[not found] ` <0d9a151e-a705-8f13-2f24-51afca5f1611-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Christian König @ 2018-08-30 6:48 UTC (permalink / raw)
To: Zhang, Jerry (Junwei), amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Am 30.08.2018 um 04:43 schrieb Zhang, Jerry (Junwei):
> On 08/29/2018 05:39 PM, Christian König wrote:
>> 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.
>
> Thanks. fine.
>
>>
>>> the address is uint64_t. is if failed in some case?
>>
>> What do you mean?
>
> Sorry for the typo without finishing the sentence before sending.
>
> I mean even if the address is uint64_t, still needs to extend the sign?
> what I was thinking is that the int64_t needs to do this.
Well, no. What we would need is an int48_t type, but such thing doesn't
exists and isn't easily implementable in C.
>
>>
>>>
>>> > -/* 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.
>
> The VA hole is going to catch the VA address out of normal range,
> which for vega10 is 48-bit?
Yes, exactly.
> if so, 0x8000_0000_0000 ULL holds from 0~46 bits, starting from 128TB,
> but vega10 VA is 256TB
Correct, the lower range is from 0x0-0x8000_0000_0000 and the higher
range is from 0xffff_8000_0000_0000-0xffff_ffff_ffff_ffff.
>
> it also could be found in old code gmc_v9.c, the mc_mask holds 48-bits
> address, like below:
>
> adev->gmc.mc_mask = 0xffff_ffff_ffff ULL; /* 48 bit MC */
>
> But the VA hole start address is 0x8000_0000_0000 ULL, then libdrm
> gets virtual_address_max
>
> dev_info.virtual_address_max = min(vm_size, AMDGPU_VA_HOLE_START)
> // that's 0x8000_0000_0000 ULL actually
We limit the reported VA size for backward compatibility with old
userspace here.
>
> Above all, it looks the VA hole start should be 0x1_0000_0000_0000 UL.
Nope, that isn't correct. The hole is between 0x8000_0000_0000 and
0xffff_8000_0000_0000.
Regards,
Christian.
>
> Regards,
> Jerry
>
>>
>> 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@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
>>>>
>>>>
>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: correctly sign extend 48bit addresses v3
[not found] ` <0d9a151e-a705-8f13-2f24-51afca5f1611-5C7GfCeVMHo@public.gmane.org>
@ 2018-08-30 7:35 ` Zhang, Jerry (Junwei)
0 siblings, 0 replies; 6+ messages in thread
From: Zhang, Jerry (Junwei) @ 2018-08-30 7:35 UTC (permalink / raw)
To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On 08/30/2018 02:48 PM, Christian König wrote:
> Am 30.08.2018 um 04:43 schrieb Zhang, Jerry (Junwei):
>> On 08/29/2018 05:39 PM, Christian König wrote:
>>> 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.
>>
>> Thanks. fine.
>>
>>>
>>>> the address is uint64_t. is if failed in some case?
>>>
>>> What do you mean?
>>
>> Sorry for the typo without finishing the sentence before sending.
>>
>> I mean even if the address is uint64_t, still needs to extend the sign?
>> what I was thinking is that the int64_t needs to do this.
>
> Well, no. What we would need is an int48_t type, but such thing doesn't exists and isn't easily implementable in C.
If so, it would be better to understand.
Thanks.
>
>>
>>>
>>>>
>>>> > -/* 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.
>>
>> The VA hole is going to catch the VA address out of normal range, which for vega10 is 48-bit?
>
> Yes, exactly.
>
>> if so, 0x8000_0000_0000 ULL holds from 0~46 bits, starting from 128TB, but vega10 VA is 256TB
>
> Correct, the lower range is from 0x0-0x8000_0000_0000 and the higher range is from 0xffff_8000_0000_0000-0xffff_ffff_ffff_ffff.
>
>>
>> it also could be found in old code gmc_v9.c, the mc_mask holds 48-bits address, like below:
>>
>> adev->gmc.mc_mask = 0xffff_ffff_ffff ULL; /* 48 bit MC */
>>
>> But the VA hole start address is 0x8000_0000_0000 ULL, then libdrm gets virtual_address_max
>>
>> dev_info.virtual_address_max = min(vm_size, AMDGPU_VA_HOLE_START)
>> // that's 0x8000_0000_0000 ULL actually
>
> We limit the reported VA size for backward compatibility with old userspace here.
fine, got it.
Thanks.
Regards,
Jerry
>
>>
>> Above all, it looks the VA hole start should be 0x1_0000_0000_0000 UL.
>
> Nope, that isn't correct. The hole is between 0x8000_0000_0000 and 0xffff_8000_0000_0000.
>
> Regards,
> Christian.
>
>>
>> Regards,
>> Jerry
>>
>>>
>>> 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@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
>>>>>
>>>>>
>>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-08-30 7:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-28 12:17 [PATCH] drm/amdgpu: correctly sign extend 48bit addresses v3 Christian König
[not found] ` <20180828121716.17269-1-christian.koenig-5C7GfCeVMHo@public.gmane.org>
2018-08-29 2:03 ` Zhang, Jerry (Junwei)
[not found] ` <5B85FF0C.7080308-5C7GfCeVMHo@public.gmane.org>
2018-08-29 9:39 ` Christian König
[not found] ` <7fc07a91-2d11-256b-6bf1-571012841d82-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-08-30 2:43 ` Zhang, Jerry (Junwei)
[not found] ` <5B8759E8.9090408-5C7GfCeVMHo@public.gmane.org>
2018-08-30 6:48 ` Christian König
[not found] ` <0d9a151e-a705-8f13-2f24-51afca5f1611-5C7GfCeVMHo@public.gmane.org>
2018-08-30 7:35 ` Zhang, Jerry (Junwei)
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.