From patchwork Tue Nov 18 20:07:22 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Gordon X-Patchwork-Id: 5332941 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 0FDC09F2ED for ; Tue, 18 Nov 2014 20:07:50 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id DCC9D2018E for ; Tue, 18 Nov 2014 20:07:48 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id C128720172 for ; Tue, 18 Nov 2014 20:07:47 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 51A566E5E8; Tue, 18 Nov 2014 12:07:47 -0800 (PST) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 18E406E5E8 for ; Tue, 18 Nov 2014 12:07:42 -0800 (PST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP; 18 Nov 2014 12:07:42 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.97,862,1389772800"; d="scan'208";a="418303190" Received: from dsgordon-linux.isw.intel.com ([10.102.226.149]) by FMSMGA003.fm.intel.com with ESMTP; 18 Nov 2014 11:58:21 -0800 From: Dave Gordon To: intel-gfx@lists.freedesktop.org Date: Tue, 18 Nov 2014 20:07:22 +0000 Message-Id: <1416341242-5352-4-git-send-email-david.s.gordon@intel.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1416341242-5352-1-git-send-email-david.s.gordon@intel.com> References: <1415021344-24875-1-git-send-email-david.s.gordon@intel.com> <1416341242-5352-1-git-send-email-david.s.gordon@intel.com> Subject: [Intel-gfx] [PATCH v2 3/3] drm/i915: Consolidate ring freespace calculations 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 There are numerous places in the code where the driver's idea of how much space is left in a ring is updated using the driver's latest notions of the positions of 'head' and 'tail' for the ring. Among them are some that update one or both of these values before (re)doing the calculation. In particular, there are four different places in the code where 'last_retired_head' is copied to 'head' and then set to -1; and two of these do not have a guard to check that it has actually been updated since last time it was consumed, leaving the possibility that the dummy -1 can be transferred from 'last_retired_head' to 'head', causing the space calculation to produce 'impossible' results (previously seen on Android/VLV). This code therefore consolidates all the calculation and updating of these values, such that there is only one place where the ring space is updated, and it ALWAYS uses (and consumes) 'last_retired_head' if (and ONLY if) it has been updated since the last call. Signed-off-by: Dave Gordon Reviewed-by: Deepak S --- drivers/gpu/drm/i915/i915_dma.c | 5 ++- drivers/gpu/drm/i915/intel_lrc.c | 25 +++++---------- drivers/gpu/drm/i915/intel_ringbuffer.c | 51 ++++++++++++++++--------------- drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + 4 files changed, 37 insertions(+), 45 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 5dc37f0..4a98399 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -154,11 +154,10 @@ void i915_kernel_lost_context(struct drm_device *dev) if (drm_core_check_feature(dev, DRIVER_MODESET)) return; + ringbuf->last_retired_head = -1; ringbuf->head = I915_READ_HEAD(ring) & HEAD_ADDR; ringbuf->tail = I915_READ_TAIL(ring) & TAIL_ADDR; - ringbuf->space = ringbuf->head - (ringbuf->tail + I915_RING_FREE_SPACE); - if (ringbuf->space < 0) - ringbuf->space += ringbuf->size; + intel_ring_update_space(ringbuf); if (!dev->primary->master) return; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index ad31373..c9a5227 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -825,14 +825,8 @@ static int logical_ring_wait_request(struct intel_ringbuffer *ringbuf, u32 seqno = 0; int ret; - if (ringbuf->last_retired_head != -1) { - ringbuf->head = ringbuf->last_retired_head; - ringbuf->last_retired_head = -1; - - ringbuf->space = intel_ring_space(ringbuf); - if (ringbuf->space >= bytes) - return 0; - } + if (intel_ring_space(ringbuf) >= bytes) + return 0; list_for_each_entry(request, &ring->request_list, list) { /* @@ -860,11 +854,8 @@ static int logical_ring_wait_request(struct intel_ringbuffer *ringbuf, return ret; i915_gem_retire_requests_ring(ring); - ringbuf->head = ringbuf->last_retired_head; - ringbuf->last_retired_head = -1; - ringbuf->space = intel_ring_space(ringbuf); - return 0; + return intel_ring_space(ringbuf) >= bytes ? 0 : -ENOSPC; } static int logical_ring_wait_for_space(struct intel_ringbuffer *ringbuf, @@ -890,12 +881,10 @@ static int logical_ring_wait_for_space(struct intel_ringbuffer *ringbuf, * case by choosing an insanely large timeout. */ end = jiffies + 60 * HZ; + ret = 0; do { - ringbuf->space = intel_ring_space(ringbuf); - if (ringbuf->space >= bytes) { - ret = 0; + if (intel_ring_space(ringbuf) >= bytes) break; - } msleep(1); @@ -936,7 +925,7 @@ static int logical_ring_wrap_buffer(struct intel_ringbuffer *ringbuf) iowrite32(MI_NOOP, virt++); ringbuf->tail = 0; - ringbuf->space = intel_ring_space(ringbuf); + intel_ring_update_space(ringbuf); return 0; } @@ -1777,8 +1766,8 @@ int intel_lr_context_deferred_create(struct intel_context *ctx, ringbuf->effective_size = ringbuf->size; ringbuf->head = 0; ringbuf->tail = 0; - ringbuf->space = ringbuf->size; ringbuf->last_retired_head = -1; + intel_ring_update_space(ringbuf); /* TODO: For now we put this in the mappable region so that we can reuse * the existing ringbuffer code which ioremaps it. When we start diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index ae09258..a08ae65 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -52,16 +52,27 @@ intel_ring_initialized(struct intel_engine_cs *ring) int __intel_ring_space(int head, int tail, int size) { - int space = head - (tail + I915_RING_FREE_SPACE); - if (space < 0) + int space = head - tail; + if (space <= 0) space += size; - return space; + return space - I915_RING_FREE_SPACE; +} + +void intel_ring_update_space(struct intel_ringbuffer *ringbuf) +{ + if (ringbuf->last_retired_head != -1) { + ringbuf->head = ringbuf->last_retired_head; + ringbuf->last_retired_head = -1; + } + + ringbuf->space = __intel_ring_space(ringbuf->head & HEAD_ADDR, + ringbuf->tail, ringbuf->size); } int intel_ring_space(struct intel_ringbuffer *ringbuf) { - return __intel_ring_space(ringbuf->head & HEAD_ADDR, - ringbuf->tail, ringbuf->size); + intel_ring_update_space(ringbuf); + return ringbuf->space; } bool intel_ring_stopped(struct intel_engine_cs *ring) @@ -592,10 +603,10 @@ static int init_ring_common(struct intel_engine_cs *ring) if (!drm_core_check_feature(ring->dev, DRIVER_MODESET)) i915_kernel_lost_context(ring->dev); else { + ringbuf->last_retired_head = -1; ringbuf->head = I915_READ_HEAD(ring); ringbuf->tail = I915_READ_TAIL(ring) & TAIL_ADDR; - ringbuf->space = intel_ring_space(ringbuf); - ringbuf->last_retired_head = -1; + intel_ring_update_space(ringbuf); } memset(&ring->hangcheck, 0, sizeof(ring->hangcheck)); @@ -1880,14 +1891,8 @@ static int intel_ring_wait_request(struct intel_engine_cs *ring, int n) u32 seqno = 0; int ret; - if (ringbuf->last_retired_head != -1) { - ringbuf->head = ringbuf->last_retired_head; - ringbuf->last_retired_head = -1; - - ringbuf->space = intel_ring_space(ringbuf); - if (ringbuf->space >= n) - return 0; - } + if (intel_ring_space(ringbuf) >= n) + return 0; list_for_each_entry(request, &ring->request_list, list) { if (__intel_ring_space(request->tail, ringbuf->tail, @@ -1905,10 +1910,7 @@ static int intel_ring_wait_request(struct intel_engine_cs *ring, int n) return ret; i915_gem_retire_requests_ring(ring); - ringbuf->head = ringbuf->last_retired_head; - ringbuf->last_retired_head = -1; - ringbuf->space = intel_ring_space(ringbuf); return 0; } @@ -1934,14 +1936,14 @@ static int ring_wait_for_space(struct intel_engine_cs *ring, int n) * case by choosing an insanely large timeout. */ end = jiffies + 60 * HZ; + ret = 0; trace_i915_ring_wait_begin(ring); do { + if (intel_ring_space(ringbuf) >= n) + break; ringbuf->head = I915_READ_HEAD(ring); - ringbuf->space = intel_ring_space(ringbuf); - if (ringbuf->space >= n) { - ret = 0; + if (intel_ring_space(ringbuf) >= n) break; - } if (!drm_core_check_feature(dev, DRIVER_MODESET) && dev->primary->master) { @@ -1989,7 +1991,7 @@ static int intel_wrap_ring_buffer(struct intel_engine_cs *ring) iowrite32(MI_NOOP, virt++); ringbuf->tail = 0; - ringbuf->space = intel_ring_space(ringbuf); + intel_ring_update_space(ringbuf); return 0; } @@ -2061,6 +2063,7 @@ int intel_ring_begin(struct intel_engine_cs *ring, int num_dwords) { struct drm_i915_private *dev_priv = ring->dev->dev_private; + struct intel_ringbuffer *ringbuf = ring->buffer; int ret; ret = i915_gem_check_wedge(&dev_priv->gpu_error, @@ -2077,7 +2080,7 @@ int intel_ring_begin(struct intel_engine_cs *ring, if (ret) return ret; - ring->buffer->space -= num_dwords * sizeof(uint32_t); + ringbuf->space -= num_dwords * sizeof(uint32_t); return 0; } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index aab2e2f..24a5723 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -404,6 +404,7 @@ static inline void intel_ring_advance(struct intel_engine_cs *ring) ringbuf->tail &= ringbuf->size - 1; } int __intel_ring_space(int head, int tail, int size); +void intel_ring_update_space(struct intel_ringbuffer *ringbuf); int intel_ring_space(struct intel_ringbuffer *ringbuf); bool intel_ring_stopped(struct intel_engine_cs *ring); void __intel_ring_advance(struct intel_engine_cs *ring);