On Fri, Sep 18, 2015 at 08:03:56PM +0300, ville.syrjala@xxxxxxxxxxxxxxx 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
