From patchwork Tue Mar 31 21:30:58 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 11468407 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id B698E159A for ; Tue, 31 Mar 2020 21:31:29 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 9FC242080C for ; Tue, 31 Mar 2020 21:31:29 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9FC242080C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=chris-wilson.co.uk Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 292666E89B; Tue, 31 Mar 2020 21:31:26 +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 6A0BE6E899 for ; Tue, 31 Mar 2020 21:31:23 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from build.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 20757565-1500050 for multiple; Tue, 31 Mar 2020 22:31:10 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Tue, 31 Mar 2020 22:30:58 +0100 Message-Id: <20200331213108.11340-1-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 01/11] drm/i915/gem: Ignore readonly failures when updating relocs X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Matthew Auld , Chris Wilson Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" If the user passes in a readonly reloc[], by the time we notice we have already commited to modifying the execobjects, or have indeed done so already. Reporting the failure just compounds the issue as we have no second pass to fall back to anymore. Testcase: igt/gem_exec_reloc/readonly Fixes: 7dc8f1143778 ("drm/i915/gem: Drop relocation slowpath") References: fddcd00a49e9 ("drm/i915: Force the slow path after a user-write error") Signed-off-by: Chris Wilson Cc: Matthew Auld Cc: Joonas Lahtinen --- drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index cda35e6dfc44..bc8aa9604787 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -1540,10 +1540,8 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct eb_vma *ev) * can read from this userspace address. */ offset = gen8_canonical_addr(offset & ~UPDATE); - if (unlikely(__put_user(offset, &urelocs[r-stack].presumed_offset))) { - remain = -EFAULT; - goto out; - } + __put_user(offset, + &urelocs[r-stack].presumed_offset); } } while (r++, --count); urelocs += ARRAY_SIZE(stack); From patchwork Tue Mar 31 21:30:59 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 11468409 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 76B0B14B4 for ; Tue, 31 Mar 2020 21:31:30 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 5F8F82080C for ; Tue, 31 Mar 2020 21:31:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5F8F82080C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=chris-wilson.co.uk Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 40EB16E89D; Tue, 31 Mar 2020 21:31:27 +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 6DB756E2EC for ; Tue, 31 Mar 2020 21:31:24 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from build.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 20757566-1500050 for multiple; Tue, 31 Mar 2020 22:31:10 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Tue, 31 Mar 2020 22:30:59 +0100 Message-Id: <20200331213108.11340-2-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200331213108.11340-1-chris@chris-wilson.co.uk> References: <20200331213108.11340-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 02/11] drm/i915/gt: Fill all the unused space in the GGTT X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Matthew Auld , stable@vger.kernel.org, Chris Wilson Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" When we allocate space in the GGTT we may have to allocate a larger region than will be populated by the object to accommodate fencing. Make sure that this space beyond the end of the buffer points safely into scratch space, in case the HW tries to access it anyway (e.g. fenced access to the last tile row). v2: Preemptively / conservatively guard gen6 ggtt as well. Reported-by: Imre Deak References: https://gitlab.freedesktop.org/drm/intel/-/issues/1554 Signed-off-by: Chris Wilson Cc: Matthew Auld Cc: Imre Deak Cc: stable@vger.kernel.org Reviewed-by: Matthew Auld --- drivers/gpu/drm/i915/gt/intel_ggtt.c | 37 ++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index d8944dabed55..ae07bcd7c226 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -191,10 +191,11 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm, enum i915_cache_level level, u32 flags) { - struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm); - struct sgt_iter sgt_iter; - gen8_pte_t __iomem *gtt_entries; const gen8_pte_t pte_encode = gen8_ggtt_pte_encode(0, level, 0); + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm); + gen8_pte_t __iomem *gte; + gen8_pte_t __iomem *end; + struct sgt_iter iter; dma_addr_t addr; /* @@ -202,10 +203,17 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm, * not to allow the user to override access to a read only page. */ - gtt_entries = (gen8_pte_t __iomem *)ggtt->gsm; - gtt_entries += vma->node.start / I915_GTT_PAGE_SIZE; - for_each_sgt_daddr(addr, sgt_iter, vma->pages) - gen8_set_pte(gtt_entries++, pte_encode | addr); + gte = (gen8_pte_t __iomem *)ggtt->gsm; + gte += vma->node.start / I915_GTT_PAGE_SIZE; + end = gte + vma->node.size / I915_GTT_PAGE_SIZE; + + for_each_sgt_daddr(addr, iter, vma->pages) + gen8_set_pte(gte++, pte_encode | addr); + GEM_BUG_ON(gte > end); + + /* Fill the allocated but "unused" space beyond the end of the buffer */ + while (gte < end) + gen8_set_pte(gte++, vm->scratch[0].encode); /* * We want to flush the TLBs only after we're certain all the PTE @@ -241,13 +249,22 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm, u32 flags) { struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm); - gen6_pte_t __iomem *entries = (gen6_pte_t __iomem *)ggtt->gsm; - unsigned int i = vma->node.start / I915_GTT_PAGE_SIZE; + gen6_pte_t __iomem *gte; + gen6_pte_t __iomem *end; struct sgt_iter iter; dma_addr_t addr; + gte = (gen6_pte_t __iomem *)ggtt->gsm; + gte += vma->node.start / I915_GTT_PAGE_SIZE; + end = gte + vma->node.size / I915_GTT_PAGE_SIZE; + for_each_sgt_daddr(addr, iter, vma->pages) - iowrite32(vm->pte_encode(addr, level, flags), &entries[i++]); + iowrite32(vm->pte_encode(addr, level, flags), gte++); + GEM_BUG_ON(gte > end); + + /* Fill the allocated but "unused" space beyond the end of the buffer */ + while (gte < end) + iowrite32(vm->scratch[0].encode, gte++); /* * We want to flush the TLBs only after we're certain all the PTE From patchwork Tue Mar 31 21:31:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 11468401 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id E9D0E159A for ; Tue, 31 Mar 2020 21:31:26 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D2E4A20838 for ; Tue, 31 Mar 2020 21:31:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D2E4A20838 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=chris-wilson.co.uk Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1AE6C6E899; Tue, 31 Mar 2020 21:31:26 +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 7210089956 for ; Tue, 31 Mar 2020 21:31:19 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from build.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 20757567-1500050 for multiple; Tue, 31 Mar 2020 22:31:10 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Tue, 31 Mar 2020 22:31:00 +0100 Message-Id: <20200331213108.11340-3-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200331213108.11340-1-chris@chris-wilson.co.uk> References: <20200331213108.11340-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 03/11] drm/i915/execlists: Peek at the next submission for error interrupts X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Chris Wilson Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" If we receive the error interrupt before the CS interrupt, we may find ourselves without an active request to reset, skipping the GPU reset. All because the attempt to reset was too early. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/gt/intel_lrc.c | 41 ++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index 3479cda37fdc..f028114714cd 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -2804,6 +2804,45 @@ static struct execlists_capture *capture_regs(struct intel_engine_cs *engine) return NULL; } +static struct i915_request * +active_context(struct intel_engine_cs *engine, u32 ccid) +{ + const struct intel_engine_execlists * const el = &engine->execlists; + struct i915_request * const *port, *rq; + + /* + * Use the most recent result from process_csb(), but just in case + * we trigger an error (via interrupt) before the first CS event has + * been written, peek at the next submission. + */ + + for (port = el->active; (rq = *port); port++) { + if (upper_32_bits(rq->context->lrc_desc) == ccid) { + ENGINE_TRACE(engine, + "ccid found at active:%zd\n", + port - el->active); + return rq; + } + } + + for (port = el->pending; (rq = *port); port++) { + if (upper_32_bits(rq->context->lrc_desc) == ccid) { + ENGINE_TRACE(engine, + "ccid found at pending:%zd\n", + port - el->pending); + return rq; + } + } + + ENGINE_TRACE(engine, "ccid:%x not found\n", ccid); + return NULL; +} + +static u32 active_ccid(struct intel_engine_cs *engine) +{ + return ENGINE_READ_FW(engine, RING_EXECLIST_STATUS_HI); +} + static bool execlists_capture(struct intel_engine_cs *engine) { struct execlists_capture *cap; @@ -2821,7 +2860,7 @@ static bool execlists_capture(struct intel_engine_cs *engine) return true; spin_lock_irq(&engine->active.lock); - cap->rq = execlists_active(&engine->execlists); + cap->rq = active_context(engine, active_ccid(engine)); if (cap->rq) { cap->rq = active_request(cap->rq->context->timeline, cap->rq); cap->rq = i915_request_get_rcu(cap->rq); From patchwork Tue Mar 31 21:31:01 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 11468395 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 8C16614B4 for ; Tue, 31 Mar 2020 21:31:20 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 67FD820838 for ; Tue, 31 Mar 2020 21:31:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 67FD820838 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=chris-wilson.co.uk Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C42F7899C4; Tue, 31 Mar 2020 21:31: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 87342899C4 for ; Tue, 31 Mar 2020 21:31:18 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from build.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 20757568-1500050 for multiple; Tue, 31 Mar 2020 22:31:10 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Tue, 31 Mar 2020 22:31:01 +0100 Message-Id: <20200331213108.11340-4-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200331213108.11340-1-chris@chris-wilson.co.uk> References: <20200331213108.11340-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 04/11] drm/i915/execlists: Record the active CCID from before reset X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Chris Wilson Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" If we cannot trust the reset will flush out the CS event queue such that process_csb() reports an accurate view of HW, we will need to search the active and pending contexts to determine which was actually running at the time we issued the reset. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/gt/intel_engine_types.h | 5 +++++ drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h index 80cdde712842..4804587442e7 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h @@ -166,6 +166,11 @@ struct intel_engine_execlists { */ u32 error_interrupt; + /** + * @reset_ccid: Active CCID [EXECLISTS_STATUS_HI] at the time of reset + */ + u32 reset_ccid; + /** * @no_priolist: priority lists disabled */ diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index f028114714cd..55bf3cdf3b38 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -3724,6 +3724,8 @@ static void execlists_reset_prepare(struct intel_engine_cs *engine) */ ring_set_paused(engine, 1); intel_engine_stop_cs(engine); + + engine->execlists.reset_ccid = active_ccid(engine); } static void reset_csb_pointers(struct intel_engine_cs *engine) @@ -3798,7 +3800,7 @@ static void __execlists_reset(struct intel_engine_cs *engine, bool stalled) * its request, it was still running at the time of the * reset and will have been clobbered. */ - rq = execlists_active(execlists); + rq = active_context(engine, engine->execlists.reset_ccid); if (!rq) goto unwind; From patchwork Tue Mar 31 21:31:02 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 11468411 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 3773714B4 for ; Tue, 31 Mar 2020 21:31:43 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 1F6882080C for ; Tue, 31 Mar 2020 21:31:42 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1F6882080C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=chris-wilson.co.uk Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 865B989B95; Tue, 31 Mar 2020 21:31:42 +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 8697789956 for ; Tue, 31 Mar 2020 21:31:18 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from build.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 20757569-1500050 for multiple; Tue, 31 Mar 2020 22:31:10 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Tue, 31 Mar 2020 22:31:02 +0100 Message-Id: <20200331213108.11340-5-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200331213108.11340-1-chris@chris-wilson.co.uk> References: <20200331213108.11340-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 05/11] drm/i915/gem: Utilize rcu iteration of context engines X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Chris Wilson Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" Now that we can peek at GEM->engines[] and obtain a reference to them using RCU, do so for instances where we can safely iterate the potentially old copy of the engines. For setting, we can do this when we know the engine properties are copied over before swapping, so we know the new engines already have the global property and we update the old before they are discarded. For reading, we only need to be safe; as we do so on behalf of the user, their races are their own problem. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 56 ++++++++++----------- 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index 50e7580f9337..a8780ddc07c0 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -757,21 +757,46 @@ __create_context(struct drm_i915_private *i915) return ERR_PTR(err); } +static inline struct i915_gem_engines * +__context_engines_await(const struct i915_gem_context *ctx) +{ + struct i915_gem_engines *engines; + + rcu_read_lock(); + do { + engines = rcu_dereference(ctx->engines); + GEM_BUG_ON(!engines); + + if (unlikely(!i915_sw_fence_await(&engines->fence))) + continue; + + if (likely(engines == rcu_access_pointer(ctx->engines))) + break; + + i915_sw_fence_complete(&engines->fence); + } while (1); + rcu_read_unlock(); + + return engines; +} + static int context_apply_all(struct i915_gem_context *ctx, int (*fn)(struct intel_context *ce, void *data), void *data) { struct i915_gem_engines_iter it; + struct i915_gem_engines *e; struct intel_context *ce; int err = 0; - for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) { + e = __context_engines_await(ctx); + for_each_gem_engine(ce, e, it) { err = fn(ce, data); if (err) break; } - i915_gem_context_unlock_engines(ctx); + i915_sw_fence_complete(&e->fence); return err; } @@ -858,10 +883,7 @@ i915_gem_create_context(struct drm_i915_private *i915, unsigned int flags) return ERR_CAST(ppgtt); } - mutex_lock(&ctx->mutex); __assign_ppgtt(ctx, &ppgtt->vm); - mutex_unlock(&ctx->mutex); - i915_vm_put(&ppgtt->vm); } @@ -1069,30 +1091,6 @@ static void cb_retire(struct i915_active *base) kfree(cb); } -static inline struct i915_gem_engines * -__context_engines_await(const struct i915_gem_context *ctx) -{ - struct i915_gem_engines *engines; - - rcu_read_lock(); - do { - engines = rcu_dereference(ctx->engines); - if (unlikely(!engines)) - break; - - if (unlikely(!i915_sw_fence_await(&engines->fence))) - continue; - - if (likely(engines == rcu_access_pointer(ctx->engines))) - break; - - i915_sw_fence_complete(&engines->fence); - } while (1); - rcu_read_unlock(); - - return engines; -} - I915_SELFTEST_DECLARE(static intel_engine_mask_t context_barrier_inject_fault); static int context_barrier_task(struct i915_gem_context *ctx, intel_engine_mask_t engines, From patchwork Tue Mar 31 21:31:03 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 11468403 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1B85414B4 for ; Tue, 31 Mar 2020 21:31:28 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 025BF2080C for ; Tue, 31 Mar 2020 21:31:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 025BF2080C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=chris-wilson.co.uk Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1455D6E898; Tue, 31 Mar 2020 21:31:26 +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 6975C6E898 for ; Tue, 31 Mar 2020 21:31:23 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from build.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 20757570-1500050 for multiple; Tue, 31 Mar 2020 22:31:10 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Tue, 31 Mar 2020 22:31:03 +0100 Message-Id: <20200331213108.11340-6-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200331213108.11340-1-chris@chris-wilson.co.uk> References: <20200331213108.11340-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 06/11] drm/i915/gem: Prevent switching of active GEM context VM X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Chris Wilson Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" A nasty issue was found with the way we serialise the update of the GTT (GPU virtual address space) on the GEM context and with execution via execbuf. On update we serialise with ctx->mutex and attempt to rewrite the GTT addresses stored within the context from inside a request (along that context), but from execbuf we were cheerfully using whatever address was stored inside the intel_context. Thus there was a race where we might be using a stale vm when the user updated the GEM context concurrently. To prevent that, deny updating an active context and only allow switching the VM when the context is unpinned (not in use on the HW). To illustrate the race, consider the execution flow through execbuf. 1. Lookup all the vma for every execobject in eb->context->vm, the current GEM context->vm 2. Submit the request along eb->context, presuming it still uses eb->context->vm Since the update of the vm inside the GEM context is not synchronised to execbuf, we can submit a request along eb->context that uses a different vm to all the vma we constructed. While this may be described as a userspace race, we have been tricked into using addresses in one VM with another loaded, albeit both belong to the same user. Reported-by: Maarten Lankhorst Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Maarten Lankhorst --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 216 ++---------------- .../drm/i915/gem/selftests/i915_gem_context.c | 125 ---------- drivers/gpu/drm/i915/gt/intel_context.h | 13 ++ drivers/gpu/drm/i915/gt/intel_context_param.c | 33 +++ drivers/gpu/drm/i915/gt/intel_context_param.h | 4 + drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +- .../drm/i915/selftests/i915_mock_selftests.h | 1 - 7 files changed, 67 insertions(+), 327 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index a8780ddc07c0..2aef6073001c 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -803,22 +803,29 @@ context_apply_all(struct i915_gem_context *ctx, static int __apply_ppgtt(struct intel_context *ce, void *vm) { - i915_vm_put(ce->vm); - ce->vm = i915_vm_get(vm); - return 0; + return intel_context_set_vm(ce, vm); } -static struct i915_address_space * +static int __set_ppgtt(struct i915_gem_context *ctx, struct i915_address_space *vm) { struct i915_address_space *old = i915_gem_context_vm(ctx); + int err; GEM_BUG_ON(old && i915_vm_is_4lvl(vm) != i915_vm_is_4lvl(old)); rcu_assign_pointer(ctx->vm, i915_vm_open(vm)); - context_apply_all(ctx, __apply_ppgtt, vm); + err = context_apply_all(ctx, __apply_ppgtt, vm); + if (err) { + /* Reapplying the old vm should not have to wait... */ + rcu_assign_pointer(ctx->vm, old); + context_apply_all(ctx, __apply_ppgtt, old); + old = vm; + } + if (old) + i915_vm_close(old); - return old; + return err; } static void __assign_ppgtt(struct i915_gem_context *ctx, @@ -827,9 +834,7 @@ static void __assign_ppgtt(struct i915_gem_context *ctx, if (vm == rcu_access_pointer(ctx->vm)) return; - vm = __set_ppgtt(ctx, vm); - if (vm) - i915_vm_close(vm); + __set_ppgtt(ctx, vm); } static void __set_timeline(struct intel_timeline **dst, @@ -1073,98 +1078,6 @@ int i915_gem_vm_destroy_ioctl(struct drm_device *dev, void *data, return 0; } -struct context_barrier_task { - struct i915_active base; - void (*task)(void *data); - void *data; -}; - -__i915_active_call -static void cb_retire(struct i915_active *base) -{ - struct context_barrier_task *cb = container_of(base, typeof(*cb), base); - - if (cb->task) - cb->task(cb->data); - - i915_active_fini(&cb->base); - kfree(cb); -} - -I915_SELFTEST_DECLARE(static intel_engine_mask_t context_barrier_inject_fault); -static int context_barrier_task(struct i915_gem_context *ctx, - intel_engine_mask_t engines, - bool (*skip)(struct intel_context *ce, void *data), - int (*emit)(struct i915_request *rq, void *data), - void (*task)(void *data), - void *data) -{ - struct context_barrier_task *cb; - struct i915_gem_engines_iter it; - struct i915_gem_engines *e; - struct intel_context *ce; - int err = 0; - - GEM_BUG_ON(!task); - - cb = kmalloc(sizeof(*cb), GFP_KERNEL); - if (!cb) - return -ENOMEM; - - i915_active_init(&cb->base, NULL, cb_retire); - err = i915_active_acquire(&cb->base); - if (err) { - kfree(cb); - return err; - } - - e = __context_engines_await(ctx); - if (!e) { - i915_active_release(&cb->base); - return -ENOENT; - } - - for_each_gem_engine(ce, e, it) { - struct i915_request *rq; - - if (I915_SELFTEST_ONLY(context_barrier_inject_fault & - ce->engine->mask)) { - err = -ENXIO; - break; - } - - if (!(ce->engine->mask & engines)) - continue; - - if (skip && skip(ce, data)) - continue; - - rq = intel_context_create_request(ce); - if (IS_ERR(rq)) { - err = PTR_ERR(rq); - break; - } - - err = 0; - if (emit) - err = emit(rq, data); - if (err == 0) - err = i915_active_add_request(&cb->base, rq); - - i915_request_add(rq); - if (err) - break; - } - i915_sw_fence_complete(&e->fence); - - cb->task = err ? NULL : task; /* caller needs to unwind instead */ - cb->data = data; - - i915_active_release(&cb->base); - - return err; -} - static int get_ppgtt(struct drm_i915_file_private *file_priv, struct i915_gem_context *ctx, struct drm_i915_gem_context_param *args) @@ -1197,93 +1110,11 @@ static int get_ppgtt(struct drm_i915_file_private *file_priv, return err; } -static void set_ppgtt_barrier(void *data) -{ - struct i915_address_space *old = data; - - if (INTEL_GEN(old->i915) < 8) - gen6_ppgtt_unpin_all(i915_vm_to_ppgtt(old)); - - i915_vm_close(old); -} - -static int emit_ppgtt_update(struct i915_request *rq, void *data) -{ - struct i915_address_space *vm = rq->context->vm; - struct intel_engine_cs *engine = rq->engine; - u32 base = engine->mmio_base; - u32 *cs; - int i; - - if (i915_vm_is_4lvl(vm)) { - struct i915_ppgtt *ppgtt = i915_vm_to_ppgtt(vm); - const dma_addr_t pd_daddr = px_dma(ppgtt->pd); - - cs = intel_ring_begin(rq, 6); - if (IS_ERR(cs)) - return PTR_ERR(cs); - - *cs++ = MI_LOAD_REGISTER_IMM(2); - - *cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(base, 0)); - *cs++ = upper_32_bits(pd_daddr); - *cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_LDW(base, 0)); - *cs++ = lower_32_bits(pd_daddr); - - *cs++ = MI_NOOP; - intel_ring_advance(rq, cs); - } else if (HAS_LOGICAL_RING_CONTEXTS(engine->i915)) { - struct i915_ppgtt *ppgtt = i915_vm_to_ppgtt(vm); - int err; - - /* Magic required to prevent forcewake errors! */ - err = engine->emit_flush(rq, EMIT_INVALIDATE); - if (err) - return err; - - cs = intel_ring_begin(rq, 4 * GEN8_3LVL_PDPES + 2); - if (IS_ERR(cs)) - return PTR_ERR(cs); - - *cs++ = MI_LOAD_REGISTER_IMM(2 * GEN8_3LVL_PDPES) | MI_LRI_FORCE_POSTED; - for (i = GEN8_3LVL_PDPES; i--; ) { - const dma_addr_t pd_daddr = i915_page_dir_dma_addr(ppgtt, i); - - *cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(base, i)); - *cs++ = upper_32_bits(pd_daddr); - *cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_LDW(base, i)); - *cs++ = lower_32_bits(pd_daddr); - } - *cs++ = MI_NOOP; - intel_ring_advance(rq, cs); - } - - return 0; -} - -static bool skip_ppgtt_update(struct intel_context *ce, void *data) -{ - if (!test_bit(CONTEXT_ALLOC_BIT, &ce->flags)) - return true; - - if (HAS_LOGICAL_RING_CONTEXTS(ce->engine->i915)) - return false; - - if (!atomic_read(&ce->pin_count)) - return true; - - /* ppGTT is not part of the legacy context image */ - if (gen6_ppgtt_pin(i915_vm_to_ppgtt(ce->vm))) - return true; - - return false; -} - static int set_ppgtt(struct drm_i915_file_private *file_priv, struct i915_gem_context *ctx, struct drm_i915_gem_context_param *args) { - struct i915_address_space *vm, *old; + struct i915_address_space *vm; int err; if (args->size) @@ -1318,22 +1149,7 @@ static int set_ppgtt(struct drm_i915_file_private *file_priv, /* Teardown the existing obj:vma cache, it will have to be rebuilt. */ lut_close(ctx); - old = __set_ppgtt(ctx, vm); - - /* - * We need to flush any requests using the current ppgtt before - * we release it as the requests do not hold a reference themselves, - * only indirectly through the context. - */ - err = context_barrier_task(ctx, ALL_ENGINES, - skip_ppgtt_update, - emit_ppgtt_update, - set_ppgtt_barrier, - old); - if (err) { - i915_vm_close(__set_ppgtt(ctx, old)); - i915_vm_close(old); - } + err = __set_ppgtt(ctx, vm); unlock: mutex_unlock(&ctx->mutex); diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c index f4f933240b39..228577a48c79 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c @@ -1865,131 +1865,6 @@ static int igt_vm_isolation(void *arg) return err; } -static bool skip_unused_engines(struct intel_context *ce, void *data) -{ - return !ce->state; -} - -static void mock_barrier_task(void *data) -{ - unsigned int *counter = data; - - ++*counter; -} - -static int mock_context_barrier(void *arg) -{ -#undef pr_fmt -#define pr_fmt(x) "context_barrier_task():" # x - struct drm_i915_private *i915 = arg; - struct i915_gem_context *ctx; - struct i915_request *rq; - unsigned int counter; - int err; - - /* - * The context barrier provides us with a callback after it emits - * a request; useful for retiring old state after loading new. - */ - - ctx = mock_context(i915, "mock"); - if (!ctx) - return -ENOMEM; - - counter = 0; - err = context_barrier_task(ctx, 0, - NULL, NULL, mock_barrier_task, &counter); - if (err) { - pr_err("Failed at line %d, err=%d\n", __LINE__, err); - goto out; - } - if (counter == 0) { - pr_err("Did not retire immediately with 0 engines\n"); - err = -EINVAL; - goto out; - } - - counter = 0; - err = context_barrier_task(ctx, ALL_ENGINES, - skip_unused_engines, - NULL, - mock_barrier_task, - &counter); - if (err) { - pr_err("Failed at line %d, err=%d\n", __LINE__, err); - goto out; - } - if (counter == 0) { - pr_err("Did not retire immediately for all unused engines\n"); - err = -EINVAL; - goto out; - } - - rq = igt_request_alloc(ctx, i915->gt.engine[RCS0]); - if (IS_ERR(rq)) { - pr_err("Request allocation failed!\n"); - goto out; - } - i915_request_add(rq); - - counter = 0; - context_barrier_inject_fault = BIT(RCS0); - err = context_barrier_task(ctx, ALL_ENGINES, - NULL, NULL, mock_barrier_task, &counter); - context_barrier_inject_fault = 0; - if (err == -ENXIO) - err = 0; - else - pr_err("Did not hit fault injection!\n"); - if (counter != 0) { - pr_err("Invoked callback on error!\n"); - err = -EIO; - } - if (err) - goto out; - - counter = 0; - err = context_barrier_task(ctx, ALL_ENGINES, - skip_unused_engines, - NULL, - mock_barrier_task, - &counter); - if (err) { - pr_err("Failed at line %d, err=%d\n", __LINE__, err); - goto out; - } - mock_device_flush(i915); - if (counter == 0) { - pr_err("Did not retire on each active engines\n"); - err = -EINVAL; - goto out; - } - -out: - mock_context_close(ctx); - return err; -#undef pr_fmt -#define pr_fmt(x) x -} - -int i915_gem_context_mock_selftests(void) -{ - static const struct i915_subtest tests[] = { - SUBTEST(mock_context_barrier), - }; - struct drm_i915_private *i915; - int err; - - i915 = mock_gem_device(); - if (!i915) - return -ENOMEM; - - err = i915_subtests(tests, i915); - - drm_dev_put(&i915->drm); - return err; -} - int i915_gem_context_live_selftests(struct drm_i915_private *i915) { static const struct i915_subtest tests[] = { diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h index 07be021882cc..6f4e6c75fbd5 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.h +++ b/drivers/gpu/drm/i915/gt/intel_context.h @@ -53,6 +53,19 @@ static inline int intel_context_lock_pinned(struct intel_context *ce) return mutex_lock_interruptible(&ce->pin_mutex); } +/** + * intel_context_lock_pinned_killable - Stablises the 'pinned' status + * @ce - the context + * + * See intel_context_lock_pinned() but will only be interrupted by process + * termination. + */ +static inline int intel_context_lock_pinned_killable(struct intel_context *ce) + __acquires(ce->pin_mutex) +{ + return mutex_lock_killable(&ce->pin_mutex); +} + /** * intel_context_is_pinned - Reports the 'pinned' status * @ce - the context diff --git a/drivers/gpu/drm/i915/gt/intel_context_param.c b/drivers/gpu/drm/i915/gt/intel_context_param.c index 65dcd090245d..c6cf5f929322 100644 --- a/drivers/gpu/drm/i915/gt/intel_context_param.c +++ b/drivers/gpu/drm/i915/gt/intel_context_param.c @@ -61,3 +61,36 @@ long intel_context_get_ring_size(struct intel_context *ce) return sz; } + +int intel_context_set_vm(struct intel_context *ce, + struct i915_address_space *vm) +{ + int err; + + if (vm == READ_ONCE(ce->vm)) + return 0; + + /* Recovering from a partial update over all engines is hard */ + if (intel_context_lock_pinned_killable(ce)) + return -EINTR; + + if (intel_context_is_pinned(ce)) { + err = -EBUSY; /* In active use, come back later! */ + goto unlock; + } + + /* Must wait until HW has finished saving the context image */ + err = __i915_active_wait(&ce->active, TASK_KILLABLE); + if (err < 0) + goto unlock; + + i915_vm_put(ce->vm); + ce->vm = i915_vm_get(vm); + + if (test_bit(CONTEXT_ALLOC_BIT, &ce->flags)) + ce->ops->reset(ce); + +unlock: + intel_context_unlock_pinned(ce); + return err; +} diff --git a/drivers/gpu/drm/i915/gt/intel_context_param.h b/drivers/gpu/drm/i915/gt/intel_context_param.h index f053d8633fe2..d09e59fcb891 100644 --- a/drivers/gpu/drm/i915/gt/intel_context_param.h +++ b/drivers/gpu/drm/i915/gt/intel_context_param.h @@ -6,9 +6,13 @@ #ifndef INTEL_CONTEXT_PARAM_H #define INTEL_CONTEXT_PARAM_H +struct i915_address_space; struct intel_context; int intel_context_set_ring_size(struct intel_context *ce, long sz); long intel_context_get_ring_size(struct intel_context *ce); +int intel_context_set_vm(struct intel_context *ce, + struct i915_address_space *vm); + #endif /* INTEL_CONTEXT_PARAM_H */ diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index 55bf3cdf3b38..0029d7ecc371 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -3174,7 +3174,7 @@ static int execlists_context_alloc(struct intel_context *ce) static void execlists_context_reset(struct intel_context *ce) { CE_TRACE(ce, "reset\n"); - GEM_BUG_ON(!intel_context_is_pinned(ce)); + GEM_BUG_ON(!test_bit(CONTEXT_ALLOC_BIT, &ce->flags)); intel_ring_reset(ce->ring, ce->ring->emit); diff --git a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h index 5b39bab4da1d..1251192fabe8 100644 --- a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h +++ b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h @@ -30,6 +30,5 @@ selftest(vma, i915_vma_mock_selftests) selftest(evict, i915_gem_evict_mock_selftests) selftest(gtt, i915_gem_gtt_mock_selftests) selftest(hugepages, i915_gem_huge_page_mock_selftests) -selftest(contexts, i915_gem_context_mock_selftests) selftest(buddy, i915_buddy_mock_selftests) selftest(memory_region, intel_memory_region_mock_selftests) From patchwork Tue Mar 31 21:31:04 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 11468405 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id E77CF159A for ; Tue, 31 Mar 2020 21:31:28 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D038E2080C for ; Tue, 31 Mar 2020 21:31:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D038E2080C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=chris-wilson.co.uk Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 201CB6E89A; Tue, 31 Mar 2020 21:31:26 +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 890D1899D6 for ; Tue, 31 Mar 2020 21:31:18 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from build.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 20757571-1500050 for multiple; Tue, 31 Mar 2020 22:31:11 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Tue, 31 Mar 2020 22:31:04 +0100 Message-Id: <20200331213108.11340-7-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200331213108.11340-1-chris@chris-wilson.co.uk> References: <20200331213108.11340-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 07/11] drm/i915/gem: Try allocating va from free space X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Chris Wilson Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" If the current node/entry location is occupied, and the object is not pinned, try assigning it some free space. We cannot wait here, so if in doubt, we unreserve and try to grab all at once. Signed-off-by: Chris Wilson Reviewed-by: Matthew Auld --- drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index bc8aa9604787..d8c746108d85 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -446,8 +446,17 @@ eb_pin_vma(struct i915_execbuffer *eb, if (unlikely(ev->flags & EXEC_OBJECT_NEEDS_GTT)) pin_flags |= PIN_GLOBAL; - if (unlikely(i915_vma_pin(vma, 0, 0, pin_flags))) - return false; + if (unlikely(i915_vma_pin(vma, 0, 0, pin_flags))) { + if (entry->flags & EXEC_OBJECT_PINNED) + return false; + + pin_flags &= ~PIN_OFFSET_FIXED; + if (unlikely(i915_vma_pin(vma, + entry->pad_to_size, + entry->alignment, + pin_flags))) + return false; + } if (unlikely(ev->flags & EXEC_OBJECT_NEEDS_FENCE)) { if (unlikely(i915_vma_pin_fence(vma))) { From patchwork Tue Mar 31 21:31:05 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 11468413 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id B855215AB for ; Tue, 31 Mar 2020 21:31:43 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A094C2080C for ; Tue, 31 Mar 2020 21:31:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A094C2080C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=chris-wilson.co.uk Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1CB016E892; Tue, 31 Mar 2020 21:31:43 +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 804E889958 for ; Tue, 31 Mar 2020 21:31:19 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from build.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 20757572-1500050 for multiple; Tue, 31 Mar 2020 22:31:11 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Tue, 31 Mar 2020 22:31:05 +0100 Message-Id: <20200331213108.11340-8-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200331213108.11340-1-chris@chris-wilson.co.uk> References: <20200331213108.11340-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 08/11] drm/i915/gt: Only wait for GPU activity before unbinding a GGTT fence X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Chris Wilson Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" Only GPU activity via the GGTT fence is asynchronous, we know that we control the CPU access directly, so we only need to wait for the GPU to stop using the fence before we relinquish it. Signed-off-by: Chris Wilson Reviewed-by: Matthew Auld --- drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 12 ++++++++---- drivers/gpu/drm/i915/gt/intel_ggtt_fencing.h | 3 +++ drivers/gpu/drm/i915/i915_vma.c | 4 ++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c index 225970f4a4ef..74f8201486b2 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c @@ -239,15 +239,18 @@ static int fence_update(struct i915_fence_reg *fence, if (!i915_vma_is_map_and_fenceable(vma)) return -EINVAL; - ret = i915_vma_sync(vma); - if (ret) - return ret; + if (INTEL_GEN(fence_to_i915(fence)) < 4) { + /* implicit 'unfenced' GPU blits */ + ret = i915_vma_sync(vma); + if (ret) + return ret; + } } old = xchg(&fence->vma, NULL); if (old) { /* XXX Ideally we would move the waiting to outside the mutex */ - ret = i915_vma_sync(old); + ret = i915_active_wait(&fence->active); if (ret) { fence->vma = old; return ret; @@ -869,6 +872,7 @@ void intel_ggtt_init_fences(struct i915_ggtt *ggtt) for (i = 0; i < num_fences; i++) { struct i915_fence_reg *fence = &ggtt->fence_regs[i]; + i915_active_init(&fence->active, NULL, NULL); fence->ggtt = ggtt; fence->id = i; list_add_tail(&fence->link, &ggtt->fence_list); diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.h b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.h index 9850f6a85d2a..08c6bb667581 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.h +++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.h @@ -28,6 +28,8 @@ #include #include +#include "i915_active.h" + struct drm_i915_gem_object; struct i915_ggtt; struct i915_vma; @@ -41,6 +43,7 @@ struct i915_fence_reg { struct i915_ggtt *ggtt; struct i915_vma *vma; atomic_t pin_count; + struct i915_active active; int id; /** * Whether the tiling parameters for the currently diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c index 18069df2a9e5..616ca5a7c875 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -1232,6 +1232,10 @@ int i915_vma_move_to_active(struct i915_vma *vma, dma_resv_add_shared_fence(vma->resv, &rq->fence); obj->write_domain = 0; } + + if (flags & EXEC_OBJECT_NEEDS_FENCE && vma->fence) + i915_active_add_request(&vma->fence->active, rq); + obj->read_domains |= I915_GEM_GPU_DOMAINS; obj->mm.dirty = true; From patchwork Tue Mar 31 21:31:06 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 11468397 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id D2F22159A for ; Tue, 31 Mar 2020 21:31:20 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id BBECD2080C for ; Tue, 31 Mar 2020 21:31:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BBECD2080C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=chris-wilson.co.uk Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D89DF899D6; Tue, 31 Mar 2020 21:31: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 864198991C for ; Tue, 31 Mar 2020 21:31:18 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from build.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 20757573-1500050 for multiple; Tue, 31 Mar 2020 22:31:11 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Tue, 31 Mar 2020 22:31:06 +0100 Message-Id: <20200331213108.11340-9-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200331213108.11340-1-chris@chris-wilson.co.uk> References: <20200331213108.11340-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 09/11] drm/i915/gt: Store the fence details on the fence X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Chris Wilson Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" Make a copy of the object tiling parameters at the point of grabbing the fence. Signed-off-by: Chris Wilson Reviewed-by: Matthew Auld --- drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 93 +++++++------------- drivers/gpu/drm/i915/gt/intel_ggtt_fencing.h | 4 + 2 files changed, 37 insertions(+), 60 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c index 74f8201486b2..d1478b56a42c 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c @@ -68,8 +68,7 @@ static struct intel_uncore *fence_to_uncore(struct i915_fence_reg *fence) return fence->ggtt->vm.gt->uncore; } -static void i965_write_fence_reg(struct i915_fence_reg *fence, - struct i915_vma *vma) +static void i965_write_fence_reg(struct i915_fence_reg *fence) { i915_reg_t fence_reg_lo, fence_reg_hi; int fence_pitch_shift; @@ -87,18 +86,16 @@ static void i965_write_fence_reg(struct i915_fence_reg *fence, } val = 0; - if (vma) { - unsigned int stride = i915_gem_object_get_stride(vma->obj); + if (fence->tiling) { + unsigned int stride = fence->stride; - GEM_BUG_ON(!i915_vma_is_map_and_fenceable(vma)); - GEM_BUG_ON(!IS_ALIGNED(vma->node.start, I965_FENCE_PAGE)); - GEM_BUG_ON(!IS_ALIGNED(vma->fence_size, I965_FENCE_PAGE)); GEM_BUG_ON(!IS_ALIGNED(stride, 128)); - val = (vma->node.start + vma->fence_size - I965_FENCE_PAGE) << 32; - val |= vma->node.start; + val = fence->start + fence->size - I965_FENCE_PAGE; + val <<= 32; + val |= fence->start; val |= (u64)((stride / 128) - 1) << fence_pitch_shift; - if (i915_gem_object_get_tiling(vma->obj) == I915_TILING_Y) + if (fence->tiling == I915_TILING_Y) val |= BIT(I965_FENCE_TILING_Y_SHIFT); val |= I965_FENCE_REG_VALID; } @@ -125,21 +122,15 @@ static void i965_write_fence_reg(struct i915_fence_reg *fence, } } -static void i915_write_fence_reg(struct i915_fence_reg *fence, - struct i915_vma *vma) +static void i915_write_fence_reg(struct i915_fence_reg *fence) { u32 val; val = 0; - if (vma) { - unsigned int tiling = i915_gem_object_get_tiling(vma->obj); + if (fence->tiling) { + unsigned int stride = fence->stride; + unsigned int tiling = fence->tiling; bool is_y_tiled = tiling == I915_TILING_Y; - unsigned int stride = i915_gem_object_get_stride(vma->obj); - - GEM_BUG_ON(!i915_vma_is_map_and_fenceable(vma)); - GEM_BUG_ON(vma->node.start & ~I915_FENCE_START_MASK); - GEM_BUG_ON(!is_power_of_2(vma->fence_size)); - GEM_BUG_ON(!IS_ALIGNED(vma->node.start, vma->fence_size)); if (is_y_tiled && HAS_128_BYTE_Y_TILING(fence_to_i915(fence))) stride /= 128; @@ -147,10 +138,10 @@ static void i915_write_fence_reg(struct i915_fence_reg *fence, stride /= 512; GEM_BUG_ON(!is_power_of_2(stride)); - val = vma->node.start; + val = fence->start; if (is_y_tiled) val |= BIT(I830_FENCE_TILING_Y_SHIFT); - val |= I915_FENCE_SIZE_BITS(vma->fence_size); + val |= I915_FENCE_SIZE_BITS(fence->size); val |= ilog2(stride) << I830_FENCE_PITCH_SHIFT; val |= I830_FENCE_REG_VALID; @@ -165,25 +156,18 @@ static void i915_write_fence_reg(struct i915_fence_reg *fence, } } -static void i830_write_fence_reg(struct i915_fence_reg *fence, - struct i915_vma *vma) +static void i830_write_fence_reg(struct i915_fence_reg *fence) { u32 val; val = 0; - if (vma) { - unsigned int stride = i915_gem_object_get_stride(vma->obj); + if (fence->tiling) { + unsigned int stride = fence->stride; - GEM_BUG_ON(!i915_vma_is_map_and_fenceable(vma)); - GEM_BUG_ON(vma->node.start & ~I830_FENCE_START_MASK); - GEM_BUG_ON(!is_power_of_2(vma->fence_size)); - GEM_BUG_ON(!is_power_of_2(stride / 128)); - GEM_BUG_ON(!IS_ALIGNED(vma->node.start, vma->fence_size)); - - val = vma->node.start; - if (i915_gem_object_get_tiling(vma->obj) == I915_TILING_Y) + val = fence->start; + if (fence->tiling == I915_TILING_Y) val |= BIT(I830_FENCE_TILING_Y_SHIFT); - val |= I830_FENCE_SIZE_BITS(vma->fence_size); + val |= I830_FENCE_SIZE_BITS(fence->size); val |= ilog2(stride / 128) << I830_FENCE_PITCH_SHIFT; val |= I830_FENCE_REG_VALID; } @@ -197,8 +181,7 @@ static void i830_write_fence_reg(struct i915_fence_reg *fence, } } -static void fence_write(struct i915_fence_reg *fence, - struct i915_vma *vma) +static void fence_write(struct i915_fence_reg *fence) { struct drm_i915_private *i915 = fence_to_i915(fence); @@ -209,18 +192,16 @@ static void fence_write(struct i915_fence_reg *fence, */ if (IS_GEN(i915, 2)) - i830_write_fence_reg(fence, vma); + i830_write_fence_reg(fence); else if (IS_GEN(i915, 3)) - i915_write_fence_reg(fence, vma); + i915_write_fence_reg(fence); else - i965_write_fence_reg(fence, vma); + i965_write_fence_reg(fence); /* * Access through the fenced region afterwards is * ordered by the posting reads whilst writing the registers. */ - - fence->dirty = false; } static int fence_update(struct i915_fence_reg *fence, @@ -232,6 +213,7 @@ static int fence_update(struct i915_fence_reg *fence, struct i915_vma *old; int ret; + fence->tiling = 0; if (vma) { GEM_BUG_ON(!i915_gem_object_get_stride(vma->obj) || !i915_gem_object_get_tiling(vma->obj)); @@ -245,7 +227,13 @@ static int fence_update(struct i915_fence_reg *fence, if (ret) return ret; } + + fence->start = vma->node.start; + fence->size = vma->fence_size; + fence->stride = i915_gem_object_get_stride(vma->obj); + fence->tiling = i915_gem_object_get_tiling(vma->obj); } + WRITE_ONCE(fence->dirty, false); old = xchg(&fence->vma, NULL); if (old) { @@ -288,7 +276,7 @@ static int fence_update(struct i915_fence_reg *fence, } WRITE_ONCE(fence->vma, vma); - fence_write(fence, vma); + fence_write(fence); if (vma) { vma->fence = fence; @@ -496,23 +484,8 @@ void intel_ggtt_restore_fences(struct i915_ggtt *ggtt) { int i; - rcu_read_lock(); /* keep obj alive as we dereference */ - for (i = 0; i < ggtt->num_fences; i++) { - struct i915_fence_reg *reg = &ggtt->fence_regs[i]; - struct i915_vma *vma = READ_ONCE(reg->vma); - - GEM_BUG_ON(vma && vma->fence != reg); - - /* - * Commit delayed tiling changes if we have an object still - * attached to the fence, otherwise just clear the fence. - */ - if (vma && !i915_gem_object_is_tiled(vma->obj)) - vma = NULL; - - fence_write(reg, vma); - } - rcu_read_unlock(); + for (i = 0; i < ggtt->num_fences; i++) + fence_write(&ggtt->fence_regs[i]); } /** diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.h b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.h index 08c6bb667581..9eef679e1311 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.h +++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.h @@ -54,6 +54,10 @@ struct i915_fence_reg { * command (such as BLT on gen2/3), as a "fence". */ bool dirty; + u32 start; + u32 size; + u32 tiling; + u32 stride; }; struct i915_fence_reg *i915_reserve_fence(struct i915_ggtt *ggtt); From patchwork Tue Mar 31 21:31:07 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 11468399 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1A6A8159A for ; Tue, 31 Mar 2020 21:31:26 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 034B320838 for ; Tue, 31 Mar 2020 21:31:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 034B320838 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=chris-wilson.co.uk Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A2FD16E2EC; Tue, 31 Mar 2020 21:31:25 +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 86FE489958 for ; Tue, 31 Mar 2020 21:31:18 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from build.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 20757574-1500050 for multiple; Tue, 31 Mar 2020 22:31:11 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Tue, 31 Mar 2020 22:31:07 +0100 Message-Id: <20200331213108.11340-10-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200331213108.11340-1-chris@chris-wilson.co.uk> References: <20200331213108.11340-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 10/11] drm/i915/gt: Make fence revocation unequivocal X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Chris Wilson Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" If we must revoke the fence because the VMA is no longer present, or because the fence no longer applies, ensure that we do and convert it into an error if we try but cannot. Signed-off-by: Chris Wilson Reviewed-by: Matthew Auld --- drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 21 +++++++++++--------- drivers/gpu/drm/i915/i915_gem.c | 12 +++++------ drivers/gpu/drm/i915/i915_vma.c | 4 +--- drivers/gpu/drm/i915/i915_vma.h | 2 +- 4 files changed, 19 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c index d1478b56a42c..2a008a4237a5 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c @@ -293,23 +293,26 @@ static int fence_update(struct i915_fence_reg *fence, * * This function force-removes any fence from the given object, which is useful * if the kernel wants to do untiled GTT access. - * - * Returns: - * - * 0 on success, negative error code on failure. */ -int i915_vma_revoke_fence(struct i915_vma *vma) +void i915_vma_revoke_fence(struct i915_vma *vma) { struct i915_fence_reg *fence = vma->fence; + intel_wakeref_t wakeref; lockdep_assert_held(&vma->vm->mutex); if (!fence) - return 0; + return; - if (atomic_read(&fence->pin_count)) - return -EBUSY; + GEM_BUG_ON(fence->vma != vma); + GEM_BUG_ON(!i915_active_is_idle(&fence->active)); + GEM_BUG_ON(atomic_read(&fence->pin_count)); + + fence->tiling = 0; + WRITE_ONCE(fence->vma, NULL); + vma->fence = NULL; - return fence_update(fence, NULL); + with_intel_runtime_pm_if_in_use(fence_to_uncore(fence)->rpm, wakeref) + fence_write(fence); } static struct i915_fence_reg *fence_find(struct i915_ggtt *ggtt) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 762b50b08d73..b0836fc47ae6 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -993,18 +993,16 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj, return ERR_PTR(ret); } + ret = i915_vma_pin(vma, size, alignment, flags | PIN_GLOBAL); + if (ret) + return ERR_PTR(ret); + if (vma->fence && !i915_gem_object_is_tiled(obj)) { mutex_lock(&ggtt->vm.mutex); - ret = i915_vma_revoke_fence(vma); + i915_vma_revoke_fence(vma); mutex_unlock(&ggtt->vm.mutex); - if (ret) - return ERR_PTR(ret); } - ret = i915_vma_pin(vma, size, alignment, flags | PIN_GLOBAL); - if (ret) - return ERR_PTR(ret); - ret = i915_vma_wait_for_bind(vma); if (ret) { i915_vma_unpin(vma); diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c index 616ca5a7c875..b5f78b0acf5d 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -1298,9 +1298,7 @@ int __i915_vma_unbind(struct i915_vma *vma) i915_vma_flush_writes(vma); /* release the fence reg _after_ flushing */ - ret = i915_vma_revoke_fence(vma); - if (ret) - return ret; + i915_vma_revoke_fence(vma); /* Force a pagefault for domain tracking on next user access */ i915_vma_revoke_mmap(vma); diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h index b958ad07f212..8ad1daabcd58 100644 --- a/drivers/gpu/drm/i915/i915_vma.h +++ b/drivers/gpu/drm/i915/i915_vma.h @@ -326,7 +326,7 @@ static inline struct page *i915_vma_first_page(struct i915_vma *vma) * True if the vma has a fence, false otherwise. */ int __must_check i915_vma_pin_fence(struct i915_vma *vma); -int __must_check i915_vma_revoke_fence(struct i915_vma *vma); +void i915_vma_revoke_fence(struct i915_vma *vma); int __i915_vma_pin_fence(struct i915_vma *vma); From patchwork Tue Mar 31 21:31:08 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 11468415 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 65C8E14B4 for ; Tue, 31 Mar 2020 21:31:49 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4E27D20838 for ; Tue, 31 Mar 2020 21:31:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4E27D20838 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=chris-wilson.co.uk Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id CAD1D6E05D; Tue, 31 Mar 2020 21:31:48 +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 663408991C for ; Tue, 31 Mar 2020 21:31:19 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from build.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 20757575-1500050 for multiple; Tue, 31 Mar 2020 22:31:11 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Tue, 31 Mar 2020 22:31:08 +0100 Message-Id: <20200331213108.11340-11-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200331213108.11340-1-chris@chris-wilson.co.uk> References: <20200331213108.11340-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 11/11] drm/i915/gem: Drop cached obj->bind_count X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Chris Wilson Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" We cached the number of vma bound to the object in order to speed up shrinker decisions. This has been superseded by being more proactive in removing objects we cannot shrink from the shrinker lists, and so we can drop the clumsy attempt at atomically counting the bind count and comparing it to the number of pinned mappings of the object. This will only get more clumsier with asynchronous binding and unbinding. Signed-off-by: Chris Wilson Reviewed-by: Matthew Auld --- drivers/gpu/drm/i915/gem/i915_gem_domain.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_object.c | 1 - .../gpu/drm/i915/gem/i915_gem_object_types.h | 3 --- drivers/gpu/drm/i915/gem/i915_gem_pages.c | 2 -- drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 18 ++----------- .../drm/i915/gem/selftests/i915_gem_mman.c | 4 --- drivers/gpu/drm/i915/i915_debugfs.c | 7 ++--- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 7 ++++- drivers/gpu/drm/i915/i915_vma.c | 24 ----------------- .../gpu/drm/i915/selftests/i915_gem_evict.c | 26 +------------------ drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 1 - 12 files changed, 13 insertions(+), 83 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c index 0cc40e77bbd2..af43e82f45c7 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c @@ -369,7 +369,7 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj) struct i915_vma *vma; GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj)); - if (!atomic_read(&obj->bind_count)) + if (list_empty(&obj->vma.list)) return; mutex_lock(&i915->ggtt.vm.mutex); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c index 5da9f9e534b9..3f01cdd1a39b 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -206,7 +206,6 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915, } obj->mmo.offsets = RB_ROOT; - GEM_BUG_ON(atomic_read(&obj->bind_count)); GEM_BUG_ON(obj->userfault_count); GEM_BUG_ON(!list_empty(&obj->lut_list)); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h index a0b10bcd8d8a..54ee658bb168 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h @@ -179,9 +179,6 @@ struct drm_i915_gem_object { #define TILING_MASK (FENCE_MINIMUM_STRIDE - 1) #define STRIDE_MASK (~TILING_MASK) - /** Count of VMA actually bound by this object */ - atomic_t bind_count; - struct { /* * Protects the pages and their use. Do not use directly, but diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c index 24f4cadea114..5d855fcd5c0f 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c @@ -199,8 +199,6 @@ int __i915_gem_object_put_pages(struct drm_i915_gem_object *obj) if (i915_gem_object_has_pinned_pages(obj)) return -EBUSY; - GEM_BUG_ON(atomic_read(&obj->bind_count)); - /* May be called by shrinker from within get_pages() (on another bo) */ mutex_lock(&obj->mm.lock); if (unlikely(atomic_read(&obj->mm.pages_pin_count))) { diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c index 03e5eb4c99d1..5b65ce738b16 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c @@ -26,18 +26,6 @@ static bool can_release_pages(struct drm_i915_gem_object *obj) if (!i915_gem_object_is_shrinkable(obj)) return false; - /* - * Only report true if by unbinding the object and putting its pages - * we can actually make forward progress towards freeing physical - * pages. - * - * If the pages are pinned for any other reason than being bound - * to the GPU, simply unbinding from the GPU is not going to succeed - * in releasing our pin count on the pages themselves. - */ - if (atomic_read(&obj->mm.pages_pin_count) > atomic_read(&obj->bind_count)) - return false; - /* * We can only return physical pages to the system if we can either * discard the contents (because the user has marked them as being @@ -54,6 +42,8 @@ static bool unsafe_drop_pages(struct drm_i915_gem_object *obj, flags = 0; if (shrink & I915_SHRINK_ACTIVE) flags = I915_GEM_OBJECT_UNBIND_ACTIVE; + if (!(shrink & I915_SHRINK_BOUND)) + flags = I915_GEM_OBJECT_UNBIND_TEST; if (i915_gem_object_unbind(obj, flags) == 0) __i915_gem_object_put_pages(obj); @@ -194,10 +184,6 @@ i915_gem_shrink(struct drm_i915_private *i915, i915_gem_object_is_framebuffer(obj)) continue; - if (!(shrink & I915_SHRINK_BOUND) && - atomic_read(&obj->bind_count)) - continue; - if (!can_release_pages(obj)) continue; diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c index 43912e9b683d..ef7abcb3f4ee 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c @@ -1156,9 +1156,6 @@ static int __igt_mmap_revoke(struct drm_i915_private *i915, if (err) goto out_unmap; - GEM_BUG_ON(mmo->mmap_type == I915_MMAP_TYPE_GTT && - !atomic_read(&obj->bind_count)); - err = check_present(addr, obj->base.size); if (err) { pr_err("%s: was not present\n", obj->mm.region->name); @@ -1175,7 +1172,6 @@ static int __igt_mmap_revoke(struct drm_i915_private *i915, pr_err("Failed to unbind object!\n"); goto out_unmap; } - GEM_BUG_ON(atomic_read(&obj->bind_count)); if (type != I915_MMAP_TYPE_GTT) { __i915_gem_object_put_pages(obj); diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 4c8a88c64c1d..b22b4e9c3138 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -217,7 +217,7 @@ i915_debugfs_describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj) struct file_stats { struct i915_address_space *vm; unsigned long count; - u64 total, unbound; + u64 total; u64 active, inactive; u64 closed; }; @@ -233,8 +233,6 @@ static int per_file_stats(int id, void *ptr, void *data) stats->count++; stats->total += obj->base.size; - if (!atomic_read(&obj->bind_count)) - stats->unbound += obj->base.size; spin_lock(&obj->vma.lock); if (!stats->vm) { @@ -284,13 +282,12 @@ static int per_file_stats(int id, void *ptr, void *data) #define print_file_stats(m, name, stats) do { \ if (stats.count) \ - seq_printf(m, "%s: %lu objects, %llu bytes (%llu active, %llu inactive, %llu unbound, %llu closed)\n", \ + seq_printf(m, "%s: %lu objects, %llu bytes (%llu active, %llu inactive, %llu closed)\n", \ name, \ stats.count, \ stats.total, \ stats.active, \ stats.inactive, \ - stats.unbound, \ stats.closed); \ } while (0) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 2ee2a2b301ef..f490e385a583 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1739,6 +1739,7 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj, unsigned long flags); #define I915_GEM_OBJECT_UNBIND_ACTIVE BIT(0) #define I915_GEM_OBJECT_UNBIND_BARRIER BIT(1) +#define I915_GEM_OBJECT_UNBIND_TEST BIT(2) void i915_gem_runtime_suspend(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index b0836fc47ae6..0cbcb9f54e7d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -118,7 +118,7 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj, struct i915_vma *vma; int ret; - if (!atomic_read(&obj->bind_count)) + if (list_empty(&obj->vma.list)) return 0; /* @@ -141,6 +141,11 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj, if (!i915_vma_is_bound(vma, I915_VMA_BIND_MASK)) continue; + if (flags & I915_GEM_OBJECT_UNBIND_TEST) { + ret = -EBUSY; + break; + } + ret = -EAGAIN; if (!i915_vm_tryopen(vm)) break; diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c index b5f78b0acf5d..4cdd883f9d66 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -608,18 +608,6 @@ bool i915_gem_valid_gtt_space(struct i915_vma *vma, unsigned long color) return true; } -static void assert_bind_count(const struct drm_i915_gem_object *obj) -{ - /* - * Combine the assertion that the object is bound and that we have - * pinned its pages. But we should never have bound the object - * more than we have pinned its pages. (For complete accuracy, we - * assume that no else is pinning the pages, but as a rough assertion - * that we will not run into problems later, this will do!) - */ - GEM_BUG_ON(atomic_read(&obj->mm.pages_pin_count) < atomic_read(&obj->bind_count)); -} - /** * i915_vma_insert - finds a slot for the vma in its address space * @vma: the vma @@ -738,12 +726,6 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags) GEM_BUG_ON(!drm_mm_node_allocated(&vma->node)); GEM_BUG_ON(!i915_gem_valid_gtt_space(vma, color)); - if (vma->obj) { - struct drm_i915_gem_object *obj = vma->obj; - - atomic_inc(&obj->bind_count); - assert_bind_count(obj); - } list_add_tail(&vma->vm_link, &vma->vm->bound_list); return 0; @@ -761,12 +743,6 @@ i915_vma_detach(struct i915_vma *vma) * it to be reaped by the shrinker. */ list_del(&vma->vm_link); - if (vma->obj) { - struct drm_i915_gem_object *obj = vma->obj; - - assert_bind_count(obj); - atomic_dec(&obj->bind_count); - } } static bool try_qad_pin(struct i915_vma *vma, unsigned int flags) diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c index 06ef88510209..028baae9631f 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c @@ -45,8 +45,8 @@ static void quirk_add(struct drm_i915_gem_object *obj, static int populate_ggtt(struct i915_ggtt *ggtt, struct list_head *objects) { - unsigned long unbound, bound, count; struct drm_i915_gem_object *obj; + unsigned long count; count = 0; do { @@ -72,30 +72,6 @@ static int populate_ggtt(struct i915_ggtt *ggtt, struct list_head *objects) pr_debug("Filled GGTT with %lu pages [%llu total]\n", count, ggtt->vm.total / PAGE_SIZE); - bound = 0; - unbound = 0; - list_for_each_entry(obj, objects, st_link) { - GEM_BUG_ON(!obj->mm.quirked); - - if (atomic_read(&obj->bind_count)) - bound++; - else - unbound++; - } - GEM_BUG_ON(bound + unbound != count); - - if (unbound) { - pr_err("%s: Found %lu objects unbound, expected %u!\n", - __func__, unbound, 0); - return -EINVAL; - } - - if (bound != count) { - pr_err("%s: Found %lu objects bound, expected %lu!\n", - __func__, bound, count); - return -EINVAL; - } - if (list_empty(&ggtt->vm.bound_list)) { pr_err("No objects on the GGTT inactive list!\n"); return -EINVAL; diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c index b342bef5e7c9..5d2a02fcf595 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c @@ -1229,7 +1229,6 @@ static void track_vma_bind(struct i915_vma *vma) { struct drm_i915_gem_object *obj = vma->obj; - atomic_inc(&obj->bind_count); /* track for eviction later */ __i915_gem_object_pin_pages(obj); GEM_BUG_ON(vma->pages);