We were reading our 64-bit value in I915_READ64 and returning 32 bits of it. The restoration of fence regs at resume then had a zero end value, and the fence had no effect.
Version 2: Split register access functions into per-size versions Sharing code between different sizes seemed reasonable when we only needed a single copy, but as 64-bit access requires its own version, it makes sense to just split them out for each size. Signed-off-by: Eric Anholt <e...@anholt.net> Signed-off-by: Keith Packard <kei...@keithp.com> --- On Thu, 18 Nov 2010 10:05:00 +0800, Eric Anholt <e...@anholt.net> wrote: > #define I915_READ8(reg) i915_read(dev_priv, (reg), 1) > #define I915_WRITE8(reg, val) i915_write(dev_priv, (reg), (val), 1) > #define I915_WRITE64(reg, val) i915_write(dev_priv, (reg), (val), 8) > -#define I915_READ64(reg) i915_read(dev_priv, (reg), 8) > +#define I915_READ64(reg) i915_read64(dev_priv, (reg)) Now that we've got two functions for this, it seems like it would be better to just create per-size versions in both directions, otherwise changes to the 8/16/32 bit version are unlikely to get propagated to the 64-bit version. drivers/gpu/drm/i915/i915_drv.h | 106 ++++++++++++++++++++++---------------- 1 files changed, 61 insertions(+), 45 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 73a41f7..f83e712 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1237,14 +1237,14 @@ extern void intel_overlay_print_error_state(struct seq_file *m, struct intel_ove LOCK_TEST_WITH_RETURN(dev, file_priv); \ } while (0) -#define I915_READ(reg) i915_read(dev_priv, (reg), 4) -#define I915_WRITE(reg, val) i915_write(dev_priv, (reg), (val), 4) -#define I915_READ16(reg) i915_read(dev_priv, (reg), 2) -#define I915_WRITE16(reg, val) i915_write(dev_priv, (reg), (val), 2) -#define I915_READ8(reg) i915_read(dev_priv, (reg), 1) -#define I915_WRITE8(reg, val) i915_write(dev_priv, (reg), (val), 1) -#define I915_WRITE64(reg, val) i915_write(dev_priv, (reg), (val), 8) -#define I915_READ64(reg) i915_read(dev_priv, (reg), 8) +#define I915_READ(reg) i915_read32(dev_priv, (reg)) +#define I915_WRITE(reg, val) i915_write32(dev_priv, (reg), (val)) +#define I915_READ16(reg) i915_read16(dev_priv, (reg)) +#define I915_WRITE16(reg, val) i915_write16(dev_priv, (reg), (val)) +#define I915_READ8(reg) i915_read8(dev_priv, (reg)) +#define I915_WRITE8(reg, val) i915_write8(dev_priv, (reg), (val)) +#define I915_WRITE64(reg, val) i915_write64(dev_priv, (reg), (val)) +#define I915_READ64(reg) i915_read64(dev_priv, (reg)) #define I915_READ_NOTRACE(reg) readl(dev_priv->regs + (reg)) #define I915_WRITE_NOTRACE(reg, val) writel(val, dev_priv->regs + (reg)) @@ -1254,27 +1254,32 @@ extern void intel_overlay_print_error_state(struct seq_file *m, struct intel_ove #define POSTING_READ(reg) (void)I915_READ_NOTRACE(reg) #define POSTING_READ16(reg) (void)I915_READ16_NOTRACE(reg) -static inline u32 i915_read(struct drm_i915_private *dev_priv, u32 reg, int len) +static inline u8 i915_read8(struct drm_i915_private *dev_priv, u32 reg) { - u64 val = 0; - - switch (len) { - case 8: - val = readq(dev_priv->regs + reg); - break; - case 4: - val = readl(dev_priv->regs + reg); - break; - case 2: - val = readw(dev_priv->regs + reg); - break; - case 1: - val = readb(dev_priv->regs + reg); - break; - } - trace_i915_reg_rw('R', reg, val, len); - - return val; + u8 val = readb(dev_priv->regs + reg); + trace_i915_reg_rw('R', reg, val, 1); + return val; +} + +static inline u16 i915_read16(struct drm_i915_private *dev_priv, u32 reg) +{ + u16 val = readw(dev_priv->regs + reg); + trace_i915_reg_rw('R', reg, val, 2); + return val; +} + +static inline u32 i915_read32(struct drm_i915_private *dev_priv, u32 reg) +{ + u32 val = readl(dev_priv->regs + reg); + trace_i915_reg_rw('R', reg, val, 4); + return val; +} + +static inline u64 i915_read64(struct drm_i915_private *dev_priv, u32 reg) +{ + u64 val = readq(dev_priv->regs + reg); + trace_i915_reg_rw('R', reg, val, 8); + return val; } /* On SNB platform, before reading ring registers forcewake bit @@ -1295,24 +1300,35 @@ static inline u32 i915_safe_read(struct drm_i915_private *dev_priv, u32 reg) } static inline void -i915_write(struct drm_i915_private *dev_priv, u32 reg, u64 val, int len) +i915_write8(struct drm_i915_private *dev_priv, u32 reg, u8 val) +{ + /* Trace down the write operation before the real write */ + trace_i915_reg_rw('W', reg, val, 1); + writeb(val, dev_priv->regs + reg); +} + +static inline void +i915_write16(struct drm_i915_private *dev_priv, u32 reg, u16 val) +{ + /* Trace down the write operation before the real write */ + trace_i915_reg_rw('W', reg, val, 2); + writew(val, dev_priv->regs + reg); +} + +static inline void +i915_write32(struct drm_i915_private *dev_priv, u32 reg, u32 val) +{ + /* Trace down the write operation before the real write */ + trace_i915_reg_rw('W', reg, val, 4); + writel(val, dev_priv->regs + reg); +} + +static inline void +i915_write64(struct drm_i915_private *dev_priv, u32 reg, u64 val) { - /* Trace down the write operation before the real write */ - trace_i915_reg_rw('W', reg, val, len); - switch (len) { - case 8: - writeq(val, dev_priv->regs + reg); - break; - case 4: - writel(val, dev_priv->regs + reg); - break; - case 2: - writew(val, dev_priv->regs + reg); - break; - case 1: - writeb(val, dev_priv->regs + reg); - break; - } + /* Trace down the write operation before the real write */ + trace_i915_reg_rw('W', reg, val, 8); + writeq(val, dev_priv->regs + reg); } #define BEGIN_LP_RING(n) \ -- 1.7.2.3 -- keith.pack...@intel.com
pgpgE1Vf0f1cL.pgp
Description: PGP signature
_______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx