drm/rockchip: MIPI fixes & improvements

PATCH Applied John Keeping 6 participants 35 messages Feb 24, 2017 – Apr 4, 2017

Subsystems (5)

Device tree bindings DRM drivers DRM misc Arm Rockchip +1 more subsystems

lore

Share & tools

lore

John Keeping

author

2017-02-24 12:54 UTC #0 lore raw

v4 cover drm/rockchip: MIPI fixes & improvements

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(-)

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:54 UTC #1 lore raw

PATCH v4 1/23 drm/rockchip: dw-mipi-dsi: don't configure hardware in mode_set for MIPI

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 <[email protected]>

Reviewed-by Sean Paul <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +21 ~~-28~~

index d9aa382bb629..bbd992299f73 100644
@@ -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,

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:54 UTC #2 lore raw

PATCH v4 2/23 drm/rockchip: dw-mipi-dsi: pass mode in where needed

In reply to John Keeping

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 <[email protected]>

Reviewed-by Chris Zhong <[email protected]>

Reviewed-by Sean Paul <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +22 ~~-19~~

index bbd992299f73..cdbd25087e83 100644
@@ -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);

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:54 UTC #3 lore raw

PATCH v4 3/23 drm/rockchip: dw-mipi-dsi: remove mode_set hook

In reply to John Keeping

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 <[email protected]>

Reviewed-by Chris Zhong <[email protected]>

Reviewed-by Sean Paul <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +1 ~~-12~~

index cdbd25087e83..bd92e58b64f3 100644
@@ -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,
 };

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:54 UTC #4 lore raw

PATCH v4 4/23 drm/rockchip: dw-mipi-dsi: fix command header writes

In reply to John Keeping

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 <[email protected]>

Tested-by Chris Zhong <[email protected]>

Reviewed-by Chris Zhong <[email protected]>

Reviewed-by Sean Paul <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +6 -4

index bd92e58b64f3..4cbbbcb619b7 100644
@@ -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,

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:54 UTC #5 lore raw

PATCH v4 5/23 drm/rockchip: dw-mipi-dsi: fix generic packet status check

In reply to John Keeping

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 <[email protected]>

Reviewed-by Chris Zhong <[email protected]>

Reviewed-by Sean Paul <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +3 -2

index 4cbbbcb619b7..4be1ff3a42bb 100644
@@ -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");

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:54 UTC #6 lore raw

PATCH v4 6/23 drm/rockchip: dw-mipi-dsi: avoid out-of-bounds read on tx_buf

In reply to John Keeping

As a side-effect of this, encode the endianness explicitly rather than casting a u16.

Signed-off-by John Keeping <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +9 -2

index 4be1ff3a42bb..f55010312f25 100644
@@ -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);
 }

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:54 UTC #7 lore raw

PATCH v4 7/23 drm/rockchip: dw-mipi-dsi: include bad value in error message

In reply to John Keeping

As an aid to debugging.

Signed-off-by John Keeping <[email protected]>

Reviewed-by Chris Zhong <[email protected]>

Reviewed-by Sean Paul <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +2 -1

index f55010312f25..374b18c550fd 100644
@@ -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;
 	}

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:54 UTC #8 lore raw

PATCH v4 8/23 drm/rockchip: dw-mipi-dsi: respect message flags

In reply to John Keeping

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 <[email protected]>

Reviewed-by Chris Zhong <[email protected]>

Reviewed-by Sean Paul <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +15 -1

index 374b18c550fd..a20d669b73ee 100644
@@ -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);
 }

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:54 UTC #9 lore raw

PATCH v4 9/23 drm/rockchip: dw-mipi-dsi: only request HS clock when required

In reply to John Keeping

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 <[email protected]>

Reviewed-by Chris Zhong <[email protected]>

Reviewed-by Sean Paul <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +4 -2

index a20d669b73ee..1b6fce2600f3 100644
@@ -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,

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:54 UTC #10 lore raw

PATCH v4 10/23 drm/rockchip: dw-mipi-dsi: don't assume buffer is aligned

In reply to John Keeping

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 <[email protected]>

Reviewed-by Chris Zhong <[email protected]>

---
v3:
- Add Chris' Reviewed-by
Unchanged in v2

---
 drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +7 -5

index 1b6fce2600f3..0c66e6eaf44a 100644
@@ -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;
 		}

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:54 UTC #11 lore raw

PATCH v4 11/23 drm/rockchip: dw-mipi-dsi: prepare panel after phy init

In reply to John Keeping

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 <[email protected]>

Reviewed-by Chris Zhong <[email protected]>

Reviewed-by Sean Paul <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +4 -2

index 0c66e6eaf44a..53515404d0ca 100644
@@ -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);

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:54 UTC #12 lore raw

PATCH v4 12/23 drm/rockchip: dw-mipi-dsi: allow commands in panel_disable

In reply to John Keeping

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 <[email protected]>

Reviewed-by Chris Zhong <[email protected]>

Reviewed-by Sean Paul <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +2 ~~-10~~

index 53515404d0ca..4201a2143295 100644
@@ -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);
 }

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:54 UTC #13 lore raw

PATCH v4 13/23 drm/rockchip: dw-mipi-dsi: fix escape clock rate

In reply to John Keeping

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 <[email protected]>

Reviewed-by Chris Zhong <[email protected]>

Reviewed-by Sean Paul <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +11 -1

index 4201a2143295..0f9be41f0361 100644
@@ -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,

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:54 UTC #14 lore raw

PATCH v4 14/23 drm/rockchip: dw-mipi-dsi: ensure PHY is reset

In reply to John Keeping

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 <[email protected]>

Reviewed-by Chris Zhong <[email protected]>

Reviewed-by Sean Paul <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +4 -1

index 0f9be41f0361..78d676b7e516 100644
@@ -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) |

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:54 UTC #15 lore raw

PATCH v4 15/23 drm/rockchip: dw-mipi-dsi: configure PHY before enabling

In reply to John Keeping

The bias, bandgap and PLL should all be configured before we enable them.

Signed-off-by John Keeping <[email protected]>

Reviewed-by Sean Paul <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +6 -5

index 78d676b7e516..4fee5176c606 100644
@@ -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);

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:54 UTC #16 lore raw

PATCH v4 16/23 drm/rockchip: dw-mipi-dsi: properly configure PHY timing

In reply to John Keeping

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 <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +31 -4

index 4fee5176c606..9b6a60deb69e 100644
@@ -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);

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:55 UTC #17 lore raw

PATCH v4 17/23 drm/rockchip: dw-mipi-dsi: improve PLL configuration

In reply to John Keeping

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 <[email protected]>

Reviewed-by Sean Paul <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +12 -1

index 9b6a60deb69e..e6b52c7cb5e3 100644
@@ -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;

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:55 UTC #18 lore raw

PATCH v4 18/23 drm/rockchip: dw-mipi-dsi: use specific poll helper

In reply to John Keeping

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 <[email protected]>

Reviewed-by Chris Zhong <[email protected]>

Reviewed-by Sean Paul <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +5 -5

index e6b52c7cb5e3..ccf818d5c7ac 100644
@@ -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) {

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:55 UTC #19 lore raw

PATCH v4 19/23 drm/rockchip: dw-mipi-dsi: use positive check for N{H,V}SYNC

In reply to John Keeping

This matches other drivers.

Signed-off-by John Keeping <[email protected]>

Reviewed-by Sean Paul <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +2 -2

index ccf818d5c7ac..60dfb5666a25 100644
@@ -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));

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:55 UTC #20 lore raw

PATCH v4 20/23 drm/rockchip: vop: test for P{H,V}SYNC

In reply to John Keeping

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 <[email protected]>

Reviewed-by Mark Yao <[email protected]>

Reviewed-by Sean Paul <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/rockchip_drm_vop.c +2 -2

index c7eba305c488..67aefc6d4e9a 100644
@@ -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) {

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:55 UTC #21 lore raw

PATCH v4 21/23 drm/rockchip: dw-mipi-dsi: defer probe if panel is not loaded

In reply to John Keeping

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 <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +18 -3

index 60dfb5666a25..9edb868f8dc1 100644
@@ -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;

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:55 UTC #22 lore raw

PATCH v4 22/23 drm/rockchip: dw-mipi-dsi: support non-burst modes

In reply to John Keeping

Signed-off-by John Keeping <[email protected]>

Reviewed-by Chris Zhong <[email protected]>

Reviewed-by Sean Paul <[email protected]>

---
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(-)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +13 -8

index 9edb868f8dc1..0c4bae711e84 100644
@@ -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);
 }

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

John Keeping

author

2017-02-24 12:55 UTC #23 lore raw

PATCH v4 23/23 drm/rockchip: dw-mipi-dsi: add reset control

In reply to John Keeping

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 <[email protected]>

Reviewed-by Chris Zhong <[email protected]>

---
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(+)

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +31 -0

index 0c4bae711e84..30da75667334 100644
@@ -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__);

Signature · 1 line

2.12.0.rc0.230.gf625d4cdb9.dirty

Brian Norris

2017-03-02 21:56 UTC #24 lore raw

Re: [PATCH v4 23/23] drm/rockchip: dw-mipi-dsi: add reset control

+ devicetree

Hi,

Quoted 39 lines · John Keeping

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-HooS5bfzL4hWk0Htik3J/[email protected]>
Reviewed-by Chris Zhong <[email protected]>
---
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(+)
drivers/gpu/drm/rockchip/dw-mipi-dsi.c +2 -0
index 0c4bae711e84..30da75667334 100644
@@ -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;

drivers/gpu/drm/rockchip/dw-mipi-dsi.c static int dw_mipi_dsi_bind(struct device *dev, struct device *master, · line 1191

Quoted 8 lines

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +5 -0

@@ -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

John Keeping

author

2017-03-03 11:39 UTC #25 lore raw

PATCH 1/1 dt-bindings: display: rk3288-mipi-dsi: add reset property

This reset is required in order to fully reset the internal state of the MIPI controller.

Signed-off-by John Keeping <[email protected]>

---

Quoted 9 lines · Brian Norris

Review
Quoted 5 lines · John Keeping
(quoted diff) +5 -0
+	/*
+	 * 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(+)

Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt +7 -0

index 1753f0cc6fad..28d0b437d3cd 100644
@@ -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 at 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";

Signature · 1 line

2.12.0.rc2.230.ga28edc07cd.dirty

Brian Norris

Reviewed-by on v1 1/1

2017-03-03 20:41 UTC #26 lore raw

Re: [PATCH] dt-bindings: display: rk3288-mipi-dsi: add reset property

Quoted 16 lines · John Keeping

This reset is required in order to fully reset the internal state of the MIPI controller.
Signed-off-by John Keeping <john-HooS5bfzL4hWk0Htik3J/[email protected]>
---
Quoted 9 lines · Brian Norris
Review
Quoted 5 lines · John Keeping
(quoted diff) +5 -0
+	/*
+	 * 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?

Quoted 1 line

Here's a patch to do that.

FWIW:

Reviewed-by Brian Norris <[email protected]>

Thanks.

Sean Paul

Reviewed-by on v1 1/1

2017-03-06 16:52 UTC #27 lore raw

Re: [PATCH] dt-bindings: display: rk3288-mipi-dsi: add reset property

In reply to John Keeping

Quoted 4 lines · John Keeping

This reset is required in order to fully reset the internal state of the MIPI controller.
Signed-off-by John Keeping <[email protected]>

I'm sorry I missed this in my review. Adding Rob Herring directly for his ack.

Also,

Reviewed-by Sean Paul <[email protected]>

Quoted 51 lines

---

Quoted 9 lines · Brian Norris

Review
Quoted 5 lines · John Keeping
(quoted diff) +5 -0
+	/*
+	 * 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(+)

Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt +7 -0

index 1753f0cc6fad..28d0b437d3cd 100644
@@ -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 at 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";

Signature · 6 lines

2.12.0.rc2.230.ga28edc07cd.dirty

_______________________________________________
dri-devel mailing list
dri-devel at lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Rob Herring

maintainer Acked-by on v1 1/1

2017-03-12 12:06 UTC #28 lore raw

Re: [PATCH] dt-bindings: display: rk3288-mipi-dsi: add reset property

In reply to John Keeping

Quoted 16 lines · John Keeping

This reset is required in order to fully reset the internal state of the MIPI controller.
Signed-off-by John Keeping <[email protected]>
---
Quoted 9 lines · Brian Norris
Review
Quoted 5 lines · John Keeping
(quoted diff) +5 -0
+	/*
+	 * 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?

Quoted 4 lines

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 <[email protected]>

John Keeping

author

2017-04-04 13:15 UTC #29 lore raw

Re: [PATCH] dt-bindings: display: rk3288-mipi-dsi: add reset property

Hi Sean,

Quoted 17 lines · Rob Herring

Quoted 16 lines · John Keeping
This reset is required in order to fully reset the internal state of the MIPI controller.
Signed-off-by John Keeping <john-HooS5bfzL4hWk0Htik3J/[email protected]>
---
Quoted 9 lines · Brian Norris
Review
Quoted 5 lines · John Keeping
(quoted diff) +5 -0
+	/*
+	 * 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?

Quoted 6 lines

Quoted 4 lines
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 <[email protected]>

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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Sean Paul

2017-04-04 18:30 UTC #30 lore raw

Re: [PATCH] dt-bindings: display: rk3288-mipi-dsi: add reset property

Quoted 23 lines · John Keeping

Hi Sean,
Quoted 19 lines · Rob Herring
Quoted 18 lines · John Keeping
This reset is required in order to fully reset the internal state of the MIPI controller.
Signed-off-by John Keeping <john-HooS5bfzL4hWk0Htik3J/[email protected]>
---
Quoted 9 lines · Brian Norris
Review
Quoted 5 lines · John Keeping
(quoted diff) +5 -0
+	/*
+	 * 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.

Quoted 9 lines

Quoted 4 lines
Quoted 2 lines
 .../devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt  | 7 +++++++
 1 file changed, 7 insertions(+)  
Acked-by Rob Herring <[email protected]>
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

Quoted 3 lines

Regards,
John

Signature · 5 lines

Sean Paul, Software Engineer, Google / Chromium OS
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Brian Norris

2017-03-02 21:59 UTC #31 lore raw

Re: [PATCH v4 23/23] drm/rockchip: dw-mipi-dsi: add reset control

In reply to John Keeping

Oh, one more thing:

Quoted 25 lines · John Keeping

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +3 -0

index 0c4bae711e84..30da75667334 100644
@@ -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;
 	}
 
+	/*

drivers/gpu/drm/rockchip/dw-mipi-dsi.c static int dw_mipi_dsi_bind(struct device *dev, struct device *master, · line 1197

Quoted 10 lines

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +10 -0

+	 * 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

Quoted 21 lines

drivers/gpu/drm/rockchip/dw-mipi-dsi.c +18 -0

+			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__);

Mark yao

Acked-by on the v4 series

2017-02-27 01:41 UTC #32 lore raw

In reply to John Keeping

Quoted 7 lines · John Keeping

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.

Quoted 5 lines

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 <[email protected]>

Quoted 31 lines

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(-)

Signature · 1 line

Ｍark Yao

Chris Zhong

Tested-by on the v4 series

2017-02-27 03:34 UTC #33 lore raw

In reply to John Keeping

Hi John

I have test this v4 series on my RK3399 board, it works well, thanks.

Tested-by Chris Zhong<[email protected]>

Quoted 43 lines · John Keeping

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(-)

Signature · 1 line

Chris Zhong

Sean Paul

Applied

2017-03-01 20:03 UTC #34 lore raw

In reply to John Keeping

Quoted 39 lines · John Keeping

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

Quoted 3 lines

 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

Quoted 3 lines

Signature · 2 lines
-- 
2.12.0.rc0.230.gf625d4cdb9.dirty