[Intel-gfx] [PATCH] drm/i915: Kill GEN_RANGE in favor of INTEL_GEN.

[Chris Wilson]

Quoting Vivi, Rodrigo (2017-09-06 21:13:07)
> On Wed, 2017-09-06 at 20:57 +0100, Chris Wilson wrote:
> > Quoting Rodrigo Vivi (2017-09-06 20:51:37)
> > > Instead of limiting the range with this unusual GEN_RANGE
> > > let's assume following platforms would use same scheme
> > > unless stated otherwise.
> > 
> > No. This is uabi that should indeed be checked before exposed and not
> > assumed that unprivileged access to a register of yesterday is still
> > safe tommorrow.
> 
> hm... makes sense..
> 
> can we at least move to 
> 
> INTEL_GEN >= 4 && INTEL_GEN <= 10
> 
> or some flag on platform definition?
> 
> I really don't like GEN_RANGE... 

Something along the lines of

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6020a94daf81..b43a20bf2f25 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2843,24 +2843,30 @@ intel_info(const struct drm_i915_private *dev_priv)
 #define INTEL_REVID(dev_priv)  ((dev_priv)->drm.pdev->revision)
 
 #define GEN_FOREVER (0)
+
+#define __GEN_RANGE(S, E) \
+       GENMASK((E) == GEN_FOREVER ? BITS_PER_LONG - 1 : (E) - 1, \
+               (S) == GEN_FOREVER ? 0 : (S) - 1)
 /*
- * Returns true if Gen is in inclusive range [Start, End].
+ * Computes the generation mask for an inclusive range [Start, End]
  *
  * Use GEN_FOREVER for unbound start and or end.
  */
-#define IS_GEN(dev_priv, s, e) ({ \
-       unsigned int __s = (s), __e = (e); \
-       BUILD_BUG_ON(!__builtin_constant_p(s)); \
-       BUILD_BUG_ON(!__builtin_constant_p(e)); \
-       if ((__s) != GEN_FOREVER) \
-               __s = (s) - 1; \
-       if ((__e) == GEN_FOREVER) \
-               __e = BITS_PER_LONG - 1; \
-       else \
-               __e = (e) - 1; \
-       !!((dev_priv)->info.gen_mask & GENMASK((__e), (__s))); \
+#define GEN_RANGE(S, E) ({ \
+       BUILD_BUG_ON(!__builtin_constant_p(S)); \
+       BUILD_BUG_ON(!__builtin_constant_p(E)); \
+       __GEN_RANGE(S, E); \
 })
 
+#define __IS_GEN(dev_priv, mask) (!!((dev_priv)->info.gen_mask & (mask)))
+
+/*
+ * Returns true if Gen is in inclusive range [Start, End].
+ *
+ * Use GEN_FOREVER for unbound start and or end.
+ */
+#define IS_GEN(dev_priv, s, e) __IS_GEN(dev_priv, GEN_RANGE(s, e))
+
 /*
  * Return true if revision is in range [since,until] inclusive.
  *
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 1d7b879cc68c..5f66313d628a 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1241,8 +1241,6 @@ void intel_uncore_fini(struct drm_i915_private *dev_priv)
        intel_uncore_forcewake_reset(dev_priv, false);
 }
 
-#define GEN_RANGE(l, h) GENMASK((h) - 1, (l) - 1)
-
 static const struct register_whitelist {
        i915_reg_t offset_ldw, offset_udw;
        uint32_t size;
@@ -1251,7 +1249,7 @@ static const struct register_whitelist {
 } whitelist[] = {
        { .offset_ldw = RING_TIMESTAMP(RENDER_RING_BASE),
          .offset_udw = RING_TIMESTAMP_UDW(RENDER_RING_BASE),
-         .size = 8, .gen_bitmask = GEN_RANGE(4, 9) },
+         .size = 8, .gen_bitmask = __GEN_RANGE(4, 9) },
 };
 
 int i915_reg_read_ioctl(struct drm_device *dev,
@@ -1266,7 +1264,7 @@ int i915_reg_read_ioctl(struct drm_device *dev,
 
        for (i = 0; i < ARRAY_SIZE(whitelist); i++, entry++) {
                if (i915_mmio_reg_offset(entry->offset_ldw) == (reg->offset & -entry->size) &&
-                   (INTEL_INFO(dev_priv)->gen_mask & entry->gen_bitmask))
+                   __IS_GEN(dev_priv, entry->gen_bitmask))
                        break;
        }