* [PATCH v8 01/10] drm: bridge: dw_mipi_dsi: add initial regmap infrastructure
2020-04-27 8:19 [PATCH v8 00/10] Genericize DW MIPI DSI bridge and add i.MX 6 driver Adrian Ratiu
@ 2020-04-27 8:19 ` Adrian Ratiu
2020-04-27 14:41 ` Enric Balletbo i Serra
2020-04-27 8:19 ` [PATCH v8 02/10] drm: bridge: dw_mipi_dsi: abstract register access using reg_fields Adrian Ratiu
` (8 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Adrian Ratiu @ 2020-04-27 8:19 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, linux-rockchip, Laurent Pinchart
Cc: Jernej Skrabec, Adrian Pop, Jonas Karlman, linux-kernel,
dri-devel, Andrzej Hajda, linux-imx, kernel, Ezequiel Garcia,
linux-stm32, Arnaud Ferraris
In order to support multiple versions of the Synopsis MIPI DSI host
controller, which have different register layouts but almost identical
HW protocols, we add a regmap infrastructure which can abstract away
register accesses for platform drivers using the bridge.
The controller HW revision is detected during bridge probe which will
be used in future commits to load the relevant register layout which
the bridge will use transparently to the platform drivers.
Cc: Enric Balletbo Serra <eballetbo@gmail.com>
Suggested-by: Ezequiel Garcia <ezequiel@collabora.com>
Tested-by: Adrian Pop <pop.adrian61@gmail.com>
Tested-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
Chnages since v7:
- Minor checkpatch line fix
Changes since v6:
- Select REGMAP_MMIO in Kconfig (Enric)
- Drop unnecessary stack variable inits (Enric)
- Make bridge error ASAP after a bad revision read (Enric)
- Drop redundant read of hw_version in dphy_timing_config (Enric)
New in v5.
---
drivers/gpu/drm/bridge/synopsys/Kconfig | 1 +
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 210 ++++++++++--------
2 files changed, 121 insertions(+), 90 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig b/drivers/gpu/drm/bridge/synopsys/Kconfig
index 21a1be3ced0f3..080146093b68e 100644
--- a/drivers/gpu/drm/bridge/synopsys/Kconfig
+++ b/drivers/gpu/drm/bridge/synopsys/Kconfig
@@ -39,3 +39,4 @@ config DRM_DW_MIPI_DSI
select DRM_KMS_HELPER
select DRM_MIPI_DSI
select DRM_PANEL_BRIDGE
+ select REGMAP_MMIO
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index 5ef0f154aa7bd..34b8668ae24ea 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -15,6 +15,7 @@
#include <linux/module.h>
#include <linux/of_device.h>
#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
#include <linux/reset.h>
#include <video/mipi_display.h>
@@ -227,6 +228,7 @@ struct dw_mipi_dsi {
struct drm_bridge *panel_bridge;
struct device *dev;
void __iomem *base;
+ struct regmap *regs;
struct clk *pclk;
@@ -235,6 +237,7 @@ struct dw_mipi_dsi {
u32 lanes;
u32 format;
unsigned long mode_flags;
+ u32 hw_version;
#ifdef CONFIG_DEBUG_FS
struct dentry *debugfs;
@@ -249,6 +252,13 @@ struct dw_mipi_dsi {
const struct dw_mipi_dsi_plat_data *plat_data;
};
+static const struct regmap_config dw_mipi_dsi_regmap_cfg = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .name = "dw-mipi-dsi",
+};
+
/*
* Check if either a link to a master or slave is present
*/
@@ -280,16 +290,6 @@ static inline struct dw_mipi_dsi *bridge_to_dsi(struct drm_bridge *bridge)
return container_of(bridge, struct dw_mipi_dsi, bridge);
}
-static inline void dsi_write(struct dw_mipi_dsi *dsi, u32 reg, u32 val)
-{
- writel(val, dsi->base + reg);
-}
-
-static inline u32 dsi_read(struct dw_mipi_dsi *dsi, u32 reg)
-{
- return readl(dsi->base + reg);
-}
-
static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
struct mipi_dsi_device *device)
{
@@ -366,8 +366,8 @@ static void dw_mipi_message_config(struct dw_mipi_dsi *dsi,
if (lpm)
val |= CMD_MODE_ALL_LP;
- dsi_write(dsi, DSI_LPCLK_CTRL, lpm ? 0 : PHY_TXREQUESTCLKHS);
- dsi_write(dsi, DSI_CMD_MODE_CFG, val);
+ regmap_write(dsi->regs, DSI_LPCLK_CTRL, lpm ? 0 : PHY_TXREQUESTCLKHS);
+ regmap_write(dsi->regs, DSI_CMD_MODE_CFG, val);
}
static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 hdr_val)
@@ -375,20 +375,20 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 hdr_val)
int ret;
u32 val, mask;
- ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
- val, !(val & GEN_CMD_FULL), 1000,
- CMD_PKT_STATUS_TIMEOUT_US);
+ ret = regmap_read_poll_timeout(dsi->regs, DSI_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;
}
- dsi_write(dsi, DSI_GEN_HDR, hdr_val);
+ regmap_write(dsi->regs, DSI_GEN_HDR, hdr_val);
mask = GEN_CMD_EMPTY | GEN_PLD_W_EMPTY;
- ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
- val, (val & mask) == mask,
- 1000, CMD_PKT_STATUS_TIMEOUT_US);
+ ret = regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS,
+ val, (val & mask) == mask,
+ 1000, CMD_PKT_STATUS_TIMEOUT_US);
if (ret) {
dev_err(dsi->dev, "failed to write command FIFO\n");
return ret;
@@ -409,18 +409,20 @@ static int dw_mipi_dsi_write(struct dw_mipi_dsi *dsi,
if (len < pld_data_bytes) {
word = 0;
memcpy(&word, tx_buf, len);
- dsi_write(dsi, DSI_GEN_PLD_DATA, le32_to_cpu(word));
+ regmap_write(dsi->regs, DSI_GEN_PLD_DATA,
+ le32_to_cpu(word));
len = 0;
} else {
memcpy(&word, tx_buf, pld_data_bytes);
- dsi_write(dsi, DSI_GEN_PLD_DATA, le32_to_cpu(word));
+ regmap_write(dsi->regs, DSI_GEN_PLD_DATA,
+ le32_to_cpu(word));
tx_buf += pld_data_bytes;
len -= pld_data_bytes;
}
- ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
- val, !(val & GEN_PLD_W_FULL), 1000,
- CMD_PKT_STATUS_TIMEOUT_US);
+ ret = regmap_read_poll_timeout(dsi->regs, DSI_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");
@@ -441,9 +443,9 @@ static int dw_mipi_dsi_read(struct dw_mipi_dsi *dsi,
u32 val;
/* Wait end of the read operation */
- ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
- val, !(val & GEN_RD_CMD_BUSY),
- 1000, CMD_PKT_STATUS_TIMEOUT_US);
+ ret = regmap_read_poll_timeout(dsi->regs, DSI_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;
@@ -451,15 +453,15 @@ 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 = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
- val, !(val & GEN_PLD_R_EMPTY),
- 1000, CMD_PKT_STATUS_TIMEOUT_US);
+ ret = regmap_read_poll_timeout(dsi->regs, DSI_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;
}
- val = dsi_read(dsi, DSI_GEN_PLD_DATA);
+ regmap_read(dsi->regs, DSI_GEN_PLD_DATA, &val);
for (j = 0; j < 4 && j + i < len; j++)
buf[i + j] = val >> (8 * j);
}
@@ -536,29 +538,29 @@ static void dw_mipi_dsi_video_mode_config(struct dw_mipi_dsi *dsi)
}
#endif /* CONFIG_DEBUG_FS */
- dsi_write(dsi, DSI_VID_MODE_CFG, val);
+ regmap_write(dsi->regs, DSI_VID_MODE_CFG, val);
}
static void dw_mipi_dsi_set_mode(struct dw_mipi_dsi *dsi,
unsigned long mode_flags)
{
- dsi_write(dsi, DSI_PWR_UP, RESET);
+ regmap_write(dsi->regs, DSI_PWR_UP, RESET);
if (mode_flags & MIPI_DSI_MODE_VIDEO) {
- dsi_write(dsi, DSI_MODE_CFG, ENABLE_VIDEO_MODE);
+ regmap_write(dsi->regs, DSI_MODE_CFG, ENABLE_VIDEO_MODE);
dw_mipi_dsi_video_mode_config(dsi);
- dsi_write(dsi, DSI_LPCLK_CTRL, PHY_TXREQUESTCLKHS);
+ regmap_write(dsi->regs, DSI_LPCLK_CTRL, PHY_TXREQUESTCLKHS);
} else {
- dsi_write(dsi, DSI_MODE_CFG, ENABLE_CMD_MODE);
+ regmap_write(dsi->regs, DSI_MODE_CFG, ENABLE_CMD_MODE);
}
- dsi_write(dsi, DSI_PWR_UP, POWERUP);
+ regmap_write(dsi->regs, DSI_PWR_UP, POWERUP);
}
static void dw_mipi_dsi_disable(struct dw_mipi_dsi *dsi)
{
- dsi_write(dsi, DSI_PWR_UP, RESET);
- dsi_write(dsi, DSI_PHY_RSTZ, PHY_RSTZ);
+ regmap_write(dsi->regs, DSI_PWR_UP, RESET);
+ regmap_write(dsi->regs, DSI_PHY_RSTZ, PHY_RSTZ);
}
static void dw_mipi_dsi_init(struct dw_mipi_dsi *dsi)
@@ -573,14 +575,14 @@ static void dw_mipi_dsi_init(struct dw_mipi_dsi *dsi)
*/
u32 esc_clk_division = (dsi->lane_mbps >> 3) / 20 + 1;
- dsi_write(dsi, DSI_PWR_UP, RESET);
+ regmap_write(dsi->regs, DSI_PWR_UP, RESET);
/*
* TODO dw drv improvements
* timeout clock division should be computed with the
* high speed transmission counter timeout and byte lane...
*/
- dsi_write(dsi, DSI_CLKMGR_CFG, TO_CLK_DIVISION(10) |
+ regmap_write(dsi->regs, DSI_CLKMGR_CFG, TO_CLK_DIVISION(10) |
TX_ESC_CLK_DIVISION(esc_clk_division));
}
@@ -609,22 +611,22 @@ static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi,
if (mode->flags & DRM_MODE_FLAG_NHSYNC)
val |= HSYNC_ACTIVE_LOW;
- dsi_write(dsi, DSI_DPI_VCID, DPI_VCID(dsi->channel));
- dsi_write(dsi, DSI_DPI_COLOR_CODING, color);
- dsi_write(dsi, DSI_DPI_CFG_POL, val);
+ 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
* should be computed according to byte lane, lane number and only
* if sending lp cmds in high speed is enable (PHY_TXREQUESTCLKHS)
*/
- dsi_write(dsi, DSI_DPI_LP_CMD_TIM, OUTVACT_LPCMD_TIME(4)
+ regmap_write(dsi->regs, DSI_DPI_LP_CMD_TIM, OUTVACT_LPCMD_TIME(4)
| INVACT_LPCMD_TIME(4));
}
static void dw_mipi_dsi_packet_handler_config(struct dw_mipi_dsi *dsi)
{
- dsi_write(dsi, DSI_PCKHDL_CFG, CRC_RX_EN | ECC_RX_EN | BTA_EN);
+ regmap_write(dsi->regs, DSI_PCKHDL_CFG, CRC_RX_EN | ECC_RX_EN | BTA_EN);
}
static void dw_mipi_dsi_video_packet_config(struct dw_mipi_dsi *dsi,
@@ -638,7 +640,7 @@ static void dw_mipi_dsi_video_packet_config(struct dw_mipi_dsi *dsi,
* non-burst video modes, see dw_mipi_dsi_video_mode_config()...
*/
- dsi_write(dsi, DSI_VID_PKT_SIZE,
+ 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));
@@ -651,14 +653,15 @@ 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...
*/
- dsi_write(dsi, DSI_TO_CNT_CFG, HSTX_TO_CNT(1000) | LPRX_TO_CNT(1000));
+ regmap_write(dsi->regs, DSI_TO_CNT_CFG,
+ HSTX_TO_CNT(1000) | LPRX_TO_CNT(1000));
/*
* TODO dw drv improvements
* the Bus-Turn-Around Timeout Counter should be computed
* according to byte lane...
*/
- dsi_write(dsi, DSI_BTA_TO_CNT, 0xd00);
- dsi_write(dsi, DSI_MODE_CFG, ENABLE_CMD_MODE);
+ regmap_write(dsi->regs, DSI_BTA_TO_CNT, 0xd00);
+ regmap_write(dsi->regs, DSI_MODE_CFG, ENABLE_CMD_MODE);
}
/* Get lane byte clock cycles. */
@@ -692,13 +695,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);
- dsi_write(dsi, DSI_VID_HLINE_TIME, lbcc);
+ regmap_write(dsi->regs, DSI_VID_HLINE_TIME, lbcc);
lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, mode, hsa);
- dsi_write(dsi, DSI_VID_HSA_TIME, lbcc);
+ regmap_write(dsi->regs, DSI_VID_HSA_TIME, lbcc);
lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, mode, hbp);
- dsi_write(dsi, DSI_VID_HBP_TIME, lbcc);
+ regmap_write(dsi->regs, DSI_VID_HBP_TIME, lbcc);
}
static void dw_mipi_dsi_vertical_timing_config(struct dw_mipi_dsi *dsi,
@@ -711,17 +714,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;
- dsi_write(dsi, DSI_VID_VACTIVE_LINES, vactive);
- dsi_write(dsi, DSI_VID_VSA_LINES, vsa);
- dsi_write(dsi, DSI_VID_VFP_LINES, vfp);
- dsi_write(dsi, DSI_VID_VBP_LINES, vbp);
+ 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);
}
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,
@@ -737,23 +739,22 @@ static void dw_mipi_dsi_dphy_timing_config(struct dw_mipi_dsi *dsi)
* DSI_CMD_MODE_CFG.MAX_RD_PKT_SIZE_LP (see CMD_MODE_ALL_LP)
*/
- hw_version = dsi_read(dsi, DSI_VERSION) & VERSION;
-
- if (hw_version >= HWVER_131) {
- dsi_write(dsi, DSI_PHY_TMR_CFG,
- PHY_HS2LP_TIME_V131(timing.data_hs2lp) |
- PHY_LP2HS_TIME_V131(timing.data_lp2hs));
- dsi_write(dsi, DSI_PHY_TMR_RD_CFG, MAX_RD_TIME_V131(10000));
+ if (dsi->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 {
- dsi_write(dsi, 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_CFG,
+ PHY_HS2LP_TIME(timing.data_hs2lp) |
+ PHY_LP2HS_TIME(timing.data_lp2hs) |
+ MAX_RD_TIME(10000));
}
- dsi_write(dsi, DSI_PHY_TMR_LPCLK_CFG,
- PHY_CLKHS2LP_TIME(timing.clk_hs2lp) |
- PHY_CLKLP2HS_TIME(timing.clk_lp2hs));
+ regmap_write(dsi->regs, DSI_PHY_TMR_LPCLK_CFG,
+ PHY_CLKHS2LP_TIME(timing.clk_hs2lp) |
+ PHY_CLKLP2HS_TIME(timing.clk_lp2hs));
}
static void dw_mipi_dsi_dphy_interface_config(struct dw_mipi_dsi *dsi)
@@ -763,18 +764,18 @@ 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
*/
- dsi_write(dsi, DSI_PHY_IF_CFG, PHY_STOP_WAIT_TIME(0x20) |
- N_LANES(dsi->lanes));
+ regmap_write(dsi->regs, DSI_PHY_IF_CFG,
+ PHY_STOP_WAIT_TIME(0x20) | N_LANES(dsi->lanes));
}
static void dw_mipi_dsi_dphy_init(struct dw_mipi_dsi *dsi)
{
/* Clear PHY state */
- dsi_write(dsi, DSI_PHY_RSTZ, PHY_DISFORCEPLL | PHY_DISABLECLK
- | PHY_RSTZ | PHY_SHUTDOWNZ);
- dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLR);
- dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLR);
- dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLR);
+ 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);
}
static void dw_mipi_dsi_dphy_enable(struct dw_mipi_dsi *dsi)
@@ -782,27 +783,30 @@ static void dw_mipi_dsi_dphy_enable(struct dw_mipi_dsi *dsi)
u32 val;
int ret;
- dsi_write(dsi, DSI_PHY_RSTZ, PHY_ENFORCEPLL | PHY_ENABLECLK |
- PHY_UNRSTZ | PHY_UNSHUTDOWNZ);
+ regmap_write(dsi->regs, DSI_PHY_RSTZ, PHY_ENFORCEPLL | PHY_ENABLECLK |
+ PHY_UNRSTZ | PHY_UNSHUTDOWNZ);
- ret = readl_poll_timeout(dsi->base + DSI_PHY_STATUS, val,
- val & PHY_LOCK, 1000, PHY_STATUS_TIMEOUT_US);
+ ret = regmap_read_poll_timeout(dsi->regs, DSI_PHY_STATUS,
+ val, val & PHY_LOCK,
+ 1000, PHY_STATUS_TIMEOUT_US);
if (ret)
DRM_DEBUG_DRIVER("failed to wait phy lock state\n");
- ret = readl_poll_timeout(dsi->base + DSI_PHY_STATUS,
- val, val & PHY_STOP_STATE_CLK_LANE, 1000,
- PHY_STATUS_TIMEOUT_US);
+ ret = regmap_read_poll_timeout(dsi->regs, DSI_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");
}
static void dw_mipi_dsi_clear_err(struct dw_mipi_dsi *dsi)
{
- dsi_read(dsi, DSI_INT_ST0);
- dsi_read(dsi, DSI_INT_ST1);
- dsi_write(dsi, DSI_INT_MSK0, 0);
- dsi_write(dsi, DSI_INT_MSK1, 0);
+ 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);
}
static void dw_mipi_dsi_bridge_post_disable(struct drm_bridge *bridge)
@@ -989,6 +993,18 @@ static void dw_mipi_dsi_debugfs_remove(struct dw_mipi_dsi *dsi) { }
#endif /* CONFIG_DEBUG_FS */
+static int dw_mipi_dsi_get_hw_version(struct dw_mipi_dsi *dsi)
+{
+ regmap_read(dsi->regs, DSI_VERSION, &dsi->hw_version);
+ dsi->hw_version &= VERSION;
+ if (!dsi->hw_version) {
+ dev_err(dsi->dev,
+ "Failed to read DSI version. Is pclk enabled?\n");
+ return -ENODEV;
+ }
+ return 0;
+}
+
static struct dw_mipi_dsi *
__dw_mipi_dsi_probe(struct platform_device *pdev,
const struct dw_mipi_dsi_plat_data *plat_data)
@@ -1020,6 +1036,14 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
dsi->base = plat_data->base;
}
+ dsi->regs = devm_regmap_init_mmio(dev, dsi->base,
+ &dw_mipi_dsi_regmap_cfg);
+ if (IS_ERR(dsi->regs)) {
+ ret = PTR_ERR(dsi->regs);
+ DRM_ERROR("Failed to create DW MIPI DSI regmap: %d\n", ret);
+ return ERR_PTR(ret);
+ }
+
dsi->pclk = devm_clk_get(dev, "pclk");
if (IS_ERR(dsi->pclk)) {
ret = PTR_ERR(dsi->pclk);
@@ -1055,6 +1079,12 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
clk_disable_unprepare(dsi->pclk);
}
+ ret = dw_mipi_dsi_get_hw_version(dsi);
+ if (ret) {
+ dev_err(dev, "Could not read HW version\n");
+ return ERR_PTR(ret);
+ }
+
dw_mipi_dsi_debugfs_init(dsi);
pm_runtime_enable(dev);
--
2.26.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v8 01/10] drm: bridge: dw_mipi_dsi: add initial regmap infrastructure
2020-04-27 8:19 ` [PATCH v8 01/10] drm: bridge: dw_mipi_dsi: add initial regmap infrastructure Adrian Ratiu
@ 2020-04-27 14:41 ` Enric Balletbo i Serra
0 siblings, 0 replies; 21+ messages in thread
From: Enric Balletbo i Serra @ 2020-04-27 14:41 UTC (permalink / raw)
To: Adrian Ratiu, linux-arm-kernel, devicetree, linux-rockchip,
Laurent Pinchart
Cc: Jernej Skrabec, Adrian Pop, Jonas Karlman, linux-kernel,
dri-devel, Andrzej Hajda, linux-imx, kernel, Ezequiel Garcia,
linux-stm32, Arnaud Ferraris
Hi Adrian,
Thank you for your patch and to apply the changes I requested
On 27/4/20 10:19, Adrian Ratiu wrote:
> In order to support multiple versions of the Synopsis MIPI DSI host
> controller, which have different register layouts but almost identical
> HW protocols, we add a regmap infrastructure which can abstract away
> register accesses for platform drivers using the bridge.
>
> The controller HW revision is detected during bridge probe which will
> be used in future commits to load the relevant register layout which
> the bridge will use transparently to the platform drivers.
>
> Cc: Enric Balletbo Serra <eballetbo@gmail.com>
> Suggested-by: Ezequiel Garcia <ezequiel@collabora.com>
> Tested-by: Adrian Pop <pop.adrian61@gmail.com>
> Tested-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Reviewed-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> Chnages since v7:
> - Minor checkpatch line fix
>
> Changes since v6:
> - Select REGMAP_MMIO in Kconfig (Enric)
> - Drop unnecessary stack variable inits (Enric)
> - Make bridge error ASAP after a bad revision read (Enric)
> - Drop redundant read of hw_version in dphy_timing_config (Enric)
>
> New in v5.
> ---
> drivers/gpu/drm/bridge/synopsys/Kconfig | 1 +
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 210 ++++++++++--------
> 2 files changed, 121 insertions(+), 90 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig b/drivers/gpu/drm/bridge/synopsys/Kconfig
> index 21a1be3ced0f3..080146093b68e 100644
> --- a/drivers/gpu/drm/bridge/synopsys/Kconfig
> +++ b/drivers/gpu/drm/bridge/synopsys/Kconfig
> @@ -39,3 +39,4 @@ config DRM_DW_MIPI_DSI
> select DRM_KMS_HELPER
> select DRM_MIPI_DSI
> select DRM_PANEL_BRIDGE
> + select REGMAP_MMIO
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 5ef0f154aa7bd..34b8668ae24ea 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -15,6 +15,7 @@
> #include <linux/module.h>
> #include <linux/of_device.h>
> #include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> #include <linux/reset.h>
>
> #include <video/mipi_display.h>
> @@ -227,6 +228,7 @@ struct dw_mipi_dsi {
> struct drm_bridge *panel_bridge;
> struct device *dev;
> void __iomem *base;
> + struct regmap *regs;
>
> struct clk *pclk;
>
> @@ -235,6 +237,7 @@ struct dw_mipi_dsi {
> u32 lanes;
> u32 format;
> unsigned long mode_flags;
> + u32 hw_version;
>
> #ifdef CONFIG_DEBUG_FS
> struct dentry *debugfs;
> @@ -249,6 +252,13 @@ struct dw_mipi_dsi {
> const struct dw_mipi_dsi_plat_data *plat_data;
> };
>
> +static const struct regmap_config dw_mipi_dsi_regmap_cfg = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .name = "dw-mipi-dsi",
> +};
> +
> /*
> * Check if either a link to a master or slave is present
> */
> @@ -280,16 +290,6 @@ static inline struct dw_mipi_dsi *bridge_to_dsi(struct drm_bridge *bridge)
> return container_of(bridge, struct dw_mipi_dsi, bridge);
> }
>
> -static inline void dsi_write(struct dw_mipi_dsi *dsi, u32 reg, u32 val)
> -{
> - writel(val, dsi->base + reg);
> -}
> -
> -static inline u32 dsi_read(struct dw_mipi_dsi *dsi, u32 reg)
> -{
> - return readl(dsi->base + reg);
> -}
> -
> static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
> struct mipi_dsi_device *device)
> {
> @@ -366,8 +366,8 @@ static void dw_mipi_message_config(struct dw_mipi_dsi *dsi,
> if (lpm)
> val |= CMD_MODE_ALL_LP;
>
> - dsi_write(dsi, DSI_LPCLK_CTRL, lpm ? 0 : PHY_TXREQUESTCLKHS);
> - dsi_write(dsi, DSI_CMD_MODE_CFG, val);
> + regmap_write(dsi->regs, DSI_LPCLK_CTRL, lpm ? 0 : PHY_TXREQUESTCLKHS);
> + regmap_write(dsi->regs, DSI_CMD_MODE_CFG, val);
> }
>
> static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 hdr_val)
> @@ -375,20 +375,20 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 hdr_val)
> int ret;
> u32 val, mask;
>
> - ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
> - val, !(val & GEN_CMD_FULL), 1000,
> - CMD_PKT_STATUS_TIMEOUT_US);
> + ret = regmap_read_poll_timeout(dsi->regs, DSI_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;
> }
>
> - dsi_write(dsi, DSI_GEN_HDR, hdr_val);
> + regmap_write(dsi->regs, DSI_GEN_HDR, hdr_val);
>
> mask = GEN_CMD_EMPTY | GEN_PLD_W_EMPTY;
> - ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
> - val, (val & mask) == mask,
> - 1000, CMD_PKT_STATUS_TIMEOUT_US);
> + ret = regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS,
> + val, (val & mask) == mask,
> + 1000, CMD_PKT_STATUS_TIMEOUT_US);
> if (ret) {
> dev_err(dsi->dev, "failed to write command FIFO\n");
> return ret;
> @@ -409,18 +409,20 @@ static int dw_mipi_dsi_write(struct dw_mipi_dsi *dsi,
> if (len < pld_data_bytes) {
> word = 0;
> memcpy(&word, tx_buf, len);
> - dsi_write(dsi, DSI_GEN_PLD_DATA, le32_to_cpu(word));
> + regmap_write(dsi->regs, DSI_GEN_PLD_DATA,
> + le32_to_cpu(word));
> len = 0;
> } else {
> memcpy(&word, tx_buf, pld_data_bytes);
> - dsi_write(dsi, DSI_GEN_PLD_DATA, le32_to_cpu(word));
> + regmap_write(dsi->regs, DSI_GEN_PLD_DATA,
> + le32_to_cpu(word));
> tx_buf += pld_data_bytes;
> len -= pld_data_bytes;
> }
>
> - ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
> - val, !(val & GEN_PLD_W_FULL), 1000,
> - CMD_PKT_STATUS_TIMEOUT_US);
> + ret = regmap_read_poll_timeout(dsi->regs, DSI_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");
> @@ -441,9 +443,9 @@ static int dw_mipi_dsi_read(struct dw_mipi_dsi *dsi,
> u32 val;
>
> /* Wait end of the read operation */
> - ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
> - val, !(val & GEN_RD_CMD_BUSY),
> - 1000, CMD_PKT_STATUS_TIMEOUT_US);
> + ret = regmap_read_poll_timeout(dsi->regs, DSI_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;
> @@ -451,15 +453,15 @@ 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 = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
> - val, !(val & GEN_PLD_R_EMPTY),
> - 1000, CMD_PKT_STATUS_TIMEOUT_US);
> + ret = regmap_read_poll_timeout(dsi->regs, DSI_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;
> }
>
> - val = dsi_read(dsi, DSI_GEN_PLD_DATA);
> + regmap_read(dsi->regs, DSI_GEN_PLD_DATA, &val);
> for (j = 0; j < 4 && j + i < len; j++)
> buf[i + j] = val >> (8 * j);
> }
> @@ -536,29 +538,29 @@ static void dw_mipi_dsi_video_mode_config(struct dw_mipi_dsi *dsi)
> }
> #endif /* CONFIG_DEBUG_FS */
>
> - dsi_write(dsi, DSI_VID_MODE_CFG, val);
> + regmap_write(dsi->regs, DSI_VID_MODE_CFG, val);
> }
>
> static void dw_mipi_dsi_set_mode(struct dw_mipi_dsi *dsi,
> unsigned long mode_flags)
> {
> - dsi_write(dsi, DSI_PWR_UP, RESET);
> + regmap_write(dsi->regs, DSI_PWR_UP, RESET);
>
> if (mode_flags & MIPI_DSI_MODE_VIDEO) {
> - dsi_write(dsi, DSI_MODE_CFG, ENABLE_VIDEO_MODE);
> + regmap_write(dsi->regs, DSI_MODE_CFG, ENABLE_VIDEO_MODE);
> dw_mipi_dsi_video_mode_config(dsi);
> - dsi_write(dsi, DSI_LPCLK_CTRL, PHY_TXREQUESTCLKHS);
> + regmap_write(dsi->regs, DSI_LPCLK_CTRL, PHY_TXREQUESTCLKHS);
> } else {
> - dsi_write(dsi, DSI_MODE_CFG, ENABLE_CMD_MODE);
> + regmap_write(dsi->regs, DSI_MODE_CFG, ENABLE_CMD_MODE);
> }
>
> - dsi_write(dsi, DSI_PWR_UP, POWERUP);
> + regmap_write(dsi->regs, DSI_PWR_UP, POWERUP);
> }
>
> static void dw_mipi_dsi_disable(struct dw_mipi_dsi *dsi)
> {
> - dsi_write(dsi, DSI_PWR_UP, RESET);
> - dsi_write(dsi, DSI_PHY_RSTZ, PHY_RSTZ);
> + regmap_write(dsi->regs, DSI_PWR_UP, RESET);
> + regmap_write(dsi->regs, DSI_PHY_RSTZ, PHY_RSTZ);
> }
>
> static void dw_mipi_dsi_init(struct dw_mipi_dsi *dsi)
> @@ -573,14 +575,14 @@ static void dw_mipi_dsi_init(struct dw_mipi_dsi *dsi)
> */
> u32 esc_clk_division = (dsi->lane_mbps >> 3) / 20 + 1;
>
> - dsi_write(dsi, DSI_PWR_UP, RESET);
> + regmap_write(dsi->regs, DSI_PWR_UP, RESET);
>
> /*
> * TODO dw drv improvements
> * timeout clock division should be computed with the
> * high speed transmission counter timeout and byte lane...
> */
> - dsi_write(dsi, DSI_CLKMGR_CFG, TO_CLK_DIVISION(10) |
> + regmap_write(dsi->regs, DSI_CLKMGR_CFG, TO_CLK_DIVISION(10) |
> TX_ESC_CLK_DIVISION(esc_clk_division));
> }
>
> @@ -609,22 +611,22 @@ static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi,
> if (mode->flags & DRM_MODE_FLAG_NHSYNC)
> val |= HSYNC_ACTIVE_LOW;
>
> - dsi_write(dsi, DSI_DPI_VCID, DPI_VCID(dsi->channel));
> - dsi_write(dsi, DSI_DPI_COLOR_CODING, color);
> - dsi_write(dsi, DSI_DPI_CFG_POL, val);
> + 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
> * should be computed according to byte lane, lane number and only
> * if sending lp cmds in high speed is enable (PHY_TXREQUESTCLKHS)
> */
> - dsi_write(dsi, DSI_DPI_LP_CMD_TIM, OUTVACT_LPCMD_TIME(4)
> + regmap_write(dsi->regs, DSI_DPI_LP_CMD_TIM, OUTVACT_LPCMD_TIME(4)
> | INVACT_LPCMD_TIME(4));
> }
>
> static void dw_mipi_dsi_packet_handler_config(struct dw_mipi_dsi *dsi)
> {
> - dsi_write(dsi, DSI_PCKHDL_CFG, CRC_RX_EN | ECC_RX_EN | BTA_EN);
> + regmap_write(dsi->regs, DSI_PCKHDL_CFG, CRC_RX_EN | ECC_RX_EN | BTA_EN);
> }
>
> static void dw_mipi_dsi_video_packet_config(struct dw_mipi_dsi *dsi,
> @@ -638,7 +640,7 @@ static void dw_mipi_dsi_video_packet_config(struct dw_mipi_dsi *dsi,
> * non-burst video modes, see dw_mipi_dsi_video_mode_config()...
> */
>
> - dsi_write(dsi, DSI_VID_PKT_SIZE,
> + 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));
> @@ -651,14 +653,15 @@ 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...
> */
> - dsi_write(dsi, DSI_TO_CNT_CFG, HSTX_TO_CNT(1000) | LPRX_TO_CNT(1000));
> + regmap_write(dsi->regs, DSI_TO_CNT_CFG,
> + HSTX_TO_CNT(1000) | LPRX_TO_CNT(1000));
> /*
> * TODO dw drv improvements
> * the Bus-Turn-Around Timeout Counter should be computed
> * according to byte lane...
> */
> - dsi_write(dsi, DSI_BTA_TO_CNT, 0xd00);
> - dsi_write(dsi, DSI_MODE_CFG, ENABLE_CMD_MODE);
> + regmap_write(dsi->regs, DSI_BTA_TO_CNT, 0xd00);
> + regmap_write(dsi->regs, DSI_MODE_CFG, ENABLE_CMD_MODE);
> }
>
> /* Get lane byte clock cycles. */
> @@ -692,13 +695,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);
> - dsi_write(dsi, DSI_VID_HLINE_TIME, lbcc);
> + regmap_write(dsi->regs, DSI_VID_HLINE_TIME, lbcc);
>
> lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, mode, hsa);
> - dsi_write(dsi, DSI_VID_HSA_TIME, lbcc);
> + regmap_write(dsi->regs, DSI_VID_HSA_TIME, lbcc);
>
> lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, mode, hbp);
> - dsi_write(dsi, DSI_VID_HBP_TIME, lbcc);
> + regmap_write(dsi->regs, DSI_VID_HBP_TIME, lbcc);
> }
>
> static void dw_mipi_dsi_vertical_timing_config(struct dw_mipi_dsi *dsi,
> @@ -711,17 +714,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;
>
> - dsi_write(dsi, DSI_VID_VACTIVE_LINES, vactive);
> - dsi_write(dsi, DSI_VID_VSA_LINES, vsa);
> - dsi_write(dsi, DSI_VID_VFP_LINES, vfp);
> - dsi_write(dsi, DSI_VID_VBP_LINES, vbp);
> + 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);
> }
>
> 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,
> @@ -737,23 +739,22 @@ static void dw_mipi_dsi_dphy_timing_config(struct dw_mipi_dsi *dsi)
> * DSI_CMD_MODE_CFG.MAX_RD_PKT_SIZE_LP (see CMD_MODE_ALL_LP)
> */
>
> - hw_version = dsi_read(dsi, DSI_VERSION) & VERSION;
> -
> - if (hw_version >= HWVER_131) {
> - dsi_write(dsi, DSI_PHY_TMR_CFG,
> - PHY_HS2LP_TIME_V131(timing.data_hs2lp) |
> - PHY_LP2HS_TIME_V131(timing.data_lp2hs));
> - dsi_write(dsi, DSI_PHY_TMR_RD_CFG, MAX_RD_TIME_V131(10000));
> + if (dsi->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 {
> - dsi_write(dsi, 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_CFG,
> + PHY_HS2LP_TIME(timing.data_hs2lp) |
> + PHY_LP2HS_TIME(timing.data_lp2hs) |
> + MAX_RD_TIME(10000));
> }
>
> - dsi_write(dsi, DSI_PHY_TMR_LPCLK_CFG,
> - PHY_CLKHS2LP_TIME(timing.clk_hs2lp) |
> - PHY_CLKLP2HS_TIME(timing.clk_lp2hs));
> + regmap_write(dsi->regs, DSI_PHY_TMR_LPCLK_CFG,
> + PHY_CLKHS2LP_TIME(timing.clk_hs2lp) |
> + PHY_CLKLP2HS_TIME(timing.clk_lp2hs));
> }
>
> static void dw_mipi_dsi_dphy_interface_config(struct dw_mipi_dsi *dsi)
> @@ -763,18 +764,18 @@ 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
> */
> - dsi_write(dsi, DSI_PHY_IF_CFG, PHY_STOP_WAIT_TIME(0x20) |
> - N_LANES(dsi->lanes));
> + regmap_write(dsi->regs, DSI_PHY_IF_CFG,
> + PHY_STOP_WAIT_TIME(0x20) | N_LANES(dsi->lanes));
> }
>
> static void dw_mipi_dsi_dphy_init(struct dw_mipi_dsi *dsi)
> {
> /* Clear PHY state */
> - dsi_write(dsi, DSI_PHY_RSTZ, PHY_DISFORCEPLL | PHY_DISABLECLK
> - | PHY_RSTZ | PHY_SHUTDOWNZ);
> - dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLR);
> - dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLR);
> - dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLR);
> + 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);
> }
>
> static void dw_mipi_dsi_dphy_enable(struct dw_mipi_dsi *dsi)
> @@ -782,27 +783,30 @@ static void dw_mipi_dsi_dphy_enable(struct dw_mipi_dsi *dsi)
> u32 val;
> int ret;
>
> - dsi_write(dsi, DSI_PHY_RSTZ, PHY_ENFORCEPLL | PHY_ENABLECLK |
> - PHY_UNRSTZ | PHY_UNSHUTDOWNZ);
> + regmap_write(dsi->regs, DSI_PHY_RSTZ, PHY_ENFORCEPLL | PHY_ENABLECLK |
> + PHY_UNRSTZ | PHY_UNSHUTDOWNZ);
>
> - ret = readl_poll_timeout(dsi->base + DSI_PHY_STATUS, val,
> - val & PHY_LOCK, 1000, PHY_STATUS_TIMEOUT_US);
> + ret = regmap_read_poll_timeout(dsi->regs, DSI_PHY_STATUS,
> + val, val & PHY_LOCK,
> + 1000, PHY_STATUS_TIMEOUT_US);
> if (ret)
> DRM_DEBUG_DRIVER("failed to wait phy lock state\n");
>
> - ret = readl_poll_timeout(dsi->base + DSI_PHY_STATUS,
> - val, val & PHY_STOP_STATE_CLK_LANE, 1000,
> - PHY_STATUS_TIMEOUT_US);
> + ret = regmap_read_poll_timeout(dsi->regs, DSI_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");
> }
>
> static void dw_mipi_dsi_clear_err(struct dw_mipi_dsi *dsi)
> {
> - dsi_read(dsi, DSI_INT_ST0);
> - dsi_read(dsi, DSI_INT_ST1);
> - dsi_write(dsi, DSI_INT_MSK0, 0);
> - dsi_write(dsi, DSI_INT_MSK1, 0);
> + 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);
> }
>
> static void dw_mipi_dsi_bridge_post_disable(struct drm_bridge *bridge)
> @@ -989,6 +993,18 @@ static void dw_mipi_dsi_debugfs_remove(struct dw_mipi_dsi *dsi) { }
>
> #endif /* CONFIG_DEBUG_FS */
>
> +static int dw_mipi_dsi_get_hw_version(struct dw_mipi_dsi *dsi)
> +{
> + regmap_read(dsi->regs, DSI_VERSION, &dsi->hw_version);
> + dsi->hw_version &= VERSION;
> + if (!dsi->hw_version) {
> + dev_err(dsi->dev,
> + "Failed to read DSI version. Is pclk enabled?\n");
> + return -ENODEV;
> + }
> + return 0;
> +}
> +
> static struct dw_mipi_dsi *
> __dw_mipi_dsi_probe(struct platform_device *pdev,
> const struct dw_mipi_dsi_plat_data *plat_data)
> @@ -1020,6 +1036,14 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> dsi->base = plat_data->base;
> }
>
> + dsi->regs = devm_regmap_init_mmio(dev, dsi->base,
> + &dw_mipi_dsi_regmap_cfg);
> + if (IS_ERR(dsi->regs)) {
> + ret = PTR_ERR(dsi->regs);
> + DRM_ERROR("Failed to create DW MIPI DSI regmap: %d\n", ret);
> + return ERR_PTR(ret);
> + }
> +
> dsi->pclk = devm_clk_get(dev, "pclk");
> if (IS_ERR(dsi->pclk)) {
> ret = PTR_ERR(dsi->pclk);
> @@ -1055,6 +1079,12 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> clk_disable_unprepare(dsi->pclk);
> }
>
> + ret = dw_mipi_dsi_get_hw_version(dsi);
> + if (ret) {
> + dev_err(dev, "Could not read HW version\n");
> + return ERR_PTR(ret);
> + }
> +
> dw_mipi_dsi_debugfs_init(dsi);
> pm_runtime_enable(dev);
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v8 02/10] drm: bridge: dw_mipi_dsi: abstract register access using reg_fields
2020-04-27 8:19 [PATCH v8 00/10] Genericize DW MIPI DSI bridge and add i.MX 6 driver Adrian Ratiu
2020-04-27 8:19 ` [PATCH v8 01/10] drm: bridge: dw_mipi_dsi: add initial regmap infrastructure Adrian Ratiu
@ 2020-04-27 8:19 ` Adrian Ratiu
2020-04-27 8:19 ` [PATCH v8 03/10] drm: bridge: dw_mipi_dsi: add dsi v1.01 support Adrian Ratiu
` (7 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Adrian Ratiu @ 2020-04-27 8:19 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, linux-rockchip, Laurent Pinchart
Cc: Jernej Skrabec, Adrian Pop, Jonas Karlman, linux-kernel,
dri-devel, Andrzej Hajda, Boris Brezillon, linux-imx, kernel,
linux-stm32, Arnaud Ferraris, Emil Velikov
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>
Tested-by: Adrian Pop <pop.adrian61@gmail.com>
Tested-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
Changes since v5:
- Fix CONFIG_DEBUG_FS build (Adrian)
- Fix DRM_MODE_FLAG_* test negation (Adrian)
- Fixed cfg_phy_status range from [0,0] to [0,2]
- Replace do {} while(0) with GCC extension ({}) (Andrzej)
- Fixed payload no-op writes on STM devices (Adrian & Arnaud)
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 | 499 ++++++++++++------
1 file changed, 347 insertions(+), 152 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index 34b8668ae24ea..f453df4eb5072 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, 2),
+ .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, 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_force_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_force_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;
/* 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->field_vid_mode_vpg_en, 1);
+ regmap_field_write(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,14 @@ static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi,
break;
}
+ regmap_field_write(dsi->field_dpi_vid, DPI_VCID(dsi->channel));
+ regmap_field_write(dsi->field_dpi_color_coding, color);
+
if (mode->flags & DRM_MODE_FLAG_NVSYNC)
- val |= VSYNC_ACTIVE_LOW;
+ regmap_field_write(dsi->field_dpi_vsync_active_low, 1);
if (mode->flags & DRM_MODE_FLAG_NHSYNC)
- val |= HSYNC_ACTIVE_LOW;
+ regmap_field_write(dsi->field_dpi_hsync_active_low, 1);
- 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 +740,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 +754,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 +766,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 +810,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,10 +829,10 @@ 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)
@@ -738,23 +853,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);
- if (dsi->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)
@@ -764,18 +868,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)
@@ -783,18 +891,21 @@ static void dw_mipi_dsi_dphy_enable(struct dw_mipi_dsi *dsi)
u32 val;
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);
- ret = regmap_read_poll_timeout(dsi->regs, DSI_PHY_STATUS,
- val, val & PHY_LOCK,
- 1000, PHY_STATUS_TIMEOUT_US);
+ regmap_field_write(dsi->field_phy_forcepll, 1);
+
+ 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");
}
@@ -803,10 +914,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 +1116,84 @@ static int dw_mipi_dsi_get_hw_version(struct dw_mipi_dsi *dsi)
return 0;
}
+#define INIT_FIELD(f) INIT_FIELD_CFG(field_##f, cfg_##f)
+#define INIT_FIELD_CFG(f, conf) ({ \
+ 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"); })
+
+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);
+ }
+
+ return 0;
+}
+
static struct dw_mipi_dsi *
__dw_mipi_dsi_probe(struct platform_device *pdev,
const struct dw_mipi_dsi_plat_data *plat_data)
@@ -1085,6 +1274,12 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
return ERR_PTR(ret);
}
+ 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);
--
2.26.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v8 03/10] drm: bridge: dw_mipi_dsi: add dsi v1.01 support
2020-04-27 8:19 [PATCH v8 00/10] Genericize DW MIPI DSI bridge and add i.MX 6 driver Adrian Ratiu
2020-04-27 8:19 ` [PATCH v8 01/10] drm: bridge: dw_mipi_dsi: add initial regmap infrastructure Adrian Ratiu
2020-04-27 8:19 ` [PATCH v8 02/10] drm: bridge: dw_mipi_dsi: abstract register access using reg_fields Adrian Ratiu
@ 2020-04-27 8:19 ` Adrian Ratiu
2020-04-27 8:19 ` [PATCH v8 04/10] drm: bridge: dw_mipi_dsi: allow bridge daisy chaining Adrian Ratiu
` (6 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Adrian Ratiu @ 2020-04-27 8:19 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, linux-rockchip, Laurent Pinchart
Cc: Jernej Skrabec, Adrian Pop, Jonas Karlman, linux-kernel,
dri-devel, Andrzej Hajda, linux-imx, kernel, linux-stm32,
Arnaud Ferraris
The Synopsis MIPI DSI v1.01 host controller is quite widely used
on platforms like i.mx6 and is not very different from the other
versions like the 1.31/1.30 used on rockchip/stm. The protocols
appear to be the same, only the register layout is different and
the newer versions have new features symbolized by new registers
so adding support for it is just a matter of defining the new
layout and adding a couple of dsi version checks.
Tested-by: Adrian Pop <pop.adrian61@gmail.com>
Tested-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
Changes since v7:
- Minor commit msg rewording for consistency
Changes since v5:
- Fixed cfg_phy_status range from [0,0] to [0,2]
New in v5.
---
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 125 +++++++++++++++++-
1 file changed, 119 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index f453df4eb5072..16fd87055e7b7 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -32,6 +32,7 @@
#define HWVER_131 0x31333100 /* IP version 1.31 */
#define HWVER_130 0x31333000 /* IP version 1.30 */
+#define HWVER_101 0x31303000 /* IP version 1.01 */
#define DSI_VERSION 0x00
#define VERSION GENMASK(31, 8)
@@ -100,6 +101,25 @@
#define DSI_EDPI_CMD_SIZE 0x64
#define DSI_CMD_MODE_CFG 0x68
+
+#define DSI_DPI_CFG 0x0c
+#define DSI_TMR_LINE_CFG 0x28
+#define DSI_VTIMING_CFG 0x2c
+#define DSI_PHY_TMR_CFG_V101 0x30
+#define DSI_PHY_IF_CFG_V101 0x58
+#define DSI_PHY_IF_CTRL 0x5c
+#define DSI_PHY_RSTZ_V101 0x54
+#define DSI_PHY_STATUS_V101 0x60
+#define DSI_PHY_TST_CTRL0_V101 0x64
+#define DSI_GEN_HDR_V101 0x34
+#define DSI_GEN_PLD_DATA_V101 0x38
+#define DSI_CMD_MODE_CFG_V101 0x24
+#define DSI_CMD_PKT_STATUS_V101 0x3c
+#define DSI_VID_PKT_CFG 0x20
+#define DSI_VID_MODE_CFG_V101 0x1c
+#define DSI_TO_CNT_CFG_V101 0x40
+#define DSI_PCKHDL_CFG_V101 0x18
+
#define MAX_RD_PKT_SIZE_LP BIT(24)
#define DCS_LW_TX_LP BIT(19)
#define DCS_SR_0P_TX_LP BIT(18)
@@ -127,6 +147,33 @@
GEN_SW_1P_TX_LP | \
GEN_SW_0P_TX_LP)
+#define EN_TEAR_FX_V101 BIT(14)
+#define DCS_LW_TX_LP_V101 BIT(12)
+#define GEN_LW_TX_LP_V101 BIT(11)
+#define MAX_RD_PKT_SIZE_LP_V101 BIT(10)
+#define DCS_SW_2P_TX_LP_V101 BIT(9)
+#define DCS_SW_1P_TX_LP_V101 BIT(8)
+#define DCS_SW_0P_TX_LP_V101 BIT(7)
+#define GEN_SR_2P_TX_LP_V101 BIT(6)
+#define GEN_SR_1P_TX_LP_V101 BIT(5)
+#define GEN_SR_0P_TX_LP_V101 BIT(4)
+#define GEN_SW_2P_TX_LP_V101 BIT(3)
+#define GEN_SW_1P_TX_LP_V101 BIT(2)
+#define GEN_SW_0P_TX_LP_V101 BIT(1)
+
+#define CMD_MODE_ALL_LP_V101 (DCS_LW_TX_LP_V101 | \
+ GEN_LW_TX_LP_V101 | \
+ MAX_RD_PKT_SIZE_LP_V101 | \
+ DCS_SW_2P_TX_LP_V101 | \
+ DCS_SW_1P_TX_LP_V101 | \
+ DCS_SW_0P_TX_LP_V101 | \
+ GEN_SR_2P_TX_LP_V101 | \
+ GEN_SR_1P_TX_LP_V101 | \
+ GEN_SR_0P_TX_LP_V101 | \
+ GEN_SW_2P_TX_LP_V101 | \
+ GEN_SW_1P_TX_LP_V101 | \
+ GEN_SW_0P_TX_LP_V101)
+
#define DSI_GEN_HDR 0x6c
#define DSI_GEN_PLD_DATA 0x70
@@ -165,6 +212,11 @@
#define DSI_INT_MSK0 0xc4
#define DSI_INT_MSK1 0xc8
+#define DSI_ERROR_ST0_V101 0x44
+#define DSI_ERROR_ST1_V101 0x48
+#define DSI_ERROR_MSK0_V101 0x4c
+#define DSI_ERROR_MSK1_V101 0x50
+
#define DSI_PHY_TMR_RD_CFG 0xf4
#define PHY_STATUS_TIMEOUT_US 10000
@@ -359,6 +411,49 @@ static const struct dw_mipi_dsi_variant dw_mipi_dsi_v130_v131_layout = {
.cfg_gen_payload = REG_FIELD(DSI_GEN_PLD_DATA, 0, 31),
};
+static const struct dw_mipi_dsi_variant dw_mipi_dsi_v101_layout = {
+ .cfg_dpi_vid = REG_FIELD(DSI_DPI_CFG, 0, 1),
+ .cfg_dpi_color_coding = REG_FIELD(DSI_DPI_CFG, 2, 4),
+ .cfg_dpi_18loosely_en = REG_FIELD(DSI_DPI_CFG, 10, 10),
+ .cfg_dpi_vsync_active_low = REG_FIELD(DSI_DPI_CFG, 6, 6),
+ .cfg_dpi_hsync_active_low = REG_FIELD(DSI_DPI_CFG, 7, 7),
+ .cfg_cmd_mode_en = REG_FIELD(DSI_CMD_MODE_CFG_V101, 0, 0),
+ .cfg_cmd_mode_all_lp_en = REG_FIELD(DSI_CMD_MODE_CFG_V101, 1, 12),
+ .cfg_cmd_mode_ack_rqst_en = REG_FIELD(DSI_CMD_MODE_CFG_V101, 13, 13),
+ .cfg_cmd_pkt_status = REG_FIELD(DSI_CMD_PKT_STATUS_V101, 0, 14),
+ .cfg_vid_mode_en = REG_FIELD(DSI_VID_MODE_CFG_V101, 0, 0),
+ .cfg_vid_mode_type = REG_FIELD(DSI_VID_MODE_CFG_V101, 1, 2),
+ .cfg_vid_mode_low_power = REG_FIELD(DSI_VID_MODE_CFG_V101, 3, 8),
+ .cfg_vid_pkt_size = REG_FIELD(DSI_VID_PKT_CFG, 0, 10),
+ .cfg_vid_hsa_time = REG_FIELD(DSI_TMR_LINE_CFG, 0, 8),
+ .cfg_vid_hbp_time = REG_FIELD(DSI_TMR_LINE_CFG, 9, 17),
+ .cfg_vid_hline_time = REG_FIELD(DSI_TMR_LINE_CFG, 18, 31),
+ .cfg_vid_vsa_time = REG_FIELD(DSI_VTIMING_CFG, 0, 3),
+ .cfg_vid_vbp_time = REG_FIELD(DSI_VTIMING_CFG, 4, 9),
+ .cfg_vid_vfp_time = REG_FIELD(DSI_VTIMING_CFG, 10, 15),
+ .cfg_vid_vactive_time = REG_FIELD(DSI_VTIMING_CFG, 16, 26),
+ .cfg_phy_txrequestclkhs = REG_FIELD(DSI_PHY_IF_CTRL, 0, 0),
+ .cfg_phy_bta_time = REG_FIELD(DSI_PHY_TMR_CFG_V101, 0, 11),
+ .cfg_phy_lp2hs_time = REG_FIELD(DSI_PHY_TMR_CFG_V101, 12, 19),
+ .cfg_phy_hs2lp_time = REG_FIELD(DSI_PHY_TMR_CFG_V101, 20, 27),
+ .cfg_phy_testclr = REG_FIELD(DSI_PHY_TST_CTRL0_V101, 0, 0),
+ .cfg_phy_unshutdownz = REG_FIELD(DSI_PHY_RSTZ_V101, 0, 0),
+ .cfg_phy_unrstz = REG_FIELD(DSI_PHY_RSTZ_V101, 1, 1),
+ .cfg_phy_enableclk = REG_FIELD(DSI_PHY_RSTZ_V101, 2, 2),
+ .cfg_phy_nlanes = REG_FIELD(DSI_PHY_IF_CFG_V101, 0, 1),
+ .cfg_phy_stop_wait_time = REG_FIELD(DSI_PHY_IF_CFG_V101, 2, 9),
+ .cfg_phy_status = REG_FIELD(DSI_PHY_STATUS_V101, 0, 2),
+ .cfg_pckhdl_cfg = REG_FIELD(DSI_PCKHDL_CFG_V101, 0, 4),
+ .cfg_hstx_timeout_counter = REG_FIELD(DSI_TO_CNT_CFG_V101, 0, 15),
+ .cfg_lprx_timeout_counter = REG_FIELD(DSI_TO_CNT_CFG_V101, 16, 31),
+ .cfg_int_stat0 = REG_FIELD(DSI_ERROR_ST0_V101, 0, 20),
+ .cfg_int_stat1 = REG_FIELD(DSI_ERROR_ST1_V101, 0, 17),
+ .cfg_int_mask0 = REG_FIELD(DSI_ERROR_MSK0_V101, 0, 20),
+ .cfg_int_mask1 = REG_FIELD(DSI_ERROR_MSK1_V101, 0, 17),
+ .cfg_gen_hdr = REG_FIELD(DSI_GEN_HDR_V101, 0, 31),
+ .cfg_gen_payload = REG_FIELD(DSI_GEN_PLD_DATA_V101, 0, 31),
+};
+
/*
* Check if either a link to a master or slave is present
*/
@@ -466,6 +561,9 @@ static void dw_mipi_message_config(struct dw_mipi_dsi *dsi,
case HWVER_131:
cmd_mode_lp = CMD_MODE_ALL_LP;
break;
+ case HWVER_101:
+ cmd_mode_lp = CMD_MODE_ALL_LP_V101;
+ break;
}
if (msg->flags & MIPI_DSI_MSG_REQ_ACK)
@@ -644,7 +742,7 @@ static void dw_mipi_dsi_video_mode_config(struct dw_mipi_dsi *dsi)
VID_MODE_TYPE_NON_BURST_SYNC_EVENTS);
#ifdef CONFIG_DEBUG_FS
- if (dsi->vpg) {
+ if (dsi->hw_version > HWVER_101 && dsi->vpg) {
regmap_field_write(dsi->field_vid_mode_vpg_en, 1);
regmap_field_write(dsi->field_vid_mode_vpg_horiz,
dsi->vpg_horizontal ? 1 : 0);
@@ -662,9 +760,15 @@ static void dw_mipi_dsi_set_mode(struct dw_mipi_dsi *dsi,
dw_mipi_dsi_video_mode_config(dsi);
+ if (dsi->hw_version == HWVER_101)
+ regmap_field_write(dsi->field_vid_mode_en, 1);
+
regmap_field_write(dsi->field_phy_txrequestclkhs, 1);
} else {
regmap_field_write(dsi->field_cmd_mode_en, 1);
+
+ if (dsi->hw_version == HWVER_101)
+ regmap_field_write(dsi->field_vid_mode_en, 0);
}
regmap_write(dsi->regs, DSI_PWR_UP, POWERUP);
@@ -856,9 +960,13 @@ static void dw_mipi_dsi_dphy_timing_config(struct dw_mipi_dsi *dsi)
regmap_field_write(dsi->field_phy_lp2hs_time, timing.data_lp2hs);
regmap_field_write(dsi->field_phy_hs2lp_time, timing.data_hs2lp);
- 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);
+ if (dsi->hw_version > HWVER_101) {
+ 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)
@@ -879,7 +987,8 @@ static void dw_mipi_dsi_dphy_init(struct dw_mipi_dsi *dsi)
regmap_field_write(dsi->field_phy_unrstz, 0);
regmap_field_write(dsi->field_phy_unshutdownz, 0);
- regmap_field_write(dsi->field_phy_forcepll, 0);
+ if (dsi->hw_version > HWVER_101)
+ regmap_field_write(dsi->field_phy_forcepll, 0);
regmap_field_write(dsi->field_phy_testclr, 0);
regmap_field_write(dsi->field_phy_testclr, 1);
@@ -895,7 +1004,8 @@ static void dw_mipi_dsi_dphy_enable(struct dw_mipi_dsi *dsi)
regmap_field_write(dsi->field_phy_unrstz, 1);
regmap_field_write(dsi->field_phy_unshutdownz, 1);
- regmap_field_write(dsi->field_phy_forcepll, 1);
+ if (dsi->hw_version > HWVER_101)
+ regmap_field_write(dsi->field_phy_forcepll, 1);
ret = regmap_field_read_poll_timeout(dsi->field_phy_status,
val, val & PHY_LOCK,
@@ -1132,6 +1242,9 @@ static int dw_mipi_dsi_regmap_fields_init(struct dw_mipi_dsi *dsi)
case HWVER_131:
variant = &dw_mipi_dsi_v130_v131_layout;
break;
+ case HWVER_101:
+ variant = &dw_mipi_dsi_v101_layout;
+ break;
default:
DRM_ERROR("Unrecognized DSI host controller HW revision\n");
return -ENODEV;
--
2.26.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v8 04/10] drm: bridge: dw_mipi_dsi: allow bridge daisy chaining
2020-04-27 8:19 [PATCH v8 00/10] Genericize DW MIPI DSI bridge and add i.MX 6 driver Adrian Ratiu
` (2 preceding siblings ...)
2020-04-27 8:19 ` [PATCH v8 03/10] drm: bridge: dw_mipi_dsi: add dsi v1.01 support Adrian Ratiu
@ 2020-04-27 8:19 ` Adrian Ratiu
2020-06-02 23:51 ` Laurent Pinchart
2020-04-27 8:19 ` [PATCH v8 05/10] drm: imx: Add i.MX 6 MIPI DSI host platform driver Adrian Ratiu
` (5 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Adrian Ratiu @ 2020-04-27 8:19 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, linux-rockchip, Laurent Pinchart
Cc: Jernej Skrabec, Jonas Karlman, linux-kernel, dri-devel,
Andrzej Hajda, linux-imx, kernel, linux-stm32, Laurent Pinchart
Up until now the assumption was that the synopsis dsi bridge will
directly connect to an encoder provided by the platform driver, but
the current practice for drivers is to leave the encoder empty via
the simple encoder API and add their logic to their own drm_bridge.
Thus we need an ablility to connect the DSI bridge to another bridge
provided by the platform driver, so we extend the dw_mipi_dsi bind()
API with a new "previous bridge" arg instead of just hardcoding NULL.
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
New in v8.
---
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 6 ++++--
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 2 +-
include/drm/bridge/dw_mipi_dsi.h | 5 ++++-
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index 16fd87055e7b7..140ff40fa1b62 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -1456,11 +1456,13 @@ EXPORT_SYMBOL_GPL(dw_mipi_dsi_remove);
/*
* Bind/unbind API, used from platforms based on the component framework.
*/
-int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi, struct drm_encoder *encoder)
+int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi,
+ struct drm_encoder *encoder,
+ struct drm_bridge *prev_bridge)
{
int ret;
- ret = drm_bridge_attach(encoder, &dsi->bridge, NULL, 0);
+ ret = drm_bridge_attach(encoder, &dsi->bridge, prev_bridge, 0);
if (ret) {
DRM_ERROR("Failed to initialize bridge with drm\n");
return ret;
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
index 3feff0c45b3f7..83ef43be78135 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
@@ -929,7 +929,7 @@ static int dw_mipi_dsi_rockchip_bind(struct device *dev,
return ret;
}
- ret = dw_mipi_dsi_bind(dsi->dmd, &dsi->encoder);
+ ret = dw_mipi_dsi_bind(dsi->dmd, &dsi->encoder, NULL);
if (ret) {
DRM_DEV_ERROR(dev, "Failed to bind: %d\n", ret);
return ret;
diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h
index b0e390b3288e8..699b3531f5b36 100644
--- a/include/drm/bridge/dw_mipi_dsi.h
+++ b/include/drm/bridge/dw_mipi_dsi.h
@@ -14,6 +14,7 @@
#include <drm/drm_modes.h>
struct drm_display_mode;
+struct drm_bridge;
struct drm_encoder;
struct dw_mipi_dsi;
struct mipi_dsi_device;
@@ -62,7 +63,9 @@ struct dw_mipi_dsi *dw_mipi_dsi_probe(struct platform_device *pdev,
const struct dw_mipi_dsi_plat_data
*plat_data);
void dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi);
-int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi, struct drm_encoder *encoder);
+int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi,
+ struct drm_encoder *encoder,
+ struct drm_bridge *prev_bridge);
void dw_mipi_dsi_unbind(struct dw_mipi_dsi *dsi);
void dw_mipi_dsi_set_slave(struct dw_mipi_dsi *dsi, struct dw_mipi_dsi *slave);
--
2.26.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v8 04/10] drm: bridge: dw_mipi_dsi: allow bridge daisy chaining
2020-04-27 8:19 ` [PATCH v8 04/10] drm: bridge: dw_mipi_dsi: allow bridge daisy chaining Adrian Ratiu
@ 2020-06-02 23:51 ` Laurent Pinchart
2020-06-03 12:03 ` Adrian Ratiu
0 siblings, 1 reply; 21+ messages in thread
From: Laurent Pinchart @ 2020-06-02 23:51 UTC (permalink / raw)
To: Adrian Ratiu
Cc: devicetree, Jernej Skrabec, Jonas Karlman, linux-kernel,
dri-devel, Andrzej Hajda, linux-imx, linux-rockchip, kernel,
linux-stm32, linux-arm-kernel
Hi Adrian,
Thank you for the patch.
On Mon, Apr 27, 2020 at 11:19:46AM +0300, Adrian Ratiu wrote:
> Up until now the assumption was that the synopsis dsi bridge will
> directly connect to an encoder provided by the platform driver, but
> the current practice for drivers is to leave the encoder empty via
> the simple encoder API and add their logic to their own drm_bridge.
>
> Thus we need an ablility to connect the DSI bridge to another bridge
> provided by the platform driver, so we extend the dw_mipi_dsi bind()
> API with a new "previous bridge" arg instead of just hardcoding NULL.
>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> ---
> New in v8.
> ---
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 6 ++++--
> drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 2 +-
> include/drm/bridge/dw_mipi_dsi.h | 5 ++++-
> 3 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 16fd87055e7b7..140ff40fa1b62 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -1456,11 +1456,13 @@ EXPORT_SYMBOL_GPL(dw_mipi_dsi_remove);
> /*
> * Bind/unbind API, used from platforms based on the component framework.
> */
> -int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi, struct drm_encoder *encoder)
> +int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi,
> + struct drm_encoder *encoder,
> + struct drm_bridge *prev_bridge)
> {
> int ret;
>
> - ret = drm_bridge_attach(encoder, &dsi->bridge, NULL, 0);
> + ret = drm_bridge_attach(encoder, &dsi->bridge, prev_bridge, 0);
Please note that chaining of bridges doesn't work well if multiple
bridges in the chain try to create a connector. This is why a
DRM_BRIDGE_ATTACH_NO_CONNECTOR flag has been added, with a helper to
create a connector for a chain of bridges (drm_bridge_connector_init()).
This won't play well with the component framework. I would recommend
using the of_drm_find_bridge() instead in the rockchip driver, and
deprecating dw_mipi_dsi_bind().
> if (ret) {
> DRM_ERROR("Failed to initialize bridge with drm\n");
> return ret;
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> index 3feff0c45b3f7..83ef43be78135 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> @@ -929,7 +929,7 @@ static int dw_mipi_dsi_rockchip_bind(struct device *dev,
> return ret;
> }
>
> - ret = dw_mipi_dsi_bind(dsi->dmd, &dsi->encoder);
> + ret = dw_mipi_dsi_bind(dsi->dmd, &dsi->encoder, NULL);
> if (ret) {
> DRM_DEV_ERROR(dev, "Failed to bind: %d\n", ret);
> return ret;
> diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h
> index b0e390b3288e8..699b3531f5b36 100644
> --- a/include/drm/bridge/dw_mipi_dsi.h
> +++ b/include/drm/bridge/dw_mipi_dsi.h
> @@ -14,6 +14,7 @@
> #include <drm/drm_modes.h>
>
> struct drm_display_mode;
> +struct drm_bridge;
> struct drm_encoder;
> struct dw_mipi_dsi;
> struct mipi_dsi_device;
> @@ -62,7 +63,9 @@ struct dw_mipi_dsi *dw_mipi_dsi_probe(struct platform_device *pdev,
> const struct dw_mipi_dsi_plat_data
> *plat_data);
> void dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi);
> -int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi, struct drm_encoder *encoder);
> +int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi,
> + struct drm_encoder *encoder,
> + struct drm_bridge *prev_bridge);
> void dw_mipi_dsi_unbind(struct dw_mipi_dsi *dsi);
> void dw_mipi_dsi_set_slave(struct dw_mipi_dsi *dsi, struct dw_mipi_dsi *slave);
>
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v8 04/10] drm: bridge: dw_mipi_dsi: allow bridge daisy chaining
2020-06-02 23:51 ` Laurent Pinchart
@ 2020-06-03 12:03 ` Adrian Ratiu
0 siblings, 0 replies; 21+ messages in thread
From: Adrian Ratiu @ 2020-06-03 12:03 UTC (permalink / raw)
To: Laurent Pinchart
Cc: devicetree, Jernej Skrabec, Jonas Karlman, linux-kernel,
dri-devel, Andrzej Hajda, linux-imx, linux-rockchip, kernel,
linux-stm32, linux-arm-kernel
On Wed, 03 Jun 2020, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Adrian,
Hi Laurent,
>
> Thank you for the patch.
>
> On Mon, Apr 27, 2020 at 11:19:46AM +0300, Adrian Ratiu wrote:
>> Up until now the assumption was that the synopsis dsi bridge
>> will directly connect to an encoder provided by the platform
>> driver, but the current practice for drivers is to leave the
>> encoder empty via the simple encoder API and add their logic to
>> their own drm_bridge. Thus we need an ablility to connect the
>> DSI bridge to another bridge provided by the platform driver,
>> so we extend the dw_mipi_dsi bind() API with a new "previous
>> bridge" arg instead of just hardcoding NULL. Cc: Laurent
>> Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by:
>> Adrian Ratiu <adrian.ratiu@collabora.com> --- New in v8. ---
>> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 6 ++++--
>> drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 2 +-
>> include/drm/bridge/dw_mipi_dsi.h | 5 ++++- 3
>> files changed, 9 insertions(+), 4 deletions(-)
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c index
>> 16fd87055e7b7..140ff40fa1b62 100644 ---
>> a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c +++
>> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c @@ -1456,11
>> +1456,13 @@ EXPORT_SYMBOL_GPL(dw_mipi_dsi_remove);
>> /*
>> * Bind/unbind API, used from platforms based on the component
>> framework. */
>> -int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi, struct
>> drm_encoder *encoder) +int dw_mipi_dsi_bind(struct dw_mipi_dsi
>> *dsi, + struct drm_encoder *encoder, +
>> struct drm_bridge *prev_bridge)
>> { int ret;
>> - ret = drm_bridge_attach(encoder, &dsi->bridge, NULL, 0); +
>> ret = drm_bridge_attach(encoder, &dsi->bridge, prev_bridge, 0);
>
> Please note that chaining of bridges doesn't work well if
> multiple bridges in the chain try to create a connector. This is
> why a DRM_BRIDGE_ATTACH_NO_CONNECTOR flag has been added, with a
> helper to create a connector for a chain of bridges
> (drm_bridge_connector_init()). This won't play well with the
> component framework. I would recommend using the
> of_drm_find_bridge() instead in the rockchip driver, and
> deprecating dw_mipi_dsi_bind().
>
Thank you for this insight, indeed the bridge dw_mipi_dsi_bind()
is clunky and we're making it even more so by possibly
re-inventing drm_bridge_connector_init() with it in a way which
can't work (well it does work but can lead to those nasty
multiple-encoder corner-cases you mention).
I'll address this before posting v9, to try to move to
of_drm_find_bridge() and remove dw_mipi_dsi_bind().
>> if (ret) {
>> DRM_ERROR("Failed to initialize bridge with drm\n");
>> return ret;
>> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
>> index 3feff0c45b3f7..83ef43be78135 100644
>> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
>> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
>> @@ -929,7 +929,7 @@ static int dw_mipi_dsi_rockchip_bind(struct device *dev,
>> return ret;
>> }
>>
>> - ret = dw_mipi_dsi_bind(dsi->dmd, &dsi->encoder);
>> + ret = dw_mipi_dsi_bind(dsi->dmd, &dsi->encoder, NULL);
>> if (ret) {
>> DRM_DEV_ERROR(dev, "Failed to bind: %d\n", ret);
>> return ret;
>> diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h
>> index b0e390b3288e8..699b3531f5b36 100644
>> --- a/include/drm/bridge/dw_mipi_dsi.h
>> +++ b/include/drm/bridge/dw_mipi_dsi.h
>> @@ -14,6 +14,7 @@
>> #include <drm/drm_modes.h>
>>
>> struct drm_display_mode;
>> +struct drm_bridge;
>> struct drm_encoder;
>> struct dw_mipi_dsi;
>> struct mipi_dsi_device;
>> @@ -62,7 +63,9 @@ struct dw_mipi_dsi *dw_mipi_dsi_probe(struct platform_device *pdev,
>> const struct dw_mipi_dsi_plat_data
>> *plat_data);
>> void dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi);
>> -int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi, struct drm_encoder *encoder);
>> +int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi,
>> + struct drm_encoder *encoder,
>> + struct drm_bridge *prev_bridge);
>> void dw_mipi_dsi_unbind(struct dw_mipi_dsi *dsi);
>> void dw_mipi_dsi_set_slave(struct dw_mipi_dsi *dsi, struct dw_mipi_dsi *slave);
>>
>
> --
> Regards,
>
> Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v8 05/10] drm: imx: Add i.MX 6 MIPI DSI host platform driver
2020-04-27 8:19 [PATCH v8 00/10] Genericize DW MIPI DSI bridge and add i.MX 6 driver Adrian Ratiu
` (3 preceding siblings ...)
2020-04-27 8:19 ` [PATCH v8 04/10] drm: bridge: dw_mipi_dsi: allow bridge daisy chaining Adrian Ratiu
@ 2020-04-27 8:19 ` Adrian Ratiu
2020-04-27 14:41 ` Enric Balletbo i Serra
2020-04-27 8:19 ` [PATCH v8 06/10] ARM: dts: imx6qdl: add missing mipi dsi properties Adrian Ratiu
` (4 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Adrian Ratiu @ 2020-04-27 8:19 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, linux-rockchip, Laurent Pinchart
Cc: Jernej Skrabec, Adrian Pop, Jonas Karlman, Martyn Welch,
Sjoerd Simons, linux-kernel, dri-devel, Andrzej Hajda, linux-imx,
kernel, linux-stm32, Arnaud Ferraris, Emil Velikov
This adds support for the Synopsis DesignWare MIPI DSI v1.01 host
controller which is embedded in i.MX 6 SoCs.
Based on following patches, but updated/extended to work with existing
support found in the kernel:
- drm: imx: Support Synopsys DesignWare MIPI DSI host controller
Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Enric Balletbo Serra <eballetbo@gmail.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Tested-by: Adrian Pop <pop.adrian61@gmail.com>
Tested-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.com>
Signed-off-by: Martyn Welch <martyn.welch@collabora.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
Changes since v7:
- Removed encoder helper ops and added drm_bridge (Laurent)
- Brought back drm_simple_encoder_init and dropped dependency on
external unify encoder creation patch (Laurent)
- Minor typo fixes
Changes since v6:
- Replaced custom noop encoder with the simple drm encoder (Enric)
- Added CONFIG_DRM_IMX6_MIPI_DSI depends on CONFIG_OF (Enric)
- Dropped imx_mipi_dsi_register() because now it only creates the
dummy encoder which can easily be done directly in imx_dsi_bind()
Changes since v5:
- Reword to remove unrelated device tree patch mention (Fabio)
- Move pllref_clk enable/disable to bind/unbind (Ezequiel)
- Fix freescale.com -> nxp.com email addresses (Fabio)
- Also added myself as module author (Fabio)
- Use DRM_DEV_* macros for consistency, print more error msg
Changes since v4:
- Split off driver-specific configuration of phy timings due
to new upstream API.
- Move regmap infrastructure logic to separate commit (Ezequiel)
- Move dsi v1.01 layout addition to a separate commit (Ezequiel)
- Minor warnings and driver name fixes
Changes since v3:
- Renamed platform driver to reflect it's i.MX6 only. (Fabio)
Changes since v2:
- 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/imx/Kconfig | 8 +
drivers/gpu/drm/imx/Makefile | 1 +
drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c | 399 +++++++++++++++++++++++++
3 files changed, 408 insertions(+)
create mode 100644 drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c
diff --git a/drivers/gpu/drm/imx/Kconfig b/drivers/gpu/drm/imx/Kconfig
index 207bf7409dfba..0dffc72df7922 100644
--- a/drivers/gpu/drm/imx/Kconfig
+++ b/drivers/gpu/drm/imx/Kconfig
@@ -39,3 +39,11 @@ config DRM_IMX_HDMI
depends on DRM_IMX
help
Choose this if you want to use HDMI on i.MX6.
+
+config DRM_IMX6_MIPI_DSI
+ tristate "Freescale i.MX6 DRM MIPI DSI"
+ select DRM_DW_MIPI_DSI
+ depends on DRM_IMX
+ depends on OF
+ help
+ Choose this if you want to use MIPI DSI on i.MX6.
diff --git a/drivers/gpu/drm/imx/Makefile b/drivers/gpu/drm/imx/Makefile
index 21cdcc2faabc8..9a7843c593478 100644
--- a/drivers/gpu/drm/imx/Makefile
+++ b/drivers/gpu/drm/imx/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_DRM_IMX_TVE) += imx-tve.o
obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o
obj-$(CONFIG_DRM_IMX_HDMI) += dw_hdmi-imx.o
+obj-$(CONFIG_DRM_IMX6_MIPI_DSI) += dw_mipi_dsi-imx6.o
diff --git a/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c b/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c
new file mode 100644
index 0000000000000..492decc418bc3
--- /dev/null
+++ b/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c
@@ -0,0 +1,399 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * i.MX6 drm driver - MIPI DSI Host Controller
+ *
+ * Copyright (C) 2011-2015 Freescale Semiconductor, Inc.
+ * Copyright (C) 2019-2020 Collabora, Ltd.
+ */
+
+#include <linux/clk.h>
+#include <linux/component.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+#include <linux/videodev2.h>
+#include <drm/bridge/dw_mipi_dsi.h>
+#include <drm/drm_atomic_state_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_of.h>
+#include <drm/drm_print.h>
+#include <drm/drm_simple_kms_helper.h>
+
+#include "imx-drm.h"
+
+#define DSI_PWR_UP 0x04
+#define RESET 0
+#define POWERUP BIT(0)
+
+#define DSI_PHY_IF_CTRL 0x5c
+#define PHY_IF_CTRL_RESET 0x0
+
+#define DSI_PHY_TST_CTRL0 0x64
+#define PHY_TESTCLK BIT(1)
+#define PHY_UNTESTCLK 0
+#define PHY_TESTCLR BIT(0)
+#define PHY_UNTESTCLR 0
+
+#define DSI_PHY_TST_CTRL1 0x68
+#define PHY_TESTEN BIT(16)
+#define PHY_UNTESTEN 0
+#define PHY_TESTDOUT(n) (((n) & 0xff) << 8)
+#define PHY_TESTDIN(n) (((n) & 0xff) << 0)
+
+struct imx_mipi_dsi {
+ struct drm_encoder encoder;
+ struct drm_bridge bridge;
+ struct device *dev;
+ struct regmap *mux_sel;
+ struct dw_mipi_dsi *mipi_dsi;
+ struct clk *pllref_clk;
+
+ void __iomem *base;
+ unsigned int lane_mbps;
+};
+
+struct dphy_pll_testdin_map {
+ unsigned int max_mbps;
+ u8 testdin;
+};
+
+/* The table is based on 27MHz DPHY pll reference clock. */
+static const struct dphy_pll_testdin_map dptdin_map[] = {
+ {160, 0x04}, {180, 0x24}, {200, 0x44}, {210, 0x06},
+ {240, 0x26}, {250, 0x46}, {270, 0x08}, {300, 0x28},
+ {330, 0x48}, {360, 0x2a}, {400, 0x4a}, {450, 0x0c},
+ {500, 0x2c}, {550, 0x0e}, {600, 0x2e}, {650, 0x10},
+ {700, 0x30}, {750, 0x12}, {800, 0x32}, {850, 0x14},
+ {900, 0x34}, {950, 0x54}, {1000, 0x74}
+};
+
+static inline struct imx_mipi_dsi *bridge_to_imxdsi(struct drm_bridge *br)
+{
+ return container_of(br, struct imx_mipi_dsi, bridge);
+}
+
+static int
+imx_mipi_dsi_bridge_atomic_check(struct drm_bridge *bridge,
+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state)
+{
+ struct imx_crtc_state *imx_crtc_state = to_imx_crtc_state(crtc_state);
+
+ /* The following values are taken from dw_hdmi_imx_atomic_check */
+ imx_crtc_state->bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+ imx_crtc_state->di_hsync_pin = 2;
+ imx_crtc_state->di_vsync_pin = 3;
+
+ return 0;
+}
+
+static void imx_mipi_dsi_bridge_atomic_enable(struct drm_bridge *bridge,
+ struct drm_bridge_state *old_st)
+{
+ struct imx_mipi_dsi *dsi = bridge_to_imxdsi(bridge);
+ int mux = drm_of_encoder_active_port_id(dsi->dev->of_node,
+ &dsi->encoder);
+ /* Set IOMUX DSI output reg to correct IPU 1 or 0 and DI 1 or 0 ports */
+ regmap_update_bits(dsi->mux_sel, IOMUXC_GPR3,
+ IMX6Q_GPR3_MIPI_MUX_CTL_MASK,
+ mux << IMX6Q_GPR3_MIPI_MUX_CTL_SHIFT);
+}
+
+static const struct drm_bridge_funcs imx_dsi_bridge_funcs = {
+ .atomic_enable = imx_mipi_dsi_bridge_atomic_enable,
+ .atomic_check = imx_mipi_dsi_bridge_atomic_check,
+ .atomic_reset = drm_atomic_helper_bridge_reset,
+ .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+};
+
+static enum drm_mode_status imx_mipi_dsi_mode_valid(void *priv_data,
+ const struct drm_display_mode *mode)
+{
+ /*
+ * The VID_PKT_SIZE field in the DSI_VID_PKT_CFG
+ * register is 11-bit.
+ */
+ if (mode->hdisplay > 0x7ff)
+ return MODE_BAD_HVALUE;
+
+ /*
+ * The V_ACTIVE_LINES field in the DSI_VTIMING_CFG
+ * register is 11-bit.
+ */
+ if (mode->vdisplay > 0x7ff)
+ return MODE_BAD_VVALUE;
+
+ return MODE_OK;
+}
+
+
+static unsigned int max_mbps_to_testdin(unsigned int max_mbps)
+{
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(dptdin_map); i++)
+ if (dptdin_map[i].max_mbps == max_mbps)
+ return dptdin_map[i].testdin;
+
+ return -EINVAL;
+}
+
+static inline void dsi_write(struct imx_mipi_dsi *dsi, u32 reg, u32 val)
+{
+ writel(val, dsi->base + reg);
+}
+
+static int imx_mipi_dsi_phy_init(void *priv_data)
+{
+ struct imx_mipi_dsi *dsi = priv_data;
+ int testdin;
+
+ testdin = max_mbps_to_testdin(dsi->lane_mbps);
+ if (testdin < 0) {
+ DRM_DEV_ERROR(dsi->dev,
+ "failed to get testdin for %dmbps lane clock\n",
+ dsi->lane_mbps);
+ return testdin;
+ }
+
+ dsi_write(dsi, DSI_PHY_IF_CTRL, PHY_IF_CTRL_RESET);
+ dsi_write(dsi, DSI_PWR_UP, POWERUP);
+
+ dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR);
+ dsi_write(dsi, DSI_PHY_TST_CTRL1, PHY_TESTEN | PHY_TESTDOUT(0) |
+ PHY_TESTDIN(0x44));
+ dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR);
+ dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR);
+ dsi_write(dsi, DSI_PHY_TST_CTRL1, PHY_UNTESTEN | PHY_TESTDOUT(0) |
+ PHY_TESTDIN(testdin));
+ dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR);
+ dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR);
+
+ return 0;
+}
+
+static int imx_mipi_dsi_get_lane_mbps(void *priv_data,
+ const struct drm_display_mode *mode,
+ unsigned long mode_flags, u32 lanes,
+ u32 format, unsigned int *lane_mbps)
+{
+ struct imx_mipi_dsi *dsi = priv_data;
+ int bpp;
+ unsigned int i, target_mbps, mpclk;
+ unsigned long pllref;
+
+ bpp = mipi_dsi_pixel_format_to_bpp(format);
+ if (bpp < 0) {
+ DRM_DEV_ERROR(dsi->dev, "failed to get bpp for format %d: %d\n",
+ format, bpp);
+ return bpp;
+ }
+
+ pllref = clk_get_rate(dsi->pllref_clk);
+ if (pllref != 27000000)
+ DRM_WARN("DSI pllref_clk not set to 27Mhz\n");
+
+ mpclk = DIV_ROUND_UP(mode->clock, MSEC_PER_SEC);
+ if (mpclk) {
+ /* take 1/0.7 blanking overhead into consideration */
+ target_mbps = (mpclk * (bpp / lanes) * 10) / 7;
+ } else {
+ DRM_DEV_ERROR(dsi->dev, "use default 1Gbps DPHY pll clock\n");
+ target_mbps = 1000;
+ }
+
+ DRM_DEV_DEBUG(dsi->dev, "target pllref_clk frequency is %uMbps\n",
+ target_mbps);
+
+ for (i = 0; i < ARRAY_SIZE(dptdin_map); i++) {
+ if (target_mbps < dptdin_map[i].max_mbps) {
+ *lane_mbps = dptdin_map[i].max_mbps;
+ dsi->lane_mbps = *lane_mbps;
+ DRM_DEV_DEBUG(dsi->dev,
+ "real pllref_clk frequency is %uMbps\n",
+ *lane_mbps);
+ return 0;
+ }
+ }
+
+ DRM_DEV_ERROR(dsi->dev, "DPHY clock frequency %uMbps is out of range\n",
+ target_mbps);
+
+ return -EINVAL;
+}
+
+static int
+dw_mipi_dsi_phy_get_timing(void *priv_data, unsigned int lane_mbps,
+ struct dw_mipi_dsi_dphy_timing *timing)
+{
+ timing->clk_hs2lp = 0x40;
+ timing->clk_lp2hs = 0x40;
+ timing->data_hs2lp = 0x40;
+ timing->data_lp2hs = 0x40;
+
+ return 0;
+}
+
+static const struct dw_mipi_dsi_phy_ops dw_mipi_dsi_imx6_phy_ops = {
+ .init = imx_mipi_dsi_phy_init,
+ .get_lane_mbps = imx_mipi_dsi_get_lane_mbps,
+ .get_timing = dw_mipi_dsi_phy_get_timing,
+};
+
+static struct dw_mipi_dsi_plat_data imx6_mipi_dsi_drv_data = {
+ .max_data_lanes = 2,
+ .mode_valid = imx_mipi_dsi_mode_valid,
+ .phy_ops = &dw_mipi_dsi_imx6_phy_ops,
+};
+
+static const struct of_device_id imx_dsi_dt_ids[] = {
+ {
+ .compatible = "fsl,imx6-mipi-dsi",
+ .data = &imx6_mipi_dsi_drv_data,
+ },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imx_dsi_dt_ids);
+
+static int imx_mipi_dsi_bind(struct device *dev, struct device *master,
+ void *data)
+{
+ struct imx_mipi_dsi *dsi = dev_get_drvdata(dev);
+ struct drm_device *drm = data;
+ int ret;
+
+ ret = clk_prepare_enable(dsi->pllref_clk);
+ if (ret) {
+ DRM_DEV_ERROR(dev, "Failed to enable pllref_clk: %d\n", ret);
+ return ret;
+ }
+
+ ret = imx_drm_encoder_parse_of(drm, &dsi->encoder, dsi->dev->of_node);
+ if (ret)
+ return ret;
+
+ ret = drm_simple_encoder_init(drm, &dsi->encoder, DRM_MODE_ENCODER_NONE);
+ if (ret) {
+ DRM_DEV_ERROR(dev, "failed to create drm encoder\n");
+ return ret;
+ }
+
+ drm_bridge_add(&dsi->bridge);
+
+ drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL, 0);
+ if (ret < 0) {
+ DRM_DEV_ERROR(dev, "failed to attach IMX6 bridge: %d\n", ret);
+ return ret;
+ }
+
+ ret = dw_mipi_dsi_bind(dsi->mipi_dsi, &dsi->encoder, &dsi->bridge);
+ if (ret) {
+ DRM_DEV_ERROR(dev, "Failed to bind: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static void imx_mipi_dsi_unbind(struct device *dev, struct device *master,
+ void *data)
+{
+ struct imx_mipi_dsi *dsi = dev_get_drvdata(dev);
+
+ dw_mipi_dsi_unbind(dsi->mipi_dsi);
+
+ clk_disable_unprepare(dsi->pllref_clk);
+}
+
+static const struct component_ops imx_mipi_dsi_ops = {
+ .bind = imx_mipi_dsi_bind,
+ .unbind = imx_mipi_dsi_unbind,
+};
+
+static int imx_mipi_dsi_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ const struct of_device_id *of_id = of_match_device(imx_dsi_dt_ids, dev);
+ struct dw_mipi_dsi_plat_data *pdata = (struct dw_mipi_dsi_plat_data *) of_id->data;
+ struct imx_mipi_dsi *dsi;
+ struct resource *res;
+ int ret;
+
+ dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
+ if (!dsi)
+ return -ENOMEM;
+
+ dsi->dev = dev;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ dsi->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(dsi->base)) {
+ ret = PTR_ERR(dsi->base);
+ DRM_DEV_ERROR(dev, "Unable to get dsi registers: %d\n", ret);
+ return ret;
+ }
+
+ dsi->pllref_clk = devm_clk_get(dev, "ref");
+ if (IS_ERR(dsi->pllref_clk)) {
+ ret = PTR_ERR(dsi->pllref_clk);
+ DRM_DEV_ERROR(dev, "Unable to get pllref_clk: %d\n", ret);
+ return ret;
+ }
+
+ dsi->mux_sel = syscon_regmap_lookup_by_phandle(dev->of_node, "fsl,gpr");
+ if (IS_ERR(dsi->mux_sel)) {
+ ret = PTR_ERR(dsi->mux_sel);
+ DRM_DEV_ERROR(dev, "Failed to get GPR regmap: %d\n", ret);
+ return ret;
+ }
+
+ dev_set_drvdata(dev, dsi);
+
+ imx6_mipi_dsi_drv_data.base = dsi->base;
+ imx6_mipi_dsi_drv_data.priv_data = dsi;
+
+ dsi->bridge.driver_private = dsi;
+ dsi->bridge.funcs = &imx_dsi_bridge_funcs;
+ dsi->bridge.of_node = dev->of_node;
+
+ dsi->mipi_dsi = dw_mipi_dsi_probe(pdev, pdata);
+ if (IS_ERR(dsi->mipi_dsi)) {
+ ret = PTR_ERR(dsi->mipi_dsi);
+ DRM_DEV_ERROR(dev, "Failed to probe DW DSI host: %d\n", ret);
+ goto err_clkdisable;
+ }
+
+ return component_add(&pdev->dev, &imx_mipi_dsi_ops);
+
+err_clkdisable:
+ clk_disable_unprepare(dsi->pllref_clk);
+ return ret;
+}
+
+static int imx_mipi_dsi_remove(struct platform_device *pdev)
+{
+ struct imx_mipi_dsi *dsi = platform_get_drvdata(pdev);
+ drm_bridge_remove(&dsi->bridge);
+ component_del(&pdev->dev, &imx_mipi_dsi_ops);
+ return 0;
+}
+
+static struct platform_driver imx_mipi_dsi_driver = {
+ .probe = imx_mipi_dsi_probe,
+ .remove = imx_mipi_dsi_remove,
+ .driver = {
+ .of_match_table = imx_dsi_dt_ids,
+ .name = "dw-mipi-dsi-imx6",
+ },
+};
+module_platform_driver(imx_mipi_dsi_driver);
+
+MODULE_DESCRIPTION("i.MX6 MIPI DSI host controller driver");
+MODULE_AUTHOR("Liu Ying <victor.liu@nxp.com>");
+MODULE_AUTHOR("Adrian Ratiu <adrian.ratiu@collabora.com>");
+MODULE_LICENSE("GPL");
--
2.26.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v8 05/10] drm: imx: Add i.MX 6 MIPI DSI host platform driver
2020-04-27 8:19 ` [PATCH v8 05/10] drm: imx: Add i.MX 6 MIPI DSI host platform driver Adrian Ratiu
@ 2020-04-27 14:41 ` Enric Balletbo i Serra
2020-04-27 18:26 ` Adrian Ratiu
0 siblings, 1 reply; 21+ messages in thread
From: Enric Balletbo i Serra @ 2020-04-27 14:41 UTC (permalink / raw)
To: Adrian Ratiu, linux-arm-kernel, devicetree, linux-rockchip,
Laurent Pinchart
Cc: Jernej Skrabec, Adrian Pop, Jonas Karlman, Martyn Welch,
Sjoerd Simons, linux-kernel, dri-devel, Andrzej Hajda, linux-imx,
kernel, linux-stm32, Arnaud Ferraris, Emil Velikov
Hi Adrian
Thank you for your patch.
On 27/4/20 10:19, Adrian Ratiu wrote:
> This adds support for the Synopsis DesignWare MIPI DSI v1.01 host
> controller which is embedded in i.MX 6 SoCs.
>
> Based on following patches, but updated/extended to work with existing
> support found in the kernel:
>
> - drm: imx: Support Synopsys DesignWare MIPI DSI host controller
> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Enric Balletbo Serra <eballetbo@gmail.com>
> Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
> Tested-by: Adrian Pop <pop.adrian61@gmail.com>
> Tested-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.com>
> Signed-off-by: Martyn Welch <martyn.welch@collabora.com>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> ---
> Changes since v7:
> - Removed encoder helper ops and added drm_bridge (Laurent)
> - Brought back drm_simple_encoder_init and dropped dependency on
> external unify encoder creation patch (Laurent)
> - Minor typo fixes
>
> Changes since v6:
> - Replaced custom noop encoder with the simple drm encoder (Enric)
> - Added CONFIG_DRM_IMX6_MIPI_DSI depends on CONFIG_OF (Enric)
> - Dropped imx_mipi_dsi_register() because now it only creates the
> dummy encoder which can easily be done directly in imx_dsi_bind()
>
> Changes since v5:
> - Reword to remove unrelated device tree patch mention (Fabio)
> - Move pllref_clk enable/disable to bind/unbind (Ezequiel)
> - Fix freescale.com -> nxp.com email addresses (Fabio)
> - Also added myself as module author (Fabio)
> - Use DRM_DEV_* macros for consistency, print more error msg
>
> Changes since v4:
> - Split off driver-specific configuration of phy timings due
> to new upstream API.
> - Move regmap infrastructure logic to separate commit (Ezequiel)
> - Move dsi v1.01 layout addition to a separate commit (Ezequiel)
> - Minor warnings and driver name fixes
>
> Changes since v3:
> - Renamed platform driver to reflect it's i.MX6 only. (Fabio)
>
> Changes since v2:
> - 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/imx/Kconfig | 8 +
> drivers/gpu/drm/imx/Makefile | 1 +
> drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c | 399 +++++++++++++++++++++++++
> 3 files changed, 408 insertions(+)
> create mode 100644 drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c
>
> diff --git a/drivers/gpu/drm/imx/Kconfig b/drivers/gpu/drm/imx/Kconfig
> index 207bf7409dfba..0dffc72df7922 100644
> --- a/drivers/gpu/drm/imx/Kconfig
> +++ b/drivers/gpu/drm/imx/Kconfig
> @@ -39,3 +39,11 @@ config DRM_IMX_HDMI
> depends on DRM_IMX
> help
> Choose this if you want to use HDMI on i.MX6.
> +
> +config DRM_IMX6_MIPI_DSI
> + tristate "Freescale i.MX6 DRM MIPI DSI"
> + select DRM_DW_MIPI_DSI
> + depends on DRM_IMX
> + depends on OF
> + help
> + Choose this if you want to use MIPI DSI on i.MX6.
> diff --git a/drivers/gpu/drm/imx/Makefile b/drivers/gpu/drm/imx/Makefile
> index 21cdcc2faabc8..9a7843c593478 100644
> --- a/drivers/gpu/drm/imx/Makefile
> +++ b/drivers/gpu/drm/imx/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_DRM_IMX_TVE) += imx-tve.o
> obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o
>
> obj-$(CONFIG_DRM_IMX_HDMI) += dw_hdmi-imx.o
> +obj-$(CONFIG_DRM_IMX6_MIPI_DSI) += dw_mipi_dsi-imx6.o
> diff --git a/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c b/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c
> new file mode 100644
> index 0000000000000..492decc418bc3
> --- /dev/null
> +++ b/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c
> @@ -0,0 +1,399 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * i.MX6 drm driver - MIPI DSI Host Controller
> + *
> + * Copyright (C) 2011-2015 Freescale Semiconductor, Inc.
> + * Copyright (C) 2019-2020 Collabora, Ltd.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/component.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
> +#include <linux/videodev2.h>
> +#include <drm/bridge/dw_mipi_dsi.h>
> +#include <drm/drm_atomic_state_helper.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_of.h>
> +#include <drm/drm_print.h>
> +#include <drm/drm_simple_kms_helper.h>
> +
> +#include "imx-drm.h"
> +
> +#define DSI_PWR_UP 0x04
> +#define RESET 0
> +#define POWERUP BIT(0)
> +
> +#define DSI_PHY_IF_CTRL 0x5c
> +#define PHY_IF_CTRL_RESET 0x0
> +
> +#define DSI_PHY_TST_CTRL0 0x64
> +#define PHY_TESTCLK BIT(1)
> +#define PHY_UNTESTCLK 0
> +#define PHY_TESTCLR BIT(0)
> +#define PHY_UNTESTCLR 0
> +
> +#define DSI_PHY_TST_CTRL1 0x68
> +#define PHY_TESTEN BIT(16)
> +#define PHY_UNTESTEN 0
> +#define PHY_TESTDOUT(n) (((n) & 0xff) << 8)
> +#define PHY_TESTDIN(n) (((n) & 0xff) << 0)
> +
> +struct imx_mipi_dsi {
> + struct drm_encoder encoder;
> + struct drm_bridge bridge;
> + struct device *dev;
> + struct regmap *mux_sel;
> + struct dw_mipi_dsi *mipi_dsi;
> + struct clk *pllref_clk;
> +
> + void __iomem *base;
> + unsigned int lane_mbps;
> +};
> +
> +struct dphy_pll_testdin_map {
> + unsigned int max_mbps;
> + u8 testdin;
> +};
> +
> +/* The table is based on 27MHz DPHY pll reference clock. */
> +static const struct dphy_pll_testdin_map dptdin_map[] = {
> + {160, 0x04}, {180, 0x24}, {200, 0x44}, {210, 0x06},
> + {240, 0x26}, {250, 0x46}, {270, 0x08}, {300, 0x28},
> + {330, 0x48}, {360, 0x2a}, {400, 0x4a}, {450, 0x0c},
> + {500, 0x2c}, {550, 0x0e}, {600, 0x2e}, {650, 0x10},
> + {700, 0x30}, {750, 0x12}, {800, 0x32}, {850, 0x14},
> + {900, 0x34}, {950, 0x54}, {1000, 0x74}
> +};
> +
> +static inline struct imx_mipi_dsi *bridge_to_imxdsi(struct drm_bridge *br)
> +{
> + return container_of(br, struct imx_mipi_dsi, bridge);
> +}
> +
> +static int
> +imx_mipi_dsi_bridge_atomic_check(struct drm_bridge *bridge,
> + struct drm_bridge_state *bridge_state,
> + struct drm_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state)
> +{
> + struct imx_crtc_state *imx_crtc_state = to_imx_crtc_state(crtc_state);
> +
> + /* The following values are taken from dw_hdmi_imx_atomic_check */
> + imx_crtc_state->bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> + imx_crtc_state->di_hsync_pin = 2;
> + imx_crtc_state->di_vsync_pin = 3;
> +
> + return 0;
> +}
> +
> +static void imx_mipi_dsi_bridge_atomic_enable(struct drm_bridge *bridge,
> + struct drm_bridge_state *old_st)
> +{
> + struct imx_mipi_dsi *dsi = bridge_to_imxdsi(bridge);
> + int mux = drm_of_encoder_active_port_id(dsi->dev->of_node,
> + &dsi->encoder);
> + /* Set IOMUX DSI output reg to correct IPU 1 or 0 and DI 1 or 0 ports */
> + regmap_update_bits(dsi->mux_sel, IOMUXC_GPR3,
> + IMX6Q_GPR3_MIPI_MUX_CTL_MASK,
> + mux << IMX6Q_GPR3_MIPI_MUX_CTL_SHIFT);
> +}
> +
> +static const struct drm_bridge_funcs imx_dsi_bridge_funcs = {
> + .atomic_enable = imx_mipi_dsi_bridge_atomic_enable,
> + .atomic_check = imx_mipi_dsi_bridge_atomic_check,
> + .atomic_reset = drm_atomic_helper_bridge_reset,
> + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> +};
> +
> +static enum drm_mode_status imx_mipi_dsi_mode_valid(void *priv_data,
> + const struct drm_display_mode *mode)
> +{
> + /*
> + * The VID_PKT_SIZE field in the DSI_VID_PKT_CFG
> + * register is 11-bit.
> + */
> + if (mode->hdisplay > 0x7ff)
> + return MODE_BAD_HVALUE;
> +
> + /*
> + * The V_ACTIVE_LINES field in the DSI_VTIMING_CFG
> + * register is 11-bit.
> + */
> + if (mode->vdisplay > 0x7ff)
> + return MODE_BAD_VVALUE;
> +
> + return MODE_OK;
> +}
> +
> +
> +static unsigned int max_mbps_to_testdin(unsigned int max_mbps)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(dptdin_map); i++)
> + if (dptdin_map[i].max_mbps == max_mbps)
> + return dptdin_map[i].testdin;
> +
> + return -EINVAL;
> +}
> +
> +static inline void dsi_write(struct imx_mipi_dsi *dsi, u32 reg, u32 val)
> +{
> + writel(val, dsi->base + reg);
> +}
> +
> +static int imx_mipi_dsi_phy_init(void *priv_data)
> +{
> + struct imx_mipi_dsi *dsi = priv_data;
> + int testdin;
> +
> + testdin = max_mbps_to_testdin(dsi->lane_mbps);
> + if (testdin < 0) {
> + DRM_DEV_ERROR(dsi->dev,
> + "failed to get testdin for %dmbps lane clock\n",
> + dsi->lane_mbps);
> + return testdin;
> + }
> +
> + dsi_write(dsi, DSI_PHY_IF_CTRL, PHY_IF_CTRL_RESET);
> + dsi_write(dsi, DSI_PWR_UP, POWERUP);
> +
> + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR);
> + dsi_write(dsi, DSI_PHY_TST_CTRL1, PHY_TESTEN | PHY_TESTDOUT(0) |
> + PHY_TESTDIN(0x44));
> + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR);
> + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR);
> + dsi_write(dsi, DSI_PHY_TST_CTRL1, PHY_UNTESTEN | PHY_TESTDOUT(0) |
> + PHY_TESTDIN(testdin));
> + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR);
> + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR);
> +
> + return 0;
> +}
> +
> +static int imx_mipi_dsi_get_lane_mbps(void *priv_data,
> + const struct drm_display_mode *mode,
> + unsigned long mode_flags, u32 lanes,
> + u32 format, unsigned int *lane_mbps)
> +{
> + struct imx_mipi_dsi *dsi = priv_data;
> + int bpp;
> + unsigned int i, target_mbps, mpclk;
> + unsigned long pllref;
> +
> + bpp = mipi_dsi_pixel_format_to_bpp(format);
> + if (bpp < 0) {
> + DRM_DEV_ERROR(dsi->dev, "failed to get bpp for format %d: %d\n",
> + format, bpp);
> + return bpp;
> + }
> +
> + pllref = clk_get_rate(dsi->pllref_clk);
> + if (pllref != 27000000)
> + DRM_WARN("DSI pllref_clk not set to 27Mhz\n");
This pllref variable is only used to print this warning, what's the point of
warn and ignore it? If pllref is not set at 27MHz what will happen?
> +
> + mpclk = DIV_ROUND_UP(mode->clock, MSEC_PER_SEC);
> + if (mpclk) {
> + /* take 1/0.7 blanking overhead into consideration */
> + target_mbps = (mpclk * (bpp / lanes) * 10) / 7;
> + } else {
> + DRM_DEV_ERROR(dsi->dev, "use default 1Gbps DPHY pll clock\n");
That's really an error? Looks to me that's just a fallback if mpclk is zero.
Maybe set the level to INFO or even better DEBUG.
> + target_mbps = 1000;
> + }
> +
> + DRM_DEV_DEBUG(dsi->dev, "target pllref_clk frequency is %uMbps\n",
> + target_mbps);
> +
> + for (i = 0; i < ARRAY_SIZE(dptdin_map); i++) {
> + if (target_mbps < dptdin_map[i].max_mbps) {
> + *lane_mbps = dptdin_map[i].max_mbps;
> + dsi->lane_mbps = *lane_mbps;
> + DRM_DEV_DEBUG(dsi->dev,
> + "real pllref_clk frequency is %uMbps\n",
> + *lane_mbps);
> + return 0;
> + }
> + }
> +
> + DRM_DEV_ERROR(dsi->dev, "DPHY clock frequency %uMbps is out of range\n",
> + target_mbps);
> +
> + return -EINVAL;
> +}
> +
> +static int
> +dw_mipi_dsi_phy_get_timing(void *priv_data, unsigned int lane_mbps,
> + struct dw_mipi_dsi_dphy_timing *timing)
> +{
> + timing->clk_hs2lp = 0x40;
> + timing->clk_lp2hs = 0x40;
> + timing->data_hs2lp = 0x40;
> + timing->data_lp2hs = 0x40;
> +
> + return 0;
> +}
> +
> +static const struct dw_mipi_dsi_phy_ops dw_mipi_dsi_imx6_phy_ops = {
> + .init = imx_mipi_dsi_phy_init,
> + .get_lane_mbps = imx_mipi_dsi_get_lane_mbps,
> + .get_timing = dw_mipi_dsi_phy_get_timing,
> +};
> +
> +static struct dw_mipi_dsi_plat_data imx6_mipi_dsi_drv_data = {
> + .max_data_lanes = 2,
> + .mode_valid = imx_mipi_dsi_mode_valid,
> + .phy_ops = &dw_mipi_dsi_imx6_phy_ops,
> +};
> +
> +static const struct of_device_id imx_dsi_dt_ids[] = {
> + {
> + .compatible = "fsl,imx6-mipi-dsi",
> + .data = &imx6_mipi_dsi_drv_data,
> + },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imx_dsi_dt_ids);
> +
> +static int imx_mipi_dsi_bind(struct device *dev, struct device *master,
> + void *data)
> +{
> + struct imx_mipi_dsi *dsi = dev_get_drvdata(dev);
> + struct drm_device *drm = data;
> + int ret;
> +
> + ret = clk_prepare_enable(dsi->pllref_clk);
> + if (ret) {
> + DRM_DEV_ERROR(dev, "Failed to enable pllref_clk: %d\n", ret);
> + return ret;
> + }
> +
> + ret = imx_drm_encoder_parse_of(drm, &dsi->encoder, dsi->dev->of_node);
> + if (ret)
> + return ret;
> +
> + ret = drm_simple_encoder_init(drm, &dsi->encoder, DRM_MODE_ENCODER_NONE);
> + if (ret) {
> + DRM_DEV_ERROR(dev, "failed to create drm encoder\n");
> + return ret;
> + }
> +
> + drm_bridge_add(&dsi->bridge);
You are adding the bridge in imx_mipi_dsi_bind() but removing in
imx_mipi_dsi_remove(). Although in this case I don't think there is any bad
effect, would be nice maintain the symetry, so I'd move the drm_bridge_add()
call to the imx_mipi_dis_probe() function just before component_add().
> +
> + drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL, 0);
> + if (ret < 0) {
> + DRM_DEV_ERROR(dev, "failed to attach IMX6 bridge: %d\n", ret);
> + return ret;
If it fails you probably want to cleanup the encoder drm_cleanup_encoder()
> + }
> +
> + ret = dw_mipi_dsi_bind(dsi->mipi_dsi, &dsi->encoder, &dsi->bridge);
> + if (ret) {
> + DRM_DEV_ERROR(dev, "Failed to bind: %d\n", ret);
> + return ret;
The same here.
> + }
> +
> + return 0;
> +}
> +
> +static void imx_mipi_dsi_unbind(struct device *dev, struct device *master,
> + void *data)
> +{
> + struct imx_mipi_dsi *dsi = dev_get_drvdata(dev);
> +
> + dw_mipi_dsi_unbind(dsi->mipi_dsi);
> +
drm_encoder_cleanup() ?
> + clk_disable_unprepare(dsi->pllref_clk);
> +}
> +
> +static const struct component_ops imx_mipi_dsi_ops = {
> + .bind = imx_mipi_dsi_bind,
> + .unbind = imx_mipi_dsi_unbind,
> +};
> +
> +static int imx_mipi_dsi_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + const struct of_device_id *of_id = of_match_device(imx_dsi_dt_ids, dev);
> + struct dw_mipi_dsi_plat_data *pdata = (struct dw_mipi_dsi_plat_data *) of_id->data;
> + struct imx_mipi_dsi *dsi;
> + struct resource *res;
> + int ret;
> +
> + dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
> + if (!dsi)
> + return -ENOMEM;
> +
> + dsi->dev = dev;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + dsi->base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(dsi->base)) {
> + ret = PTR_ERR(dsi->base);
> + DRM_DEV_ERROR(dev, "Unable to get dsi registers: %d\n", ret);
> + return ret;
This print is redundant, devm_ioremap_resource will print an error already. This
code can be simply
return PTR_ERR(res);
> + }
> +
> + dsi->pllref_clk = devm_clk_get(dev, "ref");
> + if (IS_ERR(dsi->pllref_clk)) {
> + ret = PTR_ERR(dsi->pllref_clk);
> + DRM_DEV_ERROR(dev, "Unable to get pllref_clk: %d\n", ret);
This print is redundant, devm_clk_get will print an error already. This code can
be simply
return PTR_ERR(dsi->pllref_clk);
> + return ret;
> + }
> +
> + dsi->mux_sel = syscon_regmap_lookup_by_phandle(dev->of_node, "fsl,gpr");
> + if (IS_ERR(dsi->mux_sel)) {
> + ret = PTR_ERR(dsi->mux_sel);
> + DRM_DEV_ERROR(dev, "Failed to get GPR regmap: %d\n", ret);
> + return ret;
> + }
> +
> + dev_set_drvdata(dev, dsi);
> +
> + imx6_mipi_dsi_drv_data.base = dsi->base;
> + imx6_mipi_dsi_drv_data.priv_data = dsi;
> +
> + dsi->bridge.driver_private = dsi;
> + dsi->bridge.funcs = &imx_dsi_bridge_funcs;
> + dsi->bridge.of_node = dev->of_node;
You probably want to set the bridge.type here too:
dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
> +
> + dsi->mipi_dsi = dw_mipi_dsi_probe(pdev, pdata);
> + if (IS_ERR(dsi->mipi_dsi)) {
> + ret = PTR_ERR(dsi->mipi_dsi);
> + DRM_DEV_ERROR(dev, "Failed to probe DW DSI host: %d\n", ret);
> + goto err_clkdisable;
> + }
> +
> + return component_add(&pdev->dev, &imx_mipi_dsi_ops);
> +
> +err_clkdisable:
> + clk_disable_unprepare(dsi->pllref_clk);
> + return ret;
> +}
> +
> +static int imx_mipi_dsi_remove(struct platform_device *pdev)
> +{
> + struct imx_mipi_dsi *dsi = platform_get_drvdata(pdev);
> + drm_bridge_remove(&dsi->bridge);
> + component_del(&pdev->dev, &imx_mipi_dsi_ops);
> + return 0;
> +}
> +
> +static struct platform_driver imx_mipi_dsi_driver = {
> + .probe = imx_mipi_dsi_probe,
> + .remove = imx_mipi_dsi_remove,
> + .driver = {
> + .of_match_table = imx_dsi_dt_ids,
> + .name = "dw-mipi-dsi-imx6",
> + },
> +};
> +module_platform_driver(imx_mipi_dsi_driver);
> +
> +MODULE_DESCRIPTION("i.MX6 MIPI DSI host controller driver");
> +MODULE_AUTHOR("Liu Ying <victor.liu@nxp.com>");
> +MODULE_AUTHOR("Adrian Ratiu <adrian.ratiu@collabora.com>");
> +MODULE_LICENSE("GPL");
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v8 05/10] drm: imx: Add i.MX 6 MIPI DSI host platform driver
2020-04-27 14:41 ` Enric Balletbo i Serra
@ 2020-04-27 18:26 ` Adrian Ratiu
0 siblings, 0 replies; 21+ messages in thread
From: Adrian Ratiu @ 2020-04-27 18:26 UTC (permalink / raw)
To: Enric Balletbo i Serra, linux-arm-kernel, devicetree,
linux-rockchip, Laurent Pinchart
Cc: Jernej Skrabec, Adrian Pop, Jonas Karlman, Martyn Welch,
Sjoerd Simons, linux-kernel, dri-devel, Andrzej Hajda, linux-imx,
kernel, linux-stm32, Arnaud Ferraris, Emil Velikov
Hi Enric,
Thank you very much for your review, comments below.
I'll leave this a bit more on review before resending with your
suggested fixes.
On Mon, 27 Apr 2020, Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
> Hi Adrian
>
> Thank you for your patch.
>
> On 27/4/20 10:19, Adrian Ratiu wrote:
>> This adds support for the Synopsis DesignWare MIPI DSI v1.01
>> host controller which is embedded in i.MX 6 SoCs. Based on
>> following patches, but updated/extended to work with existing
>> support found in the kernel: - drm: imx: Support Synopsys
>> DesignWare MIPI DSI host controller
>> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
>> Cc: Fabio Estevam <festevam@gmail.com> Cc: Enric Balletbo
>> Serra <eballetbo@gmail.com> Reviewed-by: Emil Velikov
>> <emil.velikov@collabora.com> Tested-by: Adrian Pop
>> <pop.adrian61@gmail.com> Tested-by: Arnaud Ferraris
>> <arnaud.ferraris@collabora.com> Signed-off-by: Sjoerd Simons
>> <sjoerd.simons@collabora.com> Signed-off-by: Martyn Welch
>> <martyn.welch@collabora.com> Signed-off-by: Adrian Ratiu
>> <adrian.ratiu@collabora.com> --- Changes since v7:
>> - Removed encoder helper ops and added drm_bridge (Laurent) -
>> Brought back drm_simple_encoder_init and dropped dependency
>> on external unify encoder creation patch (Laurent) - Minor
>> typo fixes
>> Changes since v6:
>> - Replaced custom noop encoder with the simple drm encoder
>> (Enric) - Added CONFIG_DRM_IMX6_MIPI_DSI depends on CONFIG_OF
>> (Enric) - Dropped imx_mipi_dsi_register() because now it only
>> creates the dummy encoder which can easily be done directly
>> in imx_dsi_bind()
>> Changes since v5:
>> - Reword to remove unrelated device tree patch mention
>> (Fabio) - Move pllref_clk enable/disable to bind/unbind
>> (Ezequiel) - Fix freescale.com -> nxp.com email addresses
>> (Fabio) - Also added myself as module author (Fabio) - Use
>> DRM_DEV_* macros for consistency, print more error msg
>> Changes since v4:
>> - Split off driver-specific configuration of phy timings due
>> to new upstream API. - Move regmap infrastructure logic to
>> separate commit (Ezequiel) - Move dsi v1.01 layout addition
>> to a separate commit (Ezequiel) - Minor warnings and driver
>> name fixes
>> Changes since v3:
>> - Renamed platform driver to reflect it's i.MX6 only. (Fabio)
>> Changes since v2:
>> - 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/imx/Kconfig | 8 +
>> drivers/gpu/drm/imx/Makefile | 1 +
>> drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c | 399
>> +++++++++++++++++++++++++ 3 files changed, 408 insertions(+)
>> create mode 100644 drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c
>> diff --git a/drivers/gpu/drm/imx/Kconfig
>> b/drivers/gpu/drm/imx/Kconfig index
>> 207bf7409dfba..0dffc72df7922 100644 ---
>> a/drivers/gpu/drm/imx/Kconfig +++ b/drivers/gpu/drm/imx/Kconfig
>> @@ -39,3 +39,11 @@ config DRM_IMX_HDMI
>> depends on DRM_IMX help Choose this if you want to use
>> HDMI on i.MX6.
>> + +config DRM_IMX6_MIPI_DSI + tristate "Freescale i.MX6
>> DRM MIPI DSI" + select DRM_DW_MIPI_DSI + depends on
>> DRM_IMX + depends on OF + help + Choose this if you want
>> to use MIPI DSI on i.MX6. diff --git
>> a/drivers/gpu/drm/imx/Makefile b/drivers/gpu/drm/imx/Makefile
>> index 21cdcc2faabc8..9a7843c593478 100644 ---
>> a/drivers/gpu/drm/imx/Makefile +++
>> b/drivers/gpu/drm/imx/Makefile @@ -9,3 +9,4 @@
>> obj-$(CONFIG_DRM_IMX_TVE) += imx-tve.o
>> obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o
>> obj-$(CONFIG_DRM_IMX_HDMI) += dw_hdmi-imx.o
>> +obj-$(CONFIG_DRM_IMX6_MIPI_DSI) += dw_mipi_dsi-imx6.o diff
>> --git a/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c
>> b/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c new file mode 100644
>> index 0000000000000..492decc418bc3 --- /dev/null +++
>> b/drivers/gpu/drm/imx/dw_mipi_dsi-imx6.c @@ -0,0 +1,399 @@ +//
>> SPDX-License-Identifier: GPL-2.0+ +/* + * i.MX6 drm driver -
>> MIPI DSI Host Controller + * + * Copyright (C) 2011-2015
>> Freescale Semiconductor, Inc. + * Copyright (C) 2019-2020
>> Collabora, Ltd. + */ + +#include <linux/clk.h> +#include
>> <linux/component.h> +#include <linux/mfd/syscon.h> +#include
>> <linux/mfd/syscon/imx6q-iomuxc-gpr.h> +#include
>> <linux/module.h> +#include <linux/of_device.h> +#include
>> <linux/regmap.h> +#include <linux/videodev2.h> +#include
>> <drm/bridge/dw_mipi_dsi.h> +#include
>> <drm/drm_atomic_state_helper.h> +#include
>> <drm/drm_crtc_helper.h> +#include <drm/drm_mipi_dsi.h>
>> +#include <drm/drm_of.h> +#include <drm/drm_print.h> +#include
>> <drm/drm_simple_kms_helper.h> + +#include "imx-drm.h" +
>> +#define DSI_PWR_UP 0x04 +#define RESET
>> 0 +#define POWERUP BIT(0) + +#define
>> DSI_PHY_IF_CTRL 0x5c +#define
>> PHY_IF_CTRL_RESET 0x0 + +#define DSI_PHY_TST_CTRL0
>> 0x64 +#define PHY_TESTCLK BIT(1) +#define
>> PHY_UNTESTCLK 0 +#define PHY_TESTCLR
>> BIT(0) +#define PHY_UNTESTCLR 0 +
>> +#define DSI_PHY_TST_CTRL1 0x68 +#define PHY_TESTEN
>> BIT(16) +#define PHY_UNTESTEN 0 +#define
>> PHY_TESTDOUT(n) (((n) & 0xff) << 8)
>> +#define PHY_TESTDIN(n) (((n) & 0xff) <<
>> 0) + +struct imx_mipi_dsi { + struct drm_encoder
>> encoder; + struct drm_bridge bridge; + struct device
>> *dev; + struct regmap *mux_sel; + struct dw_mipi_dsi
>> *mipi_dsi; + struct clk *pllref_clk; + + void __iomem
>> *base; + unsigned int lane_mbps; +}; + +struct
>> dphy_pll_testdin_map { + unsigned int max_mbps; + u8
>> testdin; +}; + +/* The table is based on 27MHz DPHY pll
>> reference clock. */ +static const struct dphy_pll_testdin_map
>> dptdin_map[] = { + {160, 0x04}, {180, 0x24}, {200, 0x44},
>> {210, 0x06}, + {240, 0x26}, {250, 0x46}, {270, 0x08},
>> {300, 0x28}, + {330, 0x48}, {360, 0x2a}, {400, 0x4a},
>> {450, 0x0c}, + {500, 0x2c}, {550, 0x0e}, {600, 0x2e},
>> {650, 0x10}, + {700, 0x30}, {750, 0x12}, {800, 0x32},
>> {850, 0x14}, + {900, 0x34}, {950, 0x54}, {1000, 0x74} +};
>> + +static inline struct imx_mipi_dsi *bridge_to_imxdsi(struct
>> drm_bridge *br) +{ + return container_of(br, struct
>> imx_mipi_dsi, bridge); +} + +static int
>> +imx_mipi_dsi_bridge_atomic_check(struct drm_bridge *bridge, +
>> struct drm_bridge_state *bridge_state, +
>> struct drm_crtc_state *crtc_state, +
>> struct drm_connector_state *conn_state) +{ + struct
>> imx_crtc_state *imx_crtc_state = to_imx_crtc_state(crtc_state);
>> + + /* The following values are taken from
>> dw_hdmi_imx_atomic_check */ + imx_crtc_state->bus_format
>> = MEDIA_BUS_FMT_RGB888_1X24; +
>> imx_crtc_state->di_hsync_pin = 2; +
>> imx_crtc_state->di_vsync_pin = 3; + + return 0; +} +
>> +static void imx_mipi_dsi_bridge_atomic_enable(struct
>> drm_bridge *bridge, +
>> struct drm_bridge_state *old_st) +{ + struct
>> imx_mipi_dsi *dsi = bridge_to_imxdsi(bridge); + int mux =
>> drm_of_encoder_active_port_id(dsi->dev->of_node, +
>> &dsi->encoder); + /* Set IOMUX DSI output reg to correct IPU
>> 1 or 0 and DI 1 or 0 ports */ +
>> regmap_update_bits(dsi->mux_sel, IOMUXC_GPR3, +
>> IMX6Q_GPR3_MIPI_MUX_CTL_MASK, + mux <<
>> IMX6Q_GPR3_MIPI_MUX_CTL_SHIFT); +} + +static const struct
>> drm_bridge_funcs imx_dsi_bridge_funcs = { + .atomic_enable =
>> imx_mipi_dsi_bridge_atomic_enable, + .atomic_check =
>> imx_mipi_dsi_bridge_atomic_check, + .atomic_reset =
>> drm_atomic_helper_bridge_reset, + .atomic_duplicate_state =
>> drm_atomic_helper_bridge_duplicate_state, +
>> .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
>> +}; + +static enum drm_mode_status imx_mipi_dsi_mode_valid(void
>> *priv_data, + const
>> struct drm_display_mode *mode) +{ + /* + * The
>> VID_PKT_SIZE field in the DSI_VID_PKT_CFG + * register is
>> 11-bit. + */ + if (mode->hdisplay > 0x7ff) +
>> return MODE_BAD_HVALUE; + + /* + * The V_ACTIVE_LINES
>> field in the DSI_VTIMING_CFG + * register is 11-bit. +
>> */ + if (mode->vdisplay > 0x7ff) + return
>> MODE_BAD_VVALUE; + + return MODE_OK; +} + + +static unsigned
>> int max_mbps_to_testdin(unsigned int max_mbps) +{ + unsigned
>> int i; + + for (i = 0; i < ARRAY_SIZE(dptdin_map); i++) +
>> if (dptdin_map[i].max_mbps == max_mbps) +
>> return dptdin_map[i].testdin; + + return -EINVAL; +} +
>> +static inline void dsi_write(struct imx_mipi_dsi *dsi, u32
>> reg, u32 val) +{ + writel(val, dsi->base + reg); +} + +static
>> int imx_mipi_dsi_phy_init(void *priv_data) +{ + struct
>> imx_mipi_dsi *dsi = priv_data; + int testdin; + +
>> testdin = max_mbps_to_testdin(dsi->lane_mbps); + if
>> (testdin < 0) { + DRM_DEV_ERROR(dsi->dev, +
>> "failed to get testdin for %dmbps lane clock\n", +
>> dsi->lane_mbps); + return testdin; + } + +
>> dsi_write(dsi, DSI_PHY_IF_CTRL, PHY_IF_CTRL_RESET); +
>> dsi_write(dsi, DSI_PWR_UP, POWERUP); + + dsi_write(dsi,
>> DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR); +
>> dsi_write(dsi, DSI_PHY_TST_CTRL1, PHY_TESTEN | PHY_TESTDOUT(0)
>> | + PHY_TESTDIN(0x44)); + dsi_write(dsi,
>> DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR); +
>> dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLK |
>> PHY_UNTESTCLR); + dsi_write(dsi, DSI_PHY_TST_CTRL1,
>> PHY_UNTESTEN | PHY_TESTDOUT(0) | +
>> PHY_TESTDIN(testdin)); + dsi_write(dsi, DSI_PHY_TST_CTRL0,
>> PHY_TESTCLK | PHY_UNTESTCLR); + dsi_write(dsi,
>> DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR); + +
>> return 0; +} + +static int imx_mipi_dsi_get_lane_mbps(void
>> *priv_data, + const struct
>> drm_display_mode *mode, +
>> unsigned long mode_flags, u32 lanes, +
>> u32 format, unsigned int *lane_mbps) +{ + struct
>> imx_mipi_dsi *dsi = priv_data; + int bpp; + unsigned
>> int i, target_mbps, mpclk; + unsigned long pllref; + +
>> bpp = mipi_dsi_pixel_format_to_bpp(format); + if (bpp <
>> 0) { + DRM_DEV_ERROR(dsi->dev, "failed to get bpp
>> for format %d: %d\n", + format,
>> bpp); + return bpp; + } + + pllref =
>> clk_get_rate(dsi->pllref_clk); + if (pllref != 27000000) +
>> DRM_WARN("DSI pllref_clk not set to 27Mhz\n");
>
> This pllref variable is only used to print this warning, what's
> the point of warn and ignore it? If pllref is not set at 27MHz
> what will happen?
>
According to the imx6 ref manual, 27Mhz is the maximum value of
the D-PHY TX refclk used to drive the display in high-speed mode
(in low power modes this clock is disabled). It's range is [6,27]
Mhz.
The manual-provided tables and examples for operating the
controller always assumes the ref clk is set to 27Mhz which allows
a maximum of
1Ghz pll and 125 Mhz byte clocks achieving 1000 lane mbps.
This driver does the same thing as the imx manual if you look at
the definition of `dptdin_map` above which is used to compute the
maximum lane speed for the specific mode clock.
In theory I guess power consumption can be lowered by using a
less-than max value for the refclk, but I've never seen this done
in practice and is very application specific... Of course, if this
is a real power-save gain and people care about it, optimized
tables can be added later :)
If refclk is not set to 27Mhz, in the worst case, the
configuration done in imx_mipi_dsi_phy_init() and
imx_mipi_dsi_get_lane_mbps() based on the values provided in
`dptdin_map` won't be able to drive the display.
So I guess the best course of action is to drop this warn and make
it an error in the platform driver bind() when enabling the clock,
right?
>> + + mpclk = DIV_ROUND_UP(mode->clock, MSEC_PER_SEC); + if
>> (mpclk) { + /* take 1/0.7 blanking overhead into
>> consideration */ + target_mbps = (mpclk * (bpp /
>> lanes) * 10) / 7; + } else { +
>> DRM_DEV_ERROR(dsi->dev, "use default 1Gbps DPHY pll clock\n");
>
> That's really an error? Looks to me that's just a fallback if
> mpclk is zero. Maybe set the level to INFO or even better
> DEBUG.
>
Yes, I will do this and all the other suggestions you made
below. Thanks!
BTW I also noticed the below prints are wrong, will also fix them.
>> + target_mbps = 1000; + } + +
>> DRM_DEV_DEBUG(dsi->dev, "target pllref_clk frequency is
>> %uMbps\n", + target_mbps); + + for (i = 0; i <
>> ARRAY_SIZE(dptdin_map); i++) { + if (target_mbps <
>> dptdin_map[i].max_mbps) { + *lane_mbps =
>> dptdin_map[i].max_mbps; + dsi->lane_mbps =
>> *lane_mbps; + DRM_DEV_DEBUG(dsi->dev, +
>> "real pllref_clk frequency is %uMbps\n", +
>> *lane_mbps); + return 0; + }
>> + } + + DRM_DEV_ERROR(dsi->dev, "DPHY clock frequency
>> %uMbps is out of range\n", + target_mbps); + +
>> return -EINVAL; +} + +static int
>> +dw_mipi_dsi_phy_get_timing(void *priv_data, unsigned int
>> lane_mbps, + struct dw_mipi_dsi_dphy_timing
>> *timing) +{ + timing->clk_hs2lp = 0x40; +
>> timing->clk_lp2hs = 0x40; + timing->data_hs2lp = 0x40; +
>> timing->data_lp2hs = 0x40; + + return 0; +} + +static
>> const struct dw_mipi_dsi_phy_ops dw_mipi_dsi_imx6_phy_ops = { +
>> .init = imx_mipi_dsi_phy_init, + .get_lane_mbps =
>> imx_mipi_dsi_get_lane_mbps, + .get_timing =
>> dw_mipi_dsi_phy_get_timing, +}; + +static struct
>> dw_mipi_dsi_plat_data imx6_mipi_dsi_drv_data = { +
>> .max_data_lanes = 2, + .mode_valid =
>> imx_mipi_dsi_mode_valid, + .phy_ops =
>> &dw_mipi_dsi_imx6_phy_ops, +}; + +static const struct
>> of_device_id imx_dsi_dt_ids[] = { + { +
>> .compatible = "fsl,imx6-mipi-dsi", + .data =
>> &imx6_mipi_dsi_drv_data, + }, + { /* sentinel */ } +};
>> +MODULE_DEVICE_TABLE(of, imx_dsi_dt_ids); + +static int
>> imx_mipi_dsi_bind(struct device *dev, struct device *master, +
>> void *data) +{ + struct imx_mipi_dsi *dsi =
>> dev_get_drvdata(dev); + struct drm_device *drm = data; +
>> int ret; + + ret = clk_prepare_enable(dsi->pllref_clk); + if
>> (ret) { + DRM_DEV_ERROR(dev, "Failed to enable
>> pllref_clk: %d\n", ret); + return ret; + } + +
>> ret = imx_drm_encoder_parse_of(drm, &dsi->encoder,
>> dsi->dev->of_node); + if (ret) + return
>> ret; + + ret = drm_simple_encoder_init(drm, &dsi->encoder,
>> DRM_MODE_ENCODER_NONE); + if (ret) { +
>> DRM_DEV_ERROR(dev, "failed to create drm encoder\n"); +
>> return ret; + } + + drm_bridge_add(&dsi->bridge);
>
> You are adding the bridge in imx_mipi_dsi_bind() but removing in
> imx_mipi_dsi_remove(). Although in this case I don't think there
> is any bad effect, would be nice maintain the symetry, so I'd
> move the drm_bridge_add() call to the imx_mipi_dis_probe()
> function just before component_add().
>
>> + + drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL, 0); +
>> if (ret < 0) { + DRM_DEV_ERROR(dev, "failed to
>> attach IMX6 bridge: %d\n", ret); + return ret;
>
> If it fails you probably want to cleanup the encoder
> drm_cleanup_encoder()
>
>> + }
>> +
>> + ret = dw_mipi_dsi_bind(dsi->mipi_dsi, &dsi->encoder, &dsi->bridge);
>> + if (ret) {
>> + DRM_DEV_ERROR(dev, "Failed to bind: %d\n", ret);
>> + return ret;
>
> The same here.
>
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void imx_mipi_dsi_unbind(struct device *dev, struct device *master,
>> + void *data)
>> +{
>> + struct imx_mipi_dsi *dsi = dev_get_drvdata(dev);
>> +
>> + dw_mipi_dsi_unbind(dsi->mipi_dsi);
>> +
>
> drm_encoder_cleanup() ?
>
>> + clk_disable_unprepare(dsi->pllref_clk);
>> +}
>> +
>> +static const struct component_ops imx_mipi_dsi_ops = {
>> + .bind = imx_mipi_dsi_bind,
>> + .unbind = imx_mipi_dsi_unbind,
>> +};
>> +
>> +static int imx_mipi_dsi_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + const struct of_device_id *of_id = of_match_device(imx_dsi_dt_ids, dev);
>> + struct dw_mipi_dsi_plat_data *pdata = (struct dw_mipi_dsi_plat_data *) of_id->data;
>> + struct imx_mipi_dsi *dsi;
>> + struct resource *res;
>> + int ret;
>> +
>> + dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
>> + if (!dsi)
>> + return -ENOMEM;
>> +
>> + dsi->dev = dev;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + dsi->base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(dsi->base)) {
>> + ret = PTR_ERR(dsi->base);
>> + DRM_DEV_ERROR(dev, "Unable to get dsi registers: %d\n", ret);
>> + return ret;
>
> This print is redundant, devm_ioremap_resource will print an error already. This
> code can be simply
> return PTR_ERR(res);
>
>> + }
>> +
>> + dsi->pllref_clk = devm_clk_get(dev, "ref");
>> + if (IS_ERR(dsi->pllref_clk)) {
>> + ret = PTR_ERR(dsi->pllref_clk);
>> + DRM_DEV_ERROR(dev, "Unable to get pllref_clk: %d\n", ret);
>
> This print is redundant, devm_clk_get will print an error already. This code can
> be simply
> return PTR_ERR(dsi->pllref_clk);
>
>> + return ret;
>> + }
>> +
>> + dsi->mux_sel = syscon_regmap_lookup_by_phandle(dev->of_node, "fsl,gpr");
>> + if (IS_ERR(dsi->mux_sel)) {
>> + ret = PTR_ERR(dsi->mux_sel);
>> + DRM_DEV_ERROR(dev, "Failed to get GPR regmap: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + dev_set_drvdata(dev, dsi);
>> +
>> + imx6_mipi_dsi_drv_data.base = dsi->base;
>> + imx6_mipi_dsi_drv_data.priv_data = dsi;
>> +
>> + dsi->bridge.driver_private = dsi;
>> + dsi->bridge.funcs = &imx_dsi_bridge_funcs;
>> + dsi->bridge.of_node = dev->of_node;
>
> You probably want to set the bridge.type here too:
>
> dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
>
>> +
>> + dsi->mipi_dsi = dw_mipi_dsi_probe(pdev, pdata);
>> + if (IS_ERR(dsi->mipi_dsi)) {
>> + ret = PTR_ERR(dsi->mipi_dsi);
>> + DRM_DEV_ERROR(dev, "Failed to probe DW DSI host: %d\n", ret);
>> + goto err_clkdisable;
>> + }
>> +
>> + return component_add(&pdev->dev, &imx_mipi_dsi_ops);
>> +
>> +err_clkdisable:
>> + clk_disable_unprepare(dsi->pllref_clk);
>> + return ret;
>> +}
>> +
>> +static int imx_mipi_dsi_remove(struct platform_device *pdev)
>> +{
>> + struct imx_mipi_dsi *dsi = platform_get_drvdata(pdev);
>> + drm_bridge_remove(&dsi->bridge);
>> + component_del(&pdev->dev, &imx_mipi_dsi_ops);
>> + return 0;
>> +}
>> +
>> +static struct platform_driver imx_mipi_dsi_driver = {
>> + .probe = imx_mipi_dsi_probe,
>> + .remove = imx_mipi_dsi_remove,
>> + .driver = {
>> + .of_match_table = imx_dsi_dt_ids,
>> + .name = "dw-mipi-dsi-imx6",
>> + },
>> +};
>> +module_platform_driver(imx_mipi_dsi_driver);
>> +
>> +MODULE_DESCRIPTION("i.MX6 MIPI DSI host controller driver");
>> +MODULE_AUTHOR("Liu Ying <victor.liu@nxp.com>");
>> +MODULE_AUTHOR("Adrian Ratiu <adrian.ratiu@collabora.com>");
>> +MODULE_LICENSE("GPL");
>>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v8 06/10] ARM: dts: imx6qdl: add missing mipi dsi properties
2020-04-27 8:19 [PATCH v8 00/10] Genericize DW MIPI DSI bridge and add i.MX 6 driver Adrian Ratiu
` (4 preceding siblings ...)
2020-04-27 8:19 ` [PATCH v8 05/10] drm: imx: Add i.MX 6 MIPI DSI host platform driver Adrian Ratiu
@ 2020-04-27 8:19 ` Adrian Ratiu
2020-04-27 8:19 ` [PATCH v8 07/10] dt-bindings: display: add i.MX6 MIPI DSI host controller doc Adrian Ratiu
` (3 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Adrian Ratiu @ 2020-04-27 8:19 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, linux-rockchip, Laurent Pinchart
Cc: Jernej Skrabec, Jonas Karlman, linux-kernel, dri-devel,
Andrzej Hajda, linux-imx, kernel, linux-stm32, Laurent Pinchart
Now that we have a proper driver for the imx6 mipi dsi host controller
we can fill in the missing properties to get it working.
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
New in v8.
---
arch/arm/boot/dts/imx6qdl.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 98da446aa0f27..5f754c3cec052 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -1203,7 +1203,15 @@ mipi_csi: mipi@21dc000 {
};
mipi_dsi: mipi@21e0000 {
+ compatible = "fsl,imx6-mipi-dsi", "snps,dw-mipi-dsi";
+ #address-cells = <1>;
+ #size-cells = <0>;
reg = <0x021e0000 0x4000>;
+ interrupts = <0 102 IRQ_TYPE_LEVEL_HIGH>;
+ fsl,gpr = <&gpr>;
+ clocks = <&clks IMX6QDL_CLK_MIPI_CORE_CFG>,
+ <&clks IMX6QDL_CLK_MIPI_IPG>;
+ clock-names = "ref", "pclk";
status = "disabled";
ports {
--
2.26.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v8 07/10] dt-bindings: display: add i.MX6 MIPI DSI host controller doc
2020-04-27 8:19 [PATCH v8 00/10] Genericize DW MIPI DSI bridge and add i.MX 6 driver Adrian Ratiu
` (5 preceding siblings ...)
2020-04-27 8:19 ` [PATCH v8 06/10] ARM: dts: imx6qdl: add missing mipi dsi properties Adrian Ratiu
@ 2020-04-27 8:19 ` Adrian Ratiu
2020-05-01 20:26 ` Rob Herring
2020-04-27 8:19 ` [PATCH v8 08/10] drm: stm: dw-mipi-dsi: let the bridge handle the HW version check Adrian Ratiu
` (2 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Adrian Ratiu @ 2020-04-27 8:19 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, linux-rockchip, Laurent Pinchart
Cc: Jernej Skrabec, Sjoerd Simons, Adrian Pop, Jonas Karlman,
Martyn Welch, Neil Armstrong, linux-kernel, dri-devel,
Andrzej Hajda, linux-imx, kernel, linux-stm32, Arnaud Ferraris,
Laurent Pinchart
This provides an example DT binding for the MIPI DSI host controller
present on the i.MX6 SoC based on Synopsis DesignWare v1.01 IP.
Cc: Rob Herring <robh@kernel.org>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: devicetree@vger.kernel.org
Tested-by: Adrian Pop <pop.adrian61@gmail.com>
Tested-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.com>
Signed-off-by: Martyn Welch <martyn.welch@collabora.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
Changes since v7:
- Clarified port@0,1 descriptions, marked them as required and
added missing port@0 in example (Laurent)
Changes since v6:
- Added ref to the newly created snps,dw-mipi-dsi.yaml (Laurent)
- Moved *-cells properties outside patternProperties (Laurent)
- Removed the panel port documentation (Laurent)
- Wrapped lines at 80 chars, typo fixes, sort includes (Laurent)
Changes since v5:
- Fixed missing reg warning (Fabio)
- Updated dt-schema and fixed warnings (Rob)
Changes since v4:
- Fixed yaml binding to pass `make dt_binding_check dtbs_check`
and addressed received binding feedback (Rob)
Changes since v3:
- Added commit message (Neil)
- Converted to yaml format (Neil)
- Minor dt node + driver fixes (Rob)
- Added small panel example to the host controller binding
Changes since v2:
- Fixed commit tags (Emil)
---
.../display/imx/fsl,mipi-dsi-imx6.yaml | 145 ++++++++++++++++++
1 file changed, 145 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,mipi-dsi-imx6.yaml
diff --git a/Documentation/devicetree/bindings/display/imx/fsl,mipi-dsi-imx6.yaml b/Documentation/devicetree/bindings/display/imx/fsl,mipi-dsi-imx6.yaml
new file mode 100644
index 0000000000000..c2c3489e63fa3
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/imx/fsl,mipi-dsi-imx6.yaml
@@ -0,0 +1,145 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/imx/fsl,mipi-dsi-imx6.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale i.MX6 DW MIPI DSI Host Controller
+
+maintainers:
+ - Adrian Ratiu <adrian.ratiu@collabora.com>
+
+description: |
+ The i.MX6 DSI host controller is a Synopsys DesignWare MIPI DSI v1.01
+ IP block with a companion PHY IP.
+
+ These DT bindings follow the Synopsys DW MIPI DSI bindings defined in
+ Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt with
+ the following device-specific properties.
+
+allOf:
+ - $ref: ../bridge/snps,dw-mipi-dsi.yaml#
+
+properties:
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ compatible:
+ items:
+ - const: fsl,imx6q-mipi-dsi
+ - const: snps,dw-mipi-dsi
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: Module Clock
+ - description: DSI bus clock
+
+ clock-names:
+ items:
+ - const: ref
+ - const: pclk
+
+ fsl,gpr:
+ description:
+ Phandle to the iomuxc-gpr region containing the multiplexer ctrl register.
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+ ports:
+ type: object
+ description: |
+ A node containing DSI input & output port nodes with endpoint
+ definitions as documented in
+ Documentation/devicetree/bindings/media/video-interfaces.txt
+ Documentation/devicetree/bindings/graph.txt
+ properties:
+ port@0:
+ type: object
+ description:
+ DSI input port connected to a parallel RGB LTDC output port.
+
+ port@1:
+ type: object
+ description:
+ DSI serial RGB output port connected to a panel or bridge input port.
+
+ required:
+ - port@0
+ - port@1
+
+additionalProperties: false
+
+patternProperties:
+ "^panel@[0-3]$":
+ type: object
+
+required:
+ - "#address-cells"
+ - "#size-cells"
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+ - ports
+
+examples:
+ - |+
+ #include <dt-bindings/clock/imx6qdl-clock.h>
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ dsi: dsi@21e0000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "fsl,imx6q-mipi-dsi", "snps,dw-mipi-dsi";
+ reg = <0x021e0000 0x4000>;
+ interrupts = <0 102 IRQ_TYPE_LEVEL_HIGH>;
+ fsl,gpr = <&gpr>;
+ clocks = <&clks IMX6QDL_CLK_MIPI_CORE_CFG>,
+ <&clks IMX6QDL_CLK_MIPI_IPG>;
+ clock-names = "ref", "pclk";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ mipi_mux_0: endpoint {
+ remote-endpoint = <&ipu1_di0_mipi>;
+ };
+ };
+ port@1 {
+ reg = <1>;
+ dsi_out: endpoint {
+ remote-endpoint = <&panel_in>;
+ };
+ };
+ };
+
+ panel@0 {
+ compatible = "sharp,ls032b3sx01";
+ reg = <0>;
+ reset-gpios = <&gpio6 8 GPIO_ACTIVE_LOW>;
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ panel_in: endpoint {
+ remote-endpoint = <&dsi_out>;
+ };
+ };
+ };
+ };
+ };
+
+...
--
2.26.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v8 07/10] dt-bindings: display: add i.MX6 MIPI DSI host controller doc
2020-04-27 8:19 ` [PATCH v8 07/10] dt-bindings: display: add i.MX6 MIPI DSI host controller doc Adrian Ratiu
@ 2020-05-01 20:26 ` Rob Herring
0 siblings, 0 replies; 21+ messages in thread
From: Rob Herring @ 2020-05-01 20:26 UTC (permalink / raw)
To: Adrian Ratiu
Cc: devicetree, Jernej Skrabec, Sjoerd Simons, Adrian Pop,
Jonas Karlman, Martyn Welch, Neil Armstrong, linux-kernel,
dri-devel, Andrzej Hajda, Laurent Pinchart, Arnaud Ferraris,
linux-rockchip, kernel, linux-stm32, linux-arm-kernel, linux-imx
On Mon, Apr 27, 2020 at 11:19:49AM +0300, Adrian Ratiu wrote:
> This provides an example DT binding for the MIPI DSI host controller
> present on the i.MX6 SoC based on Synopsis DesignWare v1.01 IP.
>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: devicetree@vger.kernel.org
> Tested-by: Adrian Pop <pop.adrian61@gmail.com>
> Tested-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.com>
> Signed-off-by: Martyn Welch <martyn.welch@collabora.com>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> ---
> Changes since v7:
> - Clarified port@0,1 descriptions, marked them as required and
> added missing port@0 in example (Laurent)
>
> Changes since v6:
> - Added ref to the newly created snps,dw-mipi-dsi.yaml (Laurent)
> - Moved *-cells properties outside patternProperties (Laurent)
> - Removed the panel port documentation (Laurent)
> - Wrapped lines at 80 chars, typo fixes, sort includes (Laurent)
>
> Changes since v5:
> - Fixed missing reg warning (Fabio)
> - Updated dt-schema and fixed warnings (Rob)
>
> Changes since v4:
> - Fixed yaml binding to pass `make dt_binding_check dtbs_check`
> and addressed received binding feedback (Rob)
>
> Changes since v3:
> - Added commit message (Neil)
> - Converted to yaml format (Neil)
> - Minor dt node + driver fixes (Rob)
> - Added small panel example to the host controller binding
>
> Changes since v2:
> - Fixed commit tags (Emil)
> ---
> .../display/imx/fsl,mipi-dsi-imx6.yaml | 145 ++++++++++++++++++
> 1 file changed, 145 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,mipi-dsi-imx6.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/imx/fsl,mipi-dsi-imx6.yaml b/Documentation/devicetree/bindings/display/imx/fsl,mipi-dsi-imx6.yaml
> new file mode 100644
> index 0000000000000..c2c3489e63fa3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/imx/fsl,mipi-dsi-imx6.yaml
> @@ -0,0 +1,145 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/imx/fsl,mipi-dsi-imx6.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale i.MX6 DW MIPI DSI Host Controller
> +
> +maintainers:
> + - Adrian Ratiu <adrian.ratiu@collabora.com>
> +
> +description: |
> + The i.MX6 DSI host controller is a Synopsys DesignWare MIPI DSI v1.01
> + IP block with a companion PHY IP.
> +
> + These DT bindings follow the Synopsys DW MIPI DSI bindings defined in
> + Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt with
> + the following device-specific properties.
> +
> +allOf:
> + - $ref: ../bridge/snps,dw-mipi-dsi.yaml#
> +
> +properties:
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + compatible:
> + items:
> + - const: fsl,imx6q-mipi-dsi
> + - const: snps,dw-mipi-dsi
This schema is going to be applied on any node with 'snps,dw-mipi-dsi'.
You'll need a custom 'select' with only 'fsl,imx6q-mipi-dsi'. There's a
few examples in the tree.
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + items:
> + - description: Module Clock
> + - description: DSI bus clock
> +
> + clock-names:
> + items:
> + - const: ref
> + - const: pclk
> +
> + fsl,gpr:
> + description:
> + Phandle to the iomuxc-gpr region containing the multiplexer ctrl register.
> + $ref: /schemas/types.yaml#/definitions/phandle
> +
> + ports:
> + type: object
> + description: |
> + A node containing DSI input & output port nodes with endpoint
> + definitions as documented in
> + Documentation/devicetree/bindings/media/video-interfaces.txt
> + Documentation/devicetree/bindings/graph.txt
> + properties:
> + port@0:
> + type: object
> + description:
> + DSI input port connected to a parallel RGB LTDC output port.
> +
> + port@1:
> + type: object
> + description:
> + DSI serial RGB output port connected to a panel or bridge input port.
> +
> + required:
> + - port@0
> + - port@1
> +
> +additionalProperties: false
When including other schemas, you need 'unevalatedProperties: false'
instead. Then you can drop anything here that doesn't have more
constraints like the next property:
> +
> +patternProperties:
> + "^panel@[0-3]$":
> + type: object
> +
> +required:
> + - "#address-cells"
> + - "#size-cells"
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> + - ports
> +
> +examples:
> + - |+
> + #include <dt-bindings/clock/imx6qdl-clock.h>
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + dsi: dsi@21e0000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "fsl,imx6q-mipi-dsi", "snps,dw-mipi-dsi";
> + reg = <0x021e0000 0x4000>;
> + interrupts = <0 102 IRQ_TYPE_LEVEL_HIGH>;
> + fsl,gpr = <&gpr>;
> + clocks = <&clks IMX6QDL_CLK_MIPI_CORE_CFG>,
> + <&clks IMX6QDL_CLK_MIPI_IPG>;
> + clock-names = "ref", "pclk";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + mipi_mux_0: endpoint {
> + remote-endpoint = <&ipu1_di0_mipi>;
> + };
> + };
> + port@1 {
> + reg = <1>;
> + dsi_out: endpoint {
> + remote-endpoint = <&panel_in>;
> + };
> + };
> + };
> +
> + panel@0 {
> + compatible = "sharp,ls032b3sx01";
> + reg = <0>;
> + reset-gpios = <&gpio6 8 GPIO_ACTIVE_LOW>;
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + panel_in: endpoint {
> + remote-endpoint = <&dsi_out>;
> + };
> + };
> + };
> + };
> + };
> +
> +...
> --
> 2.26.0
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v8 08/10] drm: stm: dw-mipi-dsi: let the bridge handle the HW version check
2020-04-27 8:19 [PATCH v8 00/10] Genericize DW MIPI DSI bridge and add i.MX 6 driver Adrian Ratiu
` (6 preceding siblings ...)
2020-04-27 8:19 ` [PATCH v8 07/10] dt-bindings: display: add i.MX6 MIPI DSI host controller doc Adrian Ratiu
@ 2020-04-27 8:19 ` Adrian Ratiu
2020-05-29 15:45 ` [Linux-stm32] " Philippe CORNU
2020-04-27 8:19 ` [PATCH v8 09/10] drm: bridge: dw-mipi-dsi: split low power cfg register into fields Adrian Ratiu
2020-04-27 8:19 ` [PATCH v8 10/10] drm: bridge: dw-mipi-dsi: fix bad register field offsets Adrian Ratiu
9 siblings, 1 reply; 21+ messages in thread
From: Adrian Ratiu @ 2020-04-27 8:19 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, linux-rockchip, Laurent Pinchart
Cc: Jernej Skrabec, Adrian Pop, Jonas Karlman, linux-kernel,
dri-devel, Andrzej Hajda, linux-imx, kernel, linux-stm32,
Arnaud Ferraris
The stm mipi-dsi platform driver added a version test in
commit fa6251a747b7 ("drm/stm: dsi: check hardware version")
so that HW revisions other than v1.3x get rejected. The rockchip
driver had no such check and just assumed register layouts are
v1.3x compatible.
Having such tests was a good idea because only v130/v131 layouts
were supported at the time, however since adding multiple layout
support in the bridge, the version is automatically checked for
all drivers, compatible layouts get picked and unsupported HW is
automatically rejected by the bridge, so there's no use keeping
the test in the stm driver.
The main reason prompting this change is that the stm driver
test immediately disabled the peripheral clock after reading
the version, making the bridge read version 0x0 immediately
after in its own probe(), so we move the clock disabling after
the bridge does the version test.
Tested on STM32F769 and STM32MP1.
Cc: linux-stm32@st-md-mailman.stormreply.com
Reported-by: Adrian Pop <pop.adrian61@gmail.com>
Tested-by: Adrian Pop <pop.adrian61@gmail.com>
Tested-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
New in v6.
---
drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
index 2e1f2664495d0..7218e405d7e2b 100644
--- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
+++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
@@ -402,15 +402,6 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev)
goto err_dsi_probe;
}
- dsi->hw_version = dsi_read(dsi, DSI_VERSION) & VERSION;
- clk_disable_unprepare(pclk);
-
- if (dsi->hw_version != HWVER_130 && dsi->hw_version != HWVER_131) {
- ret = -ENODEV;
- DRM_ERROR("bad dsi hardware version\n");
- goto err_dsi_probe;
- }
-
dw_mipi_dsi_stm_plat_data.base = dsi->base;
dw_mipi_dsi_stm_plat_data.priv_data = dsi;
@@ -423,6 +414,9 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev)
goto err_dsi_probe;
}
+ dsi->hw_version = dsi_read(dsi, DSI_VERSION) & VERSION;
+ clk_disable_unprepare(pclk);
+
return 0;
err_dsi_probe:
--
2.26.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Linux-stm32] [PATCH v8 08/10] drm: stm: dw-mipi-dsi: let the bridge handle the HW version check
2020-04-27 8:19 ` [PATCH v8 08/10] drm: stm: dw-mipi-dsi: let the bridge handle the HW version check Adrian Ratiu
@ 2020-05-29 15:45 ` Philippe CORNU
2020-06-01 9:15 ` Adrian Ratiu
0 siblings, 1 reply; 21+ messages in thread
From: Philippe CORNU @ 2020-05-29 15:45 UTC (permalink / raw)
To: Adrian Ratiu, linux-arm-kernel, devicetree, linux-rockchip,
Laurent Pinchart
Cc: Jernej Skrabec, Adrian Pop, Jonas Karlman, linux-kernel,
dri-devel, Yannick FERTRE, Andrzej Hajda, linux-imx, kernel,
linux-stm32, Arnaud Ferraris, Benjamin GAIGNARD
Hi Adrian,
and thank you very much for the patchset.
Thank you also for having tested it on STM32F769 and STM32MP1.
Sorry for the late response, Yannick and I will review it as soon as
possible and we will keep you posted.
Note: Do not hesitate to put us in copy for the next version
(philippe.cornu@st.com, yannick.fertre@st.com)
Regards,
Philippe :-)
On 4/27/20 10:19 AM, Adrian Ratiu wrote:
> The stm mipi-dsi platform driver added a version test in
> commit fa6251a747b7 ("drm/stm: dsi: check hardware version")
> so that HW revisions other than v1.3x get rejected. The rockchip
> driver had no such check and just assumed register layouts are
> v1.3x compatible.
>
> Having such tests was a good idea because only v130/v131 layouts
> were supported at the time, however since adding multiple layout
> support in the bridge, the version is automatically checked for
> all drivers, compatible layouts get picked and unsupported HW is
> automatically rejected by the bridge, so there's no use keeping
> the test in the stm driver.
>
> The main reason prompting this change is that the stm driver
> test immediately disabled the peripheral clock after reading
> the version, making the bridge read version 0x0 immediately
> after in its own probe(), so we move the clock disabling after
> the bridge does the version test.
>
> Tested on STM32F769 and STM32MP1.
>
> Cc: linux-stm32@st-md-mailman.stormreply.com
> Reported-by: Adrian Pop <pop.adrian61@gmail.com>
> Tested-by: Adrian Pop <pop.adrian61@gmail.com>
> Tested-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> ---
> New in v6.
> ---
> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> index 2e1f2664495d0..7218e405d7e2b 100644
> --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> @@ -402,15 +402,6 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev)
> goto err_dsi_probe;
> }
>
> - dsi->hw_version = dsi_read(dsi, DSI_VERSION) & VERSION;
> - clk_disable_unprepare(pclk);
> -
> - if (dsi->hw_version != HWVER_130 && dsi->hw_version != HWVER_131) {
> - ret = -ENODEV;
> - DRM_ERROR("bad dsi hardware version\n");
> - goto err_dsi_probe;
> - }
> -
> dw_mipi_dsi_stm_plat_data.base = dsi->base;
> dw_mipi_dsi_stm_plat_data.priv_data = dsi;
>
> @@ -423,6 +414,9 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev)
> goto err_dsi_probe;
> }
>
> + dsi->hw_version = dsi_read(dsi, DSI_VERSION) & VERSION;
> + clk_disable_unprepare(pclk);
> +
> return 0;
>
> err_dsi_probe:
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Linux-stm32] [PATCH v8 08/10] drm: stm: dw-mipi-dsi: let the bridge handle the HW version check
2020-05-29 15:45 ` [Linux-stm32] " Philippe CORNU
@ 2020-06-01 9:15 ` Adrian Ratiu
2020-06-02 12:53 ` Emil Velikov
0 siblings, 1 reply; 21+ messages in thread
From: Adrian Ratiu @ 2020-06-01 9:15 UTC (permalink / raw)
To: Philippe CORNU, Adrian Ratiu, linux-arm-kernel, devicetree,
linux-rockchip, Laurent Pinchart
Cc: Jernej Skrabec, Adrian Pop, Jonas Karlman, linux-kernel,
dri-devel, Yannick FERTRE, Andrzej Hajda, linux-imx, kernel,
linux-stm32, Arnaud Ferraris, Benjamin GAIGNARD
On Fri, 29 May 2020, Philippe CORNU <philippe.cornu@st.com> wrote:
> Hi Adrian, and thank you very much for the patchset. Thank you
> also for having tested it on STM32F769 and STM32MP1. Sorry for
> the late response, Yannick and I will review it as soon as
> possible and we will keep you posted. Note: Do not hesitate to
> put us in copy for the next version (philippe.cornu@st.com,
> yannick.fertre@st.com) Regards, Philippe :-)
Hi Philippe,
Thank you very much for your previous and future STM testing,
really appreciate it! I've CC'd Yannick until now but I'll also CC
you sure :)
It's been over a month since I posted v8 and I was just gearing up
to address all feedback, rebase & retest to prepare v9 but I'll
wait a little longer, no problem, it's no rush.
Have an awesome day,
Adrian
>
>
> On 4/27/20 10:19 AM, Adrian Ratiu wrote:
>> The stm mipi-dsi platform driver added a version test in
>> commit fa6251a747b7 ("drm/stm: dsi: check hardware version")
>> so that HW revisions other than v1.3x get rejected. The rockchip
>> driver had no such check and just assumed register layouts are
>> v1.3x compatible.
>>
>> Having such tests was a good idea because only v130/v131 layouts
>> were supported at the time, however since adding multiple layout
>> support in the bridge, the version is automatically checked for
>> all drivers, compatible layouts get picked and unsupported HW is
>> automatically rejected by the bridge, so there's no use keeping
>> the test in the stm driver.
>>
>> The main reason prompting this change is that the stm driver
>> test immediately disabled the peripheral clock after reading
>> the version, making the bridge read version 0x0 immediately
>> after in its own probe(), so we move the clock disabling after
>> the bridge does the version test.
>>
>> Tested on STM32F769 and STM32MP1.
>>
>> Cc: linux-stm32@st-md-mailman.stormreply.com
>> Reported-by: Adrian Pop <pop.adrian61@gmail.com>
>> Tested-by: Adrian Pop <pop.adrian61@gmail.com>
>> Tested-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
>> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
>> ---
>> New in v6.
>> ---
>> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 12 +++---------
>> 1 file changed, 3 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
>> index 2e1f2664495d0..7218e405d7e2b 100644
>> --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
>> +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
>> @@ -402,15 +402,6 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev)
>> goto err_dsi_probe;
>> }
>>
>> - dsi->hw_version = dsi_read(dsi, DSI_VERSION) & VERSION;
>> - clk_disable_unprepare(pclk);
>> -
>> - if (dsi->hw_version != HWVER_130 && dsi->hw_version != HWVER_131) {
>> - ret = -ENODEV;
>> - DRM_ERROR("bad dsi hardware version\n");
>> - goto err_dsi_probe;
>> - }
>> -
>> dw_mipi_dsi_stm_plat_data.base = dsi->base;
>> dw_mipi_dsi_stm_plat_data.priv_data = dsi;
>>
>> @@ -423,6 +414,9 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev)
>> goto err_dsi_probe;
>> }
>>
>> + dsi->hw_version = dsi_read(dsi, DSI_VERSION) & VERSION;
>> + clk_disable_unprepare(pclk);
>> +
>> return 0;
>>
>> err_dsi_probe:
>>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Linux-stm32] [PATCH v8 08/10] drm: stm: dw-mipi-dsi: let the bridge handle the HW version check
2020-06-01 9:15 ` Adrian Ratiu
@ 2020-06-02 12:53 ` Emil Velikov
2020-06-03 10:28 ` Adrian Ratiu
0 siblings, 1 reply; 21+ messages in thread
From: Emil Velikov @ 2020-06-02 12:53 UTC (permalink / raw)
To: Adrian Ratiu
Cc: devicetree, Jernej Skrabec, Benjamin GAIGNARD, Adrian Pop,
Jonas Karlman, Philippe CORNU, dri-devel, linux-kernel,
Yannick FERTRE, linux-rockchip, Andrzej Hajda, Laurent Pinchart,
Arnaud Ferraris, kernel, linux-stm32, linux-arm-kernel,
linux-imx
Hi Adrian,
On Mon, 1 Jun 2020 at 10:14, Adrian Ratiu <adrian.ratiu@collabora.com> wrote:
>
> On Fri, 29 May 2020, Philippe CORNU <philippe.cornu@st.com> wrote:
> > Hi Adrian, and thank you very much for the patchset. Thank you
> > also for having tested it on STM32F769 and STM32MP1. Sorry for
> > the late response, Yannick and I will review it as soon as
> > possible and we will keep you posted. Note: Do not hesitate to
> > put us in copy for the next version (philippe.cornu@st.com,
> > yannick.fertre@st.com) Regards, Philippe :-)
>
> Hi Philippe,
>
> Thank you very much for your previous and future STM testing,
> really appreciate it! I've CC'd Yannick until now but I'll also CC
> you sure :)
>
> It's been over a month since I posted v8 and I was just gearing up
> to address all feedback, rebase & retest to prepare v9 but I'll
> wait a little longer, no problem, it's no rush.
>
Small idea, pardon for joining so late:
Might be a good idea to add inline comment, why the clocks are disabled so late.
Effectively a 2 line version of the commit summary.
Feel free to make that a separate/follow-up patch.
-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Linux-stm32] [PATCH v8 08/10] drm: stm: dw-mipi-dsi: let the bridge handle the HW version check
2020-06-02 12:53 ` Emil Velikov
@ 2020-06-03 10:28 ` Adrian Ratiu
0 siblings, 0 replies; 21+ messages in thread
From: Adrian Ratiu @ 2020-06-03 10:28 UTC (permalink / raw)
To: Emil Velikov, Adrian Ratiu
Cc: linux-arm-kernel, devicetree, Jernej Skrabec, Benjamin GAIGNARD,
Adrian Pop, Jonas Karlman, Philippe CORNU, dri-devel,
linux-kernel, Yannick FERTRE, linux-rockchip, Andrzej Hajda,
Laurent Pinchart, kernel, linux-stm32, Arnaud Ferraris,
linux-imx
On Tue, 02 Jun 2020, Emil Velikov <emil.l.velikov@gmail.com>
wrote:
> Hi Adrian,
Hi Email,
>
> On Mon, 1 Jun 2020 at 10:14, Adrian Ratiu
> <adrian.ratiu@collabora.com> wrote:
>>
>> On Fri, 29 May 2020, Philippe CORNU <philippe.cornu@st.com>
>> wrote:
>> > Hi Adrian, and thank you very much for the patchset. Thank
>> > you also for having tested it on STM32F769 and STM32MP1.
>> > Sorry for the late response, Yannick and I will review it as
>> > soon as possible and we will keep you posted. Note: Do not
>> > hesitate to put us in copy for the next version
>> > (philippe.cornu@st.com, yannick.fertre@st.com) Regards,
>> > Philippe :-)
>>
>> Hi Philippe,
>>
>> Thank you very much for your previous and future STM testing,
>> really appreciate it! I've CC'd Yannick until now but I'll also
>> CC you sure :)
>>
>> It's been over a month since I posted v8 and I was just gearing
>> up to address all feedback, rebase & retest to prepare v9 but
>> I'll wait a little longer, no problem, it's no rush.
>>
> Small idea, pardon for joining so late:
>
> Might be a good idea to add inline comment, why the clocks are
> disabled so late. Effectively a 2 line version of the commit
> summary.
>
> Feel free to make that a separate/follow-up patch.
Thanks, I'll add the comment to this patch in v9.
>
> -Emil
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v8 09/10] drm: bridge: dw-mipi-dsi: split low power cfg register into fields
2020-04-27 8:19 [PATCH v8 00/10] Genericize DW MIPI DSI bridge and add i.MX 6 driver Adrian Ratiu
` (7 preceding siblings ...)
2020-04-27 8:19 ` [PATCH v8 08/10] drm: stm: dw-mipi-dsi: let the bridge handle the HW version check Adrian Ratiu
@ 2020-04-27 8:19 ` Adrian Ratiu
2020-04-27 8:19 ` [PATCH v8 10/10] drm: bridge: dw-mipi-dsi: fix bad register field offsets Adrian Ratiu
9 siblings, 0 replies; 21+ messages in thread
From: Adrian Ratiu @ 2020-04-27 8:19 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, linux-rockchip, Laurent Pinchart
Cc: Jernej Skrabec, Adrian Pop, Jonas Karlman, linux-kernel,
dri-devel, Andrzej Hajda, linux-imx, kernel, linux-stm32,
Arnaud Ferraris
According to the Host Registers documentation for IMX, STM and RK
the LP cfg register should not be written entirely in one go because
some bits are reserved and should be kept to reset values, for eg.
BIT(15) which is reserved in all versions.
This also cleans up the code by removing the the ugly defines
and making field ranges & values written more explicit.
Tested-by: Adrian Pop <pop.adrian61@gmail.com>
Tested-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
New in v6.
---
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 105 ++++++------------
1 file changed, 33 insertions(+), 72 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index 140ff40fa1b62..0903ec37289dd 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -120,60 +120,6 @@
#define DSI_TO_CNT_CFG_V101 0x40
#define DSI_PCKHDL_CFG_V101 0x18
-#define MAX_RD_PKT_SIZE_LP BIT(24)
-#define DCS_LW_TX_LP BIT(19)
-#define DCS_SR_0P_TX_LP BIT(18)
-#define DCS_SW_1P_TX_LP BIT(17)
-#define DCS_SW_0P_TX_LP BIT(16)
-#define GEN_LW_TX_LP BIT(14)
-#define GEN_SR_2P_TX_LP BIT(13)
-#define GEN_SR_1P_TX_LP BIT(12)
-#define GEN_SR_0P_TX_LP BIT(11)
-#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 TEAR_FX_EN BIT(0)
-
-#define CMD_MODE_ALL_LP (MAX_RD_PKT_SIZE_LP | \
- DCS_LW_TX_LP | \
- DCS_SR_0P_TX_LP | \
- DCS_SW_1P_TX_LP | \
- DCS_SW_0P_TX_LP | \
- GEN_LW_TX_LP | \
- GEN_SR_2P_TX_LP | \
- GEN_SR_1P_TX_LP | \
- GEN_SR_0P_TX_LP | \
- GEN_SW_2P_TX_LP | \
- GEN_SW_1P_TX_LP | \
- GEN_SW_0P_TX_LP)
-
-#define EN_TEAR_FX_V101 BIT(14)
-#define DCS_LW_TX_LP_V101 BIT(12)
-#define GEN_LW_TX_LP_V101 BIT(11)
-#define MAX_RD_PKT_SIZE_LP_V101 BIT(10)
-#define DCS_SW_2P_TX_LP_V101 BIT(9)
-#define DCS_SW_1P_TX_LP_V101 BIT(8)
-#define DCS_SW_0P_TX_LP_V101 BIT(7)
-#define GEN_SR_2P_TX_LP_V101 BIT(6)
-#define GEN_SR_1P_TX_LP_V101 BIT(5)
-#define GEN_SR_0P_TX_LP_V101 BIT(4)
-#define GEN_SW_2P_TX_LP_V101 BIT(3)
-#define GEN_SW_1P_TX_LP_V101 BIT(2)
-#define GEN_SW_0P_TX_LP_V101 BIT(1)
-
-#define CMD_MODE_ALL_LP_V101 (DCS_LW_TX_LP_V101 | \
- GEN_LW_TX_LP_V101 | \
- MAX_RD_PKT_SIZE_LP_V101 | \
- DCS_SW_2P_TX_LP_V101 | \
- DCS_SW_1P_TX_LP_V101 | \
- DCS_SW_0P_TX_LP_V101 | \
- GEN_SR_2P_TX_LP_V101 | \
- GEN_SR_1P_TX_LP_V101 | \
- GEN_SR_0P_TX_LP_V101 | \
- GEN_SW_2P_TX_LP_V101 | \
- GEN_SW_1P_TX_LP_V101 | \
- GEN_SW_0P_TX_LP_V101)
-
#define DSI_GEN_HDR 0x6c
#define DSI_GEN_PLD_DATA 0x70
@@ -257,7 +203,11 @@ struct dw_mipi_dsi {
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_gen_sw_sr_en;
+ struct regmap_field *field_cmd_mode_dcs_sw_sr_en;
+ struct regmap_field *field_cmd_mode_gen_lw_en;
+ struct regmap_field *field_cmd_mode_dcs_lw_en;
+ struct regmap_field *field_cmd_mode_max_rd_pkt_size;
struct regmap_field *field_cmd_mode_en;
struct regmap_field *field_cmd_pkt_status;
struct regmap_field *field_vid_mode_en;
@@ -315,7 +265,11 @@ struct dw_mipi_dsi_variant {
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_mode_gen_sw_sr_en;
+ struct reg_field cfg_cmd_mode_dcs_sw_sr_en;
+ struct reg_field cfg_cmd_mode_gen_lw_en;
+ struct reg_field cfg_cmd_mode_dcs_lw_en;
+ struct reg_field cfg_cmd_mode_max_rd_pkt_size;
struct reg_field cfg_cmd_pkt_status;
struct reg_field cfg_vid_mode_en;
struct reg_field cfg_vid_mode_type;
@@ -366,7 +320,11 @@ static const struct dw_mipi_dsi_variant dw_mipi_dsi_v130_v131_layout = {
.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_gen_sw_sr_en = REG_FIELD(DSI_CMD_MODE_CFG, 8, 13),
+ .cfg_cmd_mode_gen_lw_en = REG_FIELD(DSI_CMD_MODE_CFG, 14, 14),
+ .cfg_cmd_mode_dcs_sw_sr_en = REG_FIELD(DSI_CMD_MODE_CFG, 16, 18),
+ .cfg_cmd_mode_dcs_lw_en = REG_FIELD(DSI_CMD_MODE_CFG, 19, 19),
+ .cfg_cmd_mode_max_rd_pkt_size = REG_FIELD(DSI_CMD_MODE_CFG, 24, 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),
@@ -418,7 +376,11 @@ static const struct dw_mipi_dsi_variant dw_mipi_dsi_v101_layout = {
.cfg_dpi_vsync_active_low = REG_FIELD(DSI_DPI_CFG, 6, 6),
.cfg_dpi_hsync_active_low = REG_FIELD(DSI_DPI_CFG, 7, 7),
.cfg_cmd_mode_en = REG_FIELD(DSI_CMD_MODE_CFG_V101, 0, 0),
- .cfg_cmd_mode_all_lp_en = REG_FIELD(DSI_CMD_MODE_CFG_V101, 1, 12),
+ .cfg_cmd_mode_gen_sw_sr_en = REG_FIELD(DSI_CMD_MODE_CFG, 1, 6),
+ .cfg_cmd_mode_dcs_sw_sr_en = REG_FIELD(DSI_CMD_MODE_CFG, 7, 9),
+ .cfg_cmd_mode_max_rd_pkt_size = REG_FIELD(DSI_CMD_MODE_CFG, 10, 10),
+ .cfg_cmd_mode_gen_lw_en = REG_FIELD(DSI_CMD_MODE_CFG, 11, 11),
+ .cfg_cmd_mode_dcs_lw_en = REG_FIELD(DSI_CMD_MODE_CFG, 12, 12),
.cfg_cmd_mode_ack_rqst_en = REG_FIELD(DSI_CMD_MODE_CFG_V101, 13, 13),
.cfg_cmd_pkt_status = REG_FIELD(DSI_CMD_PKT_STATUS_V101, 0, 14),
.cfg_vid_mode_en = REG_FIELD(DSI_VID_MODE_CFG_V101, 0, 0),
@@ -554,23 +516,18 @@ 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 cmd_mode_lp = 0;
-
- switch (dsi->hw_version) {
- case HWVER_130:
- case HWVER_131:
- cmd_mode_lp = CMD_MODE_ALL_LP;
- break;
- case HWVER_101:
- cmd_mode_lp = CMD_MODE_ALL_LP_V101;
- break;
- }
if (msg->flags & MIPI_DSI_MSG_REQ_ACK)
regmap_field_write(dsi->field_cmd_mode_ack_rqst_en, 1);
- if (lpm)
- regmap_field_write(dsi->field_cmd_mode_all_lp_en, cmd_mode_lp);
+ if (lpm) {
+ regmap_field_write(dsi->field_cmd_mode_gen_sw_sr_en,
+ ENABLE_LOW_POWER);
+ regmap_field_write(dsi->field_cmd_mode_dcs_sw_sr_en, 7);
+ regmap_field_write(dsi->field_cmd_mode_gen_lw_en, 1);
+ regmap_field_write(dsi->field_cmd_mode_dcs_lw_en, 1);
+ regmap_field_write(dsi->field_cmd_mode_max_rd_pkt_size, 1);
+ }
regmap_field_write(dsi->field_phy_txrequestclkhs, lpm ? 0 : 1);
}
@@ -1256,7 +1213,11 @@ static int dw_mipi_dsi_regmap_fields_init(struct dw_mipi_dsi *dsi)
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_gen_sw_sr_en);
+ INIT_FIELD(cmd_mode_dcs_sw_sr_en);
+ INIT_FIELD(cmd_mode_gen_lw_en);
+ INIT_FIELD(cmd_mode_dcs_lw_en);
+ INIT_FIELD(cmd_mode_max_rd_pkt_size);
INIT_FIELD(cmd_mode_en);
INIT_FIELD(cmd_pkt_status);
INIT_FIELD(vid_mode_en);
--
2.26.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v8 10/10] drm: bridge: dw-mipi-dsi: fix bad register field offsets
2020-04-27 8:19 [PATCH v8 00/10] Genericize DW MIPI DSI bridge and add i.MX 6 driver Adrian Ratiu
` (8 preceding siblings ...)
2020-04-27 8:19 ` [PATCH v8 09/10] drm: bridge: dw-mipi-dsi: split low power cfg register into fields Adrian Ratiu
@ 2020-04-27 8:19 ` Adrian Ratiu
9 siblings, 0 replies; 21+ messages in thread
From: Adrian Ratiu @ 2020-04-27 8:19 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, linux-rockchip, Laurent Pinchart
Cc: Jernej Skrabec, Adrian Pop, Jonas Karlman, linux-kernel,
dri-devel, Andrzej Hajda, linux-imx, kernel, linux-stm32,
Arnaud Ferraris
According to the DSI Host Registers sections available in the IMX,
STM and RK ref manuals for 1.01, 1.30 and 1.31, the register fields
are smaller or bigger than what's coded in the driver, leading to
r/w in reserved spaces which might cause undefined behaviours.
Tested-by: Adrian Pop <pop.adrian61@gmail.com>
Tested-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
New in v6.
---
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 46 +++++++++----------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index 0903ec37289dd..bf22b04761fdf 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -316,7 +316,7 @@ struct dw_mipi_dsi_variant {
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_vid = REG_FIELD(DSI_DPI_VCID, 0, 1),
.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),
@@ -325,29 +325,29 @@ static const struct dw_mipi_dsi_variant dw_mipi_dsi_v130_v131_layout = {
.cfg_cmd_mode_dcs_sw_sr_en = REG_FIELD(DSI_CMD_MODE_CFG, 16, 18),
.cfg_cmd_mode_dcs_lw_en = REG_FIELD(DSI_CMD_MODE_CFG, 19, 19),
.cfg_cmd_mode_max_rd_pkt_size = REG_FIELD(DSI_CMD_MODE_CFG, 24, 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_cmd_mode_en = REG_FIELD(DSI_MODE_CFG, 0, 0),
+ .cfg_cmd_pkt_status = REG_FIELD(DSI_CMD_PKT_STATUS, 0, 6),
+ .cfg_vid_mode_en = REG_FIELD(DSI_MODE_CFG, 0, 0),
.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_vid_pkt_size = REG_FIELD(DSI_VID_PKT_SIZE, 0, 13),
+ .cfg_vid_hsa_time = REG_FIELD(DSI_VID_HSA_TIME, 0, 11),
+ .cfg_vid_hbp_time = REG_FIELD(DSI_VID_HBP_TIME, 0, 11),
+ .cfg_vid_hline_time = REG_FIELD(DSI_VID_HLINE_TIME, 0, 14),
+ .cfg_vid_vsa_time = REG_FIELD(DSI_VID_VSA_LINES, 0, 9),
+ .cfg_vid_vbp_time = REG_FIELD(DSI_VID_VBP_LINES, 0, 9),
+ .cfg_vid_vfp_time = REG_FIELD(DSI_VID_VFP_LINES, 0, 9),
+ .cfg_vid_vactive_time = REG_FIELD(DSI_VID_VACTIVE_LINES, 0, 13),
.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_bta_time = REG_FIELD(DSI_BTA_TO_CNT, 0, 15),
+ .cfg_phy_max_rd_time = REG_FIELD(DSI_PHY_TMR_CFG, 0, 14),
.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_max_rd_time_v131 = REG_FIELD(DSI_PHY_TMR_RD_CFG, 0, 14),
+ .cfg_phy_lp2hs_time_v131 = REG_FIELD(DSI_PHY_TMR_CFG, 0, 9),
+ .cfg_phy_hs2lp_time_v131 = REG_FIELD(DSI_PHY_TMR_CFG, 16, 25),
.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),
@@ -361,11 +361,11 @@ static const struct dw_mipi_dsi_variant dw_mipi_dsi_v130_v131_layout = {
.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_int_stat0 = REG_FIELD(DSI_INT_ST0, 0, 20),
+ .cfg_int_stat1 = REG_FIELD(DSI_INT_ST1, 0, 12),
+ .cfg_int_mask0 = REG_FIELD(DSI_INT_MSK0, 0, 20),
+ .cfg_int_mask1 = REG_FIELD(DSI_INT_MSK1, 0, 12),
+ .cfg_gen_hdr = REG_FIELD(DSI_GEN_HDR, 0, 23),
.cfg_gen_payload = REG_FIELD(DSI_GEN_PLD_DATA, 0, 31),
};
@@ -382,7 +382,7 @@ static const struct dw_mipi_dsi_variant dw_mipi_dsi_v101_layout = {
.cfg_cmd_mode_gen_lw_en = REG_FIELD(DSI_CMD_MODE_CFG, 11, 11),
.cfg_cmd_mode_dcs_lw_en = REG_FIELD(DSI_CMD_MODE_CFG, 12, 12),
.cfg_cmd_mode_ack_rqst_en = REG_FIELD(DSI_CMD_MODE_CFG_V101, 13, 13),
- .cfg_cmd_pkt_status = REG_FIELD(DSI_CMD_PKT_STATUS_V101, 0, 14),
+ .cfg_cmd_pkt_status = REG_FIELD(DSI_CMD_PKT_STATUS_V101, 0, 6),
.cfg_vid_mode_en = REG_FIELD(DSI_VID_MODE_CFG_V101, 0, 0),
.cfg_vid_mode_type = REG_FIELD(DSI_VID_MODE_CFG_V101, 1, 2),
.cfg_vid_mode_low_power = REG_FIELD(DSI_VID_MODE_CFG_V101, 3, 8),
--
2.26.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread