From patchwork Wed Jul 16 20:49:29 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paulo Zanoni X-Patchwork-Id: 4570351 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 18AAD9F26C for ; Wed, 16 Jul 2014 20:49:49 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id BB69F201C8 for ; Wed, 16 Jul 2014 20:49:45 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 9776D201C0 for ; Wed, 16 Jul 2014 20:49:44 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id F2B216E00C; Wed, 16 Jul 2014 13:49:43 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-vc0-f178.google.com (mail-vc0-f178.google.com [209.85.220.178]) by gabe.freedesktop.org (Postfix) with ESMTP id 7F5126E00C for ; Wed, 16 Jul 2014 13:49:43 -0700 (PDT) Received: by mail-vc0-f178.google.com with SMTP id la4so2819111vcb.23 for ; Wed, 16 Jul 2014 13:49:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=M7LxXStjR7rf1rhUBo33fqlyCVeVBKXSkU+xq2mxGog=; b=ie9ywtdfGqH+vcoIVDUOilGXHFysqN2y6Glf2k9PWbG5YYkE3QIFwcmfGj7gelZSLQ 8NEHJrOddfAaAXix4qmYF2OE1zfgT3UvYm7kB/rchI1UE00JsuzZRTt5yZ/331YV5sCo pukjKERVGw1g3ZYZA3eIEDWX9NTFg4nxjPzXf1EciqA2CJlmI7OVTLhvNF+MMT9yGbSM whvT62jzg7jOTlHdT/Jhyt3mrOSPcWVBGi7qaenAxE1kRCnVWDb3ZJ5y7M93tQavZJJF 7KKRMEbgCuJs0II7n+R29ZDNAn5zQ9I4P6K2Yrq9IgKV4YoM2BSXgU22V12izOFW/uN/ 5MJw== X-Received: by 10.220.73.137 with SMTP id q9mr11509848vcj.64.1405543782859; Wed, 16 Jul 2014 13:49:42 -0700 (PDT) Received: from localhost.localdomain ([177.132.8.34]) by mx.google.com with ESMTPSA id im7sm350882vec.14.2014.07.16.13.49.41 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 16 Jul 2014 13:49:42 -0700 (PDT) From: Paulo Zanoni To: intel-gfx@lists.freedesktop.org Date: Wed, 16 Jul 2014 17:49:29 -0300 Message-Id: <1405543770-3212-1-git-send-email-przanoni@gmail.com> X-Mailer: git-send-email 2.0.0 In-Reply-To: <20140716135707.GY15237@phenom.ffwll.local> References: <20140716135707.GY15237@phenom.ffwll.local> Cc: Paulo Zanoni Subject: [Intel-gfx] [PATCH 1/2] drm/i915: reorganize the unclaimed register detection code 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.1 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, T_DKIM_INVALID, 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: Paulo Zanoni The current code only runs when we do an I915_WRITE operation. It checks if the unclaimed register flag is set before we do the operation, and then it checks it again after we do the operation. This double check allows us to find out if the I915_WRITE operation in question is the bad one, or if some previous code is the bad one. When it finds a problem, our code uses DRM_ERROR to signal it. The good thing about the current code is that it detects the problem, so at least we can know we did something wrong. The problem is that even though we find the problem, we don't really have much information to actually debug it. So whenever I see one of these DRM_ERROR messages on my systems, the first thing I do is apply a patch to change the DRM_ERROR to a WARN and also check for unclaimed registers on I915_READ operations. This local patch makes things even slower, but it usually helps a lot in finding the bad code. The first point here is that since the current code is only useful to detect whether we have a problem or not, but it is not really good to find the cause of the problem, I don't think we should be checking both before and after every I915_WRITE operation: just doing the check once should be enough for us to quickly detect problems. With this change, the code that runs by default for every single user will only do 1 read operation for every single I915_WRITE, instead of 2. This patch does this change. The second point is that the local patch I have should be upstream, but since it makes things slower it should be disabled by default. So I added the i915.mmio_debug option to enable it. So after this patch, this is what will happen: - By default, we will try to detect unclaimed registers once after every I915_WRITE operation. Previously we tried twice for every I915_WRITE. - When we find an unclaimed register we will still print a DRM_ERROR message, but we will now tell the user to try again with i915.mmio_debug=1. - When we use i915.mmio_debug=1 we will try to find unclaimed registers both before and after every I915_READ and I915_WRITE operation, and we will print stack traces in case we find them. This should really help locating the exact point of the bad code (or at least finding out that i915.ko is not the problem). This commit also opens space for really-slow register debugging operations on other platforms. In theory we can now add lots and lots of debug code behind i915.mmio_debug, enable this option on our tests, and catch more problems. v2: - Remove not-so-useful comments (Daniel) - Fix the param definition macros (Rodrigo) Reviewed-by: Rodrigo Vivi Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_params.c | 6 ++++++ drivers/gpu/drm/i915/intel_uncore.c | 27 ++++++++++++++++++++------- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 991b663..ca0a9dd 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2135,6 +2135,7 @@ struct i915_params { bool disable_display; bool disable_vtd_wa; int use_mmio_flip; + bool mmio_debug; }; extern struct i915_params i915 __read_mostly; diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index bbdee21..62ee830 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -49,6 +49,7 @@ struct i915_params i915 __read_mostly = { .enable_cmd_parser = 1, .disable_vtd_wa = 0, .use_mmio_flip = 0, + .mmio_debug = 0, }; module_param_named(modeset, i915.modeset, int, 0400); @@ -161,3 +162,8 @@ MODULE_PARM_DESC(enable_cmd_parser, module_param_named(use_mmio_flip, i915.use_mmio_flip, int, 0600); MODULE_PARM_DESC(use_mmio_flip, "use MMIO flips (-1=never, 0=driver discretion [default], 1=always)"); + +module_param_named(mmio_debug, i915.mmio_debug, bool, 0600); +MODULE_PARM_DESC(mmio_debug, + "Enable the MMIO debug code (default: false). This may negatively " + "affect performance."); diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index e0f0843..6fee122 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -514,20 +514,30 @@ ilk_dummy_write(struct drm_i915_private *dev_priv) } static void -hsw_unclaimed_reg_clear(struct drm_i915_private *dev_priv, u32 reg) +hsw_unclaimed_reg_debug(struct drm_i915_private *dev_priv, u32 reg, bool read, + bool before) { + const char *op = read ? "reading" : "writing to"; + const char *when = before ? "before" : "after"; + + if (!i915.mmio_debug) + return; + if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) { - DRM_ERROR("Unknown unclaimed register before writing to %x\n", - reg); + WARN(1, "Unclaimed register detected %s %s register 0x%x\n", + when, op, reg); __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM); } } static void -hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg) +hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv) { + if (i915.mmio_debug) + return; + if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) { - DRM_ERROR("Unclaimed write to %x\n", reg); + DRM_ERROR("Unclaimed register detected. Please use the i915.mmio_debug=1 to debug this problem."); __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM); } } @@ -564,6 +574,7 @@ gen5_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \ static u##x \ gen6_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \ REG_READ_HEADER(x); \ + hsw_unclaimed_reg_debug(dev_priv, reg, true, true); \ if (dev_priv->uncore.forcewake_count == 0 && \ NEEDS_FORCE_WAKE((dev_priv), (reg))) { \ dev_priv->uncore.funcs.force_wake_get(dev_priv, \ @@ -574,6 +585,7 @@ gen6_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \ } else { \ val = __raw_i915_read##x(dev_priv, reg); \ } \ + hsw_unclaimed_reg_debug(dev_priv, reg, true, false); \ REG_READ_FOOTER; \ } @@ -700,12 +712,13 @@ hsw_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \ __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \ } \ - hsw_unclaimed_reg_clear(dev_priv, reg); \ + hsw_unclaimed_reg_debug(dev_priv, reg, false, true); \ __raw_i915_write##x(dev_priv, reg, val); \ if (unlikely(__fifo_ret)) { \ gen6_gt_check_fifodbg(dev_priv); \ } \ - hsw_unclaimed_reg_check(dev_priv, reg); \ + hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \ + hsw_unclaimed_reg_detect(dev_priv); \ REG_WRITE_FOOTER; \ }