Re: [PATCH v2 4/4] drm/panthor: Display heap chunk entries in DebugFS GEMS file

[Boris Brezillon <boris.brezillon@collabora.com>]

On Wed, 19 Mar 2025 15:03:19 +0000
Adrián Larumbe <adrian.larumbe@collabora.com> wrote:

> Expand the driver's DebugFS GEMS file to display entries for the heap
> chunks' GEM objects, both those allocated at heap creation time through an
> ioctl(), or in response to a tiler OOM event.
> 
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> ---
>  drivers/gpu/drm/panthor/panthor_gem.c  | 22 +++++++++++-----------
>  drivers/gpu/drm/panthor/panthor_gem.h  |  2 ++
>  drivers/gpu/drm/panthor/panthor_heap.c |  3 +++
>  3 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c
> index f7eb413d88e7..252979473fdf 100644
> --- a/drivers/gpu/drm/panthor/panthor_gem.c
> +++ b/drivers/gpu/drm/panthor/panthor_gem.c
> @@ -22,16 +22,6 @@ static void panthor_gem_debugfs_bo_init(struct panthor_gem_object *bo)
>  	get_task_comm(bo->gems.creator.process_name, current->group_leader);
>  }
>  
> -static void panthor_gem_debugfs_bo_add(struct panthor_gem_object *bo)
> -{
> -	struct panthor_device *ptdev =  container_of(bo->base.base.dev,
> -						     struct panthor_device, base);
> -
> -	mutex_lock(&ptdev->gems.lock);
> -	list_add_tail(&bo->gems.node, &ptdev->gems.node);
> -	mutex_unlock(&ptdev->gems.lock);
> -}
> -
>  static void panthor_gem_debugfs_bo_rm(struct panthor_gem_object *bo)
>  {
>  	struct panthor_device *ptdev = container_of(bo->base.base.dev,
> @@ -44,10 +34,20 @@ static void panthor_gem_debugfs_bo_rm(struct panthor_gem_object *bo)
>  	list_del_init(&bo->gems.node);
>  	mutex_unlock(&ptdev->gems.lock);
>  }
> +
> +void panthor_gem_debugfs_bo_add(struct panthor_gem_object *bo)
> +{
> +	struct panthor_device *ptdev =  container_of(bo->base.base.dev,
> +						     struct panthor_device, base);
> +
> +	mutex_lock(&ptdev->gems.lock);
> +	list_add_tail(&bo->gems.node, &ptdev->gems.node);
> +	mutex_unlock(&ptdev->gems.lock);
> +}
>  #else
>  static void panthor_gem_debugfs_bo_init(struct panthor_gem_object *bo) {}
> -static void panthor_gem_debugfs_bo_add(struct panthor_gem_object *bo) {}
>  static void panthor_gem_debugfs_bo_rm(struct panthor_gem_object *bo) {}
> +void panthor_gem_debugfs_bo_add(struct panthor_gem_object *bo) {}

Let's just define all these helpers as inline functions in
panthor_gem.h in patch 3.

>  #endif
>  
>  static void panthor_gem_free_object(struct drm_gem_object *obj)
> diff --git a/drivers/gpu/drm/panthor/panthor_gem.h b/drivers/gpu/drm/panthor/panthor_gem.h
> index 7c896ec35801..e132f14bbef8 100644
> --- a/drivers/gpu/drm/panthor/panthor_gem.h
> +++ b/drivers/gpu/drm/panthor/panthor_gem.h
> @@ -132,6 +132,8 @@ panthor_gem_create_with_handle(struct drm_file *file,
>  void panthor_gem_bo_set_label(struct drm_gem_object *obj, const char *label);
>  void panthor_gem_kernel_bo_set_label(struct panthor_kernel_bo *bo, const char *label);
>  
> +void panthor_gem_debugfs_bo_add(struct panthor_gem_object *bo);
> +
>  static inline u64
>  panthor_kernel_bo_gpuva(struct panthor_kernel_bo *bo)
>  {
> diff --git a/drivers/gpu/drm/panthor/panthor_heap.c b/drivers/gpu/drm/panthor/panthor_heap.c
> index db0285ce5812..73cf43eb4a7b 100644
> --- a/drivers/gpu/drm/panthor/panthor_heap.c
> +++ b/drivers/gpu/drm/panthor/panthor_heap.c
> @@ -180,6 +180,9 @@ static int panthor_alloc_heap_chunk(struct panthor_device *ptdev,
>  	heap->chunk_count++;
>  	mutex_unlock(&heap->lock);
>  
> +	panthor_gem_kernel_bo_set_label(chunk->bo, "\"Tiler heap chunk\"");
> +	panthor_gem_debugfs_bo_add(to_panthor_bo(chunk->bo->obj));

I'd be tempted to label all the kernel BOs, not just the heap chunks,
and if we do that, we're probably better off changing the
kernel_bo_create() prototype to pass a label.

> +
>  	return 0;
>  
>  err_destroy_bo: