On Fri, 2017-08-04 at 13:38 +0300, Jani Nikula wrote:
> This is not to try to force a new style; this is my interpretation of
> what the most common existing style is.
>
> With hopes I don't need to answer so many questions about style going
> forward.
>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>
> ---
>
> N.b. only the *interpretation* of the existing style is up for debate
> here. Proposals to *change* the style going forward can be done in other
> patches changing this description. However, I doubt the usefulness of
> such changes, with the possible exception of promoting the use of BIT().
> ---
> drivers/gpu/drm/i915/i915_reg.h | 77 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 77 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b2546ade2c45..324cf04d6bfe 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -25,6 +25,83 @@
> #ifndef _I915_REG_H_
> #define _I915_REG_H_
>
> +/*
> + * The i915 register macro definition style guide.
> + *
> + * Follow the style described here for new macros, and while changing existing
> + * macros. Do not mass change existing definitions just to update the style.
> + *
> + * LAYOUT
> + *
> + * Keep helper macros near the top. For example, _PIPE() and friends.
> + *
> + * Prefix macros that generally should not be used outside of this file with
> + * underscore '_'. For example, _PIPE() and friends, single instances of
> + * registers that are defined solely for the use by function-like macros.
> + *
> + * Avoid using the underscore prefixed macros outside of this file. There are
> + * exceptions, but keep them to a minimum.
> + *
> + * There are two basic types of register definitions: Single registers and
> + * register groups. Register groups are registers which have two or more
> + * instances, for example one per pipe, port, transcoder, etc. Register groups
> + * should be defined using function-like macros.
> + *
> + * For single registers, define the register offset first, followed by register
> + * contents.
> + *
> + * For register groups, define the register instance offsets first, prefixed
> + * with underscore, followed by a function-like macro choosing the right
> + * instance based on the parameter, followed by register contents.
> + *
> + * Define the register contents from most significant to least significant
> + * bit. Indent the bit and bit field macros using two extra spaces between
> + * #define and the macro name.
> + *
> + * For bit fields, define a _MASK and a _SHIFT macro. Define bit field contents
> + * so that they are already shifted in place, and can be directly OR'd. For
> + * convenience, function-like macros may be used to define bit fields, but do
> + * note that the macros may be needed to read as well as write the register
> + * contents.
Thanks for writing this!
Do you mind including an example for defining bit-fields using
function-like macros?
With or without that, since this guide agrees with most of the existing
definitions in i915_reg.h
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> + *
> + * Define bits using (1 << N) instead of BIT(N). We may change this in the
> + * future, but this is the prevailing style.
> + *
> + * Group the register and its contents together without blank lines, separate
> + * from other registers and their contents with one blank line.
> + *
> + * Indent macro values from macro names using TABs. Use braces in macro values
> + * as needed to avoid unintended precedence after macro substitution. Use spaces
> + * in macro values according to kernel coding style. Use lower case in
> + * hexadecimal values.
> + *
> + * NAMING
> + *
> + * Try to name registers according to the specs. If the register name changes in
> + * the specs from platform to another, stick to the original name.
> + *
> + * Try to re-use existing register macro definitions. Only add new macros for
> + * new register offsets, or when the register contents have changed enough to
> + * warrant a full redefinition.
> + *
> + * When a register or a bit (field) changes for a new platform, prefix the new
> + * macro using the platform acronym or generation. For example, SKL_ or
> + * GEN8_. The prefix signifies the start platform/generation of the register.
> + *
> + * EXAMPLE
> + *
> + * #define _FOO_A 0xf000
> + * #define _FOO_B 0xf001
> + * #define FOO(pipe) _MMIO_PIPE(pipe, _FOO_A, _FOO_B)
> + * #define FOO_ENABLE (1 << 31)
> + * #define FOO_MODE_MASK (0xf << 16)
> + * #define FOO_MODE_SHIFT 16
> + * #define FOO_MODE_BAR (0 << 16)
> + * #define FOO_MODE_BAZ (1 << 16)
> + * #define GEN6_FOO_MODE_QUX (2 << 16)
> + *
> + */
> +
> typedef struct {
> uint32_t reg;
> } i915_reg_t;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
