From patchwork Tue Dec 23 17:16:16 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michel Thierry X-Patchwork-Id: 5534481 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 4E11ABEEBA for ; Tue, 23 Dec 2014 17:16:32 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 685FA20121 for ; Tue, 23 Dec 2014 17:16:31 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id A0E0A201C0 for ; Tue, 23 Dec 2014 17:16:28 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 79D3E6E4F1; Tue, 23 Dec 2014 09:16:26 -0800 (PST) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTP id 9354E6E4E5 for ; Tue, 23 Dec 2014 09:16:23 -0800 (PST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP; 23 Dec 2014 09:13:47 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,633,1413270000"; d="scan'208";a="659125989" Received: from michelth-linux.isw.intel.com ([10.102.226.150]) by orsmga002.jf.intel.com with ESMTP; 23 Dec 2014 09:16:22 -0800 From: Michel Thierry To: intel-gfx@lists.freedesktop.org Date: Tue, 23 Dec 2014 17:16:16 +0000 Message-Id: <1419354987-4622-14-git-send-email-michel.thierry@intel.com> X-Mailer: git-send-email 2.1.1 In-Reply-To: <1419354987-4622-1-git-send-email-michel.thierry@intel.com> References: <1418922621-25818-1-git-send-email-michel.thierry@intel.com> <1419354987-4622-1-git-send-email-michel.thierry@intel.com> Subject: [Intel-gfx] [PATCH v2 13/24] drm/i915: Initialize all contexts X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 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-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Ben Widawsky The problem is we're going to switch to a new context, which could be the default context. The plan was to use restore inhibit, which would be fine, except if we are using dynamic page tables (which we will). If we use dynamic page tables and we don't load new page tables, the previous page tables might go away, and future operations will fault. CTXA runs. switch to default, restore inhibit CTXA dies and has its address space taken away. Run CTXB, tries to save using the context A's address space - this fails. The general solution is to make sure every context has it's own state, and its own address space. For cases when we must restore inhibit, first thing we do is load a valid address space. I thought this would be enough, but apparently there are references within the context itself which will refer to the old address space - therefore, we also must reinitialize. It was tricky to track this down as we don't have much insight into what happens in a context save. This is required for the next patch which enables dynamic page tables. v2: to->ppgtt is only valid in full ppgtt. Signed-off-by: Ben Widawsky Signed-off-by: Michel Thierry (v2) --- drivers/gpu/drm/i915/i915_gem_context.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index fa9d4a1..b1f3d50 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -592,13 +592,6 @@ needs_pd_load_pre(struct intel_engine_cs *ring, struct intel_context *to) (ring != &dev_priv->ring[RCS])) && to->ppgtt; } -static bool -needs_pd_load_post(struct intel_engine_cs *ring, struct intel_context *to) -{ - return IS_GEN8(ring->dev) && - (to->ppgtt || &to->ppgtt->base.pd_reload_mask); -} - static int do_switch(struct intel_engine_cs *ring, struct intel_context *to) { @@ -679,20 +672,24 @@ static int do_switch(struct intel_engine_cs *ring, /* GEN8 does *not* require an explicit reload if the PDPs have been * setup, and we do not wish to move them. - * - * XXX: If we implemented page directory eviction code, this - * optimization needs to be removed. */ - if (!to->legacy_hw_ctx.initialized || i915_gem_context_is_default(to)) + if (!to->legacy_hw_ctx.initialized) { hw_flags |= MI_RESTORE_INHIBIT; - else if (to->ppgtt && test_and_clear_bit(ring->id, &to->ppgtt->base.pd_reload_mask)) + /* NB: If we inhibit the restore, the context is not allowed to + * die because future work may end up depending on valid address + * space. This means we must enforce that a page table load + * occur when this occurs. */ + } else if (to->ppgtt && test_and_clear_bit(ring->id, &to->ppgtt->base.pd_reload_mask)) hw_flags |= MI_FORCE_RESTORE; ret = mi_set_context(ring, to, hw_flags); if (ret) goto unpin_out; - if (needs_pd_load_post(ring, to)) { + if (IS_GEN8(ring->dev) && to->ppgtt && (hw_flags & MI_RESTORE_INHIBIT)) { + /* We have a valid page directory (scratch) to switch to. This + * allows the old VM to be freed. Note that if anything occurs + * between the set context, and here, we are f*cked */ ret = to->ppgtt->switch_mm(to->ppgtt, ring); /* The hardware context switch is emitted, but we haven't * actually changed the state - so it's probably safe to bail @@ -742,7 +739,7 @@ static int do_switch(struct intel_engine_cs *ring, i915_gem_context_unreference(from); } - uninitialized = !to->legacy_hw_ctx.initialized && from == NULL; + uninitialized = !to->legacy_hw_ctx.initialized; to->legacy_hw_ctx.initialized = true; done: