[PATCH 1/2] drm/amdgpu: reverse PDBs order

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Dec 10, 2017 at 10:01 PM, Chunming Zhou <zhoucm1 at amd.com> wrote:
>
>
> On 2017å¹´12æ??08æ?¥ 22:58, Alex Deucher wrote:
>>
>> On Fri, Dec 8, 2017 at 5:56 AM, Chunming Zhou <david1.zhou at amd.com> wrote:
>>>
>>> The hiberachy of page table is as below, which aligns hw names.
>>> PDB2->PDB1->PDB0->PTB, accordingly:
>>> level3 --- PDB2
>>> level2 --- PDB1
>>> level1 --- PDB0
>>> level0 --- PTB
>>
>> What's the advantage of this change?  It's not clear from the commit
>> message.
>
> Hi Alex,
>
> previous the level is increasing, the root pdb is level0, not only me, also
> many people thought the root PDB is PDB0, but our many hw documents assume
> PDB0 is pointing to PTB, that is said, which bring us unnecessary trouble on
> understanding our VM hiberachy implementation, especially for ramp up
> beginner.
> For last Christian BLOCK_SIZE patch example, the spec said it only effects
> the PTB pointed by PDB0, but we initially thought it effects all levels,
> which mainly because of confuse 'PDB0'.
>
> Above is my starting point.

Thanks for clarifying.  Please include that information in the commit
message for clarity.

Alex

>
> Regards,
> David Zhou
>
>>
>> Alex
>>
>>> Change-Id: I2d748e5e96cffe18294c104c4b192d910b2f8e6b
>>> Signed-off-by: Chunming Zhou <david1.zhou at amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 37
>>> ++++++++++++++++++++++------------
>>>   1 file changed, 24 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index 3ecdbdfb04dd..8904ccf78fc9 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -148,8 +148,8 @@ struct amdgpu_prt_cb {
>>>   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) +
>>> +       if (level != 0)
>>> +               return 9 * (level - 1) +
>>>                          adev->vm_manager.block_size;
>>>          else
>>>                  /* For the page tables on the leaves */
>>> @@ -162,20 +162,27 @@ static unsigned amdgpu_vm_level_shift(struct
>>> amdgpu_device *adev,
>>>    * @adev: amdgpu_device pointer
>>>    *
>>>    * Calculate the number of entries in a page directory or page table.
>>> + * The hiberachy of page table is as below, which aligns hw names.
>>> + * PDB2->PDB1->PDB0->PTB, accordingly:
>>> + * level3 --- PDB2
>>> + * level2 --- PDB1
>>> + * level1 --- PDB0
>>> + * level0 --- PTB
>>>    */
>>>   static unsigned amdgpu_vm_num_entries(struct amdgpu_device *adev,
>>>                                        unsigned level)
>>>   {
>>> -       unsigned shift = amdgpu_vm_level_shift(adev, 0);
>>> +       unsigned shift = amdgpu_vm_level_shift(adev,
>>> +
>>> adev->vm_manager.num_level);
>>>
>>> -       if (level == 0)
>>> +       if (level == adev->vm_manager.num_level)
>>>                  /* For the root directory */
>>>                  return round_up(adev->vm_manager.max_pfn, 1 << shift) >>
>>> shift;
>>> -       else if (level != adev->vm_manager.num_level)
>>> +       else if (level != 0)
>>>                  /* Everything in between */
>>>                  return 512;
>>>          else
>>> -               /* For the page tables on the leaves */
>>> +               /* For the page tables on the leaves(PTB) */
>>>                  return AMDGPU_VM_PTE_COUNT(adev);
>>>   }
>>>
>>> @@ -312,6 +319,8 @@ static int amdgpu_vm_alloc_levels(struct
>>> amdgpu_device *adev,
>>>          u64 flags;
>>>          uint64_t init_value = 0;
>>>
>>> +       BUG_ON(level > adev->vm_manager.num_level);
>>> +
>>>          if (!parent->entries) {
>>>                  unsigned num_entries = amdgpu_vm_num_entries(adev,
>>> level);
>>>
>>> @@ -332,7 +341,7 @@ static int amdgpu_vm_alloc_levels(struct
>>> amdgpu_device *adev,
>>>          if (to > parent->last_entry_used)
>>>                  parent->last_entry_used = to;
>>>
>>> -       ++level;
>>> +       level--;
>>>          saddr = saddr & ((1 << shift) - 1);
>>>          eaddr = eaddr & ((1 << shift) - 1);
>>>
>>> @@ -346,7 +355,8 @@ static int amdgpu_vm_alloc_levels(struct
>>> amdgpu_device *adev,
>>>
>>>          if (vm->pte_support_ats) {
>>>                  init_value = AMDGPU_PTE_DEFAULT_ATC;
>>> -               if (level != adev->vm_manager.num_level - 1)
>>> +               /* != PDB0 */
>>> +               if (level != 1)
>>>                          init_value |= AMDGPU_PDE_PTE;
>>>
>>>          }
>>> @@ -389,7 +399,7 @@ static int amdgpu_vm_alloc_levels(struct
>>> amdgpu_device *adev,
>>>                          entry->addr = 0;
>>>                  }
>>>
>>> -               if (level < adev->vm_manager.num_level) {
>>> +               if (level > 0) {
>>>                          uint64_t sub_saddr = (pt_idx == from) ? saddr :
>>> 0;
>>>                          uint64_t sub_eaddr = (pt_idx == to) ? eaddr :
>>>                                  ((1 << shift) - 1);
>>> @@ -435,7 +445,8 @@ int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
>>>          saddr /= AMDGPU_GPU_PAGE_SIZE;
>>>          eaddr /= AMDGPU_GPU_PAGE_SIZE;
>>>
>>> -       return amdgpu_vm_alloc_levels(adev, vm, &vm->root, saddr, eaddr,
>>> 0);
>>> +       return amdgpu_vm_alloc_levels(adev, vm, &vm->root, saddr, eaddr,
>>> +                                     adev->vm_manager.num_level);
>>>   }
>>>
>>>   /**
>>> @@ -1319,19 +1330,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 level = 0;
>>> +       unsigned level = p->adev->vm_manager.num_level;
>>>
>>>          *parent = NULL;
>>>          *entry = &p->vm->root;
>>>          while ((*entry)->entries) {
>>> -               unsigned idx = addr >> amdgpu_vm_level_shift(p->adev,
>>> 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 != p->adev->vm_manager.num_level)
>>> +       if (level != 0)
>>>                  *entry = NULL;
>>>   }
>>>
>>> --
>>> 2.14.1
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>


[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux