diff mbox series

[v2] drm/i915: Introduce struct class_instance for engines across the uAPI

Message ID 20190412071416.30097-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series [v2] drm/i915: Introduce struct class_instance for engines across the uAPI | expand

Commit Message

Chris Wilson April 12, 2019, 7:14 a.m. UTC
SSEU reprogramming of the context introduced the notion of engine class
and instance for a forwards compatible method of describing any engine
beyond the old execbuf interface. We wish to adopt this class:instance
description for more interfaces, so pull it out into a separate type for
userspace convenience.

References: e46c2e99f600 ("drm/i915: Expose RPCS (SSEU) configuration to userspace (Gen11 only)")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
Cc: Tony Ye <tony.ye@intel.com>
Cc: Andi Shyti <andi@etezian.org>
---
Drop the union foible. It's only going to hinder a decision ;)
-Chris
---
 drivers/gpu/drm/i915/i915_gem_context.c |  8 ++++----
 include/uapi/drm/i915_drm.h             | 16 ++++++++++++++--
 2 files changed, 18 insertions(+), 6 deletions(-)

Comments

Tvrtko Ursulin April 12, 2019, 12:10 p.m. UTC | #1
On 12/04/2019 08:14, Chris Wilson wrote:
> SSEU reprogramming of the context introduced the notion of engine class
> and instance for a forwards compatible method of describing any engine
> beyond the old execbuf interface. We wish to adopt this class:instance
> description for more interfaces, so pull it out into a separate type for
> userspace convenience.
> 
> References: e46c2e99f600 ("drm/i915: Expose RPCS (SSEU) configuration to userspace (Gen11 only)")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
> Cc: Tony Ye <tony.ye@intel.com>
> Cc: Andi Shyti <andi@etezian.org>
> ---
> Drop the union foible. It's only going to hinder a decision ;)
> -Chris
> ---
>   drivers/gpu/drm/i915/i915_gem_context.c |  8 ++++----
>   include/uapi/drm/i915_drm.h             | 16 ++++++++++++++--
>   2 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 7fc34ab6df87..dd728b26b5aa 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -1386,8 +1386,8 @@ static int set_sseu(struct i915_gem_context *ctx,
>   		return -EINVAL;
>   
>   	engine = intel_engine_lookup_user(i915,
> -					  user_sseu.engine_class,
> -					  user_sseu.engine_instance);
> +					  user_sseu.engine.engine_class,
> +					  user_sseu.engine.engine_instance);
>   	if (!engine)
>   		return -EINVAL;
>   
> @@ -1626,8 +1626,8 @@ static int get_sseu(struct i915_gem_context *ctx,
>   		return -EINVAL;
>   
>   	engine = intel_engine_lookup_user(ctx->i915,
> -					  user_sseu.engine_class,
> -					  user_sseu.engine_instance);
> +					  user_sseu.engine.engine_class,
> +					  user_sseu.engine.engine_instance);
>   	if (!engine)
>   		return -EINVAL;
>   
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 52051d24d89d..bbc453b1e50f 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -126,6 +126,19 @@ enum drm_i915_gem_engine_class {
>   	I915_ENGINE_CLASS_INVALID	= -1
>   };
>   
> +
> +/*
> + * There may be more than one engine fulfilling any role within the system.
> + * Each engine of a class is given a unique instance number and therefore
> + * any engine can be specified by its class:instance tuplet. APIs that allow
> + * access to any engine in the system will use struct i915_engine_class_instance
> + * for this identification.
> + */
> +struct i915_engine_class_instance {
> +	__u16 engine_class; /* see enum drm_i915_gem_engine_class */
> +	__u16 engine_instance;
> +};
> +
>   /**
>    * DOC: perf_events exposed by i915 through /sys/bus/event_sources/drivers/i915
>    *
> @@ -1525,8 +1538,7 @@ struct drm_i915_gem_context_param_sseu {
>   	/*
>   	 * Engine class & instance to be configured or queried.
>   	 */
> -	__u16 engine_class;
> -	__u16 engine_instance;
> +	struct i915_engine_class_instance engine;
>   
>   	/*
>   	 * Unused for now. Must be cleared to zero.
> 

This will definitely be tidier in the near future (despite a bit of 
ugliness in engine.engine_*). I have also checked that this indeed is 
part of 5.1 which hasn't been shipped yet.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

If we can get an ack from Joonas and Tony, to whom it probably falls to 
do the media-driver change I think we are good to do.

Regards,

Tvrtko
Ye, Tony April 12, 2019, 2:17 p.m. UTC | #2
On 4/12/2019 8:10 PM, Tvrtko Ursulin wrote:
>
> On 12/04/2019 08:14, Chris Wilson wrote:
>> SSEU reprogramming of the context introduced the notion of engine class
>> and instance for a forwards compatible method of describing any engine
>> beyond the old execbuf interface. We wish to adopt this class:instance
>> description for more interfaces, so pull it out into a separate type for
>> userspace convenience.
>>
>> References: e46c2e99f600 ("drm/i915: Expose RPCS (SSEU) configuration 
>> to userspace (Gen11 only)")
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
>> Cc: Tony Ye <tony.ye@intel.com>
>> Cc: Andi Shyti <andi@etezian.org>
>> ---
>> Drop the union foible. It's only going to hinder a decision ;)
>> -Chris
>> ---
>>   drivers/gpu/drm/i915/i915_gem_context.c |  8 ++++----
>>   include/uapi/drm/i915_drm.h             | 16 ++++++++++++++--
>>   2 files changed, 18 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
>> b/drivers/gpu/drm/i915/i915_gem_context.c
>> index 7fc34ab6df87..dd728b26b5aa 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_context.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
>> @@ -1386,8 +1386,8 @@ static int set_sseu(struct i915_gem_context *ctx,
>>           return -EINVAL;
>>         engine = intel_engine_lookup_user(i915,
>> -                      user_sseu.engine_class,
>> -                      user_sseu.engine_instance);
>> +                      user_sseu.engine.engine_class,
>> +                      user_sseu.engine.engine_instance);
>>       if (!engine)
>>           return -EINVAL;
>>   @@ -1626,8 +1626,8 @@ static int get_sseu(struct i915_gem_context 
>> *ctx,
>>           return -EINVAL;
>>         engine = intel_engine_lookup_user(ctx->i915,
>> -                      user_sseu.engine_class,
>> -                      user_sseu.engine_instance);
>> +                      user_sseu.engine.engine_class,
>> +                      user_sseu.engine.engine_instance);
>>       if (!engine)
>>           return -EINVAL;
>>   diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>> index 52051d24d89d..bbc453b1e50f 100644
>> --- a/include/uapi/drm/i915_drm.h
>> +++ b/include/uapi/drm/i915_drm.h
>> @@ -126,6 +126,19 @@ enum drm_i915_gem_engine_class {
>>       I915_ENGINE_CLASS_INVALID    = -1
>>   };
>>   +
>> +/*
>> + * There may be more than one engine fulfilling any role within the 
>> system.
>> + * Each engine of a class is given a unique instance number and 
>> therefore
>> + * any engine can be specified by its class:instance tuplet. APIs 
>> that allow
>> + * access to any engine in the system will use struct 
>> i915_engine_class_instance
>> + * for this identification.
>> + */
>> +struct i915_engine_class_instance {
>> +    __u16 engine_class; /* see enum drm_i915_gem_engine_class */
>> +    __u16 engine_instance;
>> +};
>> +
>>   /**
>>    * DOC: perf_events exposed by i915 through 
>> /sys/bus/event_sources/drivers/i915
>>    *
>> @@ -1525,8 +1538,7 @@ struct drm_i915_gem_context_param_sseu {
>>       /*
>>        * Engine class & instance to be configured or queried.
>>        */
>> -    __u16 engine_class;
>> -    __u16 engine_instance;
>> +    struct i915_engine_class_instance engine;
>>         /*
>>        * Unused for now. Must be cleared to zero.
>>
>
> This will definitely be tidier in the near future (despite a bit of 
> ugliness in engine.engine_*). I have also checked that this indeed is 
> part of 5.1 which hasn't been shipped yet.
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> If we can get an ack from Joonas and Tony, to whom it probably falls 
> to do the media-driver change I think we are good to do.
>
> Regards,
>
> Tvrtko

Acked-by: Tony Ye <tony.ye@intel.com>

Regards, --Tony
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 7fc34ab6df87..dd728b26b5aa 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -1386,8 +1386,8 @@  static int set_sseu(struct i915_gem_context *ctx,
 		return -EINVAL;
 
 	engine = intel_engine_lookup_user(i915,
-					  user_sseu.engine_class,
-					  user_sseu.engine_instance);
+					  user_sseu.engine.engine_class,
+					  user_sseu.engine.engine_instance);
 	if (!engine)
 		return -EINVAL;
 
@@ -1626,8 +1626,8 @@  static int get_sseu(struct i915_gem_context *ctx,
 		return -EINVAL;
 
 	engine = intel_engine_lookup_user(ctx->i915,
-					  user_sseu.engine_class,
-					  user_sseu.engine_instance);
+					  user_sseu.engine.engine_class,
+					  user_sseu.engine.engine_instance);
 	if (!engine)
 		return -EINVAL;
 
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 52051d24d89d..bbc453b1e50f 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -126,6 +126,19 @@  enum drm_i915_gem_engine_class {
 	I915_ENGINE_CLASS_INVALID	= -1
 };
 
+
+/*
+ * There may be more than one engine fulfilling any role within the system.
+ * Each engine of a class is given a unique instance number and therefore
+ * any engine can be specified by its class:instance tuplet. APIs that allow
+ * access to any engine in the system will use struct i915_engine_class_instance
+ * for this identification.
+ */
+struct i915_engine_class_instance {
+	__u16 engine_class; /* see enum drm_i915_gem_engine_class */
+	__u16 engine_instance;
+};
+
 /**
  * DOC: perf_events exposed by i915 through /sys/bus/event_sources/drivers/i915
  *
@@ -1525,8 +1538,7 @@  struct drm_i915_gem_context_param_sseu {
 	/*
 	 * Engine class & instance to be configured or queried.
 	 */
-	__u16 engine_class;
-	__u16 engine_instance;
+	struct i915_engine_class_instance engine;
 
 	/*
 	 * Unused for now. Must be cleared to zero.