drm/i915/bdw: Fix the write setting up the WIZ hashing mode

I was playing with clang and oh surprise! a warning trigerred by
-Wshift-overflow (gcc doesn't have this one):

    WA_SET_BIT_MASKED(GEN7_GT_MODE,
                      GEN6_WIZ_HASHING_MASK | GEN6_WIZ_HASHING_16x4);

    drivers/gpu/drm/i915/intel_ringbuffer.c:786:2: warning: signed shift result
      (0x28002000000) requires 43 bits to represent, but 'int' only has 32 bits
      [-Wshift-overflow]
        WA_SET_BIT_MASKED(GEN7_GT_MODE,
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    drivers/gpu/drm/i915/intel_ringbuffer.c:737:15: note: expanded from macro
      'WA_SET_BIT_MASKED'
        WA_REG(addr, _MASKED_BIT_ENABLE(mask), (mask) & 0xffff)

Turned out GEN6_WIZ_HASHING_MASK was already shifted by 16, and we were
trying to shift it a bit more.

The other thing is that it's not the usual case of setting WA bits here, we
need to have separate mask and value.

To fix this, I've introduced a new _MASKED_FIELD() macro that takes both the
(unshifted) mask and the desired value and the rest of the patch ripples
through from it.

This bug was introduced when reworking the WA emission in:

  Commit 7225342ab501befdb64bcec76ded41f5897c0855
  Author: Mika Kuoppala <mika.kuoppala@linux.intel.com>
  Date:   Tue Oct 7 17:21:26 2014 +0300

      drm/i915: Build workaround list in ring initialization

v2: Invert the order of the mask and value arguments (Daniel Vetter)
    Rewrite _MASKED_BIT_ENABLE() and _MASKED_BIT_DISABLE() with
    _MASKED_FIELD() (Jani Nikula)
    Make sure we only evaluate 'a' once in _MASKED_BIT_ENABLE() (Dave Gordon)
    Add check to ensure the value is within the mask boundaries (Chris Wilson)

v3: Ensure the the value and mask are 16 bits (Dave Gordon)

Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Arun Siluvery <arun.siluvery@linux.intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 544675895c8dff93f6b5cdf3eb757e5643467f33..b607bbe55261c0a750930d60c5f88f3028b7f4b2 100644 (file)
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -32,8 +32,19 @@
 #define _PIPE3(pipe, a, b, c) ((pipe) == PIPE_A ? (a) : \
                               (pipe) == PIPE_B ? (b) : (c))
 
-#define _MASKED_BIT_ENABLE(a) (((a) << 16) | (a))
-#define _MASKED_BIT_DISABLE(a) ((a) << 16)
+#define _MASKED_FIELD(mask, value) ({                                     \
+       if (__builtin_constant_p(mask))                                    \
+               BUILD_BUG_ON_MSG(((mask) & 0xffff0000), "Incorrect mask"); \
+       if (__builtin_constant_p(value))                                   \
+               BUILD_BUG_ON_MSG((value) & 0xffff0000, "Incorrect value"); \
+       if (__builtin_constant_p(mask) && __builtin_constant_p(value))     \
+               BUILD_BUG_ON_MSG((value) & ~(mask),                        \
+                                "Incorrect value for mask");              \
+       (mask) << 16 | (value); })
+#define _MASKED_BIT_ENABLE(a)  ({ typeof(a) _a = (a); _MASKED_FIELD(_a, _a); })
+#define _MASKED_BIT_DISABLE(a) (_MASKED_FIELD((a), 0))
+
+
 
 /* PCI config space */
 
@@ -1282,7 +1293,7 @@ enum punit_power_well {
 #define   GEN6_WIZ_HASHING_8x8                         GEN6_WIZ_HASHING(0, 0)
 #define   GEN6_WIZ_HASHING_8x4                         GEN6_WIZ_HASHING(0, 1)
 #define   GEN6_WIZ_HASHING_16x4                                GEN6_WIZ_HASHING(1, 0)
-#define   GEN6_WIZ_HASHING_MASK                                (GEN6_WIZ_HASHING(1, 1) << 16)
+#define   GEN6_WIZ_HASHING_MASK                                GEN6_WIZ_HASHING(1, 1)
 #define   GEN6_TD_FOUR_ROW_DISPATCH_DISABLE            (1 << 5)
 
 #define GFX_MODE       0x02520

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9af0af49382e6b88f7101cf19a48768cd5848aad..1f4b56e273c811a7c33149e927985b3f973ac4b3 100644 (file)
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6508,7 +6508,7 @@ static void gen6_init_clock_gating(struct drm_device *dev)
         * to keep in mind (see 3DSTATE_PS and 3DSTATE_WM).
         */
        I915_WRITE(GEN6_GT_MODE,
-                  GEN6_WIZ_HASHING_MASK | GEN6_WIZ_HASHING_16x4);
+                  _MASKED_FIELD(GEN6_WIZ_HASHING_MASK, GEN6_WIZ_HASHING_16x4));
 
        ilk_init_lp_watermarks(dev);
 
@@ -6706,7 +6706,7 @@ static void haswell_init_clock_gating(struct drm_device *dev)
         * to keep in mind (see 3DSTATE_PS and 3DSTATE_WM).
         */
        I915_WRITE(GEN7_GT_MODE,
-                  GEN6_WIZ_HASHING_MASK | GEN6_WIZ_HASHING_16x4);
+                  _MASKED_FIELD(GEN6_WIZ_HASHING_MASK, GEN6_WIZ_HASHING_16x4));
 
        /* WaSwitchSolVfFArbitrationPriority:hsw */
        I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
@@ -6803,7 +6803,7 @@ static void ivybridge_init_clock_gating(struct drm_device *dev)
         * to keep in mind (see 3DSTATE_PS and 3DSTATE_WM).
         */
        I915_WRITE(GEN7_GT_MODE,
-                  GEN6_WIZ_HASHING_MASK | GEN6_WIZ_HASHING_16x4);
+                  _MASKED_FIELD(GEN6_WIZ_HASHING_MASK, GEN6_WIZ_HASHING_16x4));
 
        snpcr = I915_READ(GEN6_MBCUNIT_SNPCR);
        snpcr &= ~GEN6_MBC_SNPCR_MASK;

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 1d01b51ff058bd597fc636fa3530069973972104..28db934b2359ef30f8d9c8df2cb6fe7ac0bc57b2 100644 (file)
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -729,6 +729,9 @@ static int wa_add(struct drm_i915_private *dev_priv,
 #define WA_CLR_BIT_MASKED(addr, mask) \
        WA_REG(addr, _MASKED_BIT_DISABLE(mask), (mask) & 0xffff)
 
+#define WA_SET_FIELD_MASKED(addr, mask, value) \
+       WA_REG(addr, _MASKED_FIELD(mask, value), mask)
+
 #define WA_SET_BIT(addr, mask) WA_REG(addr, I915_READ(addr) | (mask), mask)
 #define WA_CLR_BIT(addr, mask) WA_REG(addr, I915_READ(addr) & ~(mask), mask)
 
@@ -773,8 +776,9 @@ static int bdw_init_workarounds(struct intel_engine_cs *ring)
         * disable bit, which we don't touch here, but it's good
         * to keep in mind (see 3DSTATE_PS and 3DSTATE_WM).
         */
-       WA_SET_BIT_MASKED(GEN7_GT_MODE,
-                         GEN6_WIZ_HASHING_MASK | GEN6_WIZ_HASHING_16x4);
+       WA_SET_FIELD_MASKED(GEN7_GT_MODE,
+                           GEN6_WIZ_HASHING_MASK,
+                           GEN6_WIZ_HASHING_16x4);
 
        return 0;
 }