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

[Christian König]

Am 23.03.2018 um 20:53 schrieb 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?

The difference is the preference. When AMDGPU_GEM_CREATE_NO_CPU_ACCESS 
is set we try to place it initially in invisible VRAM.

Since VM page directories have a low chance of being evicted it will 
just stick in visible VRAM when it was placed initially there.

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

Correct, but that only counts for userspace because userspace can't 
modify the flags :)

If I remember correctly we also enforce it in the mmap IOCTL that the 
flag isn't set, but that doesn't really matter for the kernel.

At least the AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED flag is cleared and 
set all the time in the kernel (but not for VM page tables).

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

Yeah, but that should be harmless as far as I can see.

If you want to avoid the move of the page directory on large BAR systems 
when switching a VM from GFX to compute we can also initially create the 
BO in the system domain.

Thinking about that a bit more that is probably a good idea anyway 
because it avoids using VRAM by just opening the device.

Regards,
Christian.

>
> 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,
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx