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

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

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

> On 17.03.2025 09:31, Boris Brezillon wrote:
> > On Sun, 16 Mar 2025 21:51:35 +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_heap.c | 15 +++++++++++++++
> > >  1 file changed, 15 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/panthor/panthor_heap.c b/drivers/gpu/drm/panthor/panthor_heap.c
> > > index db0285ce5812..520d1fcf5c36 100644
> > > --- a/drivers/gpu/drm/panthor/panthor_heap.c
> > > +++ b/drivers/gpu/drm/panthor/panthor_heap.c
> > > @@ -139,6 +139,10 @@ static int panthor_alloc_heap_chunk(struct panthor_device *ptdev,
> > >  	struct panthor_heap_chunk *chunk;
> > >  	struct panthor_heap_chunk_header *hdr;
> > >  	int ret;
> > > +#ifdef CONFIG_DEBUG_FS
> > > +	struct panthor_gem_object *obj;
> > > +	const char *label;
> > > +#endif
> > >
> > >  	chunk = kmalloc(sizeof(*chunk), GFP_KERNEL);
> > >  	if (!chunk)
> > > @@ -180,6 +184,17 @@ static int panthor_alloc_heap_chunk(struct panthor_device *ptdev,
> > >  	heap->chunk_count++;
> > >  	mutex_unlock(&heap->lock);
> > >
> > > +#ifdef CONFIG_DEBUG_FS
> > > +	obj = to_panthor_bo(chunk->bo->obj);
> > > +
> > > +	mutex_lock(&ptdev->gems_lock);
> > > +	list_add_tail(&obj->gems_node, &ptdev->gems);
> > > +	mutex_unlock(&ptdev->gems_lock);
> > > +
> > > +	label = kstrdup_const("\"Tiler heap chunk\"", GFP_KERNEL);  
> >
> > Do we really need the extra quotes around 'Tiler heap chunk'?  
> 
> We want them quoted like this so that the BO name can be told apart from the
> the extra tagging information (like modifiers) and any suffix sent down from gl

Feels like you want the kernel to use a format that's used by mesa but
not really standardized. What happens if we decide to change the
formatting there? Suddenly, kernel BOs become the outliers again, and
you're back to the inconsistency you were trying to avoid. My
recommendation would be to keep kernel BO labels simple (just a regular
string, no quoting, no extra props) and let the debugfs printer
segregate kernel BOs from user BOs to make the distinction clear.