Am 03.01.2018 um 09:29 schrieb Chunming Zhou:
>
>
> On 2018å¹´01æ??02æ?¥ 22:47, Christian König wrote:
>> Update the PDEs after resetting the huge flag.
>>
>> Signed-off-by: Christian König <christian.koenig at amd.com>
>> ---
>> Â drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 60
>> ++++++++++------------------------
>> Â 1 file changed, 18 insertions(+), 42 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index c1c5ccdee783..81505870eebc 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -946,54 +946,38 @@ static void amdgpu_vm_handle_huge_pages(struct
>> amdgpu_pte_update_params *p,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â unsigned nptes, uint64_t dst,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â uint64_t flags)
>> Â {
>> -Â Â Â bool use_cpu_update = (p->func == amdgpu_vm_cpu_set_ptes);
>> Â Â Â Â Â uint64_t pd_addr, pde;
>> Â Â Â Â Â Â /* In the case of a mixed PT the PDE must point to it*/
>> -Â Â Â if (p->adev->asic_type < CHIP_VEGA10 || p->src ||
>> -Â Â Â Â Â Â Â nptes != AMDGPU_VM_PTE_COUNT(p->adev)) {
>> -Â Â Â Â Â Â Â dst = amdgpu_bo_gpu_offset(entry->base.bo);
>> -Â Â Â Â Â Â Â flags = AMDGPU_PTE_VALID;
>> -Â Â Â } else {
>> +Â Â Â if (p->adev->asic_type >= CHIP_VEGA10 && !p->src &&
>> +Â Â Â Â Â Â Â nptes == AMDGPU_VM_PTE_COUNT(p->adev)) {
>> Â Â Â Â Â Â Â Â Â /* Set the huge page flag to stop scanning at this PDE */
>> Â Â Â Â Â Â Â Â Â flags |= AMDGPU_PDE_PTE;
>> Â Â Â Â Â }
>> Â -Â Â Â if (!entry->huge && !(flags & AMDGPU_PDE_PTE))
>> +Â Â Â if (!(flags & AMDGPU_PDE_PTE)) {
>> +Â Â Â Â Â Â Â if (entry->huge) {
>> +Â Â Â Â Â Â Â Â Â Â Â /* Add the entry to the relocated list to update it. */
>> +Â Â Â Â Â Â Â Â Â Â Â entry->huge = false;
>> +Â Â Â Â Â Â Â Â Â Â Â spin_lock(&p->vm->status_lock);
>> +Â Â Â Â Â Â Â Â Â Â Â list_move(&entry->base.vm_status, &p->vm->relocated);
>> +Â Â Â Â Â Â Â Â Â Â Â spin_unlock(&p->vm->status_lock);
>> +Â Â Â Â Â Â Â }
>> Â Â Â Â Â Â Â Â Â return;
>> -Â Â Â entry->huge = !!(flags & AMDGPU_PDE_PTE);
>> +Â Â Â }
>> Â +Â Â Â entry->huge = true;
>> Â Â Â Â Â amdgpu_gart_get_vm_pde(p->adev, AMDGPU_VM_PDB0,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â &dst, &flags);
>> Â -Â Â Â if (use_cpu_update) {
>> -Â Â Â Â Â Â Â /* In case a huge page is replaced with a system
>> -Â Â Â Â Â Â Â Â * memory mapping, p->pages_addr != NULL and
>> -Â Â Â Â Â Â Â Â * amdgpu_vm_cpu_set_ptes would try to translate dst
>> -Â Â Â Â Â Â Â Â * through amdgpu_vm_map_gart. But dst is already a
>> -Â Â Â Â Â Â Â Â * GPU address (of the page table). Disable
>> -Â Â Â Â Â Â Â Â * amdgpu_vm_map_gart temporarily.
>> -Â Â Â Â Â Â Â Â */
>> -Â Â Â Â Â Â Â dma_addr_t *tmp;
>> -
>> -Â Â Â Â Â Â Â tmp = p->pages_addr;
>> -Â Â Â Â Â Â Â p->pages_addr = NULL;
>> -
>> -Â Â Â Â Â Â Â pd_addr = (unsigned long)amdgpu_bo_kptr(parent->base.bo);
>> -Â Â Â Â Â Â Â pde = pd_addr + (entry - parent->entries) * 8;
>> -Â Â Â Â Â Â Â amdgpu_vm_cpu_set_ptes(p, pde, dst, 1, 0, flags);
>> -
>> -Â Â Â Â Â Â Â p->pages_addr = tmp;
>> -Â Â Â } else {
>> -Â Â Â Â Â Â Â if (parent->base.bo->shadow) {
>> -Â Â Â Â Â Â Â Â Â Â Â pd_addr = amdgpu_bo_gpu_offset(parent->base.bo->shadow);
>> -Â Â Â Â Â Â Â Â Â Â Â pde = pd_addr + (entry - parent->entries) * 8;
>> -Â Â Â Â Â Â Â Â Â Â Â amdgpu_vm_do_set_ptes(p, pde, dst, 1, 0, flags);
>> -Â Â Â Â Â Â Â }
>> -Â Â Â Â Â Â Â pd_addr = amdgpu_bo_gpu_offset(parent->base.bo);
>> +Â Â Â if (parent->base.bo->shadow) {
>> +Â Â Â Â Â Â Â pd_addr = amdgpu_bo_gpu_offset(parent->base.bo->shadow);
>> Â Â Â Â Â Â Â Â Â pde = pd_addr + (entry - parent->entries) * 8;
>> -Â Â Â Â Â Â Â amdgpu_vm_do_set_ptes(p, pde, dst, 1, 0, flags);
>> +Â Â Â Â Â Â Â p->func(p, pde, dst, 1, 0, flags);
>> Â Â Â Â Â }
>> +Â Â Â pd_addr = amdgpu_bo_gpu_offset(parent->base.bo);
>> +Â Â Â pde = pd_addr + (entry - parent->entries) * 8;
>> +Â Â Â p->func(p, pde, dst, 1, 0, flags);
>> Â }
>> Â Â /**
>> @@ -1205,12 +1189,6 @@ static int amdgpu_vm_bo_update_mapping(struct
>> amdgpu_device *adev,
>> Â Â Â Â Â /* padding, etc. */
>> Â Â Â Â Â ndw = 64;
>> Â -Â Â Â /* one PDE write for each huge page */
>> -Â Â Â if (vm->root.base.bo->shadow)
>> -Â Â Â Â Â Â Â ndw += ((nptes >> adev->vm_manager.block_size) + 1) * 6 * 2;
>> -Â Â Â else
>> -Â Â Â Â Â Â Â ndw += ((nptes >> adev->vm_manager.block_size) + 1) * 6;
> I didn't get why removing this, although you put updating PDE after
> huge flag resets, for huge flag no-reset case, we still do huge page
> PDE update, don't we?
Yeah, but we don't update the PTEs any more. So we have plenty of free
space in the IB for the PDE update.
Regards,
Christian.
>
> Regards,
> David Zhou
>> -
>> Â Â Â Â Â if (pages_addr) {
>> Â Â Â Â Â Â Â Â Â /* copy commands needed */
>> Â Â Â Â Â Â Â Â Â ndw += ncmds * adev->vm_manager.vm_pte_funcs->copy_pte_num_dw;
>> @@ -1285,8 +1263,6 @@ static int amdgpu_vm_bo_update_mapping(struct
>> amdgpu_device *adev,
>> Â Â error_free:
>> Â Â Â Â Â amdgpu_job_free(job);
>> -Â Â Â amdgpu_vm_invalidate_level(adev, vm, &vm->root,
>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â adev->vm_manager.root_level);
>> Â Â Â Â Â return r;
>> Â }
>
