[PATCH 3/3] drm/amdgpu: fix another fundamental VM bug

[Christian König]

Am 16.05.2017 um 06:58 schrieb Zhang, Jerry (Junwei):
> On 05/15/2017 07:57 PM, Christian König wrote:
>> From: Christian König <christian.koenig at amd.com>
>>
>> I always wondered why this code uses the MC address. Now it came to 
>> me that
>> this is actually a bug and only works by coincident because we used 
>> to have
>> VRAM mapped at zero.
>
> Do you mean to use tbo address in general sw path and convert it into 
> mc address before it going to the real IP (like get_vm_pde)?

Yes, see when we need an address for the MC clients 
(GFX,SDMA,UVD,VCE,DCE etc...) we should use amdgpu_bo_gpu_offset().

But when we need an address for the MC itself then using the mapping 
applied by the MC is nonsense. We need to use the relative address 
inside VRAM instead (or the physical system memory address and set the 
system memory bit).

Christian.

>
> Jerry
>
>>
>> Signed-off-by: Christian König <christian.koenig at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
>>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c  | 2 +-
>>   3 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> index e4c5d31..ba6eb73 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> @@ -884,7 +884,7 @@ static int amdgpu_cs_ib_vm_chunk(struct 
>> amdgpu_device *adev,
>>       }
>>
>>       if (p->job->vm) {
>> -        p->job->vm_pd_addr = amdgpu_bo_gpu_offset(vm->root.bo);
>> +        p->job->vm_pd_addr = vm->root.bo->tbo.mem.start << PAGE_SHIFT;
>>
>>           r = amdgpu_bo_vm_update_pte(p);
>>           if (r)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index b877f9f3..7256fcc 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1012,7 +1012,7 @@ static int amdgpu_vm_update_level(struct 
>> amdgpu_device *adev,
>>                   return r;
>>           }
>>
>> -        pt = amdgpu_bo_gpu_offset(bo);
>> +        pt = bo->tbo.mem.start << PAGE_SHIFT;
>>           if (parent->entries[pt_idx].addr == pt)
>>               continue;
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index 047b1a7..63cb573 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -360,7 +360,7 @@ static uint64_t gmc_v9_0_get_vm_pte_flags(struct 
>> amdgpu_device *adev,
>>
>>   static u64 gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, u64 addr)
>>   {
>> -    addr = adev->vm_manager.vram_base_offset + addr - 
>> adev->mc.vram_start;
>> +    addr = adev->vm_manager.vram_base_offset + addr;
>>       BUG_ON(addr & 0xFFFF00000000003FULL);
>>       return addr;
>>   }
>>