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

[Zhang, Jerry (Junwei)]

On 05/16/2017 03:51 PM, Christian König wrote:
> 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).

Sounds reasonable.

Shall we use below for pre-gfx9 as well?
addr = adev->vm_manager.vram_base_offset + addr;

Jery

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