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?
> 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.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
