[Intel-gfx] [PATCH v3 3/3] drm/i915: use REG_FIELD_PREP() to define register bitfield values

Michal Wajdeczko michal.wajdeczko at intel.com
Thu Feb 28 00:17:31 UTC 2019


On Wed, 27 Feb 2019 18:02:38 +0100, Jani Nikula <jani.nikula at intel.com>  
wrote:

> @@ -108,9 +108,9 @@
>   *  #define FOO(pipe)                   _MMIO_PIPE(pipe, _FOO_A, _FOO_B)
>   *  #define   FOO_ENABLE                REG_BIT(31)
>   *  #define   FOO_MODE_MASK             REG_GENMASK(19, 16)
> - *  #define   FOO_MODE_BAR              (0 << 16)
> - *  #define   FOO_MODE_BAZ              (1 << 16)
> - *  #define   FOO_MODE_QUX_SNB          (2 << 16)
> + *  #define   FOO_MODE_BAR              REG_FIELD_PREP(FOO_MODE_MASK, 0)
> + *  #define   FOO_MODE_BAZ              REG_FIELD_PREP(FOO_MODE_MASK, 1)
> + *  #define   FOO_MODE_QUX_SNB          REG_FIELD_PREP(FOO_MODE_MASK, 2)

hmm, shouldn't we define these values as:

#define   FOO_MODE_BAR              (0)
#define   FOO_MODE_BAZ              (1)
#define   FOO_MODE_QUX_SNB          (2)

to allow using them natively with REG_FIELD_GET/PREPARE() ?
maybe we should also consider dropping _MASK suffix?

MMIO_WRITE(...,
            REG_FIELD_PREPARE(FOO_ENABLE, true) |
            REG_FIELD_PREPARE(FOO_MODE, FOO_MODE_BAR))

mode = REG_FIELD_GET(FOO_MODE, MMIO_READ(...));
enabled = REG_FIELD_GET(FOO_ENABLE, MMIO_READ(...));

Thanks,
Michal



More information about the Intel-gfx mailing list