[igt-dev] [PATCH i-g-t 14/21] gem_wsim: Engine map load balance command

[Tvrtko Ursulin]

On 15/05/2019 12:48, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-15 12:44:41)
>>
>> On 10/05/2019 14:31, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2019-05-08 13:10:51)
>>>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>>>
>>>> A new workload command for enabling a load balanced context map (aka
>>>> Virtual Engine). Example usage:
>>>>
>>>>     B.1
>>>>
>>>> This turns on load balancing for context one, assuming it has already been
>>>> configured with an engine map. Only DEFAULT engine specifier can be used
>>>> with load balanced engine maps.
>>>
>>> Restriction makes sense for keeping linenoise^W file format simple.
>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>>> ---
>>>> @@ -1172,6 +1210,8 @@ prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
>>>>                   if (ctx->engine_map) {
>>>>                           I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
>>>>                                                             ctx->engine_map_count + 1);
>>>> +                       I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(load_balance,
>>>> +                                                                ctx->engine_map_count);
>>>>                           struct drm_i915_gem_context_param param = {
>>>>                                   .ctx_id = ctx_id,
>>>>                                   .param = I915_CONTEXT_PARAM_ENGINES,
>>>> @@ -1179,7 +1219,25 @@ prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
>>>>                                   .value = to_user_pointer(&set_engines),
>>>>                           };
>>>>    
>>>> -                       set_engines.extensions = 0;
>>>> +                       if (ctx->wants_balance) {
>>>> +                               set_engines.extensions =
>>>> +                                       to_user_pointer(&load_balance);
>>>> +
>>>> +                               memset(&load_balance, 0, sizeof(load_balance));
>>>> +                               load_balance.base.name =
>>>> +                                       I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
>>>> +                               load_balance.num_siblings =
>>>> +                                       ctx->engine_map_count;
>>>> +
>>>> +                               for (j = 0; j < ctx->engine_map_count; j++) {
>>>> +                                       load_balance.engines[j].engine_class =
>>>> +                                               I915_ENGINE_CLASS_VIDEO; /* FIXME */
>>>> +                                       load_balance.engines[j].engine_instance =
>>>> +                                               ctx->engine_map[j] - VCS1; /* FIXME */
>>>
>>> Ok, more fallout from fixing ctx->engine_map[] first?
>>
>> Not sure I understand the question.
> 
> The proliferation of FIXME, the assumption of CLASS_VIDEO and an
> impedance mismatch between engine_map and class:instance. Basically
> those FIXME raise the question of what do you intend this to look like?

Intention that implicit and explicit engine maps get populated by 
available engines.

When "-b i915":

1.VCS.1000.0.0 -> implicit map of available vcs engines

M.1.VCS
B.1	\-> explicit map of available vcs engines

That would support Icelake vcs0+vcs2 SKUs. And explicit engine map wsims 
would be more portable, like the original ones were.

Also, I am contemplating using VCS2 in wsim as meaning the 2nd VCS 
engine, so logical instances. So:

M.1.VCS1|VCS2 -> also works on both SKL and ICL (two vcs SKUs)
B.1

Regards,

Tvrtko