On Wed, 12 Sep 2018, Madhav Chauhan <madhav.chauhan@xxxxxxxxx> wrote: > On 9/12/2018 1:00 AM, Jani Nikula wrote: >> On Tue, 10 Jul 2018, Madhav Chauhan <madhav.chauhan@xxxxxxxxx> wrote: >> The convention is to define macros for field values that you can OR >> directly in place instead of requiring a shift. Please stick to the >> conventions. Use _SHIFT and _MASK. >> >> We can debate the relative merits of both approaches at some point, but >> this is not the time. > > Just to understand this point correctly, > > #define OP_MODE(x) ((x) << 28) is OK This is acceptable when the values for the field are *not* defined as separate macros. The convention is that the values for fields are defined already shifted in place, and that would conflict. > but > #define OP_MODE_MASK (0x3 << 28) is NOT OK This is okay and recommended. > and should be: > #define OP_MODE_MASK 0x3 This is not okay. > #define OP_MODE_SHIFT 28 This is okay. Please read the big comment with examples near the top of i915_reg.h, and let me know which part is not clear from that. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx