Re: [PATCH v2 04/15] drm/ttm: Export functions to initialize and finalize the ttm range manager standalone

[Christian König <christian.koenig@xxxxxxx>]

Am 18.05.21 um 10:26 schrieb Thomas Hellström:
> i915 mock selftests are run without the device set up. In order to be able
> to run the region related mock selftests, export functions in order for the
> TTM range manager to be set up without a device to attach it to.

From the code it looks good, but to be honest I don't think that this 
makes much sense from the organizational point of view.

If a self test exercises internals of TTM it should be moved into TTM as 
well.

Christian.

> Cc: Christian König <christian.koenig@xxxxxxx>
> Signed-off-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx>
> ---
>   drivers/gpu/drm/ttm/ttm_range_manager.c | 55 +++++++++++++++++--------
>   include/drm/ttm/ttm_bo_driver.h         | 23 +++++++++++
>   2 files changed, 61 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_range_manager.c b/drivers/gpu/drm/ttm/ttm_range_manager.c
> index b9d5da6e6a81..6957dfb0cf5a 100644
> --- a/drivers/gpu/drm/ttm/ttm_range_manager.c
> +++ b/drivers/gpu/drm/ttm/ttm_range_manager.c
> @@ -125,55 +125,76 @@ static const struct ttm_resource_manager_func ttm_range_manager_func = {
>   	.debug = ttm_range_man_debug
>   };
>   
> -int ttm_range_man_init(struct ttm_device *bdev,
> -		       unsigned type, bool use_tt,
> -		       unsigned long p_size)
> +struct ttm_resource_manager *
> +ttm_range_man_init_standalone(unsigned long size, bool use_tt)
>   {
>   	struct ttm_resource_manager *man;
>   	struct ttm_range_manager *rman;
>   
>   	rman = kzalloc(sizeof(*rman), GFP_KERNEL);
>   	if (!rman)
> -		return -ENOMEM;
> +		return ERR_PTR(-ENOMEM);
>   
>   	man = &rman->manager;
>   	man->use_tt = use_tt;
>   
>   	man->func = &ttm_range_manager_func;
>   
> -	ttm_resource_manager_init(man, p_size);
> +	ttm_resource_manager_init(man, size);
>   
> -	drm_mm_init(&rman->mm, 0, p_size);
> +	drm_mm_init(&rman->mm, 0, size);
>   	spin_lock_init(&rman->lock);
>   
> -	ttm_set_driver_manager(bdev, type, &rman->manager);
> +	return man;
> +}
> +EXPORT_SYMBOL(ttm_range_man_init_standalone);
> +
> +int ttm_range_man_init(struct ttm_device *bdev,
> +		       unsigned int type, bool use_tt,
> +		       unsigned long p_size)
> +{
> +	struct ttm_resource_manager *man;
> +
> +	man = ttm_range_man_init_standalone(p_size, use_tt);
> +	if (IS_ERR(man))
> +		return PTR_ERR(man);
> +
>   	ttm_resource_manager_set_used(man, true);
> +	ttm_set_driver_manager(bdev, type, man);
> +
>   	return 0;
>   }
>   EXPORT_SYMBOL(ttm_range_man_init);
>   
> +void ttm_range_man_fini_standalone(struct ttm_resource_manager *man)
> +{
> +	struct ttm_range_manager *rman = to_range_manager(man);
> +	struct drm_mm *mm = &rman->mm;
> +
> +	spin_lock(&rman->lock);
> +	drm_mm_clean(mm);
> +	drm_mm_takedown(mm);
> +	spin_unlock(&rman->lock);
> +
> +	ttm_resource_manager_cleanup(man);
> +	kfree(rman);
> +}
> +EXPORT_SYMBOL(ttm_range_man_fini_standalone);
> +
>   int ttm_range_man_fini(struct ttm_device *bdev,
>   		       unsigned type)
>   {
>   	struct ttm_resource_manager *man = ttm_manager_type(bdev, type);
> -	struct ttm_range_manager *rman = to_range_manager(man);
> -	struct drm_mm *mm = &rman->mm;
>   	int ret;
>   
>   	ttm_resource_manager_set_used(man, false);
> -
>   	ret = ttm_resource_manager_evict_all(bdev, man);
>   	if (ret)
>   		return ret;
>   
> -	spin_lock(&rman->lock);
> -	drm_mm_clean(mm);
> -	drm_mm_takedown(mm);
> -	spin_unlock(&rman->lock);
> -
> -	ttm_resource_manager_cleanup(man);
>   	ttm_set_driver_manager(bdev, type, NULL);
> -	kfree(rman);
> +	ttm_range_man_fini_standalone(man);
> +
>   	return 0;
>   }
>   EXPORT_SYMBOL(ttm_range_man_fini);
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index dbccac957f8f..734b1712ea72 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -321,6 +321,20 @@ int ttm_range_man_init(struct ttm_device *bdev,
>   		       unsigned type, bool use_tt,
>   		       unsigned long p_size);
>   
> +/**
> + * ttm_range_man_init_standalone - Initialize a ttm range manager without
> + * device interaction.
> + * @size: Size of the area to be managed in pages.
> + * @use_tt: The memory type requires tt backing.
> + *
> + * This function is intended for selftests. It initializes a range manager
> + * without any device interaction.
> + *
> + * Return: pointer to a range manager on success. Error pointer on failure.
> + */
> +struct ttm_resource_manager *
> +ttm_range_man_init_standalone(unsigned long size, bool use_tt);
> +
>   /**
>    * ttm_range_man_fini
>    *
> @@ -332,4 +346,13 @@ int ttm_range_man_init(struct ttm_device *bdev,
>   int ttm_range_man_fini(struct ttm_device *bdev,
>   		       unsigned type);
>   
> +/**
> + * ttm_range_man_fini_standalone
> + * @man: The range manager
> + *
> + * Tear down a range manager initialized with
> + * ttm_range_manager_init_standalone().
> + */
> +void ttm_range_man_fini_standalone(struct ttm_resource_manager *man);
> +
>   #endif