On 2019-02-25 10:50 a.m., Christian König wrote: > Am 22.02.19 um 22:28 schrieb Liu, Shaoyun: >> Adjust vram base offset for XGMI mapping when update the PT entry so >> the address will fall into correct XGMI aperture for peer device >> >> Change-Id: I78bdf244da699d2559481ef5afe9663b3e752236 >> Signed-off-by: shaoyunl <shaoyun.liu@xxxxxxx> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 35 >> +++++++++++++++++++++++++++++----- >> 1 file changed, 30 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> index 942b5eb..e41563b 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> @@ -1890,6 +1890,7 @@ static int amdgpu_vm_bo_split_mapping(struct >> amdgpu_device *adev, >> dma_addr_t *pages_addr, >> struct amdgpu_vm *vm, >> struct amdgpu_bo_va_mapping *mapping, >> + uint64_t vram_base_offset, > > NAK, please just make sure that the correct adev is used down below. [shaoyun] The adev not only be used to get vram_base_offset . It's also used to for all other device specific stuff including the PTE , PDE address etc . What we want is the offset that fall into peer device FB to be programmed into local device's PT entry . I don't see a better solution than add the vram_base_offset as a parameter here . > >> uint64_t flags, >> struct drm_mm_node *nodes, >> struct dma_fence **fence) >> @@ -1965,7 +1966,7 @@ static int amdgpu_vm_bo_split_mapping(struct >> amdgpu_device *adev, >> } >> } else if (flags & AMDGPU_PTE_VALID) { >> - addr += adev->vm_manager.vram_base_offset; >> + addr += vram_base_offset; >> addr += pfn << PAGE_SHIFT; >> } >> @@ -2012,6 +2013,8 @@ int amdgpu_vm_bo_update(struct amdgpu_device >> *adev, >> struct drm_mm_node *nodes; >> struct dma_fence *exclusive, **last_update; >> uint64_t flags; >> + uint64_t vram_base_offset = adev->vm_manager.vram_base_offset; >> + struct amdgpu_device *bo_adev; >> int r; >> if (clear || !bo) { >> @@ -2030,9 +2033,31 @@ int amdgpu_vm_bo_update(struct amdgpu_device >> *adev, >> exclusive = reservation_object_get_excl(bo->tbo.resv); >> } >> - if (bo) >> + if (bo) { >> flags = amdgpu_ttm_tt_pte_flags(adev, bo->tbo.ttm, mem); >> - else >> + bo_adev = amdgpu_ttm_adev(bo->tbo.bdev); >> + if (mem && mem->mem_type == TTM_PL_VRAM && adev != bo_adev) { >> + if (drm_debug & DRM_UT_DRIVER) { >> + list_for_each_entry(mapping, &bo_va->invalids, list) { >> + DRM_DEBUG_DRIVER("Try map VRAM va 0x%llx - >> 0x%llx, offset 0x%llx, from dev %s for peer GPU %s access.\n", >> + mapping->start << PAGE_SHIFT, >> + ((mapping->last + 1) << PAGE_SHIFT) - 1, >> + nodes ? nodes->start << PAGE_SHIFT : 0ll, >> + dev_name(bo_adev->dev), >> + dev_name(adev->dev)); > > Big NAK as well. If you want to make that kind of debugging the please > use trace points for it and not such extensive debug messages. [shaoyunl] This debug message can help me debug on some VM fault during XGMI bring up . Not sure whether I can get same if I move it into trace since it may lose some sequence information especially when I need to coordinate with KFD (or even user level) print message for memory allocation and mapping . But anyway , if you strongly against this debug message , I can remove it . Regards shaoyun.liu > Regards, > Christian. > >> + } >> + } >> + if (adev->gmc.xgmi.hive_id && >> + adev->gmc.xgmi.hive_id == bo_adev->gmc.xgmi.hive_id) { >> + vram_base_offset = >> bo_adev->vm_manager.vram_base_offset; >> + DRM_DEBUG_DRIVER("Used XGMI mapping, >> vram_base_offset 0x%llx.\n", >> + vram_base_offset); >> + } else { >> + DRM_DEBUG_DRIVER("Failed to map the VRAM for other >> device access.\n"); >> + return -EINVAL; >> + } >> + } >> + } else >> flags = 0x0; >> if (clear || (bo && bo->tbo.resv == vm->root.base.bo->tbo.resv)) >> @@ -2050,8 +2075,8 @@ int amdgpu_vm_bo_update(struct amdgpu_device >> *adev, >> list_for_each_entry(mapping, &bo_va->invalids, list) { >> r = amdgpu_vm_bo_split_mapping(adev, exclusive, pages_addr, >> vm, >> - mapping, flags, nodes, >> - last_update); >> + mapping, vram_base_offset, flags, >> + nodes, last_update); >> if (r) >> return r; >> } > _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx