From patchwork Mon Jun 8 18:51:36 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Gordon X-Patchwork-Id: 6567501 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 77D57C0020 for ; Mon, 8 Jun 2015 18:51:54 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 834142051C for ; Mon, 8 Jun 2015 18:51:53 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 6780D2027D for ; Mon, 8 Jun 2015 18:51:52 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C3C616E32C; Mon, 8 Jun 2015 11:51:51 -0700 (PDT) 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 D8AFC6E32C for ; Mon, 8 Jun 2015 11:51:50 -0700 (PDT) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP; 08 Jun 2015 11:51:49 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,574,1427785200"; d="scan'208";a="504964490" Received: from dsgordon-linux.isw.intel.com ([10.102.226.51]) by FMSMGA003.fm.intel.com with ESMTP; 08 Jun 2015 11:51:47 -0700 From: Dave Gordon To: intel-gfx@lists.freedesktop.org Date: Mon, 8 Jun 2015 19:51:36 +0100 Message-Id: <1433789496-8365-1-git-send-email-david.s.gordon@intel.com> X-Mailer: git-send-email 1.7.9.5 Subject: [Intel-gfx] [PATCH] drm/i915: Reinstate order of operations in {intel, logical}_ring_begin() 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. The reversal didn't introduce any problems until the introduction of dynamic pinning, in > 7ba717c drm/i915/bdw: Pin the ringbuffer backing object to GGTT on-demand With that 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, specifically 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. The original rationale for this reversal seems to no longer apply, as we shouldn't ever have anything in the ringbuffer which is not associated with a specific request, and therefore shouldn't have anything to flush. So it should now be safe to reinstate the original sequence of allocate-wrap-wait :) Between the original sequence swap and now, the ringbuffer code got cloned to become the execbuffer code, in > 82e104c drm/i915/bdw: New logical ring submission mechanism So now we have to fix it in both paths ... Signed-off-by: Alex Dai Signed-off-by: Dave Gordon Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com) --- drivers/gpu/drm/i915/intel_lrc.c | 6 +++--- drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 9b74ffa..4d82d9b 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -809,12 +809,12 @@ static int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, if (ret) return ret; - ret = logical_ring_prepare(ringbuf, ctx, num_dwords * sizeof(uint32_t)); + /* Preallocate the olr before touching the ring */ + ret = i915_gem_request_alloc(ring, ctx); if (ret) return ret; - /* Preallocate the olr before touching the ring */ - ret = i915_gem_request_alloc(ring, ctx); + ret = logical_ring_prepare(ringbuf, ctx, num_dwords * sizeof(uint32_t)); if (ret) return ret; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index b70d25b..9f7a4d2 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2222,12 +2222,12 @@ int intel_ring_begin(struct intel_engine_cs *ring, if (ret) return ret; - ret = __intel_ring_prepare(ring, num_dwords * sizeof(uint32_t)); + /* Preallocate the olr before touching the ring */ + ret = i915_gem_request_alloc(ring, ring->default_context); if (ret) return ret; - /* Preallocate the olr before touching the ring */ - ret = i915_gem_request_alloc(ring, ring->default_context); + ret = __intel_ring_prepare(ring, num_dwords * sizeof(uint32_t)); if (ret) return ret;