This version is mostly small changes in response to review comments from Sean and Chris, the details are in the individual patches.
I decided to drop the final patch which adds support for MIPI read commands because I'm not using that feature now and I can't easily test it. It's on the list if anyone wants to pick it up in the future.
Version 3 was posted here: http://www.spinics.net/lists/dri-devel/msg130977.html
Thanks to Sean Paul and Chris Zhong for their review and testing of this series.
John Keeping (23): drm/rockchip: dw-mipi-dsi: don't configure hardware in mode_set for MIPI drm/rockchip: dw-mipi-dsi: pass mode in where needed drm/rockchip: dw-mipi-dsi: remove mode_set hook drm/rockchip: dw-mipi-dsi: fix command header writes drm/rockchip: dw-mipi-dsi: fix generic packet status check drm/rockchip: dw-mipi-dsi: avoid out-of-bounds read on tx_buf drm/rockchip: dw-mipi-dsi: include bad value in error message drm/rockchip: dw-mipi-dsi: respect message flags drm/rockchip: dw-mipi-dsi: only request HS clock when required drm/rockchip: dw-mipi-dsi: don't assume buffer is aligned drm/rockchip: dw-mipi-dsi: prepare panel after phy init drm/rockchip: dw-mipi-dsi: allow commands in panel_disable drm/rockchip: dw-mipi-dsi: fix escape clock rate drm/rockchip: dw-mipi-dsi: ensure PHY is reset drm/rockchip: dw-mipi-dsi: configure PHY before enabling drm/rockchip: dw-mipi-dsi: properly configure PHY timing drm/rockchip: dw-mipi-dsi: improve PLL configuration drm/rockchip: dw-mipi-dsi: use specific poll helper drm/rockchip: dw-mipi-dsi: use positive check for N{H,V}SYNC drm/rockchip: vop: test for P{H,V}SYNC drm/rockchip: dw-mipi-dsi: defer probe if panel is not loaded drm/rockchip: dw-mipi-dsi: support non-burst modes drm/rockchip: dw-mipi-dsi: add reset control
drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 325 +++++++++++++++++++--------- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 +- 2 files changed, 220 insertions(+), 109 deletions(-)
With atomic modesetting the hardware will be powered off when the mode_set function is called. We should configure the hardware in the enable function, which is the atomic version of "commit" so let's use the enable hook rather than commit while we're at it.
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Sean Paul seanpaul@chromium.org --- v4: - Add Sean's Reviewed-by v3: - Squash together with the commit to s/commit/enable/ Unchanged in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 49 +++++++++++++++------------------- 1 file changed, 21 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index d9aa382bb629..bbd992299f73 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -819,34 +819,8 @@ static void dw_mipi_dsi_encoder_mode_set(struct drm_encoder *encoder, struct drm_display_mode *adjusted_mode) { struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder); - int ret;
dsi->mode = adjusted_mode; - - ret = dw_mipi_dsi_get_lane_bps(dsi); - if (ret < 0) - return; - - if (clk_prepare_enable(dsi->pclk)) { - dev_err(dsi->dev, "%s: Failed to enable pclk\n", __func__); - return; - } - - dw_mipi_dsi_init(dsi); - dw_mipi_dsi_dpi_config(dsi, mode); - dw_mipi_dsi_packet_handler_config(dsi); - dw_mipi_dsi_video_mode_config(dsi); - dw_mipi_dsi_video_packet_config(dsi, mode); - dw_mipi_dsi_command_mode_config(dsi); - dw_mipi_dsi_line_timer_config(dsi); - dw_mipi_dsi_vertical_timing_config(dsi); - dw_mipi_dsi_dphy_timing_config(dsi); - dw_mipi_dsi_dphy_interface_config(dsi); - dw_mipi_dsi_clear_err(dsi); - if (drm_panel_prepare(dsi->panel)) - dev_err(dsi->dev, "failed to prepare panel\n"); - - clk_disable_unprepare(dsi->pclk); }
static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder) @@ -875,17 +849,36 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder) clk_disable_unprepare(dsi->pclk); }
-static void dw_mipi_dsi_encoder_commit(struct drm_encoder *encoder) +static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder) { struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder); int mux = drm_of_encoder_active_endpoint_id(dsi->dev->of_node, encoder); u32 val; + int ret; + + ret = dw_mipi_dsi_get_lane_bps(dsi); + if (ret < 0) + return;
if (clk_prepare_enable(dsi->pclk)) { dev_err(dsi->dev, "%s: Failed to enable pclk\n", __func__); return; }
+ dw_mipi_dsi_init(dsi); + dw_mipi_dsi_dpi_config(dsi, dsi->mode); + dw_mipi_dsi_packet_handler_config(dsi); + dw_mipi_dsi_video_mode_config(dsi); + dw_mipi_dsi_video_packet_config(dsi, dsi->mode); + dw_mipi_dsi_command_mode_config(dsi); + dw_mipi_dsi_line_timer_config(dsi); + dw_mipi_dsi_vertical_timing_config(dsi); + dw_mipi_dsi_dphy_timing_config(dsi); + dw_mipi_dsi_dphy_interface_config(dsi); + dw_mipi_dsi_clear_err(dsi); + if (drm_panel_prepare(dsi->panel)) + dev_err(dsi->dev, "failed to prepare panel\n"); + dw_mipi_dsi_phy_init(dsi); dw_mipi_dsi_wait_for_two_frames(dsi);
@@ -933,7 +926,7 @@ dw_mipi_dsi_encoder_atomic_check(struct drm_encoder *encoder,
static struct drm_encoder_helper_funcs dw_mipi_dsi_encoder_helper_funcs = { - .commit = dw_mipi_dsi_encoder_commit, + .enable = dw_mipi_dsi_encoder_enable, .mode_set = dw_mipi_dsi_encoder_mode_set, .disable = dw_mipi_dsi_encoder_disable, .atomic_check = dw_mipi_dsi_encoder_atomic_check,
This shows that we only use the mode from the enable function and prepares us to remove the "mode" field and the mode_set hook in the next commit.
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Chris Zhong zyw@rock-chips.com Reviewed-by: Sean Paul seanpaul@chromium.org --- v4: - Add Sean's Reviewed-by v3: - Add Chris' Reviewed-by New in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 41 ++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index bbd992299f73..cdbd25087e83 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -330,11 +330,11 @@ static int max_mbps_to_testdin(unsigned int max_mbps) * The controller should generate 2 frames before * preparing the peripheral. */ -static void dw_mipi_dsi_wait_for_two_frames(struct dw_mipi_dsi *dsi) +static void dw_mipi_dsi_wait_for_two_frames(struct drm_display_mode *mode) { int refresh, two_frames;
- refresh = drm_mode_vrefresh(dsi->mode); + refresh = drm_mode_vrefresh(mode); two_frames = DIV_ROUND_UP(MSEC_PER_SEC, refresh) * 2; msleep(two_frames); } @@ -459,7 +459,8 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi) return ret; }
-static int dw_mipi_dsi_get_lane_bps(struct dw_mipi_dsi *dsi) +static int dw_mipi_dsi_get_lane_bps(struct dw_mipi_dsi *dsi, + struct drm_display_mode *mode) { unsigned int i, pre; unsigned long mpclk, pllref, tmp; @@ -474,7 +475,7 @@ static int dw_mipi_dsi_get_lane_bps(struct dw_mipi_dsi *dsi) return bpp; }
- mpclk = DIV_ROUND_UP(dsi->mode->clock, MSEC_PER_SEC); + mpclk = DIV_ROUND_UP(mode->clock, MSEC_PER_SEC); if (mpclk) { /* take 1 / 0.9, since mbps must big than bandwidth of RGB */ tmp = mpclk * (bpp / dsi->lanes) * 10 / 9; @@ -742,43 +743,44 @@ static void dw_mipi_dsi_command_mode_config(struct dw_mipi_dsi *dsi)
/* Get lane byte clock cycles. */ static u32 dw_mipi_dsi_get_hcomponent_lbcc(struct dw_mipi_dsi *dsi, + struct drm_display_mode *mode, u32 hcomponent) { u32 frac, lbcc;
lbcc = hcomponent * dsi->lane_mbps * MSEC_PER_SEC / 8;
- frac = lbcc % dsi->mode->clock; - lbcc = lbcc / dsi->mode->clock; + frac = lbcc % mode->clock; + lbcc = lbcc / mode->clock; if (frac) lbcc++;
return lbcc; }
-static void dw_mipi_dsi_line_timer_config(struct dw_mipi_dsi *dsi) +static void dw_mipi_dsi_line_timer_config(struct dw_mipi_dsi *dsi, + struct drm_display_mode *mode) { u32 htotal, hsa, hbp, lbcc; - struct drm_display_mode *mode = dsi->mode;
htotal = mode->htotal; hsa = mode->hsync_end - mode->hsync_start; hbp = mode->htotal - mode->hsync_end;
- lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, htotal); + lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, mode, htotal); dsi_write(dsi, DSI_VID_HLINE_TIME, lbcc);
- lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, hsa); + lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, mode, hsa); dsi_write(dsi, DSI_VID_HSA_TIME, lbcc);
- lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, hbp); + lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, mode, hbp); dsi_write(dsi, DSI_VID_HBP_TIME, lbcc); }
-static void dw_mipi_dsi_vertical_timing_config(struct dw_mipi_dsi *dsi) +static void dw_mipi_dsi_vertical_timing_config(struct dw_mipi_dsi *dsi, + struct drm_display_mode *mode) { u32 vactive, vsa, vfp, vbp; - struct drm_display_mode *mode = dsi->mode;
vactive = mode->vdisplay; vsa = mode->vsync_end - mode->vsync_start; @@ -852,11 +854,12 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder) static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder) { struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder); + struct drm_display_mode *mode = dsi->mode; int mux = drm_of_encoder_active_endpoint_id(dsi->dev->of_node, encoder); u32 val; int ret;
- ret = dw_mipi_dsi_get_lane_bps(dsi); + ret = dw_mipi_dsi_get_lane_bps(dsi, mode); if (ret < 0) return;
@@ -866,13 +869,13 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder) }
dw_mipi_dsi_init(dsi); - dw_mipi_dsi_dpi_config(dsi, dsi->mode); + dw_mipi_dsi_dpi_config(dsi, mode); dw_mipi_dsi_packet_handler_config(dsi); dw_mipi_dsi_video_mode_config(dsi); - dw_mipi_dsi_video_packet_config(dsi, dsi->mode); + dw_mipi_dsi_video_packet_config(dsi, mode); dw_mipi_dsi_command_mode_config(dsi); - dw_mipi_dsi_line_timer_config(dsi); - dw_mipi_dsi_vertical_timing_config(dsi); + dw_mipi_dsi_line_timer_config(dsi, mode); + dw_mipi_dsi_vertical_timing_config(dsi, mode); dw_mipi_dsi_dphy_timing_config(dsi); dw_mipi_dsi_dphy_interface_config(dsi); dw_mipi_dsi_clear_err(dsi); @@ -880,7 +883,7 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder) dev_err(dsi->dev, "failed to prepare panel\n");
dw_mipi_dsi_phy_init(dsi); - dw_mipi_dsi_wait_for_two_frames(dsi); + dw_mipi_dsi_wait_for_two_frames(mode);
dw_mipi_dsi_set_mode(dsi, DW_MIPI_DSI_VID_MODE); drm_panel_enable(dsi->panel);
This is not needed since we can access the mode via the CRTC from the enable hook. Also remove the "mode" field that is no longer used.
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Chris Zhong zyw@rock-chips.com Reviewed-by: Sean Paul seanpaul@chromium.org --- v4: - Add Sean's Reviewed-by v3: - Add Chris' Reviewed-by New in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index cdbd25087e83..bd92e58b64f3 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -286,7 +286,6 @@ struct dw_mipi_dsi { u32 format; u16 input_div; u16 feedback_div; - struct drm_display_mode *mode;
const struct dw_mipi_dsi_plat_data *pdata; }; @@ -816,15 +815,6 @@ static void dw_mipi_dsi_clear_err(struct dw_mipi_dsi *dsi) dsi_write(dsi, DSI_INT_MSK1, 0); }
-static void dw_mipi_dsi_encoder_mode_set(struct drm_encoder *encoder, - struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode) -{ - struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder); - - dsi->mode = adjusted_mode; -} - static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder) { struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder); @@ -854,7 +844,7 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder) static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder) { struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder); - struct drm_display_mode *mode = dsi->mode; + struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode; int mux = drm_of_encoder_active_endpoint_id(dsi->dev->of_node, encoder); u32 val; int ret; @@ -930,7 +920,6 @@ dw_mipi_dsi_encoder_atomic_check(struct drm_encoder *encoder, static struct drm_encoder_helper_funcs dw_mipi_dsi_encoder_helper_funcs = { .enable = dw_mipi_dsi_encoder_enable, - .mode_set = dw_mipi_dsi_encoder_mode_set, .disable = dw_mipi_dsi_encoder_disable, .atomic_check = dw_mipi_dsi_encoder_atomic_check, };
In a couple of places here we use "val" for the value that is about to be written to a register but then reuse the same variable for the value of a status register before we get around to writing it. Rename the value to be written to so that we write the value we intend to and not what we have just read from the status register.
Signed-off-by: John Keeping john@metanate.com Tested-by: Chris Zhong zyw@rock-chips.com Reviewed-by: Chris Zhong zyw@rock-chips.com Reviewed-by: Sean Paul seanpaul@chromium.org --- v4: - Add Sean's Reviewed-by Unchanged in v3 Unchanged in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index bd92e58b64f3..4cbbbcb619b7 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -542,9 +542,10 @@ static int dw_mipi_dsi_host_detach(struct mipi_dsi_host *host, return 0; }
-static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 val) +static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 hdr_val) { int ret; + u32 val;
ret = readx_poll_timeout(readl, dsi->base + DSI_CMD_PKT_STATUS, val, !(val & GEN_CMD_FULL), 1000, @@ -554,7 +555,7 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 val) return ret; }
- dsi_write(dsi, DSI_GEN_HDR, val); + dsi_write(dsi, DSI_GEN_HDR, hdr_val);
ret = readx_poll_timeout(readl, dsi->base + DSI_CMD_PKT_STATUS, val, val & (GEN_CMD_EMPTY | GEN_PLD_W_EMPTY), @@ -587,8 +588,9 @@ static int dw_mipi_dsi_dcs_long_write(struct dw_mipi_dsi *dsi, { const u32 *tx_buf = msg->tx_buf; int len = msg->tx_len, pld_data_bytes = sizeof(*tx_buf), ret; - u32 val = GEN_HDATA(msg->tx_len) | GEN_HTYPE(msg->type); + u32 hdr_val = GEN_HDATA(msg->tx_len) | GEN_HTYPE(msg->type); u32 remainder = 0; + u32 val;
if (msg->tx_len < 3) { dev_err(dsi->dev, "wrong tx buf length %zu for long write\n", @@ -617,7 +619,7 @@ static int dw_mipi_dsi_dcs_long_write(struct dw_mipi_dsi *dsi, } }
- return dw_mipi_dsi_gen_pkt_hdr_write(dsi, val); + return dw_mipi_dsi_gen_pkt_hdr_write(dsi, hdr_val); }
static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host,
We want to check that both the GEN_CMD_EMPTY and GEN_PLD_W_EMPTY bits are set so we can't just check "val & mask" because that will be true if either bit is set.
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Chris Zhong zyw@rock-chips.com Reviewed-by: Sean Paul seanpaul@chromium.org --- v4: - Add Sean's Reviewed-by v3: - Add Chris' Reviewed-by Unchanged in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 4cbbbcb619b7..4be1ff3a42bb 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -545,7 +545,7 @@ static int dw_mipi_dsi_host_detach(struct mipi_dsi_host *host, static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 hdr_val) { int ret; - u32 val; + u32 val, mask;
ret = readx_poll_timeout(readl, dsi->base + DSI_CMD_PKT_STATUS, val, !(val & GEN_CMD_FULL), 1000, @@ -557,8 +557,9 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 hdr_val)
dsi_write(dsi, DSI_GEN_HDR, hdr_val);
+ mask = GEN_CMD_EMPTY | GEN_PLD_W_EMPTY; ret = readx_poll_timeout(readl, dsi->base + DSI_CMD_PKT_STATUS, - val, val & (GEN_CMD_EMPTY | GEN_PLD_W_EMPTY), + val, (val & mask) == mask, 1000, CMD_PKT_STATUS_TIMEOUT_US); if (ret < 0) { dev_err(dsi->dev, "failed to write command FIFO\n");
As a side-effect of this, encode the endianness explicitly rather than casting a u16.
Signed-off-by: John Keeping john@metanate.com --- v4: - Introduce "data" variable to avoid confusion around the masking in GEN_HDATA() v3: - Add Chris' Reviewed-by Unchanged in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 4be1ff3a42bb..f55010312f25 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -572,8 +572,14 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 hdr_val) static int dw_mipi_dsi_dcs_short_write(struct dw_mipi_dsi *dsi, const struct mipi_dsi_msg *msg) { - const u16 *tx_buf = msg->tx_buf; - u32 val = GEN_HDATA(*tx_buf) | GEN_HTYPE(msg->type); + const u8 *tx_buf = msg->tx_buf; + u16 data = 0; + u32 val; + + if (msg->tx_len > 0) + data |= tx_buf[0]; + if (msg->tx_len > 1) + data |= tx_buf[1] << 8;
if (msg->tx_len > 2) { dev_err(dsi->dev, "too long tx buf length %zu for short write\n", @@ -581,6 +587,7 @@ static int dw_mipi_dsi_dcs_short_write(struct dw_mipi_dsi *dsi, return -EINVAL; }
+ val = GEN_HDATA(data) | GEN_HTYPE(msg->type); return dw_mipi_dsi_gen_pkt_hdr_write(dsi, val); }
As an aid to debugging.
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Chris Zhong zyw@rock-chips.com Reviewed-by: Sean Paul seanpaul@chromium.org --- v4: - Add Sean's Reviewed-by v3: - Add Chris' Reviewed-by Unchanged in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index f55010312f25..374b18c550fd 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -646,7 +646,8 @@ static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host, ret = dw_mipi_dsi_dcs_long_write(dsi, msg); break; default: - dev_err(dsi->dev, "unsupported message type\n"); + dev_err(dsi->dev, "unsupported message type 0x%02x\n", + msg->type); ret = -EINVAL; }
Instead of always sending commands in LP mode, respect the MIPI_DSI_MSG_USE_LPM flag to decide how to send each message. Also request acks if MIPI_DSI_MSG_REQ_ACK is set.
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Chris Zhong zyw@rock-chips.com Reviewed-by: Sean Paul seanpaul@chromium.org --- v4: - Add Sean's Reviewed-by v3: - Add Chris' Reviewed-by Unchanged in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 374b18c550fd..a20d669b73ee 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -542,6 +542,19 @@ static int dw_mipi_dsi_host_detach(struct mipi_dsi_host *host, return 0; }
+static void dw_mipi_message_config(struct dw_mipi_dsi *dsi, + const struct mipi_dsi_msg *msg) +{ + u32 val = 0; + + if (msg->flags & MIPI_DSI_MSG_REQ_ACK) + val |= EN_ACK_RQST; + if (msg->flags & MIPI_DSI_MSG_USE_LPM) + val |= CMD_MODE_ALL_LP; + + dsi_write(dsi, DSI_CMD_MODE_CFG, val); +} + static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 hdr_val) { int ret; @@ -636,6 +649,8 @@ static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host, struct dw_mipi_dsi *dsi = host_to_dsi(host); int ret;
+ dw_mipi_message_config(dsi, msg); + switch (msg->type) { case MIPI_DSI_DCS_SHORT_WRITE: case MIPI_DSI_DCS_SHORT_WRITE_PARAM: @@ -747,7 +762,6 @@ static void dw_mipi_dsi_command_mode_config(struct dw_mipi_dsi *dsi) { dsi_write(dsi, DSI_TO_CNT_CFG, HSTX_TO_CNT(1000) | LPRX_TO_CNT(1000)); dsi_write(dsi, DSI_BTA_TO_CNT, 0xd00); - dsi_write(dsi, DSI_CMD_MODE_CFG, CMD_MODE_ALL_LP); dsi_write(dsi, DSI_MODE_CFG, ENABLE_CMD_MODE); }
Requesting the HS clock from the PHY before we initialize it causes an invalid signal to be sent out since the input clock is not yet configured. The PHY databook suggests only asserting this signal when performing HS transfers, so let's do that.
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Chris Zhong zyw@rock-chips.com Reviewed-by: Sean Paul seanpaul@chromium.org --- v4: - Add Sean's Reviewed-by v3: - Add Chris' Reviewed-by Unchanged in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index a20d669b73ee..1b6fce2600f3 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -545,13 +545,15 @@ static int dw_mipi_dsi_host_detach(struct mipi_dsi_host *host, 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;
if (msg->flags & MIPI_DSI_MSG_REQ_ACK) val |= EN_ACK_RQST; - if (msg->flags & MIPI_DSI_MSG_USE_LPM) + 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); }
@@ -695,6 +697,7 @@ static void dw_mipi_dsi_set_mode(struct dw_mipi_dsi *dsi, dsi_write(dsi, DSI_PWR_UP, RESET); dsi_write(dsi, DSI_MODE_CFG, ENABLE_VIDEO_MODE); dw_mipi_dsi_video_mode_config(dsi); + dsi_write(dsi, DSI_LPCLK_CTRL, PHY_TXREQUESTCLKHS); dsi_write(dsi, DSI_PWR_UP, POWERUP); } } @@ -712,7 +715,6 @@ static void dw_mipi_dsi_init(struct dw_mipi_dsi *dsi) | PHY_RSTZ | PHY_SHUTDOWNZ); dsi_write(dsi, DSI_CLKMGR_CFG, TO_CLK_DIVIDSION(10) | TX_ESC_CLK_DIVIDSION(7)); - dsi_write(dsi, DSI_LPCLK_CTRL, PHY_TXREQUESTCLKHS); }
static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi,
By dereferencing the MIPI command buffer as a u32* we rely on it being correctly aligned on ARM, but this may not be the case. Copy it into a stack variable that will be correctly aligned.
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Chris Zhong zyw@rock-chips.com --- v3: - Add Chris' Reviewed-by Unchanged in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 1b6fce2600f3..0c66e6eaf44a 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -609,10 +609,10 @@ static int dw_mipi_dsi_dcs_short_write(struct dw_mipi_dsi *dsi, static int dw_mipi_dsi_dcs_long_write(struct dw_mipi_dsi *dsi, const struct mipi_dsi_msg *msg) { - const u32 *tx_buf = msg->tx_buf; - int len = msg->tx_len, pld_data_bytes = sizeof(*tx_buf), ret; + const u8 *tx_buf = msg->tx_buf; + int len = msg->tx_len, pld_data_bytes = sizeof(u32), ret; u32 hdr_val = GEN_HDATA(msg->tx_len) | GEN_HTYPE(msg->type); - u32 remainder = 0; + u32 remainder; u32 val;
if (msg->tx_len < 3) { @@ -623,12 +623,14 @@ static int dw_mipi_dsi_dcs_long_write(struct dw_mipi_dsi *dsi,
while (DIV_ROUND_UP(len, pld_data_bytes)) { if (len < pld_data_bytes) { + remainder = 0; memcpy(&remainder, tx_buf, len); dsi_write(dsi, DSI_GEN_PLD_DATA, remainder); len = 0; } else { - dsi_write(dsi, DSI_GEN_PLD_DATA, *tx_buf); - tx_buf++; + memcpy(&remainder, tx_buf, pld_data_bytes); + dsi_write(dsi, DSI_GEN_PLD_DATA, remainder); + tx_buf += pld_data_bytes; len -= pld_data_bytes; }
Some panels need to be configured with commands sent over the MIPI link, which they will do in the prepare hook. Call this after the PHY has been initialized so that we are able to send commands to the panel.
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Chris Zhong zyw@rock-chips.com Reviewed-by: Sean Paul seanpaul@chromium.org --- v4: - Add Sean's Reviewed-by v3: - Add Chris' Reviewed-by Unchanged in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 0c66e6eaf44a..53515404d0ca 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -898,12 +898,14 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder) dw_mipi_dsi_dphy_timing_config(dsi); dw_mipi_dsi_dphy_interface_config(dsi); dw_mipi_dsi_clear_err(dsi); - if (drm_panel_prepare(dsi->panel)) - dev_err(dsi->dev, "failed to prepare panel\n");
dw_mipi_dsi_phy_init(dsi); dw_mipi_dsi_wait_for_two_frames(mode);
+ dw_mipi_dsi_set_mode(dsi, DW_MIPI_DSI_CMD_MODE); + if (drm_panel_prepare(dsi->panel)) + dev_err(dsi->dev, "failed to prepare panel\n"); + dw_mipi_dsi_set_mode(dsi, DW_MIPI_DSI_VID_MODE); drm_panel_enable(dsi->panel);
Panel drivers may want to sent commands during the disable function, for example MIPI_DCS_SET_DISPLAY_OFF before the video signal ends. In order to send commands we need to write to registers, so pclk must be enabled.
While changing this, remove the unnecessary code after the panel unprepare call which seems to be a workaround for a specific panel and thus belongs in the panel driver.
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Chris Zhong zyw@rock-chips.com Reviewed-by: Sean Paul seanpaul@chromium.org --- v4: - Add Sean's Reviewed-by v3: - Add Chris' Reviewed-by Unchanged in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 53515404d0ca..4201a2143295 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -848,24 +848,16 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder) { struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
- drm_panel_disable(dsi->panel); - if (clk_prepare_enable(dsi->pclk)) { dev_err(dsi->dev, "%s: Failed to enable pclk\n", __func__); return; }
+ drm_panel_disable(dsi->panel); + dw_mipi_dsi_set_mode(dsi, DW_MIPI_DSI_CMD_MODE); drm_panel_unprepare(dsi->panel); - dw_mipi_dsi_set_mode(dsi, DW_MIPI_DSI_VID_MODE);
- /* - * This is necessary to make sure the peripheral will be driven - * normally when the display is enabled again later. - */ - msleep(120); - - dw_mipi_dsi_set_mode(dsi, DW_MIPI_DSI_CMD_MODE); dw_mipi_dsi_disable(dsi); clk_disable_unprepare(dsi->pclk); }
This clock rate is derived from the PHY PLL, so it should be calculated dynamically. This calculation is the same as that used by the vendor kernel and ensures that the escape clock runs at <20MHz as required by the MIPI specification.
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Chris Zhong zyw@rock-chips.com Reviewed-by: Sean Paul seanpaul@chromium.org --- v4: - Add a comment explaining the calculation and reword the commit message so that the calculation doesn't seem so magical - Add Sean's Reviewed-by v3: - Improve the commit message a bit - Add Chris' Reviewed-by Unchanged in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 4201a2143295..0f9be41f0361 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -712,11 +712,21 @@ static void dw_mipi_dsi_disable(struct dw_mipi_dsi *dsi)
static void dw_mipi_dsi_init(struct dw_mipi_dsi *dsi) { + /* + * The maximum permitted escape clock is 20MHz and it is derived from + * lanebyteclk, which is running at "lane_mbps / 8". Thus we want: + * + * (lane_mbps >> 3) / esc_clk_division < 20 + * which is: + * (lane_mbps >> 3) / 20 > esc_clk_division + */ + u32 esc_clk_division = (dsi->lane_mbps >> 3) / 20 + 1; + dsi_write(dsi, DSI_PWR_UP, RESET); dsi_write(dsi, DSI_PHY_RSTZ, PHY_DISFORCEPLL | PHY_DISABLECLK | PHY_RSTZ | PHY_SHUTDOWNZ); dsi_write(dsi, DSI_CLKMGR_CFG, TO_CLK_DIVIDSION(10) | - TX_ESC_CLK_DIVIDSION(7)); + TX_ESC_CLK_DIVIDSION(esc_clk_division)); }
static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi,
Also don't power up the DSI host at this point since this is not necessary in order to configure the PHY and we do so later when selecting video or command mode.
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Chris Zhong zyw@rock-chips.com Reviewed-by: Sean Paul seanpaul@chromium.org --- v4: - Add Sean's Reviewed-by v3: - Add Chris' Reviewed-by Unchanged in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 0f9be41f0361..78d676b7e516 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -397,7 +397,10 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi) return testdin; }
- dsi_write(dsi, DSI_PWR_UP, POWERUP); + /* Start by clearing PHY state */ + 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);
dw_mipi_dsi_phy_write(dsi, 0x10, BYPASS_VCO_RANGE | VCO_RANGE_CON_SEL(vco) |
The bias, bandgap and PLL should all be configured before we enable them.
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Sean Paul seanpaul@chromium.org --- v4: - Add Sean's Reviewed-by v3: - Squash together two patches that both affect initialization order of the PHY Unchanged in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 78d676b7e516..4fee5176c606 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -413,12 +413,17 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
dw_mipi_dsi_phy_write(dsi, 0x44, HSFREQRANGE_SEL(testdin));
- dw_mipi_dsi_phy_write(dsi, 0x19, PLL_LOOP_DIV_EN | PLL_INPUT_DIV_EN); dw_mipi_dsi_phy_write(dsi, 0x17, INPUT_DIVIDER(dsi->input_div)); dw_mipi_dsi_phy_write(dsi, 0x18, LOOP_DIV_LOW_SEL(dsi->feedback_div) | LOW_PROGRAM_EN); dw_mipi_dsi_phy_write(dsi, 0x18, LOOP_DIV_HIGH_SEL(dsi->feedback_div) | HIGH_PROGRAM_EN); + dw_mipi_dsi_phy_write(dsi, 0x19, PLL_LOOP_DIV_EN | PLL_INPUT_DIV_EN); + + dw_mipi_dsi_phy_write(dsi, 0x22, LOW_PROGRAM_EN | + BIASEXTR_SEL(BIASEXTR_127_7)); + dw_mipi_dsi_phy_write(dsi, 0x22, HIGH_PROGRAM_EN | + BANDGAP_SEL(BANDGAP_96_10));
dw_mipi_dsi_phy_write(dsi, 0x20, POWER_CONTROL | INTERNAL_REG_CURRENT | BIAS_BLOCK_ON | BANDGAP_ON); @@ -429,10 +434,6 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi) SETRD_MAX | POWER_MANAGE | TER_RESISTORS_ON);
- dw_mipi_dsi_phy_write(dsi, 0x22, LOW_PROGRAM_EN | - BIASEXTR_SEL(BIASEXTR_127_7)); - dw_mipi_dsi_phy_write(dsi, 0x22, HIGH_PROGRAM_EN | - BANDGAP_SEL(BANDGAP_96_10));
dw_mipi_dsi_phy_write(dsi, 0x70, TLP_PROGRAM_EN | 0xf); dw_mipi_dsi_phy_write(dsi, 0x71, THS_PRE_PROGRAM_EN | 0x55);
These values are specified as constant time periods but the PHY configuration is in terms of the current lane byte clock so using constant values guarantees that the timings will be outside the specification with some display configurations.
Derive the necessary configuration from the byte clock in order to ensure that the PHY configuration is correct.
Signed-off-by: John Keeping john@metanate.com --- v4: - Simplify ns2bc and ns2ui calculations as suggested by Sean Paul v3: - Wrap some long lines Unchanged in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 35 ++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 4fee5176c606..9b6a60deb69e 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -383,6 +383,22 @@ static void dw_mipi_dsi_phy_write(struct dw_mipi_dsi *dsi, u8 test_code, dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR); }
+/** + * ns2bc - Nanoseconds to byte clock cycles + */ +static inline unsigned int ns2bc(struct dw_mipi_dsi *dsi, int ns) +{ + return DIV_ROUND_UP(ns * dsi->lane_mbps / 8, 1000); +} + +/** + * ns2ui - Nanoseconds to UI time periods + */ +static inline unsigned int ns2ui(struct dw_mipi_dsi *dsi, int ns) +{ + return DIV_ROUND_UP(ns * dsi->lane_mbps, 1000); +} + static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi) { int ret, testdin, vco, val; @@ -434,10 +450,21 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi) SETRD_MAX | POWER_MANAGE | TER_RESISTORS_ON);
- - dw_mipi_dsi_phy_write(dsi, 0x70, TLP_PROGRAM_EN | 0xf); - dw_mipi_dsi_phy_write(dsi, 0x71, THS_PRE_PROGRAM_EN | 0x55); - dw_mipi_dsi_phy_write(dsi, 0x72, THS_ZERO_PROGRAM_EN | 0xa); + dw_mipi_dsi_phy_write(dsi, 0x60, TLP_PROGRAM_EN | ns2bc(dsi, 500)); + dw_mipi_dsi_phy_write(dsi, 0x61, THS_PRE_PROGRAM_EN | ns2ui(dsi, 40)); + dw_mipi_dsi_phy_write(dsi, 0x62, THS_ZERO_PROGRAM_EN | ns2bc(dsi, 300)); + dw_mipi_dsi_phy_write(dsi, 0x63, THS_PRE_PROGRAM_EN | ns2ui(dsi, 100)); + dw_mipi_dsi_phy_write(dsi, 0x64, BIT(5) | ns2bc(dsi, 100)); + dw_mipi_dsi_phy_write(dsi, 0x65, BIT(5) | (ns2bc(dsi, 60) + 7)); + + dw_mipi_dsi_phy_write(dsi, 0x70, TLP_PROGRAM_EN | ns2bc(dsi, 500)); + dw_mipi_dsi_phy_write(dsi, 0x71, + THS_PRE_PROGRAM_EN | (ns2ui(dsi, 50) + 5)); + dw_mipi_dsi_phy_write(dsi, 0x72, + THS_ZERO_PROGRAM_EN | (ns2bc(dsi, 140) + 2)); + dw_mipi_dsi_phy_write(dsi, 0x73, + THS_PRE_PROGRAM_EN | (ns2ui(dsi, 60) + 8)); + dw_mipi_dsi_phy_write(dsi, 0x74, BIT(5) | ns2bc(dsi, 100));
dsi_write(dsi, DSI_PHY_RSTZ, PHY_ENFORCEPLL | PHY_ENABLECLK | PHY_UNRSTZ | PHY_UNSHUTDOWNZ);
The multiplication ratio for the PLL is required to be even due to the use of a "by 2 pre-scaler". Currently we are likely to end up with an odd multiplier even though there is an equivalent set of parameters with an even multiplier.
For example, using the 324MHz bit rate with a reference clock of 24MHz we end up with M = 27, N = 2 whereas the example in the PHY databook gives M = 54, N = 4 for this bit rate and reference clock.
By walking down through the available multiplier instead of up we are more likely to hit an even multiplier. With the above example we do now get M = 54, N = 4 as given by the databook.
While doing this, change the loop limits to encode the actual limits on the divisor, which are:
40MHz >= (pllref / N) >= 5MHz
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Sean Paul seanpaul@chromium.org --- v4: - Add a comment explaining the limits so that it can be understood without finding the commit message above - Add Sean's Reviewed-by Unchanged in v3 Unchanged in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 9b6a60deb69e..e6b52c7cb5e3 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -518,7 +518,18 @@ static int dw_mipi_dsi_get_lane_bps(struct dw_mipi_dsi *dsi, pllref = DIV_ROUND_UP(clk_get_rate(dsi->pllref_clk), USEC_PER_SEC); tmp = pllref;
- for (i = 1; i < 6; i++) { + /* + * The limits on the PLL divisor are: + * + * 5MHz <= (pllref / n) <= 40MHz + * + * we walk over these values in descreasing order so that if we hit + * an exact match for target_mbps it is more likely that "m" will be + * even. + * + * TODO: ensure that "m" is even after this loop. + */ + for (i = pllref / 5; i > (pllref / 40); i--) { pre = pllref / i; if ((tmp > (target_mbps % pre)) && (target_mbps / pre < 512)) { tmp = target_mbps % pre;
As the documentation for readx_poll_timeout says, we want to use the specialized macro for readl rather than using the generic version directly.
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Chris Zhong zyw@rock-chips.com Reviewed-by: Sean Paul seanpaul@chromium.org --- v4: - Add Sean's Reviewed-by v3: - Add Chris' Reviewed-by Unchanged in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index e6b52c7cb5e3..ccf818d5c7ac 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -470,14 +470,14 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi) PHY_UNRSTZ | PHY_UNSHUTDOWNZ);
- ret = readx_poll_timeout(readl, dsi->base + DSI_PHY_STATUS, + ret = readl_poll_timeout(dsi->base + DSI_PHY_STATUS, val, val & LOCK, 1000, PHY_STATUS_TIMEOUT_US); if (ret < 0) { dev_err(dsi->dev, "failed to wait for phy lock state\n"); return ret; }
- ret = readx_poll_timeout(readl, dsi->base + DSI_PHY_STATUS, + ret = readl_poll_timeout(dsi->base + DSI_PHY_STATUS, val, val & STOP_STATE_CLK_LANE, 1000, PHY_STATUS_TIMEOUT_US); if (ret < 0) { @@ -604,7 +604,7 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 hdr_val) int ret; u32 val, mask;
- ret = readx_poll_timeout(readl, dsi->base + DSI_CMD_PKT_STATUS, + ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS, val, !(val & GEN_CMD_FULL), 1000, CMD_PKT_STATUS_TIMEOUT_US); if (ret < 0) { @@ -615,7 +615,7 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 hdr_val) dsi_write(dsi, DSI_GEN_HDR, hdr_val);
mask = GEN_CMD_EMPTY | GEN_PLD_W_EMPTY; - ret = readx_poll_timeout(readl, dsi->base + DSI_CMD_PKT_STATUS, + ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS, val, (val & mask) == mask, 1000, CMD_PKT_STATUS_TIMEOUT_US); if (ret < 0) { @@ -676,7 +676,7 @@ static int dw_mipi_dsi_dcs_long_write(struct dw_mipi_dsi *dsi, len -= pld_data_bytes; }
- ret = readx_poll_timeout(readl, dsi->base + DSI_CMD_PKT_STATUS, + ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS, val, !(val & GEN_PLD_W_FULL), 1000, CMD_PKT_STATUS_TIMEOUT_US); if (ret < 0) {
This matches other drivers.
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Sean Paul seanpaul@chromium.org --- v4: - Add Sean's Reviewed-by Unchanged in v3 Unchanged in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index ccf818d5c7ac..60dfb5666a25 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -791,9 +791,9 @@ static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi, break; }
- if (!(mode->flags & DRM_MODE_FLAG_PVSYNC)) + if (mode->flags & DRM_MODE_FLAG_NVSYNC) val |= VSYNC_ACTIVE_LOW; - if (!(mode->flags & DRM_MODE_FLAG_PHSYNC)) + if (mode->flags & DRM_MODE_FLAG_NHSYNC) val |= HSYNC_ACTIVE_LOW;
dsi_write(dsi, DSI_DPI_VCID, DPI_VID(dsi->channel));
When connected to the MIPI DSI output, we need to use N{H,V}SYNC for the internal connection but these flags are meaningless for DSI panels. Switch the test so that we do not set the P{H,V}SYNC bits unless the mode requires it.
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Mark Yao mark.yao@rock-chips.com Reviewed-by: Sean Paul seanpaul@chromium.org --- v4: - Add Sean's Reviewed-by v3: - Add Mark's Reviewed-by Unchanged in v2 --- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index c7eba305c488..67aefc6d4e9a 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -933,8 +933,8 @@ static void vop_crtc_enable(struct drm_crtc *crtc) }
pin_pol = 0x8; - pin_pol |= (adjusted_mode->flags & DRM_MODE_FLAG_NHSYNC) ? 0 : 1; - pin_pol |= (adjusted_mode->flags & DRM_MODE_FLAG_NVSYNC) ? 0 : (1 << 1); + pin_pol |= (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC) ? 1 : 0; + pin_pol |= (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC) ? (1 << 1) : 0; VOP_CTRL_SET(vop, pin_pol, pin_pol);
switch (s->output_type) {
This ensures that the output resolution is known before fbcon loads. mipi_dsi_host_register() is moved above dw_mipi_dsi_register() to simplify error cleanup since the order of these operations does not matter.
Signed-off-by: John Keeping john@metanate.com --- v4: - Use "return 0" to separate normal code flow from error cleanup Unchanged in v3 Unchanged in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 60dfb5666a25..9edb868f8dc1 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -1189,12 +1189,27 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master, goto err_pllref; }
- dev_set_drvdata(dev, dsi); - dsi->dsi_host.ops = &dw_mipi_dsi_host_ops; dsi->dsi_host.dev = dev; - return mipi_dsi_host_register(&dsi->dsi_host); + ret = mipi_dsi_host_register(&dsi->dsi_host); + if (ret) { + dev_err(dev, "Failed to register MIPI host: %d\n", ret); + goto err_cleanup; + } + + if (!dsi->panel) { + ret = -EPROBE_DEFER; + goto err_mipi_dsi_host; + }
+ dev_set_drvdata(dev, dsi); + return 0; + +err_mipi_dsi_host: + mipi_dsi_host_unregister(&dsi->dsi_host); +err_cleanup: + drm_encoder_cleanup(&dsi->encoder); + drm_connector_cleanup(&dsi->connector); err_pllref: clk_disable_unprepare(dsi->pllref_clk); return ret;
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Chris Zhong zyw@rock-chips.com Reviewed-by: Sean Paul seanpaul@chromium.org --- v4: - Add Sean's Reviewed-by v3: - Add Chris' Reviewed-by Unchanged in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 9edb868f8dc1..0c4bae711e84 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -82,7 +82,9 @@ #define FRAME_BTA_ACK BIT(14) #define ENABLE_LOW_POWER (0x3f << 8) #define ENABLE_LOW_POWER_MASK (0x3f << 8) -#define VID_MODE_TYPE_BURST_SYNC_PULSES 0x2 +#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 DSI_VID_PKT_SIZE 0x3c @@ -286,6 +288,7 @@ struct dw_mipi_dsi { u32 format; u16 input_div; u16 feedback_div; + unsigned long mode_flags;
const struct dw_mipi_dsi_plat_data *pdata; }; @@ -558,15 +561,10 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host, return -EINVAL; }
- if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) || - !(device->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)) { - dev_err(dsi->dev, "device mode is unsupported\n"); - return -EINVAL; - } - dsi->lanes = device->lanes; dsi->channel = device->channel; dsi->format = device->format; + dsi->mode_flags = device->mode_flags; dsi->panel = of_drm_find_panel(device->dev.of_node); if (dsi->panel) return drm_panel_attach(dsi->panel, &dsi->connector); @@ -725,7 +723,14 @@ static void dw_mipi_dsi_video_mode_config(struct dw_mipi_dsi *dsi) { u32 val;
- val = VID_MODE_TYPE_BURST_SYNC_PULSES | ENABLE_LOW_POWER; + val = ENABLE_LOW_POWER; + + if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) + val |= VID_MODE_TYPE_BURST; + else if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) + val |= VID_MODE_TYPE_NON_BURST_SYNC_PULSES; + else + val |= VID_MODE_TYPE_NON_BURST_SYNC_EVENTS;
dsi_write(dsi, DSI_VID_MODE_CFG, val); }
In order to fully reset the state of the MIPI controller we must assert this reset.
This is slightly more complicated than it could be in order to maintain compatibility with device trees that do not specify the reset property.
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Chris Zhong zyw@rock-chips.com --- v4: - Fix error check for devm_reset_control_get() to use ENOENT v3: - Add Chris' Reviewed-by Unchanged in v2 --- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 0c4bae711e84..30da75667334 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -13,6 +13,7 @@ #include <linux/module.h> #include <linux/of_device.h> #include <linux/regmap.h> +#include <linux/reset.h> #include <linux/mfd/syscon.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_crtc.h> @@ -1144,6 +1145,7 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master, of_match_device(dw_mipi_dsi_dt_ids, dev); const struct dw_mipi_dsi_plat_data *pdata = of_id->data; struct platform_device *pdev = to_platform_device(dev); + struct reset_control *apb_rst; struct drm_device *drm = data; struct dw_mipi_dsi *dsi; struct resource *res; @@ -1182,6 +1184,35 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master, return ret; }
+ /* + * Note that the reset was not defined in the initial device tree, so + * we have to be prepared for it not being found. + */ + apb_rst = devm_reset_control_get(dev, "apb"); + if (IS_ERR(apb_rst)) { + ret = PTR_ERR(apb_rst); + if (ret == -ENOENT) { + apb_rst = NULL; + } else { + dev_err(dev, "Unable to get reset control: %d\n", ret); + return ret; + } + } + + if (apb_rst) { + ret = clk_prepare_enable(dsi->pclk); + if (ret) { + dev_err(dev, "%s: Failed to enable pclk\n", __func__); + return ret; + } + + reset_control_assert(apb_rst); + usleep_range(10, 20); + reset_control_deassert(apb_rst); + + clk_disable_unprepare(dsi->pclk); + } + ret = clk_prepare_enable(dsi->pllref_clk); if (ret) { dev_err(dev, "%s: Failed to enable pllref_clk\n", __func__);
+ devicetree
Hi,
On Fri, Feb 24, 2017 at 12:55:06PM +0000, John Keeping wrote:
In order to fully reset the state of the MIPI controller we must assert this reset.
This is slightly more complicated than it could be in order to maintain compatibility with device trees that do not specify the reset property.
Signed-off-by: John Keeping john@metanate.com Reviewed-by: Chris Zhong zyw@rock-chips.com
v4:
- Fix error check for devm_reset_control_get() to use ENOENT
v3:
- Add Chris' Reviewed-by
Unchanged in v2
drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 0c4bae711e84..30da75667334 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -13,6 +13,7 @@ #include <linux/module.h> #include <linux/of_device.h> #include <linux/regmap.h> +#include <linux/reset.h> #include <linux/mfd/syscon.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_crtc.h> @@ -1144,6 +1145,7 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master, of_match_device(dw_mipi_dsi_dt_ids, dev); const struct dw_mipi_dsi_plat_data *pdata = of_id->data; struct platform_device *pdev = to_platform_device(dev);
- struct reset_control *apb_rst; struct drm_device *drm = data; struct dw_mipi_dsi *dsi; struct resource *res;
@@ -1182,6 +1184,35 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master, return ret; }
- /*
* Note that the reset was not defined in the initial device tree, so* we have to be prepared for it not being found.*/- apb_rst = devm_reset_control_get(dev, "apb");
Did this reset ever get documented in the device tree bindings? I couldn't find it. Perhaps a follow-up patch is in order?
[...]
Brian
This reset is required in order to fully reset the internal state of the MIPI controller.
Signed-off-by: John Keeping john@metanate.com --- On Thu, 2 Mar 2017 13:56:46 -0800, Brian Norris wrote:
On Fri, Feb 24, 2017 at 12:55:06PM +0000, John Keeping wrote:
- /*
* Note that the reset was not defined in the initial device tree, so* we have to be prepared for it not being found.*/- apb_rst = devm_reset_control_get(dev, "apb");
Did this reset ever get documented in the device tree bindings? I couldn't find it. Perhaps a follow-up patch is in order?
Here's a patch to do that.
.../devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt index 1753f0cc6fad..28d0b437d3cd 100644 --- a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt +++ b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt @@ -13,8 +13,13 @@ Required properties: - ports: contain a port node with endpoint definitions as defined in [2]. For vopb,set the reg = <0> and set the reg = <1> for vopl.
+Optional properties: +- resets: list of phandle + reset specifier pairs, as described in [3]. +- reset-names: string reset name, must be "apb". + [1] Documentation/devicetree/bindings/clock/clock-bindings.txt [2] Documentation/devicetree/bindings/media/video-interfaces.txt +[3] Documentation/devicetree/bindings/reset/reset.txt
Example: mipi_dsi: mipi@ff960000 { @@ -25,6 +30,8 @@ Example: interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>; clocks = <&cru SCLK_MIPI_24M>, <&cru PCLK_MIPI_DSI0>; clock-names = "ref", "pclk"; + resets = <&cru SRST_MIPIDSI0>; + reset-names = "apb"; rockchip,grf = <&grf>; status = "okay";
On Fri, Mar 03, 2017 at 11:39:45AM +0000, John Keeping wrote:
This reset is required in order to fully reset the internal state of the MIPI controller.
Signed-off-by: John Keeping john@metanate.com
On Thu, 2 Mar 2017 13:56:46 -0800, Brian Norris wrote:
On Fri, Feb 24, 2017 at 12:55:06PM +0000, John Keeping wrote:
- /*
* Note that the reset was not defined in the initial device tree, so* we have to be prepared for it not being found.*/- apb_rst = devm_reset_control_get(dev, "apb");
Did this reset ever get documented in the device tree bindings? I couldn't find it. Perhaps a follow-up patch is in order?
Here's a patch to do that.
FWIW:
Reviewed-by: Brian Norris briannorris@chromium.org
Thanks.
On Fri, Mar 03, 2017 at 11:39:45AM +0000, John Keeping wrote:
This reset is required in order to fully reset the internal state of the MIPI controller.
Signed-off-by: John Keeping john@metanate.com
I'm sorry I missed this in my review. Adding Rob Herring directly for his ack.
Also,
Reviewed-by: Sean Paul seanpaul@chromium.org
On Thu, 2 Mar 2017 13:56:46 -0800, Brian Norris wrote:
On Fri, Feb 24, 2017 at 12:55:06PM +0000, John Keeping wrote:
- /*
* Note that the reset was not defined in the initial device tree, so* we have to be prepared for it not being found.*/- apb_rst = devm_reset_control_get(dev, "apb");
Did this reset ever get documented in the device tree bindings? I couldn't find it. Perhaps a follow-up patch is in order?
Here's a patch to do that.
.../devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt index 1753f0cc6fad..28d0b437d3cd 100644 --- a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt +++ b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt @@ -13,8 +13,13 @@ Required properties:
- ports: contain a port node with endpoint definitions as defined in [2]. For vopb,set the reg = <0> and set the reg = <1> for vopl.
+Optional properties: +- resets: list of phandle + reset specifier pairs, as described in [3]. +- reset-names: string reset name, must be "apb".
[1] Documentation/devicetree/bindings/clock/clock-bindings.txt [2] Documentation/devicetree/bindings/media/video-interfaces.txt +[3] Documentation/devicetree/bindings/reset/reset.txt
Example: mipi_dsi: mipi@ff960000 { @@ -25,6 +30,8 @@ Example: interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>; clocks = <&cru SCLK_MIPI_24M>, <&cru PCLK_MIPI_DSI0>; clock-names = "ref", "pclk";
resets = <&cru SRST_MIPIDSI0>;reset-names = "apb";-- 2.12.0.rc2.230.ga28edc07cd.dirty
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, Mar 03, 2017 at 11:39:45AM +0000, John Keeping wrote:
This reset is required in order to fully reset the internal state of the MIPI controller.
Signed-off-by: John Keeping john@metanate.com
On Thu, 2 Mar 2017 13:56:46 -0800, Brian Norris wrote:
On Fri, Feb 24, 2017 at 12:55:06PM +0000, John Keeping wrote:
- /*
* Note that the reset was not defined in the initial device tree, so* we have to be prepared for it not being found.*/- apb_rst = devm_reset_control_get(dev, "apb");
Did this reset ever get documented in the device tree bindings? I couldn't find it. Perhaps a follow-up patch is in order?
Here's a patch to do that.
.../devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt | 7 +++++++ 1 file changed, 7 insertions(+)
Acked-by: Rob Herring robh@kernel.org
Hi Sean,
On Sun, 12 Mar 2017 07:06:59 -0500, Rob Herring wrote:
On Fri, Mar 03, 2017 at 11:39:45AM +0000, John Keeping wrote:
This reset is required in order to fully reset the internal state of the MIPI controller.
Signed-off-by: John Keeping john@metanate.com
On Thu, 2 Mar 2017 13:56:46 -0800, Brian Norris wrote:
On Fri, Feb 24, 2017 at 12:55:06PM +0000, John Keeping wrote:
- /*
* Note that the reset was not defined in the initial device tree, so* we have to be prepared for it not being found.*/- apb_rst = devm_reset_control_get(dev, "apb");
Did this reset ever get documented in the device tree bindings? I couldn't find it. Perhaps a follow-up patch is in order?
Here's a patch to do that.
.../devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt | 7 +++++++ 1 file changed, 7 insertions(+)
Acked-by: Rob Herring robh@kernel.org
I don't see this patch in linux-next, so I guess it has fallen through the cracks somewhere. Since we have Rob's ack, can you pick this via drm-misc?
Regards, John
On Tue, Apr 04, 2017 at 02:15:13PM +0100, John Keeping wrote:
Hi Sean,
On Sun, 12 Mar 2017 07:06:59 -0500, Rob Herring wrote:
On Fri, Mar 03, 2017 at 11:39:45AM +0000, John Keeping wrote:
This reset is required in order to fully reset the internal state of the MIPI controller.
Signed-off-by: John Keeping john@metanate.com
On Thu, 2 Mar 2017 13:56:46 -0800, Brian Norris wrote:
On Fri, Feb 24, 2017 at 12:55:06PM +0000, John Keeping wrote:
- /*
* Note that the reset was not defined in the initial device tree, so* we have to be prepared for it not being found.*/- apb_rst = devm_reset_control_get(dev, "apb");
Did this reset ever get documented in the device tree bindings? I couldn't find it. Perhaps a follow-up patch is in order?
Here's a patch to do that.
.../devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt | 7 +++++++ 1 file changed, 7 insertions(+)
Acked-by: Rob Herring robh@kernel.org
I don't see this patch in linux-next, so I guess it has fallen through the cracks somewhere. Since we have Rob's ack, can you pick this via drm-misc?
Yeah, sorry about that. Applied to misc-next
Sean
Regards, John
Oh, one more thing:
On Fri, Feb 24, 2017 at 12:55:06PM +0000, John Keeping wrote:
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 0c4bae711e84..30da75667334 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -13,6 +13,7 @@ #include <linux/module.h> #include <linux/of_device.h> #include <linux/regmap.h> +#include <linux/reset.h> #include <linux/mfd/syscon.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_crtc.h> @@ -1144,6 +1145,7 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master, of_match_device(dw_mipi_dsi_dt_ids, dev); const struct dw_mipi_dsi_plat_data *pdata = of_id->data; struct platform_device *pdev = to_platform_device(dev);
- struct reset_control *apb_rst; struct drm_device *drm = data; struct dw_mipi_dsi *dsi; struct resource *res;
@@ -1182,6 +1184,35 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master, return ret; }
- /*
* Note that the reset was not defined in the initial device tree, so* we have to be prepared for it not being found.*/- apb_rst = devm_reset_control_get(dev, "apb");
- if (IS_ERR(apb_rst)) {
ret = PTR_ERR(apb_rst);if (ret == -ENOENT) {apb_rst = NULL;} else {dev_err(dev, "Unable to get reset control: %d\n", ret);
Might want to check for -EPROBE_DEFER before printing an error?
Brian
return ret;}- }
- if (apb_rst) {
ret = clk_prepare_enable(dsi->pclk);if (ret) {dev_err(dev, "%s: Failed to enable pclk\n", __func__);return ret;}reset_control_assert(apb_rst);usleep_range(10, 20);reset_control_deassert(apb_rst);clk_disable_unprepare(dsi->pclk);- }
- ret = clk_prepare_enable(dsi->pllref_clk); if (ret) { dev_err(dev, "%s: Failed to enable pllref_clk\n", __func__);
On 2017年02月24日 20:54, John Keeping wrote:
This version is mostly small changes in response to review comments from Sean and Chris, the details are in the individual patches.
I decided to drop the final patch which adds support for MIPI read commands because I'm not using that feature now and I can't easily test it. It's on the list if anyone wants to pick it up in the future.
Version 3 was posted here: http://www.spinics.net/lists/dri-devel/msg130977.html
Thanks to Sean Paul and Chris Zhong for their review and testing of this series.
Looks good to me.
Acked-by: Mark Yao mark.yao@rock-chips.com
John Keeping (23): drm/rockchip: dw-mipi-dsi: don't configure hardware in mode_set for MIPI drm/rockchip: dw-mipi-dsi: pass mode in where needed drm/rockchip: dw-mipi-dsi: remove mode_set hook drm/rockchip: dw-mipi-dsi: fix command header writes drm/rockchip: dw-mipi-dsi: fix generic packet status check drm/rockchip: dw-mipi-dsi: avoid out-of-bounds read on tx_buf drm/rockchip: dw-mipi-dsi: include bad value in error message drm/rockchip: dw-mipi-dsi: respect message flags drm/rockchip: dw-mipi-dsi: only request HS clock when required drm/rockchip: dw-mipi-dsi: don't assume buffer is aligned drm/rockchip: dw-mipi-dsi: prepare panel after phy init drm/rockchip: dw-mipi-dsi: allow commands in panel_disable drm/rockchip: dw-mipi-dsi: fix escape clock rate drm/rockchip: dw-mipi-dsi: ensure PHY is reset drm/rockchip: dw-mipi-dsi: configure PHY before enabling drm/rockchip: dw-mipi-dsi: properly configure PHY timing drm/rockchip: dw-mipi-dsi: improve PLL configuration drm/rockchip: dw-mipi-dsi: use specific poll helper drm/rockchip: dw-mipi-dsi: use positive check for N{H,V}SYNC drm/rockchip: vop: test for P{H,V}SYNC drm/rockchip: dw-mipi-dsi: defer probe if panel is not loaded drm/rockchip: dw-mipi-dsi: support non-burst modes drm/rockchip: dw-mipi-dsi: add reset control
drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 325 +++++++++++++++++++--------- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 +- 2 files changed, 220 insertions(+), 109 deletions(-)
Hi John
I have test this v4 series on my RK3399 board, it works well, thanks.
Tested-by: Chris Zhongzyw@rock-chips.com
On 02/24/2017 08:54 PM, John Keeping wrote:
This version is mostly small changes in response to review comments from Sean and Chris, the details are in the individual patches.
I decided to drop the final patch which adds support for MIPI read commands because I'm not using that feature now and I can't easily test it. It's on the list if anyone wants to pick it up in the future.
Version 3 was posted here: http://www.spinics.net/lists/dri-devel/msg130977.html
Thanks to Sean Paul and Chris Zhong for their review and testing of this series.
John Keeping (23): drm/rockchip: dw-mipi-dsi: don't configure hardware in mode_set for MIPI drm/rockchip: dw-mipi-dsi: pass mode in where needed drm/rockchip: dw-mipi-dsi: remove mode_set hook drm/rockchip: dw-mipi-dsi: fix command header writes drm/rockchip: dw-mipi-dsi: fix generic packet status check drm/rockchip: dw-mipi-dsi: avoid out-of-bounds read on tx_buf drm/rockchip: dw-mipi-dsi: include bad value in error message drm/rockchip: dw-mipi-dsi: respect message flags drm/rockchip: dw-mipi-dsi: only request HS clock when required drm/rockchip: dw-mipi-dsi: don't assume buffer is aligned drm/rockchip: dw-mipi-dsi: prepare panel after phy init drm/rockchip: dw-mipi-dsi: allow commands in panel_disable drm/rockchip: dw-mipi-dsi: fix escape clock rate drm/rockchip: dw-mipi-dsi: ensure PHY is reset drm/rockchip: dw-mipi-dsi: configure PHY before enabling drm/rockchip: dw-mipi-dsi: properly configure PHY timing drm/rockchip: dw-mipi-dsi: improve PLL configuration drm/rockchip: dw-mipi-dsi: use specific poll helper drm/rockchip: dw-mipi-dsi: use positive check for N{H,V}SYNC drm/rockchip: vop: test for P{H,V}SYNC drm/rockchip: dw-mipi-dsi: defer probe if panel is not loaded drm/rockchip: dw-mipi-dsi: support non-burst modes drm/rockchip: dw-mipi-dsi: add reset control
drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 325 +++++++++++++++++++--------- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 +- 2 files changed, 220 insertions(+), 109 deletions(-)
On Fri, Feb 24, 2017 at 12:54:43PM +0000, John Keeping wrote:
This version is mostly small changes in response to review comments from Sean and Chris, the details are in the individual patches.
I decided to drop the final patch which adds support for MIPI read commands because I'm not using that feature now and I can't easily test it. It's on the list if anyone wants to pick it up in the future.
Version 3 was posted here: http://www.spinics.net/lists/dri-devel/msg130977.html
Thanks to Sean Paul and Chris Zhong for their review and testing of this series.
John Keeping (23): drm/rockchip: dw-mipi-dsi: don't configure hardware in mode_set for MIPI drm/rockchip: dw-mipi-dsi: pass mode in where needed drm/rockchip: dw-mipi-dsi: remove mode_set hook drm/rockchip: dw-mipi-dsi: fix command header writes drm/rockchip: dw-mipi-dsi: fix generic packet status check drm/rockchip: dw-mipi-dsi: avoid out-of-bounds read on tx_buf drm/rockchip: dw-mipi-dsi: include bad value in error message drm/rockchip: dw-mipi-dsi: respect message flags drm/rockchip: dw-mipi-dsi: only request HS clock when required drm/rockchip: dw-mipi-dsi: don't assume buffer is aligned drm/rockchip: dw-mipi-dsi: prepare panel after phy init drm/rockchip: dw-mipi-dsi: allow commands in panel_disable drm/rockchip: dw-mipi-dsi: fix escape clock rate drm/rockchip: dw-mipi-dsi: ensure PHY is reset drm/rockchip: dw-mipi-dsi: configure PHY before enabling drm/rockchip: dw-mipi-dsi: properly configure PHY timing drm/rockchip: dw-mipi-dsi: improve PLL configuration drm/rockchip: dw-mipi-dsi: use specific poll helper drm/rockchip: dw-mipi-dsi: use positive check for N{H,V}SYNC drm/rockchip: vop: test for P{H,V}SYNC drm/rockchip: dw-mipi-dsi: defer probe if panel is not loaded drm/rockchip: dw-mipi-dsi: support non-burst modes drm/rockchip: dw-mipi-dsi: add reset control
drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 325 +++++++++++++++++++--------- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 +- 2 files changed, 220 insertions(+), 109 deletions(-)
Applied to drm-misc
Thanks,
Sean
-- 2.12.0.rc0.230.gf625d4cdb9.dirty
dri-devel@lists.freedesktop.org
-
Brian Norris -
Chris Zhong -
John Keeping -
Mark yao -
Rob Herring -
Sean Paul