From: Chris Wilson Date: Wed, 18 Apr 2018 19:40:52 +0100 Subject: drm/i915: Pack params to engine->schedule() into a struct Git-commit: b7268c5eed0ab4f052d614b4b0e3fe8a51c9d5a1 Patch-mainline: v4.18-rc1 References: FATE#326289 FATE#326079 FATE#326049 FATE#322398 FATE#326166 Today we only want to pass along the priority to engine->schedule(), but in the future we want to have much more control over the various aspects of the GPU during a context's execution, for example controlling the frequency allowed. As we need an ever growing number of parameters for scheduling, move those into a struct for convenience. v2: Move the anonymous struct into its own function for legibility and ye olde gcc. Signed-off-by: Chris Wilson Reviewed-by: Joonas Lahtinen Link: https://patchwork.freedesktop.org/patch/msgid/20180418184052.7129-3-chris@chris-wilson.co.uk Acked-by: Petr Tesarik --- drivers/gpu/drm/i915/i915_drv.h | 3 ++- drivers/gpu/drm/i915/i915_gem.c | 18 ++++++++++-------- drivers/gpu/drm/i915/i915_gem_context.c | 8 ++++---- drivers/gpu/drm/i915/i915_gem_context.h | 13 +------------ drivers/gpu/drm/i915/i915_gpu_error.c | 8 ++++---- drivers/gpu/drm/i915/i915_gpu_error.h | 5 +++-- drivers/gpu/drm/i915/i915_request.c | 4 ++-- drivers/gpu/drm/i915/i915_request.h | 1 + drivers/gpu/drm/i915/i915_scheduler.h | 17 ++++++++++++++++- drivers/gpu/drm/i915/intel_display.c | 11 ++++++++++- drivers/gpu/drm/i915/intel_engine_cs.c | 18 +++++++++++++++--- drivers/gpu/drm/i915/intel_guc_submission.c | 2 +- drivers/gpu/drm/i915/intel_lrc.c | 20 +++++++++++--------- drivers/gpu/drm/i915/intel_ringbuffer.h | 4 +++- drivers/gpu/drm/i915/selftests/intel_hangcheck.c | 4 ++-- drivers/gpu/drm/i915/selftests/intel_lrc.c | 8 +++++--- 16 files changed, 90 insertions(+), 54 deletions(-) --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -75,6 +75,7 @@ #include "i915_gem_timeline.h" #include "i915_gpu_error.h" #include "i915_request.h" +#include "i915_scheduler.h" #include "i915_vma.h" #include "intel_gvt.h" @@ -3158,7 +3159,7 @@ int i915_gem_object_wait(struct drm_i915 struct intel_rps_client *rps); int i915_gem_object_wait_priority(struct drm_i915_gem_object *obj, unsigned int flags, - int priority); + const struct i915_sched_attr *attr); #define I915_PRIORITY_DISPLAY I915_PRIORITY_MAX int __must_check --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -564,7 +564,8 @@ i915_gem_object_wait_reservation(struct return timeout; } -static void __fence_set_priority(struct dma_fence *fence, int prio) +static void __fence_set_priority(struct dma_fence *fence, + const struct i915_sched_attr *attr) { struct i915_request *rq; struct intel_engine_cs *engine; @@ -577,11 +578,12 @@ static void __fence_set_priority(struct rcu_read_lock(); if (engine->schedule) - engine->schedule(rq, prio); + engine->schedule(rq, attr); rcu_read_unlock(); } -static void fence_set_priority(struct dma_fence *fence, int prio) +static void fence_set_priority(struct dma_fence *fence, + const struct i915_sched_attr *attr) { /* Recurse once into a fence-array */ if (dma_fence_is_array(fence)) { @@ -589,16 +591,16 @@ static void fence_set_priority(struct dm int i; for (i = 0; i < array->num_fences; i++) - __fence_set_priority(array->fences[i], prio); + __fence_set_priority(array->fences[i], attr); } else { - __fence_set_priority(fence, prio); + __fence_set_priority(fence, attr); } } int i915_gem_object_wait_priority(struct drm_i915_gem_object *obj, unsigned int flags, - int prio) + const struct i915_sched_attr *attr) { struct dma_fence *excl; @@ -613,7 +615,7 @@ i915_gem_object_wait_priority(struct drm return ret; for (i = 0; i < count; i++) { - fence_set_priority(shared[i], prio); + fence_set_priority(shared[i], attr); dma_fence_put(shared[i]); } @@ -623,7 +625,7 @@ i915_gem_object_wait_priority(struct drm } if (excl) { - fence_set_priority(excl, prio); + fence_set_priority(excl, attr); dma_fence_put(excl); } return 0; --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -281,7 +281,7 @@ __create_hw_context(struct drm_i915_priv kref_init(&ctx->ref); list_add_tail(&ctx->link, &dev_priv->contexts.list); ctx->i915 = dev_priv; - ctx->priority = I915_PRIORITY_NORMAL; + ctx->sched.priority = I915_PRIORITY_NORMAL; INIT_RADIX_TREE(&ctx->handles_vma, GFP_KERNEL); INIT_LIST_HEAD(&ctx->handles_list); @@ -431,7 +431,7 @@ i915_gem_context_create_kernel(struct dr return ctx; i915_gem_context_clear_bannable(ctx); - ctx->priority = prio; + ctx->sched.priority = prio; ctx->ring_size = PAGE_SIZE; GEM_BUG_ON(!i915_gem_context_is_kernel(ctx)); @@ -753,7 +753,7 @@ int i915_gem_context_getparam_ioctl(stru args->value = i915_gem_context_is_bannable(ctx); break; case I915_CONTEXT_PARAM_PRIORITY: - args->value = ctx->priority; + args->value = ctx->sched.priority; break; default: ret = -EINVAL; @@ -826,7 +826,7 @@ int i915_gem_context_setparam_ioctl(stru !capable(CAP_SYS_NICE)) ret = -EPERM; else - ctx->priority = priority; + ctx->sched.priority = priority; } break; --- a/drivers/gpu/drm/i915/i915_gem_context.h +++ b/drivers/gpu/drm/i915/i915_gem_context.h @@ -137,18 +137,7 @@ struct i915_gem_context { */ u32 user_handle; - /** - * @priority: execution and service priority - * - * All clients are equal, but some are more equal than others! - * - * Requests from a context with a greater (more positive) value of - * @priority will be executed before those with a lower @priority - * value, forming a simple QoS. - * - * The &drm_i915_private.kernel_context is assigned the lowest priority. - */ - int priority; + struct i915_sched_attr sched; /** ggtt_offset_bias: placement restriction for context objects */ u32 ggtt_offset_bias; --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -411,7 +411,7 @@ static void error_print_request(struct d err_printf(m, "%s pid %d, ban score %d, seqno %8x:%08x, prio %d, emitted %dms ago, head %08x, tail %08x\n", prefix, erq->pid, erq->ban_score, - erq->context, erq->seqno, erq->priority, + erq->context, erq->seqno, erq->sched_attr.priority, jiffies_to_msecs(jiffies - erq->jiffies), erq->head, erq->tail); } @@ -422,7 +422,7 @@ static void error_print_context(struct d { err_printf(m, "%s%s[%d] user_handle %d hw_id %d, prio %d, ban score %d%s guilty %d active %d\n", header, ctx->comm, ctx->pid, ctx->handle, ctx->hw_id, - ctx->priority, ctx->ban_score, bannable(ctx), + ctx->sched_attr.priority, ctx->ban_score, bannable(ctx), ctx->guilty, ctx->active); } @@ -1278,7 +1278,7 @@ static void record_request(struct i915_r struct drm_i915_error_request *erq) { erq->context = request->ctx->hw_id; - erq->priority = request->sched.priority; + erq->sched_attr = request->sched.attr; erq->ban_score = atomic_read(&request->ctx->ban_score); erq->seqno = request->global_seqno; erq->jiffies = request->emitted_jiffies; @@ -1372,7 +1372,7 @@ static void record_context(struct drm_i9 e->handle = ctx->user_handle; e->hw_id = ctx->hw_id; - e->priority = ctx->priority; + e->sched_attr = ctx->sched; e->ban_score = atomic_read(&ctx->ban_score); e->bannable = i915_gem_context_is_bannable(ctx); e->guilty = atomic_read(&ctx->guilty_count); --- a/drivers/gpu/drm/i915/i915_gpu_error.h +++ b/drivers/gpu/drm/i915/i915_gpu_error.h @@ -20,6 +20,7 @@ #include "i915_gem.h" #include "i915_gem_gtt.h" #include "i915_params.h" +#include "i915_scheduler.h" struct drm_i915_private; struct intel_overlay_error_state; @@ -122,11 +123,11 @@ struct i915_gpu_state { pid_t pid; u32 handle; u32 hw_id; - int priority; int ban_score; int active; int guilty; bool bannable; + struct i915_sched_attr sched_attr; } context; struct drm_i915_error_object { @@ -147,11 +148,11 @@ struct i915_gpu_state { long jiffies; pid_t pid; u32 context; - int priority; int ban_score; u32 seqno; u32 head; u32 tail; + struct i915_sched_attr sched_attr; } *requests, execlist[EXECLIST_MAX_PORTS]; unsigned int num_ports; --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -193,7 +193,7 @@ i915_sched_node_init(struct i915_sched_n INIT_LIST_HEAD(&node->signalers_list); INIT_LIST_HEAD(&node->waiters_list); INIT_LIST_HEAD(&node->link); - node->priority = I915_PRIORITY_INVALID; + node->attr.priority = I915_PRIORITY_INVALID; } static int reset_all_global_seqno(struct drm_i915_private *i915, u32 seqno) @@ -1064,7 +1064,7 @@ void __i915_request_add(struct i915_requ */ rcu_read_lock(); if (engine->schedule) - engine->schedule(request, request->ctx->priority); + engine->schedule(request, &request->ctx->sched); rcu_read_unlock(); local_bh_disable(); --- a/drivers/gpu/drm/i915/i915_request.h +++ b/drivers/gpu/drm/i915/i915_request.h @@ -30,6 +30,7 @@ #include "i915_gem.h" #include "i915_scheduler.h" #include "i915_sw_fence.h" +#include "i915_scheduler.h" #include --- a/drivers/gpu/drm/i915/i915_scheduler.h +++ b/drivers/gpu/drm/i915/i915_scheduler.h @@ -19,6 +19,21 @@ enum { I915_PRIORITY_INVALID = INT_MIN }; +struct i915_sched_attr { + /** + * @priority: execution and service priority + * + * All clients are equal, but some are more equal than others! + * + * Requests from a context with a greater (more positive) value of + * @priority will be executed before those with a lower @priority + * value, forming a simple QoS. + * + * The &drm_i915_private.kernel_context is assigned the lowest priority. + */ + int priority; +}; + /* * "People assume that time is a strict progression of cause to effect, but * actually, from a nonlinear, non-subjective viewpoint, it's more like a big @@ -42,7 +57,7 @@ struct i915_sched_node { struct list_head signalers_list; /* those before us, we depend upon */ struct list_head waiters_list; /* those after us, they depend upon us */ struct list_head link; - int priority; + struct i915_sched_attr attr; }; struct i915_dependency { --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12763,6 +12763,15 @@ static void intel_plane_unpin_fb(struct intel_unpin_fb_vma(vma, old_plane_state->flags); } +static void fb_obj_bump_render_priority(struct drm_i915_gem_object *obj) +{ + struct i915_sched_attr attr = { + .priority = I915_PRIORITY_DISPLAY, + }; + + i915_gem_object_wait_priority(obj, 0, &attr); +} + /** * intel_prepare_plane_fb - Prepare fb for usage on plane * @plane: drm plane to prepare for @@ -12839,7 +12848,7 @@ intel_prepare_plane_fb(struct drm_plane ret = intel_plane_pin_fb(to_intel_plane_state(new_state)); - i915_gem_object_wait_priority(obj, 0, I915_PRIORITY_DISPLAY); + fb_obj_bump_render_priority(obj); mutex_unlock(&dev_priv->drm.struct_mutex); i915_gem_object_unpin_pages(obj); --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1113,17 +1113,29 @@ unsigned int intel_engines_has_context_i return which; } +static void print_sched_attr(struct drm_printer *m, + const struct drm_i915_private *i915, + const struct i915_sched_attr *attr) +{ + if (attr->priority == I915_PRIORITY_INVALID) + return; + + drm_printf(m, "prio=%d", attr->priority); +} + static void print_request(struct drm_printer *m, struct i915_request *rq, const char *prefix) { const char *name = rq->fence.ops->get_timeline_name(&rq->fence); - drm_printf(m, "%s%x%s [%llx:%x] prio=%d @ %dms: %s\n", prefix, + drm_printf(m, "%s%x%s [%llx:%x] ", + prefix, rq->global_seqno, i915_request_completed(rq) ? "!" : "", - rq->fence.context, rq->fence.seqno, - rq->sched.priority, + rq->fence.context, rq->fence.seqno); + print_sched_attr(m, rq->i915, &rq->sched.attr); + drm_printf(m, " @ %dms: %s\n", jiffies_to_msecs(jiffies - rq->emitted_jiffies), name); } --- a/drivers/gpu/drm/i915/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/intel_guc_submission.c @@ -659,7 +659,7 @@ static void port_assign(struct execlist_ static inline int rq_prio(const struct i915_request *rq) { - return rq->sched.priority; + return rq->sched.attr.priority; } static inline int port_prio(const struct execlist_port *port) --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -177,7 +177,7 @@ static inline struct i915_priolist *to_p static inline int rq_prio(const struct i915_request *rq) { - return rq->sched.priority; + return rq->sched.attr.priority; } static inline bool need_preempt(const struct intel_engine_cs *engine, @@ -1173,11 +1173,13 @@ sched_lock_engine(struct i915_sched_node return engine; } -static void execlists_schedule(struct i915_request *request, int prio) +static void execlists_schedule(struct i915_request *request, + const struct i915_sched_attr *attr) { struct intel_engine_cs *engine; struct i915_dependency *dep, *p; struct i915_dependency stack; + const int prio = attr->priority; LIST_HEAD(dfs); GEM_BUG_ON(prio == I915_PRIORITY_INVALID); @@ -1185,7 +1187,7 @@ static void execlists_schedule(struct i9 if (i915_request_completed(request)) return; - if (prio <= READ_ONCE(request->sched.priority)) + if (prio <= READ_ONCE(request->sched.attr.priority)) return; /* Need BKL in order to use the temporary link inside i915_dependency */ @@ -1227,8 +1229,8 @@ static void execlists_schedule(struct i9 if (i915_sched_node_signaled(p->signaler)) continue; - GEM_BUG_ON(p->signaler->priority < node->priority); - if (prio > READ_ONCE(p->signaler->priority)) + GEM_BUG_ON(p->signaler->attr.priority < node->attr.priority); + if (prio > READ_ONCE(p->signaler->attr.priority)) list_move_tail(&p->dfs_link, &dfs); } } @@ -1239,9 +1241,9 @@ static void execlists_schedule(struct i9 * execlists_submit_request()), we can set our own priority and skip * acquiring the engine locks. */ - if (request->sched.priority == I915_PRIORITY_INVALID) { + if (request->sched.attr.priority == I915_PRIORITY_INVALID) { GEM_BUG_ON(!list_empty(&request->sched.link)); - request->sched.priority = prio; + request->sched.attr = *attr; if (stack.dfs_link.next == stack.dfs_link.prev) return; __list_del_entry(&stack.dfs_link); @@ -1258,10 +1260,10 @@ static void execlists_schedule(struct i9 engine = sched_lock_engine(node, engine); - if (prio <= node->priority) + if (prio <= node->attr.priority) continue; - node->priority = prio; + node->attr.priority = prio; if (!list_empty(&node->link)) { __list_del_entry(&node->link); queue_request(engine, node, prio); --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -13,6 +13,7 @@ #include "intel_gpu_commands.h" struct drm_printer; +struct i915_sched_attr; #define I915_CMD_HASH_ORDER 9 @@ -459,7 +460,8 @@ struct intel_engine_cs { * * Called under the struct_mutex. */ - void (*schedule)(struct i915_request *request, int priority); + void (*schedule)(struct i915_request *request, + const struct i915_sched_attr *attr); /* * Cancel all requests on the hardware, or queued for execution. --- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c +++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c @@ -628,7 +628,7 @@ static int active_engine(void *data) } if (arg->flags & TEST_PRIORITY) - ctx[idx]->priority = + ctx[idx]->sched.priority = i915_prandom_u32_max_state(512, &prng); rq[idx] = i915_request_get(new); @@ -683,7 +683,7 @@ static int __igt_reset_engines(struct dr return err; if (flags & TEST_PRIORITY) - h.ctx->priority = 1024; + h.ctx->sched.priority = 1024; } for_each_engine(engine, i915, id) { --- a/drivers/gpu/drm/i915/selftests/intel_lrc.c +++ b/drivers/gpu/drm/i915/selftests/intel_lrc.c @@ -335,12 +335,12 @@ static int live_preempt(void *arg) ctx_hi = kernel_context(i915); if (!ctx_hi) goto err_spin_lo; - ctx_hi->priority = I915_CONTEXT_MAX_USER_PRIORITY; + ctx_hi->sched.priority = I915_CONTEXT_MAX_USER_PRIORITY; ctx_lo = kernel_context(i915); if (!ctx_lo) goto err_ctx_hi; - ctx_lo->priority = I915_CONTEXT_MIN_USER_PRIORITY; + ctx_lo->sched.priority = I915_CONTEXT_MIN_USER_PRIORITY; for_each_engine(engine, i915, id) { struct i915_request *rq; @@ -407,6 +407,7 @@ static int live_late_preempt(void *arg) struct i915_gem_context *ctx_hi, *ctx_lo; struct spinner spin_hi, spin_lo; struct intel_engine_cs *engine; + struct i915_sched_attr attr = {}; enum intel_engine_id id; int err = -ENOMEM; @@ -458,7 +459,8 @@ static int live_late_preempt(void *arg) goto err_wedged; } - engine->schedule(rq, I915_PRIORITY_MAX); + attr.priority = I915_PRIORITY_MAX; + engine->schedule(rq, &attr); if (!wait_for_spinner(&spin_hi, rq)) { pr_err("High priority context failed to preempt the low priority context\n");