[PATCH v2 3/3] drm/etnaviv: export client GPU usage statistics via fdinfo

[Philipp Zabel]

On Fri, Sep 16, 2022 at 05:12:05PM +0200, Lucas Stach wrote:
> This exposes a accumulated GPU active time per client via the
> fdinfo infrastructure.
> 
> Signed-off-by: Lucas Stach <l.stach at pengutronix.de>
> ---
> v2:
> - fix code style
> - switch to raw seq_printf
> - leave some breadcrumbs about the output format
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c | 40 ++++++++++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index b69edb40ae2a..c08748472f74 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -22,6 +22,7 @@
>  #include "etnaviv_gem.h"
>  #include "etnaviv_mmu.h"
>  #include "etnaviv_perfmon.h"
> +#include "common.xml.h"
>  
>  /*
>   * DRM operations:
> @@ -471,7 +472,44 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
>  	ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
>  };
>  
> -DEFINE_DRM_GEM_FOPS(fops);
> +static void etnaviv_fop_show_fdinfo(struct seq_file *m, struct file *f)
> +{
> +	struct drm_file *file = f->private_data;
> +	struct drm_device *dev = file->minor->dev;
> +	struct etnaviv_drm_private *priv = dev->dev_private;
> +	struct etnaviv_file_private *ctx = file->driver_priv;
> +
> +	/*
> +	 * For a description of the text output format used here, see
> +	 * Documentation/gpu/drm-usage-stats.rst.
> +	 */
> +	seq_printf(m, "drm-driver:\t%s\n", dev->driver->name);
> +	seq_printf(m, "drm-client-id:\t%u\n", ctx->id);
> +
> +	for (int i = 0; i < ETNA_MAX_PIPES; i++) {
> +		struct etnaviv_gpu *gpu = priv->gpu[i];
> +		char engine[8];

Maybe initialize this as well? See below.

> +		int cur = 0;
> +
> +		if (!gpu)
> +			continue;
> +
> +		if (gpu->identity.features & chipFeatures_PIPE_2D)
> +			cur = snprintf(engine, sizeof(engine), "2D");
> +		if (gpu->identity.features & chipFeatures_PIPE_3D)
> +			cur = snprintf(engine + cur, sizeof(engine) - cur,
> +				       "%s3D", cur ? "/" : "");

Does the NPU have either bit set? If not, this must not be forgotten to
be changed when NPU support is added, to avoid uninitalized use of the
engine variable.

Reviewed-by: Philipp Zabel <p.zabel at pengutronix.de>
Tested-by: Philipp Zabel <p.zabel at pengutronix.de>

with gputop [1].

[1] https://lore.kernel.org/dri-devel/20221111155844.3290531-1-tvrtko.ursulin@linux.intel.com/

regards
Philipp