From patchwork Fri Dec 11 13:20:07 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Harrison X-Patchwork-Id: 7829631 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 1439EBEEE1 for ; Fri, 11 Dec 2015 13:20:13 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id F42362042A for ; Fri, 11 Dec 2015 13:20:11 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id BE89520383 for ; Fri, 11 Dec 2015 13:20:10 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 393636EFD6; Fri, 11 Dec 2015 05:20:10 -0800 (PST) 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 09BD26EFD6 for ; Fri, 11 Dec 2015 05:20:09 -0800 (PST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga101.fm.intel.com with ESMTP; 11 Dec 2015 05:20:08 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,413,1444719600"; d="scan'208";a="869549116" Received: from johnharr-linux.isw.intel.com ([10.102.226.93]) by orsmga002.jf.intel.com with ESMTP; 11 Dec 2015 05:20:07 -0800 From: John.C.Harrison@Intel.com To: Intel-GFX@Lists.FreeDesktop.Org Date: Fri, 11 Dec 2015 13:20:07 +0000 Message-Id: <1449840007-22608-1-git-send-email-John.C.Harrison@Intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1448278774-31376-19-git-send-email-John.C.Harrison@Intel.com> References: <1448278774-31376-19-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 19/40] drm/i915: Added scheduler support to __wait_request() calls 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 The scheduler can cause batch buffers, and hence requests, to be submitted to the ring out of order and asynchronously to their submission to the driver. Thus at the point of waiting for the completion of a given request, it is not even guaranteed that the request has actually been sent to the hardware yet. Even it is has been sent, it is possible that it could be pre-empted and thus 'unsent'. This means that it is necessary to be able to submit requests to the hardware during the wait call itself. Unfortunately, while some callers of __wait_request() release the mutex lock first, others do not (and apparently can not). Hence there is the ability to deadlock as the wait stalls for submission but the asynchronous submission is stalled for the mutex lock. This change hooks the scheduler in to the __wait_request() code to ensure correct behaviour. That is, flush the target batch buffer through to the hardware and do not deadlock waiting for something that cannot currently be submitted. Instead, the wait call must return EAGAIN at least as far back as necessary to release the mutex lock and allow the scheduler's asynchronous processing to get in and handle the pre-emption operation and eventually (re-)submit the work. v3: Removed the explicit scheduler flush from i915_wait_request(). This is no longer necessary and was causing unintended changes to the scheduler priority level which broke a validation team test. Change-Id: I31fe6bc7e38f6ffdd843fcae16e7cc8b1e52a931 For: VIZ-1587 Signed-off-by: John Harrison --- drivers/gpu/drm/i915/i915_drv.h | 3 ++- drivers/gpu/drm/i915/i915_gem.c | 33 ++++++++++++++++++++++++++------- drivers/gpu/drm/i915/i915_scheduler.c | 20 ++++++++++++++++++++ drivers/gpu/drm/i915/i915_scheduler.h | 2 ++ drivers/gpu/drm/i915/intel_display.c | 5 +++-- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- 6 files changed, 54 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 9a67f7c..5ed600c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3029,7 +3029,8 @@ int __i915_wait_request(struct drm_i915_gem_request *req, unsigned reset_counter, bool interruptible, s64 *timeout, - struct intel_rps_client *rps); + struct intel_rps_client *rps, + bool is_locked); int __must_check i915_wait_request(struct drm_i915_gem_request *req); int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf); int __must_check diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 349ff58..784000b 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1207,7 +1207,8 @@ int __i915_wait_request(struct drm_i915_gem_request *req, unsigned reset_counter, bool interruptible, s64 *timeout, - struct intel_rps_client *rps) + struct intel_rps_client *rps, + bool is_locked) { struct intel_engine_cs *ring = i915_gem_request_get_ring(req); struct drm_device *dev = ring->dev; @@ -1217,8 +1218,10 @@ int __i915_wait_request(struct drm_i915_gem_request *req, DEFINE_WAIT(wait); unsigned long timeout_expire; s64 before, now; - int ret; + int ret = 0; + bool busy; + might_sleep(); WARN(!intel_irqs_enabled(dev_priv), "IRQs disabled"); if (i915_gem_request_completed(req)) @@ -1269,6 +1272,22 @@ int __i915_wait_request(struct drm_i915_gem_request *req, break; } + if (is_locked) { + /* If this request is being processed by the scheduler + * then it is unsafe to sleep with the mutex lock held + * as the scheduler may require the lock in order to + * progress the request. */ + if (i915_scheduler_is_request_tracked(req, NULL, &busy)) { + if (busy) { + ret = -EAGAIN; + break; + } + } + + /* If the request is not tracked by the scheduler then the + * regular test can be done. */ + } + if (i915_gem_request_completed(req)) { ret = 0; break; @@ -1455,7 +1474,7 @@ i915_wait_request(struct drm_i915_gem_request *req) ret = __i915_wait_request(req, atomic_read(&dev_priv->gpu_error.reset_counter), - interruptible, NULL, NULL); + interruptible, NULL, NULL, true); if (ret) return ret; @@ -1568,7 +1587,7 @@ i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj, mutex_unlock(&dev->struct_mutex); for (i = 0; ret == 0 && i < n; i++) ret = __i915_wait_request(requests[i], reset_counter, true, - NULL, rps); + NULL, rps, false); mutex_lock(&dev->struct_mutex); for (i = 0; i < n; i++) { @@ -3494,7 +3513,7 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file) if (ret == 0) ret = __i915_wait_request(req[i], reset_counter, true, args->timeout_ns > 0 ? &args->timeout_ns : NULL, - file->driver_priv); + file->driver_priv, false); i915_gem_request_unreference(req[i]); } return ret; @@ -3527,7 +3546,7 @@ __i915_gem_object_sync(struct drm_i915_gem_object *obj, atomic_read(&i915->gpu_error.reset_counter), i915->mm.interruptible, NULL, - &i915->rps.semaphores); + &i915->rps.semaphores, true); if (ret) return ret; @@ -4486,7 +4505,7 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file) if (target == NULL) return 0; - ret = __i915_wait_request(target, reset_counter, true, NULL, NULL); + ret = __i915_wait_request(target, reset_counter, true, NULL, NULL, false); if (ret == 0) queue_delayed_work(dev_priv->wq, &dev_priv->mm.retire_work, 0); diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c index f88c871..386f157 100644 --- a/drivers/gpu/drm/i915/i915_scheduler.c +++ b/drivers/gpu/drm/i915/i915_scheduler.c @@ -847,6 +847,26 @@ static int i915_scheduler_remove_dependent(struct i915_scheduler *scheduler, return 0; } +bool i915_scheduler_is_request_tracked(struct drm_i915_gem_request *req, + bool *completed, bool *busy) +{ + struct drm_i915_private *dev_priv = req->ring->dev->dev_private; + struct i915_scheduler *scheduler = dev_priv->scheduler; + + if (!scheduler) + return false; + + if (req->scheduler_qe == NULL) + return false; + + if (completed) + *completed = I915_SQS_IS_COMPLETE(req->scheduler_qe); + if (busy) + *busy = I915_SQS_IS_QUEUED(req->scheduler_qe); + + return true; +} + int i915_scheduler_closefile(struct drm_device *dev, struct drm_file *file) { struct i915_scheduler_queue_entry *node; diff --git a/drivers/gpu/drm/i915/i915_scheduler.h b/drivers/gpu/drm/i915/i915_scheduler.h index 54d87fb..c3e7ac6 100644 --- a/drivers/gpu/drm/i915/i915_scheduler.h +++ b/drivers/gpu/drm/i915/i915_scheduler.h @@ -93,5 +93,7 @@ int i915_scheduler_queue_execbuffer(struct i915_scheduler_queue_entry *q bool i915_scheduler_notify_request(struct drm_i915_gem_request *req); void i915_scheduler_wakeup(struct drm_device *dev); void i915_gem_scheduler_work_handler(struct work_struct *work); +bool i915_scheduler_is_request_tracked(struct drm_i915_gem_request *req, + bool *completed, bool *busy); #endif /* _I915_SCHEDULER_H_ */ diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c00bc50..58c464f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11258,7 +11258,8 @@ static void intel_mmio_flip_work_func(struct work_struct *work) WARN_ON(__i915_wait_request(mmio_flip->req, mmio_flip->crtc->reset_counter, false, NULL, - &mmio_flip->i915->rps.mmioflips)); + &mmio_flip->i915->rps.mmioflips, + false)); i915_gem_request_unreference(mmio_flip->req); } @@ -13258,7 +13259,7 @@ static int intel_atomic_prepare_commit(struct drm_device *dev, ret = __i915_wait_request(intel_plane_state->wait_req, reset_counter, true, - NULL, NULL); + NULL, NULL, false); /* Swallow -EIO errors to allow updates during hw lockup. */ if (ret == -EIO) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 34fbe68..4d77886 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2294,7 +2294,7 @@ int intel_ring_idle(struct intel_engine_cs *ring) return __i915_wait_request(req, atomic_read(&to_i915(ring->dev)->gpu_error.reset_counter), to_i915(ring->dev)->mm.interruptible, - NULL, NULL); + NULL, NULL, true); } int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request)