[Intel-gfx] [PATCH 13/39] drm/i915: Extend I915_CONTEXT_PARAM_SSEU to support local ctx->engine[]

[Tvrtko Ursulin]

On 14/03/2019 17:04, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-03-14 16:49:54)
>>
>> On 13/03/2019 14:43, Chris Wilson wrote:
>>> Allow the user to specify a local engine index (as opposed to
>>> class:index) that they can use to refer to a preset engine inside the
>>> ctx->engine[] array defined by an earlier I915_CONTEXT_PARAM_ENGINES.
>>> This will be useful for setting SSEU parameters on virtual engines that
>>> are local to the context and do not have a valid global class:instance
>>> lookup.
>>>
>>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/i915_gem_context.c | 24 ++++++++++++++++++++----
>>>    include/uapi/drm/i915_drm.h             |  3 ++-
>>>    2 files changed, 22 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
>>> index 07377b75b563..7ae28622b709 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem_context.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
>>> @@ -1375,6 +1375,7 @@ static int set_sseu(struct i915_gem_context *ctx,
>>>        struct drm_i915_gem_context_param_sseu user_sseu;
>>>        struct intel_engine_cs *engine;
>>>        struct intel_sseu sseu;
>>> +     unsigned long lookup;
>>
>> I'd prefer unsigned int.
> 
> Why not register size for what should only be in a register and matches
> usage with BIT()?

My bad!

>>> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>>> index 6dde864e14e7..e17c7375248c 100644
>>> --- a/include/uapi/drm/i915_drm.h
>>> +++ b/include/uapi/drm/i915_drm.h
>>> @@ -1567,9 +1567,10 @@ struct drm_i915_gem_context_param_sseu {
>>>        __u16 engine_instance;
>>>    
>>>        /*
>>> -      * Unused for now. Must be cleared to zero.
>>> +      * Unknown flags must be cleared to zero.
>>>         */
>>>        __u32 flags;
>>> +#define I915_CONTEXT_SSEU_FLAG_ENGINE_INDEX (1u << 0)
>>>    
>>>        /*
>>>         * Mask of slices to enable for the context. Valid values are a subset
>>>
>>
>> Which patch has this hunk:
>>
>> --- a/drivers/gpu/drm/i915/i915_gem_context.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
>> @@ -1185,7 +1185,6 @@ i915_gem_context_reconfigure_sseu(struct
>> i915_gem_context *ctx,
>>          int ret = 0;
>>
>>          GEM_BUG_ON(INTEL_GEN(ctx->i915) < 8);
>> -       GEM_BUG_ON(engine->id != RCS0);
>>
>> I think it should go into this one.
> 
> That's in load balancing (introduction of virtual engine) itself.
> Consider it done.

And again. You are right, it belongs to virtual engine.

Regards,

Tvrtko