drm/i915: Type safe register read/write
Make I915_READ and I915_WRITE more type safe by wrapping the register
offset in a struct. This should eliminate most of the fumbles we've had
with misplaced parens.
This only takes care of normal mmio registers. We could extend the idea
to other register types and define each with its own struct. That way
you wouldn't be able to accidentally pass the wrong thing to a specific
register access function.
The gpio_reg setup is probably the ugliest thing left. But I figure I'd
just leave it for now, and wait for some divine inspiration to strike
before making it nice.
As for the generated code, it's actually a bit better sometimes. Eg.
looking at i915_irq_handler(), we can see the following change:
lea 0x70024(%rdx,%rax,1),%r9d
mov $0x1,%edx
- movslq %r9d,%r9
- mov %r9,%rsi
- mov %r9,-0x58(%rbp)
- callq *0xd8(%rbx)
+ mov %r9d,%esi
+ mov %r9d,-0x48(%rbp)
callq *0xd8(%rbx)
So previously gcc thought the register offset might be signed and
decided to sign extend it, just in case. The rest appears to be
mostly just minor shuffling of instructions.
v2: i915_mmio_reg_{offset,equal,valid}() helpers added
s/_REG/_MMIO/ in the register defines
mo more switch statements left to worry about
ring_emit stuff got sorted in a prep patch
cmd parser, lrc context and w/a batch buildup also in prep patch
vgpu stuff cleaned up and moved to a prep patch
all other unrelated changes split out
v3: Rebased due to BXT DSI/BLC, MOCS, etc.
v4: Rebased due to churn, s/i915_mmio_reg_t/i915_reg_t/
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1447853606-2751-1-git-send-email-ville.syrjala@linux.intel.com
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 358c9d5..aba115f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -685,18 +685,18 @@
void (*force_wake_put)(struct drm_i915_private *dev_priv,
enum forcewake_domains domains);
- uint8_t (*mmio_readb)(struct drm_i915_private *dev_priv, off_t offset, bool trace);
- uint16_t (*mmio_readw)(struct drm_i915_private *dev_priv, off_t offset, bool trace);
- uint32_t (*mmio_readl)(struct drm_i915_private *dev_priv, off_t offset, bool trace);
- uint64_t (*mmio_readq)(struct drm_i915_private *dev_priv, off_t offset, bool trace);
+ uint8_t (*mmio_readb)(struct drm_i915_private *dev_priv, i915_reg_t r, bool trace);
+ uint16_t (*mmio_readw)(struct drm_i915_private *dev_priv, i915_reg_t r, bool trace);
+ uint32_t (*mmio_readl)(struct drm_i915_private *dev_priv, i915_reg_t r, bool trace);
+ uint64_t (*mmio_readq)(struct drm_i915_private *dev_priv, i915_reg_t r, bool trace);
- void (*mmio_writeb)(struct drm_i915_private *dev_priv, off_t offset,
+ void (*mmio_writeb)(struct drm_i915_private *dev_priv, i915_reg_t r,
uint8_t val, bool trace);
- void (*mmio_writew)(struct drm_i915_private *dev_priv, off_t offset,
+ void (*mmio_writew)(struct drm_i915_private *dev_priv, i915_reg_t r,
uint16_t val, bool trace);
- void (*mmio_writel)(struct drm_i915_private *dev_priv, off_t offset,
+ void (*mmio_writel)(struct drm_i915_private *dev_priv, i915_reg_t r,
uint32_t val, bool trace);
- void (*mmio_writeq)(struct drm_i915_private *dev_priv, off_t offset,
+ void (*mmio_writeq)(struct drm_i915_private *dev_priv, i915_reg_t r,
uint64_t val, bool trace);
};
@@ -713,11 +713,11 @@
enum forcewake_domain_id id;
unsigned wake_count;
struct timer_list timer;
- u32 reg_set;
+ i915_reg_t reg_set;
u32 val_set;
u32 val_clear;
- u32 reg_ack;
- u32 reg_post;
+ i915_reg_t reg_ack;
+ i915_reg_t reg_post;
u32 val_reset;
} fw_domain[FW_DOMAIN_ID_COUNT];
};
@@ -743,7 +743,7 @@
uint32_t dmc_fw_size;
uint32_t version;
uint32_t mmio_count;
- uint32_t mmioaddr[8];
+ i915_reg_t mmioaddr[8];
uint32_t mmiodata[8];
};
@@ -996,7 +996,7 @@
struct i2c_adapter adapter;
u32 force_bit;
u32 reg0;
- u32 gpio_reg;
+ i915_reg_t gpio_reg;
struct i2c_algo_bit_data bit_algo;
struct drm_i915_private *dev_priv;
};
@@ -1645,7 +1645,7 @@
};
struct i915_wa_reg {
- u32 addr;
+ i915_reg_t addr;
u32 value;
/* bitmask representing WA bits */
u32 mask;
@@ -3434,16 +3434,16 @@
#define __raw_read(x, s) \
static inline uint##x##_t __raw_i915_read##x(struct drm_i915_private *dev_priv, \
- uint32_t reg) \
+ i915_reg_t reg) \
{ \
- return read##s(dev_priv->regs + reg); \
+ return read##s(dev_priv->regs + i915_mmio_reg_offset(reg)); \
}
#define __raw_write(x, s) \
static inline void __raw_i915_write##x(struct drm_i915_private *dev_priv, \
- uint32_t reg, uint##x##_t val) \
+ i915_reg_t reg, uint##x##_t val) \
{ \
- write##s(val, dev_priv->regs + reg); \
+ write##s(val, dev_priv->regs + i915_mmio_reg_offset(reg)); \
}
__raw_read(8, b)
__raw_read(16, w)
@@ -3474,7 +3474,7 @@
#define INTEL_BROADCAST_RGB_FULL 1
#define INTEL_BROADCAST_RGB_LIMITED 2
-static inline uint32_t i915_vgacntrl_reg(struct drm_device *dev)
+static inline i915_reg_t i915_vgacntrl_reg(struct drm_device *dev)
{
if (IS_VALLEYVIEW(dev))
return VLV_VGACNTRL;