Am 08.12.2017 um 15:58 schrieb Alex Deucher:
> 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.
Well you align a bit better with how our hardware guys tend to describe
the levels.
But level0 becomes PTB and level1 becomes PDB0 etc..., so I don't think
we win much here.
Christian.
>
> 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
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
