From patchwork Thu Dec 5 22:23:49 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 11275447 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 3AAA66C1 for ; Thu, 5 Dec 2019 22:23:57 +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 22DBE21835 for ; Thu, 5 Dec 2019 22:23:57 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 22DBE21835 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 7AEAB6E9AB; Thu, 5 Dec 2019 22:23:56 +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 378F86E9AB for ; Thu, 5 Dec 2019 22:23:55 +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 19477124-1500050 for multiple; Thu, 05 Dec 2019 22:23:51 +0000 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Thu, 5 Dec 2019 22:23:49 +0000 Message-Id: <20191205222349.903322-1-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191205222034.896901-1-chris@chris-wilson.co.uk> References: <20191205222034.896901-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH] drm/i915/gem: Pin gen6_ppgtt prior to constructing the request X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" All pinning must be done prior to i915_request_create, to avoid timeline->mutex inversions. Here we slightly abuse the context_barrier_task stages to utilise the 'skip' decision as an opportunity to acquire the pin on the new ppgtt. Consider it s/skip/prepare/. At the moment, we only have on user of context_barrier_task, so it might be worth breaking it down for the specific task of set-vm and refactor it later if we find a second purpose. <4> [402.377487] WARNING: possible circular locking dependency detected <4> [402.377493] 5.4.0-rc8-CI-CI_DRM_7491+ #1 Tainted: G U <4> [402.377497] ------------------------------------------------------ <4> [402.377502] gem_exec_parall/2506 is trying to acquire lock: <4> [402.377507] ffff888403cdac70 (&kernel#2){+.+.}, at: i915_request_create+0x16/0x1c0 [i915] <4> [402.377593] but task is already holding lock: <4> [402.377597] ffff88835efad550 (&ppgtt->pin_mutex){+.+.}, at: gen6_ppgtt_pin+0x4d/0x110 [i915] <4> [402.377660] which lock already depends on the new lock. <4> [402.377664] the existing dependency chain (in reverse order) is: <4> [402.377668] -> #1 (&ppgtt->pin_mutex){+.+.}: <4> [402.377674] __mutex_lock+0x9a/0x9d0 <4> [402.377713] gen6_ppgtt_pin+0x4d/0x110 [i915] <4> [402.377756] emit_ppgtt_update+0x1dc/0x370 [i915] <4> [402.377801] context_barrier_task+0x176/0x310 [i915] <4> [402.377844] ctx_setparam+0x400/0xb10 [i915] <4> [402.377886] i915_gem_context_setparam_ioctl+0xc8/0x160 [i915] <4> [402.377891] drm_ioctl_kernel+0xa7/0xf0 <4> [402.377895] drm_ioctl+0x2e1/0x390 <4> [402.377899] do_vfs_ioctl+0xa0/0x6f0 <4> [402.377903] ksys_ioctl+0x35/0x60 <4> [402.377906] __x64_sys_ioctl+0x11/0x20 <4> [402.377910] do_syscall_64+0x4f/0x210 <4> [402.377914] entry_SYSCALL_64_after_hwframe+0x49/0xbe <4> [402.377917] -> #0 (&kernel#2){+.+.}: <4> [402.377923] __lock_acquire+0x1328/0x15d0 <4> [402.377926] lock_acquire+0xa7/0x1c0 <4> [402.377930] __mutex_lock+0x9a/0x9d0 <4> [402.377977] i915_request_create+0x16/0x1c0 [i915] <4> [402.378013] intel_engine_flush_barriers+0x4c/0x100 [i915] <4> [402.378062] i915_ggtt_pin+0x7d/0x130 [i915] <4> [402.378108] gen6_ppgtt_pin+0x9c/0x110 [i915] <4> [402.378148] ring_context_pin+0x2e/0xc0 [i915] <4> [402.378183] __intel_context_do_pin+0x6b/0x190 [i915] <4> [402.378226] i915_gem_do_execbuffer+0x180c/0x26b0 [i915] <4> [402.378268] i915_gem_execbuffer2_ioctl+0x11b/0x460 [i915] <4> [402.378272] drm_ioctl_kernel+0xa7/0xf0 <4> [402.378275] drm_ioctl+0x2e1/0x390 <4> [402.378279] do_vfs_ioctl+0xa0/0x6f0 <4> [402.378282] ksys_ioctl+0x35/0x60 <4> [402.378286] __x64_sys_ioctl+0x11/0x20 <4> [402.378289] do_syscall_64+0x4f/0x210 <4> [402.378292] entry_SYSCALL_64_after_hwframe+0x49/0xbe <4> [402.378295] other info that might help us debug this: <4> [402.378299] Possible unsafe locking scenario: <4> [402.378302] CPU0 CPU1 <4> [402.378305] ---- ---- <4> [402.378307] lock(&ppgtt->pin_mutex); <4> [402.378310] lock(&kernel#2); <4> [402.378314] lock(&ppgtt->pin_mutex); <4> [402.378317] lock(&kernel#2); <4> [402.378320] Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index 9f1dc96b10a6..9d8d75765ee4 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -1141,8 +1141,6 @@ static int emit_ppgtt_update(struct i915_request *rq, void *data) *cs++ = MI_NOOP; intel_ring_advance(rq, cs); } else { - /* ppGTT is not part of the legacy context image */ - gen6_ppgtt_pin(i915_vm_to_ppgtt(vm)); } return 0; @@ -1150,10 +1148,20 @@ static int emit_ppgtt_update(struct i915_request *rq, void *data) 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 !ce->state; - else - return !atomic_read(&ce->pin_count); + 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,