Re: [Intel-gfx] [PATCH 27/27] drm/i915/scheduler: Support user-defined priorities

[Tvrtko Ursulin]

On 19/04/2017 11:09, Chris Wilson wrote:

On Wed, Apr 19, 2017 at 10:41:43AM +0100, Chris Wilson wrote:

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index f43a22ae955b..200f2cf393b2 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -395,6 +395,8 @@ typedef struct drm_i915_irq_wait {

 /* Query whether DRM_I915_GEM_EXECBUFFER2 supports user defined execution
  * priorities and the driver will attempt to execute batches in priority order.
+ * The initial priority for each batch is supplied by the context and is
+ * controlled via I915_CONTEXT_PARAM_PRIORITY.
  */
 #define I915_PARAM_HAS_SCHEDULER41
 #define I915_PARAM_HUC_STATUS   42
@@ -1318,6 +1320,7 @@ struct drm_i915_gem_context_param {
 #define I915_CONTEXT_PARAM_GTT_SIZE0x3
 #define I915_CONTEXT_PARAM_NO_ERROR_CAPTURE0x4
 #define I915_CONTEXT_PARAM_BANNABLE0x5
+#define I915_CONTEXT_PARAM_PRIORITY0x6


Grr. Forgot to add min/max defines.

#define I915_CONTEXT_MAX_USER_PRIORITY  1023 /* inclusive */
#define I915_CONTEXT_DEFAULT_PRIORITY   0
#define I915_CONTEXT_MIN_USER_PRIORITY  -1023 /* inclusive */


Yes, and use these in context get param, including the default instead 
of the zero I think.



Or should it be I915_CONTEXT_PRIORITY_MAX_USER etc?


Priority last somehow looks better to me since like that it is clearly a 
separate category from param names. But I don't mind either way.


Regards,

Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 27/27] drm/i915/scheduler: Support user-defined priorities

[Chris Wilson]

On Wed, Apr 19, 2017 at 10:41:43AM +0100, Chris Wilson wrote:
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index f43a22ae955b..200f2cf393b2 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -395,6 +395,8 @@ typedef struct drm_i915_irq_wait {
>  
>  /* Query whether DRM_I915_GEM_EXECBUFFER2 supports user defined execution
>   * priorities and the driver will attempt to execute batches in priority 
> order.
> + * The initial priority for each batch is supplied by the context and is
> + * controlled via I915_CONTEXT_PARAM_PRIORITY.
>   */
>  #define I915_PARAM_HAS_SCHEDULER  41
>  #define I915_PARAM_HUC_STATUS 42
> @@ -1318,6 +1320,7 @@ struct drm_i915_gem_context_param {
>  #define I915_CONTEXT_PARAM_GTT_SIZE  0x3
>  #define I915_CONTEXT_PARAM_NO_ERROR_CAPTURE  0x4
>  #define I915_CONTEXT_PARAM_BANNABLE  0x5
> +#define I915_CONTEXT_PARAM_PRIORITY  0x6

Grr. Forgot to add min/max defines.

#define I915_CONTEXT_MAX_USER_PRIORITY  1023 /* inclusive */
#define I915_CONTEXT_DEFAULT_PRIORITY   0
#define I915_CONTEXT_MIN_USER_PRIORITY  -1023 /* inclusive */

Or should it be I915_CONTEXT_PRIORITY_MAX_USER etc?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 27/27] drm/i915/scheduler: Support user-defined priorities

[Chris Wilson]

Use a priority stored in the context as the initial value when
submitting a request. This allows us to change the default priority on a
per-context basis, allowing different contexts to be favoured with GPU
time at the expense of lower importance work. The user can adjust the
context's priority via I915_CONTEXT_PARAM_PRIORITY, with more positive
values being higher priority (they will be serviced earlier, after their
dependencies have been resolved). Any prerequisite work for an execbuf
will have its priority raised to match the new request as required.

Normal users can specify any value in the range of -1023 to 0 [default],
i.e. they can reduce the priority of their workloads (and temporarily
boost it back to normal if so desired).

Privileged users can specify any value in the range of -1023 to 1023,
[default is 0], i.e. they can raise their priority above all overs and
so potentially starve the system.

Note that the existing schedulers are not fair, nor load balancing, the
execution is strictly by priority on a first-come, first-served basis,
and the driver may choose to boost some requests above the range
available to users.

This priority was originally based around nice(2), but evolved to allow
clients to adjust their priority within a small range, and allow for a
privileged high priority range.

For example, this can be used to implement EGL_IMG_context_priority
https://www.khronos.org/registry/egl/extensions/IMG/EGL_IMG_context_priority.txt

EGL_CONTEXT_PRIORITY_LEVEL_IMG determines the priority level of
the context to be created. This attribute is a hint, as an
implementation may not support multiple contexts at some
priority levels and system policy may limit access to high
priority contexts to appropriate system privilege level. The
default value for EGL_CONTEXT_PRIORITY_LEVEL_IMG is
EGL_CONTEXT_PRIORITY_MEDIUM_IMG."

so we can map

PRIORITY_HIGH -> 1023 [privileged, will failback to 0]
PRIORITY_MED -> 0 [default]
PRIORITY_LOW -> -1023

They also map onto the priorities used by VkQueue (and a VkQueue is
essentially a timeline, our i915_gem_context under full-ppgtt).

v2: s/CAP_SYS_ADMIN/CAP_SYS_NICE/

Testcase: igt/gem_exec_schedule
Signed-off-by: Chris Wilson 
Reviewed-by: Tvrtko Ursulin 
---
 drivers/gpu/drm/i915/i915_gem_context.c | 22 ++
 include/uapi/drm/i915_drm.h |  3 +++
 2 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 23fd1470a7f4..694eddba51a6 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -1141,6 +1141,9 @@ int i915_gem_context_getparam_ioctl(struct drm_device 
*dev, void *data,
case I915_CONTEXT_PARAM_BANNABLE:
args->value = i915_gem_context_is_bannable(ctx);
break;
+   case I915_CONTEXT_PARAM_PRIORITY:
+   args->value = ctx->priority;
+   break;
default:
ret = -EINVAL;
break;
@@ -1198,6 +1201,25 @@ int i915_gem_context_setparam_ioctl(struct drm_device 
*dev, void *data,
else
i915_gem_context_clear_bannable(ctx);
break;
+
+   case I915_CONTEXT_PARAM_PRIORITY:
+   {
+   int priority = args->value;
+
+   if (args->size)
+   ret = -EINVAL;
+   else if (!to_i915(dev)->engine[RCS]->schedule)
+   ret = -ENODEV;
+   else if (priority >= I915_PRIORITY_MAX ||
+priority <= I915_PRIORITY_MIN)
+   ret = -EINVAL;
+   else if (priority > 0 && !capable(CAP_SYS_NICE))
+   ret = -EPERM;
+   else
+   ctx->priority = priority;
+   }
+   break;
+
default:
ret = -EINVAL;
break;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index f43a22ae955b..200f2cf393b2 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -395,6 +395,8 @@ typedef struct drm_i915_irq_wait {
 
 /* Query whether DRM_I915_GEM_EXECBUFFER2 supports user defined execution
  * priorities and the driver will attempt to execute batches in priority order.
+ * The initial priority for each batch is supplied by the context and is
+ * controlled via I915_CONTEXT_PARAM_PRIORITY.
  */
 #define I915_PARAM_HAS_SCHEDULER41
 #define I915_PARAM_HUC_STATUS   42
@@ -1318,6 +1320,7 @@ struct drm_i915_gem_context_param {
 #define I915_CONTEXT_PARAM_GTT_SIZE0x3
 #define I915_CONTEXT_PARAM_NO_ERROR_CAPTURE0x4
 #define I915_CONTEXT_PARAM_BANNABLE0x5