Re: [PATCH 1/2] drm/amdgpu: add amdgpu_vm_entries_mask

[Christian König]

Am 17.09.2018 um 10:52 schrieb Huang Rui:

On Sat, Sep 15, 2018 at 10:05:40AM +0200, Christian König wrote:

We can't get the mask for the root directory from the number of entries.

So add a new function to avoid that problem.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index a7f9aaa47c49..aaf54fc8cafe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -190,6 +190,26 @@ static unsigned amdgpu_vm_num_entries(struct amdgpu_device 
*adev,
                return AMDGPU_VM_PTE_COUNT(adev);
  }

+/**

+ * amdgpu_vm_entries_mask - the mask to get the entry number of a PD/PT
+ *
+ * @adev: amdgpu_device pointer
+ * @level: VMPT level
+ *
+ * Returns:
+ * The mask to extract the entry number of a PD/PT from an address.
+ */
+static uint32_t amdgpu_vm_entries_mask(struct amdgpu_device *adev,
+                                      unsigned int level)
+{
+       if (level <= adev->vm_manager.root_level)
+               return 0xffffffff;
+       else if (level != AMDGPU_VM_PTB)
+               return 0x1f;

I think the mask should be 0x1ff (9 bits) while the level is on PDB. The
purpose here is to figure out the PTE entry id with the mask, am I right.
So the mask should bit width of one entry.

Oh, yes of course that is a typo.

Thanks for catching,
Christian.

Thanks,
Ray

+       else
+               return AMDGPU_VM_PTE_COUNT(adev) - 1;
+}
+
  /**
   * amdgpu_vm_bo_size - returns the size of the BOs in bytes
   *
@@ -399,17 +419,17 @@ static void amdgpu_vm_pt_start(struct amdgpu_device *adev,
  static bool amdgpu_vm_pt_descendant(struct amdgpu_device *adev,
                                    struct amdgpu_vm_pt_cursor *cursor)
  {
-       unsigned num_entries, shift, idx;
+       unsigned mask, shift, idx;

if (!cursor->entry->entries)

                return false;

BUG_ON(!cursor->entry->base.bo);

-       num_entries = amdgpu_vm_num_entries(adev, cursor->level);
+       mask = amdgpu_vm_entries_mask(adev, cursor->level);
        shift = amdgpu_vm_level_shift(adev, cursor->level);

++cursor->level;

-       idx = (cursor->pfn >> shift) % num_entries;
+       idx = (cursor->pfn >> shift) & mask;
        cursor->parent = cursor->entry;
        cursor->entry = &cursor->entry->entries[idx];
        return true;
@@ -1599,7 +1619,7 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_pte_update_params *params,
        amdgpu_vm_pt_start(adev, params->vm, start, &cursor);
        while (cursor.pfn < end) {
                struct amdgpu_bo *pt = cursor.entry->base.bo;
-               unsigned shift, parent_shift, num_entries;
+               unsigned shift, parent_shift, mask;
                uint64_t incr, entry_end, pe_start;

if (!pt)

@@ -1654,9 +1674,9 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_pte_update_params *params,

/* Looks good so far, calculate parameters for the update */

                incr = AMDGPU_GPU_PAGE_SIZE << shift;
-               num_entries = amdgpu_vm_num_entries(adev, cursor.level);
-               pe_start = ((cursor.pfn >> shift) & (num_entries - 1)) * 8;
-               entry_end = num_entries << shift;
+               mask = amdgpu_vm_entries_mask(adev, cursor.level);
+               pe_start = ((cursor.pfn >> shift) & mask) * 8;
+               entry_end = (mask + 1) << shift;
                entry_end += cursor.pfn & ~(entry_end - 1);
                entry_end = min(entry_end, end);

--

2.14.1

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

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