[Intel-gfx] [PATCH v2 0/4] drm/i915: introduce macros to define register contents

[Chris Wilson]

Quoting Jani Nikula (2019-02-27 11:01:58)
> On Thu, 17 Jan 2019, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > Quoting Jani Nikula (2019-01-17 12:13:59)
> >> The v1 [1] kind of died down because the FIELD_PREP() build-time checks
> >> were lost as it didn't evaluate to integer constant expression. I looked
> >> at this again, and managed to include the checks in the local copy by
> >> using BUILD_BUG_ON_ZERO() instead.
> >> 
> >> On the naming bikeshedding department, I noticed a clash with regmap.h
> >> REG_FIELD() and, since it all looked pretty verbose anyway, decided to
> >> see if local _BIT(), _MASK(), and _FIELD() would stick.
> >
> > MMIO_BIT, MMIO_MASK, MMIO_FIELD
> > #define MMIO_PREP(mask, val) FIELD_PREP(mask, val)
> 
> This would mean two wrappers/implementations for FIELD_PREP(), plus the
> original.
> 
> > ?
> >
> > The leading underscore and the inconsistency with FIELD_PREP is getting
> > to me.
> 
> Fair enough.
> 
> So we need constant expression and/or u32 implementations or wrappers
> for BIT(), GENMASK(), and FIELD_PREP(). Do we want to use wrappers for
> FIELD_PREP() and FIELD_GET() in code outside of the macro definitions?
> Might be nice for consistency.
> 
> I'm not fond of the MMIO prefix here, mostly because these are not
> strictly related to MMIO. Feels like conflating too much.
> 
> BIT
> -> REG_BIT
> -> I915_BIT
> 
> GENMASK
> -> REG_MASK
> -> REG_GENMASK
> -> I915_MASK
> -> I915_GENMASK
> 
> FIELD_PREP
> -> REG_BITFIELD
> -> REG_FIELD_PREP
> -> I915_FIELD
> -> I915_FIELD_PREP
> -> I915_BITFIELD
> 
> FIELD_GET (not strictly needed)
> -> REG_FIELD_GET
> -> I915_FIELD_GET
> 
> Dunno, I kind of liked the short underscored versions, but if we're
> going to make them longer, why not just prefix all the originals with
> REG_ or I915_ and be done with it?

REG_* works for me. I prefer REG over I915 as I think using I915 implies
that this is something special for our hw, whereas we are just trying to
interact with registers. And I think there will be no harm in us making
this include/linux/regfield.h with a nod to bitfield.h
-Chris