On Wed, 27 Feb 2019 18:02:38 +0100, Jani Nikula <jani.nikula@xxxxxxxxx>
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
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
