From patchwork Mon Aug 6 11:26:05 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 10556837 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 5B9C313B4 for ; Mon, 6 Aug 2018 11:26:21 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4023329319 for ; Mon, 6 Aug 2018 11:26:21 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 321932931C; Mon, 6 Aug 2018 11:26:21 +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 1A93E29319 for ; Mon, 6 Aug 2018 11:26:20 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 126AF6E1DB; Mon, 6 Aug 2018 11:26:19 +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 8175A6E1DB for ; Mon, 6 Aug 2018 11:26:17 +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 12651048-1500050 for multiple; Mon, 06 Aug 2018 12:26:06 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Mon, 6 Aug 2018 12:26:05 +0100 Message-Id: <20180806112605.20725-1-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.18.0 In-Reply-To: <20180806111121.12998-1-chris@chris-wilson.co.uk> References: <20180806111121.12998-1-chris@chris-wilson.co.uk> Subject: [Intel-gfx] [PATCH] drm/i915: Pull seqno started checks together 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 We have a few instances of checking seqno-1 to see if the HW has started the request. Pull those together under a helper. v2: Pull the !seqno assertion higher, as we given seqno==1 we may indeed check to see if we have started using seqno==0. Suggested-by: Tvrtko Ursulin Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Reviewed-by: Mika Kuoppala --- drivers/gpu/drm/i915/i915_request.c | 9 +++--- drivers/gpu/drm/i915/i915_request.h | 39 +++++++++++++++--------- drivers/gpu/drm/i915/intel_breadcrumbs.c | 6 ++-- drivers/gpu/drm/i915/intel_engine_cs.c | 3 +- drivers/gpu/drm/i915/intel_hangcheck.c | 2 +- drivers/gpu/drm/i915/intel_ringbuffer.h | 33 ++++++++++++++++---- 6 files changed, 60 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 5c2c93cbab12..09ed48833b54 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -527,7 +527,7 @@ void __i915_request_submit(struct i915_request *request) seqno = timeline_get_seqno(&engine->timeline); GEM_BUG_ON(!seqno); - GEM_BUG_ON(i915_seqno_passed(intel_engine_get_seqno(engine), seqno)); + GEM_BUG_ON(intel_engine_signaled(engine, seqno)); /* We may be recursing from the signal callback of another i915 fence */ spin_lock_nested(&request->lock, SINGLE_DEPTH_NESTING); @@ -579,8 +579,7 @@ void __i915_request_unsubmit(struct i915_request *request) */ GEM_BUG_ON(!request->global_seqno); GEM_BUG_ON(request->global_seqno != engine->timeline.seqno); - GEM_BUG_ON(i915_seqno_passed(intel_engine_get_seqno(engine), - request->global_seqno)); + GEM_BUG_ON(intel_engine_has_completed(engine, request->global_seqno)); engine->timeline.seqno--; /* We may be recursing from the signal callback of another i915 fence */ @@ -1205,7 +1204,7 @@ static bool __i915_spin_request(const struct i915_request *rq, * it is a fair assumption that it will not complete within our * relatively short timeout. */ - if (!i915_seqno_passed(intel_engine_get_seqno(engine), seqno - 1)) + if (!intel_engine_has_started(engine, seqno)) return false; /* @@ -1222,7 +1221,7 @@ static bool __i915_spin_request(const struct i915_request *rq, irq = READ_ONCE(engine->breadcrumbs.irq_count); timeout_us += local_clock_us(&cpu); do { - if (i915_seqno_passed(intel_engine_get_seqno(engine), seqno)) + if (intel_engine_has_completed(engine, seqno)) return seqno == i915_request_global_seqno(rq); /* diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h index e1c9365dfefb..05888de2e045 100644 --- a/drivers/gpu/drm/i915/i915_request.h +++ b/drivers/gpu/drm/i915/i915_request.h @@ -272,7 +272,10 @@ long i915_request_wait(struct i915_request *rq, #define I915_WAIT_ALL BIT(2) /* used by i915_gem_object_wait() */ #define I915_WAIT_FOR_IDLE_BOOST BIT(3) -static inline u32 intel_engine_get_seqno(struct intel_engine_cs *engine); +static inline bool intel_engine_has_started(struct intel_engine_cs *engine, + u32 seqno); +static inline bool intel_engine_has_completed(struct intel_engine_cs *engine, + u32 seqno); /** * Returns true if seq1 is later than seq2. @@ -282,11 +285,31 @@ static inline bool i915_seqno_passed(u32 seq1, u32 seq2) return (s32)(seq1 - seq2) >= 0; } +/** + * i915_request_started - check if the request has begun being executed + * @rq: the request + * + * Returns true if the request has been submitted to hardware, and the hardware + * has advanced passed the end of the previous request and so should be either + * currently processing the request (though it may be preempted and so + * not necessarily the next request to complete) or have completed the request. + */ +static inline bool i915_request_started(const struct i915_request *rq) +{ + u32 seqno; + + seqno = i915_request_global_seqno(rq); + if (!seqno) /* not yet submitted to HW */ + return false; + + return intel_engine_has_started(rq->engine, seqno); +} + static inline bool __i915_request_completed(const struct i915_request *rq, u32 seqno) { GEM_BUG_ON(!seqno); - return i915_seqno_passed(intel_engine_get_seqno(rq->engine), seqno) && + return intel_engine_has_completed(rq->engine, seqno) && seqno == i915_request_global_seqno(rq); } @@ -301,18 +324,6 @@ static inline bool i915_request_completed(const struct i915_request *rq) return __i915_request_completed(rq, seqno); } -static inline bool i915_request_started(const struct i915_request *rq) -{ - u32 seqno; - - seqno = i915_request_global_seqno(rq); - if (!seqno) - return false; - - return i915_seqno_passed(intel_engine_get_seqno(rq->engine), - seqno - 1); -} - static inline bool i915_sched_node_signaled(const struct i915_sched_node *node) { const struct i915_request *rq = diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c index 1db6ba7d926e..84bf8d827136 100644 --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c @@ -256,8 +256,7 @@ void intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine) spin_unlock(&b->irq_lock); rbtree_postorder_for_each_entry_safe(wait, n, &b->waiters, node) { - GEM_BUG_ON(!i915_seqno_passed(intel_engine_get_seqno(engine), - wait->seqno)); + GEM_BUG_ON(!intel_engine_signaled(engine, wait->seqno)); RB_CLEAR_NODE(&wait->node); wake_up_process(wait->tsk); } @@ -508,8 +507,7 @@ bool intel_engine_add_wait(struct intel_engine_cs *engine, return armed; /* Make the caller recheck if its request has already started. */ - return i915_seqno_passed(intel_engine_get_seqno(engine), - wait->seqno - 1); + return intel_engine_has_started(engine, wait->seqno); } static inline bool chain_wakeup(struct rb_node *rb, int priority) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 67c4fc5d737c..99d5a24219c1 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -968,8 +968,7 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine) return true; /* Any inflight/incomplete requests? */ - if (!i915_seqno_passed(intel_engine_get_seqno(engine), - intel_engine_last_submit(engine))) + if (!intel_engine_signaled(engine, intel_engine_last_submit(engine))) return false; if (I915_SELFTEST_ONLY(engine->breadcrumbs.mock)) diff --git a/drivers/gpu/drm/i915/intel_hangcheck.c b/drivers/gpu/drm/i915/intel_hangcheck.c index 2fc7a0dd0df9..e26d05a46451 100644 --- a/drivers/gpu/drm/i915/intel_hangcheck.c +++ b/drivers/gpu/drm/i915/intel_hangcheck.c @@ -142,7 +142,7 @@ static int semaphore_passed(struct intel_engine_cs *engine) if (signaller->hangcheck.deadlock >= I915_NUM_ENGINES) return -1; - if (i915_seqno_passed(intel_engine_get_seqno(signaller), seqno)) + if (intel_engine_signaled(signaller, seqno)) return 1; /* cursory check for an unkickable deadlock */ diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 57f3787ed6ec..70b2c6a7eb84 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -911,14 +911,10 @@ int intel_engine_stop_cs(struct intel_engine_cs *engine); u64 intel_engine_get_active_head(const struct intel_engine_cs *engine); u64 intel_engine_get_last_batch_head(const struct intel_engine_cs *engine); -static inline u32 intel_engine_get_seqno(struct intel_engine_cs *engine) -{ - return intel_read_status_page(engine, I915_GEM_HWS_INDEX); -} - static inline u32 intel_engine_last_submit(struct intel_engine_cs *engine) { - /* We are only peeking at the tail of the submit queue (and not the + /* + * We are only peeking at the tail of the submit queue (and not the * queue itself) in order to gain a hint as to the current active * state of the engine. Callers are not expected to be taking * engine->timeline->lock, nor are they expected to be concerned @@ -928,6 +924,31 @@ static inline u32 intel_engine_last_submit(struct intel_engine_cs *engine) return READ_ONCE(engine->timeline.seqno); } +static inline u32 intel_engine_get_seqno(struct intel_engine_cs *engine) +{ + return intel_read_status_page(engine, I915_GEM_HWS_INDEX); +} + +static inline bool intel_engine_signaled(struct intel_engine_cs *engine, + u32 seqno) +{ + return i915_seqno_passed(intel_engine_get_seqno(engine), seqno); +} + +static inline bool intel_engine_has_completed(struct intel_engine_cs *engine, + u32 seqno) +{ + GEM_BUG_ON(!seqno); + return intel_engine_signaled(engine, seqno); +} + +static inline bool intel_engine_has_started(struct intel_engine_cs *engine, + u32 seqno) +{ + GEM_BUG_ON(!seqno); + return intel_engine_signaled(engine, seqno - 1); +} + void intel_engine_get_instdone(struct intel_engine_cs *engine, struct intel_instdone *instdone);