lkml.org 
[lkml]   [2022]   [Mar]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [RFC PATCH 5/7] drm/ttm: add range busy check for range manager
From


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
>>>
>

\
 
 \ /
  Last update: 2022-09-17 16:17    [W:0.082 / U:0.716 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site