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@xxxxxxxxxxxxx>
> Reviewed-by: Emil Velikov <emil.velikov@xxxxxxxxxxxxx>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@xxxxxxxxxxxxx>
> ---
> 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);
>
