From patchwork Fri Jun 12 20:25:38 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Gordon X-Patchwork-Id: 6600801 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.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 230CB9F3D1 for ; Fri, 12 Jun 2015 20:26:02 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 1683F20646 for ; Fri, 12 Jun 2015 20:26:01 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id E2BE120658 for ; Fri, 12 Jun 2015 20:25:59 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 5A3446E5BD; Fri, 12 Jun 2015 13:25:59 -0700 (PDT) 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 B4FCE6E59C for ; Fri, 12 Jun 2015 13:25:56 -0700 (PDT) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP; 12 Jun 2015 13:25:56 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,604,1427785200"; d="scan'208";a="745766765" Received: from dsgordon-linux.isw.intel.com ([10.102.226.51]) by orsmga002.jf.intel.com with ESMTP; 12 Jun 2015 13:25:55 -0700 From: Dave Gordon To: intel-gfx@lists.freedesktop.org Date: Fri, 12 Jun 2015 21:25:38 +0100 Message-Id: <1434140738-19521-3-git-send-email-david.s.gordon@intel.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1434140738-19521-1-git-send-email-david.s.gordon@intel.com> References: <1434128948-9221-3-git-send-email-david.s.gordon@intel.com> <1434140738-19521-1-git-send-email-david.s.gordon@intel.com> Subject: [Intel-gfx] [PATCH 2/2] drm/i915: Allocate OLR more safely (workaround until OLR goes away) 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 original idea of preallocating the OLR was implemented in > 9d773091 drm/i915: Preallocate next seqno before touching the ring and the sequence of operations was to allocate the OLR, then wrap past the end of the ring if necessary, then wait for space if necessary. But subsequently intel_ring_begin() was refactored, in > 304d695 drm/i915: Flush outstanding requests before allocating new seqno to ensure that pending work that might need to be flushed used the old and not the newly-allocated request. This changed the sequence to wrap and/or wait, then allocate, although the comment still said /* Preallocate the olr before touching the ring */ which was no longer true as intel_wrap_ring_buffer() touches the ring. However, with the introduction of dynamic pinning, in > 7ba717c drm/i915/bdw: Pin the ringbuffer backing object to GGTT on-demand came the possibility that the ringbuffer might not be pinned to the GTT or mapped into CPU address space when intel_ring_begin() is called. It gets pinned when the request is allocated, so it's now important that this comes *before* anything that can write into the ringbuffer, in this case intel_wrap_ring_buffer(), as this will fault if (a) the ringbuffer happens not to be mapped, and (b) tail happens to be sufficiently close to the end of the ring to trigger wrapping. So the correct order is neither the original allocate-wait-pad-wait, nor the subsequent wait-pad-wait-allocate, but wait-allocate-pad, avoiding both the problems described in the two commits mentioned above. So this commit moves the calls to i915_gem_request_alloc() into the middle of {__intel,logical}_ring_prepare() rather than either before or after them. Signed-off-by: Dave Gordon --- drivers/gpu/drm/i915/intel_lrc.c | 12 +++++++----- drivers/gpu/drm/i915/intel_ringbuffer.c | 12 +++++++----- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index a4bb5d3..3ef5fb6 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -764,6 +764,13 @@ static int logical_ring_prepare(struct intel_ringbuffer *ringbuf, return ret; } + /* Ensure we have a request before touching the ring */ + if (!ringbuf->ring->outstanding_lazy_request) { + ret = i915_gem_request_alloc(ringbuf->ring, ctx); + if (ret) + return ret; + } + if (unlikely(fill)) { uint32_t __iomem *virt = ringbuf->virtual_start + ringbuf->tail; @@ -812,11 +819,6 @@ static int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, if (ret) return ret; - /* Preallocate the olr before touching the ring */ - ret = i915_gem_request_alloc(ring, ctx); - if (ret) - return ret; - ringbuf->space -= num_dwords * sizeof(uint32_t); return 0; } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 5a1cd20..ddf580d 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2195,6 +2195,13 @@ static int __intel_ring_prepare(struct intel_engine_cs *ring, return ret; } + /* Ensure we have a request before touching the ring */ + if (!ringbuf->ring->outstanding_lazy_request) { + ret = i915_gem_request_alloc(ringbuf->ring, ring->default_context); + if (ret) + return ret; + } + if (unlikely(fill)) { uint32_t __iomem *virt = ringbuf->virtual_start + ringbuf->tail; @@ -2228,11 +2235,6 @@ int intel_ring_begin(struct intel_engine_cs *ring, if (ret) return ret; - /* Preallocate the olr before touching the ring */ - ret = i915_gem_request_alloc(ring, ring->default_context); - if (ret) - return ret; - ring->buffer->space -= num_dwords * sizeof(uint32_t); return 0; }