On 2018å¹´01æ??03æ?¥ 17:24, Christian König wrote:
> 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.
OK, got it, thanks for explain.
patch#3 and #4 are Reviewed-by: Chunming Zhou <david1.zhou at amd.com>
>
> 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;
>>> Â }
>>
>
