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
>
>
