[igt-dev] [PATCH i-g-t 15/17] gem_wsim: Engine bond command

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-10-18 16:28:13)
> @@ -1196,6 +1256,7 @@ prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
>                                 { .base.name = I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE,
>                                   .engines_mask = -1,
>                                 };
> +                       struct i915_context_engines_bond *bonds = NULL;
>  
>                         if (ctx->wants_balance) {
>                                 set_engines.extensions =
> @@ -1211,7 +1272,31 @@ prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
>                                         ctx->engine_map[j] - VCS1; /* FIXME */
>                         }
>  
> +                       if (ctx->bond_count) {
> +                               bonds = calloc(ctx->bond_count, sizeof(*bonds));
> +                               load_balance.base.next_extension =
> +                                       to_user_pointer(&bonds[0]);
> +                       }
> +
> +                       for (j = 0; j < ctx->bond_count; j++) {
> +                               struct i915_context_engines_bond *bond =
> +                                       &bonds[j];
> +
> +                               if (j < (ctx->bond_count - 1))
> +                                       bond->base.next_extension =
> +                                               to_user_pointer(bond + 1);
> +
> +                               bond->base.name = I915_CONTEXT_ENGINES_EXT_BOND;
> +                               bond->master_class = I915_ENGINE_CLASS_VIDEO;
> +                               bond->master_instance =
> +                                       ctx->bonds[j].master - VCS1;
> +                               bond->sibling_mask = ctx->bonds[j].mask;
> +                       }
> +
>                         gem_context_set_param(fd, &param);
> +
> +                       if (bonds)
> +                               free(bonds);

free(NULL) is legal, so just free(bonds) here.

Looking at how you have constructed the map for the extension, I'm
reasonably happy with how this works in practice (outside of the igt
tests). Is the flexibility (of next_extension and separate bond structs)
too much?
-Chris