[PATCH] drm/amdgpu: Enable XGMI mapping for peer device
Christian König
ckoenig.leichtzumerken at gmail.com
Tue Feb 26 19:00:44 UTC 2019
Am 26.02.19 um 19:10 schrieb Liu, Shaoyun:
> 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 at amd.com>
>>>>> ---
>>>>> 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 .
Quite simple it is much more descriptive to do so.
BTW while at it please put it before to the mm nodes, because it is
actually an attribute describing those.
Christian.
>
>
>>>>> 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 at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
More information about the amd-gfx
mailing list