Am 11.12.2017 um 11:28 schrieb Christian König:
> Am 11.12.2017 um 09:16 schrieb Chunming Zhou:
>> 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
>>
>> v2:
>> previous 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'.
>
> I'm still not very keen about this.
>
> How about we additional to this add an enum for the PDB2..PTB values?
Please rebase on top of the changes I've just submitted, add the enum
and resend.
Thanks,
Christian.
>
> Regards,
> Christian.
>
>>
>> Change-Id: I2d748e5e96cffe18294c104c4b192d910b2f8e6b
>> Signed-off-by: Chunming Zhou <david1.zhou at amd.com>
>> ---
>> Â drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 36
>> ++++++++++++++++++++++------------
>> Â 1 file changed, 23 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index bb191fca60f3..bdb7bdfee0fb 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,7 @@ 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)
>> +Â Â Â Â Â Â Â if (level != 0)
>> Â Â Â Â Â Â Â Â Â Â Â Â Â init_value |= AMDGPU_PDE_PTE;
>> Â Â Â Â Â Â }
>> @@ -389,7 +398,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 +444,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 +1329,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;
>> Â }
>
