On Mon, 10 Jan 2022, Matt Roper <matthew.d.roper@xxxxxxxxx> wrote:
> We'd like to start splitting i915_reg.h into various domain-specific
> register files and cleaning them up. Let's move the basic macros and
> type definitions to their own header file that can be including in each
> of the new split headers.
>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
One nitpick near the end, can be fixed while applying, otherwise,
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 88 +------------------------
> drivers/gpu/drm/i915/i915_reg_defs.h | 98 ++++++++++++++++++++++++++++
> 2 files changed, 99 insertions(+), 87 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/i915_reg_defs.h
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 25f6bde36add..b7e03b6e886d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -25,8 +25,7 @@
> #ifndef _I915_REG_H_
> #define _I915_REG_H_
>
> -#include <linux/bitfield.h>
> -#include <linux/bits.h>
> +#include "i915_reg_defs.h"
>
> /**
> * DOC: The i915 register macro definition style guide
> @@ -116,91 +115,6 @@
> * #define GEN8_BAR _MMIO(0xb888)
> */
>
> -/**
> - * REG_BIT() - Prepare a u32 bit value
> - * @__n: 0-based bit number
> - *
> - * Local wrapper for BIT() to force u32, with compile time checks.
> - *
> - * @return: Value with bit @__n set.
> - */
> -#define REG_BIT(__n) \
> - ((u32)(BIT(__n) + \
> - BUILD_BUG_ON_ZERO(__is_constexpr(__n) && \
> - ((__n) < 0 || (__n) > 31))))
> -
> -/**
> - * REG_GENMASK() - Prepare a continuous u32 bitmask
> - * @__high: 0-based high bit
> - * @__low: 0-based low bit
> - *
> - * Local wrapper for GENMASK() to force u32, with compile time checks.
> - *
> - * @return: Continuous bitmask from @__high to @__low, inclusive.
> - */
> -#define REG_GENMASK(__high, __low) \
> - ((u32)(GENMASK(__high, __low) + \
> - BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \
> - __is_constexpr(__low) && \
> - ((__low) < 0 || (__high) > 31 || (__low) > (__high)))))
> -
> -/*
> - * Local integer constant expression version of is_power_of_2().
> - */
> -#define IS_POWER_OF_2(__x) ((__x) && (((__x) & ((__x) - 1)) == 0))
> -
> -/**
> - * REG_FIELD_PREP() - Prepare a u32 bitfield value
> - * @__mask: shifted mask defining the field's length and position
> - * @__val: value to put in the field
> - *
> - * Local copy of FIELD_PREP() to generate an integer constant expression, force
> - * u32 and for consistency with REG_FIELD_GET(), REG_BIT() and REG_GENMASK().
> - *
> - * @return: @__val masked and shifted into the field defined by @__mask.
> - */
> -#define REG_FIELD_PREP(__mask, __val) \
> - ((u32)((((typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask)) + \
> - BUILD_BUG_ON_ZERO(!__is_constexpr(__mask)) + \
> - BUILD_BUG_ON_ZERO((__mask) == 0 || (__mask) > U32_MAX) + \
> - BUILD_BUG_ON_ZERO(!IS_POWER_OF_2((__mask) + (1ULL << __bf_shf(__mask)))) + \
> - BUILD_BUG_ON_ZERO(__builtin_choose_expr(__is_constexpr(__val), (~((__mask) >> __bf_shf(__mask)) & (__val)), 0))))
> -
> -/**
> - * REG_FIELD_GET() - Extract a u32 bitfield value
> - * @__mask: shifted mask defining the field's length and position
> - * @__val: value to extract the bitfield value from
> - *
> - * Local wrapper for FIELD_GET() to force u32 and for consistency with
> - * REG_FIELD_PREP(), REG_BIT() and REG_GENMASK().
> - *
> - * @return: Masked and shifted value of the field defined by @__mask in @__val.
> - */
> -#define REG_FIELD_GET(__mask, __val) ((u32)FIELD_GET(__mask, __val))
> -
> -typedef struct {
> - u32 reg;
> -} i915_reg_t;
> -
> -#define _MMIO(r) ((const i915_reg_t){ .reg = (r) })
> -
> -#define INVALID_MMIO_REG _MMIO(0)
> -
> -static __always_inline u32 i915_mmio_reg_offset(i915_reg_t reg)
> -{
> - return reg.reg;
> -}
> -
> -static inline bool i915_mmio_reg_equal(i915_reg_t a, i915_reg_t b)
> -{
> - return i915_mmio_reg_offset(a) == i915_mmio_reg_offset(b);
> -}
> -
> -static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> -{
> - return !i915_mmio_reg_equal(reg, INVALID_MMIO_REG);
> -}
> -
> #define VLV_DISPLAY_BASE 0x180000
> #define VLV_MIPI_BASE VLV_DISPLAY_BASE
> #define BXT_MIPI_BASE 0x60000
> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h b/drivers/gpu/drm/i915/i915_reg_defs.h
> new file mode 100644
> index 000000000000..5f64aa086ace
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
> @@ -0,0 +1,98 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#ifndef __I915_REG_DEFS__
> +#define __I915_REG_DEFS__
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +
> +/**
> + * REG_BIT() - Prepare a u32 bit value
> + * @__n: 0-based bit number
> + *
> + * Local wrapper for BIT() to force u32, with compile time checks.
> + *
> + * @return: Value with bit @__n set.
> + */
> +#define REG_BIT(__n) \
> + ((u32)(BIT(__n) + \
> + BUILD_BUG_ON_ZERO(__is_constexpr(__n) && \
> + ((__n) < 0 || (__n) > 31))))
> +
> +/**
> + * REG_GENMASK() - Prepare a continuous u32 bitmask
> + * @__high: 0-based high bit
> + * @__low: 0-based low bit
> + *
> + * Local wrapper for GENMASK() to force u32, with compile time checks.
> + *
> + * @return: Continuous bitmask from @__high to @__low, inclusive.
> + */
> +#define REG_GENMASK(__high, __low) \
> + ((u32)(GENMASK(__high, __low) + \
> + BUILD_BUG_ON_ZERO(__is_constexpr(__high) && \
> + __is_constexpr(__low) && \
> + ((__low) < 0 || (__high) > 31 || (__low) > (__high)))))
> +
> +/*
> + * Local integer constant expression version of is_power_of_2().
> + */
> +#define IS_POWER_OF_2(__x) ((__x) && (((__x) & ((__x) - 1)) == 0))
> +
> +/**
> + * REG_FIELD_PREP() - Prepare a u32 bitfield value
> + * @__mask: shifted mask defining the field's length and position
> + * @__val: value to put in the field
> + *
> + * Local copy of FIELD_PREP() to generate an integer constant expression, force
> + * u32 and for consistency with REG_FIELD_GET(), REG_BIT() and REG_GENMASK().
> + *
> + * @return: @__val masked and shifted into the field defined by @__mask.
> + */
> +#define REG_FIELD_PREP(__mask, __val) \
> + ((u32)((((typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask)) + \
> + BUILD_BUG_ON_ZERO(!__is_constexpr(__mask)) + \
> + BUILD_BUG_ON_ZERO((__mask) == 0 || (__mask) > U32_MAX) + \
> + BUILD_BUG_ON_ZERO(!IS_POWER_OF_2((__mask) + (1ULL << __bf_shf(__mask)))) + \
> + BUILD_BUG_ON_ZERO(__builtin_choose_expr(__is_constexpr(__val), (~((__mask) >> __bf_shf(__mask)) & (__val)), 0))))
> +
> +/**
> + * REG_FIELD_GET() - Extract a u32 bitfield value
> + * @__mask: shifted mask defining the field's length and position
> + * @__val: value to extract the bitfield value from
> + *
> + * Local wrapper for FIELD_GET() to force u32 and for consistency with
> + * REG_FIELD_PREP(), REG_BIT() and REG_GENMASK().
> + *
> + * @return: Masked and shifted value of the field defined by @__mask in @__val.
> + */
> +#define REG_FIELD_GET(__mask, __val) ((u32)FIELD_GET(__mask, __val))
> +
> +typedef struct {
> + u32 reg;
> +} i915_reg_t;
> +
> +#define _MMIO(r) ((const i915_reg_t){ .reg = (r) })
> +
> +#define INVALID_MMIO_REG _MMIO(0)
> +
> +static __always_inline u32 i915_mmio_reg_offset(i915_reg_t reg)
> +{
> + return reg.reg;
> +}
> +
> +static inline bool i915_mmio_reg_equal(i915_reg_t a, i915_reg_t b)
> +{
> + return i915_mmio_reg_offset(a) == i915_mmio_reg_offset(b);
> +}
> +
> +static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> +{
> + return !i915_mmio_reg_equal(reg, INVALID_MMIO_REG);
> +}
> +
> +
The double newline is going to give a checkpatch complaint.
> +#endif /* __I915_REG_DEFS__ */
--
Jani Nikula, Intel Open Source Graphics Center
