Re: [PATCH v2] drm/amdgpu: Reserve shared fence slot in amdgpu_vm_update_directories

From: "Zhang, Jerry (Junwei)" <Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>
To: christian.koenig-5C7GfCeVMHo@public.gmane.org,
	"Michel Dänzer" <michel-otUistvHUpPR7s880joybQ@public.gmane.org>,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH v2] drm/amdgpu: Reserve shared fence slot in amdgpu_vm_update_directories
Date: Tue, 3 Jul 2018 17:03:04 +0800	[thread overview]
Message-ID: <5B3B3BC8.6090708@amd.com> (raw)
In-Reply-To: <f6dc6a5b-f6c4-5693-6bad-b2ce29ea54bc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On 07/03/2018 04:42 PM, Christian König wrote:
> Am 03.07.2018 um 10:12 schrieb Zhang, Jerry (Junwei):
>> On 07/03/2018 02:58 PM, Christian König wrote:
>>> Am 03.07.2018 um 03:59 schrieb Zhang, Jerry (Junwei):
>>>> On 07/02/2018 05:23 PM, Christian König wrote:
>>>>> [SNIP]
>>> The reservation_object_reserve_shared() function needs to be called before you make submissions to the hardware to make sure that you have a free slot after you made the submission.
>>>
>>> The problem is now that we always reserve one free slot during command submission, but for the page directory reservation object it can happen that this free slot is used by VM updates.
>>>
>>> So the safest approach is to make sure that we have another free slot after the VM updates are done.
>>>
>>> Michel's patch nearly did that, because we rarely have only one submission in amdgpu_vm_update_directories(). But it isn't 100% correct.
>>
>> Thanks to explain that, I think got your meaning that we should reserve slot for command submission after VM update.
>>
>> if so, shall we add reservation_object_reserve_shared() for both of them, since any of place may run out of slot.
>>
>> While the gap from me is I fail to find reserving slot during command submission,
>
> That's done by ttm_eu_reserve_buffers().
>
>> the root bo reservation seems to be used by VM update only(if not correct me).
>
> No, that is not correct. The root BO is also part of the list of BOs for the command submission used by ttm_eu_reserve_buffers() and ttm_eu_fence_buffer_objects().
>
> All VM updates will use the same slot, so we just need one extra reservation_object_reserve_shared() after all the VM updates are done.

Yes, thanks to point it out.
the root bo indeed needs at least one extra shared slot for command submission.

Jerry

>
> Regards,
> Christian.
>
>> command submission is likely to use bo's reservation from bo list rather than root bo reservation.
>>
>> BTW, reservation_object_reserve_shared() will extend the max num as 2 times if it's not enough.
>> (default value is 4)
>>
>> Jerry
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx