From patchwork Thu Sep 6 07:51:57 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 10589971 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 915F1920 for ; Thu, 6 Sep 2018 07:53:51 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7823D1FFD8 for ; Thu, 6 Sep 2018 07:53:51 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6C7712A34D; Thu, 6 Sep 2018 07:53:51 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 076851FFD8 for ; Thu, 6 Sep 2018 07:53:51 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id EF54B6E5FA; Thu, 6 Sep 2018 07:53:49 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from fireflyinternet.com (mail.fireflyinternet.com [109.228.58.192]) by gabe.freedesktop.org (Postfix) with ESMTPS id C59796E5F5 for ; Thu, 6 Sep 2018 07:53:45 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from haswell.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 13600605-1500050 for multiple; Thu, 06 Sep 2018 08:52:01 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Thu, 6 Sep 2018 08:51:57 +0100 Message-Id: <20180906075201.22619-3-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.19.0.rc2 In-Reply-To: <20180906075201.22619-1-chris@chris-wilson.co.uk> References: <20180906075201.22619-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 3/7] drm/i915/execlists: Avoid kicking priority on the current context X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP If the request is currently on the HW (in port 0), then we do not need to kick the submission tasklet to evaluate whether we should be preempting itself in order to execute it again. In the case that was annoying me: execlists_schedule: rq(18:211173).prio=0 -> 2 need_preempt: last(18:211174).prio=0, queue.prio=2 We are bumping the priority of the first of a pair of requests running in the current context. Then when evaluating preempt, we would see that that our priority request is higher than the last executing request in ELSP0 and so trigger preemption, not realising that our intended request was already executing. v2: As we assume state of the execlists->port[] that is only valid while we hold the timeline lock we have to repeat some earlier tests that on the validity of the node. v3: Wrap guc submission under the timeline.lock as is now the way of all things. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/intel_guc_submission.c | 18 +++------ drivers/gpu/drm/i915/intel_lrc.c | 41 +++++++++++++++------ 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c index 07b9d313b019..7b878790228a 100644 --- a/drivers/gpu/drm/i915/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/intel_guc_submission.c @@ -771,19 +771,8 @@ static bool __guc_dequeue(struct intel_engine_cs *engine) static void guc_dequeue(struct intel_engine_cs *engine) { - unsigned long flags; - bool submit; - - local_irq_save(flags); - - spin_lock(&engine->timeline.lock); - submit = __guc_dequeue(engine); - spin_unlock(&engine->timeline.lock); - - if (submit) + if ( __guc_dequeue(engine)) guc_submit(engine); - - local_irq_restore(flags); } static void guc_submission_tasklet(unsigned long data) @@ -792,6 +781,9 @@ static void guc_submission_tasklet(unsigned long data) struct intel_engine_execlists * const execlists = &engine->execlists; struct execlist_port *port = execlists->port; struct i915_request *rq; + unsigned long flags; + + spin_lock_irqsave(&engine->timeline.lock, flags); rq = port_request(port); while (rq && i915_request_completed(rq)) { @@ -815,6 +807,8 @@ static void guc_submission_tasklet(unsigned long data) if (!execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT)) guc_dequeue(engine); + + spin_unlock_irqrestore(&engine->timeline.lock, flags); } static struct i915_request * diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 066ab178a8b2..881ab979f02f 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -355,13 +355,8 @@ execlists_unwind_incomplete_requests(struct intel_engine_execlists *execlists) { struct intel_engine_cs *engine = container_of(execlists, typeof(*engine), execlists); - unsigned long flags; - - spin_lock_irqsave(&engine->timeline.lock, flags); __unwind_incomplete_requests(engine); - - spin_unlock_irqrestore(&engine->timeline.lock, flags); } static inline void @@ -1234,9 +1229,13 @@ static void execlists_schedule(struct i915_request *request, engine = sched_lock_engine(node, engine); + /* Recheck after acquiring the engine->timeline.lock */ if (prio <= node->attr.priority) continue; + if (i915_sched_node_signaled(node)) + continue; + node->attr.priority = prio; if (!list_empty(&node->link)) { if (last != engine) { @@ -1245,14 +1244,34 @@ static void execlists_schedule(struct i915_request *request, } GEM_BUG_ON(pl->priority != prio); list_move_tail(&node->link, &pl->requests); + } else { + /* + * If the request is not in the priolist queue because + * it is not yet runnable, then it doesn't contribute + * to our preemption decisions. On the other hand, + * if the request is on the HW, it too is not in the + * queue; but in that case we may still need to reorder + * the inflight requests. + */ + if (!i915_sw_fence_done(&sched_to_request(node)->submit)) + continue; } - if (prio > engine->execlists.queue_priority && - i915_sw_fence_done(&sched_to_request(node)->submit)) { - /* defer submission until after all of our updates */ - __update_queue(engine, prio); - tasklet_hi_schedule(&engine->execlists.tasklet); - } + if (prio <= engine->execlists.queue_priority) + continue; + + /* + * If we are already the currently executing context, don't + * bother evaluating if we should preempt ourselves. + */ + if (sched_to_request(node)->global_seqno && + i915_seqno_passed(port_request(engine->execlists.port)->global_seqno, + sched_to_request(node)->global_seqno)) + continue; + + /* Defer (tasklet) submission until after all of our updates. */ + __update_queue(engine, prio); + tasklet_hi_schedule(&engine->execlists.tasklet); } spin_unlock_irq(&engine->timeline.lock);