drm/i915: switch uncore mmio funcs to use intel_uncore
authorDaniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Mon, 25 Mar 2019 21:49:36 +0000 (14:49 -0700)
committerChris Wilson <chris@chris-wilson.co.uk>
Tue, 26 Mar 2019 20:16:13 +0000 (20:16 +0000)
The full read/write ops can now work on the intel_uncore struct.
Introduce intel_uncore_read/write functions working on intel_uncore
and switch the I915_READ/WRITE macro to internally call those.

v2: no change
v3: add intel_uncore_read/write functions (Chris), update commit msg

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20190325214940.23632-6-daniele.ceraolospurio@intel.com
drivers/gpu/drm/i915/i915_drv.h
drivers/gpu/drm/i915/intel_uncore.c
drivers/gpu/drm/i915/intel_uncore.h
drivers/gpu/drm/i915/selftests/mock_uncore.c

index 681bcbd5c06b913bdd8044658ae2300430f0c5d5..b8df3eb28d35a5750cc80ee2a9c5113bce0a618e 100644 (file)
@@ -3476,18 +3476,21 @@ static inline u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv,
        return DIV_ROUND_UP_ULL(intel_rc6_residency_ns(dev_priv, reg), 1000);
 }
 
-#define I915_READ8(reg)                dev_priv->uncore.funcs.mmio_readb(dev_priv, (reg), true)
-#define I915_WRITE8(reg, val)  dev_priv->uncore.funcs.mmio_writeb(dev_priv, (reg), (val), true)
+#define __I915_REG_OP(op__, dev_priv__, ...) \
+       intel_uncore_##op__(&(dev_priv__)->uncore, __VA_ARGS__)
 
-#define I915_READ16(reg)       dev_priv->uncore.funcs.mmio_readw(dev_priv, (reg), true)
-#define I915_WRITE16(reg, val) dev_priv->uncore.funcs.mmio_writew(dev_priv, (reg), (val), true)
-#define I915_READ16_NOTRACE(reg)       dev_priv->uncore.funcs.mmio_readw(dev_priv, (reg), false)
-#define I915_WRITE16_NOTRACE(reg, val) dev_priv->uncore.funcs.mmio_writew(dev_priv, (reg), (val), false)
+#define I915_READ8(reg__)        __I915_REG_OP(read8, dev_priv, (reg__))
+#define I915_WRITE8(reg__, val__) __I915_REG_OP(write8, dev_priv, (reg__), (val__))
 
-#define I915_READ(reg)         dev_priv->uncore.funcs.mmio_readl(dev_priv, (reg), true)
-#define I915_WRITE(reg, val)   dev_priv->uncore.funcs.mmio_writel(dev_priv, (reg), (val), true)
-#define I915_READ_NOTRACE(reg)         dev_priv->uncore.funcs.mmio_readl(dev_priv, (reg), false)
-#define I915_WRITE_NOTRACE(reg, val)   dev_priv->uncore.funcs.mmio_writel(dev_priv, (reg), (val), false)
+#define I915_READ16(reg__)        __I915_REG_OP(read16, dev_priv, (reg__))
+#define I915_WRITE16(reg__, val__) __I915_REG_OP(write16, dev_priv, (reg__), (val__))
+#define I915_READ16_NOTRACE(reg__)        __I915_REG_OP(read16_notrace, dev_priv, (reg__))
+#define I915_WRITE16_NOTRACE(reg__, val__) __I915_REG_OP(write16_notrace, dev_priv, (reg__), (val__))
+
+#define I915_READ(reg__)        __I915_REG_OP(read, dev_priv, (reg__))
+#define I915_WRITE(reg__, val__) __I915_REG_OP(write, dev_priv, (reg__), (val__))
+#define I915_READ_NOTRACE(reg__)        __I915_REG_OP(read_notrace, dev_priv, (reg__))
+#define I915_WRITE_NOTRACE(reg__, val__) __I915_REG_OP(write_notrace, dev_priv, (reg__), (val__))
 
 /* Be very careful with read/write 64-bit values. On 32-bit machines, they
  * will be implemented using 2 32-bit writes in an arbitrary order with
@@ -3503,20 +3506,12 @@ static inline u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv,
  *
  * You have been warned.
  */
-#define I915_READ64(reg)       dev_priv->uncore.funcs.mmio_readq(dev_priv, (reg), true)
-
-#define I915_READ64_2x32(lower_reg, upper_reg) ({                      \
-       u32 upper, lower, old_upper, loop = 0;                          \
-       upper = I915_READ(upper_reg);                                   \
-       do {                                                            \
-               old_upper = upper;                                      \
-               lower = I915_READ(lower_reg);                           \
-               upper = I915_READ(upper_reg);                           \
-       } while (upper != old_upper && loop++ < 2);                     \
-       (u64)upper << 32 | lower; })
+#define I915_READ64(reg__)     __I915_REG_OP(read64, dev_priv, (reg__))
+#define I915_READ64_2x32(lower_reg__, upper_reg__) \
+       __I915_REG_OP(read64_2x32, dev_priv, (lower_reg__), (upper_reg__))
 
-#define POSTING_READ(reg)      (void)I915_READ_NOTRACE(reg)
-#define POSTING_READ16(reg)    (void)I915_READ16_NOTRACE(reg)
+#define POSTING_READ(reg__)    __I915_REG_OP(posting_read, dev_priv, (reg__))
+#define POSTING_READ16(reg__)  __I915_REG_OP(posting_read16, dev_priv, (reg__))
 
 /* These are untraced mmio-accessors that are only valid to be used inside
  * critical sections, such as inside IRQ handlers, where forcewake is explicitly
@@ -3544,10 +3539,10 @@ static inline u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv,
  * therefore generally be serialised, by either the dev_priv->uncore.lock or
  * a more localised lock guarding all access to that bank of registers.
  */
-#define I915_READ_FW(reg__) __raw_uncore_read32(&dev_priv->uncore, (reg__))
-#define I915_WRITE_FW(reg__, val__) __raw_uncore_write32(&dev_priv->uncore, (reg__), (val__))
-#define I915_WRITE64_FW(reg__, val__) __raw_uncore_write64(&dev_priv->uncore, (reg__), (val__))
-#define POSTING_READ_FW(reg__) (void)I915_READ_FW(reg__)
+#define I915_READ_FW(reg__) __I915_REG_OP(read_fw, dev_priv, (reg__))
+#define I915_WRITE_FW(reg__, val__) __I915_REG_OP(write_fw, dev_priv, (reg__), (val__))
+#define I915_WRITE64_FW(reg__, val__) __I915_REG_OP(write64_fw, dev_priv, (reg__), (val__))
+#define POSTING_READ_FW(reg__) __I915_REG_OP(posting_read_fw, dev_priv, (reg__))
 
 /* "Broadcast RGB" property */
 #define INTEL_BROADCAST_RGB_AUTO 0
index 78e4d5f2320e081744db442d07ec324344706f99..7b7708506d647518a236e3c06e3badd8af207a2c 100644 (file)
@@ -1093,7 +1093,6 @@ unclaimed_reg_debug(struct intel_uncore *uncore,
 }
 
 #define GEN2_READ_HEADER(x) \
-       struct intel_uncore *uncore = &dev_priv->uncore; \
        u##x val = 0; \
        __assert_rpm_wakelock_held(uncore->rpm);
 
@@ -1103,7 +1102,7 @@ unclaimed_reg_debug(struct intel_uncore *uncore,
 
 #define __gen2_read(x) \
 static u##x \
-gen2_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
+gen2_read##x(struct intel_uncore *uncore, i915_reg_t reg, bool trace) { \
        GEN2_READ_HEADER(x); \
        val = __raw_uncore_read##x(uncore, reg); \
        GEN2_READ_FOOTER; \
@@ -1111,7 +1110,7 @@ gen2_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
 
 #define __gen5_read(x) \
 static u##x \
-gen5_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
+gen5_read##x(struct intel_uncore *uncore, i915_reg_t reg, bool trace) { \
        GEN2_READ_HEADER(x); \
        ilk_dummy_write(uncore); \
        val = __raw_uncore_read##x(uncore, reg); \
@@ -1134,7 +1133,6 @@ __gen2_read(64)
 #undef GEN2_READ_HEADER
 
 #define GEN6_READ_HEADER(x) \
-       struct intel_uncore *uncore = &dev_priv->uncore; \
        u32 offset = i915_mmio_reg_offset(reg); \
        unsigned long irqflags; \
        u##x val = 0; \
@@ -1178,7 +1176,7 @@ static inline void __force_wake_auto(struct intel_uncore *uncore,
 
 #define __gen_read(func, x) \
 static u##x \
-func##_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
+func##_read##x(struct intel_uncore *uncore, i915_reg_t reg, bool trace) { \
        enum forcewake_domains fw_engine; \
        GEN6_READ_HEADER(x); \
        fw_engine = __##func##_reg_read_fw_domains(uncore, offset); \
@@ -1211,7 +1209,6 @@ __gen6_read(64)
 #undef GEN6_READ_HEADER
 
 #define GEN2_WRITE_HEADER \
-       struct intel_uncore *uncore = &dev_priv->uncore; \
        trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
        __assert_rpm_wakelock_held(uncore->rpm); \
 
@@ -1219,7 +1216,7 @@ __gen6_read(64)
 
 #define __gen2_write(x) \
 static void \
-gen2_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool trace) { \
+gen2_write##x(struct intel_uncore *uncore, i915_reg_t reg, u##x val, bool trace) { \
        GEN2_WRITE_HEADER; \
        __raw_uncore_write##x(uncore, reg, val); \
        GEN2_WRITE_FOOTER; \
@@ -1227,7 +1224,7 @@ gen2_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool
 
 #define __gen5_write(x) \
 static void \
-gen5_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool trace) { \
+gen5_write##x(struct intel_uncore *uncore, i915_reg_t reg, u##x val, bool trace) { \
        GEN2_WRITE_HEADER; \
        ilk_dummy_write(uncore); \
        __raw_uncore_write##x(uncore, reg, val); \
@@ -1248,7 +1245,6 @@ __gen2_write(32)
 #undef GEN2_WRITE_HEADER
 
 #define GEN6_WRITE_HEADER \
-       struct intel_uncore *uncore = &dev_priv->uncore; \
        u32 offset = i915_mmio_reg_offset(reg); \
        unsigned long irqflags; \
        trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
@@ -1262,7 +1258,7 @@ __gen2_write(32)
 
 #define __gen6_write(x) \
 static void \
-gen6_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool trace) { \
+gen6_write##x(struct intel_uncore *uncore, i915_reg_t reg, u##x val, bool trace) { \
        GEN6_WRITE_HEADER; \
        if (NEEDS_FORCE_WAKE(offset)) \
                __gen6_gt_wait_for_fifo(uncore); \
@@ -1272,7 +1268,7 @@ gen6_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool
 
 #define __gen_write(func, x) \
 static void \
-func##_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool trace) { \
+func##_write##x(struct intel_uncore *uncore, i915_reg_t reg, u##x val, bool trace) { \
        enum forcewake_domains fw_engine; \
        GEN6_WRITE_HEADER; \
        fw_engine = __##func##_reg_write_fw_domains(uncore, offset); \
index 112571c3f41138fd500551b86f4595dd95bfb610..b7c3b829b3cbdd62b09128e5a63bbc4adf31346a 100644 (file)
@@ -69,20 +69,20 @@ struct intel_uncore_funcs {
        void (*force_wake_put)(struct intel_uncore *uncore,
                               enum forcewake_domains domains);
 
-       u8 (*mmio_readb)(struct drm_i915_private *dev_priv,
+       u8 (*mmio_readb)(struct intel_uncore *uncore,
                         i915_reg_t r, bool trace);
-       u16 (*mmio_readw)(struct drm_i915_private *dev_priv,
+       u16 (*mmio_readw)(struct intel_uncore *uncore,
                          i915_reg_t r, bool trace);
-       u32 (*mmio_readl)(struct drm_i915_private *dev_priv,
+       u32 (*mmio_readl)(struct intel_uncore *uncore,
                          i915_reg_t r, bool trace);
-       u64 (*mmio_readq)(struct drm_i915_private *dev_priv,
+       u64 (*mmio_readq)(struct intel_uncore *uncore,
                          i915_reg_t r, bool trace);
 
-       void (*mmio_writeb)(struct drm_i915_private *dev_priv,
+       void (*mmio_writeb)(struct intel_uncore *uncore,
                            i915_reg_t r, u8 val, bool trace);
-       void (*mmio_writew)(struct drm_i915_private *dev_priv,
+       void (*mmio_writew)(struct intel_uncore *uncore,
                            i915_reg_t r, u16 val, bool trace);
-       void (*mmio_writel)(struct drm_i915_private *dev_priv,
+       void (*mmio_writel)(struct intel_uncore *uncore,
                            i915_reg_t r, u32 val, bool trace);
 };
 
@@ -275,6 +275,98 @@ __raw_write(64, q)
 #undef __raw_read
 #undef __raw_write
 
+#define __uncore_read(name__, x__, s__, trace__) \
+static inline u##x__ intel_uncore_##name__(struct intel_uncore *uncore, \
+                                          i915_reg_t reg) \
+{ \
+       return uncore->funcs.mmio_read##s__(uncore, reg, (trace__)); \
+}
+
+#define __uncore_write(name__, x__, s__, trace__) \
+static inline void intel_uncore_##name__(struct intel_uncore *uncore, \
+                                        i915_reg_t reg, u##x__ val) \
+{ \
+       uncore->funcs.mmio_write##s__(uncore, reg, val, (trace__)); \
+}
+
+__uncore_read(read8, 8, b, true)
+__uncore_read(read16, 16, w, true)
+__uncore_read(read, 32, l, true)
+__uncore_read(read16_notrace, 16, w, false)
+__uncore_read(read_notrace, 32, l, false)
+
+__uncore_write(write8, 8, b, true)
+__uncore_write(write16, 16, w, true)
+__uncore_write(write, 32, l, true)
+__uncore_write(write_notrace, 32, l, false)
+
+/* Be very careful with read/write 64-bit values. On 32-bit machines, they
+ * will be implemented using 2 32-bit writes in an arbitrary order with
+ * an arbitrary delay between them. This can cause the hardware to
+ * act upon the intermediate value, possibly leading to corruption and
+ * machine death. For this reason we do not support I915_WRITE64, or
+ * uncore->funcs.mmio_writeq.
+ *
+ * When reading a 64-bit value as two 32-bit values, the delay may cause
+ * the two reads to mismatch, e.g. a timestamp overflowing. Also note that
+ * occasionally a 64-bit register does not actually support a full readq
+ * and must be read using two 32-bit reads.
+ *
+ * You have been warned.
+ */
+__uncore_read(read64, 64, q, true)
+
+static inline u64
+intel_uncore_read64_2x32(struct intel_uncore *uncore,
+                        i915_reg_t lower_reg, i915_reg_t upper_reg)
+{
+       u32 upper, lower, old_upper, loop = 0;
+       upper = intel_uncore_read(uncore, upper_reg);
+       do {
+               old_upper = upper;
+               lower = intel_uncore_read(uncore, lower_reg);
+               upper = intel_uncore_read(uncore, upper_reg);
+       } while (upper != old_upper && loop++ < 2);
+       return (u64)upper << 32 | lower;
+}
+
+#define intel_uncore_posting_read(...) ((void)intel_uncore_read_notrace(__VA_ARGS__))
+#define intel_uncore_posting_read16(...) ((void)intel_uncore_read16_notrace(__VA_ARGS__))
+
+#undef __uncore_read
+#undef __uncore_write
+
+/* These are untraced mmio-accessors that are only valid to be used inside
+ * critical sections, such as inside IRQ handlers, where forcewake is explicitly
+ * controlled.
+ *
+ * Think twice, and think again, before using these.
+ *
+ * As an example, these accessors can possibly be used between:
+ *
+ * spin_lock_irq(&uncore->lock);
+ * intel_uncore_forcewake_get__locked();
+ *
+ * and
+ *
+ * intel_uncore_forcewake_put__locked();
+ * spin_unlock_irq(&uncore->lock);
+ *
+ *
+ * Note: some registers may not need forcewake held, so
+ * intel_uncore_forcewake_{get,put} can be omitted, see
+ * intel_uncore_forcewake_for_reg().
+ *
+ * Certain architectures will die if the same cacheline is concurrently accessed
+ * by different clients (e.g. on Ivybridge). Access to registers should
+ * therefore generally be serialised, by either the dev_priv->uncore.lock or
+ * a more localised lock guarding all access to that bank of registers.
+ */
+#define intel_uncore_read_fw(...) __raw_uncore_read32(__VA_ARGS__)
+#define intel_uncore_write_fw(...) __raw_uncore_write32(__VA_ARGS__)
+#define intel_uncore_write64_fw(...) __raw_uncore_write64(__VA_ARGS__)
+#define intel_uncore_posting_read_fw(...) ((void)intel_uncore_read_fw(__VA_ARGS__))
+
 #define raw_reg_read(base, reg) \
        readl(base + i915_mmio_reg_offset(reg))
 #define raw_reg_write(base, reg, value) \
index c3896c1fd5515714e54325c213d26682758c371b..ff8999c63a12b9c3a2d55b8a37862983564debd3 100644 (file)
 
 #define __nop_write(x) \
 static void \
-nop_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool trace) { }
+nop_write##x(struct intel_uncore *uncore, i915_reg_t reg, u##x val, bool trace) { }
 __nop_write(8)
 __nop_write(16)
 __nop_write(32)
 
 #define __nop_read(x) \
 static u##x \
-nop_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { return 0; }
+nop_read##x(struct intel_uncore *uncore, i915_reg_t reg, bool trace) { return 0; }
 __nop_read(8)
 __nop_read(16)
 __nop_read(32)