* [PATCH] drm/amdgpu: PRT support for gfx9
@ 2017-04-17 9:04 Junwei Zhang
[not found] ` <1492419870-2901-1-git-send-email-Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Junwei Zhang @ 2017-04-17 9:04 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Junwei Zhang
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;
+ }
+
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;
return pte_flag;
}
--
1.9.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu: PRT support for gfx9
[not found] ` <1492419870-2901-1-git-send-email-Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>
@ 2017-04-18 1:51 ` Zhang, Jerry (Junwei)
[not found] ` <58F57126.1040904-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Zhang, Jerry (Junwei) @ 2017-04-18 1:51 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: zhoucm1, Christian König, Mao, David
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;
> + }
> +
> 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;
>
> return pte_flag;
> }
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu: PRT support for gfx9
[not found] ` <58F57126.1040904-5C7GfCeVMHo@public.gmane.org>
@ 2017-04-18 2:47 ` zhoucm1
[not found] ` <58F57E45.2040208-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: zhoucm1 @ 2017-04-18 2:47 UTC (permalink / raw)
To: Zhang, Jerry (Junwei), amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Christian König, Mao, David
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?
>> 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?
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu: PRT support for gfx9
[not found] ` <58F57E45.2040208-5C7GfCeVMHo@public.gmane.org>
@ 2017-04-18 3:13 ` Zhang, Jerry (Junwei)
[not found] ` <58F5843F.3050700-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Zhang, Jerry (Junwei) @ 2017-04-18 3:13 UTC (permalink / raw)
To: zhoucm1, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Christian König, Mao, David
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().
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.
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu: PRT support for gfx9
[not found] ` <58F5843F.3050700-5C7GfCeVMHo@public.gmane.org>
@ 2017-04-18 3:25 ` zhoucm1
2017-04-18 11:25 ` Nicolai Hähnle
1 sibling, 0 replies; 7+ messages in thread
From: zhoucm1 @ 2017-04-18 3:25 UTC (permalink / raw)
To: Zhang, Jerry (Junwei), amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Christian König, Mao, David
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu: PRT support for gfx9
[not found] ` <58F5843F.3050700-5C7GfCeVMHo@public.gmane.org>
2017-04-18 3:25 ` zhoucm1
@ 2017-04-18 11:25 ` Nicolai Hähnle
[not found] ` <ee145d04-5a08-db82-6406-41bb208b81ca-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
1 sibling, 1 reply; 7+ messages in thread
From: Nicolai Hähnle @ 2017-04-18 11:25 UTC (permalink / raw)
To: Zhang, Jerry (Junwei), zhoucm1, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Christian König, Mao, David
On 18.04.2017 05: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;
Why do you need to explicitly clear the VALID bit here? I'd expect
whoever creates the mapping to already ensure that the VALID bit is cleared.
>>>> + }
>>>> +
>> 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().
>
> 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.
Wouldn't it be better to keep the AMDGPU_PTE_PRT name, and just make
sure that the bit isn't set on older ASICs (unless the bit is simply
ignored on older ASICs, in which case we don't have to bother either way)?
Cheers,
Nicolai
>
> BTW, all ASICs will use the general flag in UMD now.
> But different handling inside PTE.
>
> 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
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu: PRT support for gfx9
[not found] ` <ee145d04-5a08-db82-6406-41bb208b81ca-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-04-19 1:41 ` Zhang, Jerry (Junwei)
0 siblings, 0 replies; 7+ messages in thread
From: Zhang, Jerry (Junwei) @ 2017-04-19 1:41 UTC (permalink / raw)
To: Nicolai Hähnle, zhoucm1, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Christian König, Mao, David
On 04/18/2017 07:25 PM, Nicolai Hähnle wrote:
> On 18.04.2017 05: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;
>
> Why do you need to explicitly clear the VALID bit here? I'd expect whoever
> creates the mapping to already ensure that the VALID bit is cleared.
Yes, the VALID bit is clear actually.
Here clearing it explicitly is to notice that TILED bit should work with V=0.
Just double confirm and reminder to the reader.
>
>
>>>>> + }
>>>>> +
>>> 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().
>>
>> 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.
>
> Wouldn't it be better to keep the AMDGPU_PTE_PRT name, and just make sure that
> the bit isn't set on older ASICs (unless the bit is simply ignored on older
> ASICs, in which case we don't have to bother either way)?
Thanks for you advice.
It's a way to unify the PRT bit in PTE and it doesn't bother older ASIC as
reserved bit.
I will revise the patch.
BTW, actually I have discussed it with Christian before.
Then we thinked AMDGPU_PTE_PRT was a little hacking to set dummy bit in PTE and
consider to clear it next. Actually TILED bit is the real one for gfx9 PRT.
Jerry
>
> Cheers,
> Nicolai
>
>
>>
>> BTW, all ASICs will use the general flag in UMD now.
>> But different handling inside PTE.
>>
>> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-04-19 1:41 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-17 9:04 [PATCH] drm/amdgpu: PRT support for gfx9 Junwei Zhang
[not found] ` <1492419870-2901-1-git-send-email-Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>
2017-04-18 1:51 ` Zhang, Jerry (Junwei)
[not found] ` <58F57126.1040904-5C7GfCeVMHo@public.gmane.org>
2017-04-18 2:47 ` zhoucm1
[not found] ` <58F57E45.2040208-5C7GfCeVMHo@public.gmane.org>
2017-04-18 3:13 ` Zhang, Jerry (Junwei)
[not found] ` <58F5843F.3050700-5C7GfCeVMHo@public.gmane.org>
2017-04-18 3:25 ` zhoucm1
2017-04-18 11:25 ` Nicolai Hähnle
[not found] ` <ee145d04-5a08-db82-6406-41bb208b81ca-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-04-19 1:41 ` Zhang, Jerry (Junwei)
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.