Am 16.03.22 um 15:26 schrieb Robert Beckett:
>
> [SNIP]
> this is where I replace an existing range check via drm_mm with the
> range check I added in this patch.
Mhm, I still don't get the use case from the code, but I don't think it
matters any more.
>>> 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.
Ah! So you are trying to get access to the drm_mm inside the
ttm_range_manager and not add some additional range check function! Now
I got your use case.
>>> 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.
Yeah, because the range manager is the backend which handles ranges
using the drm_mm :)
> 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?
Call ttm_bo_mem_space() with the fpfn/lpfn filled in as required. See
function amdgpu_bo_create_kernel_at() for an example.
Regards,
Christian.
>
>>
>> Regards,
>> Christian.
>>
>>>
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>>
>>>>> Signed-off-by: Robert Beckett <[email protected]>
>>>>> ---
>>>>> 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
>>>>
>>