[PATCH] drm/amdgpu: fix gtt_mgr bo's offset

[PATCH] drm/amdgpu: fix gtt_mgr bo's offset

[Flora Cui]

Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609
Signed-off-by: Flora Cui <Flora.Cui@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 262e872..b266cf7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -128,8 +128,8 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
 
 	if (!r) {
 		mem->start = node->start;
-		tbo->offset = (tbo->mem.start << PAGE_SHIFT) +
-		    tbo->bdev->man[tbo->mem.mem_type].gpu_offset;
+		tbo->offset = (mem->start << PAGE_SHIFT) +
+		    tbo->bdev->man[mem->mem_type].gpu_offset;
 	}
 
 	return r;
-- 
2.7.4

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

Re: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset

[zhoucm1]

On 2016年09月20日 17:09, Flora Cui wrote:
> Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609
> Signed-off-by: Flora Cui <Flora.Cui@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> index 262e872..b266cf7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> @@ -128,8 +128,8 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
>   
>   	if (!r) {
>   		mem->start = node->start;
> -		tbo->offset = (tbo->mem.start << PAGE_SHIFT) +
> -		    tbo->bdev->man[tbo->mem.mem_type].gpu_offset;
> +		tbo->offset = (mem->start << PAGE_SHIFT) +
> +		    tbo->bdev->man[mem->mem_type].gpu_offset;
mem->mem_type seems not set yet as well if I am correct.
IIRC, I feel it's better that tbo->offset is set out of this function 
after get_node successfully.

Regards,
David Zhou

>   	}
>   
>   	return r;

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

Re: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset

[Flora Cui]

On Tue, Sep 20, 2016 at 06:19:33PM +0800, zhoucm1 wrote:
> 
> 
> On 2016年09月20日 17:09, Flora Cui wrote:
> >Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609
> >Signed-off-by: Flora Cui <Flora.Cui@amd.com>
> >---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> >index 262e872..b266cf7 100644
> >--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> >+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> >@@ -128,8 +128,8 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
> >  	if (!r) {
> >  		mem->start = node->start;
> >-		tbo->offset = (tbo->mem.start << PAGE_SHIFT) +
> >-		    tbo->bdev->man[tbo->mem.mem_type].gpu_offset;
> >+		tbo->offset = (mem->start << PAGE_SHIFT) +
> >+		    tbo->bdev->man[mem->mem_type].gpu_offset;
> mem->mem_type seems not set yet as well if I am correct.
> IIRC, I feel it's better that tbo->offset is set out of this function after
> get_node successfully.
> 
how about change to tbo->bdev->man[TTM_PL_TT].gpu_offset? since
gtt_mgr is for GTT only.

> Regards,
> David Zhou
> 
> >  	}
> >  	return r;
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset

[Christian König]

Am 20.09.2016 um 12:19 schrieb zhoucm1:
>
>
> On 2016年09月20日 17:09, Flora Cui wrote:
>> Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609
>> Signed-off-by: Flora Cui <Flora.Cui@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>> index 262e872..b266cf7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>> @@ -128,8 +128,8 @@ int amdgpu_gtt_mgr_alloc(struct 
>> ttm_mem_type_manager *man,
>>         if (!r) {
>>           mem->start = node->start;
>> -        tbo->offset = (tbo->mem.start << PAGE_SHIFT) +
>> - tbo->bdev->man[tbo->mem.mem_type].gpu_offset;
>> +        tbo->offset = (mem->start << PAGE_SHIFT) +
>> +            tbo->bdev->man[mem->mem_type].gpu_offset;
> mem->mem_type seems not set yet as well if I am correct.
> IIRC, I feel it's better that tbo->offset is set out of this function 
> after get_node successfully.

The patch itself is indeed not correct, but it points me to the bug I've 
been searching for weeks now.

So thanks a lot for this.

Regards,
Christian.

>
> Regards,
> David Zhou
>
>>       }
>>         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

Re: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset

[Christian König]

Am 20.09.2016 um 12:41 schrieb Flora Cui:
> On Tue, Sep 20, 2016 at 06:19:33PM +0800, zhoucm1 wrote:
>>
>> On 2016年09月20日 17:09, Flora Cui wrote:
>>> Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609
>>> Signed-off-by: Flora Cui <Flora.Cui@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>> index 262e872..b266cf7 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>> @@ -128,8 +128,8 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
>>>   	if (!r) {
>>>   		mem->start = node->start;
>>> -		tbo->offset = (tbo->mem.start << PAGE_SHIFT) +
>>> -		    tbo->bdev->man[tbo->mem.mem_type].gpu_offset;
>>> +		tbo->offset = (mem->start << PAGE_SHIFT) +
>>> +		    tbo->bdev->man[mem->mem_type].gpu_offset;
>> mem->mem_type seems not set yet as well if I am correct.
>> IIRC, I feel it's better that tbo->offset is set out of this function after
>> get_node successfully.
>>
> how about change to tbo->bdev->man[TTM_PL_TT].gpu_offset? since
> gtt_mgr is for GTT only.

Not 100% correct either.

The problem is that sometimes a GTT MM node will be bound for a BO which 
is still in VRAM (e.g. on eviction for example).

So you need to check if (&tbo->mem == mem) and only then update the 
tbo's offset.

Regards,
Christian.

>
>> Regards,
>> David Zhou
>>
>>>   	}
>>>   	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

Re: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset

[Flora Cui]

[-- Attachment #1: Type: text/plain, Size: 2123 bytes --]

Does this change make sense? 

On Tue, Sep 20, 2016 at 01:02:20PM +0200, Christian König wrote:
> Am 20.09.2016 um 12:41 schrieb Flora Cui:
> >On Tue, Sep 20, 2016 at 06:19:33PM +0800, zhoucm1 wrote:
> >>
> >>On 2016年09月20日 17:09, Flora Cui wrote:
> >>>Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609
> >>>Signed-off-by: Flora Cui <Flora.Cui-5C7GfCeVMHo@public.gmane.org>
> >>>---
> >>>  drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++--
> >>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> >>>index 262e872..b266cf7 100644
> >>>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> >>>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> >>>@@ -128,8 +128,8 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
> >>>  	if (!r) {
> >>>  		mem->start = node->start;
> >>>-		tbo->offset = (tbo->mem.start << PAGE_SHIFT) +
> >>>-		    tbo->bdev->man[tbo->mem.mem_type].gpu_offset;
> >>>+		tbo->offset = (mem->start << PAGE_SHIFT) +
> >>>+		    tbo->bdev->man[mem->mem_type].gpu_offset;
> >>mem->mem_type seems not set yet as well if I am correct.
> >>IIRC, I feel it's better that tbo->offset is set out of this function after
> >>get_node successfully.
> >>
> >how about change to tbo->bdev->man[TTM_PL_TT].gpu_offset? since
> >gtt_mgr is for GTT only.
> 
> Not 100% correct either.
> 
> The problem is that sometimes a GTT MM node will be bound for a BO which is
> still in VRAM (e.g. on eviction for example).
> 
> So you need to check if (&tbo->mem == mem) and only then update the tbo's
> offset.
> 
> Regards,
> Christian.
> 
> >
> >>Regards,
> >>David Zhou
> >>
> >>>  	}
> >>>  	return r;
> >_______________________________________________
> >amd-gfx mailing list
> >amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> >https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[-- Attachment #2: 0001-drm-amdgpu-fix-gtt_mgr-bo-s-offset.patch --]
[-- Type: text/x-diff, Size: 1138 bytes --]

>From 411f93d78e8f3691ce480146374c1e603c2c6db6 Mon Sep 17 00:00:00 2001
From: Flora Cui <Flora.Cui-5C7GfCeVMHo@public.gmane.org>
Date: Tue, 20 Sep 2016 17:07:31 +0800
Subject: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset

Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609
Signed-off-by: Flora Cui <Flora.Cui-5C7GfCeVMHo@public.gmane.org>
Reviewed-by: Hawking Zhang <Hawking.Zhang-5C7GfCeVMHo@public.gmane.org>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 262e872..f86c844 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -128,8 +128,9 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
 
 	if (!r) {
 		mem->start = node->start;
-		tbo->offset = (tbo->mem.start << PAGE_SHIFT) +
-		    tbo->bdev->man[tbo->mem.mem_type].gpu_offset;
+		if (&tbo->mem == mem)
+			tbo->offset = (tbo->mem.start << PAGE_SHIFT) +
+			    tbo->bdev->man[tbo->mem.mem_type].gpu_offset;
 	}
 
 	return r;
-- 
2.7.4


[-- Attachment #3: Type: text/plain, Size: 154 bytes --]

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

Re: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset

[Flora Cui]

we need this fix for urgent promotion. so please please reply me asap.

On Tue, Sep 20, 2016 at 01:02:20PM +0200, Christian König wrote:
> Am 20.09.2016 um 12:41 schrieb Flora Cui:
> >On Tue, Sep 20, 2016 at 06:19:33PM +0800, zhoucm1 wrote:
> >>
> >>On 2016年09月20日 17:09, Flora Cui wrote:
> >>>Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609
> >>>Signed-off-by: Flora Cui <Flora.Cui@amd.com>
> >>>---
> >>>  drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++--
> >>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> >>>index 262e872..b266cf7 100644
> >>>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> >>>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> >>>@@ -128,8 +128,8 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
> >>>  	if (!r) {
> >>>  		mem->start = node->start;
> >>>-		tbo->offset = (tbo->mem.start << PAGE_SHIFT) +
> >>>-		    tbo->bdev->man[tbo->mem.mem_type].gpu_offset;
> >>>+		tbo->offset = (mem->start << PAGE_SHIFT) +
> >>>+		    tbo->bdev->man[mem->mem_type].gpu_offset;
> >>mem->mem_type seems not set yet as well if I am correct.
> >>IIRC, I feel it's better that tbo->offset is set out of this function after
> >>get_node successfully.
> >>
> >how about change to tbo->bdev->man[TTM_PL_TT].gpu_offset? since
> >gtt_mgr is for GTT only.
> 
> Not 100% correct either.
> 
> The problem is that sometimes a GTT MM node will be bound for a BO which is
> still in VRAM (e.g. on eviction for example).
> 
> So you need to check if (&tbo->mem == mem) and only then update the tbo's
> offset.
> 
> Regards,
> Christian.
> 
> >
> >>Regards,
> >>David Zhou
> >>
> >>>  	}
> >>>  	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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset

[Christian König]

Am 20.09.2016 um 13:16 schrieb Flora Cui:
> Does this change make sense?

Yes, exactly. Patch is Reviewed-by: Christian König 
<christian.koenig@amd.com>.

But please be aware that there is at least one other bug I'm currently 
investigating.

Regards,
Christian.

>
> On Tue, Sep 20, 2016 at 01:02:20PM +0200, Christian König wrote:
>> Am 20.09.2016 um 12:41 schrieb Flora Cui:
>>> On Tue, Sep 20, 2016 at 06:19:33PM +0800, zhoucm1 wrote:
>>>> On 2016年09月20日 17:09, Flora Cui wrote:
>>>>> Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609
>>>>> Signed-off-by: Flora Cui <Flora.Cui@amd.com>
>>>>> ---
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++--
>>>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>> index 262e872..b266cf7 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>> @@ -128,8 +128,8 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
>>>>>   	if (!r) {
>>>>>   		mem->start = node->start;
>>>>> -		tbo->offset = (tbo->mem.start << PAGE_SHIFT) +
>>>>> -		    tbo->bdev->man[tbo->mem.mem_type].gpu_offset;
>>>>> +		tbo->offset = (mem->start << PAGE_SHIFT) +
>>>>> +		    tbo->bdev->man[mem->mem_type].gpu_offset;
>>>> mem->mem_type seems not set yet as well if I am correct.
>>>> IIRC, I feel it's better that tbo->offset is set out of this function after
>>>> get_node successfully.
>>>>
>>> how about change to tbo->bdev->man[TTM_PL_TT].gpu_offset? since
>>> gtt_mgr is for GTT only.
>> Not 100% correct either.
>>
>> The problem is that sometimes a GTT MM node will be bound for a BO which is
>> still in VRAM (e.g. on eviction for example).
>>
>> So you need to check if (&tbo->mem == mem) and only then update the tbo's
>> offset.
>>
>> Regards,
>> Christian.
>>
>>>> Regards,
>>>> David Zhou
>>>>
>>>>>   	}
>>>>>   	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


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

Re: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset

[zhoucm1]

On 2016年09月20日 19:40, Christian König wrote:
> Am 20.09.2016 um 13:16 schrieb Flora Cui:
>> Does this change make sense?
>
> Yes, exactly. Patch is Reviewed-by: Christian König 
> <christian.koenig@amd.com>.
Hi Flora,
Does this change work?

if (&tbo->mem == mem)
this checking seems always be false, right?

Regards,
David Zhou
>
> But please be aware that there is at least one other bug I'm currently 
> investigating.
>
> Regards,
> Christian.
>
>>
>> On Tue, Sep 20, 2016 at 01:02:20PM +0200, Christian König wrote:
>>> Am 20.09.2016 um 12:41 schrieb Flora Cui:
>>>> On Tue, Sep 20, 2016 at 06:19:33PM +0800, zhoucm1 wrote:
>>>>> On 2016年09月20日 17:09, Flora Cui wrote:
>>>>>> Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609
>>>>>> Signed-off-by: Flora Cui <Flora.Cui@amd.com>
>>>>>> ---
>>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++--
>>>>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c 
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>>> index 262e872..b266cf7 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>>>>> @@ -128,8 +128,8 @@ int amdgpu_gtt_mgr_alloc(struct 
>>>>>> ttm_mem_type_manager *man,
>>>>>>       if (!r) {
>>>>>>           mem->start = node->start;
>>>>>> -        tbo->offset = (tbo->mem.start << PAGE_SHIFT) +
>>>>>> - tbo->bdev->man[tbo->mem.mem_type].gpu_offset;
>>>>>> +        tbo->offset = (mem->start << PAGE_SHIFT) +
>>>>>> + tbo->bdev->man[mem->mem_type].gpu_offset;
>>>>> mem->mem_type seems not set yet as well if I am correct.
>>>>> IIRC, I feel it's better that tbo->offset is set out of this 
>>>>> function after
>>>>> get_node successfully.
>>>>>
>>>> how about change to tbo->bdev->man[TTM_PL_TT].gpu_offset? since
>>>> gtt_mgr is for GTT only.
>>> Not 100% correct either.
>>>
>>> The problem is that sometimes a GTT MM node will be bound for a BO 
>>> which is
>>> still in VRAM (e.g. on eviction for example).
>>>
>>> So you need to check if (&tbo->mem == mem) and only then update the 
>>> tbo's
>>> offset.
>>>
>>> Regards,
>>> Christian.
>>>
>>>>> Regards,
>>>>> David Zhou
>>>>>
>>>>>>       }
>>>>>>       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
>
>

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

Re: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset

[Flora Cui]

On Wed, Sep 21, 2016 at 09:54:04AM +0800, zhoucm1 wrote:
> 
> 
> On 2016年09月20日 19:40, Christian König wrote:
> >Am 20.09.2016 um 13:16 schrieb Flora Cui:
> >>Does this change make sense?
> >
> >Yes, exactly. Patch is Reviewed-by: Christian König
> ><christian.koenig@amd.com>.
> Hi Flora,
> Does this change work?
> 
> if (&tbo->mem == mem)
> this checking seems always be false, right?
mostly the check is true. eg
amdgpu_bo_pin_restricted->amdgpu_ttm_bind(), the mem is &bo->tbo.mem
> 
> Regards,
> David Zhou
> >
> >But please be aware that there is at least one other bug I'm currently
> >investigating.
> >
> >Regards,
> >Christian.
> >
> >>
> >>On Tue, Sep 20, 2016 at 01:02:20PM +0200, Christian König wrote:
> >>>Am 20.09.2016 um 12:41 schrieb Flora Cui:
> >>>>On Tue, Sep 20, 2016 at 06:19:33PM +0800, zhoucm1 wrote:
> >>>>>On 2016年09月20日 17:09, Flora Cui wrote:
> >>>>>>Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609
> >>>>>>Signed-off-by: Flora Cui <Flora.Cui@amd.com>
> >>>>>>---
> >>>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++--
> >>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>>>
> >>>>>>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> >>>>>>b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> >>>>>>index 262e872..b266cf7 100644
> >>>>>>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> >>>>>>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> >>>>>>@@ -128,8 +128,8 @@ int amdgpu_gtt_mgr_alloc(struct
> >>>>>>ttm_mem_type_manager *man,
> >>>>>>      if (!r) {
> >>>>>>          mem->start = node->start;
> >>>>>>-        tbo->offset = (tbo->mem.start << PAGE_SHIFT) +
> >>>>>>- tbo->bdev->man[tbo->mem.mem_type].gpu_offset;
> >>>>>>+        tbo->offset = (mem->start << PAGE_SHIFT) +
> >>>>>>+ tbo->bdev->man[mem->mem_type].gpu_offset;
> >>>>>mem->mem_type seems not set yet as well if I am correct.
> >>>>>IIRC, I feel it's better that tbo->offset is set out of this
> >>>>>function after
> >>>>>get_node successfully.
> >>>>>
> >>>>how about change to tbo->bdev->man[TTM_PL_TT].gpu_offset? since
> >>>>gtt_mgr is for GTT only.
> >>>Not 100% correct either.
> >>>
> >>>The problem is that sometimes a GTT MM node will be bound for a BO
> >>>which is
> >>>still in VRAM (e.g. on eviction for example).
> >>>
> >>>So you need to check if (&tbo->mem == mem) and only then update the
> >>>tbo's
> >>>offset.
> >>>
> >>>Regards,
> >>>Christian.
> >>>
> >>>>>Regards,
> >>>>>David Zhou
> >>>>>
> >>>>>>      }
> >>>>>>      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
> >
> >
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx