[Intel-gfx] [PATCH v3 29/29] drm/i915: Type safe register read/write

[Chris Wilson]

On Wed, Nov 04, 2015 at 11:20:17PM +0200, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> 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.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

Looks very good,
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>

I looked at all the code conversions but only spot checked about 50% of
the reg wrapping in the headers.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre