Re: [PATCH 1/7] drm/amdgpu: fix VM PD addr shift

[Christian König]

Am 28.11.2017 um 03:42 schrieb Chunming Zhou:


On 2017年11月28日 00:02, Christian König wrote:
The block size only affects the leave nodes, everything else is fixed.
This is not true, block size affects every level entries, see the 
register explains for VM_CONTEXTx_CNTL.PAGE_TABLE_BLOCK_SIZE:
"LOG2(number of 2MB logical address ranges) pointed to by a PDE0 page 
text directory entry. The native page size for the component ptes 
comes from the PDE0.block_fragment_size field or 
BASE_ADDR.block_fragment_size field (for a flat page table). The 
PAGE_TABLE_BLOCK_SIZE field only has an effect on address calculations 
for >= 2 level page tables however. A flat page table will utilize as 
many ptes as are required to represent the logical address between 
START_ADDR and END_ADDR, not limited by PAGE_TABLE_BLOCK_SIZE."

The description of the register is incorrect or at least misleading. 
I've fallen into the same trap as well.

Take a look at the VM documentation, there is a small footnote that 
intermediate page directories are always 512 entries in size.

I've confirmed that by configuring a Vega10 into 3 level page directory 
instead of the usual 4 and then using a block size of 18 for the page 
tables.

Regards,
Christian.


Regards,
David Zhou

Signed-off-by: Christian König <christian.koe...@amd.com>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 28 
+++++++++++++++++++++++-----
  1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 122379dfc7d8..f1e541e9b514 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -139,6 +139,24 @@ struct amdgpu_prt_cb {
  };
    /**
+ * amdgpu_vm_level_shift - return the addr shift for each level
+ *
+ * @adev: amdgpu_device pointer
+ *
+ * Returns the number of bits the pfn needs to be right shifted for 
a level.
+ */
+static unsigned amdgpu_vm_level_shift(struct amdgpu_device *adev,
+                      unsigned level)
+{
+    if (level != adev->vm_manager.num_level)
+        return 9 * (adev->vm_manager.num_level - level - 1) +
+            adev->vm_manager.block_size;
+    else
+        /* For the page tables on the leaves */
+        return 0;
+}
+
+/**
   * amdgpu_vm_num_entries - return the number of entries in a PD/PT
   *
   * @adev: amdgpu_device pointer
@@ -288,8 +306,7 @@ static int amdgpu_vm_alloc_levels(struct 
amdgpu_device *adev,
                    uint64_t saddr, uint64_t eaddr,
                    unsigned level)
  {
-    unsigned shift = (adev->vm_manager.num_level - level) *
-        adev->vm_manager.block_size;
+    unsigned shift = amdgpu_vm_level_shift(adev, level);
      unsigned pt_idx, from, to;
      int r;
      u64 flags;
@@ -1302,18 +1319,19 @@ void amdgpu_vm_get_entry(struct 
amdgpu_pte_update_params *p, uint64_t addr,
               struct amdgpu_vm_pt **entry,
               struct amdgpu_vm_pt **parent)
  {
-    unsigned idx, level = p->adev->vm_manager.num_level;
+    unsigned level = 0;
        *parent = NULL;
      *entry = &p->vm->root;
      while ((*entry)->entries) {
-        idx = addr >> (p->adev->vm_manager.block_size * level--);
+        unsigned idx = addr >> amdgpu_vm_level_shift(p->adev, level++);
+
          idx %= amdgpu_bo_size((*entry)->base.bo) / 8;
          *parent = *entry;
          *entry = &(*entry)->entries[idx];
      }
  -    if (level)
+    if (level != p->adev->vm_manager.num_level)
          *entry = NULL;
  }


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx