From patchwork Wed Jul 17 12:51:28 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 2828671 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 5614DC0AB2 for ; Wed, 17 Jul 2013 14:11:39 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 1754F20222 for ; Wed, 17 Jul 2013 14:11:38 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id CFB2020217 for ; Wed, 17 Jul 2013 14:11:36 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E2424E6166 for ; Wed, 17 Jul 2013 07:11:36 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ee0-f49.google.com (mail-ee0-f49.google.com [74.125.83.49]) by gabe.freedesktop.org (Postfix) with ESMTP id 7CCF2E5EEE for ; Wed, 17 Jul 2013 07:06:40 -0700 (PDT) Received: by mail-ee0-f49.google.com with SMTP id b57so1059133eek.36 for ; Wed, 17 Jul 2013 07:06:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:x-mailer:mime-version :content-type:content-transfer-encoding; bh=DuKCfylYzWfOH9/9doL+w8og2IDTBB8b196kefd7tGA=; b=SH/MKexUGwgGEanp8X4Zit/yPoUqNqLYzNr2g+B7iFhAI+aDDKsVdhFuTM9J9BDD82 tfTSYeL1izUjDzmrgSefJRwM1uO7mp/FxmL62B7zGJ/qBaBUWouHBSjAOQ/nWFQAkfyn pdoYNfQ3vdVVPseM5d1WweDD/K3fM90jtppYQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer:mime-version :content-type:content-transfer-encoding:x-gm-message-state; bh=DuKCfylYzWfOH9/9doL+w8og2IDTBB8b196kefd7tGA=; b=KvFBDi9oVF/VM8C30dQ5datlz757dxHarN4n+rbdt75bYfI7rdkZHIsQqlA5+4EoGq hXpE4rhrNgUwy5f3Ye1Io9oMwceGVEFCyEub5A/42sZy/8mB9zI5X2X97k5dp1sNQWCt l1aeXVuPoWg/5PhwxlW1tGnGuRG5c7o92yDek1fviRdBA1Hdv4PnaV7n7uAgH5AL8CH4 4dAE9m8qASLVLVQtfaAD6ismFBmVAQQOS2vHCjcDFOcO40aqRoNghzOQ7I7w+VTnD6MH lw3t+Dsl77v3w+uIH3S18eBHYJrQ+lXjZjkuUtForUj3xV+wmGQq9RgykviWq68WCSIU BOFQ== X-Received: by 10.14.193.199 with SMTP id k47mr6613482een.83.1374069999409; Wed, 17 Jul 2013 07:06:39 -0700 (PDT) Received: from wespe.ffwll.local (178-83-130-250.dynamic.hispeed.ch. [178.83.130.250]) by mx.google.com with ESMTPSA id cg12sm11098142eeb.7.2013.07.17.07.06.36 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Wed, 17 Jul 2013 07:06:37 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Wed, 17 Jul 2013 14:51:28 +0200 Message-Id: <1374065488-4485-1-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.8.1.4 MIME-Version: 1.0 X-Gm-Message-State: ALoCoQnmOfbKpepdk9+KATkcA7Qb9HbUqvW15ufZdj5zW/kkeQYTZ51CfEHvhXoLNE6zpYnAzwkI Cc: Daniel Vetter , "for 3.10 only" Subject: [Intel-gfx] [PATCH] drm/i915: correctly restore fences with objects attached X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,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 To avoid stalls we delay tiling changes and especially hold of committing the new fence state for as long as possible. Synchronization points are in the execbuf code and in our gtt fault handler. Unfortunately we've missed that tricky detail when adding proper fence restore code in commit 19b2dbde5732170a03bd82cc8bd442cf88d856f7 Author: Chris Wilson Date: Wed Jun 12 10:15:12 2013 +0100 drm/i915: Restore fences after resume and GPU resets The result was that we've restored fences for objects with no tiling, since the object<->fence link still existed after resume. Now that wouldn't have been too bad since any subsequent access would have fixed things up, but if we've changed from tiled to untiled real havoc happened: The tiling stride is stored -1 in the fence register, so a stride of 0 resulted in all 1s in the top 32bits, and so a completely bogus fence spanning everything from the start of the object to the top of the GTT. The tell-tale in the register dumps looks like: FENCE START 2: 0x0214d001 FENCE END 2: 0xfffff3ff Bit 11 isn't set since the hw doesn't store it, even when writing all 1s (at least on my snb here). To prevent such a gaffle in the future add a sanity check for fences with an untiled object attached in i915_gem_write_fence. v2: Fix the WARN, spotted by Chris. v3: Trying to reuse get_fences looked ugly and obfuscated the code. Instead reuse update_fence and to make it really dtrt also move the fence dirty state clearing into update_fence. Cc: Chris Wilson Cc: Stéphane Marchesin Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=60530 Cc: stable@vger.kernel.org (for 3.10 only) Signed-off-by: Daniel Vetter Reviewed-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index c9d9d20..e2370a2 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2257,7 +2257,17 @@ void i915_gem_restore_fences(struct drm_device *dev) for (i = 0; i < dev_priv->num_fence_regs; i++) { struct drm_i915_fence_reg *reg = &dev_priv->fence_regs[i]; - i915_gem_write_fence(dev, i, reg->obj); + + /* + * Commit delayed tiling changes if we have an object still + * attached to the fence, otherwise just clear the fence. + */ + if (reg->obj) { + i915_gem_object_update_fence(reg->obj, reg, + reg->obj->tiling_mode); + } else { + i915_gem_write_fence(dev, i, NULL); + } } } @@ -2792,6 +2802,10 @@ static void i915_gem_write_fence(struct drm_device *dev, int reg, if (i915_gem_object_needs_mb(dev_priv->fence_regs[reg].obj)) mb(); + WARN(obj && (!obj->stride || !obj->tiling_mode), + "bogus fence setup with stride: 0x%x, tiling mode: %i\n", + obj->stride, obj->tiling_mode); + switch (INTEL_INFO(dev)->gen) { case 7: case 6: @@ -2833,6 +2847,7 @@ static void i915_gem_object_update_fence(struct drm_i915_gem_object *obj, fence->obj = NULL; list_del_init(&fence->lru_list); } + obj->fence_dirty = false; } static int @@ -2962,7 +2977,6 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj) return 0; i915_gem_object_update_fence(obj, reg, enable); - obj->fence_dirty = false; return 0; }