Messages in this thread | | | Date | Wed, 16 Mar 2022 14:26:08 +0000 | Subject | Re: [RFC PATCH 5/7] drm/ttm: add range busy check for range manager | From | Robert Beckett <> |
| |
On 16/03/2022 13:43, Christian König wrote: > Am 16.03.22 um 14:19 schrieb Robert Beckett: >> >> >> On 16/03/2022 09:54, Christian König wrote: >>> Am 15.03.22 um 19:04 schrieb Robert Beckett: >>>> RFC: do we want this to become a generic interface in >>>> ttm_resource_manager_func? >>>> >>>> RFC: would we prefer a different interface? e.g. >>>> for_each_resource_in_range or for_each_bo_in_range >>> >>> Well completely NAK to that. Why do you need that? >>> >>> The long term goal is to completely remove the range checks from TTM >>> instead. >> >> ah, I did not know that. >> I wanted it just to enable parity with a selftest that checks whether >> a range is allocated before initializing a given range with test data >> behind the allocator's back. It needs to check the range so that it >> doesn't destroy in use data. > > Mhm, of hand that doesn't sounds like a valid test case. Do you have the > code at hand?
https://patchwork.freedesktop.org/patch/478347/?series=101396&rev=1 this is where I replace an existing range check via drm_mm with the range check I added in this patch.
> >> >> I suppose we could add another drm_mm range tracker just for testing >> and shadow track each allocation in the range, but that seemed like a >> lot of extra infrastructure for no general runtime use. > > I have no idea what you mean with that.
I meant as a potential solution to tracking allocations without a range check, we would need to add something external. e.g. adding a shadow drm_mm range tracker, or a bitmask across the range, or stick objects in a list etc.
> >> >> would you mind explaining the rationale for removing range checks? It >> seems to me like a natural fit for a memory manager > > TTM manages buffer objects and resources, not address space. The > lpfn/fpfn parameter for the resource allocators are actually used as > just two independent parameters and not define any range. We just keep > the names for historical reasons. > > The only places we still use and compare them as ranges are > ttm_resource_compat() and ttm_bo_eviction_valuable() and I already have > patches to clean up those and move them into the backend resource handling.
except the ttm_range_manager seems to still use them as a range specifier.
If the general design going forward is to not consider ranges, how would you recommend constructing buffers around pre-allocated regions e.g. uefi frame buffers who's range is dictated externally?
> > Regards, > Christian. > >> >>> >>> Regards, >>> Christian. >>> >>>> >>>> Signed-off-by: Robert Beckett <bob.beckett@collabora.com> >>>> --- >>>> drivers/gpu/drm/ttm/ttm_range_manager.c | 21 +++++++++++++++++++++ >>>> include/drm/ttm/ttm_range_manager.h | 3 +++ >>>> 2 files changed, 24 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/ttm/ttm_range_manager.c >>>> b/drivers/gpu/drm/ttm/ttm_range_manager.c >>>> index 8cd4f3fb9f79..5662627bb933 100644 >>>> --- a/drivers/gpu/drm/ttm/ttm_range_manager.c >>>> +++ b/drivers/gpu/drm/ttm/ttm_range_manager.c >>>> @@ -206,3 +206,24 @@ int ttm_range_man_fini_nocheck(struct >>>> ttm_device *bdev, >>>> return 0; >>>> } >>>> EXPORT_SYMBOL(ttm_range_man_fini_nocheck); >>>> + >>>> +/** >>>> + * ttm_range_man_range_busy - Check whether anything is allocated >>>> with a range >>>> + * >>>> + * @man: memory manager to check >>>> + * @fpfn: first page number to check >>>> + * @lpfn: last page number to check >>>> + * >>>> + * Return: true if anything allocated within the range, false >>>> otherwise. >>>> + */ >>>> +bool ttm_range_man_range_busy(struct ttm_resource_manager *man, >>>> + unsigned fpfn, unsigned lpfn) >>>> +{ >>>> + struct ttm_range_manager *rman = to_range_manager(man); >>>> + struct drm_mm *mm = &rman->mm; >>>> + >>>> + if (__drm_mm_interval_first(mm, PFN_PHYS(fpfn), PFN_PHYS(lpfn + >>>> 1) - 1)) >>>> + return true; >>>> + return false; >>>> +} >>>> +EXPORT_SYMBOL(ttm_range_man_range_busy); >>>> diff --git a/include/drm/ttm/ttm_range_manager.h >>>> b/include/drm/ttm/ttm_range_manager.h >>>> index 7963b957e9ef..86794a3f9101 100644 >>>> --- a/include/drm/ttm/ttm_range_manager.h >>>> +++ b/include/drm/ttm/ttm_range_manager.h >>>> @@ -53,4 +53,7 @@ static __always_inline int >>>> ttm_range_man_fini(struct ttm_device *bdev, >>>> BUILD_BUG_ON(__builtin_constant_p(type) && type >= >>>> TTM_NUM_MEM_TYPES); >>>> return ttm_range_man_fini_nocheck(bdev, type); >>>> } >>>> + >>>> +bool ttm_range_man_range_busy(struct ttm_resource_manager *man, >>>> + unsigned fpfn, unsigned lpfn); >>>> #endif >>> >
| |