From patchwork Fri May 29 16:44:09 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Harrison X-Patchwork-Id: 6509631 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 3319DC0433 for ; Fri, 29 May 2015 16:45:45 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 11D3520797 for ; Fri, 29 May 2015 16:45:44 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id BB20820825 for ; Fri, 29 May 2015 16:45:41 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 6792A7215F; Fri, 29 May 2015 09:45:40 -0700 (PDT) X-Original-To: Intel-GFX@lists.freedesktop.org Delivered-To: Intel-GFX@lists.freedesktop.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 1144F7215A for ; Fri, 29 May 2015 09:45:35 -0700 (PDT) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga102.jf.intel.com with ESMTP; 29 May 2015 09:45:10 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,518,1427785200"; d="scan'208";a="578884368" Received: from johnharr-linux.isw.intel.com ([10.102.226.188]) by orsmga003.jf.intel.com with ESMTP; 29 May 2015 09:45:08 -0700 From: John.C.Harrison@Intel.com To: Intel-GFX@Lists.FreeDesktop.Org Date: Fri, 29 May 2015 17:44:09 +0100 Message-Id: <1432917856-12261-49-git-send-email-John.C.Harrison@Intel.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1432917856-12261-1-git-send-email-John.C.Harrison@Intel.com> References: <1432917856-12261-1-git-send-email-John.C.Harrison@Intel.com> Organization: Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ Subject: [Intel-gfx] [PATCH 48/55] drm/i915: Add *_ring_begin() to request allocation 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 From: John Harrison Now that the *_ring_begin() functions no longer call the request allocation code, it is finally safe for the request allocation code to call *_ring_begin(). This is important to guarantee that the space reserved for the subsequent i915_add_request() call does actually get reserved. v2: Renamed functions according to review feedback (Tomas Elf). For: VIZ-5115 Signed-off-by: John Harrison --- drivers/gpu/drm/i915/i915_gem.c | 25 +++++++++++++------------ drivers/gpu/drm/i915/intel_lrc.c | 15 +++++++++++++++ drivers/gpu/drm/i915/intel_lrc.h | 1 + drivers/gpu/drm/i915/intel_ringbuffer.c | 29 ++++++++++++++++------------- drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + 5 files changed, 46 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 9f3e0717..1261792 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2680,19 +2680,20 @@ int i915_gem_request_alloc(struct intel_engine_cs *ring, * i915_add_request() call can't fail. Note that the reserve may need * to be redone if the request is not actually submitted straight * away, e.g. because a GPU scheduler has deferred it. - * - * Note further that this call merely notes the reserve request. A - * subsequent call to *_ring_begin() is required to actually ensure - * that the reservation is available. Without the begin, if the - * request creator immediately submitted the request without adding - * any commands to it then there might not actually be sufficient - * room for the submission commands. Unfortunately, the current - * *_ring_begin() implementations potentially call back here to - * i915_gem_request_alloc(). Thus calling _begin() here would lead to - * infinite recursion! Until that back call path is removed, it is - * necessary to do a manual _begin() outside. */ - intel_ring_reserved_space_reserve(req->ringbuf, MIN_SPACE_FOR_ADD_REQUEST); + if (i915.enable_execlists) + ret = intel_logical_ring_reserve_space(req); + else + ret = intel_ring_reserve_space(req); + if (ret) { + /* + * At this point, the request is fully allocated even if not + * fully prepared. Thus it can be cleaned up using the proper + * free code. + */ + i915_gem_request_cancel(req); + return ret; + } *req_out = ring->outstanding_lazy_request = req; return 0; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 548c53d..e164ac0 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -823,6 +823,21 @@ static int intel_logical_ring_begin(struct drm_i915_gem_request *req, return 0; } +int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request) +{ + /* + * The first call merely notes the reserve request and is common for + * all back ends. The subsequent localised _begin() call actually + * ensures that the reservation is available. Without the begin, if + * the request creator immediately submitted the request without + * adding any commands to it then there might not actually be + * sufficient room for the submission commands. + */ + intel_ring_reserved_space_reserve(request->ringbuf, MIN_SPACE_FOR_ADD_REQUEST); + + return intel_logical_ring_begin(request, 0); +} + /** * execlists_submission() - submit a batchbuffer for execution, Execlists style * @dev: DRM device. diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h index 044c0e5..f59940a 100644 --- a/drivers/gpu/drm/i915/intel_lrc.h +++ b/drivers/gpu/drm/i915/intel_lrc.h @@ -37,6 +37,7 @@ /* Logical Rings */ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request); +int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request); void intel_logical_ring_stop(struct intel_engine_cs *ring); void intel_logical_ring_cleanup(struct intel_engine_cs *ring); int intel_logical_rings_init(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index bb10fc2..0ba5787 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2192,24 +2192,27 @@ int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request) return 0; } +int intel_ring_reserve_space(struct drm_i915_gem_request *request) +{ + /* + * The first call merely notes the reserve request and is common for + * all back ends. The subsequent localised _begin() call actually + * ensures that the reservation is available. Without the begin, if + * the request creator immediately submitted the request without + * adding any commands to it then there might not actually be + * sufficient room for the submission commands. + */ + intel_ring_reserved_space_reserve(request->ringbuf, MIN_SPACE_FOR_ADD_REQUEST); + + return intel_ring_begin(request, 0); +} + void intel_ring_reserved_space_reserve(struct intel_ringbuffer *ringbuf, int size) { - /* NB: Until request management is fully tidied up and the OLR is - * removed, there are too many ways for get false hits on this - * anti-recursion check! */ - /*WARN_ON(ringbuf->reserved_size);*/ + WARN_ON(ringbuf->reserved_size); WARN_ON(ringbuf->reserved_in_use); ringbuf->reserved_size = size; - - /* - * Really need to call _begin() here but that currently leads to - * recursion problems! This will be fixed later but for now just - * return and hope for the best. Note that there is only a real - * problem if the create of the request never actually calls _begin() - * but if they are not submitting any work then why did they create - * the request in the first place? - */ } void intel_ring_reserved_space_cancel(struct intel_ringbuffer *ringbuf) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 16fd9ba..f4633ca 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -450,6 +450,7 @@ intel_ring_get_request(struct intel_engine_cs *ring) #define MIN_SPACE_FOR_ADD_REQUEST 128 +int intel_ring_reserve_space(struct drm_i915_gem_request *request); void intel_ring_reserved_space_reserve(struct intel_ringbuffer *ringbuf, int size); void intel_ring_reserved_space_cancel(struct intel_ringbuffer *ringbuf); void intel_ring_reserved_space_use(struct intel_ringbuffer *ringbuf);