Re: [PATCH v2] drm/amdgpu: PRT support for gfx9 (v2)

["Zhang, Jerry (Junwei)" <Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>]

On 04/19/2017 07:40 PM, Christian König wrote:
> Am 19.04.2017 um 10:10 schrieb Zhang, Jerry (Junwei):
>> On 04/19/2017 03:28 PM, Christian König wrote:
>>> Am 19.04.2017 um 03:53 schrieb Junwei Zhang:
>>>> v2: unify PRT bit for all ASICs
>>>>
>>>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>>>> Acked-by: David Zhou <david1.zhou@amd.com>
>>>>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 5 +++++
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 ++-
>>>>   2 files changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> index f804d38..f4087c4 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> @@ -1473,6 +1473,11 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>>>>       spin_unlock(&vm->status_lock);
>>>>       list_for_each_entry(mapping, &bo_va->invalids, list) {
>>>> +        if (mapping->flags & AMDGPU_PTE_PRT) {
>>>> +            flags |= AMDGPU_PTE_PRT;
>>>> +            flags &= ~AMDGPU_PTE_VALID;
>>>> +        }
>>>
>>> Please move that into amdgpu_vm_bo_split_mapping() where we apply the other
>>> flags as well.
>>
>> Actually I tried this way at first.
>> But it invalidates the CLEAR op and any calling amdgpu_vm_clear_freed(), as
>> the input flags 0 cannot clear the PTE TILED bit.
>
> Good point, but calling amdgpu_vm_bo_split_mapping() from
> amdgpu_vm_clear_freed() is nonsense in the first place.
>
> We can clear a whole PT with just a single command, no need for splitting it
> into smaller operations.
>
> I'm going to send a patch to fix this in a minute.

Thanks for your patch.
Based on that, I prepare the patch v3, it works on my side.
Please help to check it.

Jerry


>
> Christian.
>
>>
>>>
>>> Additional to that we need a check like "&& adev->asic_type >= CHIP_VEGA10"
>>> here.
>>>
>>> Not nice to have that ASIC specifc code here, but we can clean that up later
>>> on.
>>
>> although that's reserved bit for older ASIC, it sounds reasonable.
>>
>> Jerry
>>
>>>
>>> Regards,
>>> Christian.
>>>
>>>> +
>>>>           r = amdgpu_vm_bo_split_mapping(adev, exclusive,
>>>>                              gtt_flags, pages_addr, vm,
>>>>                              mapping, flags, mem,
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>>> index 0f547c6..8eba2d3 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>>> @@ -65,7 +65,8 @@
>>>>   #define AMDGPU_PTE_FRAG(x)    ((x & 0x1fULL) << 7)
>>>> -#define AMDGPU_PTE_PRT        (1ULL << 63)
>>>> +/* TILED for VEGA10, reserved for older ASICs  */
>>>> +#define AMDGPU_PTE_PRT        (1ULL << 51)
>>>>   /* VEGA10 only */
>>>>   #define AMDGPU_PTE_MTYPE(a)    ((uint64_t)a << 57)
>>>
>>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx