On 2019-02-26 2:55 a.m., Christian König wrote: > Am 25.02.19 um 19:47 schrieb Liu, Shaoyun: >> 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 . > > Sounds like you didn't understood me. You should just not add the > vram_base_offset as the parameter, but the adev to use. > > E.g. something like bo_adev, src_adev or mem_adev or something like that. > > Christian. [shaoyunl] Do you plan to do some extra with this adev( bo_adev as you suggested) other than get the vram_base_offset ? Otherwise I still don't see why it's better than just pass in the offset directly . >> >>>> 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 > _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx