[Intel-gfx] [PATCH 2/6] drm/i915: Allow clients to query own per-engine busyness

[Tvrtko Ursulin]

On 19/01/2018 21:08, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-01-19 13:45:24)
>> +       case I915_CONTEXT_GET_ENGINE_BUSY:
>> +               engine = intel_engine_lookup_user(i915, args->class,
>> +                                                 args->instance);
>> +               if (!engine) {
>> +                       ret = -EINVAL;
>> +                       break;
>> +               }
>> +
>> +               ce = &ctx->engine[engine->id];
>> +               if (!READ_ONCE(ce->stats.enabled)) {
>> +                       ret = i915_mutex_lock_interruptible(dev);
>> +                       if (!ret)
>> +                               break;
>> +
>> +                       if (!ce->stats.enabled) {
>> +                               ret = intel_enable_engine_stats(engine);
> 
> * Blink.
> 
> This caught me by surprise. (Other than struct_mutex) Not too offensive,
> but surprising. At the very least call out to a function to handle the
> request. Where did args->class, args->instance come from? You surely
> didn't extend the ioctl struct just for that?

Haven't extended it no, just did this:

--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1468,7 +1468,16 @@ struct drm_i915_gem_context_param {
  #define   I915_CONTEXT_MAX_USER_PRIORITY	1023 /* inclusive */
  #define   I915_CONTEXT_DEFAULT_PRIORITY		0
  #define   I915_CONTEXT_MIN_USER_PRIORITY	-1023 /* inclusive */
-	__u64 value;
+#define I915_CONTEXT_GET_ENGINE_BUSY	0x7
+	union {
+		__u64 value;
+		struct {
+			__u8 pad[6]; /* unused */
+
+			__u8 class;
+			__u8 instance;
+		};
+	};
  };

Regards,

Tvrtko