Re: [PATCH 03/13] drm/xe/sa: Cleanup internal BO data at device removal

[Matthew Brost <matthew.brost@intel.com>]

On Thu, Dec 12, 2024 at 02:01:31AM +0100, Michal Wajdeczko wrote:
> The underlying buffer object used by the sub-allocator is
> device managed and it will be released on device removal.
> Thus we should not wait with cleanup of related BO members
> until drm cleanup, but do that at the same time as BO.
> Add another device action and move there related cleanups.
> 

Kinda seems overkill to split this. I'd leave as is or just remove 'if
(!bo)' from xe_sa_bo_manager_fini.

Matt

> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_sa.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_sa.c b/drivers/gpu/drm/xe/xe_sa.c
> index eb314ca75355..a5e05237b646 100644
> --- a/drivers/gpu/drm/xe/xe_sa.c
> +++ b/drivers/gpu/drm/xe/xe_sa.c
> @@ -16,18 +16,19 @@
>  static void xe_sa_bo_manager_fini(struct drm_device *drm, void *arg)
>  {
>  	struct xe_sa_manager *sa_manager = arg;
> -	struct xe_bo *bo = sa_manager->bo;
> -
> -	if (!bo) {
> -		drm_err(drm, "no bo for sa manager\n");
> -		return;
> -	}
>  
>  	drm_suballoc_manager_fini(&sa_manager->base);
> +}
> +
> +static void sa_bo_manager_fini_bo(void *arg)
> +{
> +	struct xe_sa_manager *sa_manager = arg;
>  
>  	if (sa_manager->is_iomem)
>  		kvfree(sa_manager->cpu_ptr);
>  
> +	sa_manager->cpu_ptr = NULL;
> +	sa_manager->gpu_addr = 0;
>  	sa_manager->bo = NULL;
>  }
>  
> @@ -66,6 +67,10 @@ struct xe_sa_manager *xe_sa_bo_manager_init(struct xe_tile *tile, u32 size, u32
>  		memset(sa_manager->cpu_ptr, 0, bo->ttm.base.size);
>  	}
>  
> +	ret = devm_add_action_or_reset(xe->drm.dev, sa_bo_manager_fini_bo, sa_manager);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
>  	drm_suballoc_manager_init(&sa_manager->base, managed_size, align);
>  	ret = drmm_add_action_or_reset(&xe->drm, xe_sa_bo_manager_fini,
>  				       sa_manager);
> -- 
> 2.47.1
>