Skip to content

Commit f0f59a0

Browse files
vsyrjalavsyrjala
committed
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
1 parent 9bca5d0 commit f0f59a0

35 files changed

+1540
-1573
lines changed

‎drivers/gpu/drm/i915/dvo.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ struct intel_dvo_device {
3232
const char *name;
3333
int type;
3434
/* DVOA/B/C output register */
35-
u32 dvo_reg;
36-
u32 dvo_srcdim_reg;
35+
i915_reg_t dvo_reg;
36+
i915_reg_t dvo_srcdim_reg;
3737
/* GPIO register used for i2c bus to control this device */
3838
u32 gpio;
3939
int slave_addr;

‎drivers/gpu/drm/i915/i915_cmd_parser.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ static const struct drm_i915_cmd_table hsw_blt_ring_cmds[] = {
407407
* LRI.
408408
*/
409409
struct drm_i915_reg_descriptor {
410-
u32 addr;
410+
i915_reg_t addr;
411411
u32 mask;
412412
u32 value;
413413
};
@@ -597,7 +597,7 @@ static bool check_sorted(int ring_id,
597597
bool ret = true;
598598

599599
for (i = 0; i < reg_count; i++) {
600-
u32 curr = reg_table[i].addr;
600+
u32 curr = i915_mmio_reg_offset(reg_table[i].addr);
601601

602602
if (curr < previous) {
603603
DRM_ERROR("CMD: table not sorted ring=%d entry=%d reg=0x%08X prev=0x%08X\n",
@@ -852,7 +852,7 @@ find_reg(const struct drm_i915_reg_descriptor *table,
852852
int i;
853853

854854
for (i = 0; i < count; i++) {
855-
if (table[i].addr == addr)
855+
if (i915_mmio_reg_offset(table[i].addr) == addr)
856856
return &table[i];
857857
}
858858
}
@@ -1028,7 +1028,7 @@ static bool check_cmd(const struct intel_engine_cs *ring,
10281028
* to the register. Hence, limit OACONTROL writes to
10291029
* only MI_LOAD_REGISTER_IMM commands.
10301030
*/
1031-
if (reg_addr == OACONTROL) {
1031+
if (reg_addr == i915_mmio_reg_offset(OACONTROL)) {
10321032
if (desc->cmd.value == MI_LOAD_REGISTER_MEM) {
10331033
DRM_DEBUG_DRIVER("CMD: Rejected LRM to OACONTROL\n");
10341034
return false;

‎drivers/gpu/drm/i915/i915_debugfs.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3267,7 +3267,8 @@ static int i915_wa_registers(struct seq_file *m, void *unused)
32673267

32683268
seq_printf(m, "Workarounds applied: %d\n", dev_priv->workarounds.count);
32693269
for (i = 0; i < dev_priv->workarounds.count; ++i) {
3270-
u32 addr, mask, value, read;
3270+
i915_reg_t addr;
3271+
u32 mask, value, read;
32713272
bool ok;
32723273

32733274
addr = dev_priv->workarounds.reg[i].addr;
@@ -3276,7 +3277,7 @@ static int i915_wa_registers(struct seq_file *m, void *unused)
32763277
read = I915_READ(addr);
32773278
ok = (value & mask) == (read & mask);
32783279
seq_printf(m, "0x%X: 0x%08X, mask: 0x%08X, read: 0x%08x, status: %s\n",
3279-
addr, value, mask, read, ok ? "OK" : "FAIL");
3280+
i915_mmio_reg_offset(addr), value, mask, read, ok ? "OK" : "FAIL");
32803281
}
32813282

32823283
intel_runtime_pm_put(dev_priv);

‎drivers/gpu/drm/i915/i915_drv.h

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -685,18 +685,18 @@ struct intel_uncore_funcs {
685685
void (*force_wake_put)(struct drm_i915_private *dev_priv,
686686
enum forcewake_domains domains);
687687

688-
uint8_t (*mmio_readb)(struct drm_i915_private *dev_priv, off_t offset, bool trace);
689-
uint16_t (*mmio_readw)(struct drm_i915_private *dev_priv, off_t offset, bool trace);
690-
uint32_t (*mmio_readl)(struct drm_i915_private *dev_priv, off_t offset, bool trace);
691-
uint64_t (*mmio_readq)(struct drm_i915_private *dev_priv, off_t offset, bool trace);
688+
uint8_t (*mmio_readb)(struct drm_i915_private *dev_priv, i915_reg_t r, bool trace);
689+
uint16_t (*mmio_readw)(struct drm_i915_private *dev_priv, i915_reg_t r, bool trace);
690+
uint32_t (*mmio_readl)(struct drm_i915_private *dev_priv, i915_reg_t r, bool trace);
691+
uint64_t (*mmio_readq)(struct drm_i915_private *dev_priv, i915_reg_t r, bool trace);
692692

693-
void (*mmio_writeb)(struct drm_i915_private *dev_priv, off_t offset,
693+
void (*mmio_writeb)(struct drm_i915_private *dev_priv, i915_reg_t r,
694694
uint8_t val, bool trace);
695-
void (*mmio_writew)(struct drm_i915_private *dev_priv, off_t offset,
695+
void (*mmio_writew)(struct drm_i915_private *dev_priv, i915_reg_t r,
696696
uint16_t val, bool trace);
697-
void (*mmio_writel)(struct drm_i915_private *dev_priv, off_t offset,
697+
void (*mmio_writel)(struct drm_i915_private *dev_priv, i915_reg_t r,
698698
uint32_t val, bool trace);
699-
void (*mmio_writeq)(struct drm_i915_private *dev_priv, off_t offset,
699+
void (*mmio_writeq)(struct drm_i915_private *dev_priv, i915_reg_t r,
700700
uint64_t val, bool trace);
701701
};
702702

@@ -713,11 +713,11 @@ struct intel_uncore {
713713
enum forcewake_domain_id id;
714714
unsigned wake_count;
715715
struct timer_list timer;
716-
u32 reg_set;
716+
i915_reg_t reg_set;
717717
u32 val_set;
718718
u32 val_clear;
719-
u32 reg_ack;
720-
u32 reg_post;
719+
i915_reg_t reg_ack;
720+
i915_reg_t reg_post;
721721
u32 val_reset;
722722
} fw_domain[FW_DOMAIN_ID_COUNT];
723723
};
@@ -743,7 +743,7 @@ struct intel_csr {
743743
uint32_t dmc_fw_size;
744744
uint32_t version;
745745
uint32_t mmio_count;
746-
uint32_t mmioaddr[8];
746+
i915_reg_t mmioaddr[8];
747747
uint32_t mmiodata[8];
748748
};
749749

@@ -996,7 +996,7 @@ struct intel_gmbus {
996996
struct i2c_adapter adapter;
997997
u32 force_bit;
998998
u32 reg0;
999-
u32 gpio_reg;
999+
i915_reg_t gpio_reg;
10001000
struct i2c_algo_bit_data bit_algo;
10011001
struct drm_i915_private *dev_priv;
10021002
};
@@ -1645,7 +1645,7 @@ struct i915_frontbuffer_tracking {
16451645
};
16461646

16471647
struct i915_wa_reg {
1648-
u32 addr;
1648+
i915_reg_t addr;
16491649
u32 value;
16501650
/* bitmask representing WA bits */
16511651
u32 mask;
@@ -3434,16 +3434,16 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val);
34343434

34353435
#define __raw_read(x, s) \
34363436
static inline uint##x##_t __raw_i915_read##x(struct drm_i915_private *dev_priv, \
3437-
uint32_t reg) \
3437+
i915_reg_t reg) \
34383438
{ \
3439-
return read##s(dev_priv->regs + reg); \
3439+
return read##s(dev_priv->regs + i915_mmio_reg_offset(reg)); \
34403440
}
34413441

34423442
#define __raw_write(x, s) \
34433443
static inline void __raw_i915_write##x(struct drm_i915_private *dev_priv, \
3444-
uint32_t reg, uint##x##_t val) \
3444+
i915_reg_t reg, uint##x##_t val) \
34453445
{ \
3446-
write##s(val, dev_priv->regs + reg); \
3446+
write##s(val, dev_priv->regs + i915_mmio_reg_offset(reg)); \
34473447
}
34483448
__raw_read(8, b)
34493449
__raw_read(16, w)
@@ -3474,7 +3474,7 @@ __raw_write(64, q)
34743474
#define INTEL_BROADCAST_RGB_FULL 1
34753475
#define INTEL_BROADCAST_RGB_LIMITED 2
34763476

3477-
static inline uint32_t i915_vgacntrl_reg(struct drm_device *dev)
3477+
static inline i915_reg_t i915_vgacntrl_reg(struct drm_device *dev)
34783478
{
34793479
if (IS_VALLEYVIEW(dev))
34803480
return VLV_VGACNTRL;

‎drivers/gpu/drm/i915/i915_gem_fence.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ static void i965_write_fence_reg(struct drm_device *dev, int reg,
5959
struct drm_i915_gem_object *obj)
6060
{
6161
struct drm_i915_private *dev_priv = dev->dev_private;
62-
int fence_reg_lo, fence_reg_hi;
62+
i915_reg_t fence_reg_lo, fence_reg_hi;
6363
int fence_pitch_shift;
6464

6565
if (INTEL_INFO(dev)->gen >= 6) {

‎drivers/gpu/drm/i915/i915_gpu_error.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -910,7 +910,7 @@ static void i915_record_ring_state(struct drm_device *dev,
910910
ering->ctl = I915_READ_CTL(ring);
911911

912912
if (I915_NEED_GFX_HWS(dev)) {
913-
int mmio;
913+
i915_reg_t mmio;
914914

915915
if (IS_GEN7(dev)) {
916916
switch (ring->id) {

‎drivers/gpu/drm/i915/i915_guc_reg.h

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626

2727
/* Definitions of GuC H/W registers, bits, etc */
2828

29-
#define GUC_STATUS 0xc000
29+
#define GUC_STATUS _MMIO(0xc000)
3030
#define GS_BOOTROM_SHIFT 1
3131
#define GS_BOOTROM_MASK (0x7F << GS_BOOTROM_SHIFT)
3232
#define GS_BOOTROM_RSA_FAILED (0x50 << GS_BOOTROM_SHIFT)
@@ -39,41 +39,41 @@
3939
#define GS_MIA_MASK (0x07 << GS_MIA_SHIFT)
4040
#define GS_MIA_CORE_STATE (1 << GS_MIA_SHIFT)
4141

42-
#define SOFT_SCRATCH(n) (0xc180 + ((n) * 4))
42+
#define SOFT_SCRATCH(n) _MMIO(0xc180 + (n) * 4)
4343

44-
#define UOS_RSA_SCRATCH(i) (0xc200 + (i) * 4)
44+
#define UOS_RSA_SCRATCH(i) _MMIO(0xc200 + (i) * 4)
4545
#define UOS_RSA_SCRATCH_MAX_COUNT 64
46-
#define DMA_ADDR_0_LOW 0xc300
47-
#define DMA_ADDR_0_HIGH 0xc304
48-
#define DMA_ADDR_1_LOW 0xc308
49-
#define DMA_ADDR_1_HIGH 0xc30c
46+
#define DMA_ADDR_0_LOW _MMIO(0xc300)
47+
#define DMA_ADDR_0_HIGH _MMIO(0xc304)
48+
#define DMA_ADDR_1_LOW _MMIO(0xc308)
49+
#define DMA_ADDR_1_HIGH _MMIO(0xc30c)
5050
#define DMA_ADDRESS_SPACE_WOPCM (7 << 16)
5151
#define DMA_ADDRESS_SPACE_GTT (8 << 16)
52-
#define DMA_COPY_SIZE 0xc310
53-
#define DMA_CTRL 0xc314
52+
#define DMA_COPY_SIZE _MMIO(0xc310)
53+
#define DMA_CTRL _MMIO(0xc314)
5454
#define UOS_MOVE (1<<4)
5555
#define START_DMA (1<<0)
56-
#define DMA_GUC_WOPCM_OFFSET 0xc340
56+
#define DMA_GUC_WOPCM_OFFSET _MMIO(0xc340)
5757
#define GUC_WOPCM_OFFSET_VALUE 0x80000 /* 512KB */
58-
#define GUC_MAX_IDLE_COUNT 0xC3E4
58+
#define GUC_MAX_IDLE_COUNT _MMIO(0xC3E4)
5959

60-
#define GUC_WOPCM_SIZE 0xc050
60+
#define GUC_WOPCM_SIZE _MMIO(0xc050)
6161
#define GUC_WOPCM_SIZE_VALUE (0x80 << 12) /* 512KB */
6262

6363
/* GuC addresses below GUC_WOPCM_TOP don't map through the GTT */
6464
#define GUC_WOPCM_TOP (GUC_WOPCM_SIZE_VALUE)
6565

66-
#define GEN8_GT_PM_CONFIG 0x138140
67-
#define GEN9LP_GT_PM_CONFIG 0x138140
68-
#define GEN9_GT_PM_CONFIG 0x13816c
66+
#define GEN8_GT_PM_CONFIG _MMIO(0x138140)
67+
#define GEN9LP_GT_PM_CONFIG _MMIO(0x138140)
68+
#define GEN9_GT_PM_CONFIG _MMIO(0x13816c)
6969
#define GT_DOORBELL_ENABLE (1<<0)
7070

71-
#define GEN8_GTCR 0x4274
71+
#define GEN8_GTCR _MMIO(0x4274)
7272
#define GEN8_GTCR_INVALIDATE (1<<0)
7373

74-
#define GUC_ARAT_C6DIS 0xA178
74+
#define GUC_ARAT_C6DIS _MMIO(0xA178)
7575

76-
#define GUC_SHIM_CONTROL 0xc064
76+
#define GUC_SHIM_CONTROL _MMIO(0xc064)
7777
#define GUC_DISABLE_SRAM_INIT_TO_ZEROES (1<<0)
7878
#define GUC_ENABLE_READ_CACHE_LOGIC (1<<1)
7979
#define GUC_ENABLE_MIA_CACHING (1<<2)
@@ -90,21 +90,21 @@
9090
GUC_ENABLE_READ_CACHE_FOR_WOPCM_DATA | \
9191
GUC_ENABLE_MIA_CLOCK_GATING)
9292

93-
#define HOST2GUC_INTERRUPT 0xc4c8
93+
#define HOST2GUC_INTERRUPT _MMIO(0xc4c8)
9494
#define HOST2GUC_TRIGGER (1<<0)
9595

9696
#define DRBMISC1 0x1984
9797
#define DOORBELL_ENABLE (1<<0)
9898

99-
#define GEN8_DRBREGL(x) (0x1000 + (x) * 8)
99+
#define GEN8_DRBREGL(x) _MMIO(0x1000 + (x) * 8)
100100
#define GEN8_DRB_VALID (1<<0)
101-
#define GEN8_DRBREGU(x) (GEN8_DRBREGL(x) + 4)
101+
#define GEN8_DRBREGU(x) _MMIO(0x1000 + (x) * 8 + 4)
102102

103-
#define DE_GUCRMR 0x44054
103+
#define DE_GUCRMR _MMIO(0x44054)
104104

105-
#define GUC_BCS_RCS_IER 0xC550
106-
#define GUC_VCS2_VCS1_IER 0xC554
107-
#define GUC_WD_VECS_IER 0xC558
108-
#define GUC_PM_P24C_IER 0xC55C
105+
#define GUC_BCS_RCS_IER _MMIO(0xC550)
106+
#define GUC_VCS2_VCS1_IER _MMIO(0xC554)
107+
#define GUC_WD_VECS_IER _MMIO(0xC558)
108+
#define GUC_PM_P24C_IER _MMIO(0xC55C)
109109

110110
#endif

‎drivers/gpu/drm/i915/i915_guc_submission.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ static void guc_disable_doorbell(struct intel_guc *guc,
258258
struct drm_i915_private *dev_priv = guc_to_i915(guc);
259259
struct guc_doorbell_info *doorbell;
260260
void *base;
261-
int drbreg = GEN8_DRBREGL(client->doorbell_id);
261+
i915_reg_t drbreg = GEN8_DRBREGL(client->doorbell_id);
262262
int value;
263263

264264
base = kmap_atomic(i915_gem_object_get_page(client->client_obj, 0));

0 commit comments

Comments
 (0)