[PATCH 0/9] BYT DSI Dual Link Support

[PATCH 0/9] BYT DSI Dual Link Support

[Gaurav K Singh]

Hi,
These set of patches build on top of the existing DSI Video mode support to
enable dual link MIPI panels with high resolutions. These patches have been 
tested on a 25x16 panel and works well.

Regards
Gaurav

Gaurav K Singh (9):
  drm/i915: New functions added for enabling & disabling MIPI Port Ctrl
    reg
  drm/i915: MIPI Sequence to be sent to the DSI Controller based on the
    port no from VBT
  drm/i915: MIPI Port Ctrl related changes for dual link configuration
  drm/i915: Pixel Clock and pixel overlap related changes for dual link
    Configuration
  drm/i915: SHUTDOWN & Turn ON packets to be sent for both MIPI Ports
    in case of dual link Configuration
  drm/i915: Dsipll clk to be enabled for DSI1 in case of dual link
    configuration
  drm/i915: MIPI Timings related changes for dual link Configuration
  drm/i915: MIPI encoder disable related changes for dual link
    Configuration
  drm/i915: MIPI Encoder enable related changes for dual link
    configuration

 drivers/gpu/drm/i915/i915_reg.h            |    5 +
 drivers/gpu/drm/i915/intel_bios.h          |    3 +-
 drivers/gpu/drm/i915/intel_dsi.c           |  471 ++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_dsi.h           |    8 +
 drivers/gpu/drm/i915/intel_dsi_cmd.c       |   44 +--
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |   25 ++
 drivers/gpu/drm/i915/intel_dsi_pll.c       |   14 +-
 7 files changed, 382 insertions(+), 188 deletions(-)

-- 
1.7.9.5

[PATCH 1/9] drm/i915: New functions added for enabling & disabling MIPI Port Ctrl reg

[Gaurav K Singh]

This patch is in preparation for the dual link port enable and disable related changes.

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c |   43 ++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 5bd9e09..e456ca9 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -102,6 +102,36 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
 	return true;
 }
 
+static void intel_dsi_port_enable(struct intel_encoder *encoder)
+{
+	struct drm_device *dev = encoder->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+	enum pipe pipe = intel_crtc->pipe;
+	u32 temp;
+
+	/* assert ip_tg_enable signal */
+	temp = I915_READ(MIPI_PORT_CTRL(pipe)) & ~LANE_CONFIGURATION_MASK;
+	temp = temp | intel_dsi->port_bits;
+	I915_WRITE(MIPI_PORT_CTRL(pipe), temp | DPI_ENABLE);
+	POSTING_READ(MIPI_PORT_CTRL(pipe));
+}
+
+static void intel_dsi_port_disable(struct intel_encoder *encoder)
+{
+	struct drm_device *dev = encoder->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+	enum pipe pipe = intel_crtc->pipe;
+	u32 temp;
+
+	/* de-assert ip_tg_enable signal */
+	temp = I915_READ(MIPI_PORT_CTRL(pipe));
+	I915_WRITE(MIPI_PORT_CTRL(pipe), temp & ~DPI_ENABLE);
+	POSTING_READ(MIPI_PORT_CTRL(pipe));
+}
+
 static void intel_dsi_device_ready(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
@@ -141,7 +171,6 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
 	int pipe = intel_crtc->pipe;
-	u32 temp;
 
 	DRM_DEBUG_KMS("\n");
 
@@ -157,11 +186,7 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
 
 		wait_for_dsi_fifo_empty(intel_dsi);
 
-		/* assert ip_tg_enable signal */
-		temp = I915_READ(MIPI_PORT_CTRL(pipe)) & ~LANE_CONFIGURATION_MASK;
-		temp = temp | intel_dsi->port_bits;
-		I915_WRITE(MIPI_PORT_CTRL(pipe), temp | DPI_ENABLE);
-		POSTING_READ(MIPI_PORT_CTRL(pipe));
+		intel_dsi_port_enable(encoder);
 	}
 }
 
@@ -245,11 +270,7 @@ static void intel_dsi_disable(struct intel_encoder *encoder)
 	if (is_vid_mode(intel_dsi)) {
 		wait_for_dsi_fifo_empty(intel_dsi);
 
-		/* de-assert ip_tg_enable signal */
-		temp = I915_READ(MIPI_PORT_CTRL(pipe));
-		I915_WRITE(MIPI_PORT_CTRL(pipe), temp & ~DPI_ENABLE);
-		POSTING_READ(MIPI_PORT_CTRL(pipe));
-
+		intel_dsi_port_disable(encoder);
 		msleep(2);
 	}
 
-- 
1.7.9.5

[PATCH 2/9] drm/i915: MIPI Sequence to be sent to the DSI Controller based on the port no from VBT

[Gaurav K Singh]

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.h           |    1 +
 drivers/gpu/drm/i915/intel_dsi_cmd.c       |    9 +++------
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |    3 +++
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 657eb5c..587e71f 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -115,6 +115,7 @@ struct intel_dsi {
 	u16 clk_lp_to_hs_count;
 	u16 clk_hs_to_lp_count;
 
+	u16 port;
 	u16 init_count;
 	u32 pclk;
 	u16 burst_mode_ratio;
diff --git a/drivers/gpu/drm/i915/intel_dsi_cmd.c b/drivers/gpu/drm/i915/intel_dsi_cmd.c
index f4767fd..eb698b1 100644
--- a/drivers/gpu/drm/i915/intel_dsi_cmd.c
+++ b/drivers/gpu/drm/i915/intel_dsi_cmd.c
@@ -130,8 +130,7 @@ static int dsi_vc_send_short(struct intel_dsi *intel_dsi, int channel,
 	struct drm_encoder *encoder = &intel_dsi->base.base;
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
-	enum pipe pipe = intel_crtc->pipe;
+	enum pipe pipe = intel_dsi->port;
 	u32 ctrl_reg;
 	u32 ctrl;
 	u32 mask;
@@ -172,8 +171,7 @@ static int dsi_vc_send_long(struct intel_dsi *intel_dsi, int channel,
 	struct drm_encoder *encoder = &intel_dsi->base.base;
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
-	enum pipe pipe = intel_crtc->pipe;
+	enum pipe pipe = intel_dsi->port;
 	u32 data_reg;
 	int i, j, n;
 	u32 mask;
@@ -291,8 +289,7 @@ static int dsi_read_data_return(struct intel_dsi *intel_dsi,
 	struct drm_encoder *encoder = &intel_dsi->base.base;
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
-	enum pipe pipe = intel_crtc->pipe;
+	enum pipe pipe = intel_dsi->port;
 	int i, len = 0;
 	u32 data_reg, val;
 
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index f6bdd44..051bfff 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -106,6 +106,8 @@ static u8 *mipi_exec_send_packet(struct intel_dsi *intel_dsi, u8 *data)
 
 	/* LP or HS mode */
 	intel_dsi->hs = mode;
+	/*MIPI Port A or MIPI Port C*/
+	intel_dsi->port = port;
 
 	/* get packet type and increment the pointer */
 	type = *data++;
@@ -280,6 +282,7 @@ static bool generic_init(struct intel_dsi_device *dsi)
 	intel_dsi->clock_stop = mipi_config->enable_clk_stop ? 1 : 0;
 	intel_dsi->lane_count = mipi_config->lane_cnt + 1;
 	intel_dsi->pixel_format = mipi_config->videomode_color_format << 7;
+	intel_dsi->port = 0;
 
 	if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB666)
 		bits_per_pixel = 18;
-- 
1.7.9.5

[PATCH 3/9] drm/i915: MIPI Port Ctrl related changes for dual link configuration

[Gaurav K Singh]

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h            |    1 +
 drivers/gpu/drm/i915/intel_dsi.c           |   53 ++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_dsi.h           |    1 +
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |    1 +
 4 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ad8179b..922d807 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6215,6 +6215,7 @@ enum punit_power_well {
 #define  DPI_ENABLE					(1 << 31) /* A + B */
 #define  MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT		27
 #define  MIPIA_MIPI4DPHY_DELAY_COUNT_MASK		(0xf << 27)
+#define  DUAL_LINK_MODE_SHIFT				26
 #define  DUAL_LINK_MODE_MASK				(1 << 26)
 #define  DUAL_LINK_MODE_FRONT_BACK			(0 << 26)
 #define  DUAL_LINK_MODE_PIXEL_ALTERNATIVE		(1 << 26)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index e456ca9..3b1890e 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -109,13 +109,31 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
 	enum pipe pipe = intel_crtc->pipe;
-	u32 temp;
-
-	/* assert ip_tg_enable signal */
-	temp = I915_READ(MIPI_PORT_CTRL(pipe)) & ~LANE_CONFIGURATION_MASK;
-	temp = temp | intel_dsi->port_bits;
-	I915_WRITE(MIPI_PORT_CTRL(pipe), temp | DPI_ENABLE);
-	POSTING_READ(MIPI_PORT_CTRL(pipe));
+	u32 temp, port_control = 0;
+
+	if (intel_dsi->dual_link) {
+		port_control = (intel_dsi->dual_link - 1)
+					<< DUAL_LINK_MODE_SHIFT;
+		port_control |= pipe ? LANE_CONFIGURATION_DUAL_LINK_B :
+					LANE_CONFIGURATION_DUAL_LINK_A;
+		/*For Port A */
+		temp = I915_READ(MIPI_PORT_CTRL(0));
+		temp = temp | port_control;
+		I915_WRITE(MIPI_PORT_CTRL(0), temp | DPI_ENABLE);
+		POSTING_READ(MIPI_PORT_CTRL(0));
+
+		/* For Port C */
+		temp = I915_READ(MIPI_PORT_CTRL(1));
+		I915_WRITE(MIPI_PORT_CTRL(1), temp | DPI_ENABLE);
+		POSTING_READ(MIPI_PORT_CTRL(1));
+	} else {
+		/* assert ip_tg_enable signal */
+		temp = I915_READ(MIPI_PORT_CTRL(pipe)) &
+					~LANE_CONFIGURATION_MASK;
+		temp = temp | intel_dsi->port_bits;
+		I915_WRITE(MIPI_PORT_CTRL(pipe), temp | DPI_ENABLE);
+		POSTING_READ(MIPI_PORT_CTRL(pipe));
+	}
 }
 
 static void intel_dsi_port_disable(struct intel_encoder *encoder)
@@ -123,13 +141,26 @@ static void intel_dsi_port_disable(struct intel_encoder *encoder)
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
 	enum pipe pipe = intel_crtc->pipe;
 	u32 temp;
 
-	/* de-assert ip_tg_enable signal */
-	temp = I915_READ(MIPI_PORT_CTRL(pipe));
-	I915_WRITE(MIPI_PORT_CTRL(pipe), temp & ~DPI_ENABLE);
-	POSTING_READ(MIPI_PORT_CTRL(pipe));
+	if (intel_dsi->dual_link) {
+		/*For Port A */
+		temp = I915_READ(MIPI_PORT_CTRL(0));
+		I915_WRITE(MIPI_PORT_CTRL(0), temp & ~DPI_ENABLE);
+		POSTING_READ(MIPI_PORT_CTRL(0));
+
+		/* For Port C */
+		temp = I915_READ(MIPI_PORT_CTRL(1));
+		I915_WRITE(MIPI_PORT_CTRL(1), temp & ~DPI_ENABLE);
+		POSTING_READ(MIPI_PORT_CTRL(1));
+	} else {
+		/* de-assert ip_tg_enable signal */
+		temp = I915_READ(MIPI_PORT_CTRL(pipe));
+		I915_WRITE(MIPI_PORT_CTRL(pipe), temp & ~DPI_ENABLE);
+		POSTING_READ(MIPI_PORT_CTRL(pipe));
+	}
 }
 
 static void intel_dsi_device_ready(struct intel_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 587e71f..950ab41 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -101,6 +101,7 @@ struct intel_dsi {
 	u8 clock_stop;
 
 	u8 escape_clk_div;
+	u8 dual_link;
 	u32 port_bits;
 	u32 bw_timer;
 	u32 dphy_reg;
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 051bfff..d424ebc 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -283,6 +283,7 @@ static bool generic_init(struct intel_dsi_device *dsi)
 	intel_dsi->lane_count = mipi_config->lane_cnt + 1;
 	intel_dsi->pixel_format = mipi_config->videomode_color_format << 7;
 	intel_dsi->port = 0;
+	intel_dsi->dual_link = mipi_config->dual_link;
 
 	if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB666)
 		bits_per_pixel = 18;
-- 
1.7.9.5

[PATCH 4/9] drm/i915: Pixel Clock and pixel overlap related changes for dual link Configuration

[Gaurav K Singh]

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h            |    4 ++++
 drivers/gpu/drm/i915/intel_bios.h          |    3 ++-
 drivers/gpu/drm/i915/intel_dsi.c           |    8 ++++++++
 drivers/gpu/drm/i915/intel_dsi.h           |    6 ++++++
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |   21 +++++++++++++++++++++
 5 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 922d807..3ed5774 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5662,6 +5662,10 @@ enum punit_power_well {
 #define GEN8_PMINTR_REDIRECT_TO_NON_DISP	(1<<31)
 #define VLV_PWRDWNUPCTL				0xA294
 
+#define VLV_CHICKEN_3				0x7040C
+#define  PIXEL_OVERLAP_CNT_MASK			(3 << 30)
+#define  PIXEL_OVERLAP_CNT_SHIFT		30
+
 #define GEN6_PMISR				0x44020
 #define GEN6_PMIMR				0x44024 /* rps_lock */
 #define GEN6_PMIIR				0x44028
diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
index 7603765..39dfb65 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -798,7 +798,8 @@ struct mipi_config {
 #define DUAL_LINK_PIXEL_ALT	2
 	u16 dual_link:2;
 	u16 lane_cnt:2;
-	u16 rsvd3:12;
+	u16 pixel_overlap:3;
+	u16 rsvd3:9;
 
 	u16 rsvd4;
 
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 3b1890e..583c7fd 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -116,6 +116,14 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
 					<< DUAL_LINK_MODE_SHIFT;
 		port_control |= pipe ? LANE_CONFIGURATION_DUAL_LINK_B :
 					LANE_CONFIGURATION_DUAL_LINK_A;
+
+		if (intel_dsi->dual_link & MIPI_DUAL_LINK_FRONT_BACK) {
+			temp = I915_READ(VLV_CHICKEN_3);
+			temp &= ~PIXEL_OVERLAP_CNT_MASK |
+				intel_dsi->pixel_overlap <<
+				PIXEL_OVERLAP_CNT_SHIFT;
+			I915_WRITE(VLV_CHICKEN_3, temp);
+		}
 		/*For Port A */
 		temp = I915_READ(MIPI_PORT_CTRL(0));
 		temp = temp | port_control;
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 950ab41..7fac460 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -28,6 +28,11 @@
 #include <drm/drm_crtc.h>
 #include "intel_drv.h"
 
+/* Dual Link support */
+#define MIPI_DUAL_LINK_NONE		0
+#define MIPI_DUAL_LINK_FRONT_BACK	1
+#define MIPI_DUAL_LINK_PIXEL_ALT	2
+
 struct intel_dsi_device {
 	unsigned int panel_id;
 	const char *name;
@@ -102,6 +107,7 @@ struct intel_dsi {
 
 	u8 escape_clk_div;
 	u8 dual_link;
+	u8 pixel_overlap;
 	u32 port_bits;
 	u32 bw_timer;
 	u32 dphy_reg;
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index d424ebc..8bc911b 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -284,6 +284,7 @@ static bool generic_init(struct intel_dsi_device *dsi)
 	intel_dsi->pixel_format = mipi_config->videomode_color_format << 7;
 	intel_dsi->port = 0;
 	intel_dsi->dual_link = mipi_config->dual_link;
+	intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
 
 	if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB666)
 		bits_per_pixel = 18;
@@ -303,6 +304,20 @@ static bool generic_init(struct intel_dsi_device *dsi)
 
 	pclk = mode->clock;
 
+	/* In dual link mode each port needs half of pixel clock */
+	if (intel_dsi->dual_link) {
+		pclk = pclk / 2;
+
+		/* we can enable pixel_overlap if needed by panel. In this
+		 * case we need to increase the pixelclock for extra pixels
+		 */
+		if (intel_dsi->dual_link & MIPI_DUAL_LINK_FRONT_BACK) {
+			pclk += DIV_ROUND_UP(mode->vtotal *
+						intel_dsi->pixel_overlap *
+						60, 1000);
+		}
+	}
+
 	/* Burst Mode Ratio
 	 * Target ddr frequency from VBT / non burst ddr freq
 	 * multiply by 100 to preserve remainder
@@ -497,6 +512,12 @@ static bool generic_init(struct intel_dsi_device *dsi)
 	DRM_DEBUG_KMS("Clockstop %s\n", intel_dsi->clock_stop ?
 						"disabled" : "enabled");
 	DRM_DEBUG_KMS("Mode %s\n", intel_dsi->operation_mode ? "command" : "video");
+	if (intel_dsi->dual_link == MIPI_DUAL_LINK_FRONT_BACK)
+		DRM_DEBUG_KMS("Dual link: MIPI_DUAL_LINK_FRONT_BACK\n");
+	else if (intel_dsi->dual_link == MIPI_DUAL_LINK_PIXEL_ALT)
+		DRM_DEBUG_KMS("Dual link: MIPI_DUAL_LINK_PIXEL_ALT\n");
+	else
+		DRM_DEBUG_KMS("Dual link: NONE\n");
 	DRM_DEBUG_KMS("Pixel Format %d\n", intel_dsi->pixel_format);
 	DRM_DEBUG_KMS("TLPX %d\n", intel_dsi->escape_clk_div);
 	DRM_DEBUG_KMS("LP RX Timeout 0x%x\n", intel_dsi->lp_rx_timeout);
-- 
1.7.9.5

[PATCH 5/9] drm/i915: SHUTDOWN & Turn ON packets to be sent for both MIPI Ports in case of dual link Configuration

[Gaurav K Singh]

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi_cmd.c |   35 ++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi_cmd.c b/drivers/gpu/drm/i915/intel_dsi_cmd.c
index eb698b1..a70656e 100644
--- a/drivers/gpu/drm/i915/intel_dsi_cmd.c
+++ b/drivers/gpu/drm/i915/intel_dsi_cmd.c
@@ -394,6 +394,7 @@ int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs)
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
 	enum pipe pipe = intel_crtc->pipe;
 	u32 mask;
+	int count = 1;
 
 	/* XXX: pipe, hs */
 	if (hs)
@@ -401,18 +402,28 @@ int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs)
 	else
 		cmd |= DPI_LP_MODE;
 
-	/* clear bit */
-	I915_WRITE(MIPI_INTR_STAT(pipe), SPL_PKT_SENT_INTERRUPT);
-
-	/* XXX: old code skips write if control unchanged */
-	if (cmd == I915_READ(MIPI_DPI_CONTROL(pipe)))
-		DRM_ERROR("Same special packet %02x twice in a row.\n", cmd);
-
-	I915_WRITE(MIPI_DPI_CONTROL(pipe), cmd);
-
-	mask = SPL_PKT_SENT_INTERRUPT;
-	if (wait_for((I915_READ(MIPI_INTR_STAT(pipe)) & mask) == mask, 100))
-		DRM_ERROR("Video mode command 0x%08x send failed.\n", cmd);
+	if (intel_dsi->dual_link)
+		count = 2;
+
+	do {
+		/* clear bit */
+		I915_WRITE(MIPI_INTR_STAT(pipe), SPL_PKT_SENT_INTERRUPT);
+
+		/* XXX: old code skips write if control unchanged */
+		if (cmd == I915_READ(MIPI_DPI_CONTROL(pipe)))
+			DRM_ERROR("Same special packet %02x twice in a row.\n",
+									cmd);
+		I915_WRITE(MIPI_DPI_CONTROL(pipe), cmd);
+
+		mask = SPL_PKT_SENT_INTERRUPT;
+		if (wait_for((I915_READ(MIPI_INTR_STAT(pipe)) & mask) ==
+								mask, 100))
+			DRM_ERROR("Video mode command 0x%08x send failed.\n",
+									cmd);
+		/* For Port C for dual link */
+		if (intel_dsi->dual_link)
+			pipe = PIPE_B;
+	} while (--count > 0);
 
 	return 0;
 }
-- 
1.7.9.5

[PATCH 6/9] drm/i915: Dsipll clk to be enabled for DSI1 in case of dual link configuration

[Gaurav K Singh]

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi_pll.c |   14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index fa7a6ca..2464089 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -243,6 +243,9 @@ static void vlv_configure_dsi_pll(struct intel_encoder *encoder)
 
 	dsi_mnp.dsi_pll_ctrl |= DSI_PLL_CLK_GATE_DSI0_DSIPLL;
 
+	if (intel_dsi->dual_link)
+		dsi_mnp.dsi_pll_ctrl |= DSI_PLL_CLK_GATE_DSI1_DSIPLL;
+
 	DRM_DEBUG_KMS("dsi pll div %08x, ctrl %08x\n",
 		      dsi_mnp.dsi_pll_div, dsi_mnp.dsi_pll_ctrl);
 
@@ -271,12 +274,11 @@ void vlv_enable_dsi_pll(struct intel_encoder *encoder)
 
 	mutex_unlock(&dev_priv->dpio_lock);
 
-	if (wait_for(I915_READ(PIPECONF(PIPE_A)) & PIPECONF_DSI_PLL_LOCKED, 20)) {
-		DRM_ERROR("DSI PLL lock failed\n");
-		return;
-	}
-
-	DRM_DEBUG_KMS("DSI PLL locked\n");
+	tmp = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL);
+	if (tmp & DSI_PLL_LOCK)
+		DRM_DEBUG_KMS("DSI PLL locked\n");
+	else
+		DRM_DEBUG_KMS("DSI PLL lock failed\n");
 }
 
 void vlv_disable_dsi_pll(struct intel_encoder *encoder)
-- 
1.7.9.5

[PATCH 7/9] drm/i915: MIPI Timings related changes for dual link Configuration

[Gaurav K Singh]

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c |   37 +++++++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 583c7fd..6aac420 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -502,12 +502,23 @@ static void set_dsi_timings(struct drm_encoder *encoder,
 	unsigned int lane_count = intel_dsi->lane_count;
 
 	u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp;
+	int count = 1;
 
 	hactive = mode->hdisplay;
 	hfp = mode->hsync_start - mode->hdisplay;
 	hsync = mode->hsync_end - mode->hsync_start;
 	hbp = mode->htotal - mode->hsync_end;
 
+	if (intel_dsi->dual_link) {
+		hactive /= 2;
+		if (intel_dsi->dual_link & MIPI_DUAL_LINK_FRONT_BACK)
+			hactive += intel_dsi->pixel_overlap;
+		hfp /= 2;
+		hsync /= 2;
+		hbp /= 2;
+		count = 2;
+	}
+
 	vfp = mode->vsync_start - mode->vdisplay;
 	vsync = mode->vsync_end - mode->vsync_start;
 	vbp = mode->vtotal - mode->vsync_end;
@@ -520,18 +531,24 @@ static void set_dsi_timings(struct drm_encoder *encoder,
 			    intel_dsi->burst_mode_ratio);
 	hbp = txbyteclkhs(hbp, bpp, lane_count, intel_dsi->burst_mode_ratio);
 
-	I915_WRITE(MIPI_HACTIVE_AREA_COUNT(pipe), hactive);
-	I915_WRITE(MIPI_HFP_COUNT(pipe), hfp);
+	do {
+		I915_WRITE(MIPI_HACTIVE_AREA_COUNT(pipe), hactive);
+		I915_WRITE(MIPI_HFP_COUNT(pipe), hfp);
+
+		/* meaningful for video mode non-burst sync pulse mode only,
+		 * can be zero for non-burst sync events and burst modes */
+		I915_WRITE(MIPI_HSYNC_PADDING_COUNT(pipe), hsync);
+		I915_WRITE(MIPI_HBP_COUNT(pipe), hbp);
 
-	/* meaningful for video mode non-burst sync pulse mode only, can be zero
-	 * for non-burst sync events and burst modes */
-	I915_WRITE(MIPI_HSYNC_PADDING_COUNT(pipe), hsync);
-	I915_WRITE(MIPI_HBP_COUNT(pipe), hbp);
+		/* vertical values are in terms of lines */
+		I915_WRITE(MIPI_VFP_COUNT(pipe), vfp);
+		I915_WRITE(MIPI_VSYNC_PADDING_COUNT(pipe), vsync);
+		I915_WRITE(MIPI_VBP_COUNT(pipe), vbp);
 
-	/* vertical values are in terms of lines */
-	I915_WRITE(MIPI_VFP_COUNT(pipe), vfp);
-	I915_WRITE(MIPI_VSYNC_PADDING_COUNT(pipe), vsync);
-	I915_WRITE(MIPI_VBP_COUNT(pipe), vbp);
+		/* For Port C for dual link */
+		if (intel_dsi->dual_link)
+			pipe = PIPE_B;
+	} while (--count > 0);
 }
 
 static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
-- 
1.7.9.5

[PATCH 8/9] drm/i915: MIPI encoder disable related changes for dual link Configuration

[Gaurav K Singh]

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c |   74 ++++++++++++++++++++++++--------------
 1 file changed, 48 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 6aac420..477b79d 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -303,6 +303,7 @@ static void intel_dsi_disable(struct intel_encoder *encoder)
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
 	int pipe = intel_crtc->pipe;
 	u32 temp;
+	int count = 1;
 
 	DRM_DEBUG_KMS("\n");
 
@@ -313,22 +314,30 @@ static void intel_dsi_disable(struct intel_encoder *encoder)
 		msleep(2);
 	}
 
-	/* Panel commands can be sent when clock is in LP11 */
-	I915_WRITE(MIPI_DEVICE_READY(pipe), 0x0);
+	if (intel_dsi->dual_link)
+		count = 2;
+	do {
+		/* Panel commands can be sent when clock is in LP11 */
+		I915_WRITE(MIPI_DEVICE_READY(pipe), 0x0);
 
-	temp = I915_READ(MIPI_CTRL(pipe));
-	temp &= ~ESCAPE_CLOCK_DIVIDER_MASK;
-	I915_WRITE(MIPI_CTRL(pipe), temp |
-		   intel_dsi->escape_clk_div <<
-		   ESCAPE_CLOCK_DIVIDER_SHIFT);
+		temp = I915_READ(MIPI_CTRL(pipe));
+		temp &= ~ESCAPE_CLOCK_DIVIDER_MASK;
+		I915_WRITE(MIPI_CTRL(pipe), temp |
+			   intel_dsi->escape_clk_div <<
+			   ESCAPE_CLOCK_DIVIDER_SHIFT);
 
-	I915_WRITE(MIPI_EOT_DISABLE(pipe), CLOCKSTOP);
+		I915_WRITE(MIPI_EOT_DISABLE(pipe), CLOCKSTOP);
 
-	temp = I915_READ(MIPI_DSI_FUNC_PRG(pipe));
-	temp &= ~VID_MODE_FORMAT_MASK;
-	I915_WRITE(MIPI_DSI_FUNC_PRG(pipe), temp);
+		temp = I915_READ(MIPI_DSI_FUNC_PRG(pipe));
+		temp &= ~VID_MODE_FORMAT_MASK;
+		I915_WRITE(MIPI_DSI_FUNC_PRG(pipe), temp);
 
-	I915_WRITE(MIPI_DEVICE_READY(pipe), 0x1);
+		I915_WRITE(MIPI_DEVICE_READY(pipe), 0x1);
+
+		/* For Port C for dual link */
+		if (intel_dsi->dual_link)
+			pipe = PIPE_B;
+	} while (--count > 0);
 
 	/* if disable packets are sent before sending shutdown packet then in
 	 * some next enable sequence send turn on packet error is observed */
@@ -341,31 +350,44 @@ static void intel_dsi_disable(struct intel_encoder *encoder)
 static void intel_dsi_clear_device_ready(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
 	int pipe = intel_crtc->pipe;
 	u32 val;
+	int count = 1;
 
 	DRM_DEBUG_KMS("\n");
 
-	I915_WRITE(MIPI_DEVICE_READY(pipe), DEVICE_READY | ULPS_STATE_ENTER);
-	usleep_range(2000, 2500);
+	if (intel_dsi->dual_link)
+		count = 2;
 
-	I915_WRITE(MIPI_DEVICE_READY(pipe), DEVICE_READY | ULPS_STATE_EXIT);
-	usleep_range(2000, 2500);
+	do {
+		I915_WRITE(MIPI_DEVICE_READY(pipe), DEVICE_READY |
+							ULPS_STATE_ENTER);
+		usleep_range(2000, 2500);
 
-	I915_WRITE(MIPI_DEVICE_READY(pipe), DEVICE_READY | ULPS_STATE_ENTER);
-	usleep_range(2000, 2500);
+		I915_WRITE(MIPI_DEVICE_READY(pipe), DEVICE_READY |
+							ULPS_STATE_EXIT);
+		usleep_range(2000, 2500);
 
-	if (wait_for(((I915_READ(MIPI_PORT_CTRL(pipe)) & AFE_LATCHOUT)
-		      == 0x00000), 30))
-		DRM_ERROR("DSI LP not going Low\n");
+		I915_WRITE(MIPI_DEVICE_READY(pipe), DEVICE_READY |
+							ULPS_STATE_ENTER);
+		usleep_range(2000, 2500);
 
-	val = I915_READ(MIPI_PORT_CTRL(pipe));
-	I915_WRITE(MIPI_PORT_CTRL(pipe), val & ~LP_OUTPUT_HOLD);
-	usleep_range(1000, 1500);
+		if (wait_for(((I915_READ(MIPI_PORT_CTRL(0)) & AFE_LATCHOUT)
+					== 0x00000), 30))
+			DRM_ERROR("DSI LP not going Low\n");
+
+		val = I915_READ(MIPI_PORT_CTRL(0));
+		I915_WRITE(MIPI_PORT_CTRL(0), val & ~LP_OUTPUT_HOLD);
+		usleep_range(1000, 1500);
 
-	I915_WRITE(MIPI_DEVICE_READY(pipe), 0x00);
-	usleep_range(2000, 2500);
+		I915_WRITE(MIPI_DEVICE_READY(pipe), 0x00);
+		usleep_range(2000, 2500);
+		/* For Port C for dual link */
+		if (intel_dsi->dual_link)
+			pipe = PIPE_B;
+	} while (--count > 0);
 
 	vlv_disable_dsi_pll(encoder);
 }
-- 
1.7.9.5

[PATCH 9/9] drm/i915: MIPI Encoder enable related changes for dual link configuration

[Gaurav K Singh]

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c |  276 ++++++++++++++++++++++----------------
 1 file changed, 161 insertions(+), 115 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 477b79d..43212d3 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -175,8 +175,10 @@ static void intel_dsi_device_ready(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
 	int pipe = intel_crtc->pipe;
 	u32 val;
+	int count = 1;
 
 	DRM_DEBUG_KMS("\n");
 
@@ -189,18 +191,26 @@ static void intel_dsi_device_ready(struct intel_encoder *encoder)
 	/* bandgap reset is needed after everytime we do power gate */
 	band_gap_reset(dev_priv);
 
-	I915_WRITE(MIPI_DEVICE_READY(pipe), ULPS_STATE_ENTER);
-	usleep_range(2500, 3000);
+	if (intel_dsi->dual_link)
+		count = 2;
+	do {
+
+		I915_WRITE(MIPI_DEVICE_READY(pipe), ULPS_STATE_ENTER);
+		usleep_range(2500, 3000);
 
-	val = I915_READ(MIPI_PORT_CTRL(pipe));
-	I915_WRITE(MIPI_PORT_CTRL(pipe), val | LP_OUTPUT_HOLD);
-	usleep_range(1000, 1500);
+		val = I915_READ(MIPI_PORT_CTRL(pipe));
+		I915_WRITE(MIPI_PORT_CTRL(pipe), val | LP_OUTPUT_HOLD);
+		usleep_range(1000, 1500);
 
-	I915_WRITE(MIPI_DEVICE_READY(pipe), ULPS_STATE_EXIT);
-	usleep_range(2500, 3000);
+		I915_WRITE(MIPI_DEVICE_READY(pipe), ULPS_STATE_EXIT);
+		usleep_range(2500, 3000);
 
-	I915_WRITE(MIPI_DEVICE_READY(pipe), DEVICE_READY);
-	usleep_range(2500, 3000);
+		I915_WRITE(MIPI_DEVICE_READY(pipe), DEVICE_READY);
+		usleep_range(2500, 3000);
+		/* For Port C for dual link */
+		if (intel_dsi->dual_link)
+			pipe = PIPE_B;
+	} while (--count > 0);
 }
 
 static void intel_dsi_enable(struct intel_encoder *encoder)
@@ -585,131 +595,167 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
 	int pipe = intel_crtc->pipe;
 	unsigned int bpp = intel_crtc->config.pipe_bpp;
 	u32 val, tmp;
+	int count = 1;
+	u16 mode_hactive;
 
 	DRM_DEBUG_KMS("pipe %c\n", pipe_name(pipe));
 
-	/* escape clock divider, 20MHz, shared for A and C. device ready must be
-	 * off when doing this! txclkesc? */
-	tmp = I915_READ(MIPI_CTRL(0));
-	tmp &= ~ESCAPE_CLOCK_DIVIDER_MASK;
-	I915_WRITE(MIPI_CTRL(0), tmp | ESCAPE_CLOCK_DIVIDER_1);
-
-	/* read request priority is per pipe */
-	tmp = I915_READ(MIPI_CTRL(pipe));
-	tmp &= ~READ_REQUEST_PRIORITY_MASK;
-	I915_WRITE(MIPI_CTRL(pipe), tmp | READ_REQUEST_PRIORITY_HIGH);
-
-	/* XXX: why here, why like this? handling in irq handler?! */
-	I915_WRITE(MIPI_INTR_STAT(pipe), 0xffffffff);
-	I915_WRITE(MIPI_INTR_EN(pipe), 0xffffffff);
-
-	I915_WRITE(MIPI_DPHY_PARAM(pipe), intel_dsi->dphy_reg);
+	mode_hactive = adjusted_mode->hdisplay;
+	if (intel_dsi->dual_link) {
+		count = 2;
+		mode_hactive /= 2;
+		if (intel_dsi->dual_link & MIPI_DUAL_LINK_FRONT_BACK)
+			mode_hactive += intel_dsi->pixel_overlap;
+	}
 
-	I915_WRITE(MIPI_DPI_RESOLUTION(pipe),
-		   adjusted_mode->vdisplay << VERTICAL_ADDRESS_SHIFT |
-		   adjusted_mode->hdisplay << HORIZONTAL_ADDRESS_SHIFT);
+	do {
+		/* escape clock divider, 20MHz, shared for A and C.
+		 * device ready must be off when doing this! txclkesc?
+		 */
+		tmp = I915_READ(MIPI_CTRL(0));
+		tmp &= ~ESCAPE_CLOCK_DIVIDER_MASK;
+		I915_WRITE(MIPI_CTRL(0), tmp | ESCAPE_CLOCK_DIVIDER_1);
+
+		/* read request priority is per pipe */
+		tmp = I915_READ(MIPI_CTRL(pipe));
+		tmp &= ~READ_REQUEST_PRIORITY_MASK;
+		I915_WRITE(MIPI_CTRL(pipe), tmp | READ_REQUEST_PRIORITY_HIGH);
+
+		/* XXX: why here, why like this? handling in irq handler?! */
+		I915_WRITE(MIPI_INTR_STAT(pipe), 0xffffffff);
+		I915_WRITE(MIPI_INTR_EN(pipe), 0xffffffff);
+
+		I915_WRITE(MIPI_DPHY_PARAM(pipe), intel_dsi->dphy_reg);
+
+		I915_WRITE(MIPI_DPI_RESOLUTION(pipe),
+			adjusted_mode->vdisplay << VERTICAL_ADDRESS_SHIFT |
+			mode_hactive << HORIZONTAL_ADDRESS_SHIFT);
+		/* For Port C for dual link */
+		if (intel_dsi->dual_link)
+			pipe = PIPE_B;
+	} while (--count > 0);
 
 	set_dsi_timings(encoder, adjusted_mode);
 
+	if (intel_dsi->dual_link) {
+		pipe = PIPE_A;
+		count = 2;
+	} else
+		count = 1;
+
 	val = intel_dsi->lane_count << DATA_LANES_PRG_REG_SHIFT;
 	if (is_cmd_mode(intel_dsi)) {
-		val |= intel_dsi->channel << CMD_MODE_CHANNEL_NUMBER_SHIFT;
+		val |= intel_dsi->channel <<
+				CMD_MODE_CHANNEL_NUMBER_SHIFT;
 		val |= CMD_MODE_DATA_WIDTH_8_BIT; /* XXX */
 	} else {
-		val |= intel_dsi->channel << VID_MODE_CHANNEL_NUMBER_SHIFT;
-
+		val |= intel_dsi->channel <<
+				VID_MODE_CHANNEL_NUMBER_SHIFT;
 		/* XXX: cross-check bpp vs. pixel format? */
 		val |= intel_dsi->pixel_format;
 	}
-	I915_WRITE(MIPI_DSI_FUNC_PRG(pipe), val);
-
-	/* timeouts for recovery. one frame IIUC. if counter expires, EOT and
-	 * stop state. */
-
-	/*
-	 * In burst mode, value greater than one DPI line Time in byte clock
-	 * (txbyteclkhs) To timeout this timer 1+ of the above said value is
-	 * recommended.
-	 *
-	 * In non-burst mode, Value greater than one DPI frame time in byte
-	 * clock(txbyteclkhs) To timeout this timer 1+ of the above said value
-	 * is recommended.
-	 *
-	 * In DBI only mode, value greater than one DBI frame time in byte
-	 * clock(txbyteclkhs) To timeout this timer 1+ of the above said value
-	 * is recommended.
-	 */
-
-	if (is_vid_mode(intel_dsi) &&
-	    intel_dsi->video_mode_format == VIDEO_MODE_BURST) {
-		I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe),
-			   txbyteclkhs(adjusted_mode->htotal, bpp,
-				       intel_dsi->lane_count,
-				       intel_dsi->burst_mode_ratio) + 1);
-	} else {
-		I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe),
-			   txbyteclkhs(adjusted_mode->vtotal *
-				       adjusted_mode->htotal,
-				       bpp, intel_dsi->lane_count,
-				       intel_dsi->burst_mode_ratio) + 1);
-	}
-	I915_WRITE(MIPI_LP_RX_TIMEOUT(pipe), intel_dsi->lp_rx_timeout);
-	I915_WRITE(MIPI_TURN_AROUND_TIMEOUT(pipe), intel_dsi->turn_arnd_val);
-	I915_WRITE(MIPI_DEVICE_RESET_TIMER(pipe), intel_dsi->rst_timer_val);
-
-	/* dphy stuff */
 
-	/* in terms of low power clock */
-	I915_WRITE(MIPI_INIT_COUNT(pipe), txclkesc(intel_dsi->escape_clk_div, 100));
-
-	val = 0;
+	tmp = 0;
 	if (intel_dsi->eotp_pkt == 0)
-		val |= EOT_DISABLE;
+		tmp |= EOT_DISABLE;
 
 	if (intel_dsi->clock_stop)
-		val |= CLOCKSTOP;
-
-	/* recovery disables */
-	I915_WRITE(MIPI_EOT_DISABLE(pipe), val);
-
-	/* in terms of low power clock */
-	I915_WRITE(MIPI_INIT_COUNT(pipe), intel_dsi->init_count);
+		tmp |= CLOCKSTOP;
 
-	/* in terms of txbyteclkhs. actual high to low switch +
-	 * MIPI_STOP_STATE_STALL * MIPI_LP_BYTECLK.
-	 *
-	 * XXX: write MIPI_STOP_STATE_STALL?
-	 */
-	I915_WRITE(MIPI_HIGH_LOW_SWITCH_COUNT(pipe),
-		   intel_dsi->hs_to_lp_count);
-
-	/* XXX: low power clock equivalence in terms of byte clock. the number
-	 * of byte clocks occupied in one low power clock. based on txbyteclkhs
-	 * and txclkesc. txclkesc time / txbyteclk time * (105 +
-	 * MIPI_STOP_STATE_STALL) / 105.???
-	 */
-	I915_WRITE(MIPI_LP_BYTECLK(pipe), intel_dsi->lp_byte_clk);
-
-	/* the bw essential for transmitting 16 long packets containing 252
-	 * bytes meant for dcs write memory command is programmed in this
-	 * register in terms of byte clocks. based on dsi transfer rate and the
-	 * number of lanes configured the time taken to transmit 16 long packets
-	 * in a dsi stream varies. */
-	I915_WRITE(MIPI_DBI_BW_CTRL(pipe), intel_dsi->bw_timer);
-
-	I915_WRITE(MIPI_CLK_LANE_SWITCH_TIME_CNT(pipe),
-		   intel_dsi->clk_lp_to_hs_count << LP_HS_SSW_CNT_SHIFT |
-		   intel_dsi->clk_hs_to_lp_count << HS_LP_PWR_SW_CNT_SHIFT);
-
-	if (is_vid_mode(intel_dsi))
-		/* Some panels might have resolution which is not a multiple of
-		 * 64 like 1366 x 768. Enable RANDOM resolution support for such
-		 * panels by default */
-		I915_WRITE(MIPI_VIDEO_MODE_FORMAT(pipe),
-			   intel_dsi->video_frmt_cfg_bits |
-			   intel_dsi->video_mode_format |
-			   IP_TG_CONFIG |
-			   RANDOM_DPI_DISPLAY_RESOLUTION);
+	do {
+		I915_WRITE(MIPI_DSI_FUNC_PRG(pipe), val);
+
+		/* timeouts for recovery. one frame IIUC. if counter expires,
+		 * EOT and stop state. */
+
+		/*
+		 * In burst mode, value greater than one DPI line Time in byte
+		 * clock (txbyteclkhs) To timeout this timer 1+ of the above
+		 * said value is recommended.
+		 *
+		 * In non-burst mode, Value greater than one DPI frame time in
+		 * byte clock(txbyteclkhs) To timeout this timer 1+ of the above
+		 * said value is recommended.
+		 *
+		 * In DBI only mode, value greater than one DBI frame time in
+		 * byte clock(txbyteclkhs) To timeout this timer 1+ of the above
+		 * said value is recommended.
+		 */
+
+		if (is_vid_mode(intel_dsi) &&
+		    intel_dsi->video_mode_format == VIDEO_MODE_BURST) {
+			I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe),
+					txbyteclkhs(adjusted_mode->htotal, bpp,
+					intel_dsi->lane_count,
+					intel_dsi->burst_mode_ratio) + 1);
+		} else {
+			I915_WRITE(MIPI_HS_TX_TIMEOUT(pipe),
+					txbyteclkhs(adjusted_mode->vtotal *
+					adjusted_mode->htotal,
+					bpp, intel_dsi->lane_count,
+					intel_dsi->burst_mode_ratio) + 1);
+		}
+		I915_WRITE(MIPI_LP_RX_TIMEOUT(pipe), intel_dsi->lp_rx_timeout);
+		I915_WRITE(MIPI_TURN_AROUND_TIMEOUT(pipe),
+					intel_dsi->turn_arnd_val);
+		I915_WRITE(MIPI_DEVICE_RESET_TIMER(pipe),
+					intel_dsi->rst_timer_val);
+
+		/* dphy stuff */
+
+		/* in terms of low power clock */
+		I915_WRITE(MIPI_INIT_COUNT(pipe),
+				txclkesc(intel_dsi->escape_clk_div, 100));
+
+
+		/* recovery disables */
+		I915_WRITE(MIPI_EOT_DISABLE(pipe), tmp);
+
+		/* in terms of low power clock */
+		I915_WRITE(MIPI_INIT_COUNT(pipe), intel_dsi->init_count);
+
+		/* in terms of txbyteclkhs. actual high to low switch +
+		 * MIPI_STOP_STATE_STALL * MIPI_LP_BYTECLK.
+		 *
+		 * XXX: write MIPI_STOP_STATE_STALL?
+		 */
+		I915_WRITE(MIPI_HIGH_LOW_SWITCH_COUNT(pipe),
+					intel_dsi->hs_to_lp_count);
+
+		/* XXX: low power clock equivalence in terms of byte clock.
+		 * the number of byte clocks occupied in one low power clock.
+		 * based on txbyteclkhs and txclkesc.
+		 * txclkesc time / txbyteclk time *
+		 * (105 + MIPI_STOP_STATE_STALL) / 105.???
+		 */
+		I915_WRITE(MIPI_LP_BYTECLK(pipe), intel_dsi->lp_byte_clk);
+
+		/* the bw essential for transmitting 16 long packets containing
+		 * 252 bytes meant for dcs write memory command is programmed in
+		 * this register in terms of byte clocks. based on dsi transfer
+		 * rate and the number of lanes configured the time taken to
+		 * transmit 16 long packets in a dsi stream varies. */
+		I915_WRITE(MIPI_DBI_BW_CTRL(pipe), intel_dsi->bw_timer);
+
+		I915_WRITE(MIPI_CLK_LANE_SWITCH_TIME_CNT(pipe),
+			intel_dsi->clk_lp_to_hs_count <<
+			LP_HS_SSW_CNT_SHIFT |
+			intel_dsi->clk_hs_to_lp_count <<
+			HS_LP_PWR_SW_CNT_SHIFT);
+
+		if (is_vid_mode(intel_dsi))
+			/* Some panels might have resolution which is not a
+			 * multiple of 64 like 1366 x 768. Enable RANDOM
+			 * resolution support for such panels by default */
+			I915_WRITE(MIPI_VIDEO_MODE_FORMAT(pipe),
+				intel_dsi->video_frmt_cfg_bits |
+				intel_dsi->video_mode_format |
+				IP_TG_CONFIG |
+				RANDOM_DPI_DISPLAY_RESOLUTION);
+		/* For Port C for dual link */
+		if (intel_dsi->dual_link)
+			pipe = PIPE_B;
+	} while (--count > 0);
 }
 
 static void intel_dsi_pre_pll_enable(struct intel_encoder *encoder)
-- 
1.7.9.5

Re: [PATCH 0/9] BYT DSI Dual Link Support

[Daniel Vetter]

On Wed, Sep 24, 2014 at 02:16:49PM +0530, Gaurav K Singh wrote:
> Hi,
> These set of patches build on top of the existing DSI Video mode support to
> enable dual link MIPI panels with high resolutions. These patches have been 
> tested on a 25x16 panel and works well.

Except for the first patch (which only has a trivial "this is prep work"
sentence) all your patches lack a commit message. If you do mechanical and
trivial changes all over the driver split up into a bunch of patches this
is ok, but for everything else it makes reviewing your code really hard
since the reviewer has no idea _why_ some change is done.

So please give your patches some good commit messages, focusing
specifically on why you change things like you do (and also e.g. why other
approaches would be worse). Also, the subject/summary should be at most 50
chars (with a hard limit at 80 characters), lots of them are longer. So
probably some of the details of what exactly a patch changes should also
be moved into the commit message.

See "2) Describe your changes" in Documentation/SubmittingPatches for a
some good examples and explanations. Also my review BKM training has some
material about what a good commit message should entail.

Thanks, Daniel

> 
> Regards
> Gaurav
> 
> Gaurav K Singh (9):
>   drm/i915: New functions added for enabling & disabling MIPI Port Ctrl
>     reg
>   drm/i915: MIPI Sequence to be sent to the DSI Controller based on the
>     port no from VBT
>   drm/i915: MIPI Port Ctrl related changes for dual link configuration
>   drm/i915: Pixel Clock and pixel overlap related changes for dual link
>     Configuration
>   drm/i915: SHUTDOWN & Turn ON packets to be sent for both MIPI Ports
>     in case of dual link Configuration
>   drm/i915: Dsipll clk to be enabled for DSI1 in case of dual link
>     configuration
>   drm/i915: MIPI Timings related changes for dual link Configuration
>   drm/i915: MIPI encoder disable related changes for dual link
>     Configuration
>   drm/i915: MIPI Encoder enable related changes for dual link
>     configuration
> 
>  drivers/gpu/drm/i915/i915_reg.h            |    5 +
>  drivers/gpu/drm/i915/intel_bios.h          |    3 +-
>  drivers/gpu/drm/i915/intel_dsi.c           |  471 ++++++++++++++++++----------
>  drivers/gpu/drm/i915/intel_dsi.h           |    8 +
>  drivers/gpu/drm/i915/intel_dsi_cmd.c       |   44 +--
>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |   25 ++
>  drivers/gpu/drm/i915/intel_dsi_pll.c       |   14 +-
>  7 files changed, 382 insertions(+), 188 deletions(-)
> 
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 4/9] drm/i915: Pixel Clock and pixel overlap related changes for dual link Configuration

[Jani Nikula]

On Wed, 24 Sep 2014, Gaurav K Singh <gaurav.k.singh@intel.com> wrote:
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h            |    4 ++++
>  drivers/gpu/drm/i915/intel_bios.h          |    3 ++-
>  drivers/gpu/drm/i915/intel_dsi.c           |    8 ++++++++
>  drivers/gpu/drm/i915/intel_dsi.h           |    6 ++++++
>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |   21 +++++++++++++++++++++
>  5 files changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 922d807..3ed5774 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5662,6 +5662,10 @@ enum punit_power_well {
>  #define GEN8_PMINTR_REDIRECT_TO_NON_DISP	(1<<31)
>  #define VLV_PWRDWNUPCTL				0xA294
>  
> +#define VLV_CHICKEN_3				0x7040C
> +#define  PIXEL_OVERLAP_CNT_MASK			(3 << 30)
> +#define  PIXEL_OVERLAP_CNT_SHIFT		30
> +
>  #define GEN6_PMISR				0x44020
>  #define GEN6_PMIMR				0x44024 /* rps_lock */
>  #define GEN6_PMIIR				0x44028
> diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
> index 7603765..39dfb65 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -798,7 +798,8 @@ struct mipi_config {
>  #define DUAL_LINK_PIXEL_ALT	2
>  	u16 dual_link:2;
>  	u16 lane_cnt:2;
> -	u16 rsvd3:12;
> +	u16 pixel_overlap:3;
> +	u16 rsvd3:9;
>  
>  	u16 rsvd4;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 3b1890e..583c7fd 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -116,6 +116,14 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
>  					<< DUAL_LINK_MODE_SHIFT;
>  		port_control |= pipe ? LANE_CONFIGURATION_DUAL_LINK_B :
>  					LANE_CONFIGURATION_DUAL_LINK_A;
> +
> +		if (intel_dsi->dual_link & MIPI_DUAL_LINK_FRONT_BACK) {

It's really confusing to treat ->dual_link as a bit mask when it's not.

> +			temp = I915_READ(VLV_CHICKEN_3);
> +			temp &= ~PIXEL_OVERLAP_CNT_MASK |
> +				intel_dsi->pixel_overlap <<
> +				PIXEL_OVERLAP_CNT_SHIFT;
> +			I915_WRITE(VLV_CHICKEN_3, temp);
> +		}
>  		/*For Port A */
>  		temp = I915_READ(MIPI_PORT_CTRL(0));
>  		temp = temp | port_control;
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index 950ab41..7fac460 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -28,6 +28,11 @@
>  #include <drm/drm_crtc.h>
>  #include "intel_drv.h"
>  
> +/* Dual Link support */
> +#define MIPI_DUAL_LINK_NONE		0
> +#define MIPI_DUAL_LINK_FRONT_BACK	1
> +#define MIPI_DUAL_LINK_PIXEL_ALT	2
> +

Spread the word, using the word "MIPI" for "DSI" feels like saying I
have an "embedded VESA" display on my laptop. Really.

>  struct intel_dsi_device {
>  	unsigned int panel_id;
>  	const char *name;
> @@ -102,6 +107,7 @@ struct intel_dsi {
>  
>  	u8 escape_clk_div;
>  	u8 dual_link;
> +	u8 pixel_overlap;
>  	u32 port_bits;
>  	u32 bw_timer;
>  	u32 dphy_reg;
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index d424ebc..8bc911b 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -284,6 +284,7 @@ static bool generic_init(struct intel_dsi_device *dsi)
>  	intel_dsi->pixel_format = mipi_config->videomode_color_format << 7;
>  	intel_dsi->port = 0;
>  	intel_dsi->dual_link = mipi_config->dual_link;
> +	intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
>  
>  	if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB666)
>  		bits_per_pixel = 18;
> @@ -303,6 +304,20 @@ static bool generic_init(struct intel_dsi_device *dsi)
>  
>  	pclk = mode->clock;
>  
> +	/* In dual link mode each port needs half of pixel clock */
> +	if (intel_dsi->dual_link) {
> +		pclk = pclk / 2;
> +
> +		/* we can enable pixel_overlap if needed by panel. In this
> +		 * case we need to increase the pixelclock for extra pixels
> +		 */
> +		if (intel_dsi->dual_link & MIPI_DUAL_LINK_FRONT_BACK) {
> +			pclk += DIV_ROUND_UP(mode->vtotal *
> +						intel_dsi->pixel_overlap *
> +						60, 1000);
> +		}
> +	}
> +
>  	/* Burst Mode Ratio
>  	 * Target ddr frequency from VBT / non burst ddr freq
>  	 * multiply by 100 to preserve remainder
> @@ -497,6 +512,12 @@ static bool generic_init(struct intel_dsi_device *dsi)
>  	DRM_DEBUG_KMS("Clockstop %s\n", intel_dsi->clock_stop ?
>  						"disabled" : "enabled");
>  	DRM_DEBUG_KMS("Mode %s\n", intel_dsi->operation_mode ? "command" : "video");
> +	if (intel_dsi->dual_link == MIPI_DUAL_LINK_FRONT_BACK)
> +		DRM_DEBUG_KMS("Dual link: MIPI_DUAL_LINK_FRONT_BACK\n");
> +	else if (intel_dsi->dual_link == MIPI_DUAL_LINK_PIXEL_ALT)
> +		DRM_DEBUG_KMS("Dual link: MIPI_DUAL_LINK_PIXEL_ALT\n");
> +	else
> +		DRM_DEBUG_KMS("Dual link: NONE\n");
>  	DRM_DEBUG_KMS("Pixel Format %d\n", intel_dsi->pixel_format);
>  	DRM_DEBUG_KMS("TLPX %d\n", intel_dsi->escape_clk_div);
>  	DRM_DEBUG_KMS("LP RX Timeout 0x%x\n", intel_dsi->lp_rx_timeout);
> -- 
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH 3/9] drm/i915: MIPI Port Ctrl related changes for dual link configuration

[Jani Nikula]

On Wed, 24 Sep 2014, Gaurav K Singh <gaurav.k.singh@intel.com> wrote:
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h            |    1 +
>  drivers/gpu/drm/i915/intel_dsi.c           |   53 ++++++++++++++++++++++------
>  drivers/gpu/drm/i915/intel_dsi.h           |    1 +
>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |    1 +
>  4 files changed, 45 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index ad8179b..922d807 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6215,6 +6215,7 @@ enum punit_power_well {
>  #define  DPI_ENABLE					(1 << 31) /* A + B */
>  #define  MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT		27
>  #define  MIPIA_MIPI4DPHY_DELAY_COUNT_MASK		(0xf << 27)
> +#define  DUAL_LINK_MODE_SHIFT				26
>  #define  DUAL_LINK_MODE_MASK				(1 << 26)
>  #define  DUAL_LINK_MODE_FRONT_BACK			(0 << 26)
>  #define  DUAL_LINK_MODE_PIXEL_ALTERNATIVE		(1 << 26)
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index e456ca9..3b1890e 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -109,13 +109,31 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
>  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>  	enum pipe pipe = intel_crtc->pipe;
> -	u32 temp;
> -
> -	/* assert ip_tg_enable signal */
> -	temp = I915_READ(MIPI_PORT_CTRL(pipe)) & ~LANE_CONFIGURATION_MASK;
> -	temp = temp | intel_dsi->port_bits;
> -	I915_WRITE(MIPI_PORT_CTRL(pipe), temp | DPI_ENABLE);
> -	POSTING_READ(MIPI_PORT_CTRL(pipe));
> +	u32 temp, port_control = 0;
> +
> +	if (intel_dsi->dual_link) {
> +		port_control = (intel_dsi->dual_link - 1)
> +					<< DUAL_LINK_MODE_SHIFT;
> +		port_control |= pipe ? LANE_CONFIGURATION_DUAL_LINK_B :
> +					LANE_CONFIGURATION_DUAL_LINK_A;
> +		/*For Port A */
> +		temp = I915_READ(MIPI_PORT_CTRL(0));
> +		temp = temp | port_control;
> +		I915_WRITE(MIPI_PORT_CTRL(0), temp | DPI_ENABLE);
> +		POSTING_READ(MIPI_PORT_CTRL(0));
> +
> +		/* For Port C */
> +		temp = I915_READ(MIPI_PORT_CTRL(1));
> +		I915_WRITE(MIPI_PORT_CTRL(1), temp | DPI_ENABLE);
> +		POSTING_READ(MIPI_PORT_CTRL(1));

This calls for a cleanup in i915_reg.h for per port vs. per transcoder
registers. MIPI_PORT_CTRL(1) uses _TRANSCODER macro. We also have enum
port with PORT_C == 2. This gets confusing.

> +	} else {
> +		/* assert ip_tg_enable signal */
> +		temp = I915_READ(MIPI_PORT_CTRL(pipe)) &
> +					~LANE_CONFIGURATION_MASK;
> +		temp = temp | intel_dsi->port_bits;
> +		I915_WRITE(MIPI_PORT_CTRL(pipe), temp | DPI_ENABLE);
> +		POSTING_READ(MIPI_PORT_CTRL(pipe));
> +	}
>  }
>  
>  static void intel_dsi_port_disable(struct intel_encoder *encoder)
> @@ -123,13 +141,26 @@ static void intel_dsi_port_disable(struct intel_encoder *encoder)
>  	struct drm_device *dev = encoder->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>  	enum pipe pipe = intel_crtc->pipe;
>  	u32 temp;
>  
> -	/* de-assert ip_tg_enable signal */
> -	temp = I915_READ(MIPI_PORT_CTRL(pipe));
> -	I915_WRITE(MIPI_PORT_CTRL(pipe), temp & ~DPI_ENABLE);
> -	POSTING_READ(MIPI_PORT_CTRL(pipe));
> +	if (intel_dsi->dual_link) {
> +		/*For Port A */
> +		temp = I915_READ(MIPI_PORT_CTRL(0));
> +		I915_WRITE(MIPI_PORT_CTRL(0), temp & ~DPI_ENABLE);
> +		POSTING_READ(MIPI_PORT_CTRL(0));
> +
> +		/* For Port C */
> +		temp = I915_READ(MIPI_PORT_CTRL(1));
> +		I915_WRITE(MIPI_PORT_CTRL(1), temp & ~DPI_ENABLE);
> +		POSTING_READ(MIPI_PORT_CTRL(1));
> +	} else {
> +		/* de-assert ip_tg_enable signal */
> +		temp = I915_READ(MIPI_PORT_CTRL(pipe));
> +		I915_WRITE(MIPI_PORT_CTRL(pipe), temp & ~DPI_ENABLE);
> +		POSTING_READ(MIPI_PORT_CTRL(pipe));
> +	}
>  }
>  
>  static void intel_dsi_device_ready(struct intel_encoder *encoder)
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index 587e71f..950ab41 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -101,6 +101,7 @@ struct intel_dsi {
>  	u8 clock_stop;
>  
>  	u8 escape_clk_div;
> +	u8 dual_link;
>  	u32 port_bits;
>  	u32 bw_timer;
>  	u32 dphy_reg;
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 051bfff..d424ebc 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -283,6 +283,7 @@ static bool generic_init(struct intel_dsi_device *dsi)
>  	intel_dsi->lane_count = mipi_config->lane_cnt + 1;
>  	intel_dsi->pixel_format = mipi_config->videomode_color_format << 7;
>  	intel_dsi->port = 0;
> +	intel_dsi->dual_link = mipi_config->dual_link;
>  
>  	if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB666)
>  		bits_per_pixel = 18;
> -- 
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH 5/9] drm/i915: SHUTDOWN & Turn ON packets to be sent for both MIPI Ports in case of dual link Configuration

[Jani Nikula]

On Wed, 24 Sep 2014, Gaurav K Singh <gaurav.k.singh@intel.com> wrote:
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi_cmd.c |   35 ++++++++++++++++++++++------------
>  1 file changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi_cmd.c b/drivers/gpu/drm/i915/intel_dsi_cmd.c
> index eb698b1..a70656e 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_cmd.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_cmd.c
> @@ -394,6 +394,7 @@ int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs)
>  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
>  	enum pipe pipe = intel_crtc->pipe;
>  	u32 mask;
> +	int count = 1;
>  
>  	/* XXX: pipe, hs */
>  	if (hs)
> @@ -401,18 +402,28 @@ int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs)
>  	else
>  		cmd |= DPI_LP_MODE;
>  
> -	/* clear bit */
> -	I915_WRITE(MIPI_INTR_STAT(pipe), SPL_PKT_SENT_INTERRUPT);
> -
> -	/* XXX: old code skips write if control unchanged */
> -	if (cmd == I915_READ(MIPI_DPI_CONTROL(pipe)))
> -		DRM_ERROR("Same special packet %02x twice in a row.\n", cmd);
> -
> -	I915_WRITE(MIPI_DPI_CONTROL(pipe), cmd);
> -
> -	mask = SPL_PKT_SENT_INTERRUPT;
> -	if (wait_for((I915_READ(MIPI_INTR_STAT(pipe)) & mask) == mask, 100))
> -		DRM_ERROR("Video mode command 0x%08x send failed.\n", cmd);
> +	if (intel_dsi->dual_link)
> +		count = 2;
> +
> +	do {

Please never use a do-while when a regular for loop will do.

> +		/* clear bit */
> +		I915_WRITE(MIPI_INTR_STAT(pipe), SPL_PKT_SENT_INTERRUPT);
> +
> +		/* XXX: old code skips write if control unchanged */
> +		if (cmd == I915_READ(MIPI_DPI_CONTROL(pipe)))
> +			DRM_ERROR("Same special packet %02x twice in a row.\n",
> +									cmd);
> +		I915_WRITE(MIPI_DPI_CONTROL(pipe), cmd);
> +
> +		mask = SPL_PKT_SENT_INTERRUPT;
> +		if (wait_for((I915_READ(MIPI_INTR_STAT(pipe)) & mask) ==
> +								mask, 100))
> +			DRM_ERROR("Video mode command 0x%08x send failed.\n",
> +									cmd);
> +		/* For Port C for dual link */
> +		if (intel_dsi->dual_link)
> +			pipe = PIPE_B;
> +	} while (--count > 0);
>  
>  	return 0;
>  }
> -- 
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH 6/9] drm/i915: Dsipll clk to be enabled for DSI1 in case of dual link configuration

[Jani Nikula]

On Wed, 24 Sep 2014, Gaurav K Singh <gaurav.k.singh@intel.com> wrote:
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi_pll.c |   14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index fa7a6ca..2464089 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -243,6 +243,9 @@ static void vlv_configure_dsi_pll(struct intel_encoder *encoder)
>  
>  	dsi_mnp.dsi_pll_ctrl |= DSI_PLL_CLK_GATE_DSI0_DSIPLL;
>  
> +	if (intel_dsi->dual_link)
> +		dsi_mnp.dsi_pll_ctrl |= DSI_PLL_CLK_GATE_DSI1_DSIPLL;
> +
>  	DRM_DEBUG_KMS("dsi pll div %08x, ctrl %08x\n",
>  		      dsi_mnp.dsi_pll_div, dsi_mnp.dsi_pll_ctrl);
>  
> @@ -271,12 +274,11 @@ void vlv_enable_dsi_pll(struct intel_encoder *encoder)
>  
>  	mutex_unlock(&dev_priv->dpio_lock);
>  
> -	if (wait_for(I915_READ(PIPECONF(PIPE_A)) & PIPECONF_DSI_PLL_LOCKED, 20)) {
> -		DRM_ERROR("DSI PLL lock failed\n");
> -		return;
> -	}
> -
> -	DRM_DEBUG_KMS("DSI PLL locked\n");
> +	tmp = vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL);

Is there no need to wait for some timeout for the pll to lock?

> +	if (tmp & DSI_PLL_LOCK)
> +		DRM_DEBUG_KMS("DSI PLL locked\n");
> +	else
> +		DRM_DEBUG_KMS("DSI PLL lock failed\n");

Please keep the happy day scenario without indent, and bail out on the
fail paths.

>  }
>  
>  void vlv_disable_dsi_pll(struct intel_encoder *encoder)
> -- 
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH 0/9] BYT DSI Dual Link Support

[Shobhit Kumar]

On Wednesday 24 September 2014 02:31 PM, Daniel Vetter wrote:
> On Wed, Sep 24, 2014 at 02:16:49PM +0530, Gaurav K Singh wrote:
>> Hi,
>> These set of patches build on top of the existing DSI Video mode support to
>> enable dual link MIPI panels with high resolutions. These patches have been
>> tested on a 25x16 panel and works well.
>
> Except for the first patch (which only has a trivial "this is prep work"
> sentence) all your patches lack a commit message. If you do mechanical and
> trivial changes all over the driver split up into a bunch of patches this
> is ok, but for everything else it makes reviewing your code really hard
> since the reviewer has no idea _why_ some change is done.
>
> So please give your patches some good commit messages, focusing
> specifically on why you change things like you do (and also e.g. why other
> approaches would be worse). Also, the subject/summary should be at most 50
> chars (with a hard limit at 80 characters), lots of them are longer. So
> probably some of the details of what exactly a patch changes should also
> be moved into the commit message.
>
> See "2) Describe your changes" in Documentation/SubmittingPatches for a
> some good examples and explanations. Also my review BKM training has some
> material about what a good commit message should entail.
>

Yeah I agree, more information is needed and we will submit patches 
again after addressing comments from Jani as well.

Regards
Shobhit

Re: [PATCH 5/9] drm/i915: SHUTDOWN & Turn ON packets to be sent for both MIPI Ports in case of dual link Configuration

[Shobhit Kumar]

On Wednesday 24 September 2014 03:02 PM, Jani Nikula wrote:
> On Wed, 24 Sep 2014, Gaurav K Singh <gaurav.k.singh@intel.com> wrote:
>> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_dsi_cmd.c |   35 ++++++++++++++++++++++------------
>>   1 file changed, 23 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dsi_cmd.c b/drivers/gpu/drm/i915/intel_dsi_cmd.c
>> index eb698b1..a70656e 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi_cmd.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi_cmd.c
>> @@ -394,6 +394,7 @@ int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs)
>>   	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
>>   	enum pipe pipe = intel_crtc->pipe;
>>   	u32 mask;
>> +	int count = 1;
>>
>>   	/* XXX: pipe, hs */
>>   	if (hs)
>> @@ -401,18 +402,28 @@ int dpi_send_cmd(struct intel_dsi *intel_dsi, u32 cmd, bool hs)
>>   	else
>>   		cmd |= DPI_LP_MODE;
>>
>> -	/* clear bit */
>> -	I915_WRITE(MIPI_INTR_STAT(pipe), SPL_PKT_SENT_INTERRUPT);
>> -
>> -	/* XXX: old code skips write if control unchanged */
>> -	if (cmd == I915_READ(MIPI_DPI_CONTROL(pipe)))
>> -		DRM_ERROR("Same special packet %02x twice in a row.\n", cmd);
>> -
>> -	I915_WRITE(MIPI_DPI_CONTROL(pipe), cmd);
>> -
>> -	mask = SPL_PKT_SENT_INTERRUPT;
>> -	if (wait_for((I915_READ(MIPI_INTR_STAT(pipe)) & mask) == mask, 100))
>> -		DRM_ERROR("Video mode command 0x%08x send failed.\n", cmd);
>> +	if (intel_dsi->dual_link)
>> +		count = 2;
>> +
>> +	do {
>
> Please never use a do-while when a regular for loop will do.

Hmm, ok but reasoning ? Point here is that anyway we have to do once for 
first port and do..while helps maintain that simple flow

All other comments are valid. Thanks for them and will address them in 
next series after I update with more details in the commit messages. 
Guess that would have to be sometime next week now due to other 
priorities atm.

Regards
Shobhit

Re: [PATCH 5/9] drm/i915: SHUTDOWN & Turn ON packets to be sent for both MIPI Ports in case of dual link Configuration

[Jani Nikula]

On Thu, 25 Sep 2014, Shobhit Kumar <shobhit.kumar@linux.intel.com> wrote:
> On Wednesday 24 September 2014 03:02 PM, Jani Nikula wrote:
>> On Wed, 24 Sep 2014, Gaurav K Singh <gaurav.k.singh@intel.com> wrote:
>>> +	do {
>>
>> Please never use a do-while when a regular for loop will do.
>
> Hmm, ok but reasoning ? Point here is that anyway we have to do once for 
> first port and do..while helps maintain that simple flow

Okay, this is subjective. It's my opinion that for doing things N times
in C, the basic for (i = 0; i < N; i++) *is* the paradigm to use. A
sub-second glance at that, and you know what it does. Not so with do {
... } while (--count > 0), particularly when the block has lots of
stuff.

So I'd go with something like:

	for (i = 0; i < intel_dsi->dual_link ? 2 : 1; i++)

where it's immediately obvious that this stuff is done twice for dual
link. Makes sense, right?

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH 5/9] drm/i915: SHUTDOWN & Turn ON packets to be sent for both MIPI Ports in case of dual link Configuration

[Shobhit Kumar]

On Thursday 25 September 2014 07:09 PM, Jani Nikula wrote:
> On Thu, 25 Sep 2014, Shobhit Kumar <shobhit.kumar@linux.intel.com> wrote:
>> On Wednesday 24 September 2014 03:02 PM, Jani Nikula wrote:
>>> On Wed, 24 Sep 2014, Gaurav K Singh <gaurav.k.singh@intel.com> wrote:
>>>> +	do {
>>>
>>> Please never use a do-while when a regular for loop will do.
>>
>> Hmm, ok but reasoning ? Point here is that anyway we have to do once for
>> first port and do..while helps maintain that simple flow
>
> Okay, this is subjective. It's my opinion that for doing things N times
> in C, the basic for (i = 0; i < N; i++) *is* the paradigm to use. A
> sub-second glance at that, and you know what it does. Not so with do {
> ... } while (--count > 0), particularly when the block has lots of
> stuff.

Well the do..while I felt makes it obvious that the first iteration has 
to be always done anyways and second is optional depeding on the panel 
and thats why I used it,

>
> So I'd go with something like:
>
> 	for (i = 0; i < intel_dsi->dual_link ? 2 : 1; i++)
>
> where it's immediately obvious that this stuff is done twice for dual
> link. Makes sense, right?

but the way you put across your for loop does make me feel that it makes 
the whole logic much more clear. Will do the change. Thanks.

Regards
Shobhit

Re: [PATCH 3/9] drm/i915: MIPI Port Ctrl related changes for dual link configuration

[Singh, Gaurav K]

On 9/24/2014 2:57 PM, Jani Nikula wrote:
> On Wed, 24 Sep 2014, Gaurav K Singh <gaurav.k.singh@intel.com> wrote:
>> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h            |    1 +
>>   drivers/gpu/drm/i915/intel_dsi.c           |   53 ++++++++++++++++++++++------
>>   drivers/gpu/drm/i915/intel_dsi.h           |    1 +
>>   drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |    1 +
>>   4 files changed, 45 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index ad8179b..922d807 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6215,6 +6215,7 @@ enum punit_power_well {
>>   #define  DPI_ENABLE					(1 << 31) /* A + B */
>>   #define  MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT		27
>>   #define  MIPIA_MIPI4DPHY_DELAY_COUNT_MASK		(0xf << 27)
>> +#define  DUAL_LINK_MODE_SHIFT				26
>>   #define  DUAL_LINK_MODE_MASK				(1 << 26)
>>   #define  DUAL_LINK_MODE_FRONT_BACK			(0 << 26)
>>   #define  DUAL_LINK_MODE_PIXEL_ALTERNATIVE		(1 << 26)
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> index e456ca9..3b1890e 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -109,13 +109,31 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
>>   	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>>   	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>>   	enum pipe pipe = intel_crtc->pipe;
>> -	u32 temp;
>> -
>> -	/* assert ip_tg_enable signal */
>> -	temp = I915_READ(MIPI_PORT_CTRL(pipe)) & ~LANE_CONFIGURATION_MASK;
>> -	temp = temp | intel_dsi->port_bits;
>> -	I915_WRITE(MIPI_PORT_CTRL(pipe), temp | DPI_ENABLE);
>> -	POSTING_READ(MIPI_PORT_CTRL(pipe));
>> +	u32 temp, port_control = 0;
>> +
>> +	if (intel_dsi->dual_link) {
>> +		port_control = (intel_dsi->dual_link - 1)
>> +					<< DUAL_LINK_MODE_SHIFT;
>> +		port_control |= pipe ? LANE_CONFIGURATION_DUAL_LINK_B :
>> +					LANE_CONFIGURATION_DUAL_LINK_A;
>> +		/*For Port A */
>> +		temp = I915_READ(MIPI_PORT_CTRL(0));
>> +		temp = temp | port_control;
>> +		I915_WRITE(MIPI_PORT_CTRL(0), temp | DPI_ENABLE);
>> +		POSTING_READ(MIPI_PORT_CTRL(0));
>> +
>> +		/* For Port C */
>> +		temp = I915_READ(MIPI_PORT_CTRL(1));
>> +		I915_WRITE(MIPI_PORT_CTRL(1), temp | DPI_ENABLE);
>> +		POSTING_READ(MIPI_PORT_CTRL(1));
> This calls for a cleanup in i915_reg.h for per port vs. per transcoder
> registers. MIPI_PORT_CTRL(1) uses _TRANSCODER macro. We also have enum
> port with PORT_C == 2. This gets confusing.

Are you suggesting to use _PORT macro instead of _TRANSCODER macro?
>
>> +	} else {
>> +		/* assert ip_tg_enable signal */
>> +		temp = I915_READ(MIPI_PORT_CTRL(pipe)) &
>> +					~LANE_CONFIGURATION_MASK;
>> +		temp = temp | intel_dsi->port_bits;
>> +		I915_WRITE(MIPI_PORT_CTRL(pipe), temp | DPI_ENABLE);
>> +		POSTING_READ(MIPI_PORT_CTRL(pipe));
>> +	}
>>   }
>>   
>>   static void intel_dsi_port_disable(struct intel_encoder *encoder)
>> @@ -123,13 +141,26 @@ static void intel_dsi_port_disable(struct intel_encoder *encoder)
>>   	struct drm_device *dev = encoder->base.dev;
>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>>   	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>> +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>>   	enum pipe pipe = intel_crtc->pipe;
>>   	u32 temp;
>>   
>> -	/* de-assert ip_tg_enable signal */
>> -	temp = I915_READ(MIPI_PORT_CTRL(pipe));
>> -	I915_WRITE(MIPI_PORT_CTRL(pipe), temp & ~DPI_ENABLE);
>> -	POSTING_READ(MIPI_PORT_CTRL(pipe));
>> +	if (intel_dsi->dual_link) {
>> +		/*For Port A */
>> +		temp = I915_READ(MIPI_PORT_CTRL(0));
>> +		I915_WRITE(MIPI_PORT_CTRL(0), temp & ~DPI_ENABLE);
>> +		POSTING_READ(MIPI_PORT_CTRL(0));
>> +
>> +		/* For Port C */
>> +		temp = I915_READ(MIPI_PORT_CTRL(1));
>> +		I915_WRITE(MIPI_PORT_CTRL(1), temp & ~DPI_ENABLE);
>> +		POSTING_READ(MIPI_PORT_CTRL(1));
>> +	} else {
>> +		/* de-assert ip_tg_enable signal */
>> +		temp = I915_READ(MIPI_PORT_CTRL(pipe));
>> +		I915_WRITE(MIPI_PORT_CTRL(pipe), temp & ~DPI_ENABLE);
>> +		POSTING_READ(MIPI_PORT_CTRL(pipe));
>> +	}
>>   }
>>   
>>   static void intel_dsi_device_ready(struct intel_encoder *encoder)
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
>> index 587e71f..950ab41 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.h
>> +++ b/drivers/gpu/drm/i915/intel_dsi.h
>> @@ -101,6 +101,7 @@ struct intel_dsi {
>>   	u8 clock_stop;
>>   
>>   	u8 escape_clk_div;
>> +	u8 dual_link;
>>   	u32 port_bits;
>>   	u32 bw_timer;
>>   	u32 dphy_reg;
>> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> index 051bfff..d424ebc 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> @@ -283,6 +283,7 @@ static bool generic_init(struct intel_dsi_device *dsi)
>>   	intel_dsi->lane_count = mipi_config->lane_cnt + 1;
>>   	intel_dsi->pixel_format = mipi_config->videomode_color_format << 7;
>>   	intel_dsi->port = 0;
>> +	intel_dsi->dual_link = mipi_config->dual_link;
>>   
>>   	if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB666)
>>   		bits_per_pixel = 18;
>> -- 
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/9] drm/i915: MIPI Port Ctrl related changes for dual link configuration

[Daniel Vetter]

On Tue, Oct 21, 2014 at 12:00:30PM +0530, Singh, Gaurav K wrote:
> 
> On 9/24/2014 2:57 PM, Jani Nikula wrote:
> >On Wed, 24 Sep 2014, Gaurav K Singh <gaurav.k.singh@intel.com> wrote:
> >>Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> >>Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> >>---
> >>  drivers/gpu/drm/i915/i915_reg.h            |    1 +
> >>  drivers/gpu/drm/i915/intel_dsi.c           |   53 ++++++++++++++++++++++------
> >>  drivers/gpu/drm/i915/intel_dsi.h           |    1 +
> >>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |    1 +
> >>  4 files changed, 45 insertions(+), 11 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >>index ad8179b..922d807 100644
> >>--- a/drivers/gpu/drm/i915/i915_reg.h
> >>+++ b/drivers/gpu/drm/i915/i915_reg.h
> >>@@ -6215,6 +6215,7 @@ enum punit_power_well {
> >>  #define  DPI_ENABLE					(1 << 31) /* A + B */
> >>  #define  MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT		27
> >>  #define  MIPIA_MIPI4DPHY_DELAY_COUNT_MASK		(0xf << 27)
> >>+#define  DUAL_LINK_MODE_SHIFT				26
> >>  #define  DUAL_LINK_MODE_MASK				(1 << 26)
> >>  #define  DUAL_LINK_MODE_FRONT_BACK			(0 << 26)
> >>  #define  DUAL_LINK_MODE_PIXEL_ALTERNATIVE		(1 << 26)
> >>diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> >>index e456ca9..3b1890e 100644
> >>--- a/drivers/gpu/drm/i915/intel_dsi.c
> >>+++ b/drivers/gpu/drm/i915/intel_dsi.c
> >>@@ -109,13 +109,31 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
> >>  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> >>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> >>  	enum pipe pipe = intel_crtc->pipe;
> >>-	u32 temp;
> >>-
> >>-	/* assert ip_tg_enable signal */
> >>-	temp = I915_READ(MIPI_PORT_CTRL(pipe)) & ~LANE_CONFIGURATION_MASK;
> >>-	temp = temp | intel_dsi->port_bits;
> >>-	I915_WRITE(MIPI_PORT_CTRL(pipe), temp | DPI_ENABLE);
> >>-	POSTING_READ(MIPI_PORT_CTRL(pipe));
> >>+	u32 temp, port_control = 0;
> >>+
> >>+	if (intel_dsi->dual_link) {
> >>+		port_control = (intel_dsi->dual_link - 1)
> >>+					<< DUAL_LINK_MODE_SHIFT;
> >>+		port_control |= pipe ? LANE_CONFIGURATION_DUAL_LINK_B :
> >>+					LANE_CONFIGURATION_DUAL_LINK_A;
> >>+		/*For Port A */
> >>+		temp = I915_READ(MIPI_PORT_CTRL(0));
> >>+		temp = temp | port_control;
> >>+		I915_WRITE(MIPI_PORT_CTRL(0), temp | DPI_ENABLE);
> >>+		POSTING_READ(MIPI_PORT_CTRL(0));
> >>+
> >>+		/* For Port C */
> >>+		temp = I915_READ(MIPI_PORT_CTRL(1));
> >>+		I915_WRITE(MIPI_PORT_CTRL(1), temp | DPI_ENABLE);
> >>+		POSTING_READ(MIPI_PORT_CTRL(1));
> >This calls for a cleanup in i915_reg.h for per port vs. per transcoder
> >registers. MIPI_PORT_CTRL(1) uses _TRANSCODER macro. We also have enum
> >port with PORT_C == 2. This gets confusing.
> 
> Are you suggesting to use _PORT macro instead of _TRANSCODER macro?

I think so, and given that we already have some offenders the existing
code should be converted with a prep patch first.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 3/9] drm/i915: MIPI Port Ctrl related changes for dual link configuration

[Jani Nikula]

On Tue, 21 Oct 2014, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Oct 21, 2014 at 12:00:30PM +0530, Singh, Gaurav K wrote:
>> 
>> On 9/24/2014 2:57 PM, Jani Nikula wrote:
>> >On Wed, 24 Sep 2014, Gaurav K Singh <gaurav.k.singh@intel.com> wrote:
>> >>Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
>> >>Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>> >>---
>> >>  drivers/gpu/drm/i915/i915_reg.h            |    1 +
>> >>  drivers/gpu/drm/i915/intel_dsi.c           |   53 ++++++++++++++++++++++------
>> >>  drivers/gpu/drm/i915/intel_dsi.h           |    1 +
>> >>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |    1 +
>> >>  4 files changed, 45 insertions(+), 11 deletions(-)
>> >>
>> >>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> >>index ad8179b..922d807 100644
>> >>--- a/drivers/gpu/drm/i915/i915_reg.h
>> >>+++ b/drivers/gpu/drm/i915/i915_reg.h
>> >>@@ -6215,6 +6215,7 @@ enum punit_power_well {
>> >>  #define  DPI_ENABLE					(1 << 31) /* A + B */
>> >>  #define  MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT		27
>> >>  #define  MIPIA_MIPI4DPHY_DELAY_COUNT_MASK		(0xf << 27)
>> >>+#define  DUAL_LINK_MODE_SHIFT				26
>> >>  #define  DUAL_LINK_MODE_MASK				(1 << 26)
>> >>  #define  DUAL_LINK_MODE_FRONT_BACK			(0 << 26)
>> >>  #define  DUAL_LINK_MODE_PIXEL_ALTERNATIVE		(1 << 26)
>> >>diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> >>index e456ca9..3b1890e 100644
>> >>--- a/drivers/gpu/drm/i915/intel_dsi.c
>> >>+++ b/drivers/gpu/drm/i915/intel_dsi.c
>> >>@@ -109,13 +109,31 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
>> >>  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>> >>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>> >>  	enum pipe pipe = intel_crtc->pipe;
>> >>-	u32 temp;
>> >>-
>> >>-	/* assert ip_tg_enable signal */
>> >>-	temp = I915_READ(MIPI_PORT_CTRL(pipe)) & ~LANE_CONFIGURATION_MASK;
>> >>-	temp = temp | intel_dsi->port_bits;
>> >>-	I915_WRITE(MIPI_PORT_CTRL(pipe), temp | DPI_ENABLE);
>> >>-	POSTING_READ(MIPI_PORT_CTRL(pipe));
>> >>+	u32 temp, port_control = 0;
>> >>+
>> >>+	if (intel_dsi->dual_link) {
>> >>+		port_control = (intel_dsi->dual_link - 1)
>> >>+					<< DUAL_LINK_MODE_SHIFT;
>> >>+		port_control |= pipe ? LANE_CONFIGURATION_DUAL_LINK_B :
>> >>+					LANE_CONFIGURATION_DUAL_LINK_A;
>> >>+		/*For Port A */
>> >>+		temp = I915_READ(MIPI_PORT_CTRL(0));
>> >>+		temp = temp | port_control;
>> >>+		I915_WRITE(MIPI_PORT_CTRL(0), temp | DPI_ENABLE);
>> >>+		POSTING_READ(MIPI_PORT_CTRL(0));
>> >>+
>> >>+		/* For Port C */
>> >>+		temp = I915_READ(MIPI_PORT_CTRL(1));
>> >>+		I915_WRITE(MIPI_PORT_CTRL(1), temp | DPI_ENABLE);
>> >>+		POSTING_READ(MIPI_PORT_CTRL(1));
>> >This calls for a cleanup in i915_reg.h for per port vs. per transcoder
>> >registers. MIPI_PORT_CTRL(1) uses _TRANSCODER macro. We also have enum
>> >port with PORT_C == 2. This gets confusing.
>> 
>> Are you suggesting to use _PORT macro instead of _TRANSCODER macro?
>
> I think so, and given that we already have some offenders the existing
> code should be converted with a prep patch first.

I guess what we have now works fine for single-link modes, with fixed
(per HW limitation IIUC) mappings MIPI DSI port A to pipe A and MIPI DSI
port C to pipe B. This particular problem probably leads back to my
original DSI enabling patches. (Except they were _PIPE back then, since
converted to _TRANSCODER.)

The dual-link config works with either pipe. I think you'll need some
helpers to do the mappings. Given a "dsi_config" - which may be just
intel_dsi->dual_link - you could have:

	for_each_dsi_port_per_pipe(port, pipe, dsi_config) {
	}

which would do one of:

	1) one iteration with PORT_A for PIPE_A in single-link
	2) one iteration with PORT_C for PIPE_B in single-link
	3) two iterations first PORT_A then PORT_C in dual-link

Maybe. Just an idea to help out. Maybe look at what's needed in the
future. Some places obviously need to have the dual link config spread
out due to differences in the registers and set up.

In any case it's bound to end in tears if we pass just magic 0 and 1
instead of PORT_A and PORT_C (== 2) to the macros.


BR,
Jani.




> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Jani Nikula, Intel Open Source Technology Center