From patchwork Thu Aug 8 01:44:21 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniele Ceraolo Spurio X-Patchwork-Id: 11083067 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 0991F912 for ; Thu, 8 Aug 2019 01:44:47 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EC99028694 for ; Thu, 8 Aug 2019 01:44:46 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E06B828AE0; Thu, 8 Aug 2019 01:44:46 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 41BF328694 for ; Thu, 8 Aug 2019 01:44:46 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 604E46E77F; Thu, 8 Aug 2019 01:44:45 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 47F966E0A2 for ; Thu, 8 Aug 2019 01:44:43 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Aug 2019 18:44:43 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,358,1559545200"; d="scan'208";a="165538120" Received: from dceraolo-linux.fm.intel.com ([10.1.27.145]) by orsmga007.jf.intel.com with ESMTP; 07 Aug 2019 18:44:43 -0700 From: Daniele Ceraolo Spurio To: intel-gfx@lists.freedesktop.org Date: Wed, 7 Aug 2019 18:44:21 -0700 Message-Id: <20190808014423.20377-2-daniele.ceraolospurio@intel.com> X-Mailer: git-send-email 2.22.0 In-Reply-To: <20190808014423.20377-1-daniele.ceraolospurio@intel.com> References: <20190808014423.20377-1-daniele.ceraolospurio@intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [RFC 1/3] drm/i915: split out uncore_mmio_debug X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP Multiple uncore structures will share the debug infrastructure, so move it to a common place and add extra locking around it. Also, since we now have a separate object, it is cleaner to have dedicated functions working on the object to stop and restart the mmio debug. Apart from the cosmetic changes, this patch introduces 2 functional updates: - All calls to check_for_unclaimed_mmio will now return false when the debug is suspended, not just the ones that are active only when i915_modparams.mmio_debug is set. If we don't trust the result of the check while a user is doing mmio access then we shouldn't attempt the check anywhere. - i915_modparams.mmio_debug is not save/restored anymore around user access. The value is now never touched by the kernel while debug is disabled so no need for save/restore. v2: squash mmio_debug patches, restrict mmio_debug lock usage (Chris) Signed-off-by: Daniele Ceraolo Spurio Cc: Chris Wilson Reviewed-by: Chris Wilson --- drivers/gpu/drm/i915/i915_debugfs.c | 2 +- drivers/gpu/drm/i915/i915_drv.c | 3 +- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_uncore.c | 91 ++++++++++++++++++++--------- drivers/gpu/drm/i915/intel_uncore.h | 18 +++--- 5 files changed, 79 insertions(+), 36 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 3b15266c54fd..2310512111ab 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1129,7 +1129,7 @@ static int i915_forcewake_domains(struct seq_file *m, void *data) unsigned int tmp; seq_printf(m, "user.bypass_count = %u\n", - uncore->user_forcewake.count); + uncore->user_forcewake_count); for_each_fw_domain(fw_domain, uncore, tmp) seq_printf(m, "%s.wake_count = %u\n", diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 3480db36b63f..fbbff4a133ba 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -744,6 +744,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv) intel_device_info_subplatform_init(dev_priv); + intel_uncore_mmio_debug_init_early(&dev_priv->mmio_debug); intel_uncore_init_early(&dev_priv->uncore, dev_priv); spin_lock_init(&dev_priv->irq_lock); @@ -2044,7 +2045,7 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation) out: enable_rpm_wakeref_asserts(rpm); - if (!dev_priv->uncore.user_forcewake.count) + if (!dev_priv->uncore.user_forcewake_count) intel_runtime_pm_driver_release(rpm); return ret; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index c9476f24f5c1..13c27a75dca8 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1346,6 +1346,7 @@ struct drm_i915_private { resource_size_t stolen_usable_size; /* Total size minus reserved ranges */ struct intel_uncore uncore; + struct intel_uncore_mmio_debug mmio_debug; struct i915_virtual_gpu vgpu; diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 39e8710afdd9..9e583f13a9e4 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -34,6 +34,32 @@ #define __raw_posting_read(...) ((void)__raw_uncore_read32(__VA_ARGS__)) +void +intel_uncore_mmio_debug_init_early(struct intel_uncore_mmio_debug *mmio_debug) +{ + spin_lock_init(&mmio_debug->lock); + mmio_debug->unclaimed_mmio_check = 1; +} + +static void mmio_debug_suspend(struct intel_uncore_mmio_debug *mmio_debug) +{ + lockdep_assert_held(&mmio_debug->lock); + + /* Save and disable mmio debugging for the user bypass */ + if (!mmio_debug->suspend_count++) { + mmio_debug->saved_mmio_check = mmio_debug->unclaimed_mmio_check; + mmio_debug->unclaimed_mmio_check = 0; + } +} + +static void mmio_debug_resume(struct intel_uncore_mmio_debug *mmio_debug) +{ + lockdep_assert_held(&mmio_debug->lock); + + if (!--mmio_debug->suspend_count) + mmio_debug->unclaimed_mmio_check = mmio_debug->saved_mmio_check; +} + static const char * const forcewake_domain_names[] = { "render", "blitter", @@ -476,6 +502,11 @@ check_for_unclaimed_mmio(struct intel_uncore *uncore) { bool ret = false; + lockdep_assert_held(&uncore->debug->lock); + + if (uncore->debug->suspend_count) + return false; + if (intel_uncore_has_fpga_dbg_unclaimed(uncore)) ret |= fpga_check_for_unclaimed_mmio(uncore); @@ -608,17 +639,11 @@ void intel_uncore_forcewake_get(struct intel_uncore *uncore, void intel_uncore_forcewake_user_get(struct intel_uncore *uncore) { spin_lock_irq(&uncore->lock); - if (!uncore->user_forcewake.count++) { + if (!uncore->user_forcewake_count++) { intel_uncore_forcewake_get__locked(uncore, FORCEWAKE_ALL); - - /* Save and disable mmio debugging for the user bypass */ - uncore->user_forcewake.saved_mmio_check = - uncore->unclaimed_mmio_check; - uncore->user_forcewake.saved_mmio_debug = - i915_modparams.mmio_debug; - - uncore->unclaimed_mmio_check = 0; - i915_modparams.mmio_debug = 0; + spin_lock(&uncore->debug->lock); + mmio_debug_suspend(uncore->debug); + spin_unlock(&uncore->debug->lock); } spin_unlock_irq(&uncore->lock); } @@ -633,15 +658,14 @@ void intel_uncore_forcewake_user_get(struct intel_uncore *uncore) void intel_uncore_forcewake_user_put(struct intel_uncore *uncore) { spin_lock_irq(&uncore->lock); - if (!--uncore->user_forcewake.count) { - if (intel_uncore_unclaimed_mmio(uncore)) + if (!--uncore->user_forcewake_count) { + spin_lock(&uncore->debug->lock); + mmio_debug_resume(uncore->debug); + + if (check_for_unclaimed_mmio(uncore)) dev_info(uncore->i915->drm.dev, "Invalid mmio detected during user access\n"); - - uncore->unclaimed_mmio_check = - uncore->user_forcewake.saved_mmio_check; - i915_modparams.mmio_debug = - uncore->user_forcewake.saved_mmio_debug; + spin_unlock(&uncore->debug->lock); intel_uncore_forcewake_put__locked(uncore, FORCEWAKE_ALL); } @@ -1088,7 +1112,16 @@ unclaimed_reg_debug(struct intel_uncore *uncore, if (likely(!i915_modparams.mmio_debug)) return; + /* interrupts are disabled and re-enabled around uncore->lock usage */ + lockdep_assert_held(&uncore->lock); + + if (before) + spin_lock(&uncore->debug->lock); + __unclaimed_reg_debug(uncore, reg, read, before); + + if (!before) + spin_unlock(&uncore->debug->lock); } #define GEN2_READ_HEADER(x) \ @@ -1607,6 +1640,7 @@ void intel_uncore_init_early(struct intel_uncore *uncore, spin_lock_init(&uncore->lock); uncore->i915 = i915; uncore->rpm = &i915->runtime_pm; + uncore->debug = &i915->mmio_debug; } static void uncore_raw_init(struct intel_uncore *uncore) @@ -1632,7 +1666,6 @@ static int uncore_forcewake_init(struct intel_uncore *uncore) ret = intel_uncore_fw_domains_init(uncore); if (ret) return ret; - forcewake_early_sanitize(uncore, 0); if (IS_GEN_RANGE(i915, 6, 7)) { @@ -1681,8 +1714,6 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore) if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915)) uncore->flags |= UNCORE_HAS_FORCEWAKE; - uncore->unclaimed_mmio_check = 1; - if (!intel_uncore_has_forcewake(uncore)) { uncore_raw_init(uncore); } else { @@ -1707,7 +1738,7 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore) uncore->flags |= UNCORE_HAS_FIFO; /* clear out unclaimed reg detection bit */ - if (check_for_unclaimed_mmio(uncore)) + if (intel_uncore_unclaimed_mmio(uncore)) DRM_DEBUG("unclaimed mmio detected on uncore init, clearing\n"); return 0; @@ -1952,7 +1983,13 @@ int __intel_wait_for_register(struct intel_uncore *uncore, bool intel_uncore_unclaimed_mmio(struct intel_uncore *uncore) { - return check_for_unclaimed_mmio(uncore); + bool ret; + + spin_lock_irq(&uncore->debug->lock); + ret = check_for_unclaimed_mmio(uncore); + spin_unlock_irq(&uncore->debug->lock); + + return ret; } bool @@ -1960,24 +1997,24 @@ intel_uncore_arm_unclaimed_mmio_detection(struct intel_uncore *uncore) { bool ret = false; - spin_lock_irq(&uncore->lock); + spin_lock_irq(&uncore->debug->lock); - if (unlikely(uncore->unclaimed_mmio_check <= 0)) + if (unlikely(uncore->debug->unclaimed_mmio_check <= 0)) goto out; - if (unlikely(intel_uncore_unclaimed_mmio(uncore))) { + if (unlikely(check_for_unclaimed_mmio(uncore))) { if (!i915_modparams.mmio_debug) { DRM_DEBUG("Unclaimed register detected, " "enabling oneshot unclaimed register reporting. " "Please use i915.mmio_debug=N for more information.\n"); i915_modparams.mmio_debug++; } - uncore->unclaimed_mmio_check--; + uncore->debug->unclaimed_mmio_check--; ret = true; } out: - spin_unlock_irq(&uncore->lock); + spin_unlock_irq(&uncore->debug->lock); return ret; } diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h index e603d210a34d..414fc2cb0459 100644 --- a/drivers/gpu/drm/i915/intel_uncore.h +++ b/drivers/gpu/drm/i915/intel_uncore.h @@ -36,6 +36,13 @@ struct drm_i915_private; struct intel_runtime_pm; struct intel_uncore; +struct intel_uncore_mmio_debug { + spinlock_t lock; /** lock is also taken in irq contexts. */ + int unclaimed_mmio_check; + int saved_mmio_check; + u32 suspend_count; +}; + enum forcewake_domain_id { FW_DOMAIN_ID_RENDER = 0, FW_DOMAIN_ID_BLITTER, @@ -137,14 +144,9 @@ struct intel_uncore { u32 __iomem *reg_ack; } *fw_domain[FW_DOMAIN_ID_COUNT]; - struct { - unsigned int count; - - int saved_mmio_check; - int saved_mmio_debug; - } user_forcewake; + unsigned int user_forcewake_count; - int unclaimed_mmio_check; + struct intel_uncore_mmio_debug *debug; }; /* Iterate over initialised fw domains */ @@ -179,6 +181,8 @@ intel_uncore_has_fifo(const struct intel_uncore *uncore) return uncore->flags & UNCORE_HAS_FIFO; } +void +intel_uncore_mmio_debug_init_early(struct intel_uncore_mmio_debug *mmio_debug); void intel_uncore_init_early(struct intel_uncore *uncore, struct drm_i915_private *i915); int intel_uncore_init_mmio(struct intel_uncore *uncore);