[Intel-gfx] [RFC][PATCH 43/43] WIP: drm/i915: Type safe register read/write

[Chris Wilson]

On Fri, Sep 18, 2015 at 08:03:56PM +0300, ville.syrjala at linux.intel.com wrote:
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 140076d..0589aba 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -25,16 +25,28 @@
>  #ifndef _I915_REG_H_
>  #define _I915_REG_H_
>  
> +struct i915_reg {
> +	uint32_t reg;
> +};

Fundamental objection averted. Maybe even typedef it to hide the struct,
ignornace is bliss.  i915_reg_t reg; looks reasonable in this instance.

For sanity's sake could you disassemble a simple function and include the
diff in the changelog. It will be reasuring to know that gcc handles the
struct-in-register just fine.

Once upon a time the plan was to use sparse for detecting type
mismatches, hence enum plane and enum pipe, but I know I don't run make
C=1 regularly at all, so using gcc itself is a win.

The downside with this patch is that we have a lot of places that cares
about the reg.reg, and even more where we make up reg on the fly, so it
looks like we are just doing I915_WRITE(_REG(old_u32), x) i.e. more or
less subverting the extra safety, and we would still want something like

	i915_write16(reg) { WARN_ON(reg & 1); }
	i915_write32(reg) { WARN_ON(reg & 3); }

to catch the class of bug where we create an invalid reg address.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre