Re: [PATCH v6 1/7] drm/panthor: Add performance counter uAPI

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

On Mon, 15 Dec 2025 17:14:47 +0000
Lukas Zapolskas <lukas.zapolskas@arm.com> wrote:

> +/**
> + * enum drm_panthor_perf_block_type - Performance counter supported block types.
> + */
> +enum drm_panthor_perf_block_type {
> +	/** @DRM_PANTHOR_PERF_BLOCK_METADATA: Internal use only. */
> +	DRM_PANTHOR_PERF_BLOCK_METADATA = 0,

If it's internal-use-only, it shouldn't be exposed here. That's another
case of "I want an HW-agnostic uAPI so I can reconcile things up in
the KMD if things change on new HW, but I want it to match the current
HW mapping because it's simpler". If we really want to standardize the
block types that are exposed to the user (and after discussing it with
you, it seems you have valid reasons to want that), I'd rather have a
tightly packed enum with only the block types visible to the user. It
looks like METADATA is the only block we want to hide at the moment, so
it shouldn't be too hard to special-case this block.

> +
> +	/** @DRM_PANTHOR_PERF_BLOCK_FW: The FW counter block. */
> +	DRM_PANTHOR_PERF_BLOCK_FW,

Let's prefix those values with DRM_PANTHOR_PERF_BLOCK_TYPE_ to avoid
the blk vs blk-type confusion.

> +
> +	/** @DRM_PANTHOR_PERF_BLOCK_CSHW: The CSHW counter block. */
> +	DRM_PANTHOR_PERF_BLOCK_CSHW,
> +
> +	/** @DRM_PANTHOR_PERF_BLOCK_TILER: The tiler counter block. */
> +	DRM_PANTHOR_PERF_BLOCK_TILER,
> +
> +	/** @DRM_PANTHOR_PERF_BLOCK_MEMSYS: A memsys counter block. */
> +	DRM_PANTHOR_PERF_BLOCK_MEMSYS,
> +
> +	/** @DRM_PANTHOR_PERF_BLOCK_SHADER: A shader core counter block. */
> +	DRM_PANTHOR_PERF_BLOCK_SHADER,
> +
> +	/** @DRM_PANTHOR_PERF_BLOCK_FIRST: Internal use only. */
> +	DRM_PANTHOR_PERF_BLOCK_FIRST = DRM_PANTHOR_PERF_BLOCK_FW,
> +
> +	/** @DRM_PANTHOR_PERF_BLOCK_LAST: Internal use only. */
> +	DRM_PANTHOR_PERF_BLOCK_LAST = DRM_PANTHOR_PERF_BLOCK_SHADER,
> +
> +	/** @DRM_PANTHOR_PERF_BLOCK_MAX: Internal use only. */
> +	DRM_PANTHOR_PERF_BLOCK_MAX = DRM_PANTHOR_PERF_BLOCK_LAST + 1,

I think I'd just go for

	DRM_PANTHOR_PERF_BLOCK_TYPE_COUNT,

and drop DRM_PANTHOR_PERF_BLOCK_{FIRST,LAST,MAX) definitions, with a
comment stating that DRM_PANTHOR_PERF_BLOCK_TYPE_COUNT must stay last
in this enum.

> +};