From a2b4abfc626b13d2f2728f11435c8f3c4b9b39d8 Mon Sep 17 00:00:00 2001 From: Daniele Ceraolo Spurio Date: Mon, 25 Mar 2019 14:49:36 -0700 Subject: [PATCH] drm/i915: switch uncore mmio funcs to use intel_uncore 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 Cc: Paulo Zanoni Cc: Chris Wilson Signed-off-by: Chris Wilson Link: https://patchwork.freedesktop.org/patch/msgid/20190325214940.23632-6-daniele.ceraolospurio@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 49 ++++----- drivers/gpu/drm/i915/intel_uncore.c | 18 ++-- drivers/gpu/drm/i915/intel_uncore.h | 106 +++++++++++++++++-- drivers/gpu/drm/i915/selftests/mock_uncore.c | 4 +- 4 files changed, 130 insertions(+), 47 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 681bcbd5c06b..b8df3eb28d35 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -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 diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 78e4d5f2320e..7b7708506d64 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -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); \ diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h index 112571c3f411..b7c3b829b3cb 100644 --- a/drivers/gpu/drm/i915/intel_uncore.h +++ b/drivers/gpu/drm/i915/intel_uncore.h @@ -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) \ diff --git a/drivers/gpu/drm/i915/selftests/mock_uncore.c b/drivers/gpu/drm/i915/selftests/mock_uncore.c index c3896c1fd551..ff8999c63a12 100644 --- a/drivers/gpu/drm/i915/selftests/mock_uncore.c +++ b/drivers/gpu/drm/i915/selftests/mock_uncore.c @@ -26,14 +26,14 @@ #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) -- 2.30.2