On Wed, 27 Feb 2019 18:02:38 +0100, Jani Nikula <[email protected]> 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

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to