From patchwork Thu May 17 06:03:34 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 10405339 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id BADF26037D for ; Thu, 17 May 2018 06:08:24 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A98692887E for ; Thu, 17 May 2018 06:08:24 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9E28328939; Thu, 17 May 2018 06:08:24 +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 267622887E for ; Thu, 17 May 2018 06:08:24 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 347A66E5DB; Thu, 17 May 2018 06:08:23 +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 AF0346E5D2 for ; Thu, 17 May 2018 06:08:21 +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 11730244-1500050 for multiple; Thu, 17 May 2018 07:08:19 +0100 Received: by haswell.alporthouse.com (sSMTP sendmail emulation); Thu, 17 May 2018 07:08:18 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Thu, 17 May 2018 07:03:34 +0100 Message-Id: <20180517060738.19193-18-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.17.0 In-Reply-To: <20180517060738.19193-1-chris@chris-wilson.co.uk> References: <20180517060738.19193-1-chris@chris-wilson.co.uk> X-Originating-IP: 78.156.65.138 X-Country: code=GB country="United Kingdom" ip=78.156.65.138 Subject: [Intel-gfx] [PATCH 018/262] drm/i915/execlists: Direct submission (avoid tasklet/ksoftirqd) 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: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP Back in commit 27af5eea54d1 ("drm/i915: Move execlists irq handler to a bottom half"), we came to the conclusion that running our CSB processing and ELSP submission from inside the irq handler was a bad idea. A really bad idea as we could impose nearly 1s latency on other users of the system, on average! Deferring our work to a tasklet allowed us to do the processing with irqs enabled, reducing the impact to an average of about 50us. We have since eradicated the use of forcewaked mmio from inside the CSB processing and ELSP submission, bringing the impact down to around 5us (on Kabylake); an order of magnitude better than our measurements 2 years ago on Broadwell and only about 2x worse on average than the gem_syslatency on an unladen system. Comparing the impact on the maximum latency observed over a 120s interval, repeated several times (using gem_syslatency, similar to RT's cyclictest) while the system is fully laden with i915 nops, we see that direct submission definitely worsens the response but not to the same outlandish degree as before. x Unladen baseline + Using tasklet * Direct submission +------------------------------------------------------------------------+ |xx x ++ +++ + * * * ** *** * *| ||A| |__AM__| |_____A_M___| | +------------------------------------------------------------------------+ N Min Max Median Avg Stddev x 10 5 18 10 9.3 3.6530049 + 10 72 120 108 102.9 15.758243 * 10 255 348 316 305.7 28.74814 And with a background load +------------------------------------------------------------------------+ |x + * * | |x + + + + + + +* * ** ++ * * * *| |A |_______A_____|__|_______A___M______| | +------------------------------------------------------------------------+ N Min Max Median Avg Stddev x 10 4 11 9 8.5 2.1730675 + 10 633 1388 972 993 243.33744 * 10 1152 2109 1608 1488.3 314.80719 References: 27af5eea54d1 ("drm/i915: Move execlists irq handler to a bottom half") Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_irq.c | 11 ++------ drivers/gpu/drm/i915/intel_lrc.c | 44 +++++++++++++++++--------------- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 3f139ff64385..8b61ebf5cb4a 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1462,22 +1462,15 @@ static void snb_gt_irq_handler(struct drm_i915_private *dev_priv, static void gen8_cs_irq_handler(struct intel_engine_cs *engine, u32 iir) { - bool tasklet = false; - - if (iir & GT_CONTEXT_SWITCH_INTERRUPT) { + if (iir & GT_CONTEXT_SWITCH_INTERRUPT) intel_engine_handle_execlists_irq(engine); - tasklet = true; - } if (iir & GT_RENDER_USER_INTERRUPT) { if (intel_engine_uses_guc(engine)) - tasklet = true; + tasklet_hi_schedule(&engine->execlists.tasklet); notify_ring(engine); } - - if (tasklet) - tasklet_hi_schedule(&engine->execlists.tasklet); } static void gen8_gt_irq_ack(struct drm_i915_private *i915, diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 954eb3a71051..37839d89e03a 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -575,7 +575,7 @@ static void complete_preempt_context(struct intel_engine_execlists *execlists) execlists_clear_active(execlists, EXECLISTS_ACTIVE_PREEMPT); } -static void __execlists_dequeue(struct intel_engine_cs *engine) +static void execlists_dequeue(struct intel_engine_cs *engine) { struct intel_engine_execlists * const execlists = &engine->execlists; struct execlist_port *port = execlists->port; @@ -587,7 +587,11 @@ static void __execlists_dequeue(struct intel_engine_cs *engine) lockdep_assert_held(&engine->timeline.lock); - /* Hardware submission is through 2 ports. Conceptually each port + if (execlists_is_active(&engine->execlists, EXECLISTS_ACTIVE_PREEMPT)) + return; + + /* + * Hardware submission is through 2 ports. Conceptually each port * has a (RING_START, RING_HEAD, RING_TAIL) tuple. RING_START is * static for a context, and unique to each, so we only execute * requests belonging to a single context from each ring. RING_HEAD @@ -777,15 +781,6 @@ static void __execlists_dequeue(struct intel_engine_cs *engine) !port_isset(engine->execlists.port)); } -static void execlists_dequeue(struct intel_engine_cs *engine) -{ - unsigned long flags; - - spin_lock_irqsave(&engine->timeline.lock, flags); - __execlists_dequeue(engine); - spin_unlock_irqrestore(&engine->timeline.lock, flags); -} - void execlists_cancel_port_requests(struct intel_engine_execlists * const execlists) { @@ -1106,6 +1101,7 @@ void intel_engine_handle_execlists_irq(struct intel_engine_cs *engine) execlists->csb_read); execlists->csb_head = head; + execlists_dequeue(engine); spin_unlock(&engine->timeline.lock); } @@ -1122,8 +1118,9 @@ static void execlists_submission_tasklet(unsigned long data) engine->i915->gt.awake, engine->execlists.active); - if (!execlists_is_active(&engine->execlists, EXECLISTS_ACTIVE_PREEMPT)) - execlists_dequeue(engine); + spin_lock_irq(&engine->timeline.lock); + execlists_dequeue(engine); + spin_unlock_irq(&engine->timeline.lock); } static void queue_request(struct intel_engine_cs *engine, @@ -1134,16 +1131,20 @@ static void queue_request(struct intel_engine_cs *engine, &lookup_priolist(engine, prio)->requests); } -static void __submit_queue(struct intel_engine_cs *engine, int prio) +static void __update_queue(struct intel_engine_cs *engine, int prio) { engine->execlists.queue_priority = prio; - tasklet_hi_schedule(&engine->execlists.tasklet); } static void submit_queue(struct intel_engine_cs *engine, int prio) { - if (prio > engine->execlists.queue_priority) - __submit_queue(engine, prio); + if (prio > engine->execlists.queue_priority) { + __update_queue(engine, prio); + if (!intel_engine_uses_guc(engine)) + execlists_dequeue(engine); + else + tasklet_hi_schedule(&engine->execlists.tasklet); + } } static void execlists_submit_request(struct i915_request *request) @@ -1155,11 +1156,12 @@ static void execlists_submit_request(struct i915_request *request) spin_lock_irqsave(&engine->timeline.lock, flags); queue_request(engine, &request->sched, rq_prio(request)); - submit_queue(engine, rq_prio(request)); GEM_BUG_ON(!engine->execlists.first); GEM_BUG_ON(list_empty(&request->sched.link)); + submit_queue(engine, rq_prio(request)); + spin_unlock_irqrestore(&engine->timeline.lock, flags); } @@ -1286,8 +1288,10 @@ static void execlists_schedule(struct i915_request *request, } if (prio > engine->execlists.queue_priority && - i915_sw_fence_done(&sched_to_request(node)->submit)) - __submit_queue(engine, prio); + i915_sw_fence_done(&sched_to_request(node)->submit)) { + __update_queue(engine, prio); + tasklet_hi_schedule(&engine->execlists.tasklet); + } } spin_unlock_irq(&engine->timeline.lock);