Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v5 2/5] drm: bridge: dw_mipi_dsi: abstract register
 access using reg_fields
To:     Adrian Ratiu <adrian.ratiu@collabora.com>,
        linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org
Cc:     Jernej Skrabec <jernej.skrabec@siol.net>,
        Jonas Karlman <jonas@kwiboo.se>, linux-kernel@vger.kernel.org,
        dri-devel@lists.freedesktop.org,
        Emil Velikov <emil.velikov@collabora.com>,
        linux-rockchip@lists.infradead.org,
        Boris Brezillon <boris.brezillon@collabora.com>,
        linux-imx@nxp.com, kernel@collabora.com,
        linux-stm32@st-md-mailman.stormreply.com,
        Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
From:   Andrzej Hajda <a.hajda@samsung.com>
Message-ID: <37751774-ba48-39d0-3b1b-4ac5363420ae@samsung.com>
Date:   Mon, 6 Apr 2020 17:39:55 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
        Thunderbird/68.6.0
MIME-Version: 1.0
In-Reply-To: <20200330113542.181752-3-adrian.ratiu@collabora.com>
Content-Transfer-Encoding: 8bit
Content-Language: pl
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <20200330113542.181752-1-adrian.ratiu@collabora.com>
        <CGME20200330113504eucas1p15157d5c21e1e42cb1d5638d03903a5af@eucas1p1.samsung.com>
        <20200330113542.181752-3-adrian.ratiu@collabora.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


W dniu 30.03.2020 o 13:35, Adrian Ratiu pisze:
> Register existence, address/offsets, field layouts, reserved bits and
> so on differ between MIPI-DSI versions and between SoC vendor boards.
> Despite these differences the hw IP and protocols are mostly the same
> so the generic bridge can be made to compensate these differences.
>
> The current Rockchip and STM drivers hardcoded a lot of their common
> definitions in the bridge code because they're based on DSI v1.30 and
> 1.31 which are relatively close, but in order to support older/future
> versions with more diverging layouts like the v1.01 present on imx6,
> we abstract some of the register accesses via the regmap field APIs.
>
> The bridge detects the DSI core version and initializes the required
> regmap register layout. Other DSI versions / register layouts can
> easily be added in the future by only changing the bridge code.
>
> The platform drivers don't require any changes, DSI register layout
> versioning will be handled transparently by the bridge, but if in
> the future the regmap or layouts needs to be exposed to the drivres,
> it could easily be done via plat_data or a new API in dw_mipi_dsi.h.
>
> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
> Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> ---
> Changes since v4:
>    - Move regmap infrastructure logic to a separate commit (Ezequiel)
>    - Consolidate field infrastructure in this commit (Ezequiel)
>    - Move the dsi v1.01 layout logic to a separate commit (Ezequiel)
>
> Changes since v2:
>    - Added const declarations to dw_mipi_dsi structs (Emil)
>    - Fixed commit tags (Emil)
>
> Changes since v1:
>    - Moved register definitions & regmap initialization into bridge
>    module. Platform drivers get the regmap via plat_data after calling
>    the bridge probe (Emil).
> ---
>   drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 510 ++++++++++++------
>   1 file changed, 352 insertions(+), 158 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 6d9e2f21c9cc..5b78ff925af0 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -31,6 +31,7 @@
>   #include <drm/drm_probe_helper.h>
>   
>   #define HWVER_131			0x31333100	/* IP version 1.31 */
> +#define HWVER_130			0x31333000	/* IP version 1.30 */
>   
>   #define DSI_VERSION			0x00
>   #define VERSION				GENMASK(31, 8)
> @@ -47,7 +48,6 @@
>   #define DPI_VCID(vcid)			((vcid) & 0x3)
>   
>   #define DSI_DPI_COLOR_CODING		0x10
> -#define LOOSELY18_EN			BIT(8)
>   #define DPI_COLOR_CODING_16BIT_1	0x0
>   #define DPI_COLOR_CODING_16BIT_2	0x1
>   #define DPI_COLOR_CODING_16BIT_3	0x2
> @@ -56,11 +56,6 @@
>   #define DPI_COLOR_CODING_24BIT		0x5
>   
>   #define DSI_DPI_CFG_POL			0x14
> -#define COLORM_ACTIVE_LOW		BIT(4)
> -#define SHUTD_ACTIVE_LOW		BIT(3)
> -#define HSYNC_ACTIVE_LOW		BIT(2)
> -#define VSYNC_ACTIVE_LOW		BIT(1)
> -#define DATAEN_ACTIVE_LOW		BIT(0)
>   
>   #define DSI_DPI_LP_CMD_TIM		0x18
>   #define OUTVACT_LPCMD_TIME(p)		(((p) & 0xff) << 16)
> @@ -81,27 +76,19 @@
>   #define DSI_GEN_VCID			0x30
>   
>   #define DSI_MODE_CFG			0x34
> -#define ENABLE_VIDEO_MODE		0
> -#define ENABLE_CMD_MODE			BIT(0)
>   
>   #define DSI_VID_MODE_CFG		0x38
> -#define ENABLE_LOW_POWER		(0x3f << 8)
> -#define ENABLE_LOW_POWER_MASK		(0x3f << 8)
> +#define ENABLE_LOW_POWER		0x3f
> +
>   #define VID_MODE_TYPE_NON_BURST_SYNC_PULSES	0x0
>   #define VID_MODE_TYPE_NON_BURST_SYNC_EVENTS	0x1
>   #define VID_MODE_TYPE_BURST			0x2
> -#define VID_MODE_TYPE_MASK			0x3
> -#define VID_MODE_VPG_ENABLE		BIT(16)
> -#define VID_MODE_VPG_HORIZONTAL		BIT(24)
>   
>   #define DSI_VID_PKT_SIZE		0x3c
> -#define VID_PKT_SIZE(p)			((p) & 0x3fff)
>   
>   #define DSI_VID_NUM_CHUNKS		0x40
> -#define VID_NUM_CHUNKS(c)		((c) & 0x1fff)
>   
>   #define DSI_VID_NULL_SIZE		0x44
> -#define VID_NULL_SIZE(b)		((b) & 0x1fff)
>   
>   #define DSI_VID_HSA_TIME		0x48
>   #define DSI_VID_HBP_TIME		0x4c
> @@ -125,7 +112,6 @@
>   #define GEN_SW_2P_TX_LP			BIT(10)
>   #define GEN_SW_1P_TX_LP			BIT(9)
>   #define GEN_SW_0P_TX_LP			BIT(8)
> -#define ACK_RQST_EN			BIT(1)
>   #define TEAR_FX_EN			BIT(0)
>   
>   #define CMD_MODE_ALL_LP			(MAX_RD_PKT_SIZE_LP | \
> @@ -154,8 +140,6 @@
>   #define GEN_CMD_EMPTY			BIT(0)
>   
>   #define DSI_TO_CNT_CFG			0x78
> -#define HSTX_TO_CNT(p)			(((p) & 0xffff) << 16)
> -#define LPRX_TO_CNT(p)			((p) & 0xffff)
>   
>   #define DSI_HS_RD_TO_CNT		0x7c
>   #define DSI_LP_RD_TO_CNT		0x80
> @@ -164,52 +148,17 @@
>   #define DSI_BTA_TO_CNT			0x8c
>   
>   #define DSI_LPCLK_CTRL			0x94
> -#define AUTO_CLKLANE_CTRL		BIT(1)
> -#define PHY_TXREQUESTCLKHS		BIT(0)
> -
>   #define DSI_PHY_TMR_LPCLK_CFG		0x98
> -#define PHY_CLKHS2LP_TIME(lbcc)		(((lbcc) & 0x3ff) << 16)
> -#define PHY_CLKLP2HS_TIME(lbcc)		((lbcc) & 0x3ff)
> -
>   #define DSI_PHY_TMR_CFG			0x9c
> -#define PHY_HS2LP_TIME(lbcc)		(((lbcc) & 0xff) << 24)
> -#define PHY_LP2HS_TIME(lbcc)		(((lbcc) & 0xff) << 16)
> -#define MAX_RD_TIME(lbcc)		((lbcc) & 0x7fff)
> -#define PHY_HS2LP_TIME_V131(lbcc)	(((lbcc) & 0x3ff) << 16)
> -#define PHY_LP2HS_TIME_V131(lbcc)	((lbcc) & 0x3ff)
> -
>   #define DSI_PHY_RSTZ			0xa0
> -#define PHY_DISFORCEPLL			0
> -#define PHY_ENFORCEPLL			BIT(3)
> -#define PHY_DISABLECLK			0
> -#define PHY_ENABLECLK			BIT(2)
> -#define PHY_RSTZ			0
> -#define PHY_UNRSTZ			BIT(1)
> -#define PHY_SHUTDOWNZ			0
> -#define PHY_UNSHUTDOWNZ			BIT(0)
> -
>   #define DSI_PHY_IF_CFG			0xa4
> -#define PHY_STOP_WAIT_TIME(cycle)	(((cycle) & 0xff) << 8)
> -#define N_LANES(n)			(((n) - 1) & 0x3)
> -
> -#define DSI_PHY_ULPS_CTRL		0xa8
> -#define DSI_PHY_TX_TRIGGERS		0xac
>   
>   #define DSI_PHY_STATUS			0xb0
>   #define PHY_STOP_STATE_CLK_LANE		BIT(2)
>   #define PHY_LOCK			BIT(0)
>   
>   #define DSI_PHY_TST_CTRL0		0xb4
> -#define PHY_TESTCLK			BIT(1)
> -#define PHY_UNTESTCLK			0
> -#define PHY_TESTCLR			BIT(0)
> -#define PHY_UNTESTCLR			0
> -
>   #define DSI_PHY_TST_CTRL1		0xb8
> -#define PHY_TESTEN			BIT(16)
> -#define PHY_UNTESTEN			0
> -#define PHY_TESTDOUT(n)			(((n) & 0xff) << 8)
> -#define PHY_TESTDIN(n)			((n) & 0xff)
>   
>   #define DSI_INT_ST0			0xbc
>   #define DSI_INT_ST1			0xc0
> @@ -217,7 +166,6 @@
>   #define DSI_INT_MSK1			0xc8
>   
>   #define DSI_PHY_TMR_RD_CFG		0xf4
> -#define MAX_RD_TIME_V131(lbcc)		((lbcc) & 0x7fff)
>   
>   #define PHY_STATUS_TIMEOUT_US		10000
>   #define CMD_PKT_STATUS_TIMEOUT_US	20000
> @@ -250,6 +198,53 @@ struct dw_mipi_dsi {
>   	struct dw_mipi_dsi *slave; /* dual-dsi slave ptr */
>   
>   	const struct dw_mipi_dsi_plat_data *plat_data;
> +
> +	struct regmap_field	*field_dpi_18loosely_en;
> +	struct regmap_field	*field_dpi_color_coding;
> +	struct regmap_field	*field_dpi_vid;
> +	struct regmap_field	*field_dpi_vsync_active_low;
> +	struct regmap_field	*field_dpi_hsync_active_low;
> +	struct regmap_field	*field_cmd_mode_ack_rqst_en;
> +	struct regmap_field	*field_cmd_mode_all_lp_en;
> +	struct regmap_field	*field_cmd_mode_en;
> +	struct regmap_field	*field_cmd_pkt_status;
> +	struct regmap_field	*field_vid_mode_en;
> +	struct regmap_field	*field_vid_mode_type;
> +	struct regmap_field	*field_vid_mode_low_power;
> +	struct regmap_field	*field_vid_mode_vpg_en;
> +	struct regmap_field	*field_vid_mode_vpg_horiz;
> +	struct regmap_field	*field_vid_pkt_size;
> +	struct regmap_field	*field_vid_hsa_time;
> +	struct regmap_field	*field_vid_hbp_time;
> +	struct regmap_field	*field_vid_hline_time;
> +	struct regmap_field	*field_vid_vsa_time;
> +	struct regmap_field	*field_vid_vbp_time;
> +	struct regmap_field	*field_vid_vfp_time;
> +	struct regmap_field	*field_vid_vactive_time;
> +	struct regmap_field	*field_phy_txrequestclkhs;
> +	struct regmap_field	*field_phy_bta_time;
> +	struct regmap_field	*field_phy_max_rd_time;
> +	struct regmap_field	*field_phy_lp2hs_time;
> +	struct regmap_field	*field_phy_hs2lp_time;
> +	struct regmap_field	*field_phy_clklp2hs_time;
> +	struct regmap_field	*field_phy_clkhs2lp_time;
> +	struct regmap_field	*field_phy_testclr;
> +	struct regmap_field	*field_phy_unshutdownz;
> +	struct regmap_field	*field_phy_unrstz;
> +	struct regmap_field	*field_phy_enableclk;
> +	struct regmap_field	*field_phy_forcepll;
> +	struct regmap_field	*field_phy_nlanes;
> +	struct regmap_field	*field_phy_stop_wait_time;
> +	struct regmap_field	*field_phy_status;
> +	struct regmap_field	*field_pckhdl_cfg;
> +	struct regmap_field	*field_hstx_timeout_counter;
> +	struct regmap_field	*field_lprx_timeout_counter;
> +	struct regmap_field	*field_int_stat0;
> +	struct regmap_field	*field_int_stat1;
> +	struct regmap_field	*field_int_mask0;
> +	struct regmap_field	*field_int_mask1;
> +	struct regmap_field	*field_gen_hdr;
> +	struct regmap_field	*field_gen_payload;
>   };
>   
>   static const struct regmap_config dw_mipi_dsi_regmap_cfg = {
> @@ -259,6 +254,111 @@ static const struct regmap_config dw_mipi_dsi_regmap_cfg = {
>   	.name = "dw-mipi-dsi",
>   };
>   
> +struct dw_mipi_dsi_variant {
> +	/* Regmap field configs for DSI adapter */
> +	struct reg_field	cfg_dpi_18loosely_en;
> +	struct reg_field	cfg_dpi_color_coding;
> +	struct reg_field	cfg_dpi_vid;
> +	struct reg_field	cfg_dpi_vsync_active_low;
> +	struct reg_field	cfg_dpi_hsync_active_low;
> +	struct reg_field	cfg_cmd_mode_en;
> +	struct reg_field	cfg_cmd_mode_ack_rqst_en;
> +	struct reg_field	cfg_cmd_mode_all_lp_en;
> +	struct reg_field	cfg_cmd_pkt_status;
> +	struct reg_field	cfg_vid_mode_en;
> +	struct reg_field	cfg_vid_mode_type;
> +	struct reg_field	cfg_vid_mode_low_power;
> +	struct reg_field	cfg_vid_mode_vpg_en;
> +	struct reg_field	cfg_vid_mode_vpg_horiz;
> +	struct reg_field	cfg_vid_pkt_size;
> +	struct reg_field	cfg_vid_hsa_time;
> +	struct reg_field	cfg_vid_hbp_time;
> +	struct reg_field	cfg_vid_hline_time;
> +	struct reg_field	cfg_vid_vsa_time;
> +	struct reg_field	cfg_vid_vbp_time;
> +	struct reg_field	cfg_vid_vfp_time;
> +	struct reg_field	cfg_vid_vactive_time;
> +	struct reg_field	cfg_phy_txrequestclkhs;
> +	struct reg_field	cfg_phy_bta_time;
> +	struct reg_field	cfg_phy_max_rd_time;
> +	struct reg_field	cfg_phy_lp2hs_time;
> +	struct reg_field	cfg_phy_hs2lp_time;
> +	struct reg_field	cfg_phy_max_rd_time_v131;
> +	struct reg_field	cfg_phy_lp2hs_time_v131;
> +	struct reg_field	cfg_phy_hs2lp_time_v131;
> +	struct reg_field	cfg_phy_clklp2hs_time;
> +	struct reg_field	cfg_phy_clkhs2lp_time;
> +	struct reg_field	cfg_phy_testclr;
> +	struct reg_field	cfg_phy_unshutdownz;
> +	struct reg_field	cfg_phy_unrstz;
> +	struct reg_field	cfg_phy_enableclk;
> +	struct reg_field	cfg_phy_forcepll;
> +	struct reg_field	cfg_phy_nlanes;
> +	struct reg_field	cfg_phy_stop_wait_time;
> +	struct reg_field	cfg_phy_status;
> +	struct reg_field	cfg_pckhdl_cfg;
> +	struct reg_field	cfg_hstx_timeout_counter;
> +	struct reg_field	cfg_lprx_timeout_counter;
> +	struct reg_field	cfg_int_stat0;
> +	struct reg_field	cfg_int_stat1;
> +	struct reg_field	cfg_int_mask0;
> +	struct reg_field	cfg_int_mask1;
> +	struct reg_field	cfg_gen_hdr;
> +	struct reg_field	cfg_gen_payload;
> +};
> +
> +static const struct dw_mipi_dsi_variant dw_mipi_dsi_v130_v131_layout = {
> +	.cfg_dpi_color_coding =		REG_FIELD(DSI_DPI_COLOR_CODING, 0, 3),
> +	.cfg_dpi_18loosely_en =		REG_FIELD(DSI_DPI_COLOR_CODING, 8, 8),
> +	.cfg_dpi_vid =			REG_FIELD(DSI_DPI_VCID, 0, 2),
> +	.cfg_dpi_vsync_active_low =	REG_FIELD(DSI_DPI_CFG_POL, 1, 1),
> +	.cfg_dpi_hsync_active_low =	REG_FIELD(DSI_DPI_CFG_POL, 2, 2),
> +	.cfg_cmd_mode_ack_rqst_en =	REG_FIELD(DSI_CMD_MODE_CFG, 1, 1),
> +	.cfg_cmd_mode_all_lp_en =	REG_FIELD(DSI_CMD_MODE_CFG, 8, 24),
> +	.cfg_cmd_mode_en =		REG_FIELD(DSI_MODE_CFG, 0, 31),
> +	.cfg_cmd_pkt_status =		REG_FIELD(DSI_CMD_PKT_STATUS, 0, 31),
> +	.cfg_vid_mode_en =		REG_FIELD(DSI_MODE_CFG, 0, 31),
> +	.cfg_vid_mode_type =		REG_FIELD(DSI_VID_MODE_CFG, 0, 1),
> +	.cfg_vid_mode_low_power =	REG_FIELD(DSI_VID_MODE_CFG, 8, 13),
> +	.cfg_vid_mode_vpg_en =		REG_FIELD(DSI_VID_MODE_CFG, 16, 16),
> +	.cfg_vid_mode_vpg_horiz =	REG_FIELD(DSI_VID_MODE_CFG, 24, 24),
> +	.cfg_vid_pkt_size =		REG_FIELD(DSI_VID_PKT_SIZE, 0, 10),
> +	.cfg_vid_hsa_time =		REG_FIELD(DSI_VID_HSA_TIME, 0, 31),
> +	.cfg_vid_hbp_time =		REG_FIELD(DSI_VID_HBP_TIME, 0, 31),
> +	.cfg_vid_hline_time =		REG_FIELD(DSI_VID_HLINE_TIME, 0, 31),
> +	.cfg_vid_vsa_time =		REG_FIELD(DSI_VID_VSA_LINES, 0, 31),
> +	.cfg_vid_vbp_time =		REG_FIELD(DSI_VID_VBP_LINES, 0, 31),
> +	.cfg_vid_vfp_time =		REG_FIELD(DSI_VID_VFP_LINES, 0, 31),
> +	.cfg_vid_vactive_time =		REG_FIELD(DSI_VID_VACTIVE_LINES, 0, 31),
> +	.cfg_phy_txrequestclkhs =	REG_FIELD(DSI_LPCLK_CTRL, 0, 0),
> +	.cfg_phy_bta_time =		REG_FIELD(DSI_BTA_TO_CNT, 0, 31),
> +	.cfg_phy_max_rd_time =		REG_FIELD(DSI_PHY_TMR_CFG, 0, 15),
> +	.cfg_phy_lp2hs_time =		REG_FIELD(DSI_PHY_TMR_CFG, 16, 23),
> +	.cfg_phy_hs2lp_time =		REG_FIELD(DSI_PHY_TMR_CFG, 24, 31),
> +	.cfg_phy_max_rd_time_v131 =	REG_FIELD(DSI_PHY_TMR_RD_CFG, 0, 15),
> +	.cfg_phy_lp2hs_time_v131 =	REG_FIELD(DSI_PHY_TMR_CFG, 0, 15),
> +	.cfg_phy_hs2lp_time_v131 =	REG_FIELD(DSI_PHY_TMR_CFG, 16, 31),
> +	.cfg_phy_clklp2hs_time =	REG_FIELD(DSI_PHY_TMR_LPCLK_CFG, 0, 15),
> +	.cfg_phy_clkhs2lp_time =	REG_FIELD(DSI_PHY_TMR_LPCLK_CFG, 16, 31),
> +	.cfg_phy_testclr =		REG_FIELD(DSI_PHY_TST_CTRL0, 0, 0),
> +	.cfg_phy_unshutdownz =		REG_FIELD(DSI_PHY_RSTZ, 0, 0),
> +	.cfg_phy_unrstz =		REG_FIELD(DSI_PHY_RSTZ, 1, 1),
> +	.cfg_phy_enableclk =		REG_FIELD(DSI_PHY_RSTZ, 2, 2),
> +	.cfg_phy_forcepll =		REG_FIELD(DSI_PHY_RSTZ, 3, 3),
> +	.cfg_phy_nlanes =		REG_FIELD(DSI_PHY_IF_CFG, 0, 1),
> +	.cfg_phy_stop_wait_time =	REG_FIELD(DSI_PHY_IF_CFG, 8, 15),
> +	.cfg_phy_status =		REG_FIELD(DSI_PHY_STATUS, 0, 0),
> +	.cfg_pckhdl_cfg =		REG_FIELD(DSI_PCKHDL_CFG, 0, 4),
> +	.cfg_hstx_timeout_counter =	REG_FIELD(DSI_TO_CNT_CFG, 16, 31),
> +	.cfg_lprx_timeout_counter =	REG_FIELD(DSI_TO_CNT_CFG, 0, 15),
> +	.cfg_int_stat0 =		REG_FIELD(DSI_INT_ST0, 0, 31),
> +	.cfg_int_stat1 =		REG_FIELD(DSI_INT_ST1, 0, 31),
> +	.cfg_int_mask0 =		REG_FIELD(DSI_INT_MSK0, 0, 31),
> +	.cfg_int_mask1 =		REG_FIELD(DSI_INT_MSK1, 0, 31),
> +	.cfg_gen_hdr =			REG_FIELD(DSI_GEN_HDR, 0, 31),
> +	.cfg_gen_payload =		REG_FIELD(DSI_GEN_PLD_DATA, 0, 31),
> +};
> +
>   /*
>    * Check if either a link to a master or slave is present
>    */
> @@ -359,15 +459,22 @@ static void dw_mipi_message_config(struct dw_mipi_dsi *dsi,
>   				   const struct mipi_dsi_msg *msg)
>   {
>   	bool lpm = msg->flags & MIPI_DSI_MSG_USE_LPM;
> -	u32 val = 0;
> +	u32 cmd_mode_lp = 0;
> +
> +	switch (dsi->hw_version) {
> +	case HWVER_130:
> +	case HWVER_131:
> +		cmd_mode_lp = CMD_MODE_ALL_LP;
> +		break;
> +	}
>   
>   	if (msg->flags & MIPI_DSI_MSG_REQ_ACK)
> -		val |= ACK_RQST_EN;
> +		regmap_field_write(dsi->field_cmd_mode_ack_rqst_en, 1);
> +
>   	if (lpm)
> -		val |= CMD_MODE_ALL_LP;
> +		regmap_field_write(dsi->field_cmd_mode_all_lp_en, cmd_mode_lp);
>   
> -	regmap_write(dsi->regs, DSI_LPCLK_CTRL, lpm ? 0 : PHY_TXREQUESTCLKHS);
> -	regmap_write(dsi->regs, DSI_CMD_MODE_CFG, val);
> +	regmap_field_write(dsi->field_phy_txrequestclkhs, lpm ? 0 : 1);
>   }
>   
>   static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 hdr_val)
> @@ -375,18 +482,18 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 hdr_val)
>   	int ret;
>   	u32 val = 0, mask;
>   
> -	ret = regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS,
> -				       val, !(val & GEN_CMD_FULL), 1000,
> -				       CMD_PKT_STATUS_TIMEOUT_US);
> +	ret = regmap_field_read_poll_timeout(dsi->field_cmd_pkt_status,
> +					     val, !(val & GEN_CMD_FULL),
> +					     1000, CMD_PKT_STATUS_TIMEOUT_US);
>   	if (ret) {
>   		dev_err(dsi->dev, "failed to get available command FIFO\n");
>   		return ret;
>   	}
>   
> -	regmap_write(dsi->regs, DSI_GEN_HDR, hdr_val);
> +	regmap_field_write(dsi->field_gen_hdr, hdr_val);
>   
>   	mask = GEN_CMD_EMPTY | GEN_PLD_W_EMPTY;
> -	ret = regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS,
> +	ret = regmap_field_read_poll_timeout(dsi->field_cmd_pkt_status,
>   				       val, (val & mask) == mask,
>   				       1000, CMD_PKT_STATUS_TIMEOUT_US);
>   	if (ret) {
> @@ -409,20 +516,22 @@ static int dw_mipi_dsi_write(struct dw_mipi_dsi *dsi,
>   		if (len < pld_data_bytes) {
>   			word = 0;
>   			memcpy(&word, tx_buf, len);
> -			regmap_write(dsi->regs, DSI_GEN_PLD_DATA,
> -				     le32_to_cpu(word));
> +			regmap_field_write(dsi->field_gen_payload,
> +					   le32_to_cpu(word));
>   			len = 0;
>   		} else {
>   			memcpy(&word, tx_buf, pld_data_bytes);
> -			regmap_write(dsi->regs, DSI_GEN_PLD_DATA,
> -				     le32_to_cpu(word));
> +			regmap_field_write(dsi->field_gen_payload,
> +					   le32_to_cpu(word));
>   			tx_buf += pld_data_bytes;
>   			len -= pld_data_bytes;
>   		}
>   
> -		ret = regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS,
> -					       val, !(val & GEN_PLD_W_FULL),
> -					       1000, CMD_PKT_STATUS_TIMEOUT_US);
> +		ret = regmap_field_read_poll_timeout(dsi->field_cmd_pkt_status,
> +						     val,
> +						     !(val & GEN_PLD_W_FULL),
> +						     1000,
> +						     CMD_PKT_STATUS_TIMEOUT_US);
>   		if (ret) {
>   			dev_err(dsi->dev,
>   				"failed to get available write payload FIFO\n");
> @@ -443,9 +552,9 @@ static int dw_mipi_dsi_read(struct dw_mipi_dsi *dsi,
>   	u32 val = 0;
>   
>   	/* Wait end of the read operation */
> -	ret = regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS,
> -				       val, !(val & GEN_RD_CMD_BUSY),
> -				       1000, CMD_PKT_STATUS_TIMEOUT_US);
> +	ret = regmap_field_read_poll_timeout(dsi->field_cmd_pkt_status,
> +					     val, !(val & GEN_RD_CMD_BUSY),
> +					     1000, CMD_PKT_STATUS_TIMEOUT_US);
>   	if (ret) {
>   		dev_err(dsi->dev, "Timeout during read operation\n");
>   		return ret;
> @@ -453,15 +562,17 @@ static int dw_mipi_dsi_read(struct dw_mipi_dsi *dsi,
>   
>   	for (i = 0; i < len; i += 4) {
>   		/* Read fifo must not be empty before all bytes are read */
> -		ret = regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS,
> -					       val, !(val & GEN_PLD_R_EMPTY),
> -					       1000, CMD_PKT_STATUS_TIMEOUT_US);
> +		ret = regmap_field_read_poll_timeout(dsi->field_cmd_pkt_status,
> +						     val,
> +						     !(val & GEN_PLD_R_EMPTY),
> +						     1000,
> +						     CMD_PKT_STATUS_TIMEOUT_US);
>   		if (ret) {
>   			dev_err(dsi->dev, "Read payload FIFO is empty\n");
>   			return ret;
>   		}
>   
> -		regmap_read(dsi->regs, DSI_GEN_PLD_DATA, &val);
> +		regmap_field_read(dsi->field_gen_payload, &val);
>   		for (j = 0; j < 4 && j + i < len; j++)
>   			buf[i + j] = val >> (8 * j);
>   	}
> @@ -515,30 +626,30 @@ static const struct mipi_dsi_host_ops dw_mipi_dsi_host_ops = {
>   
>   static void dw_mipi_dsi_video_mode_config(struct dw_mipi_dsi *dsi)
>   {
> -	u32 val;
> -
>   	/*
>   	 * TODO dw drv improvements
>   	 * enabling low power is panel-dependent, we should use the
>   	 * panel configuration here...
>   	 */
> -	val = ENABLE_LOW_POWER;
> +	regmap_field_write(dsi->field_vid_mode_low_power, ENABLE_LOW_POWER);
>   
>   	if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST)
> -		val |= VID_MODE_TYPE_BURST;
> +		regmap_field_write(dsi->field_vid_mode_type,
> +				   VID_MODE_TYPE_BURST);
>   	else if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
> -		val |= VID_MODE_TYPE_NON_BURST_SYNC_PULSES;
> +		regmap_field_write(dsi->field_vid_mode_type,
> +				   VID_MODE_TYPE_NON_BURST_SYNC_PULSES);
>   	else
> -		val |= VID_MODE_TYPE_NON_BURST_SYNC_EVENTS;
> +		regmap_field_write(dsi->field_vid_mode_type,
> +				   VID_MODE_TYPE_NON_BURST_SYNC_EVENTS);
>   
>   #ifdef CONFIG_DEBUG_FS
>   	if (dsi->vpg) {
> -		val |= VID_MODE_VPG_ENABLE;
> -		val |= dsi->vpg_horizontal ? VID_MODE_VPG_HORIZONTAL : 0;
> +		regmap_field_write(dsi->regs, dsi->field_vid_mode_vpg_en, 1);
> +		regmap_field_write(dsi->regs, dsi->field_vid_mode_vpg_horiz,
> +				   dsi->vpg_horizontal ? 1 : 0);
>   	}
>   #endif /* CONFIG_DEBUG_FS */
> -
> -	regmap_write(dsi->regs, DSI_VID_MODE_CFG, val);
>   }
>   
>   static void dw_mipi_dsi_set_mode(struct dw_mipi_dsi *dsi,
> @@ -547,11 +658,13 @@ static void dw_mipi_dsi_set_mode(struct dw_mipi_dsi *dsi,
>   	regmap_write(dsi->regs, DSI_PWR_UP, RESET);
>   
>   	if (mode_flags & MIPI_DSI_MODE_VIDEO) {
> -		regmap_write(dsi->regs, DSI_MODE_CFG, ENABLE_VIDEO_MODE);
> +		regmap_field_write(dsi->field_cmd_mode_en, 0);
> +
>   		dw_mipi_dsi_video_mode_config(dsi);
> -		regmap_write(dsi->regs, DSI_LPCLK_CTRL, PHY_TXREQUESTCLKHS);
> +
> +		regmap_field_write(dsi->field_phy_txrequestclkhs, 1);
>   	} else {
> -		regmap_write(dsi->regs, DSI_MODE_CFG, ENABLE_CMD_MODE);
> +		regmap_field_write(dsi->field_cmd_mode_en, 1);
>   	}
>   
>   	regmap_write(dsi->regs, DSI_PWR_UP, POWERUP);
> @@ -560,7 +673,7 @@ static void dw_mipi_dsi_set_mode(struct dw_mipi_dsi *dsi,
>   static void dw_mipi_dsi_disable(struct dw_mipi_dsi *dsi)
>   {
>   	regmap_write(dsi->regs, DSI_PWR_UP, RESET);
> -	regmap_write(dsi->regs, DSI_PHY_RSTZ, PHY_RSTZ);
> +	regmap_field_write(dsi->field_phy_unrstz, 0);
>   }
>   
>   static void dw_mipi_dsi_init(struct dw_mipi_dsi *dsi)
> @@ -589,14 +702,15 @@ static void dw_mipi_dsi_init(struct dw_mipi_dsi *dsi)
>   static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi,
>   				   const struct drm_display_mode *mode)
>   {
> -	u32 val = 0, color = 0;
> +	u32 color = 0;
>   
>   	switch (dsi->format) {
>   	case MIPI_DSI_FMT_RGB888:
>   		color = DPI_COLOR_CODING_24BIT;
>   		break;
>   	case MIPI_DSI_FMT_RGB666:
> -		color = DPI_COLOR_CODING_18BIT_2 | LOOSELY18_EN;
> +		color = DPI_COLOR_CODING_18BIT_2;
> +		regmap_field_write(dsi->field_dpi_18loosely_en, 1);
>   		break;
>   	case MIPI_DSI_FMT_RGB666_PACKED:
>   		color = DPI_COLOR_CODING_18BIT_1;
> @@ -606,14 +720,15 @@ static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi,
>   		break;
>   	}
>   
> -	if (mode->flags & DRM_MODE_FLAG_NVSYNC)
> -		val |= VSYNC_ACTIVE_LOW;
> -	if (mode->flags & DRM_MODE_FLAG_NHSYNC)
> -		val |= HSYNC_ACTIVE_LOW;
> +	regmap_field_write(dsi->field_dpi_color_coding, color);
> +
> +	if (!(mode->flags & DRM_MODE_FLAG_NVSYNC))
> +		regmap_field_write(dsi->field_dpi_vsync_active_low, 1);
> +	if (!(mode->flags & DRM_MODE_FLAG_NHSYNC))
> +		regmap_field_write(dsi->field_dpi_hsync_active_low, 1);
> +
> +	regmap_field_write(dsi->field_dpi_vid, DPI_VCID(dsi->channel));
>   
> -	regmap_write(dsi->regs, DSI_DPI_VCID, DPI_VCID(dsi->channel));
> -	regmap_write(dsi->regs, DSI_DPI_COLOR_CODING, color);
> -	regmap_write(dsi->regs, DSI_DPI_CFG_POL, val);
>   	/*
>   	 * TODO dw drv improvements
>   	 * largest packet sizes during hfp or during vsa/vpb/vfp
> @@ -626,7 +741,8 @@ static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi,
>   
>   static void dw_mipi_dsi_packet_handler_config(struct dw_mipi_dsi *dsi)
>   {
> -	regmap_write(dsi->regs, DSI_PCKHDL_CFG, CRC_RX_EN | ECC_RX_EN | BTA_EN);
> +	regmap_field_write(dsi->field_pckhdl_cfg,
> +			   CRC_RX_EN | ECC_RX_EN | BTA_EN);
>   }
>   
>   static void dw_mipi_dsi_video_packet_config(struct dw_mipi_dsi *dsi,
> @@ -639,11 +755,9 @@ static void dw_mipi_dsi_video_packet_config(struct dw_mipi_dsi *dsi,
>   	 * DSI_VNPCR.NPSIZE... especially because this driver supports
>   	 * non-burst video modes, see dw_mipi_dsi_video_mode_config()...
>   	 */
> -
> -	regmap_write(dsi->regs, DSI_VID_PKT_SIZE,
> -		       dw_mipi_is_dual_mode(dsi) ?
> -				VID_PKT_SIZE(mode->hdisplay / 2) :
> -				VID_PKT_SIZE(mode->hdisplay));
> +	regmap_field_write(dsi->field_vid_pkt_size,
> +			   dw_mipi_is_dual_mode(dsi) ?
> +				mode->hdisplay / 2 : mode->hdisplay);
>   }
>   
>   static void dw_mipi_dsi_command_mode_config(struct dw_mipi_dsi *dsi)
> @@ -653,15 +767,17 @@ static void dw_mipi_dsi_command_mode_config(struct dw_mipi_dsi *dsi)
>   	 * compute high speed transmission counter timeout according
>   	 * to the timeout clock division (TO_CLK_DIVISION) and byte lane...
>   	 */
> -	regmap_write(dsi->regs, DSI_TO_CNT_CFG,
> -		     HSTX_TO_CNT(1000) | LPRX_TO_CNT(1000));
> +	regmap_field_write(dsi->field_hstx_timeout_counter, 1000);
> +	regmap_field_write(dsi->field_lprx_timeout_counter, 1000);
> +
>   	/*
>   	 * TODO dw drv improvements
>   	 * the Bus-Turn-Around Timeout Counter should be computed
>   	 * according to byte lane...
>   	 */
> -	regmap_write(dsi->regs, DSI_BTA_TO_CNT, 0xd00);
> -	regmap_write(dsi->regs, DSI_MODE_CFG, ENABLE_CMD_MODE);
> +	regmap_field_write(dsi->field_phy_bta_time, 0xd00);
> +
> +	regmap_field_write(dsi->field_cmd_mode_en, 1);
>   }
>   
>   /* Get lane byte clock cycles. */
> @@ -695,13 +811,13 @@ static void dw_mipi_dsi_line_timer_config(struct dw_mipi_dsi *dsi,
>   	 * computations below may be improved...
>   	 */
>   	lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, mode, htotal);
> -	regmap_write(dsi->regs, DSI_VID_HLINE_TIME, lbcc);
> +	regmap_field_write(dsi->field_vid_hline_time, lbcc);
>   
>   	lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, mode, hsa);
> -	regmap_write(dsi->regs, DSI_VID_HSA_TIME, lbcc);
> +	regmap_field_write(dsi->field_vid_hsa_time, lbcc);
>   
>   	lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, mode, hbp);
> -	regmap_write(dsi->regs, DSI_VID_HBP_TIME, lbcc);
> +	regmap_field_write(dsi->field_vid_hbp_time, lbcc);
>   }
>   
>   static void dw_mipi_dsi_vertical_timing_config(struct dw_mipi_dsi *dsi,
> @@ -714,17 +830,16 @@ static void dw_mipi_dsi_vertical_timing_config(struct dw_mipi_dsi *dsi,
>   	vfp = mode->vsync_start - mode->vdisplay;
>   	vbp = mode->vtotal - mode->vsync_end;
>   
> -	regmap_write(dsi->regs, DSI_VID_VACTIVE_LINES, vactive);
> -	regmap_write(dsi->regs, DSI_VID_VSA_LINES, vsa);
> -	regmap_write(dsi->regs, DSI_VID_VFP_LINES, vfp);
> -	regmap_write(dsi->regs, DSI_VID_VBP_LINES, vbp);
> +	regmap_field_write(dsi->field_vid_vactive_time, vactive);
> +	regmap_field_write(dsi->field_vid_vsa_time, vsa);
> +	regmap_field_write(dsi->field_vid_vfp_time, vfp);
> +	regmap_field_write(dsi->field_vid_vbp_time, vbp);
>   }
>   
>   static void dw_mipi_dsi_dphy_timing_config(struct dw_mipi_dsi *dsi)
>   {
>   	const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops;
>   	struct dw_mipi_dsi_dphy_timing timing;
> -	u32 hw_version;
>   	int ret;
>   
>   	ret = phy_ops->get_timing(dsi->plat_data->priv_data,
> @@ -739,26 +854,12 @@ static void dw_mipi_dsi_dphy_timing_config(struct dw_mipi_dsi *dsi)
>   	 * note: DSI_PHY_TMR_CFG.MAX_RD_TIME should be in line with
>   	 * DSI_CMD_MODE_CFG.MAX_RD_PKT_SIZE_LP (see CMD_MODE_ALL_LP)
>   	 */
> +	regmap_field_write(dsi->field_phy_lp2hs_time, timing.data_lp2hs);
> +	regmap_field_write(dsi->field_phy_hs2lp_time, timing.data_hs2lp);
>   
> -	regmap_read(dsi->regs, DSI_VERSION, &hw_version);
> -	hw_version &= VERSION;
> -
> -	if (hw_version >= HWVER_131) {
> -		regmap_write(dsi->regs, DSI_PHY_TMR_CFG,
> -			     PHY_HS2LP_TIME_V131(timing.data_hs2lp) |
> -			     PHY_LP2HS_TIME_V131(timing.data_lp2hs));
> -		regmap_write(dsi->regs, DSI_PHY_TMR_RD_CFG,
> -			     MAX_RD_TIME_V131(10000));
> -	} else {
> -		regmap_write(dsi->regs, DSI_PHY_TMR_CFG,
> -			     PHY_HS2LP_TIME(timing.data_hs2lp) |
> -			     PHY_LP2HS_TIME(timing.data_lp2hs) |
> -			     MAX_RD_TIME(10000));
> -	}
> -
> -	regmap_write(dsi->regs, DSI_PHY_TMR_LPCLK_CFG,
> -		     PHY_CLKHS2LP_TIME(timing.clk_hs2lp) |
> -		     PHY_CLKLP2HS_TIME(timing.clk_lp2hs));
> +	regmap_field_write(dsi->field_phy_max_rd_time, 10000);
> +	regmap_field_write(dsi->field_phy_clkhs2lp_time, timing.clk_hs2lp);
> +	regmap_field_write(dsi->field_phy_clklp2hs_time, timing.clk_lp2hs);
>   }
>   
>   static void dw_mipi_dsi_dphy_interface_config(struct dw_mipi_dsi *dsi)
> @@ -768,18 +869,22 @@ static void dw_mipi_dsi_dphy_interface_config(struct dw_mipi_dsi *dsi)
>   	 * stop wait time should be the maximum between host dsi
>   	 * and panel stop wait times
>   	 */
> -	regmap_write(dsi->regs, DSI_PHY_IF_CFG,
> -		     PHY_STOP_WAIT_TIME(0x20) | N_LANES(dsi->lanes));
> +	regmap_field_write(dsi->field_phy_stop_wait_time, 0x20);
> +	regmap_field_write(dsi->field_phy_nlanes, dsi->lanes - 1);
>   }
>   
>   static void dw_mipi_dsi_dphy_init(struct dw_mipi_dsi *dsi)
>   {
>   	/* Clear PHY state */
> -	regmap_write(dsi->regs, DSI_PHY_RSTZ, PHY_DISFORCEPLL | PHY_DISABLECLK
> -		     | PHY_RSTZ | PHY_SHUTDOWNZ);
> -	regmap_write(dsi->regs, DSI_PHY_TST_CTRL0, PHY_UNTESTCLR);
> -	regmap_write(dsi->regs, DSI_PHY_TST_CTRL0, PHY_TESTCLR);
> -	regmap_write(dsi->regs, DSI_PHY_TST_CTRL0, PHY_UNTESTCLR);
> +	regmap_field_write(dsi->field_phy_enableclk, 0);
> +	regmap_field_write(dsi->field_phy_unrstz, 0);
> +	regmap_field_write(dsi->field_phy_unshutdownz, 0);
> +
> +	regmap_field_write(dsi->field_phy_forcepll, 0);
> +
> +	regmap_field_write(dsi->field_phy_testclr, 0);
> +	regmap_field_write(dsi->field_phy_testclr, 1);
> +	regmap_field_write(dsi->field_phy_testclr, 0);
>   }
>   
>   static void dw_mipi_dsi_dphy_enable(struct dw_mipi_dsi *dsi)
> @@ -787,18 +892,21 @@ static void dw_mipi_dsi_dphy_enable(struct dw_mipi_dsi *dsi)
>   	u32 val = 0;
>   	int ret;
>   
> -	regmap_write(dsi->regs, DSI_PHY_RSTZ, PHY_ENFORCEPLL | PHY_ENABLECLK |
> -		     PHY_UNRSTZ | PHY_UNSHUTDOWNZ);
> +	regmap_field_write(dsi->field_phy_enableclk, 1);
> +	regmap_field_write(dsi->field_phy_unrstz, 1);
> +	regmap_field_write(dsi->field_phy_unshutdownz, 1);
> +
> +	regmap_field_write(dsi->field_phy_forcepll, 1);
>   
> -	ret = regmap_read_poll_timeout(dsi->regs, DSI_PHY_STATUS,
> -				       val, val & PHY_LOCK,
> -				       1000, PHY_STATUS_TIMEOUT_US);
> +	ret = regmap_field_read_poll_timeout(dsi->field_phy_status,
> +					     val, val & PHY_LOCK,
> +					     1000, PHY_STATUS_TIMEOUT_US);
>   	if (ret)
>   		DRM_DEBUG_DRIVER("failed to wait phy lock state\n");
>   
> -	ret = regmap_read_poll_timeout(dsi->regs, DSI_PHY_STATUS,
> -				       val, val & PHY_STOP_STATE_CLK_LANE, 1000,
> -				       PHY_STATUS_TIMEOUT_US);
> +	ret = regmap_field_read_poll_timeout(dsi->field_phy_status,
> +					     val, val & PHY_STOP_STATE_CLK_LANE,
> +					     1000, PHY_STATUS_TIMEOUT_US);
>   	if (ret)
>   		DRM_DEBUG_DRIVER("failed to wait phy clk lane stop state\n");
>   }
> @@ -807,10 +915,10 @@ static void dw_mipi_dsi_clear_err(struct dw_mipi_dsi *dsi)
>   {
>   	u32 val;
>   
> -	regmap_read(dsi->regs, DSI_INT_ST0, &val);
> -	regmap_read(dsi->regs, DSI_INT_ST1, &val);
> -	regmap_write(dsi->regs, DSI_INT_MSK0, 0);
> -	regmap_write(dsi->regs, DSI_INT_MSK1, 0);
> +	regmap_field_read(dsi->field_int_stat0, &val);
> +	regmap_field_read(dsi->field_int_stat1, &val);
> +	regmap_field_write(dsi->field_int_mask0, 0);
> +	regmap_field_write(dsi->field_int_mask1, 0);
>   }
>   
>   static void dw_mipi_dsi_bridge_post_disable(struct drm_bridge *bridge)
> @@ -1005,6 +1113,86 @@ static void dw_mipi_dsi_get_hw_version(struct dw_mipi_dsi *dsi)
>   		dev_err(dsi->dev, "Failed to read DSI hw version register\n");
>   }
>   
> +#define INIT_FIELD(f) INIT_FIELD_CFG(field_##f, cfg_##f)
> +#define INIT_FIELD_CFG(f, conf)						\
> +	do {								\
> +		dsi->f = devm_regmap_field_alloc(dsi->dev, dsi->regs,	\
> +						 variant->conf);	\
> +		if (IS_ERR(dsi->f))					\
> +			dev_warn(dsi->dev, "Ignoring regmap field " #f "\n"); \
> +	} while (0)


In kernel you can use gcc extension ({ ... }) instead "do { ... } while 
(0)" [1].

[1]: https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html


> +
> +static int dw_mipi_dsi_regmap_fields_init(struct dw_mipi_dsi *dsi)
> +{
> +	const struct dw_mipi_dsi_variant *variant;
> +
> +	switch (dsi->hw_version) {
> +	case HWVER_130:
> +	case HWVER_131:
> +		variant = &dw_mipi_dsi_v130_v131_layout;
> +		break;
> +	default:
> +		DRM_ERROR("Unrecognized DSI host controller HW revision\n");
> +		return -ENODEV;
> +	}
> +
> +	INIT_FIELD(dpi_18loosely_en);
> +	INIT_FIELD(dpi_vid);
> +	INIT_FIELD(dpi_color_coding);
> +	INIT_FIELD(dpi_vsync_active_low);
> +	INIT_FIELD(dpi_hsync_active_low);
> +	INIT_FIELD(cmd_mode_ack_rqst_en);
> +	INIT_FIELD(cmd_mode_all_lp_en);
> +	INIT_FIELD(cmd_mode_en);
> +	INIT_FIELD(cmd_pkt_status);
> +	INIT_FIELD(vid_mode_en);
> +	INIT_FIELD(vid_mode_type);
> +	INIT_FIELD(vid_mode_low_power);
> +	INIT_FIELD(vid_pkt_size);
> +	INIT_FIELD(vid_hsa_time);
> +	INIT_FIELD(vid_hbp_time);
> +	INIT_FIELD(vid_hline_time);
> +	INIT_FIELD(vid_vsa_time);
> +	INIT_FIELD(vid_vbp_time);
> +	INIT_FIELD(vid_vfp_time);
> +	INIT_FIELD(vid_vactive_time);
> +	INIT_FIELD(phy_txrequestclkhs);
> +	INIT_FIELD(phy_testclr);
> +	INIT_FIELD(phy_unshutdownz);
> +	INIT_FIELD(phy_unrstz);
> +	INIT_FIELD(phy_enableclk);
> +	INIT_FIELD(phy_nlanes);
> +	INIT_FIELD(phy_stop_wait_time);
> +	INIT_FIELD(phy_status);
> +	INIT_FIELD(pckhdl_cfg);
> +	INIT_FIELD(hstx_timeout_counter);
> +	INIT_FIELD(lprx_timeout_counter);
> +	INIT_FIELD(int_stat0);
> +	INIT_FIELD(int_stat1);
> +	INIT_FIELD(int_mask0);
> +	INIT_FIELD(int_mask1);
> +	INIT_FIELD(gen_hdr);
> +	INIT_FIELD(gen_payload);
> +	INIT_FIELD(phy_bta_time);
> +	INIT_FIELD(vid_mode_vpg_en);
> +	INIT_FIELD(vid_mode_vpg_horiz);
> +	INIT_FIELD(phy_clklp2hs_time);
> +	INIT_FIELD(phy_clkhs2lp_time);
> +	INIT_FIELD(phy_forcepll);
> +
> +	if (dsi->hw_version == HWVER_131) {
> +		INIT_FIELD_CFG(field_phy_max_rd_time, cfg_phy_max_rd_time_v131);
> +		INIT_FIELD_CFG(field_phy_lp2hs_time, cfg_phy_lp2hs_time_v131);
> +		INIT_FIELD_CFG(field_phy_hs2lp_time, cfg_phy_hs2lp_time_v131);
> +	} else {
> +		INIT_FIELD(phy_max_rd_time);
> +		INIT_FIELD(phy_lp2hs_time);
> +		INIT_FIELD(phy_hs2lp_time);
> +	}


And here we have devres storm - for every field we allocate memory, 
enqueue deallocator, copy static values to dynamic struct and more.

I know that CPU power and memory are cheap, but this hurts my eyes :)

Other thing is that this complicates the driver - adding new field will 
require changes at least in 4 (?) places, without counting real field usage.

And here is very simple alternative how different hw register layout can 
be coded without much ado: [1][2].

[1]: 
https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/exynos/exynos_hdmi.c#L55

[2]: 
https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/exynos/exynos_hdmi.c#L667


To be clear, I am not totally against this approach - the patch seems to 
me correct, it is just 'baroque' :) If you can show that approach from 
[1] and [2] in this case will be more problematic we can go your way.

Anyway more comments appreciated.


Regards

Andrzej


> +
> +	return 0;
> +}
> +
>   static struct dw_mipi_dsi *
>   __dw_mipi_dsi_probe(struct platform_device *pdev,
>   		    const struct dw_mipi_dsi_plat_data *plat_data)
> @@ -1081,6 +1269,12 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
>   
>   	dw_mipi_dsi_get_hw_version(dsi);
>   
> +	ret = dw_mipi_dsi_regmap_fields_init(dsi);
> +	if (ret) {
> +		dev_err(dev, "Failed to init register layout map: %d\n", ret);
> +		return ERR_PTR(ret);
> +	}
> +
>   	dw_mipi_dsi_debugfs_init(dsi);
>   	pm_runtime_enable(dev);
>