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

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

 




On 13/05/2019 14:29, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2019-05-13 14:18:59)

On 10/05/2019 14:26, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2019-05-08 13:10:48)
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.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
+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;
+
+               tstart = NULL;
+
+               if (!strcmp(token, "DEFAULT"))
+                       return -1;
+               else if (!strcmp(token, "VCS"))
+                       return -1;
+
+               engine = str_to_engine(token);
+               if ((int)engine < 0)
+                       return -1;
+
+               if (engine != VCS1 && engine != VCS2)
+                       return -1; /* TODO */
+
+               step->engine_map_count++;
+               step->engine_map = realloc(step->engine_map,
+                                          step->engine_map_count *
+                                          sizeof(step->engine_map[0]));
+               step->engine_map[step->engine_map_count - 1] = engine;


+               if (ctx->engine_map) {
+                       I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
+                                                         ctx->engine_map_count + 1);
+                       struct drm_i915_gem_context_param param = {
+                               .ctx_id = ctx_id,
+                               .param = I915_CONTEXT_PARAM_ENGINES,
+                               .size = sizeof(set_engines),
+                               .value = to_user_pointer(&set_engines),
+                       };
+
+                       set_engines.extensions = 0;
+
+                       /* Reserve slot for virtual engine. */
+                       set_engines.engines[0].engine_class =
+                               I915_ENGINE_CLASS_INVALID;
+                       set_engines.engines[0].engine_instance =
+                               I915_ENGINE_CLASS_INVALID_NONE;
+
+                       for (j = 1; j <= ctx->engine_map_count; j++) {
+                               set_engines.engines[j].engine_class =
+                                       I915_ENGINE_CLASS_VIDEO; /* FIXME */
+                               set_engines.engines[j].engine_instance =
+                                       ctx->engine_map[j - 1] - VCS1; /* FIXME */
+                       }

I would suggest the file format starts with class:instance specifiers.
Too much FIXME that I think will need a file format change.

Where do you see the need for a file format change?

Nah, I made the assumption the FIXMEs were because the implementation
was dictated by the file format.
These FIXMEs can be addressed by either adding engine discovery or
fixing the code to not assume class and engines to be balanced.

The code is just obeying the .wsim; the question is how to handle a
mismatch between the file and hw -- whether to do a transparent fixup to
use bcs instead of a secondary vcs?

It would be of little use since we cannot load balance the two...

Larger rework might be needed to deal with the internal engine
representation after adding engine discovery. Or at least an audit and
checking legacy paths. Might be that refactor would be limited to engine
string to internal engine id lookup.

But to change file format I don't see an immediate need. VCS is already
defined as any VCS and there are explicit VCS1 and VCS2.

I was more concerned in case vcs was implicit since it was heavily
assumed by the code.

...and the thing I just realised I am not actually happy with is the lack of ability to write portable .wsim's when using engine maps.

Legacy files can configure implicit engine maps based on class (VCS), so I think the engine map command needs the same capability. Otherwise the .wsim's won't be portable. I want to be able to do:

M.1.VCS
B.1

And that to mean create engine map with all VCS class engines and enable load balancing. It can be achieved with legacy (implicit) load balancing but that cannot be tied with frame split.

Regards,

Tvrtko
_______________________________________________
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