From patchwork Tue Aug 5 08:47:07 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: alistair.mcaulay@intel.com X-Patchwork-Id: 4676531 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 2FE51C0338 for ; Tue, 5 Aug 2014 08:47:22 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 5FEEC2015A for ; Tue, 5 Aug 2014 08:47:18 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 7070C20158 for ; Tue, 5 Aug 2014 08:47:14 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 0AE9D6E14E; Tue, 5 Aug 2014 01:47:14 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id 0D93C6E14E for ; Tue, 5 Aug 2014 01:47:13 -0700 (PDT) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga101.fm.intel.com with ESMTP; 05 Aug 2014 01:47:12 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.01,804,1400050800"; d="scan'208";a="571868869" Received: from mcaulaya-linux2.isw.intel.com ([10.102.226.45]) by fmsmga001.fm.intel.com with ESMTP; 05 Aug 2014 01:47:11 -0700 From: alistair.mcaulay@intel.com To: intel-gfx@lists.freedesktop.org Date: Tue, 5 Aug 2014 09:47:07 +0100 Message-Id: <1407228427-27797-1-git-send-email-alistair.mcaulay@intel.com> X-Mailer: git-send-email 2.0.0 In-Reply-To: <1405523159-8502-1-git-send-email-alistair.mcaulay@intel.com> References: <1405523159-8502-1-git-send-email-alistair.mcaulay@intel.com> Subject: [Intel-gfx] [PATCH v2] drm/i915: Rework GPU reset sequence to match driver load & thaw X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.15 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.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, 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: "McAulay, Alistair" This patch is to address Daniels concerns over different code during reset: http://lists.freedesktop.org/archives/intel-gfx/2014-June/047758.html "The reason for aiming as hard as possible to use the exact same code for driver load, gpu reset and runtime pm/system resume is that we've simply seen too many bugs due to slight variations and unintended omissions." Tested using igt drv_hangman. V2: Cleaner way of preventing check_wedge returning -EAGAIN Signed-off-by: McAulay, Alistair --- drivers/gpu/drm/i915/i915_drv.c | 6 +++ drivers/gpu/drm/i915/i915_drv.h | 3 ++ drivers/gpu/drm/i915/i915_gem.c | 6 +-- drivers/gpu/drm/i915/i915_gem_context.c | 42 +-------------------- drivers/gpu/drm/i915/i915_gem_gtt.c | 67 +++++---------------------------- drivers/gpu/drm/i915/i915_gem_gtt.h | 3 +- 6 files changed, 23 insertions(+), 104 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 5e4fefd..3bfafe6 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -806,7 +806,13 @@ int i915_reset(struct drm_device *dev) !dev_priv->ums.mm_suspended) { dev_priv->ums.mm_suspended = 0; + /* Used to prevent gem_check_wedged returning -EAGAIN during gpu reset */ + dev_priv->gpu_error.reload_in_reset = true; + ret = i915_gem_init_hw(dev); + + dev_priv->gpu_error.reload_in_reset = false; + mutex_unlock(&dev->struct_mutex); if (ret) { DRM_ERROR("Failed hw init on reset %d\n", ret); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 991b663..116daff 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1217,6 +1217,9 @@ struct i915_gpu_error { /* For missed irq/seqno simulation. */ unsigned int test_irq_rings; + + /* Used to prevent gem_check_wedged returning -EAGAIN during gpu reset */ + bool reload_in_reset; }; enum modeset_restore { diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ef047bc..14e1770 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1085,7 +1085,9 @@ i915_gem_check_wedge(struct i915_gpu_error *error, if (i915_terminally_wedged(error)) return -EIO; - return -EAGAIN; + /* Check if GPU Reset is in progress */ + if (!error->reload_in_reset) + return -EAGAIN; } return 0; @@ -2590,8 +2592,6 @@ void i915_gem_reset(struct drm_device *dev) for_each_ring(ring, dev_priv, i) i915_gem_reset_ring_cleanup(dev_priv, ring); - i915_gem_context_reset(dev); - i915_gem_restore_fences(dev); } diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index de72a28..d96219f 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -372,42 +372,6 @@ err_destroy: return ERR_PTR(ret); } -void i915_gem_context_reset(struct drm_device *dev) -{ - struct drm_i915_private *dev_priv = dev->dev_private; - int i; - - /* Prevent the hardware from restoring the last context (which hung) on - * the next switch */ - for (i = 0; i < I915_NUM_RINGS; i++) { - struct intel_engine_cs *ring = &dev_priv->ring[i]; - struct intel_context *dctx = ring->default_context; - struct intel_context *lctx = ring->last_context; - - /* Do a fake switch to the default context */ - if (lctx == dctx) - continue; - - if (!lctx) - continue; - - if (dctx->legacy_hw_ctx.rcs_state && i == RCS) { - WARN_ON(i915_gem_obj_ggtt_pin(dctx->legacy_hw_ctx.rcs_state, - get_context_alignment(dev), 0)); - /* Fake a finish/inactive */ - dctx->legacy_hw_ctx.rcs_state->base.write_domain = 0; - dctx->legacy_hw_ctx.rcs_state->active = 0; - } - - if (lctx->legacy_hw_ctx.rcs_state && i == RCS) - i915_gem_object_ggtt_unpin(lctx->legacy_hw_ctx.rcs_state); - - i915_gem_context_unreference(lctx); - i915_gem_context_reference(dctx); - ring->last_context = dctx; - } -} - int i915_gem_context_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -498,10 +462,6 @@ int i915_gem_context_enable(struct drm_i915_private *dev_priv) ppgtt->enable(ppgtt); } - /* FIXME: We should make this work, even in reset */ - if (i915_reset_in_progress(&dev_priv->gpu_error)) - return 0; - BUG_ON(!dev_priv->ring[RCS].default_context); for_each_ring(ring, dev_priv, i) { @@ -645,7 +605,7 @@ static int do_switch(struct intel_engine_cs *ring, from = ring->last_context; if (USES_FULL_PPGTT(ring->dev)) { - ret = ppgtt->switch_mm(ppgtt, ring, false); + ret = ppgtt->switch_mm(ppgtt, ring); if (ret) goto unpin_out; } diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 5188936..450c8a9 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -216,19 +216,12 @@ static gen6_gtt_pte_t iris_pte_encode(dma_addr_t addr, /* Broadwell Page Directory Pointer Descriptors */ static int gen8_write_pdp(struct intel_engine_cs *ring, unsigned entry, - uint64_t val, bool synchronous) + uint64_t val) { - struct drm_i915_private *dev_priv = ring->dev->dev_private; int ret; BUG_ON(entry >= 4); - if (synchronous) { - I915_WRITE(GEN8_RING_PDP_UDW(ring, entry), val >> 32); - I915_WRITE(GEN8_RING_PDP_LDW(ring, entry), (u32)val); - return 0; - } - ret = intel_ring_begin(ring, 6); if (ret) return ret; @@ -245,8 +238,7 @@ static int gen8_write_pdp(struct intel_engine_cs *ring, unsigned entry, } static int gen8_mm_switch(struct i915_hw_ppgtt *ppgtt, - struct intel_engine_cs *ring, - bool synchronous) + struct intel_engine_cs *ring) { int i, ret; @@ -255,7 +247,7 @@ static int gen8_mm_switch(struct i915_hw_ppgtt *ppgtt, for (i = used_pd - 1; i >= 0; i--) { dma_addr_t addr = ppgtt->pd_dma_addr[i]; - ret = gen8_write_pdp(ring, i, addr, synchronous); + ret = gen8_write_pdp(ring, i, addr); if (ret) return ret; } @@ -724,29 +716,10 @@ static uint32_t get_pd_offset(struct i915_hw_ppgtt *ppgtt) } static int hsw_mm_switch(struct i915_hw_ppgtt *ppgtt, - struct intel_engine_cs *ring, - bool synchronous) + struct intel_engine_cs *ring) { - struct drm_device *dev = ppgtt->base.dev; - struct drm_i915_private *dev_priv = dev->dev_private; int ret; - /* If we're in reset, we can assume the GPU is sufficiently idle to - * manually frob these bits. Ideally we could use the ring functions, - * except our error handling makes it quite difficult (can't use - * intel_ring_begin, ring->flush, or intel_ring_advance) - * - * FIXME: We should try not to special case reset - */ - if (synchronous || - i915_reset_in_progress(&dev_priv->gpu_error)) { - WARN_ON(ppgtt != dev_priv->mm.aliasing_ppgtt); - I915_WRITE(RING_PP_DIR_DCLV(ring), PP_DIR_DCLV_2G); - I915_WRITE(RING_PP_DIR_BASE(ring), get_pd_offset(ppgtt)); - POSTING_READ(RING_PP_DIR_BASE(ring)); - return 0; - } - /* NB: TLBs must be flushed and invalidated before a switch */ ret = ring->flush(ring, I915_GEM_GPU_DOMAINS, I915_GEM_GPU_DOMAINS); if (ret) @@ -768,29 +741,10 @@ static int hsw_mm_switch(struct i915_hw_ppgtt *ppgtt, } static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt, - struct intel_engine_cs *ring, - bool synchronous) + struct intel_engine_cs *ring) { - struct drm_device *dev = ppgtt->base.dev; - struct drm_i915_private *dev_priv = dev->dev_private; int ret; - /* If we're in reset, we can assume the GPU is sufficiently idle to - * manually frob these bits. Ideally we could use the ring functions, - * except our error handling makes it quite difficult (can't use - * intel_ring_begin, ring->flush, or intel_ring_advance) - * - * FIXME: We should try not to special case reset - */ - if (synchronous || - i915_reset_in_progress(&dev_priv->gpu_error)) { - WARN_ON(ppgtt != dev_priv->mm.aliasing_ppgtt); - I915_WRITE(RING_PP_DIR_DCLV(ring), PP_DIR_DCLV_2G); - I915_WRITE(RING_PP_DIR_BASE(ring), get_pd_offset(ppgtt)); - POSTING_READ(RING_PP_DIR_BASE(ring)); - return 0; - } - /* NB: TLBs must be flushed and invalidated before a switch */ ret = ring->flush(ring, I915_GEM_GPU_DOMAINS, I915_GEM_GPU_DOMAINS); if (ret) @@ -819,14 +773,11 @@ static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt, } static int gen6_mm_switch(struct i915_hw_ppgtt *ppgtt, - struct intel_engine_cs *ring, - bool synchronous) + struct intel_engine_cs *ring) { struct drm_device *dev = ppgtt->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; - if (!synchronous) - return 0; I915_WRITE(RING_PP_DIR_DCLV(ring), PP_DIR_DCLV_2G); I915_WRITE(RING_PP_DIR_BASE(ring), get_pd_offset(ppgtt)); @@ -852,7 +803,7 @@ static int gen8_ppgtt_enable(struct i915_hw_ppgtt *ppgtt) if (USES_FULL_PPGTT(dev)) continue; - ret = ppgtt->switch_mm(ppgtt, ring, true); + ret = ppgtt->switch_mm(ppgtt, ring); if (ret) goto err_out; } @@ -897,7 +848,7 @@ static int gen7_ppgtt_enable(struct i915_hw_ppgtt *ppgtt) if (USES_FULL_PPGTT(dev)) continue; - ret = ppgtt->switch_mm(ppgtt, ring, true); + ret = ppgtt->switch_mm(ppgtt, ring); if (ret) return ret; } @@ -926,7 +877,7 @@ static int gen6_ppgtt_enable(struct i915_hw_ppgtt *ppgtt) I915_WRITE(GFX_MODE, _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE)); for_each_ring(ring, dev_priv, i) { - int ret = ppgtt->switch_mm(ppgtt, ring, true); + int ret = ppgtt->switch_mm(ppgtt, ring); if (ret) return ret; } diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index 8d6f7c1..bf1e4fc 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -262,8 +262,7 @@ struct i915_hw_ppgtt { int (*enable)(struct i915_hw_ppgtt *ppgtt); int (*switch_mm)(struct i915_hw_ppgtt *ppgtt, - struct intel_engine_cs *ring, - bool synchronous); + struct intel_engine_cs *ring); void (*debug_dump)(struct i915_hw_ppgtt *ppgtt, struct seq_file *m); };