Patch is Tested-by: Rex Zhu <Rex.Zhu at amd.com> Best Regards Rex -----Original Message----- From: amd-gfx [mailto:amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of Christian König Sent: Tuesday, October 17, 2017 4:28 PM To: Zhou, David(ChunMing); amd-gfx at lists.freedesktop.org Subject: Re: [PATCH 2/2] drm/amdgpu: allow GTT overcommit during bind Am 17.10.2017 um 10:15 schrieb Chunming Zhou: > > > On 2017å¹´10æ??17æ?¥ 15:46, Christian König wrote: >> Am 17.10.2017 um 06:11 schrieb Chunming Zhou: >>> >>> >>> On 2017å¹´10æ??16æ?¥ 19:40, Christian König wrote: >>>> Am 16.10.2017 um 11:42 schrieb Chunming Zhou: >>>>> >>>>> >>>>> On 2017å¹´10æ??16æ?¥ 17:26, Christian König wrote: >>>>>> From: Christian König <christian.koenig at amd.com> >>>>>> >>>>>> While binding BOs to GART we need to allow a bit overcommit in >>>>>> the GTT domain. >>>>> If allowing overcommit, will the new node not over the GART mc >>>>> range? Which is also allowed? >>>> No that is checked separately by drm_mm_insert_node_in_range(). >>>> >>>> This is just to cover the case when we have a BO in GTT space which >>>> needs to be bound into the GART table. >>> Sorry, I missed that even gart BO is also without node during creating. >>> One nitpick, atomic64_sub(mem->num_pages, &mgr->available) will be >>> calculated twice for one gart bo create and pin, which results in >>> available isn't correct. >> >> Yeah, that is true but not as problematic as you think. >> >> The BO is transfered from the old mem object (without GART mapping) >> to the new mem object (with GART mapping). While doing this the old >> mem object is released, so we actually don't leak the memory. >> >> It's just that for a moment the BO is accounted twice, once for the >> old location and once for the new one. > If in memory pressure, I think we could allocate memory over the mgr > limitation. > For example, the limitation is 1024M, the BO allocation is 800M, when > the second counting, the available will be -576M, Yes, correct so far. That's why I've added the extra check in amdgpu_gtt_mgr_usage(). > since available is unsigned available is an atomic64_t and that is signed IIRC. We could cast mem->num_pages to signed as well to be double sure, but as far as I can see that should work. Regards, Christian. > > Regards, > David Zhou >> >> But we had that behavior previously as well, so that is not something >> introduced with this patch. >> >> Regards, >> Christian. >> >>> >>> Regards, >>> David Zhou >>>> >>>> Regards, >>>> Christian. >>>> >>>>> >>>>> Regards, >>>>> David Zhou >>>>>> Otherwise we can never use the full GART space when GART >>>>>> size=GTT size. >>>>>> >>>>>> Signed-off-by: Christian König <christian.koenig at amd.com> >>>>>> --- >>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 8 +++++--- >>>>>> 1 file changed, 5 insertions(+), 3 deletions(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >>>>>> index 0d15eb7d31d7..33535d347734 100644 >>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >>>>>> @@ -169,7 +169,8 @@ static int amdgpu_gtt_mgr_new(struct >>>>>> ttm_mem_type_manager *man, >>>>>> int r; >>>>>> spin_lock(&mgr->lock); >>>>>> - if (atomic64_read(&mgr->available) < mem->num_pages) { >>>>>> + if ((&tbo->mem == mem || tbo->mem.mem_type != TTM_PL_TT) && >>>>>> + atomic64_read(&mgr->available) < mem->num_pages) { >>>>>> spin_unlock(&mgr->lock); >>>>>> return 0; >>>>>> } >>>>>> @@ -244,8 +245,9 @@ static void amdgpu_gtt_mgr_del(struct >>>>>> ttm_mem_type_manager *man, >>>>>> uint64_t amdgpu_gtt_mgr_usage(struct ttm_mem_type_manager *man) >>>>>> { >>>>>> struct amdgpu_gtt_mgr *mgr = man->priv; >>>>>> + s64 result = man->size - atomic64_read(&mgr->available); >>>>>> - return (u64)(man->size - atomic64_read(&mgr->available)) * >>>>>> PAGE_SIZE; >>>>>> + return (result > 0 ? result : 0) * PAGE_SIZE; >>>>>> } >>>>>> /** >>>>>> @@ -265,7 +267,7 @@ static void amdgpu_gtt_mgr_debug(struct >>>>>> ttm_mem_type_manager *man, >>>>>> drm_mm_print(&mgr->mm, printer); >>>>>> spin_unlock(&mgr->lock); >>>>>> - drm_printf(printer, "man size:%llu pages, gtt >>>>>> available:%llu pages, usage:%lluMB\n", >>>>>> + drm_printf(printer, "man size:%llu pages, gtt available:%lld >>>>>> pages, usage:%lluMB\n", >>>>>> man->size, (u64)atomic64_read(&mgr->available), >>>>>> amdgpu_gtt_mgr_usage(man) >> 20); >>>>>> } >>>>> >>>> >>> >> > _______________________________________________ amd-gfx mailing list amd-gfx at lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
