Re: [PATCH 1/8] drm/amdgpu: fix ATC handling for Ryzen

["Christian König" <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

Am 01.04.19 um 19:59 schrieb Kuehling, Felix:
> On 2019-04-01 7:23 a.m., Christian König wrote:
>> Am 30.03.19 um 01:41 schrieb Kuehling, Felix:
>>> Patches 1-3 are Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
>> Thanks.
>>
>>> About the direct mode, that removes a bunch of synchronization, so it
>>> must make some assumptions about the state of the page tables. What
>>> makes that safe?
>> Direct mode is only supposed to be used during page fault handling.
>>
>> E.g. we know that the page tables are in the correct place in this
>> situation because the hardware is hammering on a PTE and waiting for
>> it to become valid.
> A fence could also indicate a concurrent modification of the page table.
> For example a PTB may be allocated and initialized concurrently, not in
> direct mode. Would direct mode need to wait for a fence that indicates
> completion of the PTB initialization? Or do we have some way to ensure
> such concurrent allocation and initialization of a PTB cannot happen?

Yeah, that is a very good question I haven't solved yet either.

My currently best idea is to separate the address space, e.g. use the 
lower address space for on demand paging and the higher with classic 
pre-filled page tables for the MM and display engines.

Christian.

>
> Regards,
>     Felix
>
>
>> Christian.
>>
>>>    Is it safe to use direct-mode on a
>>> per-page-table-update basis? Or do all page table updates have to go
>>> through direct mode to avoid hazards? If yes, then maybe this should be
>>> a property of the VM rather than a parameter that gets passed to a bunch
>>> of function calls.
>>>
>>> Regards,
>>>      Felix
>>>
>>> On 2019-03-29 6:45 a.m., Christian König wrote:
>>>> Otherwise we don't correctly use translate further.
>>>>
>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>> ---
>>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 13 ++++++++-----
>>>>     1 file changed, 8 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> index 3d221f044183..059d9802e713 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> @@ -767,14 +767,17 @@ static int amdgpu_vm_clear_bo(struct
>>>> amdgpu_device *adev,
>>>>            addr = 0;
>>>>         if (ats_entries) {
>>>> -        uint64_t ats_value;
>>>> +        uint64_t value = 0, flags;
>>>>     -        ats_value = AMDGPU_PTE_DEFAULT_ATC;
>>>> -        if (level != AMDGPU_VM_PTB)
>>>> -            ats_value |= AMDGPU_PDE_PTE;
>>>> +        flags = AMDGPU_PTE_DEFAULT_ATC;
>>>> +        if (level != AMDGPU_VM_PTB) {
>>>> +            /* Handle leaf PDEs as PTEs */
>>>> +            flags |= AMDGPU_PDE_PTE;
>>>> +            amdgpu_gmc_get_vm_pde(adev, level, &value, &flags);
>>>> +        }
>>>>                r = vm->update_funcs->update(&params, bo, addr, 0,
>>>> ats_entries,
>>>> -                         0, ats_value);
>>>> +                         value, flags);
>>>>             if (r)
>>>>                 return r;
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx