[Intel-gfx] [PATCH v5 12/13] drm/i915/icl: Define TRANS_DSI_FUNC_CONF register

[Jani Nikula]

On Wed, 12 Sep 2018, Madhav Chauhan <madhav.chauhan at intel.com> wrote:
> On 9/12/2018 1:00 AM, Jani Nikula wrote:
>> On Tue, 10 Jul 2018, Madhav Chauhan <madhav.chauhan at intel.com> 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