Re: [Intel-gfx] [igt-dev] [RFC i-g-t 1/1] intel-gpu-top: Support for client stats

2019-10-25 Thread Tvrtko Ursulin


On 25/10/2019 16:13, Chris Wilson wrote:

Quoting Tvrtko Ursulin (2019-10-25 15:24:10)

From: Tvrtko Ursulin 

Adds support for per-client engine busyness stats i915 exports in sysfs
and produces output like the below:

==
intel-gpu-top -  935/ 935 MHz;0% RC6; 14.73 Watts; 1097 irqs/s


Could we get "gpu / pkg Watts" pretty please?


Sure, next week or so.


Are irq/s interesting with execlists? Originally the idea was to say how
many times clients were sleeping and being woken up. Now we interrupt
to wipe the gpu's nose when it sneezes.



   IMC reads: 1401 MiB/s
  IMC writes:4 MiB/s

   ENGINE  BUSY MI_SEMA MI_WAIT
  Render/3D/0   63.73% |███   |  3%  0%
Blitter/09.53% |██▊   |  6%  0%
  Video/0   39.32% |███▊  | 16%  0%
  Video/1   15.62% |▋ |  0%  0%
   VideoEnhance/00.00% |  |  0%  0%

   PIDNAME RCS  BCS  VCS VECS
  4084gem_wsim |█▌ ||█  ||   ||   |
  4086gem_wsim |█▌ ||   ||███||   |
==

Apart from the existing physical engine utilization it now also shows
utilization per client and per engine class.

Signed-off-by: Tvrtko Ursulin 
---



+#define SYSFS_CLIENTS "/sys/class/drm/card0/clients"


We need to somehow pull the right card.


Yeah, as I said RFC and reference only. :)


Nothing shocking here. Where's the intel-gpu-overlay integration? ;)


Maybe intel-gpu-overlay should become an output plugin for intel_gpu_top. :)

Regards,

Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [igt-dev] [RFC i-g-t 1/1] intel-gpu-top: Support for client stats

2019-10-25 Thread Chris Wilson
Quoting Tvrtko Ursulin (2019-10-25 15:24:10)
> From: Tvrtko Ursulin 
> 
> Adds support for per-client engine busyness stats i915 exports in sysfs
> and produces output like the below:
> 
> ==
> intel-gpu-top -  935/ 935 MHz;0% RC6; 14.73 Watts; 1097 irqs/s

Could we get "gpu / pkg Watts" pretty please?

Are irq/s interesting with execlists? Originally the idea was to say how
many times clients were sleeping and being woken up. Now we interrupt
to wipe the gpu's nose when it sneezes.

> 
>   IMC reads: 1401 MiB/s
>  IMC writes:4 MiB/s
> 
>   ENGINE  BUSY MI_SEMA MI_WAIT
>  Render/3D/0   63.73% |███   |  3%  0%
>Blitter/09.53% |██▊   |  6%  0%
>  Video/0   39.32% |███▊  | 16%  0%
>  Video/1   15.62% |▋ |  0%  0%
>   VideoEnhance/00.00% |  |  0%  0%
> 
>   PIDNAME RCS  BCS  VCS VECS
>  4084gem_wsim |█▌ ||█  ||   ||   |
>  4086gem_wsim |█▌ ||   ||███||   |
> ==
> 
> Apart from the existing physical engine utilization it now also shows
> utilization per client and per engine class.
> 
> Signed-off-by: Tvrtko Ursulin 
> ---

> +#define SYSFS_CLIENTS "/sys/class/drm/card0/clients"

We need to somehow pull the right card.

Nothing shocking here. Where's the intel-gpu-overlay integration? ;)
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [igt-dev] [RFC i-g-t 1/1] intel-gpu-top: Support for client stats

2019-05-10 Thread Chris Wilson
Quoting Tvrtko Ursulin (2019-05-10 14:23:12)
> From: Tvrtko Ursulin 
> 
> Adds support for per-client engine busyness stats i915 exports in sysfs
> and produces output like the below:
> 
> ==
> intel-gpu-top -  935/ 935 MHz;0% RC6; 14.73 Watts; 1097 irqs/s
> 
>   IMC reads: 1401 MiB/s
>  IMC writes:4 MiB/s
> 
>   ENGINE  BUSY MI_SEMA MI_WAIT
>  Render/3D/0   63.73% |███   |  3%  0%
>Blitter/09.53% |██▊   |  6%  0%
>  Video/0   39.32% |███▊  | 16%  0%
>  Video/1   15.62% |▋ |  0%  0%
>   VideoEnhance/00.00% |  |  0%  0%
> 
>   PIDNAME RCS  BCS  VCS VECS
>  4084gem_wsim |█▌ ||█  ||   ||   |
>  4086gem_wsim |█▌ ||   ||███||   |
> ==
> 
> Apart from the existing physical engine utilization it now also shows
> utilization per client and per engine class.
> 
> Signed-off-by: Tvrtko Ursulin 
> ---
>  tools/intel_gpu_top.c | 590 +-
>  1 file changed, 584 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> index cc8db7c539ed..88e1ad52d17c 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -659,8 +659,403 @@ static void pmu_sample(struct engines *engines)
> }
>  }
>  
> +enum client_status {
> +   FREE = 0, /* mbz */
> +   ALIVE,
> +   PROBE
> +};
> +
> +struct clients;
> +
> +struct client {
> +   struct clients *clients;
> +
> +   enum client_status status;
> +   unsigned int id;
> +   unsigned int pid;
> +   char name[128];
> +   unsigned int samples;
> +   unsigned long total;
> +   struct engines *engines;
> +   unsigned long *val;
> +   uint64_t *last;
> +};
> +
> +struct engine_class {
> +   unsigned int class;
> +   const char *name;
> +   unsigned int num_engines;
> +};
> +
> +struct clients {
> +   unsigned int num_classes;
> +   struct engine_class *class;
> +
> +   unsigned int num_clients;
> +   struct client *client;
> +};
> +
> +#define for_each_client(clients, c, tmp) \
> +   for ((tmp) = (clients)->num_clients, c = (clients)->client; \
> +(tmp > 0); (tmp)--, (c)++)
> +
> +#define SYSFS_ENABLE "/sys/class/drm/card0/clients/enable_stats"
> +
> +bool __stats_enabled;
> +
> +static int __set_stats(bool val)
> +{
> +   int fd, ret;
> +
> +   fd = open(SYSFS_ENABLE, O_WRONLY);
> +   if (fd < 0)
> +   return -errno;
> +
> +   ret = write(fd, val ? "1" : "0", 2);
close(fd);

Might as well still be tidy on error when it's trivial to do so.

> +   if (ret < 0)
> +   return -errno;
> +   else if (ret < 2)
> +   return 1;
> +
> +   close(fd);
> +
> +   return 0;
> +}
> +
> +static void __restore_stats(void)
> +{
> +   int ret;
> +
> +   if (__stats_enabled)
> +   return;
> +
> +   ret = __set_stats(false);
> +   if (ret)
> +   fprintf(stderr, "Failed to disable per-client stats! (%d)\n",
> +   ret);
> +}
> +
> +static void __restore_stats_signal(int sig)
> +{
> +   exit(0);
> +}
> +
> +static int enable_stats(void)
> +{
> +   int fd, ret;
> +
> +   fd = open(SYSFS_ENABLE, O_RDONLY);
> +   if (fd < 0)
> +   return -errno;
> +
> +   close(fd);
> +
> +   __stats_enabled = filename_to_u64(SYSFS_ENABLE, 10);
> +   if (__stats_enabled)
> +   return 0;
> +
> +   ret = __set_stats(true);
> +   if (!ret) {
> +   if (atexit(__restore_stats))
> +   fprintf(stderr, "Failed to register exit handler!");
> +
> +   if (signal(SIGINT, __restore_stats_signal))
> +   fprintf(stderr, "Failed to register signal handler!");

That really suggests an alternative mechanism where the stats are only
active for as long as the open(sysfs/stats) is. However, iirc, sysfs
doesn't allow us to hook into the open!

In which case we could hook into the write, and keep it enabled for as
long as the user write("1") until the fd is closed. Food for thought, I
hope you convince me we don't need optional stats in the first place :)

> +   } else {
> +   fprintf(stderr, "Failed to enable per-client stats! (%d)\n",
> +   ret);
> +   }
> +
> +   return ret;
> +}
> +
> +static struct clients *init_clients(void)
> +{
> +   struct clients *clients = malloc(sizeof(*clients));

We purport this to be a user-facing tool, it should