Re: [igt-dev] [PATCH i-g-t 12/25] gem_wsim: Engine map support

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

 



Quoting Tvrtko Ursulin (2019-05-17 12:25:13)
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> 
> Support new i915 uAPI for configuring contexts with engine maps.
> 
> Please refer to the README file for more detailed explanation.
> 
> v2:
>  * Allow defining engine maps by class.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> ---
>  benchmarks/gem_wsim.c  | 211 +++++++++++++++++++++++++++++++++++------
>  benchmarks/wsim/README |  25 ++++-
>  2 files changed, 204 insertions(+), 32 deletions(-)
> 
> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
> index 60b7d32e22d4..e5b12e37490e 100644
> --- a/benchmarks/gem_wsim.c
> +++ b/benchmarks/gem_wsim.c
> @@ -57,6 +57,7 @@
>  #include "ewma.h"
>  
>  enum intel_engine_id {
> +       DEFAULT,
>         RCS,
>         BCS,
>         VCS,
> @@ -81,7 +82,8 @@ enum w_type
>         SW_FENCE,
>         SW_FENCE_SIGNAL,
>         CTX_PRIORITY,
> -       PREEMPTION
> +       PREEMPTION,
> +       ENGINE_MAP
>  };
>  
>  struct deps
> @@ -115,6 +117,10 @@ struct w_step
>                 int throttle;
>                 int fence_signal;
>                 int priority;
> +               struct {
> +                       unsigned int engine_map_count;
> +                       enum intel_engine_id *engine_map;
> +               };
>         };
>  
>         /* Implementation details */
> @@ -142,6 +148,8 @@ DECLARE_EWMA(uint64_t, rt, 4, 2)
>  struct ctx {
>         uint32_t id;
>         int priority;
> +       unsigned int engine_map_count;
> +       enum intel_engine_id *engine_map;
>         bool targets_instance;
>         bool wants_balance;
>         unsigned int static_vcs;
> @@ -200,10 +208,10 @@ struct workload
>                 int fd;
>                 bool first;
>                 unsigned int num_engines;
> -               unsigned int engine_map[5];
> +               unsigned int engine_map[NUM_ENGINES];
>                 uint64_t t_prev;
> -               uint64_t prev[5];
> -               double busy[5];
> +               uint64_t prev[NUM_ENGINES];
> +               double busy[NUM_ENGINES];
>         } busy_balancer;
>  };
>  
> @@ -234,6 +242,7 @@ static int fd;
>  #define REG(x) (volatile uint32_t *)((volatile char *)igt_global_mmio + x)
>  
>  static const char *ring_str_map[NUM_ENGINES] = {
> +       [DEFAULT] = "DEFAULT",
>         [RCS] = "RCS",
>         [BCS] = "BCS",
>         [VCS] = "VCS",
> @@ -330,6 +339,43 @@ static int str_to_engine(const char *str)
>         return -1;
>  }
>  
> +static int parse_engine_map(struct w_step *step, const char *_str)
> +{
> +       char *token, *tctx = NULL, *tstart = (char *)_str;
> +
> +       while ((token = strtok_r(tstart, "|", &tctx))) {
> +               enum intel_engine_id engine;
> +               unsigned int add;
> +
> +               tstart = NULL;
> +
> +               if (!strcmp(token, "DEFAULT"))
> +                       return -1;
> +
> +               engine = str_to_engine(token);
> +               if ((int)engine < 0)
> +                       return -1;
> +
> +               if (engine != VCS && engine != VCS1 && engine != VCS2)
> +                       return -1; /* TODO */

Still a little concerned that the map is VCS only. It just doesn't fit
my expectations of what the map will be.

> +
> +               add = engine == VCS ? 2 : 1;

Will we not every ask what happens if we had millions of engines at our
disposal. But that's a tommorrow problem, ok.

> +               step->engine_map_count += add;
> +               step->engine_map = realloc(step->engine_map,
> +                                          step->engine_map_count *
> +                                          sizeof(step->engine_map[0]));
> +
> +               if (engine != VCS) {
> +                       step->engine_map[step->engine_map_count - 1] = engine;
> +               } else {
> +                       step->engine_map[step->engine_map_count - 2] = VCS1;
> +                       step->engine_map[step->engine_map_count - 1] = VCS2;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
>  static struct workload *
>  parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w)
>  {
> @@ -448,6 +494,33 @@ parse_workload(struct w_arg *arg, unsigned int flags, struct workload *app_w)
>                         } else if (!strcmp(field, "f")) {
>                                 step.type = SW_FENCE;
>                                 goto add_step;
> +                       } else if (!strcmp(field, "M")) {
> +                               unsigned int nr = 0;
> +                               while ((field = strtok_r(fstart, ".", &fctx)) !=
> +                                   NULL) {
> +                                       tmp = atoi(field);
> +                                       check_arg(nr == 0 && tmp <= 0,
> +                                                 "Invalid context at step %u!\n",
> +                                                 nr_steps);
> +                                       check_arg(nr > 1,
> +                                                 "Invalid engine map format at step %u!\n",
> +                                                 nr_steps);
> +
> +                                       if (nr == 0) {
> +                                               step.context = tmp;
> +                                       } else {
> +                                               tmp = parse_engine_map(&step,
> +                                                                      field);
> +                                               check_arg(tmp < 0,
> +                                                         "Invalid engine map list at step %u!\n",
> +                                                         nr_steps);
> +                                       }
> +
> +                                       nr++;
> +                               }
> +
> +                               step.type = ENGINE_MAP;
> +                               goto add_step;
>                         } else if (!strcmp(field, "X")) {
>                                 unsigned int nr = 0;
>                                 while ((field = strtok_r(fstart, ".", &fctx)) !=
> @@ -774,6 +847,7 @@ terminate_bb(struct w_step *w, unsigned int flags)
>  }
>  
>  static const unsigned int eb_engine_map[NUM_ENGINES] = {
> +       [DEFAULT] = I915_EXEC_DEFAULT,
>         [RCS] = I915_EXEC_RENDER,
>         [BCS] = I915_EXEC_BLT,
>         [VCS] = I915_EXEC_BSD,
> @@ -796,11 +870,36 @@ eb_set_engine(struct drm_i915_gem_execbuffer2 *eb,
>                 eb->flags = eb_engine_map[engine];
>  }
>  
> +static unsigned int
> +find_engine_in_map(struct ctx *ctx, enum intel_engine_id engine)
> +{
> +       unsigned int i;
> +
> +       for (i = 0; i < ctx->engine_map_count; i++) {
> +               if (ctx->engine_map[i] == engine)
> +                       return i + 1;
> +       }
> +
> +       igt_assert(0);
> +       return 0;

No balancer in the map at this point?

> +}
> +
> +static struct ctx *
> +__get_ctx(struct workload *wrk, struct w_step *w)
> +{
> +       return &wrk->ctx_list[w->context * 2];
> +}
> +
>  static void
> -eb_update_flags(struct w_step *w, enum intel_engine_id engine,
> -               unsigned int flags)
> +eb_update_flags(struct workload *wrk, struct w_step *w,
> +               enum intel_engine_id engine, unsigned int flags)
>  {
> -       eb_set_engine(&w->eb, engine, flags);
> +       struct ctx *ctx = __get_ctx(wrk, w);
> +
> +       if (ctx->engine_map)
> +               w->eb.flags = find_engine_in_map(ctx, engine);
> +       else
> +               eb_set_engine(&w->eb, engine, flags);
>  
>         w->eb.flags |= I915_EXEC_HANDLE_LUT;
>         w->eb.flags |= I915_EXEC_NO_RELOC;
> @@ -819,12 +918,6 @@ get_status_objects(struct workload *wrk)
>                 return wrk->status_object;
>  }
>  
> -static struct ctx *
> -__get_ctx(struct workload *wrk, struct w_step *w)
> -{
> -       return &wrk->ctx_list[w->context * 2];
> -}
> -
>  static uint32_t
>  get_ctxid(struct workload *wrk, struct w_step *w)
>  {
> @@ -894,7 +987,7 @@ alloc_step_batch(struct workload *wrk, struct w_step *w, unsigned int flags)
>                 engine = VCS2;
>         else if (flags & SWAPVCS && engine == VCS2)
>                 engine = VCS1;
> -       eb_update_flags(w, engine, flags);
> +       eb_update_flags(wrk, w, engine, flags);
>  #ifdef DEBUG
>         printf("%u: %u:|", w->idx, w->eb.buffer_count);
>         for (i = 0; i <= j; i++)
> @@ -936,7 +1029,7 @@ static void vm_destroy(int i915, uint32_t vm_id)
>         igt_assert_eq(__vm_destroy(i915, vm_id), 0);
>  }
>  
> -static void
> +static int
>  prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
>  {
>         unsigned int ctx_vcs;
> @@ -999,30 +1092,53 @@ prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
>         /*
>          * Identify if contexts target specific engine instances and if they
>          * want to be balanced.
> +        *
> +        * Transfer over engine map configuration from the workload step.
>          */
>         for (j = 0; j < wrk->nr_ctxs; j += 2) {
>                 bool targets = false;
>                 bool balance = false;
>  
>                 for (i = 0, w = wrk->steps; i < wrk->nr_steps; i++, w++) {
> -                       if (w->type != BATCH)
> -                               continue;
> -
>                         if (w->context != (j / 2))
>                                 continue;
>  
> -                       if (w->engine == VCS)
> -                               balance = true;
> -                       else
> -                               targets = true;
> +                       if (w->type == BATCH) {
> +                               if (w->engine == VCS)
> +                                       balance = true;
> +                               else
> +                                       targets = true;
> +                       } else if (w->type == ENGINE_MAP) {
> +                               wrk->ctx_list[j].engine_map = w->engine_map;
> +                               wrk->ctx_list[j].engine_map_count =
> +                                       w->engine_map_count;
> +                       }
>                 }
>  
> -               if (flags & I915) {
> -                       wrk->ctx_list[j].targets_instance = targets;
> +               wrk->ctx_list[j].targets_instance = targets;
> +               if (flags & I915)
>                         wrk->ctx_list[j].wants_balance = balance;
> +       }
> +
> +       /*
> +        * Ensure VCS is not allowed with engine map contexts.
> +        */
> +       for (j = 0; j < wrk->nr_ctxs; j += 2) {
> +               for (i = 0, w = wrk->steps; i < wrk->nr_steps; i++, w++) {
> +                       if (w->context != (j / 2))
> +                               continue;
> +
> +                       if (w->type != BATCH)
> +                               continue;
> +
> +                       if (wrk->ctx_list[j].engine_map && w->engine == VCS) {

But wouldn't VCS still be meaning use the balancer and not a specific
engine???

I'm not understanding how you are using maps in the .wsim :(
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux