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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxx>
Tested-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>

with gputop [1].

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

regards
Philipp



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux