[PATCH 3/4] drm/amdgpu: Fix acquiring VM on large-BAR systems

[Felix Kuehling]

On 2018-03-23 03:35 PM, Christian König wrote:
> Am 23.03.2018 um 20:30 schrieb Felix Kuehling:
>> On large-BAR systems the VM page tables for compute are accessed by
>> the CPU. Always allow CPU access to the page directory so that it can
>> be used later by the CPU when a VM is converted to a compute VM.
>
> NAK, that means we initial place the PD in the visible are for GFX
> which is not desired (not much of an issue for Vega/Raven, but bad for
> older generations).

I'm not setting AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED. Doesn't that mean
the buffer can be placed in invisible VRAM initially?

>
> Better to clear the flag later on and set the
> AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED and then re-validate.

The AMDGPU_GEM_CREATE_NO_CPU_ACCESS seems to mean "this BO must never be
CPU mapped". Clearing that flag later kind of defeats the purpose of
making such a declaration at allocation time. If I know that the buffer
may need CPU access at some point in the future, I figured I shouldn't
set the flag in the first place.

FWIW, I didn't see any example of the AMDGPU_GEM_CREATE_NO_CPU_ACCESS
flag ever being removed from an existing BO.

Regards,
  Felix

>
> Regards,
> Christian.
>
>>
>> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 90ff79e..bc3557b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -2406,8 +2406,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev,
>> struct amdgpu_vm *vm,
>>       if (vm->use_cpu_for_update)
>>           flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
>>       else
>> -        flags |= (AMDGPU_GEM_CREATE_NO_CPU_ACCESS |
>> -                AMDGPU_GEM_CREATE_SHADOW);
>> +        flags |= AMDGPU_GEM_CREATE_SHADOW;
>>         size = amdgpu_vm_bo_size(adev, adev->vm_manager.root_level);
>>       r = amdgpu_bo_create(adev, size, align, true,
>> AMDGPU_GEM_DOMAIN_VRAM,
>