From 8a47eb198d6e59e5d2509324287d0d913b2d5af1 Mon Sep 17 00:00:00 2001 From: Mika Kuoppala Date: Tue, 15 Dec 2015 19:24:47 +0200 Subject: drm/i915: Consolidate unclaimed mmio detection Access the unclaimed reg detection register through one helper which also does cleanup. Note that we now access the register only if the platform has the actual non claimed access bit. This prevents reading the register with gens that doesn't have the register or the unclaimed bit, when debug_mmio > 0. Note that we post after clearing the bit. This makes sure that the next unclaimed write access would get detected also if it happened right after clearing, and not fold into the previous detection. v2: s/unclaimed_reg_access/check_for_unclaimed_mmio (Chris) debug log on unclaimed detection on uncore init (Joonas) v3: remove posting read (Ville) Cc: Joonas Lahtinen Cc: Chris Wilson Cc: Paulo Zanoni Reviewed-by: Chris Wilson Signed-off-by: Mika Kuoppala Link: http://patchwork.freedesktop.org/patch/msgid/1450200287-24080-1-git-send-email-mika.kuoppala@intel.com --- drivers/gpu/drm/i915/intel_uncore.c | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_uncore.c') diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 277e60ae0e47..358cb9cb219e 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -327,13 +327,31 @@ static void intel_uncore_ellc_detect(struct drm_device *dev) } } +static bool +check_for_unclaimed_mmio(struct drm_i915_private *dev_priv) +{ + u32 dbg; + + if (!HAS_FPGA_DBG_UNCLAIMED(dev_priv)) + return false; + + dbg = __raw_i915_read32(dev_priv, FPGA_DBG); + if (likely(!(dbg & FPGA_DBG_RM_NOCLAIM))) + return false; + + __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM); + + return true; +} + static void __intel_uncore_early_sanitize(struct drm_device *dev, bool restore_forcewake) { struct drm_i915_private *dev_priv = dev->dev_private; - if (HAS_FPGA_DBG_UNCLAIMED(dev)) - __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM); + /* clear out unclaimed reg detection bit */ + if (check_for_unclaimed_mmio(dev_priv)) + DRM_DEBUG("unclaimed mmio detected on uncore init, clearing\n"); /* clear out old GT FIFO errors */ if (IS_GEN6(dev) || IS_GEN7(dev)) @@ -594,10 +612,9 @@ hsw_unclaimed_reg_debug(struct drm_i915_private *dev_priv, if (!i915.mmio_debug) return; - if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) { + if (check_for_unclaimed_mmio(dev_priv)) { WARN(1, "Unclaimed register detected %s %s register 0x%x\n", when, op, i915_mmio_reg_offset(reg)); - __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM); i915.mmio_debug--; /* Only report the first N failures */ } } @@ -610,11 +627,10 @@ hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv) if (i915.mmio_debug || !mmio_debug_once) return; - if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) { + if (check_for_unclaimed_mmio(dev_priv)) { DRM_DEBUG("Unclaimed register detected, " "enabling oneshot unclaimed register reporting. " "Please use i915.mmio_debug=N for more information.\n"); - __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM); i915.mmio_debug = mmio_debug_once--; } } @@ -1582,11 +1598,6 @@ bool intel_has_gpu_reset(struct drm_device *dev) void intel_uncore_check_errors(struct drm_device *dev) { - struct drm_i915_private *dev_priv = dev->dev_private; - - if (HAS_FPGA_DBG_UNCLAIMED(dev) && - (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM)) { + if (check_for_unclaimed_mmio(to_i915(dev))) DRM_ERROR("Unclaimed register before interrupt\n"); - __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM); - } } -- cgit v1.2.3 From fc97618bf373b496858eca463e5154085835ae41 Mon Sep 17 00:00:00 2001 From: Mika Kuoppala Date: Tue, 15 Dec 2015 16:25:07 +0200 Subject: drm/i915: Introduce intel_uncore_unclaimed_mmio Currently interrupt code is the only place checking for the unclaimed register access prior to actual register macros using the same functionality. Rename the function and make it return bool so that the possible error message context is clear in the caller side. The motivation is to allow usage of unclaimed detection on arbitrary places. v2: rebase, s/access/mmio, s/dev/dev_priv Reviewed-by: Chris Wilson Signed-off-by: Mika Kuoppala Link: http://patchwork.freedesktop.org/patch/msgid/1450189512-30360-2-git-send-email-mika.kuoppala@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/i915_irq.c | 3 ++- drivers/gpu/drm/i915/intel_uncore.c | 5 ++--- 3 files changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_uncore.c') diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 8cf655c6fc03..aa32e8872a95 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2717,7 +2717,7 @@ extern void intel_uncore_sanitize(struct drm_device *dev); extern void intel_uncore_early_sanitize(struct drm_device *dev, bool restore_forcewake); extern void intel_uncore_init(struct drm_device *dev); -extern void intel_uncore_check_errors(struct drm_device *dev); +extern bool intel_uncore_unclaimed_mmio(struct drm_i915_private *dev_priv); extern void intel_uncore_fini(struct drm_device *dev); extern void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore); const char *intel_uncore_forcewake_domain_to_str(const enum forcewake_domain_id id); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index b775decf9d27..e7e44a972905 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2190,7 +2190,8 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg) /* We get interrupts on unclaimed registers, so check for this before we * do any I915_{READ,WRITE}. */ - intel_uncore_check_errors(dev); + if (intel_uncore_unclaimed_mmio(dev_priv)) + DRM_ERROR("Unclaimed register before interrupt\n"); /* disable master interrupt before clearing iir */ de_ier = I915_READ(DEIER); diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 358cb9cb219e..7ab5916f90dd 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -1596,8 +1596,7 @@ bool intel_has_gpu_reset(struct drm_device *dev) return intel_get_gpu_reset(dev) != NULL; } -void intel_uncore_check_errors(struct drm_device *dev) +bool intel_uncore_unclaimed_mmio(struct drm_i915_private *dev_priv) { - if (check_for_unclaimed_mmio(to_i915(dev))) - DRM_ERROR("Unclaimed register before interrupt\n"); + return check_for_unclaimed_mmio(dev_priv); } -- cgit v1.2.3 From 7571494004d88249bcee2a20fa74cb50753676fe Mon Sep 17 00:00:00 2001 From: Mika Kuoppala Date: Wed, 16 Dec 2015 09:26:48 +0200 Subject: drm/i915: Do one shot unclaimed mmio detection less frequently We have done unclaimed register access check in normal (mmio_debug=0) mode once per write. This adds probability of finding the exact sequence where we did the bad access, but also adds burden to each write. As we have mmio_debug available for more fine grained analysis, give up accuracy of detecting correct spot at the first occurrence by doing the one shot detection and arming of mmio_debug in hangcheck and in modeset. This removes the write path performance burden. v2: Remove gratuitous DRM_DEBUG and return value, comments (Chris) Cc: Chris Wilson Cc: Paulo Zanoni Reviewed-by: Chris Wilson Signed-off-by: Mika Kuoppala Link: http://patchwork.freedesktop.org/patch/msgid/1450250808-14864-1-git-send-email-mika.kuoppala@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/i915_irq.c | 11 ++++++----- drivers/gpu/drm/i915/intel_display.c | 13 +++++++++++++ drivers/gpu/drm/i915/intel_uncore.c | 37 ++++++++++++++++++------------------ 4 files changed, 40 insertions(+), 24 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_uncore.c') diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index aa32e8872a95..6ef03eea3518 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -718,6 +718,8 @@ struct intel_uncore { i915_reg_t reg_post; u32 val_reset; } fw_domain[FW_DOMAIN_ID_COUNT]; + + int unclaimed_mmio_check; }; /* Iterate over initialised fw domains */ @@ -2718,6 +2720,7 @@ extern void intel_uncore_early_sanitize(struct drm_device *dev, bool restore_forcewake); extern void intel_uncore_init(struct drm_device *dev); extern bool intel_uncore_unclaimed_mmio(struct drm_i915_private *dev_priv); +extern void intel_uncore_arm_unclaimed_mmio_detection(struct drm_i915_private *dev_priv); extern void intel_uncore_fini(struct drm_device *dev); extern void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore); const char *intel_uncore_forcewake_domain_to_str(const enum forcewake_domain_id id); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index e7e44a972905..f04d799153ca 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2188,11 +2188,6 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg) /* IRQs are synced during runtime_suspend, we don't require a wakeref */ disable_rpm_wakeref_asserts(dev_priv); - /* We get interrupts on unclaimed registers, so check for this before we - * do any I915_{READ,WRITE}. */ - if (intel_uncore_unclaimed_mmio(dev_priv)) - DRM_ERROR("Unclaimed register before interrupt\n"); - /* disable master interrupt before clearing iir */ de_ier = I915_READ(DEIER); I915_WRITE(DEIER, de_ier & ~DE_MASTER_IRQ_CONTROL); @@ -3081,6 +3076,12 @@ static void i915_hangcheck_elapsed(struct work_struct *work) */ DISABLE_RPM_WAKEREF_ASSERTS(dev_priv); + /* As enabling the GPU requires fairly extensive mmio access, + * periodically arm the mmio checker to see if we are triggering + * any invalid access. + */ + intel_uncore_arm_unclaimed_mmio_detection(dev_priv); + for_each_ring(ring, dev_priv, i) { u64 acthd; u32 seqno; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 738d0dca47a8..3edf22ad7673 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13684,6 +13684,19 @@ static int intel_atomic_commit(struct drm_device *dev, drm_atomic_state_free(state); + /* As one of the primary mmio accessors, KMS has a high likelihood + * of triggering bugs in unclaimed access. After we finish + * modesetting, see if an error has been flagged, and if so + * enable debugging for the next modeset - and hope we catch + * the culprit. + * + * XXX note that we assume display power is on at this point. + * This might hold true now but we need to add pm helper to check + * unclaimed only when the hardware is on, as atomic commits + * can happen also when the device is completely off. + */ + intel_uncore_arm_unclaimed_mmio_detection(dev_priv); + return 0; } diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 7ab5916f90dd..c7af339a6366 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -619,22 +619,6 @@ hsw_unclaimed_reg_debug(struct drm_i915_private *dev_priv, } } -static void -hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv) -{ - static bool mmio_debug_once = true; - - if (i915.mmio_debug || !mmio_debug_once) - return; - - if (check_for_unclaimed_mmio(dev_priv)) { - DRM_DEBUG("Unclaimed register detected, " - "enabling oneshot unclaimed register reporting. " - "Please use i915.mmio_debug=N for more information.\n"); - i915.mmio_debug = mmio_debug_once--; - } -} - #define GEN2_READ_HEADER(x) \ u##x val = 0; \ assert_rpm_wakelock_held(dev_priv); @@ -914,7 +898,6 @@ hsw_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool t gen6_gt_check_fifodbg(dev_priv); \ } \ hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \ - hsw_unclaimed_reg_detect(dev_priv); \ GEN6_WRITE_FOOTER; \ } @@ -949,7 +932,6 @@ gen8_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool __force_wake_get(dev_priv, FORCEWAKE_RENDER); \ __raw_i915_write##x(dev_priv, reg, val); \ hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \ - hsw_unclaimed_reg_detect(dev_priv); \ GEN6_WRITE_FOOTER; \ } @@ -1019,7 +1001,6 @@ gen9_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, \ __force_wake_get(dev_priv, fw_engine); \ __raw_i915_write##x(dev_priv, reg, val); \ hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \ - hsw_unclaimed_reg_detect(dev_priv); \ GEN6_WRITE_FOOTER; \ } @@ -1239,6 +1220,8 @@ void intel_uncore_init(struct drm_device *dev) intel_uncore_fw_domains_init(dev); __intel_uncore_early_sanitize(dev, false); + dev_priv->uncore.unclaimed_mmio_check = 1; + switch (INTEL_INFO(dev)->gen) { default: case 9: @@ -1600,3 +1583,19 @@ bool intel_uncore_unclaimed_mmio(struct drm_i915_private *dev_priv) { return check_for_unclaimed_mmio(dev_priv); } + +void +intel_uncore_arm_unclaimed_mmio_detection(struct drm_i915_private *dev_priv) +{ + if (unlikely(i915.mmio_debug || + dev_priv->uncore.unclaimed_mmio_check <= 0)) + return; + + if (unlikely(intel_uncore_unclaimed_mmio(dev_priv))) { + DRM_DEBUG("Unclaimed register detected, " + "enabling oneshot unclaimed register reporting. " + "Please use i915.mmio_debug=N for more information.\n"); + i915.mmio_debug++; + dev_priv->uncore.unclaimed_mmio_check--; + } +} -- cgit v1.2.3 From 4bd0a25d9be2963a15e16126022214e88580103f Mon Sep 17 00:00:00 2001 From: Mika Kuoppala Date: Tue, 15 Dec 2015 16:25:10 +0200 Subject: drm/i915: Streamline unclaimed reg debug trace Remove char* assignments and add branching hint and also constify the parameters. This results in a 35 bytes shorter fast path, so author boldly assumes it helps without doing in-depth assembly analysis. v2: use WARN's branching (Chris), commit name (Joonas) Cc: Joonas Lahtinen Cc: Chris Wilson Reviewed-by: Chris Wilson Signed-off-by: Mika Kuoppala Link: http://patchwork.freedesktop.org/patch/msgid/1450189512-30360-5-git-send-email-mika.kuoppala@intel.com --- drivers/gpu/drm/i915/intel_uncore.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_uncore.c') diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index c7af339a6366..bd0933cefe5d 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -604,19 +604,19 @@ ilk_dummy_write(struct drm_i915_private *dev_priv) static void hsw_unclaimed_reg_debug(struct drm_i915_private *dev_priv, - i915_reg_t reg, bool read, bool before) + const i915_reg_t reg, + const bool read, + const bool before) { - const char *op = read ? "reading" : "writing to"; - const char *when = before ? "before" : "after"; - - if (!i915.mmio_debug) + if (likely(!i915.mmio_debug)) return; - if (check_for_unclaimed_mmio(dev_priv)) { - WARN(1, "Unclaimed register detected %s %s register 0x%x\n", - when, op, i915_mmio_reg_offset(reg)); + if (WARN(check_for_unclaimed_mmio(dev_priv), + "Unclaimed register detected %s %s register 0x%x\n", + before ? "before" : "after", + read ? "reading" : "writing to", + i915_mmio_reg_offset(reg))) i915.mmio_debug--; /* Only report the first N failures */ - } } #define GEN2_READ_HEADER(x) \ -- cgit v1.2.3 From 8ac3e1bb76cc2e1585be3a5fa7551480e69487c8 Mon Sep 17 00:00:00 2001 From: Mika Kuoppala Date: Tue, 15 Dec 2015 19:45:42 +0200 Subject: drm/i915: Add non claimed mmio checking for vlv/chv MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Imre mentioned that chv might also have capability to track unclaimed mmio accesses. Ville added that both chv and vlv has this capability and he had already made this way back [1]. Mimic what Ville's patch does but adapt on top of less frequent mmio accesses by omitting checking always on reg writes. This patch is untested as of now. v2: overflow handling and posting omitted (Ville) References: [1] http://lists.freedesktop.org/archives/intel-gfx/2013-May/027599.html Cc: Imre Deak Cc: Ville Syrjälä Reviewed-by: Ville Syrjälä Signed-off-by: Mika Kuoppala Link: http://patchwork.freedesktop.org/patch/msgid/1450201542-22918-1-git-send-email-mika.kuoppala@intel.com --- drivers/gpu/drm/i915/i915_reg.h | 5 +++++ drivers/gpu/drm/i915/intel_uncore.c | 31 +++++++++++++++++++++++++++---- 2 files changed, 32 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_uncore.c') diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 7510d508e373..556a458d669e 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1711,6 +1711,11 @@ enum skl_disp_power_wells { #define FPGA_DBG _MMIO(0x42300) #define FPGA_DBG_RM_NOCLAIM (1<<31) +#define CLAIM_ER _MMIO(VLV_DISPLAY_BASE + 0x2028) +#define CLAIM_ER_CLR (1 << 31) +#define CLAIM_ER_OVERFLOW (1 << 16) +#define CLAIM_ER_CTR_MASK 0xffff + #define DERRMR _MMIO(0x44050) /* Note that HBLANK events are reserved on bdw+ */ #define DERRMR_PIPEA_SCANLINE (1<<0) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index bd0933cefe5d..d0973e08e7eb 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -328,13 +328,10 @@ static void intel_uncore_ellc_detect(struct drm_device *dev) } static bool -check_for_unclaimed_mmio(struct drm_i915_private *dev_priv) +fpga_check_for_unclaimed_mmio(struct drm_i915_private *dev_priv) { u32 dbg; - if (!HAS_FPGA_DBG_UNCLAIMED(dev_priv)) - return false; - dbg = __raw_i915_read32(dev_priv, FPGA_DBG); if (likely(!(dbg & FPGA_DBG_RM_NOCLAIM))) return false; @@ -344,6 +341,32 @@ check_for_unclaimed_mmio(struct drm_i915_private *dev_priv) return true; } +static bool +vlv_check_for_unclaimed_mmio(struct drm_i915_private *dev_priv) +{ + u32 cer; + + cer = __raw_i915_read32(dev_priv, CLAIM_ER); + if (likely(!(cer & (CLAIM_ER_OVERFLOW | CLAIM_ER_CTR_MASK)))) + return false; + + __raw_i915_write32(dev_priv, CLAIM_ER, CLAIM_ER_CLR); + + return true; +} + +static bool +check_for_unclaimed_mmio(struct drm_i915_private *dev_priv) +{ + if (HAS_FPGA_DBG_UNCLAIMED(dev_priv)) + return fpga_check_for_unclaimed_mmio(dev_priv); + + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) + return vlv_check_for_unclaimed_mmio(dev_priv); + + return false; +} + static void __intel_uncore_early_sanitize(struct drm_device *dev, bool restore_forcewake) { -- cgit v1.2.3 From 9c053501b950cf4dcea3dbbcd42da65e376acce6 Mon Sep 17 00:00:00 2001 From: Mika Kuoppala Date: Fri, 8 Jan 2016 15:51:19 +0200 Subject: drm/i915: Enable mmio_debug for vlv/chv MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With commit 8ac3e1bb76cc ("drm/i915: Add non claimed mmio checking for vlv/chv") we now have chv/vlv support in place for detecting unclaimed access. Also the perf hit of extra mmio read is now only suffered if mmio_debug is set. This allows us to stuff the macro for unclaimed reg detection inside a generic gen6 register access, as now all gens using these macros uses also unclaimed debugs, the one exception being snb. We gain more clean and generic macros and only downside is that snb will suffer one branch perf hit without upside. Note that the hsw write path debug register check now happens before fifo check, but this should not make any real difference. As vlv/chv use the generic gen6 access macros, the consequence is that they gain the mmio_debug feature. Cc: Ville Syrjälä Cc: Chris Wilson Signed-off-by: Mika Kuoppala Reviewed-by: Chris Wilson Link: http://patchwork.freedesktop.org/patch/msgid/1452261080-6979-1-git-send-email-mika.kuoppala@intel.com --- drivers/gpu/drm/i915/intel_uncore.c | 41 ++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 19 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_uncore.c') diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index d0973e08e7eb..0b47bc891dc7 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -626,14 +626,11 @@ ilk_dummy_write(struct drm_i915_private *dev_priv) } static void -hsw_unclaimed_reg_debug(struct drm_i915_private *dev_priv, - const i915_reg_t reg, - const bool read, - const bool before) +__unclaimed_reg_debug(struct drm_i915_private *dev_priv, + const i915_reg_t reg, + const bool read, + const bool before) { - if (likely(!i915.mmio_debug)) - return; - if (WARN(check_for_unclaimed_mmio(dev_priv), "Unclaimed register detected %s %s register 0x%x\n", before ? "before" : "after", @@ -642,6 +639,18 @@ hsw_unclaimed_reg_debug(struct drm_i915_private *dev_priv, i915.mmio_debug--; /* Only report the first N failures */ } +static inline void +unclaimed_reg_debug(struct drm_i915_private *dev_priv, + const i915_reg_t reg, + const bool read, + const bool before) +{ + if (likely(!i915.mmio_debug)) + return; + + __unclaimed_reg_debug(dev_priv, reg, read, before); +} + #define GEN2_READ_HEADER(x) \ u##x val = 0; \ assert_rpm_wakelock_held(dev_priv); @@ -687,9 +696,11 @@ __gen2_read(64) unsigned long irqflags; \ u##x val = 0; \ assert_rpm_wakelock_held(dev_priv); \ - spin_lock_irqsave(&dev_priv->uncore.lock, irqflags) + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); \ + unclaimed_reg_debug(dev_priv, reg, true, true) #define GEN6_READ_FOOTER \ + unclaimed_reg_debug(dev_priv, reg, true, false); \ spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); \ trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \ return val @@ -722,11 +733,9 @@ static inline void __force_wake_get(struct drm_i915_private *dev_priv, static u##x \ gen6_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \ GEN6_READ_HEADER(x); \ - hsw_unclaimed_reg_debug(dev_priv, reg, true, true); \ if (NEEDS_FORCE_WAKE(offset)) \ __force_wake_get(dev_priv, FORCEWAKE_RENDER); \ val = __raw_i915_read##x(dev_priv, reg); \ - hsw_unclaimed_reg_debug(dev_priv, reg, true, false); \ GEN6_READ_FOOTER; \ } @@ -774,7 +783,6 @@ static u##x \ gen9_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \ enum forcewake_domains fw_engine; \ GEN6_READ_HEADER(x); \ - hsw_unclaimed_reg_debug(dev_priv, reg, true, true); \ if (!SKL_NEEDS_FORCE_WAKE(offset)) \ fw_engine = 0; \ else if (FORCEWAKE_GEN9_RENDER_RANGE_OFFSET(offset)) \ @@ -788,7 +796,6 @@ gen9_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \ if (fw_engine) \ __force_wake_get(dev_priv, fw_engine); \ val = __raw_i915_read##x(dev_priv, reg); \ - hsw_unclaimed_reg_debug(dev_priv, reg, true, false); \ GEN6_READ_FOOTER; \ } @@ -887,9 +894,11 @@ __gen2_write(64) unsigned long irqflags; \ trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \ assert_rpm_wakelock_held(dev_priv); \ - spin_lock_irqsave(&dev_priv->uncore.lock, irqflags) + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); \ + unclaimed_reg_debug(dev_priv, reg, false, true) #define GEN6_WRITE_FOOTER \ + unclaimed_reg_debug(dev_priv, reg, false, false); \ spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags) #define __gen6_write(x) \ @@ -915,12 +924,10 @@ hsw_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool t if (NEEDS_FORCE_WAKE(offset)) { \ __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \ } \ - 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_debug(dev_priv, reg, false, false); \ GEN6_WRITE_FOOTER; \ } @@ -950,11 +957,9 @@ static bool is_gen8_shadowed(struct drm_i915_private *dev_priv, static void \ gen8_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool trace) { \ GEN6_WRITE_HEADER; \ - hsw_unclaimed_reg_debug(dev_priv, reg, false, true); \ if (NEEDS_FORCE_WAKE(offset) && !is_gen8_shadowed(dev_priv, reg)) \ __force_wake_get(dev_priv, FORCEWAKE_RENDER); \ __raw_i915_write##x(dev_priv, reg, val); \ - hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \ GEN6_WRITE_FOOTER; \ } @@ -1008,7 +1013,6 @@ gen9_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, \ bool trace) { \ enum forcewake_domains fw_engine; \ GEN6_WRITE_HEADER; \ - hsw_unclaimed_reg_debug(dev_priv, reg, false, true); \ if (!SKL_NEEDS_FORCE_WAKE(offset) || \ is_gen9_shadowed(dev_priv, reg)) \ fw_engine = 0; \ @@ -1023,7 +1027,6 @@ gen9_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, \ if (fw_engine) \ __force_wake_get(dev_priv, fw_engine); \ __raw_i915_write##x(dev_priv, reg, val); \ - hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \ GEN6_WRITE_FOOTER; \ } -- cgit v1.2.3 From bc3b9346cd47148db8601fb425a4949902c475ec Mon Sep 17 00:00:00 2001 From: Mika Kuoppala Date: Fri, 8 Jan 2016 15:51:20 +0200 Subject: drm/i915: Arm the unclaimed mmio debugs on suspend path If we go into suspend with unclaimed access detected, it would be nice to catch that access on a next suspend path. So instead of just notifying about it, arm the unclaimed mmio checks on suspend side. We want to keep the asymmetry on resume, as if it was on resume path, it was not driver that is responsible so no point in arming mmio debugs. Cc: Chris Wilson Signed-off-by: Mika Kuoppala Reviewed-by: Chris Wilson Link: http://patchwork.freedesktop.org/patch/msgid/1452261080-6979-2-git-send-email-mika.kuoppala@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/intel_uncore.c | 7 +++++-- 3 files changed, 7 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_uncore.c') diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 288fec7691dc..9de993d5fed2 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1502,7 +1502,7 @@ static int intel_runtime_suspend(struct device *device) enable_rpm_wakeref_asserts(dev_priv); WARN_ON_ONCE(atomic_read(&dev_priv->pm.wakeref_count)); - if (intel_uncore_unclaimed_mmio(dev_priv)) + if (intel_uncore_arm_unclaimed_mmio_detection(dev_priv)) DRM_ERROR("Unclaimed access detected prior to suspending\n"); dev_priv->pm.suspended = true; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 747d2d84a18c..104bd1809936 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2720,7 +2720,7 @@ extern void intel_uncore_early_sanitize(struct drm_device *dev, bool restore_forcewake); extern void intel_uncore_init(struct drm_device *dev); extern bool intel_uncore_unclaimed_mmio(struct drm_i915_private *dev_priv); -extern void intel_uncore_arm_unclaimed_mmio_detection(struct drm_i915_private *dev_priv); +extern bool intel_uncore_arm_unclaimed_mmio_detection(struct drm_i915_private *dev_priv); extern void intel_uncore_fini(struct drm_device *dev); extern void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore); const char *intel_uncore_forcewake_domain_to_str(const enum forcewake_domain_id id); diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 0b47bc891dc7..c3c13dc929cb 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -1610,12 +1610,12 @@ bool intel_uncore_unclaimed_mmio(struct drm_i915_private *dev_priv) return check_for_unclaimed_mmio(dev_priv); } -void +bool intel_uncore_arm_unclaimed_mmio_detection(struct drm_i915_private *dev_priv) { if (unlikely(i915.mmio_debug || dev_priv->uncore.unclaimed_mmio_check <= 0)) - return; + return false; if (unlikely(intel_uncore_unclaimed_mmio(dev_priv))) { DRM_DEBUG("Unclaimed register detected, " @@ -1623,5 +1623,8 @@ intel_uncore_arm_unclaimed_mmio_detection(struct drm_i915_private *dev_priv) "Please use i915.mmio_debug=N for more information.\n"); i915.mmio_debug++; dev_priv->uncore.unclaimed_mmio_check--; + return true; } + + return false; } -- cgit v1.2.3 From c81eeea6c14b212016104f4256c65f93ad230a86 Mon Sep 17 00:00:00 2001 From: Mika Kuoppala Date: Wed, 20 Jan 2016 12:32:23 +0200 Subject: drm/i915: Limit the auto arming of mmio debugs on vlv/chv The capability to detect unclaimed register access was recently introduced for vlv/chv platforms. Apparently there are plenty of unclaimed access on these platforms, resulting in new dmesg warns. But as we are trying to form a beachhead for CI/Bat, all new warns are adding to the noise and thus not desirable at this point in time. Make it so that if in these platforms the automatic arming was responsible for mmio_debug enabling, ignore the warns. If user/dev wants to fix these, he can still do so by i915.mmio_debug=1234. Cc: Daniel Vetter Signed-off-by: Mika Kuoppala Reviewed-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1453285943-24614-1-git-send-email-mika.kuoppala@intel.com --- drivers/gpu/drm/i915/intel_uncore.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'drivers/gpu/drm/i915/intel_uncore.c') diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index c3c13dc929cb..bfa79e5c214e 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -631,6 +631,15 @@ __unclaimed_reg_debug(struct drm_i915_private *dev_priv, const bool read, const bool before) { + /* XXX. We limit the auto arming traces for mmio + * debugs on these platforms. There are just too many + * revealed by these and CI/Bat suffers from the noise. + * Please fix and then re-enable the automatic traces. + */ + if (i915.mmio_debug < 2 && + (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))) + return; + if (WARN(check_for_unclaimed_mmio(dev_priv), "Unclaimed register detected %s %s register 0x%x\n", before ? "before" : "after", -- cgit v1.2.3 From 274008e89d78ded4324c7d81d9d106f62f5d5727 Mon Sep 17 00:00:00 2001 From: Sagar Arun Kamble Date: Sat, 6 Feb 2016 00:13:29 +0530 Subject: drm/i915/bxt: Check BIOS RC6 setup before enabling RC6 RC6 setup is shared between BIOS and Driver. BIOS sets up subset of RC6 setup registers. If those are not setup Driver should not enable RC6. For implementing this, driver can check RC_CTRL0 and RC_CTRL1 values to know if BIOS has enabled HW/SW RC6. This will also enable user to control RC6 using BIOS settings alone. RC6 related instability can be avoided by disabling via BIOS settings till driver fixes it. v2: Had placed logic in gen8 function by mistake. Fixed it. Ensuring RPM is not enabled in case BIOS disabled RC6. v3: Need to disable RPM if RC6 is disabled due to BIOS settings. (Daniel) Runtime PM enabling happens before gen9_enable_rc6. Moved the updation of enable_rc6 parameter in intel_uncore_sanitize. v4: Added elaborate check for BIOS RC6 setup. Prepared check_pctx for bxt. (Imre) v5: Caching reserved stolen base and size in the driver private data. Reorganized RC6 setup check. Moved from gen9_enable_rc6 to intel_uncore_sanitize. (Imre) v6: Rebasing on the patch submitted by Imre that moves gem_init_stolen earlier in the load. v7: Removed PWRCTX_MAXCNT_VCSUNIT1 check as it applies to SKL. (Imre) v8: Fixed formatting and checkpatch issues. Fixed functional issue where RC6 ctx size check was missing. (Imre) Cc: Imre Deak Signed-off-by: Sagar Arun Kamble Signed-off-by: Imre Deak Link: http://patchwork.freedesktop.org/patch/msgid/1454697809-22113-1-git-send-email-sagar.a.kamble@intel.com --- drivers/gpu/drm/i915/i915_gem_gtt.h | 2 ++ drivers/gpu/drm/i915/i915_gem_stolen.c | 3 ++ drivers/gpu/drm/i915/i915_reg.h | 11 +++++++ drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 53 ++++++++++++++++++++++++++++++++-- drivers/gpu/drm/i915/intel_uncore.c | 2 ++ 6 files changed, 70 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_uncore.c') diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index f520c90e5377..66a6da2396a2 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -342,6 +342,8 @@ struct i915_gtt { size_t stolen_size; /* Total size of stolen memory */ size_t stolen_usable_size; /* Total size minus BIOS reserved */ + size_t stolen_reserved_base; + size_t stolen_reserved_size; u64 mappable_end; /* End offset that we can CPU map */ struct io_mapping *mappable; /* Mapping to our CPU mappable region */ phys_addr_t mappable_base; /* PA of our GMADR */ diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index c384dc9c8a63..ba1a00d815d3 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -458,6 +458,9 @@ int i915_gem_init_stolen(struct drm_device *dev) return 0; } + dev_priv->gtt.stolen_reserved_base = reserved_base; + dev_priv->gtt.stolen_reserved_size = reserved_size; + /* It is possible for the reserved area to end before the end of stolen * memory, so just consider the start. */ reserved_total = stolen_top - reserved_base; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 6732fc139196..188ad5de020f 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6785,6 +6785,16 @@ enum skl_disp_power_wells { #define VLV_PMWGICZ _MMIO(0x1300a4) +#define RC6_LOCATION _MMIO(0xD40) +#define RC6_CTX_IN_DRAM (1 << 0) +#define RC6_CTX_BASE _MMIO(0xD48) +#define RC6_CTX_BASE_MASK 0xFFFFFFF0 +#define PWRCTX_MAXCNT_RCSUNIT _MMIO(0x2054) +#define PWRCTX_MAXCNT_VCSUNIT0 _MMIO(0x12054) +#define PWRCTX_MAXCNT_BCSUNIT _MMIO(0x22054) +#define PWRCTX_MAXCNT_VECSUNIT _MMIO(0x1A054) +#define PWRCTX_MAXCNT_VCSUNIT1 _MMIO(0x1C054) +#define IDLE_TIME_MASK 0xFFFFF #define FORCEWAKE _MMIO(0xA18C) #define FORCEWAKE_VLV _MMIO(0x1300b0) #define FORCEWAKE_ACK_VLV _MMIO(0x1300b4) @@ -6923,6 +6933,7 @@ enum skl_disp_power_wells { #define GEN6_RPDEUC _MMIO(0xA084) #define GEN6_RPDEUCSW _MMIO(0xA088) #define GEN6_RC_STATE _MMIO(0xA094) +#define RC6_STATE (1 << 18) #define GEN6_RC1_WAKE_RATE_LIMIT _MMIO(0xA098) #define GEN6_RC6_WAKE_RATE_LIMIT _MMIO(0xA09C) #define GEN6_RC6pp_WAKE_RATE_LIMIT _MMIO(0xA0A0) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 93ba14a3bb76..1251a7ae4a6a 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1566,6 +1566,7 @@ void skl_wm_get_hw_state(struct drm_device *dev); void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv, struct skl_ddb_allocation *ddb /* out */); uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config); +int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6); /* intel_sdvo.c */ bool intel_sdvo_init(struct drm_device *dev, diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 3da7935c1665..379eabe093cb 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4562,12 +4562,62 @@ static void intel_print_rc6_info(struct drm_device *dev, u32 mode) onoff(mode & GEN6_RC_CTL_RC6_ENABLE)); } -static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6) +static bool bxt_check_bios_rc6_setup(const struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + bool enable_rc6 = true; + unsigned long rc6_ctx_base; + + if (!(I915_READ(RC6_LOCATION) & RC6_CTX_IN_DRAM)) { + DRM_DEBUG_KMS("RC6 Base location not set properly.\n"); + enable_rc6 = false; + } + + /* + * The exact context size is not known for BXT, so assume a page size + * for this check. + */ + rc6_ctx_base = I915_READ(RC6_CTX_BASE) & RC6_CTX_BASE_MASK; + if (!((rc6_ctx_base >= dev_priv->gtt.stolen_reserved_base) && + (rc6_ctx_base + PAGE_SIZE <= dev_priv->gtt.stolen_reserved_base + + dev_priv->gtt.stolen_reserved_size))) { + DRM_DEBUG_KMS("RC6 Base address not as expected.\n"); + enable_rc6 = false; + } + + if (!(((I915_READ(PWRCTX_MAXCNT_RCSUNIT) & IDLE_TIME_MASK) > 1) && + ((I915_READ(PWRCTX_MAXCNT_VCSUNIT0) & IDLE_TIME_MASK) > 1) && + ((I915_READ(PWRCTX_MAXCNT_BCSUNIT) & IDLE_TIME_MASK) > 1) && + ((I915_READ(PWRCTX_MAXCNT_VECSUNIT) & IDLE_TIME_MASK) > 1))) { + DRM_DEBUG_KMS("Engine Idle wait time not set properly.\n"); + enable_rc6 = false; + } + + if (!(I915_READ(GEN6_RC_CONTROL) & (GEN6_RC_CTL_RC6_ENABLE | + GEN6_RC_CTL_HW_ENABLE)) && + ((I915_READ(GEN6_RC_CONTROL) & GEN6_RC_CTL_HW_ENABLE) || + !(I915_READ(GEN6_RC_STATE) & RC6_STATE))) { + DRM_DEBUG_KMS("HW/SW RC6 is not enabled by BIOS.\n"); + enable_rc6 = false; + } + + return enable_rc6; +} + +int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6) { /* No RC6 before Ironlake and code is gone for ilk. */ if (INTEL_INFO(dev)->gen < 6) return 0; + if (!enable_rc6) + return 0; + + if (IS_BROXTON(dev) && !bxt_check_bios_rc6_setup(dev)) { + DRM_INFO("RC6 disabled by BIOS\n"); + return 0; + } + /* Respect the kernel parameter if it is set */ if (enable_rc6 >= 0) { int mask; @@ -6057,7 +6107,6 @@ void intel_init_gt_powersave(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6); /* * RPM depends on RC6 to save restore the GT HW context, so make RC6 a * requirement. diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index bfa79e5c214e..436d8f2b8682 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -400,6 +400,8 @@ void intel_uncore_early_sanitize(struct drm_device *dev, bool restore_forcewake) void intel_uncore_sanitize(struct drm_device *dev) { + i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6); + /* BIOS often leaves RC6 enabled, but disable it for hw init */ intel_disable_gt_powersave(dev); } -- cgit v1.2.3