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

From: zhoucm1 <david1.zhou-5C7GfCeVMHo@public.gmane.org>
To: "Christian König"
	<deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>,
	"Flora Cui" <flora.cui-5C7GfCeVMHo@public.gmane.org>
Cc: christian.koenig-5C7GfCeVMHo@public.gmane.org,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset
Date: Wed, 21 Sep 2016 09:54:04 +0800	[thread overview]
Message-ID: <57E1E83C.5070107@amd.com> (raw)
In-Reply-To: <ec255145-4ee3-caab-bfc5-644f4afba6ad-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>

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