[PATCH v9 4/4] drm/panthor: show device-wide list of DRM GEM objects over DebugFS

[Adrián Larumbe]

On 18.04.2025 10:26, Boris Brezillon wrote:
On Fri, 18 Apr 2025 10:11:56 +0200
Boris Brezillon <boris.brezillon at collabora.com> wrote:

> On Fri, 18 Apr 2025 03:27:07 +0100
> Adrián Larumbe <adrian.larumbe at collabora.com> wrote:
>
> > +	static const char * const gem_state_flags_names[] = {
> > +		[PANTHOR_DEBUGFS_GEM_STATE_FLAG_IMPORTED] = "imported",

> FYI, the compiler seems to be happy with:
>
> 		[ffs(PANTHOR_DEBUGFS_GEM_STATE_FLAG_IMPORTED) - 1] = "imported",
>
> but I'm not sure we want to fix it this way. The other
> option would be to define bit pos in the enum and then
> define flags according to these bit pos:
>
> enum panthor_debugfs_gem_state_flags {
> 	PANTHOR_DEBUGFS_GEM_STATE_IMPORTED_BIT = 0,
> 	PANTHOR_DEBUGFS_GEM_STATE_EXPORTED_BIT = 1,
>
> 	/** @PANTHOR_DEBUGFS_GEM_STATE_FLAG_IMPORTED: GEM BO is PRIME imported. */
> 	PANTHOR_DEBUGFS_GEM_STATE_FLAG_IMPORTED = BIT(PANTHOR_DEBUGFS_GEM_STATE_IMPORTED_BIT),
>
> 	/** @PANTHOR_DEBUGFS_GEM_STATE_FLAG_EXPORTED: GEM BO is PRIME exported. */
> 	PANTHOR_DEBUGFS_GEM_STATE_FLAG_EXPORTED = BIT(PANTHOR_DEBUGFS_GEM_STATE_EXPORTED_BIT),
> };

I'm happy with this solution too. To be frank I thought of something like this
when I realised flag names array indices an bit values were askew by one, but
since the names array is constant and mostly meant to be read in the future by
programmers who want to add new flag meanings to the same table, I didn't think
of having a hole in index 0 as a problem.

> > +		[PANTHOR_DEBUGFS_GEM_STATE_FLAG_EXPORTED] = "exported",
>
> Okay, I think I know where the flag indexing issue comes from:
> PANTHOR_DEBUGFS_GEM_STATE_FLAG_xx are flags, not bit positions, so we
> can't use them as indices here.
>
> > +	};
> > +
> > +	static const char * const gem_usage_flags_names[] = {
> > +		[PANTHOR_DEBUGFS_GEM_USAGE_FLAG_KERNEL] = "kernel",
> > +		[PANTHOR_DEBUGFS_GEM_USAGE_FLAG_FW_MAPPED] = "fw-mapped",
>
> Same problem here.
>
> > +	};
> > +


Adrian Larumbe