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 <[email protected]>
---
  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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to