From patchwork Wed Nov 25 19:29:19 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: yu.dai@intel.com X-Patchwork-Id: 7702641 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 231D7BF90C for ; Wed, 25 Nov 2015 19:31:56 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 1EA182085B for ; Wed, 25 Nov 2015 19:31:55 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 08E6920850 for ; Wed, 25 Nov 2015 19:31:54 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 7E1C6721C0; Wed, 25 Nov 2015 11:31:52 -0800 (PST) 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 1BFBA6E3A9 for ; Wed, 25 Nov 2015 11:31:51 -0800 (PST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP; 25 Nov 2015 11:31:50 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,343,1444719600"; d="scan'208";a="847059894" Received: from alex-hsw.fm.intel.com ([10.19.83.10]) by fmsmga001.fm.intel.com with ESMTP; 25 Nov 2015 11:31:50 -0800 From: yu.dai@intel.com To: intel-gfx@lists.freedesktop.org Date: Wed, 25 Nov 2015 11:29:19 -0800 Message-Id: <1448479759-22734-1-git-send-email-yu.dai@intel.com> X-Mailer: git-send-email 2.5.0 In-Reply-To: <1448319778-29282-1-git-send-email-yu.dai@intel.com> References: <1448319778-29282-1-git-send-email-yu.dai@intel.com> Subject: [Intel-gfx] [PATCH v2] drm/i915/guc: Clean up locks in GuC 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.8 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, 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: Alex Dai When GuC Work Queue is full, driver will wait GuC for avaliable space by delaying 1ms. The wait needs to be out of spinlockirq / unlock. Otherwise, lockup happens because jiffies won't be updated dur to irq is disabled. The unnecessary locks has been cleared. dev->struct_mutex is used instead where needed. Issue is found in igt/gem_close_race. v2: Clean up wq_lock too v1: Clean up host2guc lock as well Signed-off-by: Alex Dai --- drivers/gpu/drm/i915/i915_debugfs.c | 12 +++++------ drivers/gpu/drm/i915/i915_guc_submission.c | 32 +++++++----------------------- drivers/gpu/drm/i915/intel_guc.h | 4 ---- 3 files changed, 13 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index a728ff1..d6b7817 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2473,15 +2473,15 @@ static int i915_guc_info(struct seq_file *m, void *data) if (!HAS_GUC_SCHED(dev_priv->dev)) return 0; + if (mutex_lock_interruptible(&dev->struct_mutex)) + return 0; + /* Take a local copy of the GuC data, so we can dump it at leisure */ - spin_lock(&dev_priv->guc.host2guc_lock); guc = dev_priv->guc; - if (guc.execbuf_client) { - spin_lock(&guc.execbuf_client->wq_lock); + if (guc.execbuf_client) client = *guc.execbuf_client; - spin_unlock(&guc.execbuf_client->wq_lock); - } - spin_unlock(&dev_priv->guc.host2guc_lock); + + mutex_unlock(&dev->struct_mutex); seq_printf(m, "GuC total action count: %llu\n", guc.action_count); seq_printf(m, "GuC action failure count: %u\n", guc.action_fail); diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index ed9f100..97996e5 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -86,7 +86,6 @@ static int host2guc_action(struct intel_guc *guc, u32 *data, u32 len) return -EINVAL; intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL); - spin_lock(&dev_priv->guc.host2guc_lock); dev_priv->guc.action_count += 1; dev_priv->guc.action_cmd = data[0]; @@ -119,7 +118,6 @@ static int host2guc_action(struct intel_guc *guc, u32 *data, u32 len) } dev_priv->guc.action_status = status; - spin_unlock(&dev_priv->guc.host2guc_lock); intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); return ret; @@ -249,6 +247,7 @@ static int guc_ring_doorbell(struct i915_guc_client *gc) } kunmap_atomic(base); + return ret; } @@ -292,16 +291,12 @@ static uint32_t select_doorbell_cacheline(struct intel_guc *guc) const uint32_t cacheline_size = cache_line_size(); uint32_t offset; - spin_lock(&guc->host2guc_lock); - /* Doorbell uses a single cache line within a page */ offset = offset_in_page(guc->db_cacheline); /* Moving to next cache line to reduce contention */ guc->db_cacheline += cacheline_size; - spin_unlock(&guc->host2guc_lock); - DRM_DEBUG_DRIVER("selected doorbell cacheline 0x%x, next 0x%x, linesize %u\n", offset, guc->db_cacheline, cacheline_size); @@ -322,13 +317,11 @@ static uint16_t assign_doorbell(struct intel_guc *guc, uint32_t priority) const uint16_t end = start + half; uint16_t id; - spin_lock(&guc->host2guc_lock); id = find_next_zero_bit(guc->doorbell_bitmap, end, start); if (id == end) id = GUC_INVALID_DOORBELL_ID; else bitmap_set(guc->doorbell_bitmap, id, 1); - spin_unlock(&guc->host2guc_lock); DRM_DEBUG_DRIVER("assigned %s priority doorbell id 0x%x\n", hi_pri ? "high" : "normal", id); @@ -338,9 +331,7 @@ static uint16_t assign_doorbell(struct intel_guc *guc, uint32_t priority) static void release_doorbell(struct intel_guc *guc, uint16_t id) { - spin_lock(&guc->host2guc_lock); bitmap_clear(guc->doorbell_bitmap, id, 1); - spin_unlock(&guc->host2guc_lock); } /* @@ -487,16 +478,13 @@ static int guc_get_workqueue_space(struct i915_guc_client *gc, u32 *offset) struct guc_process_desc *desc; void *base; u32 size = sizeof(struct guc_wq_item); - int ret = 0, timeout_counter = 200; + int ret = -ETIMEDOUT, timeout_counter = 200; base = kmap_atomic(i915_gem_object_get_page(gc->client_obj, 0)); desc = base + gc->proc_desc_offset; while (timeout_counter-- > 0) { - ret = wait_for_atomic(CIRC_SPACE(gc->wq_tail, desc->head, - gc->wq_size) >= size, 1); - - if (!ret) { + if (CIRC_SPACE(gc->wq_tail, desc->head, gc->wq_size) >= size) { *offset = gc->wq_tail; /* advance the tail for next workqueue item */ @@ -505,7 +493,11 @@ static int guc_get_workqueue_space(struct i915_guc_client *gc, u32 *offset) /* this will break the loop */ timeout_counter = 0; + ret = 0; } + + if (timeout_counter) + usleep_range(1000, 2000); }; kunmap_atomic(base); @@ -597,15 +589,12 @@ int i915_guc_submit(struct i915_guc_client *client, { struct intel_guc *guc = client->guc; enum intel_ring_id ring_id = rq->ring->id; - unsigned long flags; int q_ret, b_ret; /* Need this because of the deferred pin ctx and ring */ /* Shall we move this right after ring is pinned? */ lr_context_update(rq); - spin_lock_irqsave(&client->wq_lock, flags); - q_ret = guc_add_workqueue_item(client, rq); if (q_ret == 0) b_ret = guc_ring_doorbell(client); @@ -620,12 +609,8 @@ int i915_guc_submit(struct i915_guc_client *client, } else { client->retcode = 0; } - spin_unlock_irqrestore(&client->wq_lock, flags); - - spin_lock(&guc->host2guc_lock); guc->submissions[ring_id] += 1; guc->last_seqno[ring_id] = rq->seqno; - spin_unlock(&guc->host2guc_lock); return q_ret; } @@ -768,7 +753,6 @@ static struct i915_guc_client *guc_client_alloc(struct drm_device *dev, client->client_obj = obj; client->wq_offset = GUC_DB_SIZE; client->wq_size = GUC_WQ_SIZE; - spin_lock_init(&client->wq_lock); client->doorbell_offset = select_doorbell_cacheline(guc); @@ -871,8 +855,6 @@ int i915_guc_submission_init(struct drm_device *dev) if (!guc->ctx_pool_obj) return -ENOMEM; - spin_lock_init(&dev_priv->guc.host2guc_lock); - ida_init(&guc->ctx_ids); guc_create_log(guc); diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h index 5ba5866..8229522 100644 --- a/drivers/gpu/drm/i915/intel_guc.h +++ b/drivers/gpu/drm/i915/intel_guc.h @@ -42,8 +42,6 @@ struct i915_guc_client { uint32_t wq_offset; uint32_t wq_size; - - spinlock_t wq_lock; /* Protects all data below */ uint32_t wq_tail; /* GuC submission statistics & status */ @@ -95,8 +93,6 @@ struct intel_guc { struct i915_guc_client *execbuf_client; - spinlock_t host2guc_lock; /* Protects all data below */ - DECLARE_BITMAP(doorbell_bitmap, GUC_MAX_DOORBELLS); uint32_t db_cacheline; /* Cyclic counter mod pagesize */