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.
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
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.
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.
would you mind explaining the rationale for removing range checks? It
seems to me like a natural fit for a memory manager
>
> 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
>