On 18/10/2018 22:48, Chris Wilson wrote:
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, ¶m);
+
+ if (bonds)
+ free(bonds);
free(NULL) is legal, so just free(bonds) here.
The only one i am usually sure of is kfree. :)
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?
Shrug. My only gripe is that it is a nice generic mechanism used only in
ctx set param, which feels a bit on it's head (*), but in principle I
think it is fine.
Regards,
Tvrtko
(*) Sounds like it would be nicer if it was a core concept in more of
our uapi's. Going from the bottom up like create_context + extension
this + extension that. And the same for other our ioctls, where
applicable. But perhaps it is a stretch it would fit to many more places.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
