Am 2018-03-24 um 04:34 AM schrieb 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. Where is that done? I'm looking at amdgpu_ttm_placement_from_domain. It only looks at AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED. It does not consider the AMDGPU_GEM_CREATE_NO_CPU_ACCESS flag. So creating the BO without AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED should have the intended effect of placing the BO into invisible VRAM initially. As far as I can tell, my patch does not change the initial placement. It only allows the BO to be kmapped later. > > 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). Right, I see it being set while pinning. But page tables don't usually get pinned. > >> 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. Sure, I'd do that as a separate change because it actually changes the initial placement, whereas my patch did not (as far as I can tell). Regards,  Felix > > 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 > > _______________________________________________ > amd-gfx mailing list > amd-gfx at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 213 bytes Desc: OpenPGP digital signature URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20180325/14448d92/attachment.sig>
