[PATCH] drm/amdgpu: Enable XGMI mapping for peer device

[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 .

>
>>                         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;
>>       }
>