From patchwork Thu Jun 26 17:24:14 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Harrison X-Patchwork-Id: 4429191 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 46C079FBBD for ; Thu, 26 Jun 2014 17:26:01 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 7BF1A20351 for ; Thu, 26 Jun 2014 17:25:57 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 6BE5620394 for ; Thu, 26 Jun 2014 17:25:56 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id BF4AB6E2EF; Thu, 26 Jun 2014 10:25:51 -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 48F436E1F9 for ; Thu, 26 Jun 2014 10:25:33 -0700 (PDT) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP; 26 Jun 2014 10:25:32 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.01,554,1400050800"; d="scan'208";a="561434732" Received: from johnharr-linux.iwi.intel.com ([172.28.253.52]) by fmsmga002.fm.intel.com with ESMTP; 26 Jun 2014 10:25:20 -0700 From: John.C.Harrison@Intel.com To: Intel-GFX@lists.freedesktop.org Date: Thu, 26 Jun 2014 18:24:14 +0100 Message-Id: <1403803475-16337-24-git-send-email-John.C.Harrison@Intel.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1403803475-16337-1-git-send-email-John.C.Harrison@Intel.com> References: <1403803475-16337-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] [RFC 23/44] drm/i915: Added manipulation of OLS/PLR 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.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 The scheduler requires each batch buffer to be tagged with the seqno it has been assigned and for that seqno to only be attached to the given batch buffer. Note that the seqno assigned to a batch buffer that is being submitted to the hardware might be very different to the next seqno that would be assigned automatically on ring submission. This means manipulating the lazy seqno and request values around batch buffer submission. At the start of execbuffer() the lazy seqno should be zero, if not it means that something has been written to the ring without a request being added. The lazy seqno also needs to be reset back to zero at the end ready for the next request to start. Then, execbuffer_final() needs to manually set the lazy seqno to the batch buffer's pre-assigned value rather than grabbing the next available value. There is no need to explictly clear the lazy seqno at the end of _final() as the add_request() call within _retire_commands() will do that automatically. --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 68 +++++++++++++++++++++++++++- drivers/gpu/drm/i915/i915_scheduler.h | 2 + 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 6bb1fd6..98cc95e 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1328,10 +1328,22 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, vma->bind_vma(vma, batch_obj->cache_level, GLOBAL_BIND); } + /* OLS should be zero at this point. If not then this buffer is going + * to be tagged as someone else's work! */ + BUG_ON(ring->outstanding_lazy_seqno != 0); + BUG_ON(ring->preallocated_lazy_request != NULL); + /* Allocate a seqno for this batch buffer nice and early. */ ret = intel_ring_alloc_seqno(ring); if (ret) goto err; + qe.params.seqno = ring->outstanding_lazy_seqno; + qe.params.request = ring->preallocated_lazy_request; + + BUG_ON(ring->outstanding_lazy_seqno == 0); + BUG_ON(ring->outstanding_lazy_seqno != qe.params.seqno); + BUG_ON(ring->preallocated_lazy_request != qe.params.request); + BUG_ON(ring->preallocated_lazy_request == NULL); /* Save assorted stuff away to pass through to execbuffer_final() */ qe.params.dev = dev; @@ -1373,6 +1385,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, qe.params.ctx = ctx; #endif // CONFIG_DRM_I915_SCHEDULER + /* OLS should have been set to something useful above */ + BUG_ON(ring->outstanding_lazy_seqno != qe.params.seqno); + BUG_ON(ring->preallocated_lazy_request != qe.params.request); + if (flags & I915_DISPATCH_SECURE) qe.params.batch_obj_vm_offset = i915_gem_obj_ggtt_offset(batch_obj); else @@ -1384,6 +1400,19 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, i915_gem_execbuffer_move_to_active(&eb->vmas, ring); + /* Make sure the OLS hasn't advanced (which would indicate a flush + * of the work in progess which in turn would be a Bad Thing). */ + BUG_ON(ring->outstanding_lazy_seqno != qe.params.seqno); + BUG_ON(ring->preallocated_lazy_request != qe.params.request); + + /* + * A new seqno has been assigned to the buffer and saved away for + * future reference. So clear the OLS to ensure that any further + * work is assigned a brand new seqno: + */ + ring->outstanding_lazy_seqno = 0; + ring->preallocated_lazy_request = NULL; + ret = i915_scheduler_queue_execbuffer(&qe); if (ret) goto err; @@ -1425,6 +1454,12 @@ err: } #endif // CONFIG_DRM_I915_SCHEDULER + /* Clear the OLS again in case the failure occurred after it had been + * assigned. */ + kfree(ring->preallocated_lazy_request); + ring->preallocated_lazy_request = NULL; + ring->outstanding_lazy_seqno = 0; + mutex_unlock(&dev->struct_mutex); pre_mutex_err: @@ -1443,6 +1478,7 @@ int i915_gem_do_execbuffer_final(struct i915_execbuffer_params *params) struct intel_engine_cs *ring = params->ring; u64 exec_start, exec_len; int ret, i; + u32 seqno; /* The mutex must be acquired before calling this function */ BUG_ON(!mutex_is_locked(¶ms->dev->struct_mutex)); @@ -1454,6 +1490,14 @@ int i915_gem_do_execbuffer_final(struct i915_execbuffer_params *params) intel_runtime_pm_get(dev_priv); + /* Ensure the correct seqno gets assigned to the correct buffer: */ + BUG_ON(ring->outstanding_lazy_seqno != 0); + BUG_ON(ring->preallocated_lazy_request != NULL); + ring->outstanding_lazy_seqno = params->seqno; + ring->preallocated_lazy_request = params->request; + + seqno = params->seqno; + /* Unconditionally invalidate gpu caches and ensure that we do flush * any residual writes from the previous batch. */ @@ -1466,6 +1510,10 @@ int i915_gem_do_execbuffer_final(struct i915_execbuffer_params *params) if (ret) goto err; + /* Seqno matches? */ + BUG_ON(seqno != params->seqno); + BUG_ON(ring->outstanding_lazy_seqno != params->seqno); + if (ring == &dev_priv->ring[RCS] && params->mode != dev_priv->relative_constants_mode) { ret = intel_ring_begin(ring, 4); @@ -1487,6 +1535,9 @@ int i915_gem_do_execbuffer_final(struct i915_execbuffer_params *params) goto err; } + /* Seqno matches? */ + BUG_ON(ring->outstanding_lazy_seqno != params->seqno); + BUG_ON(ring->preallocated_lazy_request != params->request); exec_len = params->args_batch_len; exec_start = params->batch_obj_vm_offset + @@ -1513,12 +1564,27 @@ int i915_gem_do_execbuffer_final(struct i915_execbuffer_params *params) goto err; } - trace_i915_gem_ring_dispatch(ring, intel_ring_get_seqno(ring), params->eb_flags); + trace_i915_gem_ring_dispatch(ring, seqno, params->eb_flags); + + /* Seqno matches? */ + BUG_ON(params->seqno != ring->outstanding_lazy_seqno); + BUG_ON(params->request != ring->preallocated_lazy_request); i915_gem_execbuffer_retire_commands(params->dev, params->file, ring, params->batch_obj); + /* OLS should be zero by now! */ + BUG_ON(ring->outstanding_lazy_seqno); + BUG_ON(ring->preallocated_lazy_request); + err: + if (ret) { + /* Reset the OLS ready to try again later. */ + kfree(ring->preallocated_lazy_request); + ring->preallocated_lazy_request = NULL; + ring->outstanding_lazy_seqno = 0; + } + /* intel_gpu_busy should also get a ref, so it will free when the device * is really idle. */ intel_runtime_pm_put(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_scheduler.h b/drivers/gpu/drm/i915/i915_scheduler.h index 7c88a26..e62254a 100644 --- a/drivers/gpu/drm/i915/i915_scheduler.h +++ b/drivers/gpu/drm/i915/i915_scheduler.h @@ -42,6 +42,8 @@ struct i915_execbuffer_params { uint32_t mask; int mode; struct intel_context *ctx; + int seqno; + struct drm_i915_gem_request *request; uint32_t scheduler_index; };