From patchwork Thu Oct 15 10:47:04 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 7404811 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.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id BB7BDBEEA4 for ; Thu, 15 Oct 2015 10:49:33 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B62CD20852 for ; Thu, 15 Oct 2015 10:49:32 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 8CE9520851 for ; Thu, 15 Oct 2015 10:49:31 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D11A76E1D6; Thu, 15 Oct 2015 03:49:29 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from fireflyinternet.com (mail.fireflyinternet.com [87.106.93.118]) by gabe.freedesktop.org (Postfix) with ESMTP id 7B7026E1D6 for ; Thu, 15 Oct 2015 03:49:28 -0700 (PDT) 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 46638675-1500048 for multiple; Thu, 15 Oct 2015 11:47:23 +0100 Received: by haswell.alporthouse.com (sSMTP sendmail emulation); Thu, 15 Oct 2015 11:47:05 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Thu, 15 Oct 2015 11:47:04 +0100 Message-Id: <1444906024-27778-1-git-send-email-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.6.1 In-Reply-To: <20151015100534.GV26718@phenom.ffwll.local> References: <20151015100534.GV26718@phenom.ffwll.local> X-Originating-IP: 78.156.65.138 X-Country: code=GB country="United Kingdom" ip=78.156.65.138 Cc: Daniel Vetter Subject: [Intel-gfx] [PATCH] drm/i915: Split out aliasing-ppgtt from ggtt_bind_vma() 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 The presence of the aliasing-ppgtt (or rather its absence) causes some confusion on old generations where we still pretend that there is a difference between PIN_USER and PIN_GLOBAL even though we only have the global GTT. The confusion has resulted in a bug where we do not mark the vma as suitably bound for both types of access and so end up processing the binding twice. The double bind was accidentally introduced in commit 75d04a3773ecee617847de963ae4195d6aa74c28 Author: Mika Kuoppala Date: Tue Apr 28 17:56:17 2015 +0300 drm/i915/gtt: Allocate va range only if vma is not bound While at it implement an improved version suggested by Chris which avoids the double-bind irrespective of what type of bind is done first. Note that this exact bug was already addressed in commit d0e30adc42d979e4adc36b6c112b57337423b70c Author: Chris Wilson Date: Wed Jul 29 20:02:48 2015 +0100 drm/i915: Mark PIN_USER binding as GLOBAL_BIND without the aliasing ppgtt but the problem is still that originally in commit 0875546c5318c85c13d07014af5350e9000bc9e9 Author: Daniel Vetter Date: Mon Apr 20 09:04:05 2015 -0700 drm/i915: Fix up the vma aliasing ppgtt binding if forgotten to take into account there case where we have a GLOBAL_BIND before a LOCAL_BIND. Here we separate out the binding into the global GTT for machines where we have the aliasing-ppgtt and where we do not, so that we can perform the fixup without further confusion. Signed-off-by: Chris Wilson Cc: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_gtt.c | 62 +++++++++++++++++++++++++++---------- 1 file changed, 45 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 0796aa06e9f5..81f4628ae0e6 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -2493,7 +2493,6 @@ static int ggtt_bind_vma(struct i915_vma *vma, enum i915_cache_level cache_level, u32 flags) { - struct drm_i915_private *dev_priv = to_i915(vma->vm->dev); u32 pte_flags; int ret; @@ -2506,26 +2505,49 @@ static int ggtt_bind_vma(struct i915_vma *vma, if (vma->obj->gt_ro) pte_flags |= PTE_READ_ONLY; - if (!dev_priv->mm.aliasing_ppgtt || flags & GLOBAL_BIND) { + vma->vm->insert_entries(vma->vm, + vma->ggtt_view.pages, + vma->node.start, + cache_level, pte_flags); + + /* Note the inconsistency here is due to absence of the + * aliasing ppgtt on gen5 and earlier. Though we always + * request PIN_USER for execbuffer (translated to LOCAL_BIND), + * without the appgtt, we cannot honour that request and so + * must substitute it with a global binding. Since we do this + * behind the upper layers back, we need to explicitly set + * the bound flag ourselves. + */ + vma->bound |= GLOBAL_BIND | LOCAL_BIND; + return 0; +} + +static int agtt_bind_vma(struct i915_vma *vma, + enum i915_cache_level cache_level, + u32 flags) +{ + u32 pte_flags; + int ret; + + ret = i915_get_ggtt_vma_pages(vma); + if (ret) + return ret; + + /* Currently applicable only to VLV */ + pte_flags = 0; + if (vma->obj->gt_ro) + pte_flags |= PTE_READ_ONLY; + + if (flags & GLOBAL_BIND) { vma->vm->insert_entries(vma->vm, vma->ggtt_view.pages, vma->node.start, cache_level, pte_flags); - - /* Note the inconsistency here is due to absence of the - * aliasing ppgtt on gen4 and earlier. Though we always - * request PIN_USER for execbuffer (translated to LOCAL_BIND), - * without the appgtt, we cannot honour that request and so - * must substitute it with a global binding. Since we do this - * behind the upper layers back, we need to explicitly set - * the bound flag ourselves. - */ - vma->bound |= GLOBAL_BIND; - } - if (dev_priv->mm.aliasing_ppgtt && flags & LOCAL_BIND) { - struct i915_hw_ppgtt *appgtt = dev_priv->mm.aliasing_ppgtt; + if (flags & LOCAL_BIND) { + struct i915_hw_ppgtt *appgtt = + to_i915(vma->vm->dev)->mm.aliasing_ppgtt; appgtt->base.insert_entries(&appgtt->base, vma->ggtt_view.pages, vma->node.start, @@ -2959,7 +2981,10 @@ static int gen8_gmch_probe(struct drm_device *dev, dev_priv->gtt.base.clear_range = gen8_ggtt_clear_range; dev_priv->gtt.base.insert_page = gen8_ggtt_insert_page; dev_priv->gtt.base.insert_entries = gen8_ggtt_insert_entries; - dev_priv->gtt.base.bind_vma = ggtt_bind_vma; + if (dev_priv->mm.aliasing_ppgtt) + dev_priv->gtt.base.bind_vma = agtt_bind_vma; + else + dev_priv->gtt.base.bind_vma = ggtt_bind_vma; dev_priv->gtt.base.unbind_vma = ggtt_unbind_vma; return ret; @@ -3002,7 +3027,10 @@ static int gen6_gmch_probe(struct drm_device *dev, dev_priv->gtt.base.clear_range = gen6_ggtt_clear_range; dev_priv->gtt.base.insert_page = gen6_ggtt_insert_page; dev_priv->gtt.base.insert_entries = gen6_ggtt_insert_entries; - dev_priv->gtt.base.bind_vma = ggtt_bind_vma; + if (dev_priv->mm.aliasing_ppgtt) + dev_priv->gtt.base.bind_vma = agtt_bind_vma; + else + dev_priv->gtt.base.bind_vma = ggtt_bind_vma; dev_priv->gtt.base.unbind_vma = ggtt_unbind_vma; return ret;