| Message ID | 20250319150953.1634322-5-adrian.larumbe@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Panthor BO tagging and GEMS debug display | expand |
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:
On 19.03.2025 18:00, Boris Brezillon wrote: > 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. The only function that can so far be used from a different compilation unit is 'add'. The other two are internal to panthor_gem.c, so I'm incluned to keep them there as static functions, but move 'add' into the header file as a static inline function instead. > > #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. I think is a good idea going forward, but in keeping things tight I'd say doing it now might be an overkill, since the only user of tagged BO's at the moment is the gem debugfs knob. I think if in the future we find new ways of accounting or displaying labelled kernel BO's other than heap chunks, then we can expand the kernel_bo_create() argument list and then tag every single one of them at creation time. > > + > > return 0; > > > > err_destroy_bo: Adrian Larumbe
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) {} #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)); + return 0; err_destroy_bo:
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(-)