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

[Jani Nikula <jani.nikula@intel.com>]

On Thu, 17 Jan 2019, Chris Wilson <chris@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?

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx