drm/rockchip: MIPI fixes & improvements

With atomic modesetting the hardware will be powered off when the mode_set function is called. We should configure the hardware in the commit function (or even the enable function, but switching from commit to enable is left for a future patch).

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

---
v2:
- also move dw_mipi_dsi_get_lane_bps into the commit function

---
  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 45 ++++++++++++++--------------------
  1 file changed, 19 insertions(+), 26 deletions(-)

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

index d9aa382bb629..8b83ae50c8f3 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)

@@ -880,12 +854,31 @@ static void dw_mipi_dsi_encoder_commit(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);
 

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

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

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

---
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,
  };

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

---
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");

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

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

---
Unchanged in v2

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

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

index 4be1ff3a42bb..2e6ad4591ebf 100644
@@ -572,8 +572,13 @@ 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;
+	u32 val = GEN_HTYPE(msg->type);
+
+	if (msg->tx_len > 0)
+		val |= GEN_HDATA(tx_buf[0]);
+	if (msg->tx_len > 1)
+		val |= GEN_HDATA(tx_buf[1] << 8);
 
  	if (msg->tx_len > 2) {
  		dev_err(dsi->dev, "too long tx buf length %zu for short write\n",

As an aid to debugging.

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

---
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 2e6ad4591ebf..92dbc3e56603 100644
@@ -644,7 +644,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 <[email protected]>

---
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 92dbc3e56603..15d33c3c8cb7 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;

@@ -634,6 +647,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:

@@ -745,7 +760,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 <[email protected]>

---
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 15d33c3c8cb7..03fc096fe1bd 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);
  }
 

@@ -693,6 +695,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);
  	}
  }

@@ -710,7 +713,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 <[email protected]>

---
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 03fc096fe1bd..ddbc037e7ced 100644
@@ -607,10 +607,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) {

@@ -621,12 +621,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 <[email protected]>

---
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 ddbc037e7ced..7ada6d8ed143 100644
@@ -896,12 +896,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 <[email protected]>

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

index 7ada6d8ed143..290282e86d16 100644
@@ -846,24 +846,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);

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

  	dw_mipi_dsi_disable(dsi);
  	clk_disable_unprepare(dsi->pclk);
  }

Use the same calculation as the vendor kernel to derive the escape clock speed.

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

---
Unchanged in v2

---
  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

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

index 290282e86d16..c2e0ba96e0a0 100644
@@ -710,11 +710,13 @@ static void dw_mipi_dsi_disable(struct dw_mipi_dsi *dsi)
 
  static void dw_mipi_dsi_init(struct dw_mipi_dsi *dsi)
  {
+	u32 esc_clk_division = (dsi->lane_mbps >> 3) / 20 + 1;
+
  	dsi_write(dsi, DSI_PWR_UP, RESET);
  	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 <[email protected]>

---
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 c2e0ba96e0a0..5b3068e9e8db 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) |

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

---
Unchanged in v2

---
  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

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

index ce1e6f9a2041..cfe7e4ba305c 100644
@@ -413,12 +413,12 @@ 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));

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

---
Unchanged in v2

---
  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 36 ++++++++++++++++++++++++++++++----
  1 file changed, 32 insertions(+), 4 deletions(-)

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

index cfe7e4ba305c..12432e41971b 100644
@@ -383,6 +383,26 @@ 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)
+{
+	unsigned long byte_clk_khz = dsi->lane_mbps * MSEC_PER_SEC / 8;
+
+	return (ns * (byte_clk_khz / 1000) + 999) / 1000;
+}
+
+/**
+ * ns2ui - Nanoseconds to UI time periods
+ */
+static inline unsigned int ns2ui(struct dw_mipi_dsi *dsi, int ns)
+{
+	unsigned long byte_clk_khz = dsi->lane_mbps * MSEC_PER_SEC;
+
+	return (ns * (byte_clk_khz / 1000) + 999) / 1000;
+}
+
  static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
  {
  	int ret, testdin, vco, val;

@@ -434,10 +454,18 @@ 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);

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

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

---
Unchanged in v2

---
  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

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

index 12432e41971b..f2320cf1366c 100644
@@ -519,7 +519,7 @@ 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++) {
+	for (i = pllref / 5; i > (pllref / 40); i--) {
  		pre = pllref / i;
  		if ((tmp > (target_mbps % pre)) && (target_mbps / pre < 512)) {
  			tmp = target_mbps % pre;

Quoted 9 lines · 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.

Quoted 8 lines

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  

This formula is limit for N, but we still can not guarantee to get an even M.
Do you think we should do a check for M.
such as:
if (m % 2)

     continue;
...
     for (i = pllref / 5; i > (pllref / 40); i--) {
         pre = pllref / i;
         if ((tmp > (target_mbps % pre)) && (target_mbps / pre < 512)) {
             tmp = target_mbps % pre;
             n = i;
             m = target_mbps / pre;
             if (m % 2)
                 continue;
         }
         if (tmp == 0)
             break;
     }

if (m % 2)

     m++;

     dsi->lane_mbps = pllref / n * m;
     dsi->input_div = n;
     dsi->feedback_div = m;

Quoted 20 lines

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

---
Unchanged in v2

---
  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

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

index 12432e41971b..f2320cf1366c 100644
@@ -519,7 +519,7 @@ 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++) {
+	for (i = pllref / 5; i > (pllref / 40); i--) {
  		pre = pllref / i;
  		if ((tmp > (target_mbps % pre)) && (target_mbps / pre < 512)) {
  			tmp = target_mbps % pre;  

Hi Chris,

Quoted 38 lines · Chris Zhong

Quoted 9 lines · 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.

Quoted 8 lines

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

This formula is limit for N, but we still can not guarantee to get an even M.
Do you think we should do a check for M.
such as:
if (m % 2)

      continue;
...
      for (i = pllref / 5; i > (pllref / 40); i--) {
          pre = pllref / i;
          if ((tmp > (target_mbps % pre)) && (target_mbps / pre < 512)) {
              tmp = target_mbps % pre;
              n = i;
              m = target_mbps / pre;
              if (m % 2)
                  continue;
          }
          if (tmp == 0)
              break;
      }

Quoted 6 lines

if (m % 2)

      m++;

      dsi->lane_mbps = pllref / n * m;
      dsi->input_div = n;
      dsi->feedback_div = m;

Yes, I agree that there should be a check for M, but I'm not sure if the version above is sufficient. The "m % 2" check inside the loop means that we don't break immediately when tmp=0 but then we are guaranteed to break next time without having modified n, m because now tmp=0 so "tmp > (target_mbps % pre)" is always false and we just hit the "if (tmp == 0) break" case next time.

Given that the descending loop already means that if we can hit "tmp" exactly we are more likely to do so with a bigger N and even M, I think it might be better to just fix M after the loop like:

	if (m % 2) {
		if (m < 256 && (n * 2) <= (pllref / 5)) {
			n *= 2;
			m *= 2;
		} else {
			m++;
		}
	}

but I haven't thought about this too carefully.

For this series, I'd rather either keep this patch as it is or drop it in favour of a more comprehensive solution. I don't want to block the other fixes waiting for a perfect fix here and we can always improve this further with a follow-up patch.

Quoted 22 lines

Quoted 21 lines

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

---
Unchanged in v2

---
   drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

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

index 12432e41971b..f2320cf1366c 100644
@@ -519,7 +519,7 @@ 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++) {
+	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 <[email protected]>

---
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 f2320cf1366c..3e19693ae59b 100644
@@ -471,14 +471,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) {

@@ -594,7 +594,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) {

@@ -605,7 +605,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) {

@@ -664,7 +664,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) {

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

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

index c7eba305c488..67aefc6d4e9a 100644

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

  	VOP_CTRL_SET(vop, pin_pol, pin_pol);
 
  	switch (s->output_type) {

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

---
Unchanged in v2

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

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

index 2dafb17e31d2..881bb0c62ca5 100644
@@ -82,6 +82,7 @@
  #define FRAME_BTA_ACK			BIT(14)
  #define ENABLE_LOW_POWER		(0x3f << 8)
  #define ENABLE_LOW_POWER_MASK		(0x3f << 8)
+#define VID_MODE_TYPE_NON_BURST_SYNC_EVENTS	0x1
  #define VID_MODE_TYPE_BURST_SYNC_PULSES		0x2
  #define VID_MODE_TYPE_MASK			0x3
 

@@ -286,6 +287,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;
  };

@@ -548,15 +550,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);

@@ -713,7 +710,12 @@ 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_SYNC_PULSES;
+	else if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE))
+		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 <[email protected]>

---
Unchanged in v2

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

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

index 881bb0c62ca5..f780401e8b5e 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>

@@ -1121,6 +1122,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;

@@ -1159,6 +1161,34 @@ 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)) {
+		if (PTR_ERR(apb_rst) == -ENODEV) {
+			apb_rst = NULL;
+		} else {
+			dev_err(dev, "Unable to get reset control: %d\n", ret);
+			return PTR_ERR(apb_rst);
+		}
+	}
+
+	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__);

I haven't found any method for getting the length of a response, so this just uses the requested rx_len

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

---
Unchanged in v2

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

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

index f780401e8b5e..f0cb005c6d76 100644
@@ -675,6 +675,54 @@ static int dw_mipi_dsi_dcs_long_write(struct dw_mipi_dsi *dsi,
  	return dw_mipi_dsi_gen_pkt_hdr_write(dsi, hdr_val);
  }
 

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

+
+	ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
+				 val, !(val & GEN_RD_CMD_BUSY), 1000,
+				 CMD_PKT_STATUS_TIMEOUT_US);
+	if (ret < 0) {
+		dev_err(dsi->dev, "failed to read command response\n");
+		return ret;
+	}
+
+	for (i = 0; i < msg->rx_len;) {
+		u32 pld = dsi_read(dsi, DSI_GEN_PLD_DATA);
+		while (i < msg->rx_len) {
+			rx_buf[i] = pld & 0xff;
+			pld >>= 8;
+			i++;
+		}
+	}
+
+	return msg->rx_len;
+}
+
+static int dw_mipi_dsi_set_max_return_packet_size(struct dw_mipi_dsi *dsi,
+						  size_t len)
+{
+	u8 val[] = { len & 0xff, (len >> 8) & 0xff };
+	struct mipi_dsi_msg msg = {
+		.channel = dsi->channel,
+		.type = MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE,
+		.tx_buf = val,
+		.tx_len = 2,
+	};
+
+	if (len > 0xffff)
+		return -EINVAL;
+
+	return dw_mipi_dsi_dcs_short_write(dsi, &msg);
+}
+
  static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host,
  					 const struct mipi_dsi_msg *msg)
  {

@@ -692,6 +740,12 @@ static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host,
  	case MIPI_DSI_DCS_LONG_WRITE:
  		ret = dw_mipi_dsi_dcs_long_write(dsi, msg);
  		break;
+	case MIPI_DSI_DCS_READ:
+		ret = dw_mipi_dsi_set_max_return_packet_size(dsi, msg->rx_len);
+		if (ret < 0)
+			return ret;
+		ret = dw_mipi_dsi_dcs_read(dsi, msg);
+		break;
  	default:
  		dev_err(dsi->dev, "unsupported message type 0x%02x\n",
  			msg->type);