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

[zhoucm1 <david1.zhou-5C7GfCeVMHo@public.gmane.org>]

On 2017年04月18日 11:13, Zhang, Jerry (Junwei) wrote:
> On 04/18/2017 10:47 AM, zhoucm1 wrote:
>>
>>
>> On 2017年04月18日 09:51, Zhang, Jerry (Junwei) wrote:
>>>
>>> Anyone could help to review it?
>>>
>>> On 04/17/2017 05:04 PM, Junwei Zhang wrote:
>>>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 5 +++++
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 1 +
>>>>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c  | 2 +-
>>>>   3 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 9ff445c..51aedf9 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> @@ -1269,6 +1269,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_TILED) {
>>>> +            flags |= AMDGPU_PTE_TILED;
>>>> +            flags &= ~AMDGPU_PTE_VALID;
>>>> +        }
>>>> +
>> How about clear operation?
>
> CLEAR op will clear all mapping with flag=0, put into free list, and 
> then clear them by amdgpu_vm_clear_freed().
Yes, I looked into wrong function amdgpu_vm_clear_invalids by low 
mistake. Sorry for that.

>
> When amdgpu_vm_bo_update() is performed, the mapping's flag is 0 now.
>
>>
>>>>           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 4904740..8d25914 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>>> @@ -70,6 +70,7 @@
>>>>   /* VEGA10 only */
>>>>   #define AMDGPU_PTE_MTYPE(a)    ((uint64_t)a << 57)
>>>>   #define AMDGPU_PTE_MTYPE_MASK    AMDGPU_PTE_MTYPE(3ULL)
>>>> +#define AMDGPU_PTE_TILED    (1ULL << 51)
>>>>
>>>>   /* How to programm VM fault handling */
>>>>   #define AMDGPU_VM_FAULT_STOP_NEVER    0
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> index 51a1919..6d033ae 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> @@ -324,7 +324,7 @@ static uint64_t gmc_v9_0_get_vm_pte_flags(struct
>>>> amdgpu_device *adev,
>>>>       }
>>>>
>>>>       if (flags & AMDGPU_VM_PAGE_PRT)
>>>> -        pte_flag |= AMDGPU_PTE_PRT;
>>>> +        pte_flag |= AMDGPU_PTE_TILED;
>> PTE_PRT name doesn't make sense?
>
> This naming is tricky for PRT feature, as there is not PRT bit on PTE 
> for pre-gfx9.
> So PTE_PRT is used in reserve bit of PTE.
> But for gfx9, there is a actual bit for PTE, it's better to use the 
> real one.
>
> BTW, all ASICs will use the general flag in UMD now.
> But different handling inside PTE.
Hmm, the alternative is ok to me, but you need to confirm with the 
author of name.
So Acked-by: Chunming Zhou <david1.zhou@amd.com>

Regards,
David Zhou
>
> Jerry
>
>>
>> Regards,
>> David Zhou
>>>>
>>>>       return pte_flag;
>>>>   }
>>>>
>>> _______________________________________________
>>> 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
> _______________________________________________
> 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