Re: [PATCH i-g-t 2/7] intel-gpu-overlay: Consolidate perf PMU access to library

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

 



Quoting Tvrtko Ursulin (2017-09-25 16:14:57)
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> ---
>  lib/igt_perf.c           | 33 +++++++++++++++++++++++++++++++++
>  lib/igt_perf.h           |  2 ++
>  overlay/gem-interrupts.c | 16 +---------------
>  overlay/gpu-freq.c       | 22 ++--------------------
>  overlay/gpu-top.c        | 32 ++++++++------------------------
>  overlay/power.c          | 17 +----------------
>  overlay/rc6.c            | 24 +++---------------------
>  7 files changed, 50 insertions(+), 96 deletions(-)
> 
> diff --git a/lib/igt_perf.c b/lib/igt_perf.c
> index 45cccff0ae53..0fa5ae3acb66 100644
> --- a/lib/igt_perf.c
> +++ b/lib/igt_perf.c
> @@ -2,6 +2,8 @@
>  #include <fcntl.h>
>  #include <unistd.h>
>  #include <stdlib.h>
> +#include <string.h>
> +#include <errno.h>
>  
>  #include "igt_perf.h"
>  
> @@ -24,3 +26,34 @@ uint64_t i915_type_id(void)
>         return strtoull(buf, 0, 0);
>  }
>  
> +static int _perf_open(int config, int group, int format)
> +{
> +       struct perf_event_attr attr;
> +
> +       memset(&attr, 0, sizeof (attr));
> +
> +       attr.type = i915_type_id();
> +       if (attr.type == 0)
> +               return -ENOENT;
> +
> +       attr.config = config;
> +
> +       if (group >= 0)
> +               format &= ~PERF_FORMAT_GROUP;
> +
> +       attr.read_format = format;
> +
> +       return perf_event_open(&attr, -1, 0, group, 0);
> +

Stray \n.

> +}
> +
> +int perf_i915_open(int config)
> +{
> +       return _perf_open(config, -1, PERF_FORMAT_TOTAL_TIME_ENABLED);
> +}
> +
> +int perf_i915_open_group(int config, int group)
> +{
> +       return _perf_open(config, group,
> +                         PERF_FORMAT_TOTAL_TIME_ENABLED | PERF_FORMAT_GROUP);
> +}
> diff --git a/lib/igt_perf.h b/lib/igt_perf.h
> index a80b311cd1d1..8e674c3a3755 100644
> --- a/lib/igt_perf.h
> +++ b/lib/igt_perf.h
> @@ -62,5 +62,7 @@ perf_event_open(struct perf_event_attr *attr,
>  }
>  
>  uint64_t i915_type_id(void);
> +int perf_i915_open(int config);
> +int perf_i915_open_group(int config, int group);
>  
>  #endif /* I915_PERF_H */
> diff --git a/overlay/gem-interrupts.c b/overlay/gem-interrupts.c
> index 7ba54fcd487d..a84aef0398a7 100644
> --- a/overlay/gem-interrupts.c
> +++ b/overlay/gem-interrupts.c
> @@ -36,20 +36,6 @@
>  #include "gem-interrupts.h"
>  #include "debugfs.h"
>  
> -static int perf_open(void)
> -{
> -       struct perf_event_attr attr;
> -
> -       memset(&attr, 0, sizeof (attr));
> -
> -       attr.type = i915_type_id();
> -       if (attr.type == 0)
> -               return -ENOENT;
> -       attr.config = I915_PERF_INTERRUPTS;
> -
> -       return perf_event_open(&attr, -1, 0, -1, 0);
> -}
> -
>  static long long debugfs_read(void)
>  {
>         char buf[8192], *b;
> @@ -127,7 +113,7 @@ int gem_interrupts_init(struct gem_interrupts *irqs)
>  {
>         memset(irqs, 0, sizeof(*irqs));
>  
> -       irqs->fd = perf_open();
> +       irqs->fd = perf_i915_open(I915_PERF_INTERRUPTS);
>         if (irqs->fd < 0 && interrupts_read() < 0)
>                 irqs->error = ENODEV;
>  
> diff --git a/overlay/gpu-freq.c b/overlay/gpu-freq.c
> index 7f29b1aa986e..76c5ed9acfd1 100644
> --- a/overlay/gpu-freq.c
> +++ b/overlay/gpu-freq.c
> @@ -33,30 +33,12 @@
>  #include "gpu-freq.h"
>  #include "debugfs.h"
>  
> -static int perf_i915_open(int config, int group)
> -{
> -       struct perf_event_attr attr;
> -
> -       memset(&attr, 0, sizeof (attr));
> -
> -       attr.type = i915_type_id();
> -       if (attr.type == 0)
> -               return -ENOENT;
> -       attr.config = config;
> -
> -       attr.read_format = PERF_FORMAT_TOTAL_TIME_ENABLED;
> -       if (group == -1)
> -               attr.read_format |= PERF_FORMAT_GROUP;
> -
> -       return perf_event_open(&attr, -1, 0, group, 0);
> -}
> -
>  static int perf_open(void)
>  {
>         int fd;
>  
> -       fd = perf_i915_open(I915_PERF_ACTUAL_FREQUENCY, -1);
> -       if (perf_i915_open(I915_PERF_REQUESTED_FREQUENCY, fd) < 0) {
> +       fd = perf_i915_open_group(I915_PERF_ACTUAL_FREQUENCY, -1);
> +       if (perf_i915_open_group(I915_PERF_REQUESTED_FREQUENCY, fd) < 0) {
>                 close(fd);
>                 fd = -1;
>         }
> diff --git a/overlay/gpu-top.c b/overlay/gpu-top.c
> index 06f489dfdc83..812f47d5aced 100644
> --- a/overlay/gpu-top.c
> +++ b/overlay/gpu-top.c
> @@ -48,24 +48,6 @@
>  #define I915_PERF_RING_WAIT(n) (__I915_PERF_RING(n) + 1)
>  #define I915_PERF_RING_SEMA(n) (__I915_PERF_RING(n) + 2)
>  
> -static int perf_i915_open(int config, int group)
> -{
> -       struct perf_event_attr attr;
> -
> -       memset(&attr, 0, sizeof (attr));
> -
> -       attr.type = i915_type_id();
> -       if (attr.type == 0)
> -               return -ENOENT;
> -       attr.config = config;
> -
> -       attr.read_format = PERF_FORMAT_TOTAL_TIME_ENABLED;
> -       if (group == -1)
> -               attr.read_format |= PERF_FORMAT_GROUP;
> -
> -       return perf_event_open(&attr, -1, 0, group, 0);
> -}
> -
>  static int perf_init(struct gpu_top *gt)
>  {
>         const char *names[] = {
> @@ -77,27 +59,29 @@ static int perf_init(struct gpu_top *gt)
>         };
>         int n;
>  
> -       gt->fd = perf_i915_open(I915_PERF_RING_BUSY(0), -1);
> +       gt->fd = perf_i915_open_group(I915_PERF_RING_BUSY(0), -1);
>         if (gt->fd < 0)
>                 return -1;
>  
> -       if (perf_i915_open(I915_PERF_RING_WAIT(0), gt->fd) >= 0)
> +       if (perf_i915_open_group(I915_PERF_RING_WAIT(0), gt->fd) >= 0)
>                 gt->have_wait = 1;
>  
> -       if (perf_i915_open(I915_PERF_RING_SEMA(0), gt->fd) >= 0)
> +       if (perf_i915_open_group(I915_PERF_RING_SEMA(0), gt->fd) >= 0)
>                 gt->have_sema = 1;
>  
>         gt->ring[0].name = names[0];
>         gt->num_rings = 1;
>  
>         for (n = 1; names[n]; n++) {
> -               if (perf_i915_open(I915_PERF_RING_BUSY(n), gt->fd) >= 0) {
> +               if (perf_i915_open_group(I915_PERF_RING_BUSY(n), gt->fd) >= 0) {
>                         if (gt->have_wait &&
> -                           perf_i915_open(I915_PERF_RING_WAIT(n), gt->fd) < 0)
> +                           perf_i915_open_group(I915_PERF_RING_WAIT(n),
> +                                                gt->fd) < 0)
>                                 return -1;
>  
>                         if (gt->have_sema &&
> -                           perf_i915_open(I915_PERF_RING_SEMA(n), gt->fd) < 0)
> +                           perf_i915_open_group(I915_PERF_RING_SEMA(n),
> +                                                gt->fd) < 0)
>                                 return -1;
>  
>                         gt->ring[gt->num_rings++].name = names[n];
> diff --git a/overlay/power.c b/overlay/power.c
> index 84d860cae40c..dd4aec6bffd9 100644
> --- a/overlay/power.c
> +++ b/overlay/power.c
> @@ -38,21 +38,6 @@
>  
>  /* XXX Is this exposed through RAPL? */
>  
> -static int perf_open(void)
> -{
> -       struct perf_event_attr attr;
> -
> -       memset(&attr, 0, sizeof (attr));
> -
> -       attr.type = i915_type_id();
> -       if (attr.type == 0)
> -               return -1;
> -       attr.config = I915_PERF_ENERGY;
> -
> -       attr.read_format = PERF_FORMAT_TOTAL_TIME_ENABLED;
> -       return perf_event_open(&attr, -1, 0, -1, 0);
> -}
> -
>  int power_init(struct power *power)
>  {
>         char buf[4096];
> @@ -60,7 +45,7 @@ int power_init(struct power *power)
>  
>         memset(power, 0, sizeof(*power));
>  
> -       power->fd = perf_open();
> +       power->fd = perf_i915_open(I915_PERF_ENERGY);
>         if (power->fd != -1)
>                 return 0;
>  
> diff --git a/overlay/rc6.c b/overlay/rc6.c
> index 3175bb22308f..46c975a557ff 100644
> --- a/overlay/rc6.c
> +++ b/overlay/rc6.c
> @@ -35,24 +35,6 @@
>  
>  #include "rc6.h"
>  
> -static int perf_i915_open(int config, int group)
> -{
> -       struct perf_event_attr attr;
> -
> -       memset(&attr, 0, sizeof (attr));
> -
> -       attr.type = i915_type_id();
> -       if (attr.type == 0)
> -               return -ENOENT;
> -       attr.config = config;
> -
> -       attr.read_format = PERF_FORMAT_TOTAL_TIME_ENABLED;
> -       if (group == -1)
> -               attr.read_format |= PERF_FORMAT_GROUP;
> -
> -       return perf_event_open(&attr, -1, 0, group, 0);
> -}
> -
>  #define RC6    (1<<0)
>  #define RC6p   (1<<1)
>  #define RC6pp  (1<<2)
> @@ -61,15 +43,15 @@ static int perf_open(unsigned *flags)
>  {
>         int fd;
>  
> -       fd = perf_i915_open(I915_PERF_RC6_RESIDENCY, -1);
> +       fd = perf_i915_open_group(I915_PERF_RC6_RESIDENCY, -1);
>         if (fd < 0)
>                 return -1;
>  
>         *flags |= RC6;
> -       if (perf_i915_open(I915_PERF_RC6p_RESIDENCY, fd) >= 0)
> +       if (perf_i915_open_group(I915_PERF_RC6p_RESIDENCY, fd) >= 0)
>                 *flags |= RC6p;
>  
> -       if (perf_i915_open(I915_PERF_RC6pp_RESIDENCY, fd) >= 0)
> +       if (perf_i915_open_group(I915_PERF_RC6pp_RESIDENCY, fd) >= 0)
>                 *flags |= RC6pp;
>  

Ok, sensible consolidation.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux