[Intel-gfx] [PATCH v5 2/9] drm/i915: Define an engine class enum for the uABI

[Lionel Landwerlin]

On 09/11/17 21:29, Chris Wilson wrote:
> Quoting Lionel Landwerlin (2017-11-09 09:13:03)
>> On 08/11/17 19:14, Chris Wilson wrote:
>>>    
>>> +/*
>>> + * Different engines serve different roles, and there may be more than one
>>> + * engine serving each role. enum drm_i915_gem_engine_class provides a
>>> + * classification of the role of the engine, which may be used when requesting
>>> + * operations to be performed on a certain subset of engines, or for providing
>>> + * information about that group.
>>> + */
>>> +enum drm_i915_gem_engine_class {
>>> +     I915_ENGINE_CLASS_OTHER = 0,
>>> +     I915_ENGINE_CLASS_RENDER = 1,
>>> +     I915_ENGINE_CLASS_COPY = 2,
>>> +     I915_ENGINE_CLASS_VIDEO = 3,
>>> +     I915_ENGINE_CLASS_VIDEO_ENHANCE = 4,
>>> +};
>>> +
>> I've tried to build a bit UI in GPUTop to show this.
>> I'm a bit skeptical about the OTHER type because if this enum is meant
>> to be extended, then why do we need an OTHER class? We should create new
>> classes instead.
> Whilst OTHER is certainly a question of "why not add a define for the
> new class", my 2c is that we do want an explicit INVALID enum. It
> doesn't have to 0, -1 will work just fine as well, but there will be a
> time either in userspace or in reporting from the kernel where we will
> need to store an invalid value that is guaranteed not to map onto a real
> class.
>
> So I915_ENGINE_CLASS_INVALID = -1 ?
> -Chris
>
Okay, so you want to use that value for userspace to say "execute this 
batch buffer on whatever engine".
And in that case that's fine with me, I didn't get that would be used in 
the userspace->kernel direction.

But I'm still struggling to understand why the kernel would report that 
there is an "invalid" class of engine on this device, rather than 
something more descriptive.

-
Lionel