[Intel-gfx] [RFC v3] drm/i915: Select engines via class and instance in execbuffer2

[Tvrtko Ursulin]

On 25/05/2017 15:14, Chris Wilson wrote:
> On Wed, May 24, 2017 at 12:28:07PM +0100, Tvrtko Ursulin wrote:
>>
>> On 18/05/2017 18:00, Chris Wilson wrote:
>>> On Thu, May 18, 2017 at 05:20:38PM +0100, Tvrtko Ursulin wrote:
>>>>
>>>> On 18/05/2017 14:37, Chris Wilson wrote:
>>>>> On Thu, May 18, 2017 at 02:06:35PM +0100, Tvrtko Ursulin wrote:
>>>>>>
>>>>>> But this problem in general can also be solved separately from
>>>>>> class-instance addressing via engine feature masking.
>>>>>
>>>>> But imo all members of a class should have the same features. That would
>>>>> be my definition of a class!
>>>>
>>>> That sounds very totalitarian! :)) To me a class is a group of some
>>>> entities which share some common characteristics - not necessarily
>>>> completely uniform.
>>>
>>> The problem otherwise is that we then have to define yet another
>>> interface based on features. To me that sounds like too much
>>> duplication, that we could avoid from the beginning. Curse the hw for
>>> being asymmetical!
>>
>> Hm I don't see a problem with the feature base engine selection on
>> top. You still do because of the desire classes were equal in
>> features?
> 
> Ok, having another think about it, I agree that the features define a
> subclass. Basically it comes down to VCS | HEVC being a subset of the
> VCS group and we need a way to express any of VCS | HEVC and any of VCS
> cleanly.

Cool, I'll respin with engine features on the top to see how that looks. 
And I am also thinking about dropping the GT discovery bits. More on 
that below.

>> To sum up what I (and we) talked about in various parts of the thread(s):
>>
>> Step 1a: New execbuf engine selection uAPI.
>>
>>   - execbuf class=VCS instance=1
>>
>> Step 1b: Engine discovery uAPI.
>>
>> Same as above but userpace can figure out how many VCS engines there
>> are without PCI probing.
>>
>> I didn't get much feedback on this one. :(
> 
> Mainly because we didn't emphasis that this would be the only way to
> discover the parameters for execbuf / svm and it lost in the idea that
> this would replace their device_info tables. We are suggesting that we
> can phase out those tables (but we should put some effort into making
> sure we discard unwanted ones after init) since we will have to provide
> the information anyway.

It has become more unlikely we would get libva (my hopeful first user) 
on board for this in the short/medium term. So as said above, I am 
thinking to drop this for now.

If at some point we go for "step 2" below that would also enable some 
sort of engine enumeration/probing via execbuf null batches.

>> Step 2: Feature masks for execbuf.
>>
>>   - execbuf class=VCS instance=0 features=HEVC = OK
>>   - execbuf class=VCS instance=1 features=HEVC = FAIL
>>
>> But userspace can use engine discovery to figure out which are the
>> valid combinations.
>>
>> This could be a simpler, but less featureful and not very elegant
>> alternative to step 2.
>>
>> Otherwise just a prep step for the subsequent steps below.
>>
>> Step 3a: (One day maybe) userspace selects a class, i915 picks the engine
>>
>>   - execbuf class=VCS instance=any
>>
>> Step 3b: userspace selected class and features
>>
>>   - execbuf class=VCS instance=any features=HEVC
>>
>> This RFC proposed steps 1a and 1b. The rest we leave for later.
>>
>> How does that sound? Acceptable?
> 
> I want "1c: extensible interface for per-engine settings elsewhere in the
> uAPI" as well.

What do you have in mind here, what other uAPI deals with engines?

Regards,

Tvrtko
> We also need an answer to the i915_gem_busy_ioctl conundrum - if/when we
> should deprecated the engine id being exposed. It's the same question
> more or less as for how long (or whether we should) continue to support
> I915_EXEC_RING. Plus how to expose these via tracepoints.
> 
> The quick answer is while we have less than 16 engines, we don't have to
> worry about the uABI breakage.
>   
>> In case of engine discovery useful enough or what other features
>> could we put it in to make it more useful for userspace? Potentially
>> enable dropping PCI id probing altogether and enable libva/mesa/???
>> to probe everything using i915 ioctls.
> 
> To be convenient we have to beat the ease of device_info[PCIID], and
> being futureproof. The resistance will be because then they are
> dependent on the kernel for features that are not dependent on that
> kernel for execution, i.e. being able to use new userspace on old
> kernels and remain feature complete. To counter that we need to
> have a complete description of a new device the moment we enable it.
> That makes it a hard sell. The benefit is basically only a reduction in
> RO library mmappings, a very small improvement of start up speed at
> best.
> -Chris
>