[GLK MIPI DSI V2 0/9] GLK MIPI DSI VIDEO MODE PATCHES

[GLK MIPI DSI V2 0/9] GLK MIPI DSI VIDEO MODE PATCHES

[Madhav Chauhan]

The patches in this list enable MIPI DSI video mode
support for GLK platform. Tesed locally.
v2: Renamed bitfields macros as per review comments(Jani)

Deepak M (7):
  drm/i915/glk: Add new bit fields in MIPI CTRL register
  drm/i915/glk: Program new MIPI DSI PHY registers for GLK
  drm/i915/glk: Add MIPIIO Enable/disable sequence
  drm/i915: Set the Z inversion overlap field
  drm/i915/glk: Add DSI PLL divider range for glk
  drm/i915i/glk: Program MIPI_CLOCK_CTRL only for BXT
  drm/i915/glk: Program txesc clock divider for GLK

Madhav Chauhan (1):
  drm/i915/glk: Program dphy param reg for GLK

Vincente Tsou (1):
  drm/915: Parsing the missed out DTD fields from the VBT

 drivers/gpu/drm/i915/i915_reg.h            |  42 ++++++++
 drivers/gpu/drm/i915/intel_bios.c          |   8 +-
 drivers/gpu/drm/i915/intel_dsi.c           | 166 ++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |  33 ++++--
 drivers/gpu/drm/i915/intel_dsi_pll.c       | 106 ++++++++++++++----
 drivers/gpu/drm/i915/intel_vbt_defs.h      |   6 +-
 6 files changed, 327 insertions(+), 34 deletions(-)

-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[GLK MIPI DSI V2 1/9] drm/i915/glk: Add new bit fields in MIPI CTRL register

[Madhav Chauhan]

From: Deepak M <m.deepak@intel.com>

v2: Addressed Jani's Review comments (renamed bit field macros)

Signed-off-by: Deepak M <m.deepak@intel.com>
Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 90685d2..8e47b59 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8672,6 +8672,21 @@ enum {
 #define  BXT_PIPE_SELECT_SHIFT				7
 #define  BXT_PIPE_SELECT_MASK				(7 << 7)
 #define  BXT_PIPE_SELECT(pipe)				((pipe) << 7)
+#define  GLK_PHY_STATUS_PORT_READY			(1 << 31) /* RO */
+#define  GLK_ULPS_NOT_ACTIVE				(1 << 30) /* RO */
+#define  GLK_MIPIIO_RESET_RELEASED			(1 << 28)
+#define  GLK_CLOCK_LANE_STOP_STATE			(1 << 27) /* RO */
+#define  GLK_DATA_LANE_STOP_STATE			(1 << 26) /* RO */
+#define  GLK_LP_WAKE					(1 << 22)
+#define  GLK_LP11_LOW_PWR_MODE				(1 << 21)
+#define  GLK_LP00_LOW_PWR_MODE				(1 << 20)
+#define  GLK_FIREWALL_ENABLE				(1 << 16)
+#define  BXT_PIXEL_OVERLAP_CNT_MASK			(0xf << 10)
+#define  BXT_PIXEL_OVERLAP_CNT_SHIFT			10
+#define  BXT_DSC_ENABLE					(1 << 3)
+#define  BXT_RGB_FLIP					(1 << 2)
+#define  GLK_MIPIIO_PORT_POWERED			(1 << 1) /* RO */
+#define  GLK_MIPIIO_ENABLE				(1 << 0)
 
 #define _MIPIA_DATA_ADDRESS		(dev_priv->mipi_mmio_base + 0xb108)
 #define _MIPIC_DATA_ADDRESS		(dev_priv->mipi_mmio_base + 0xb908)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[GLK MIPI DSI V2 2/9] drm/i915/glk: Program new MIPI DSI PHY registers for GLK

[Madhav Chauhan]

From: Deepak M <m.deepak@intel.com>

Program the clk lane and tlpx time count registers
to configure DSI PHY.

v2: Addressed Jani's Review comments(renamed bit field macros)

Signed-off-by: Deepak M <m.deepak@intel.com>
Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  | 18 ++++++++++++++++++
 drivers/gpu/drm/i915/intel_dsi.c | 15 +++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8e47b59..03858f9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8550,6 +8550,24 @@ enum {
 #define  LP_BYTECLK_SHIFT				0
 #define  LP_BYTECLK_MASK				(0xffff << 0)
 
+#define _MIPIA_TLPX_TIME_COUNT		(dev_priv->mipi_mmio_base + 0xb0a4)
+#define _MIPIC_TLPX_TIME_COUNT		(dev_priv->mipi_mmio_base + 0xb8a4)
+#define MIPI_TLPX_TIME_COUNT(port)	 _MMIO_MIPI(port, _MIPIA_TLPX_TIME_COUNT, _MIPIC_TLPX_TIME_COUNT)
+#define  GLK_DPHY_TLPX_TIME_CNT_SHIFT			0
+#define  GLK_DPHY_TLPX_TIME_CNT_MASK			(0xff << 0)
+
+#define _MIPIA_CLK_LANE_TIMING		(dev_priv->mipi_mmio_base + 0xb098)
+#define _MIPIC_CLK_LANE_TIMING		(dev_priv->mipi_mmio_base + 0xb898)
+#define MIPI_CLK_LANE_TIMING(port)	 _MMIO_MIPI(port, _MIPIA_CLK_LANE_TIMING, _MIPIC_CLK_LANE_TIMING)
+#define  GLK_MIPI_CLK_LANE_HS_PREP_SHIFT			0
+#define  GLK_MIPI_CLK_LANE_HS_PREP_MASK			(0xff << 0)
+#define  GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT			8
+#define  GLK_MIPI_CLK_LANE_HS_ZERO_MASK			(0xff00 << 0)
+#define  GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT			16
+#define  GLK_MIPI_CLK_LANE_HS_TRAIL_MASK			(0xff0000 << 0)
+#define  GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT			24
+#define  GLK_MIPI_CLK_LANE_HS_EXIT_MASK			(0xff000000 << 0)
+
 /* bits 31:0 */
 #define _MIPIA_LP_GEN_DATA		(dev_priv->mipi_mmio_base + 0xb064)
 #define _MIPIC_LP_GEN_DATA		(dev_priv->mipi_mmio_base + 0xb864)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 6b63355..b78c686 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1123,6 +1123,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder,
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
+	struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
 	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
 	enum port port;
 	unsigned int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
@@ -1278,6 +1279,20 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder,
 		 */
 		I915_WRITE(MIPI_LP_BYTECLK(port), intel_dsi->lp_byte_clk);
 
+		if (IS_GEMINILAKE(dev_priv)) {
+			I915_WRITE(MIPI_TLPX_TIME_COUNT(port),
+					intel_dsi->lp_byte_clk);
+			val = ((mipi_config->ths_prepare <<
+				GLK_MIPI_CLK_LANE_HS_PREP_SHIFT) |
+				(mipi_config->ths_prepare_hszero <<
+				GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT) |
+				(mipi_config->ths_trail <<
+				GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT) |
+				(mipi_config->ths_exit <<
+				GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT));
+			I915_WRITE(MIPI_CLK_LANE_TIMING(port), val);
+		}
+
 		/* 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
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO Enable/disable sequence

[Madhav Chauhan]

From: Deepak M <m.deepak@intel.com>

v2: Addressed Jani's Review comments(renamed bit field macros)

Signed-off-by: Deepak M <m.deepak@intel.com>
Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 134 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 134 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index b78c686..c0697e9 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -357,6 +357,134 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
 	return true;
 }
 
+static void intel_dsi_disable_mipiio(struct intel_encoder *encoder)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+	enum port port;
+	u32 tmp;
+
+	/* Put the IO into reset */
+	tmp = I915_READ(MIPI_CTRL(PORT_A));
+	tmp &= ~GLK_MIPIIO_RESET_RELEASED;
+	I915_WRITE(MIPI_CTRL(PORT_A), tmp);
+
+	/* Wait for MIPI PHY status bit to unset */
+	for_each_dsi_port(port, intel_dsi->ports) {
+		if (intel_wait_for_register(dev_priv,
+				MIPI_CTRL(port),
+				GLK_PHY_STATUS_PORT_READY, 0, 20))
+			DRM_ERROR("PHY is not turning OFF\n");
+	}
+
+	/* Clear MIPI mode */
+	for_each_dsi_port(port, intel_dsi->ports) {
+		tmp = I915_READ(MIPI_CTRL(port));
+		tmp &= ~GLK_MIPIIO_ENABLE;
+		I915_WRITE(MIPI_CTRL(port), tmp);
+	}
+}
+
+static void intel_dsi_enable_mipiio(struct intel_encoder *encoder)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+	enum port port;
+	u32 tmp, val;
+
+	/* Put the IO into reset */
+	tmp = I915_READ(MIPI_CTRL(PORT_A));
+	tmp &= ~GLK_MIPIIO_RESET_RELEASED;
+	I915_WRITE(MIPI_CTRL(PORT_A), tmp);
+
+	/* Program LP Wake */
+	for_each_dsi_port(port, intel_dsi->ports) {
+		tmp = I915_READ(MIPI_CTRL(port));
+		tmp &= ~GLK_LP_WAKE;
+		I915_WRITE(MIPI_CTRL(port), tmp);
+	}
+
+	/* Set the MIPI mode */
+	for_each_dsi_port(port, intel_dsi->ports) {
+		tmp = I915_READ(MIPI_CTRL(port));
+		I915_WRITE(MIPI_CTRL(port), tmp | GLK_MIPIIO_ENABLE);
+	}
+
+	/* Wait for Pwr ACK */
+	for_each_dsi_port(port, intel_dsi->ports) {
+		if (intel_wait_for_register(dev_priv,
+				MIPI_CTRL(port), GLK_MIPIIO_PORT_POWERED,
+				GLK_MIPIIO_PORT_POWERED, 20))
+			DRM_ERROR("Power ACK not received\n");
+	}
+
+	/* Wait for MIPI PHY status bit to set */
+	for_each_dsi_port(port, intel_dsi->ports) {
+		if (intel_wait_for_register(dev_priv,
+				MIPI_CTRL(port), GLK_MIPIIO_PORT_POWERED,
+				GLK_MIPIIO_PORT_POWERED, 20))
+			DRM_ERROR("PHY is not ON\n");
+	}
+
+	/* Get IO out of reset */
+	tmp = I915_READ(MIPI_CTRL(PORT_A));
+	I915_WRITE(MIPI_CTRL(PORT_A), tmp | GLK_MIPIIO_RESET_RELEASED);
+
+	/* Get IO out of Low power state*/
+	for_each_dsi_port(port, intel_dsi->ports) {
+		if (!(I915_READ(MIPI_DEVICE_READY(port)) & DEVICE_READY)) {
+			val = I915_READ(MIPI_DEVICE_READY(port));
+			val &= ~ULPS_STATE_MASK;
+			val |= DEVICE_READY;
+			I915_WRITE(MIPI_DEVICE_READY(port), val);
+			usleep_range(10, 15);
+		}
+
+		/* Enter ULPS */
+		val = I915_READ(MIPI_DEVICE_READY(port));
+		val &= ~ULPS_STATE_MASK;
+		val |= (ULPS_STATE_ENTER | DEVICE_READY);
+		I915_WRITE(MIPI_DEVICE_READY(port), val);
+
+		/* Wait for ULPS Not active */
+		if (intel_wait_for_register(dev_priv,
+				MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE,
+				GLK_ULPS_NOT_ACTIVE, 20))
+
+		/* Exit ULPS */
+		val = I915_READ(MIPI_DEVICE_READY(port));
+		val &= ~ULPS_STATE_MASK;
+		val |= (ULPS_STATE_EXIT | DEVICE_READY);
+		I915_WRITE(MIPI_DEVICE_READY(port), val);
+
+		/* Enter Normal Mode */
+		val = I915_READ(MIPI_DEVICE_READY(port));
+		val &= ~ULPS_STATE_MASK;
+		val |= (ULPS_STATE_NORMAL_OPERATION | DEVICE_READY);
+		I915_WRITE(MIPI_DEVICE_READY(port), val);
+
+		tmp = I915_READ(MIPI_CTRL(port));
+		tmp &= ~GLK_LP_WAKE;
+		I915_WRITE(MIPI_CTRL(port), tmp);
+	}
+
+	/* Wait for Stop state */
+	for_each_dsi_port(port, intel_dsi->ports) {
+		if (intel_wait_for_register(dev_priv,
+				MIPI_CTRL(port), GLK_DATA_LANE_STOP_STATE,
+				GLK_DATA_LANE_STOP_STATE, 20))
+			DRM_ERROR("Date lane not in STOP state\n");
+	}
+
+	/* Wait for AFE LATCH */
+	for_each_dsi_port(port, intel_dsi->ports) {
+		if (intel_wait_for_register(dev_priv,
+				BXT_MIPI_PORT_CTRL(port), AFE_LATCHOUT,
+				AFE_LATCHOUT, 20))
+			DRM_ERROR("D-PHY not entering LP-11 state\n");
+	}
+}
+
 static void bxt_dsi_device_ready(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
@@ -559,6 +687,9 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder,
 
 	intel_dsi_prepare(encoder, pipe_config);
 
+	if (IS_GEMINILAKE(dev_priv))
+		intel_dsi_enable_mipiio(encoder);
+
 	/* Panel Enable over CRC PMIC */
 	if (intel_dsi->gpio_panel)
 		gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1);
@@ -699,6 +830,9 @@ static void intel_dsi_clear_device_ready(struct intel_encoder *encoder)
 		usleep_range(2000, 2500);
 	}
 
+	if (IS_GEMINILAKE(dev_priv))
+		intel_dsi_disable_mipiio(encoder);
+
 	intel_disable_dsi_pll(encoder);
 }
 
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[GLK MIPI DSI V2 4/9] drm/i915: Set the Z inversion overlap field

[Madhav Chauhan]

From: Deepak M <m.deepak@intel.com>

Dual link Z-inversion overlap field is present
in MIPI_CTRL register unlike the older platforms,
hence setting the same in this patch.

Signed-off-by: Deepak M <m.deepak@intel.com>
Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index c0697e9..588490d 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -583,12 +583,21 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
 
 	if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
 		u32 temp;
-
-		temp = I915_READ(VLV_CHICKEN_3);
-		temp &= ~PIXEL_OVERLAP_CNT_MASK |
+		if (IS_GEN9_LP(dev_priv)) {
+			for_each_dsi_port(port, intel_dsi->ports) {
+				temp = I915_READ(MIPI_CTRL(port));
+				temp &= ~BXT_PIXEL_OVERLAP_CNT_MASK |
+					intel_dsi->pixel_overlap <<
+					BXT_PIXEL_OVERLAP_CNT_SHIFT;
+				I915_WRITE(MIPI_CTRL(port), temp);
+			}
+		} else {
+			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);
+			I915_WRITE(VLV_CHICKEN_3, temp);
+		}
 	}
 
 	for_each_dsi_port(port, intel_dsi->ports) {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[GLK MIPI DSI V2 5/9] drm/i915/glk: Add DSI PLL divider range for glk

[Madhav Chauhan]

From: Deepak M <m.deepak@intel.com>

PLL divider range for GLK is different than that of
BXT, hence adding the GLK range check in this patch.

Signed-off-by: Deepak M <m.deepak@intel.com>
Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  4 ++++
 drivers/gpu/drm/i915/intel_dsi_pll.c | 20 ++++++++++++++------
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 03858f9..65cac83 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8268,10 +8268,12 @@ enum {
 #define  BXT_DSI_PLL_PVD_RATIO_SHIFT	16
 #define  BXT_DSI_PLL_PVD_RATIO_MASK	(3 << BXT_DSI_PLL_PVD_RATIO_SHIFT)
 #define  BXT_DSI_PLL_PVD_RATIO_1	(1 << BXT_DSI_PLL_PVD_RATIO_SHIFT)
+#define  BXT_DSIC_16X_BY1		(0 << 10)
 #define  BXT_DSIC_16X_BY2		(1 << 10)
 #define  BXT_DSIC_16X_BY3		(2 << 10)
 #define  BXT_DSIC_16X_BY4		(3 << 10)
 #define  BXT_DSIC_16X_MASK		(3 << 10)
+#define  BXT_DSIA_16X_BY1		(0 << 8)
 #define  BXT_DSIA_16X_BY2		(1 << 8)
 #define  BXT_DSIA_16X_BY3		(2 << 8)
 #define  BXT_DSIA_16X_BY4		(3 << 8)
@@ -8281,6 +8283,8 @@ enum {
 
 #define BXT_DSI_PLL_RATIO_MAX		0x7D
 #define BXT_DSI_PLL_RATIO_MIN		0x22
+#define GLK_DSI_PLL_RATIO_MAX		0x6F
+#define GLK_DSI_PLL_RATIO_MIN		0x22
 #define BXT_DSI_PLL_RATIO_MASK		0xFF
 #define BXT_REF_CLOCK_KHZ		19200
 
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index cf8c1b0..6fdd08c 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -428,9 +428,10 @@ static void bxt_dsi_program_clocks(struct drm_device *dev, enum port port,
 	I915_WRITE(BXT_MIPI_CLOCK_CTL, tmp);
 }
 
-static int bxt_compute_dsi_pll(struct intel_encoder *encoder,
+static int gen9lp_compute_dsi_pll(struct intel_encoder *encoder,
 			       struct intel_crtc_state *config)
 {
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
 	u8 dsi_ratio;
 	u32 dsi_clk;
@@ -444,11 +445,18 @@ static int bxt_compute_dsi_pll(struct intel_encoder *encoder,
 	 * round 'up' the result
 	 */
 	dsi_ratio = DIV_ROUND_UP(dsi_clk * 2, BXT_REF_CLOCK_KHZ);
-	if (dsi_ratio < BXT_DSI_PLL_RATIO_MIN ||
-	    dsi_ratio > BXT_DSI_PLL_RATIO_MAX) {
+
+	if (IS_BROXTON(dev_priv) && (dsi_ratio < BXT_DSI_PLL_RATIO_MIN ||
+			dsi_ratio > BXT_DSI_PLL_RATIO_MAX)) {
 		DRM_ERROR("Cant get a suitable ratio from DSI PLL ratios\n");
 		return -ECHRNG;
-	}
+	} else if (IS_GEMINILAKE(dev_priv) &&
+			(dsi_ratio < GLK_DSI_PLL_RATIO_MIN ||
+			dsi_ratio > GLK_DSI_PLL_RATIO_MAX)) {
+		DRM_ERROR("Cant get a suitable ratio from DSI PLL ratios\n");
+		return -ECHRNG;
+	} else
+		DRM_DEBUG_KMS("DSI PLL calculation is Done!!\n");
 
 	/*
 	 * Program DSI ratio and Select MIPIC and MIPIA PLL output as 8x
@@ -460,7 +468,7 @@ static int bxt_compute_dsi_pll(struct intel_encoder *encoder,
 	/* As per recommendation from hardware team,
 	 * Prog PVD ratio =1 if dsi ratio <= 50
 	 */
-	if (dsi_ratio <= 50)
+	if (IS_BROXTON(dev_priv) && dsi_ratio <= 50)
 		config->dsi_pll.ctrl |= BXT_DSI_PLL_PVD_RATIO_1;
 
 	return 0;
@@ -520,7 +528,7 @@ int intel_compute_dsi_pll(struct intel_encoder *encoder,
 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 		return vlv_compute_dsi_pll(encoder, config);
 	else if (IS_GEN9_LP(dev_priv))
-		return bxt_compute_dsi_pll(encoder, config);
+		return gen9lp_compute_dsi_pll(encoder, config);
 
 	return -ENODEV;
 }
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[GLK MIPI DSI V2 6/9] drm/i915i/glk: Program MIPI_CLOCK_CTRL only for BXT

[Madhav Chauhan]

From: Deepak M <m.deepak@intel.com>

Register MIPI_CLOCK_CTRL is applicable only
for BXT platform. Future platform have other
registers to program the escape clock dividers.

Signed-off-by: Deepak M <m.deepak@intel.com>
Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi_pll.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index 6fdd08c..f37f61f 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -489,8 +489,10 @@ static void bxt_enable_dsi_pll(struct intel_encoder *encoder,
 	POSTING_READ(BXT_DSI_PLL_CTL);
 
 	/* Program TX, RX, Dphy clocks */
-	for_each_dsi_port(port, intel_dsi->ports)
-		bxt_dsi_program_clocks(encoder->base.dev, port, config);
+	if (IS_BROXTON(dev_priv)) {
+		for_each_dsi_port(port, intel_dsi->ports)
+			bxt_dsi_program_clocks(encoder->base.dev, port, config);
+	}
 
 	/* Enable DSI PLL */
 	val = I915_READ(BXT_DSI_PLL_ENABLE);
@@ -554,19 +556,22 @@ void intel_disable_dsi_pll(struct intel_encoder *encoder)
 		bxt_disable_dsi_pll(encoder);
 }
 
-static void bxt_dsi_reset_clocks(struct intel_encoder *encoder, enum port port)
+static void gen9lp_dsi_reset_clocks(struct intel_encoder *encoder,
+				    enum port port)
 {
 	u32 tmp;
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
 	/* Clear old configurations */
-	tmp = I915_READ(BXT_MIPI_CLOCK_CTL);
-	tmp &= ~(BXT_MIPI_TX_ESCLK_FIXDIV_MASK(port));
-	tmp &= ~(BXT_MIPI_RX_ESCLK_UPPER_FIXDIV_MASK(port));
-	tmp &= ~(BXT_MIPI_8X_BY3_DIVIDER_MASK(port));
-	tmp &= ~(BXT_MIPI_RX_ESCLK_LOWER_FIXDIV_MASK(port));
-	I915_WRITE(BXT_MIPI_CLOCK_CTL, tmp);
+	if (IS_BROXTON(dev_priv)) {
+		tmp = I915_READ(BXT_MIPI_CLOCK_CTL);
+		tmp &= ~(BXT_MIPI_TX_ESCLK_FIXDIV_MASK(port));
+		tmp &= ~(BXT_MIPI_RX_ESCLK_UPPER_FIXDIV_MASK(port));
+		tmp &= ~(BXT_MIPI_8X_BY3_DIVIDER_MASK(port));
+		tmp &= ~(BXT_MIPI_RX_ESCLK_LOWER_FIXDIV_MASK(port));
+		I915_WRITE(BXT_MIPI_CLOCK_CTL, tmp);
+	}
 	I915_WRITE(MIPI_EOT_DISABLE(port), CLOCKSTOP);
 }
 
@@ -575,7 +580,7 @@ void intel_dsi_reset_clocks(struct intel_encoder *encoder, enum port port)
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 
 	if (IS_GEN9_LP(dev_priv))
-		bxt_dsi_reset_clocks(encoder, port);
+		gen9lp_dsi_reset_clocks(encoder, port);
 	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 		vlv_dsi_reset_clocks(encoder, port);
 }
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[GLK MIPI DSI V2 7/9] drm/i915/glk: Program txesc clock divider for GLK

[Madhav Chauhan]

From: Deepak M <m.deepak@intel.com>

v2: Addressed Jani's Review comments(renamed bit field macros)

Txesc clock divider is calculated and programmed
for geminilake platform.

Signed-off-by: Deepak M <m.deepak@intel.com>
Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  5 +++
 drivers/gpu/drm/i915/intel_dsi_pll.c | 61 ++++++++++++++++++++++++++++++++++--
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 65cac83..0ffce07 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8182,6 +8182,11 @@ enum {
 #define _MIPI_PORT(port, a, c)	_PORT3(port, a, 0, c)	/* ports A and C only */
 #define _MMIO_MIPI(port, a, c)	_MMIO(_MIPI_PORT(port, a, c))
 
+#define MIPIO_TXESC_CLK_DIV1			_MMIO(0x160004)
+#define  GLK_TX_ESC_CLK_DIV1_MASK			0x3FF
+#define MIPIO_TXESC_CLK_DIV2			_MMIO(0x160008)
+#define  GLK_TX_ESC_CLK_DIV2_MASK			0x3FF
+
 /* BXT MIPI clock controls */
 #define BXT_MAX_VAR_OUTPUT_KHZ			39500
 
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index f37f61f..0291b56 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -370,6 +370,53 @@ static void vlv_dsi_reset_clocks(struct intel_encoder *encoder, enum port port)
 			ESCAPE_CLOCK_DIVIDER_SHIFT);
 }
 
+static void glk_dsi_program_esc_clock(struct drm_device *dev,
+				   const struct intel_crtc_state *config)
+{
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	u32 dsi_rate = 0;
+	u32 pll_ratio = 0;
+	u32 ddr_clk = 0;
+	u32 div1_value = 0;
+	u32 div2_value = 0;
+	u32 txesc1_div = 0;
+	u32 txesc2_div = 0;
+
+	pll_ratio = config->dsi_pll.ctrl & BXT_DSI_PLL_RATIO_MASK;
+
+	dsi_rate = (BXT_REF_CLOCK_KHZ * pll_ratio) / 2;
+
+	ddr_clk = dsi_rate / 2;
+
+	/* Variable divider value */
+	div1_value = DIV_ROUND_CLOSEST(ddr_clk, 20000);
+
+	/* Calculate TXESC1 divider */
+	if (div1_value <= 10)
+		txesc1_div = div1_value;
+	else if ((div1_value > 10) && (div1_value <= 20))
+		txesc1_div = DIV_ROUND_UP(div1_value, 2);
+	else if ((div1_value > 20) && (div1_value <= 30))
+		txesc1_div = DIV_ROUND_UP(div1_value, 4);
+	else if ((div1_value > 30) && (div1_value <= 40))
+		txesc1_div = DIV_ROUND_UP(div1_value, 6);
+	else if ((div1_value > 40) && (div1_value <= 50))
+		txesc1_div = DIV_ROUND_UP(div1_value, 8);
+	else
+		txesc1_div = 10;
+
+	/* Calculate TXESC2 divider */
+	div2_value = DIV_ROUND_UP(div1_value, txesc1_div);
+
+	if (div2_value < 10)
+		txesc2_div = div2_value;
+	else
+		txesc2_div = 10;
+
+	I915_WRITE(MIPIO_TXESC_CLK_DIV1, txesc1_div & GLK_TX_ESC_CLK_DIV1_MASK);
+	I915_WRITE(MIPIO_TXESC_CLK_DIV2, txesc2_div & GLK_TX_ESC_CLK_DIV2_MASK);
+}
+
 /* Program BXT Mipi clocks and dividers */
 static void bxt_dsi_program_clocks(struct drm_device *dev, enum port port,
 				   const struct intel_crtc_state *config)
@@ -474,7 +521,7 @@ static int gen9lp_compute_dsi_pll(struct intel_encoder *encoder,
 	return 0;
 }
 
-static void bxt_enable_dsi_pll(struct intel_encoder *encoder,
+static void gen9lp_enable_dsi_pll(struct intel_encoder *encoder,
 			       const struct intel_crtc_state *config)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
@@ -492,6 +539,8 @@ static void bxt_enable_dsi_pll(struct intel_encoder *encoder,
 	if (IS_BROXTON(dev_priv)) {
 		for_each_dsi_port(port, intel_dsi->ports)
 			bxt_dsi_program_clocks(encoder->base.dev, port, config);
+	} else {
+		glk_dsi_program_esc_clock(encoder->base.dev, config);
 	}
 
 	/* Enable DSI PLL */
@@ -543,7 +592,7 @@ void intel_enable_dsi_pll(struct intel_encoder *encoder,
 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 		vlv_enable_dsi_pll(encoder, config);
 	else if (IS_GEN9_LP(dev_priv))
-		bxt_enable_dsi_pll(encoder, config);
+		gen9lp_enable_dsi_pll(encoder, config);
 }
 
 void intel_disable_dsi_pll(struct intel_encoder *encoder)
@@ -571,6 +620,14 @@ static void gen9lp_dsi_reset_clocks(struct intel_encoder *encoder,
 		tmp &= ~(BXT_MIPI_8X_BY3_DIVIDER_MASK(port));
 		tmp &= ~(BXT_MIPI_RX_ESCLK_LOWER_FIXDIV_MASK(port));
 		I915_WRITE(BXT_MIPI_CLOCK_CTL, tmp);
+	} else {
+		tmp = I915_READ(MIPIO_TXESC_CLK_DIV1);
+		tmp &= ~GLK_TX_ESC_CLK_DIV1_MASK;
+		I915_WRITE(MIPIO_TXESC_CLK_DIV1, tmp);
+
+		tmp = I915_READ(MIPIO_TXESC_CLK_DIV2);
+		tmp &= ~GLK_TX_ESC_CLK_DIV2_MASK;
+		I915_WRITE(MIPIO_TXESC_CLK_DIV2, tmp);
 	}
 	I915_WRITE(MIPI_EOT_DISABLE(port), CLOCKSTOP);
 }
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[GLK MIPI DSI V2 8/9] drm/i915/glk: Program dphy param reg for GLK

[Madhav Chauhan]

For GEMINILAKE, dphy param reg values are programmed in terms
of HS byte clock while for legacy platforms in terms of ddrclk

Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 33 +++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 0d8ff00..647eca4 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -668,16 +668,26 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	/* count values in UI = (ns value) * (bitrate / (2 * 10^6))
 	 *
 	 * Since txddrclkhs_i is 2xUI, all the count values programmed in
-	 * DPHY param register are divided by 2
+	 * DPHY param register are divided by 2 except GEMINILAKE where it is
+	 * programmed in terms of HS byte clock so divided by 8
 	 *
 	 * prepare count
 	 */
 	ths_prepare_ns = max(mipi_config->ths_prepare,
 			     mipi_config->tclk_prepare);
-	prepare_cnt = DIV_ROUND_UP(ths_prepare_ns * ui_den, ui_num * 2);
+	if (IS_GEMINILAKE(dev_priv))
+		prepare_cnt = DIV_ROUND_UP(ths_prepare_ns * ui_den, ui_num * 8);
+	else
+		prepare_cnt = DIV_ROUND_UP(ths_prepare_ns * ui_den, ui_num * 2);
 
 	/* exit zero count */
-	exit_zero_cnt = DIV_ROUND_UP(
+	if (IS_GEMINILAKE(dev_priv))
+		exit_zero_cnt = DIV_ROUND_UP(
+				(ths_prepare_hszero - ths_prepare_ns) * ui_den,
+				ui_num * 8
+				);
+	else
+		exit_zero_cnt = DIV_ROUND_UP(
 				(ths_prepare_hszero - ths_prepare_ns) * ui_den,
 				ui_num * 2
 				);
@@ -692,13 +702,22 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
 		exit_zero_cnt += 1;
 
 	/* clk zero count */
-	clk_zero_cnt = DIV_ROUND_UP(
-			(tclk_prepare_clkzero -	ths_prepare_ns)
-			* ui_den, 2 * ui_num);
+	if (IS_GEMINILAKE(dev_priv))
+		clk_zero_cnt = DIV_ROUND_UP(
+				(tclk_prepare_clkzero -	ths_prepare_ns)
+				* ui_den, 8 * ui_num);
+	else
+		clk_zero_cnt = DIV_ROUND_UP(
+				(tclk_prepare_clkzero -	ths_prepare_ns)
+				* ui_den, 2 * ui_num);
 
 	/* trail count */
 	tclk_trail_ns = max(mipi_config->tclk_trail, mipi_config->ths_trail);
-	trail_cnt = DIV_ROUND_UP(tclk_trail_ns * ui_den, 2 * ui_num);
+
+	if (IS_GEMINILAKE(dev_priv))
+		trail_cnt = DIV_ROUND_UP(tclk_trail_ns * ui_den, 8 * ui_num);
+	else
+		trail_cnt = DIV_ROUND_UP(tclk_trail_ns * ui_den, 2 * ui_num);
 
 	if (prepare_cnt > PREPARE_CNT_MAX ||
 		exit_zero_cnt > EXIT_ZERO_CNT_MAX ||
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[GLK MIPI DSI V2 9/9] drm/915: Parsing the missed out DTD fields from the VBT

[Madhav Chauhan]

From: Vincente Tsou <vincente.tsou@intel.com>

The upper bits of the vsync width, vsync offset and hsync width
were not parsed form the VBT. Parse these fields in this patch.

Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
Signed-off-by: Vincente Tsou <vincente.tsou@intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c     | 8 +++++---
 drivers/gpu/drm/i915/intel_vbt_defs.h | 6 ++++--
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index eaade27..e1d014b 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -114,16 +114,18 @@ static u32 get_blocksize(const void *block_data)
 	panel_fixed_mode->hsync_start = panel_fixed_mode->hdisplay +
 		((dvo_timing->hsync_off_hi << 8) | dvo_timing->hsync_off_lo);
 	panel_fixed_mode->hsync_end = panel_fixed_mode->hsync_start +
-		dvo_timing->hsync_pulse_width;
+		((dvo_timing->hsync_pulse_width_hi << 8) |
+				    dvo_timing->hsync_pulse_width);
 	panel_fixed_mode->htotal = panel_fixed_mode->hdisplay +
 		((dvo_timing->hblank_hi << 8) | dvo_timing->hblank_lo);
 
 	panel_fixed_mode->vdisplay = (dvo_timing->vactive_hi << 8) |
 		dvo_timing->vactive_lo;
 	panel_fixed_mode->vsync_start = panel_fixed_mode->vdisplay +
-		dvo_timing->vsync_off;
+		((dvo_timing->vsync_off_hi << 4) | dvo_timing->vsync_off);
 	panel_fixed_mode->vsync_end = panel_fixed_mode->vsync_start +
-		dvo_timing->vsync_pulse_width;
+		((dvo_timing->vsync_pulse_width_hi << 4) |
+				    dvo_timing->vsync_pulse_width);
 	panel_fixed_mode->vtotal = panel_fixed_mode->vdisplay +
 		((dvo_timing->vblank_hi << 8) | dvo_timing->vblank_lo);
 	panel_fixed_mode->clock = dvo_timing->clock * 10;
diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h b/drivers/gpu/drm/i915/intel_vbt_defs.h
index 8886cab1..bf9d2d3 100644
--- a/drivers/gpu/drm/i915/intel_vbt_defs.h
+++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
@@ -402,7 +402,9 @@ struct lvds_dvo_timing {
 	u8 hsync_pulse_width;
 	u8 vsync_pulse_width:4;
 	u8 vsync_off:4;
-	u8 rsvd0:6;
+	u8 vsync_pulse_width_hi:2;
+	u8 vsync_off_hi:2;
+	u8 hsync_pulse_width_hi:2;
 	u8 hsync_off_hi:2;
 	u8 himage_lo;
 	u8 vimage_lo;
@@ -414,7 +416,7 @@ struct lvds_dvo_timing {
 	u8 digital:2;
 	u8 vsync_positive:1;
 	u8 hsync_positive:1;
-	u8 rsvd2:1;
+	u8 interlaced:1;
 } __packed;
 
 struct lvds_pnp_id {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for GLK MIPI DSI VIDEO MODE PATCHES (rev2)

[Patchwork]

== Series Details ==

Series: GLK MIPI DSI VIDEO MODE PATCHES (rev2)
URL   : https://patchwork.freedesktop.org/series/16542/
State : success

== Summary ==

Series 16542v2 GLK MIPI DSI VIDEO MODE PATCHES
https://patchwork.freedesktop.org/api/1.0/series/16542/revisions/2/mbox/


fi-bdw-5557u     total:247  pass:233  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:247  pass:208  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-t5700     total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-j1900     total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-ilk-650       total:247  pass:195  dwarn:0   dfail:0   fail:0   skip:52 
fi-ivb-3520m     total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u     total:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:247  pass:227  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:247  pass:224  dwarn:3   dfail:0   fail:0   skip:20 
fi-skl-6770hq    total:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:247  pass:215  dwarn:0   dfail:0   fail:0   skip:32 

c8109e51ef622698a43d4e8382a5f7fbbae6375b drm-tip: 2016y-12m-15d-07h-22m-49s UTC integration manifest
6295a9c drm/915: Parsing the missed out DTD fields from the VBT
1fb6cbb drm/i915/glk: Program dphy param reg for GLK
41cad10 drm/i915/glk: Program txesc clock divider for GLK
7672118 drm/i915i/glk: Program MIPI_CLOCK_CTRL only for BXT
afff049 drm/i915/glk: Add DSI PLL divider range for glk
140ad28 drm/i915: Set the Z inversion overlap field
120cc17 drm/i915/glk: Add MIPIIO Enable/disable sequence
c2c81ac drm/i915/glk: Program new MIPI DSI PHY registers for GLK
5874254 drm/i915/glk: Add new bit fields in MIPI CTRL register

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3292/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [GLK MIPI DSI V2 9/9] drm/915: Parsing the missed out DTD fields from the VBT

[Jani Nikula]

On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com> wrote:
> From: Vincente Tsou <vincente.tsou@intel.com>
>
> The upper bits of the vsync width, vsync offset and hsync width
> were not parsed form the VBT. Parse these fields in this patch.
>
> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> Signed-off-by: Vincente Tsou <vincente.tsou@intel.com>

The author Signed-off-by should be first, others are added below.

> ---
>  drivers/gpu/drm/i915/intel_bios.c     | 8 +++++---
>  drivers/gpu/drm/i915/intel_vbt_defs.h | 6 ++++--
>  2 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index eaade27..e1d014b 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -114,16 +114,18 @@ static u32 get_blocksize(const void *block_data)
>  	panel_fixed_mode->hsync_start = panel_fixed_mode->hdisplay +
>  		((dvo_timing->hsync_off_hi << 8) | dvo_timing->hsync_off_lo);
>  	panel_fixed_mode->hsync_end = panel_fixed_mode->hsync_start +
> -		dvo_timing->hsync_pulse_width;
> +		((dvo_timing->hsync_pulse_width_hi << 8) |
> +				    dvo_timing->hsync_pulse_width);
>  	panel_fixed_mode->htotal = panel_fixed_mode->hdisplay +
>  		((dvo_timing->hblank_hi << 8) | dvo_timing->hblank_lo);
>  
>  	panel_fixed_mode->vdisplay = (dvo_timing->vactive_hi << 8) |
>  		dvo_timing->vactive_lo;
>  	panel_fixed_mode->vsync_start = panel_fixed_mode->vdisplay +
> -		dvo_timing->vsync_off;
> +		((dvo_timing->vsync_off_hi << 4) | dvo_timing->vsync_off);
>  	panel_fixed_mode->vsync_end = panel_fixed_mode->vsync_start +
> -		dvo_timing->vsync_pulse_width;
> +		((dvo_timing->vsync_pulse_width_hi << 4) |
> +				    dvo_timing->vsync_pulse_width);

The indentation for the above changes seem to be off.

>  	panel_fixed_mode->vtotal = panel_fixed_mode->vdisplay +
>  		((dvo_timing->vblank_hi << 8) | dvo_timing->vblank_lo);
>  	panel_fixed_mode->clock = dvo_timing->clock * 10;
> diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h b/drivers/gpu/drm/i915/intel_vbt_defs.h
> index 8886cab1..bf9d2d3 100644
> --- a/drivers/gpu/drm/i915/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
> @@ -402,7 +402,9 @@ struct lvds_dvo_timing {
>  	u8 hsync_pulse_width;
>  	u8 vsync_pulse_width:4;
>  	u8 vsync_off:4;
> -	u8 rsvd0:6;
> +	u8 vsync_pulse_width_hi:2;
> +	u8 vsync_off_hi:2;
> +	u8 hsync_pulse_width_hi:2;

Please rename the lo counterparts of these fields to have _lo suffix,
for consistency with other hi/lo split fields. With that, the compiler
will help you in making sure you found all the places you need to
fix. ;)

>  	u8 hsync_off_hi:2;
>  	u8 himage_lo;
>  	u8 vimage_lo;
> @@ -414,7 +416,7 @@ struct lvds_dvo_timing {
>  	u8 digital:2;
>  	u8 vsync_positive:1;
>  	u8 hsync_positive:1;
> -	u8 rsvd2:1;
> +	u8 interlaced:1;

This should be non_interlaced, as that's how the bit is defined.

Otherwise, seems like a good find.

BR,
Jani.

>  } __packed;
>  
>  struct lvds_pnp_id {

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [GLK MIPI DSI V2 2/9] drm/i915/glk: Program new MIPI DSI PHY registers for GLK

[Ville Syrjälä]

On Thu, Dec 15, 2016 at 02:31:33PM +0530, Madhav Chauhan wrote:
> From: Deepak M <m.deepak@intel.com>
> 
> Program the clk lane and tlpx time count registers
> to configure DSI PHY.
> 
> v2: Addressed Jani's Review comments(renamed bit field macros)
> 
> Signed-off-by: Deepak M <m.deepak@intel.com>
> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  | 18 ++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dsi.c | 15 +++++++++++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8e47b59..03858f9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8550,6 +8550,24 @@ enum {
>  #define  LP_BYTECLK_SHIFT				0
>  #define  LP_BYTECLK_MASK				(0xffff << 0)
>  
> +#define _MIPIA_TLPX_TIME_COUNT		(dev_priv->mipi_mmio_base + 0xb0a4)
> +#define _MIPIC_TLPX_TIME_COUNT		(dev_priv->mipi_mmio_base + 0xb8a4)
> +#define MIPI_TLPX_TIME_COUNT(port)	 _MMIO_MIPI(port, _MIPIA_TLPX_TIME_COUNT, _MIPIC_TLPX_TIME_COUNT)
> +#define  GLK_DPHY_TLPX_TIME_CNT_SHIFT			0
> +#define  GLK_DPHY_TLPX_TIME_CNT_MASK			(0xff << 0)
> +
> +#define _MIPIA_CLK_LANE_TIMING		(dev_priv->mipi_mmio_base + 0xb098)
> +#define _MIPIC_CLK_LANE_TIMING		(dev_priv->mipi_mmio_base + 0xb898)
> +#define MIPI_CLK_LANE_TIMING(port)	 _MMIO_MIPI(port, _MIPIA_CLK_LANE_TIMING, _MIPIC_CLK_LANE_TIMING)
> +#define  GLK_MIPI_CLK_LANE_HS_PREP_SHIFT			0
> +#define  GLK_MIPI_CLK_LANE_HS_PREP_MASK			(0xff << 0)
> +#define  GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT			8
> +#define  GLK_MIPI_CLK_LANE_HS_ZERO_MASK			(0xff00 << 0)
> +#define  GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT			16
> +#define  GLK_MIPI_CLK_LANE_HS_TRAIL_MASK			(0xff0000 << 0)
> +#define  GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT			24
> +#define  GLK_MIPI_CLK_LANE_HS_EXIT_MASK			(0xff000000 << 0)
> +
>  /* bits 31:0 */
>  #define _MIPIA_LP_GEN_DATA		(dev_priv->mipi_mmio_base + 0xb064)
>  #define _MIPIC_LP_GEN_DATA		(dev_priv->mipi_mmio_base + 0xb864)
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 6b63355..b78c686 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -1123,6 +1123,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder,
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> +	struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
>  	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>  	enum port port;
>  	unsigned int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
> @@ -1278,6 +1279,20 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder,
>  		 */
>  		I915_WRITE(MIPI_LP_BYTECLK(port), intel_dsi->lp_byte_clk);
>  
> +		if (IS_GEMINILAKE(dev_priv)) {
> +			I915_WRITE(MIPI_TLPX_TIME_COUNT(port),
> +					intel_dsi->lp_byte_clk);
> +			val = ((mipi_config->ths_prepare <<
> +				GLK_MIPI_CLK_LANE_HS_PREP_SHIFT) |
> +				(mipi_config->ths_prepare_hszero <<
> +				GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT) |
> +				(mipi_config->ths_trail <<
> +				GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT) |
> +				(mipi_config->ths_exit <<
> +				GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT));

I don't think anything else here uses the mipi_config block directly.
Rather everything gets duplicated in intel_dsi. This here looks
like the same (or similar) thing to dphy_reg. Probably should
reuse that, or split it up into its component pieces if we want
to keep duplicated information between the mipi_config and intel_dsi.

> +			I915_WRITE(MIPI_CLK_LANE_TIMING(port), val);
> +		}
> +
>  		/* 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
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [GLK MIPI DSI V2 2/9] drm/i915/glk: Program new MIPI DSI PHY registers for GLK

[Jani Nikula]

On Thu, 22 Dec 2016, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Dec 15, 2016 at 02:31:33PM +0530, Madhav Chauhan wrote:
>> From: Deepak M <m.deepak@intel.com>
>> 
>> Program the clk lane and tlpx time count registers
>> to configure DSI PHY.
>> 
>> v2: Addressed Jani's Review comments(renamed bit field macros)
>> 
>> Signed-off-by: Deepak M <m.deepak@intel.com>
>> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h  | 18 ++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_dsi.c | 15 +++++++++++++++
>>  2 files changed, 33 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 8e47b59..03858f9 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -8550,6 +8550,24 @@ enum {
>>  #define  LP_BYTECLK_SHIFT				0
>>  #define  LP_BYTECLK_MASK				(0xffff << 0)
>>  
>> +#define _MIPIA_TLPX_TIME_COUNT		(dev_priv->mipi_mmio_base + 0xb0a4)
>> +#define _MIPIC_TLPX_TIME_COUNT		(dev_priv->mipi_mmio_base + 0xb8a4)
>> +#define MIPI_TLPX_TIME_COUNT(port)	 _MMIO_MIPI(port, _MIPIA_TLPX_TIME_COUNT, _MIPIC_TLPX_TIME_COUNT)
>> +#define  GLK_DPHY_TLPX_TIME_CNT_SHIFT			0
>> +#define  GLK_DPHY_TLPX_TIME_CNT_MASK			(0xff << 0)
>> +
>> +#define _MIPIA_CLK_LANE_TIMING		(dev_priv->mipi_mmio_base + 0xb098)
>> +#define _MIPIC_CLK_LANE_TIMING		(dev_priv->mipi_mmio_base + 0xb898)
>> +#define MIPI_CLK_LANE_TIMING(port)	 _MMIO_MIPI(port, _MIPIA_CLK_LANE_TIMING, _MIPIC_CLK_LANE_TIMING)
>> +#define  GLK_MIPI_CLK_LANE_HS_PREP_SHIFT			0
>> +#define  GLK_MIPI_CLK_LANE_HS_PREP_MASK			(0xff << 0)
>> +#define  GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT			8
>> +#define  GLK_MIPI_CLK_LANE_HS_ZERO_MASK			(0xff00 << 0)
>> +#define  GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT			16
>> +#define  GLK_MIPI_CLK_LANE_HS_TRAIL_MASK			(0xff0000 << 0)
>> +#define  GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT			24
>> +#define  GLK_MIPI_CLK_LANE_HS_EXIT_MASK			(0xff000000 << 0)
>> +
>>  /* bits 31:0 */
>>  #define _MIPIA_LP_GEN_DATA		(dev_priv->mipi_mmio_base + 0xb064)
>>  #define _MIPIC_LP_GEN_DATA		(dev_priv->mipi_mmio_base + 0xb864)
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> index 6b63355..b78c686 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -1123,6 +1123,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder,
>>  	struct drm_i915_private *dev_priv = to_i915(dev);
>>  	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
>>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>> +	struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
>>  	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>>  	enum port port;
>>  	unsigned int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
>> @@ -1278,6 +1279,20 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder,
>>  		 */
>>  		I915_WRITE(MIPI_LP_BYTECLK(port), intel_dsi->lp_byte_clk);
>>  
>> +		if (IS_GEMINILAKE(dev_priv)) {
>> +			I915_WRITE(MIPI_TLPX_TIME_COUNT(port),
>> +					intel_dsi->lp_byte_clk);
>> +			val = ((mipi_config->ths_prepare <<
>> +				GLK_MIPI_CLK_LANE_HS_PREP_SHIFT) |
>> +				(mipi_config->ths_prepare_hszero <<
>> +				GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT) |
>> +				(mipi_config->ths_trail <<
>> +				GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT) |
>> +				(mipi_config->ths_exit <<
>> +				GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT));
>
> I don't think anything else here uses the mipi_config block directly.
> Rather everything gets duplicated in intel_dsi. This here looks
> like the same (or similar) thing to dphy_reg. Probably should
> reuse that, or split it up into its component pieces if we want
> to keep duplicated information between the mipi_config and intel_dsi.

My thoughts exactly. I've just been thinking about Hans' patches, and
whether we should give in and split intel_dsi.c and
intel_dsi_panel_vbt.c differently.

BR,
Jani.

>
>> +			I915_WRITE(MIPI_CLK_LANE_TIMING(port), val);
>> +		}
>> +
>>  		/* 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
>> -- 
>> 1.9.1
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [GLK MIPI DSI V2 9/9] drm/915: Parsing the missed out DTD fields from the VBT

[Chauhan, Madhav]

> -----Original Message-----
> From: Nikula, Jani
> Sent: Thursday, December 22, 2016 5:09 PM
> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>;
> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra
> <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit
> <shobhit.kumar@intel.com>; Tsou, Vincente <vincente.tsou@intel.com>;
> Chauhan, Madhav <madhav.chauhan@intel.com>
> Subject: Re: [GLK MIPI DSI V2 9/9] drm/915: Parsing the missed out DTD
> fields from the VBT
> 
> On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com>
> wrote:
> > From: Vincente Tsou <vincente.tsou@intel.com>
> >
> > The upper bits of the vsync width, vsync offset and hsync width were
> > not parsed form the VBT. Parse these fields in this patch.
> >
> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> > Signed-off-by: Vincente Tsou <vincente.tsou@intel.com>
> 
> The author Signed-off-by should be first, others are added below.
> 
> > ---
> >  drivers/gpu/drm/i915/intel_bios.c     | 8 +++++---
> >  drivers/gpu/drm/i915/intel_vbt_defs.h | 6 ++++--
> >  2 files changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_bios.c
> > b/drivers/gpu/drm/i915/intel_bios.c
> > index eaade27..e1d014b 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > @@ -114,16 +114,18 @@ static u32 get_blocksize(const void *block_data)
> >  	panel_fixed_mode->hsync_start = panel_fixed_mode->hdisplay +
> >  		((dvo_timing->hsync_off_hi << 8) | dvo_timing-
> >hsync_off_lo);
> >  	panel_fixed_mode->hsync_end = panel_fixed_mode->hsync_start +
> > -		dvo_timing->hsync_pulse_width;
> > +		((dvo_timing->hsync_pulse_width_hi << 8) |
> > +				    dvo_timing->hsync_pulse_width);
> >  	panel_fixed_mode->htotal = panel_fixed_mode->hdisplay +
> >  		((dvo_timing->hblank_hi << 8) | dvo_timing->hblank_lo);
> >
> >  	panel_fixed_mode->vdisplay = (dvo_timing->vactive_hi << 8) |
> >  		dvo_timing->vactive_lo;
> >  	panel_fixed_mode->vsync_start = panel_fixed_mode->vdisplay +
> > -		dvo_timing->vsync_off;
> > +		((dvo_timing->vsync_off_hi << 4) | dvo_timing->vsync_off);
> >  	panel_fixed_mode->vsync_end = panel_fixed_mode->vsync_start +
> > -		dvo_timing->vsync_pulse_width;
> > +		((dvo_timing->vsync_pulse_width_hi << 4) |
> > +				    dvo_timing->vsync_pulse_width);
> 
> The indentation for the above changes seem to be off.
> 
> >  	panel_fixed_mode->vtotal = panel_fixed_mode->vdisplay +
> >  		((dvo_timing->vblank_hi << 8) | dvo_timing->vblank_lo);
> >  	panel_fixed_mode->clock = dvo_timing->clock * 10; diff --git
> > a/drivers/gpu/drm/i915/intel_vbt_defs.h
> > b/drivers/gpu/drm/i915/intel_vbt_defs.h
> > index 8886cab1..bf9d2d3 100644
> > --- a/drivers/gpu/drm/i915/intel_vbt_defs.h
> > +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
> > @@ -402,7 +402,9 @@ struct lvds_dvo_timing {
> >  	u8 hsync_pulse_width;
> >  	u8 vsync_pulse_width:4;
> >  	u8 vsync_off:4;
> > -	u8 rsvd0:6;
> > +	u8 vsync_pulse_width_hi:2;
> > +	u8 vsync_off_hi:2;
> > +	u8 hsync_pulse_width_hi:2;
> 
> Please rename the lo counterparts of these fields to have _lo suffix, for
> consistency with other hi/lo split fields. With that, the compiler will help you
> in making sure you found all the places you need to fix. ;)
> 
> >  	u8 hsync_off_hi:2;
> >  	u8 himage_lo;
> >  	u8 vimage_lo;
> > @@ -414,7 +416,7 @@ struct lvds_dvo_timing {
> >  	u8 digital:2;
> >  	u8 vsync_positive:1;
> >  	u8 hsync_positive:1;
> > -	u8 rsvd2:1;
> > +	u8 interlaced:1;
> 
> This should be non_interlaced, as that's how the bit is defined.
> 
> Otherwise, seems like a good find.

Thanks Jani for review. Will address the review comment and resend it.

> 
> BR,
> Jani.
> 
> >  } __packed;
> >
> >  struct lvds_pnp_id {
> 
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [GLK MIPI DSI V2 9/9] drm/915: Parsing the missed out DTD fields from the VBT

[Jani Nikula]

On Thu, 22 Dec 2016, "Chauhan, Madhav" <madhav.chauhan@intel.com> wrote:
>> -----Original Message-----
>> From: Nikula, Jani
>> Sent: Thursday, December 22, 2016 5:09 PM
>> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>;
>> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra
>> <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
>> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit
>> <shobhit.kumar@intel.com>; Tsou, Vincente <vincente.tsou@intel.com>;
>> Chauhan, Madhav <madhav.chauhan@intel.com>
>> Subject: Re: [GLK MIPI DSI V2 9/9] drm/915: Parsing the missed out DTD
>> fields from the VBT
>> 
>> On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com>
>> wrote:
>> > From: Vincente Tsou <vincente.tsou@intel.com>
>> >
>> > The upper bits of the vsync width, vsync offset and hsync width were
>> > not parsed form the VBT. Parse these fields in this patch.
>> >
>> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
>> > Signed-off-by: Vincente Tsou <vincente.tsou@intel.com>
>> 
>> The author Signed-off-by should be first, others are added below.
>> 
>> > ---
>> >  drivers/gpu/drm/i915/intel_bios.c     | 8 +++++---
>> >  drivers/gpu/drm/i915/intel_vbt_defs.h | 6 ++++--
>> >  2 files changed, 9 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_bios.c
>> > b/drivers/gpu/drm/i915/intel_bios.c
>> > index eaade27..e1d014b 100644
>> > --- a/drivers/gpu/drm/i915/intel_bios.c
>> > +++ b/drivers/gpu/drm/i915/intel_bios.c
>> > @@ -114,16 +114,18 @@ static u32 get_blocksize(const void *block_data)
>> >  	panel_fixed_mode->hsync_start = panel_fixed_mode->hdisplay +
>> >  		((dvo_timing->hsync_off_hi << 8) | dvo_timing-
>> >hsync_off_lo);
>> >  	panel_fixed_mode->hsync_end = panel_fixed_mode->hsync_start +
>> > -		dvo_timing->hsync_pulse_width;
>> > +		((dvo_timing->hsync_pulse_width_hi << 8) |
>> > +				    dvo_timing->hsync_pulse_width);
>> >  	panel_fixed_mode->htotal = panel_fixed_mode->hdisplay +
>> >  		((dvo_timing->hblank_hi << 8) | dvo_timing->hblank_lo);
>> >
>> >  	panel_fixed_mode->vdisplay = (dvo_timing->vactive_hi << 8) |
>> >  		dvo_timing->vactive_lo;
>> >  	panel_fixed_mode->vsync_start = panel_fixed_mode->vdisplay +
>> > -		dvo_timing->vsync_off;
>> > +		((dvo_timing->vsync_off_hi << 4) | dvo_timing->vsync_off);
>> >  	panel_fixed_mode->vsync_end = panel_fixed_mode->vsync_start +
>> > -		dvo_timing->vsync_pulse_width;
>> > +		((dvo_timing->vsync_pulse_width_hi << 4) |
>> > +				    dvo_timing->vsync_pulse_width);
>> 
>> The indentation for the above changes seem to be off.
>> 
>> >  	panel_fixed_mode->vtotal = panel_fixed_mode->vdisplay +
>> >  		((dvo_timing->vblank_hi << 8) | dvo_timing->vblank_lo);
>> >  	panel_fixed_mode->clock = dvo_timing->clock * 10; diff --git
>> > a/drivers/gpu/drm/i915/intel_vbt_defs.h
>> > b/drivers/gpu/drm/i915/intel_vbt_defs.h
>> > index 8886cab1..bf9d2d3 100644
>> > --- a/drivers/gpu/drm/i915/intel_vbt_defs.h
>> > +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
>> > @@ -402,7 +402,9 @@ struct lvds_dvo_timing {
>> >  	u8 hsync_pulse_width;
>> >  	u8 vsync_pulse_width:4;
>> >  	u8 vsync_off:4;
>> > -	u8 rsvd0:6;
>> > +	u8 vsync_pulse_width_hi:2;
>> > +	u8 vsync_off_hi:2;
>> > +	u8 hsync_pulse_width_hi:2;
>> 
>> Please rename the lo counterparts of these fields to have _lo suffix, for
>> consistency with other hi/lo split fields. With that, the compiler will help you
>> in making sure you found all the places you need to fix. ;)
>> 
>> >  	u8 hsync_off_hi:2;
>> >  	u8 himage_lo;
>> >  	u8 vimage_lo;
>> > @@ -414,7 +416,7 @@ struct lvds_dvo_timing {
>> >  	u8 digital:2;
>> >  	u8 vsync_positive:1;
>> >  	u8 hsync_positive:1;
>> > -	u8 rsvd2:1;
>> > +	u8 interlaced:1;
>> 
>> This should be non_interlaced, as that's how the bit is defined.
>> 
>> Otherwise, seems like a good find.
>
> Thanks Jani for review. Will address the review comment and resend it.

Please send it as a separate patch outside of this series, it's not
really related.

BR,
Jani.


>
>> 
>> BR,
>> Jani.
>> 
>> >  } __packed;
>> >
>> >  struct lvds_pnp_id {
>> 
>> --
>> Jani Nikula, Intel Open Source Technology Center

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [GLK MIPI DSI V2 9/9] drm/915: Parsing the missed out DTD fields from the VBT

[Chauhan, Madhav]

> -----Original Message-----
> From: Nikula, Jani
> Sent: Thursday, December 22, 2016 7:13 PM
> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>;
> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra
> <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit
> <shobhit.kumar@intel.com>; Tsou, Vincente <vincente.tsou@intel.com>
> Subject: RE: [GLK MIPI DSI V2 9/9] drm/915: Parsing the missed out DTD
> fields from the VBT
> 
> On Thu, 22 Dec 2016, "Chauhan, Madhav" <madhav.chauhan@intel.com>
> wrote:
> >> -----Original Message-----
> >> From: Nikula, Jani
> >> Sent: Thursday, December 22, 2016 5:09 PM
> >> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
> >> gfx@lists.freedesktop.org
> >> Cc: Conselvan De Oliveira, Ander
> >> <ander.conselvan.de.oliveira@intel.com>;
> >> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra
> >> <chandra.konduru@intel.com>; Shankar, Uma
> <uma.shankar@intel.com>;
> >> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit
> >> <shobhit.kumar@intel.com>; Tsou, Vincente <vincente.tsou@intel.com>;
> >> Chauhan, Madhav <madhav.chauhan@intel.com>
> >> Subject: Re: [GLK MIPI DSI V2 9/9] drm/915: Parsing the missed out
> >> DTD fields from the VBT
> >>
> >> On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com>
> >> wrote:
> >> > From: Vincente Tsou <vincente.tsou@intel.com>
> >> >
> >> > The upper bits of the vsync width, vsync offset and hsync width
> >> > were not parsed form the VBT. Parse these fields in this patch.
> >> >
> >> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> >> > Signed-off-by: Vincente Tsou <vincente.tsou@intel.com>
> >>
> >> The author Signed-off-by should be first, others are added below.
> >>
> >> > ---
> >> >  drivers/gpu/drm/i915/intel_bios.c     | 8 +++++---
> >> >  drivers/gpu/drm/i915/intel_vbt_defs.h | 6 ++++--
> >> >  2 files changed, 9 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/intel_bios.c
> >> > b/drivers/gpu/drm/i915/intel_bios.c
> >> > index eaade27..e1d014b 100644
> >> > --- a/drivers/gpu/drm/i915/intel_bios.c
> >> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> >> > @@ -114,16 +114,18 @@ static u32 get_blocksize(const void
> *block_data)
> >> >  	panel_fixed_mode->hsync_start = panel_fixed_mode->hdisplay +
> >> >  		((dvo_timing->hsync_off_hi << 8) | dvo_timing-
> hsync_off_lo);
> >> >  	panel_fixed_mode->hsync_end = panel_fixed_mode->hsync_start +
> >> > -		dvo_timing->hsync_pulse_width;
> >> > +		((dvo_timing->hsync_pulse_width_hi << 8) |
> >> > +				    dvo_timing->hsync_pulse_width);
> >> >  	panel_fixed_mode->htotal = panel_fixed_mode->hdisplay +
> >> >  		((dvo_timing->hblank_hi << 8) | dvo_timing->hblank_lo);
> >> >
> >> >  	panel_fixed_mode->vdisplay = (dvo_timing->vactive_hi << 8) |
> >> >  		dvo_timing->vactive_lo;
> >> >  	panel_fixed_mode->vsync_start = panel_fixed_mode->vdisplay +
> >> > -		dvo_timing->vsync_off;
> >> > +		((dvo_timing->vsync_off_hi << 4) | dvo_timing->vsync_off);
> >> >  	panel_fixed_mode->vsync_end = panel_fixed_mode->vsync_start +
> >> > -		dvo_timing->vsync_pulse_width;
> >> > +		((dvo_timing->vsync_pulse_width_hi << 4) |
> >> > +				    dvo_timing->vsync_pulse_width);
> >>
> >> The indentation for the above changes seem to be off.
> >>
> >> >  	panel_fixed_mode->vtotal = panel_fixed_mode->vdisplay +
> >> >  		((dvo_timing->vblank_hi << 8) | dvo_timing->vblank_lo);
> >> >  	panel_fixed_mode->clock = dvo_timing->clock * 10; diff --git
> >> > a/drivers/gpu/drm/i915/intel_vbt_defs.h
> >> > b/drivers/gpu/drm/i915/intel_vbt_defs.h
> >> > index 8886cab1..bf9d2d3 100644
> >> > --- a/drivers/gpu/drm/i915/intel_vbt_defs.h
> >> > +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
> >> > @@ -402,7 +402,9 @@ struct lvds_dvo_timing {
> >> >  	u8 hsync_pulse_width;
> >> >  	u8 vsync_pulse_width:4;
> >> >  	u8 vsync_off:4;
> >> > -	u8 rsvd0:6;
> >> > +	u8 vsync_pulse_width_hi:2;
> >> > +	u8 vsync_off_hi:2;
> >> > +	u8 hsync_pulse_width_hi:2;
> >>
> >> Please rename the lo counterparts of these fields to have _lo suffix,
> >> for consistency with other hi/lo split fields. With that, the
> >> compiler will help you in making sure you found all the places you
> >> need to fix. ;)
> >>
> >> >  	u8 hsync_off_hi:2;
> >> >  	u8 himage_lo;
> >> >  	u8 vimage_lo;
> >> > @@ -414,7 +416,7 @@ struct lvds_dvo_timing {
> >> >  	u8 digital:2;
> >> >  	u8 vsync_positive:1;
> >> >  	u8 hsync_positive:1;
> >> > -	u8 rsvd2:1;
> >> > +	u8 interlaced:1;
> >>
> >> This should be non_interlaced, as that's how the bit is defined.
> >>
> >> Otherwise, seems like a good find.
> >
> > Thanks Jani for review. Will address the review comment and resend it.
> 
> Please send it as a separate patch outside of this series, it's not really related.

Agree, had same thought when V1 series was sent. Included it so that review and merge can be combined :)

> 
> BR,
> Jani.
> 
> 
> >
> >>
> >> BR,
> >> Jani.
> >>
> >> >  } __packed;
> >> >
> >> >  struct lvds_pnp_id {
> >>
> >> --
> >> Jani Nikula, Intel Open Source Technology Center
> 
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [GLK MIPI DSI V2 1/9] drm/i915/glk: Add new bit fields in MIPI CTRL register

[Jani Nikula]

On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com> wrote:
> From: Deepak M <m.deepak@intel.com>
>
> v2: Addressed Jani's Review comments (renamed bit field macros)
>
> Signed-off-by: Deepak M <m.deepak@intel.com>
> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>

Pushed to drm-intel-next-queued, thanks for the patch.

BR,
Jani.


> ---
>  drivers/gpu/drm/i915/i915_reg.h | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 90685d2..8e47b59 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8672,6 +8672,21 @@ enum {
>  #define  BXT_PIPE_SELECT_SHIFT				7
>  #define  BXT_PIPE_SELECT_MASK				(7 << 7)
>  #define  BXT_PIPE_SELECT(pipe)				((pipe) << 7)
> +#define  GLK_PHY_STATUS_PORT_READY			(1 << 31) /* RO */
> +#define  GLK_ULPS_NOT_ACTIVE				(1 << 30) /* RO */
> +#define  GLK_MIPIIO_RESET_RELEASED			(1 << 28)
> +#define  GLK_CLOCK_LANE_STOP_STATE			(1 << 27) /* RO */
> +#define  GLK_DATA_LANE_STOP_STATE			(1 << 26) /* RO */
> +#define  GLK_LP_WAKE					(1 << 22)
> +#define  GLK_LP11_LOW_PWR_MODE				(1 << 21)
> +#define  GLK_LP00_LOW_PWR_MODE				(1 << 20)
> +#define  GLK_FIREWALL_ENABLE				(1 << 16)
> +#define  BXT_PIXEL_OVERLAP_CNT_MASK			(0xf << 10)
> +#define  BXT_PIXEL_OVERLAP_CNT_SHIFT			10
> +#define  BXT_DSC_ENABLE					(1 << 3)
> +#define  BXT_RGB_FLIP					(1 << 2)
> +#define  GLK_MIPIIO_PORT_POWERED			(1 << 1) /* RO */
> +#define  GLK_MIPIIO_ENABLE				(1 << 0)
>  
>  #define _MIPIA_DATA_ADDRESS		(dev_priv->mipi_mmio_base + 0xb108)
>  #define _MIPIC_DATA_ADDRESS		(dev_priv->mipi_mmio_base + 0xb908)

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [GLK MIPI DSI V2 2/9] drm/i915/glk: Program new MIPI DSI PHY registers for GLK

[Jani Nikula]

On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com> wrote:
> From: Deepak M <m.deepak@intel.com>
>
> Program the clk lane and tlpx time count registers
> to configure DSI PHY.
>
> v2: Addressed Jani's Review comments(renamed bit field macros)
>
> Signed-off-by: Deepak M <m.deepak@intel.com>
> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  | 18 ++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dsi.c | 15 +++++++++++++++
>  2 files changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8e47b59..03858f9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8550,6 +8550,24 @@ enum {
>  #define  LP_BYTECLK_SHIFT				0
>  #define  LP_BYTECLK_MASK				(0xffff << 0)
>  
> +#define _MIPIA_TLPX_TIME_COUNT		(dev_priv->mipi_mmio_base + 0xb0a4)
> +#define _MIPIC_TLPX_TIME_COUNT		(dev_priv->mipi_mmio_base + 0xb8a4)
> +#define MIPI_TLPX_TIME_COUNT(port)	 _MMIO_MIPI(port, _MIPIA_TLPX_TIME_COUNT, _MIPIC_TLPX_TIME_COUNT)
> +#define  GLK_DPHY_TLPX_TIME_CNT_SHIFT			0
> +#define  GLK_DPHY_TLPX_TIME_CNT_MASK			(0xff << 0)
> +
> +#define _MIPIA_CLK_LANE_TIMING		(dev_priv->mipi_mmio_base + 0xb098)
> +#define _MIPIC_CLK_LANE_TIMING		(dev_priv->mipi_mmio_base + 0xb898)
> +#define MIPI_CLK_LANE_TIMING(port)	 _MMIO_MIPI(port, _MIPIA_CLK_LANE_TIMING, _MIPIC_CLK_LANE_TIMING)
> +#define  GLK_MIPI_CLK_LANE_HS_PREP_SHIFT			0
> +#define  GLK_MIPI_CLK_LANE_HS_PREP_MASK			(0xff << 0)
> +#define  GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT			8
> +#define  GLK_MIPI_CLK_LANE_HS_ZERO_MASK			(0xff00 << 0)

0xff << 8

> +#define  GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT			16
> +#define  GLK_MIPI_CLK_LANE_HS_TRAIL_MASK			(0xff0000 << 0)

0xff << 16

> +#define  GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT			24
> +#define  GLK_MIPI_CLK_LANE_HS_EXIT_MASK			(0xff000000 << 0)

0xff << 24

> +
>  /* bits 31:0 */
>  #define _MIPIA_LP_GEN_DATA		(dev_priv->mipi_mmio_base + 0xb064)
>  #define _MIPIC_LP_GEN_DATA		(dev_priv->mipi_mmio_base + 0xb864)
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 6b63355..b78c686 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -1123,6 +1123,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder,
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> +	struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
>  	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>  	enum port port;
>  	unsigned int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
> @@ -1278,6 +1279,20 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder,
>  		 */
>  		I915_WRITE(MIPI_LP_BYTECLK(port), intel_dsi->lp_byte_clk);
>  
> +		if (IS_GEMINILAKE(dev_priv)) {
> +			I915_WRITE(MIPI_TLPX_TIME_COUNT(port),
> +					intel_dsi->lp_byte_clk);
> +			val = ((mipi_config->ths_prepare <<
> +				GLK_MIPI_CLK_LANE_HS_PREP_SHIFT) |
> +				(mipi_config->ths_prepare_hszero <<
> +				GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT) |
> +				(mipi_config->ths_trail <<
> +				GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT) |
> +				(mipi_config->ths_exit <<
> +				GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT));
> +			I915_WRITE(MIPI_CLK_LANE_TIMING(port), val);
> +		}

Please fix this as Ville suggested, i.e. don't look at mipi_config
directly in intel_dsi.c. See what gets done with dphy_reg.

We may change this later, but for now I think this is the right thing to
do.

BR,
Jani.

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

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO Enable/disable sequence

[Jani Nikula]

On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com> wrote:
> From: Deepak M <m.deepak@intel.com>
>
> v2: Addressed Jani's Review comments(renamed bit field macros)
>
> Signed-off-by: Deepak M <m.deepak@intel.com>
> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi.c | 134 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 134 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index b78c686..c0697e9 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -357,6 +357,134 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
>  	return true;
>  }
>  
> +static void intel_dsi_disable_mipiio(struct intel_encoder *encoder)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	enum port port;
> +	u32 tmp;
> +
> +	/* Put the IO into reset */
> +	tmp = I915_READ(MIPI_CTRL(PORT_A));
> +	tmp &= ~GLK_MIPIIO_RESET_RELEASED;
> +	I915_WRITE(MIPI_CTRL(PORT_A), tmp);
> +
> +	/* Wait for MIPI PHY status bit to unset */
> +	for_each_dsi_port(port, intel_dsi->ports) {
> +		if (intel_wait_for_register(dev_priv,
> +				MIPI_CTRL(port),
> +				GLK_PHY_STATUS_PORT_READY, 0, 20))
> +			DRM_ERROR("PHY is not turning OFF\n");
> +	}
> +
> +	/* Clear MIPI mode */
> +	for_each_dsi_port(port, intel_dsi->ports) {
> +		tmp = I915_READ(MIPI_CTRL(port));
> +		tmp &= ~GLK_MIPIIO_ENABLE;
> +		I915_WRITE(MIPI_CTRL(port), tmp);
> +	}
> +}
> +
> +static void intel_dsi_enable_mipiio(struct intel_encoder *encoder)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	enum port port;
> +	u32 tmp, val;
> +
> +	/* Put the IO into reset */
> +	tmp = I915_READ(MIPI_CTRL(PORT_A));
> +	tmp &= ~GLK_MIPIIO_RESET_RELEASED;
> +	I915_WRITE(MIPI_CTRL(PORT_A), tmp);
> +
> +	/* Program LP Wake */
> +	for_each_dsi_port(port, intel_dsi->ports) {
> +		tmp = I915_READ(MIPI_CTRL(port));
> +		tmp &= ~GLK_LP_WAKE;
> +		I915_WRITE(MIPI_CTRL(port), tmp);
> +	}
> +
> +	/* Set the MIPI mode */
> +	for_each_dsi_port(port, intel_dsi->ports) {
> +		tmp = I915_READ(MIPI_CTRL(port));
> +		I915_WRITE(MIPI_CTRL(port), tmp | GLK_MIPIIO_ENABLE);
> +	}
> +
> +	/* Wait for Pwr ACK */
> +	for_each_dsi_port(port, intel_dsi->ports) {
> +		if (intel_wait_for_register(dev_priv,
> +				MIPI_CTRL(port), GLK_MIPIIO_PORT_POWERED,
> +				GLK_MIPIIO_PORT_POWERED, 20))
> +			DRM_ERROR("Power ACK not received\n");
> +	}
> +
> +	/* Wait for MIPI PHY status bit to set */
> +	for_each_dsi_port(port, intel_dsi->ports) {
> +		if (intel_wait_for_register(dev_priv,
> +				MIPI_CTRL(port), GLK_MIPIIO_PORT_POWERED,
> +				GLK_MIPIIO_PORT_POWERED, 20))
> +			DRM_ERROR("PHY is not ON\n");
> +	}
> +
> +	/* Get IO out of reset */
> +	tmp = I915_READ(MIPI_CTRL(PORT_A));
> +	I915_WRITE(MIPI_CTRL(PORT_A), tmp | GLK_MIPIIO_RESET_RELEASED);
> +
> +	/* Get IO out of Low power state*/
> +	for_each_dsi_port(port, intel_dsi->ports) {
> +		if (!(I915_READ(MIPI_DEVICE_READY(port)) & DEVICE_READY)) {
> +			val = I915_READ(MIPI_DEVICE_READY(port));
> +			val &= ~ULPS_STATE_MASK;
> +			val |= DEVICE_READY;
> +			I915_WRITE(MIPI_DEVICE_READY(port), val);
> +			usleep_range(10, 15);
> +		}
> +
> +		/* Enter ULPS */
> +		val = I915_READ(MIPI_DEVICE_READY(port));
> +		val &= ~ULPS_STATE_MASK;
> +		val |= (ULPS_STATE_ENTER | DEVICE_READY);
> +		I915_WRITE(MIPI_DEVICE_READY(port), val);
> +
> +		/* Wait for ULPS Not active */
> +		if (intel_wait_for_register(dev_priv,
> +				MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE,
> +				GLK_ULPS_NOT_ACTIVE, 20))
> +
> +		/* Exit ULPS */
> +		val = I915_READ(MIPI_DEVICE_READY(port));
> +		val &= ~ULPS_STATE_MASK;
> +		val |= (ULPS_STATE_EXIT | DEVICE_READY);
> +		I915_WRITE(MIPI_DEVICE_READY(port), val);
> +
> +		/* Enter Normal Mode */
> +		val = I915_READ(MIPI_DEVICE_READY(port));
> +		val &= ~ULPS_STATE_MASK;
> +		val |= (ULPS_STATE_NORMAL_OPERATION | DEVICE_READY);
> +		I915_WRITE(MIPI_DEVICE_READY(port), val);
> +
> +		tmp = I915_READ(MIPI_CTRL(port));
> +		tmp &= ~GLK_LP_WAKE;
> +		I915_WRITE(MIPI_CTRL(port), tmp);
> +	}
> +
> +	/* Wait for Stop state */
> +	for_each_dsi_port(port, intel_dsi->ports) {
> +		if (intel_wait_for_register(dev_priv,
> +				MIPI_CTRL(port), GLK_DATA_LANE_STOP_STATE,
> +				GLK_DATA_LANE_STOP_STATE, 20))
> +			DRM_ERROR("Date lane not in STOP state\n");
> +	}
> +
> +	/* Wait for AFE LATCH */
> +	for_each_dsi_port(port, intel_dsi->ports) {
> +		if (intel_wait_for_register(dev_priv,
> +				BXT_MIPI_PORT_CTRL(port), AFE_LATCHOUT,
> +				AFE_LATCHOUT, 20))
> +			DRM_ERROR("D-PHY not entering LP-11 state\n");
> +	}
> +}
> +

I'm wondering if these should just be GLK versions of
intel_dsi_device_ready and intel_dsi_clear_device_ready. It seems
totally wrong that you're doing device ready stuff twice on GLK...


>  static void bxt_dsi_device_ready(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> @@ -559,6 +687,9 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder,
>  
>  	intel_dsi_prepare(encoder, pipe_config);
>  
> +	if (IS_GEMINILAKE(dev_priv))
> +		intel_dsi_enable_mipiio(encoder);
> +
>  	/* Panel Enable over CRC PMIC */
>  	if (intel_dsi->gpio_panel)
>  		gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1);
> @@ -699,6 +830,9 @@ static void intel_dsi_clear_device_ready(struct intel_encoder *encoder)
>  		usleep_range(2000, 2500);
>  	}
>  
> +	if (IS_GEMINILAKE(dev_priv))
> +		intel_dsi_disable_mipiio(encoder);
> +

When you're doing enable/disable of something, they should be called
from the corresponding functions in the enable/disable paths. But this
is just a general remark, if we conclude that these should be
alternative device ready/unready calls instead.

BR,
Jani.


>  	intel_disable_dsi_pll(encoder);
>  }

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [GLK MIPI DSI V2 2/9] drm/i915/glk: Program new MIPI DSI PHY registers for GLK

[Chauhan, Madhav]

> -----Original Message-----
> From: Nikula, Jani
> Sent: Friday, December 23, 2016 7:27 PM
> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>;
> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra
> <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit
> <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com>; Chauhan,
> Madhav <madhav.chauhan@intel.com>
> Subject: Re: [GLK MIPI DSI V2 2/9] drm/i915/glk: Program new MIPI DSI PHY
> registers for GLK
> 
> On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com>
> wrote:
> > From: Deepak M <m.deepak@intel.com>
> >
> > Program the clk lane and tlpx time count registers to configure DSI
> > PHY.
> >
> > v2: Addressed Jani's Review comments(renamed bit field macros)
> >
> > Signed-off-by: Deepak M <m.deepak@intel.com>
> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h  | 18 ++++++++++++++++++
> > drivers/gpu/drm/i915/intel_dsi.c | 15 +++++++++++++++
> >  2 files changed, 33 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index 8e47b59..03858f9 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -8550,6 +8550,24 @@ enum {
> >  #define  LP_BYTECLK_SHIFT				0
> >  #define  LP_BYTECLK_MASK				(0xffff << 0)
> >
> > +#define _MIPIA_TLPX_TIME_COUNT		(dev_priv->mipi_mmio_base
> + 0xb0a4)
> > +#define _MIPIC_TLPX_TIME_COUNT		(dev_priv->mipi_mmio_base
> + 0xb8a4)
> > +#define MIPI_TLPX_TIME_COUNT(port)	 _MMIO_MIPI(port,
> _MIPIA_TLPX_TIME_COUNT, _MIPIC_TLPX_TIME_COUNT)
> > +#define  GLK_DPHY_TLPX_TIME_CNT_SHIFT			0
> > +#define  GLK_DPHY_TLPX_TIME_CNT_MASK			(0xff << 0)
> > +
> > +#define _MIPIA_CLK_LANE_TIMING		(dev_priv->mipi_mmio_base
> + 0xb098)
> > +#define _MIPIC_CLK_LANE_TIMING		(dev_priv->mipi_mmio_base
> + 0xb898)
> > +#define MIPI_CLK_LANE_TIMING(port)	 _MMIO_MIPI(port,
> _MIPIA_CLK_LANE_TIMING, _MIPIC_CLK_LANE_TIMING)
> > +#define  GLK_MIPI_CLK_LANE_HS_PREP_SHIFT			0
> > +#define  GLK_MIPI_CLK_LANE_HS_PREP_MASK			(0xff
> << 0)
> > +#define  GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT			8
> > +#define  GLK_MIPI_CLK_LANE_HS_ZERO_MASK
> 	(0xff00 << 0)
> 
> 0xff << 8
> 
> > +#define  GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT			16
> > +#define  GLK_MIPI_CLK_LANE_HS_TRAIL_MASK
> 	(0xff0000 << 0)
> 
> 0xff << 16
> 
> > +#define  GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT			24
> > +#define  GLK_MIPI_CLK_LANE_HS_EXIT_MASK
> 	(0xff000000 << 0)
> 
> 0xff << 24
> 
> > +
> >  /* bits 31:0 */
> >  #define _MIPIA_LP_GEN_DATA		(dev_priv->mipi_mmio_base
> + 0xb064)
> >  #define _MIPIC_LP_GEN_DATA		(dev_priv->mipi_mmio_base
> + 0xb864)
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> > b/drivers/gpu/drm/i915/intel_dsi.c
> > index 6b63355..b78c686 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -1123,6 +1123,7 @@ static void intel_dsi_prepare(struct intel_encoder
> *intel_encoder,
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
> >  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> > +	struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
> >  	const struct drm_display_mode *adjusted_mode = &pipe_config-
> >base.adjusted_mode;
> >  	enum port port;
> >  	unsigned int bpp =
> > mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
> > @@ -1278,6 +1279,20 @@ static void intel_dsi_prepare(struct
> intel_encoder *intel_encoder,
> >  		 */
> >  		I915_WRITE(MIPI_LP_BYTECLK(port), intel_dsi->lp_byte_clk);
> >
> > +		if (IS_GEMINILAKE(dev_priv)) {
> > +			I915_WRITE(MIPI_TLPX_TIME_COUNT(port),
> > +					intel_dsi->lp_byte_clk);
> > +			val = ((mipi_config->ths_prepare <<
> > +				GLK_MIPI_CLK_LANE_HS_PREP_SHIFT) |
> > +				(mipi_config->ths_prepare_hszero <<
> > +				GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT) |
> > +				(mipi_config->ths_trail <<
> > +				GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT) |
> > +				(mipi_config->ths_exit <<
> > +				GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT));
> > +			I915_WRITE(MIPI_CLK_LANE_TIMING(port), val);
> > +		}
> 
> Please fix this as Ville suggested, i.e. don't look at mipi_config directly in
> intel_dsi.c. See what gets done with dphy_reg.
> 
> We may change this later, but for now I think this is the right thing to do.

Thanks. Will send the patch in next series, early next week.

> 
> BR,
> Jani.
> 
> > +
> >  		/* 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
> 
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO Enable/disable sequence

[Chauhan, Madhav]

> -----Original Message-----
> From: Nikula, Jani
> Sent: Friday, December 23, 2016 7:40 PM
> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>;
> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra
> <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit
> <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com>; Chauhan,
> Madhav <madhav.chauhan@intel.com>
> Subject: Re: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO Enable/disable
> sequence
> 
> On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com>
> wrote:
> > From: Deepak M <m.deepak@intel.com>
> >
> > v2: Addressed Jani's Review comments(renamed bit field macros)
> >
> > Signed-off-by: Deepak M <m.deepak@intel.com>
> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dsi.c | 134
> > +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 134 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> > b/drivers/gpu/drm/i915/intel_dsi.c
> > index b78c686..c0697e9 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -357,6 +357,134 @@ static bool intel_dsi_compute_config(struct
> intel_encoder *encoder,
> >  	return true;
> >  }
> >
> > +static void intel_dsi_disable_mipiio(struct intel_encoder *encoder) {
> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> > +	enum port port;
> > +	u32 tmp;
> > +
> > +	/* Put the IO into reset */
> > +	tmp = I915_READ(MIPI_CTRL(PORT_A));
> > +	tmp &= ~GLK_MIPIIO_RESET_RELEASED;
> > +	I915_WRITE(MIPI_CTRL(PORT_A), tmp);
> > +
> > +	/* Wait for MIPI PHY status bit to unset */
> > +	for_each_dsi_port(port, intel_dsi->ports) {
> > +		if (intel_wait_for_register(dev_priv,
> > +				MIPI_CTRL(port),
> > +				GLK_PHY_STATUS_PORT_READY, 0, 20))
> > +			DRM_ERROR("PHY is not turning OFF\n");
> > +	}
> > +
> > +	/* Clear MIPI mode */
> > +	for_each_dsi_port(port, intel_dsi->ports) {
> > +		tmp = I915_READ(MIPI_CTRL(port));
> > +		tmp &= ~GLK_MIPIIO_ENABLE;
> > +		I915_WRITE(MIPI_CTRL(port), tmp);
> > +	}
> > +}
> > +
> > +static void intel_dsi_enable_mipiio(struct intel_encoder *encoder) {
> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> > +	enum port port;
> > +	u32 tmp, val;
> > +
> > +	/* Put the IO into reset */
> > +	tmp = I915_READ(MIPI_CTRL(PORT_A));
> > +	tmp &= ~GLK_MIPIIO_RESET_RELEASED;
> > +	I915_WRITE(MIPI_CTRL(PORT_A), tmp);
> > +
> > +	/* Program LP Wake */
> > +	for_each_dsi_port(port, intel_dsi->ports) {
> > +		tmp = I915_READ(MIPI_CTRL(port));
> > +		tmp &= ~GLK_LP_WAKE;
> > +		I915_WRITE(MIPI_CTRL(port), tmp);
> > +	}
> > +
> > +	/* Set the MIPI mode */
> > +	for_each_dsi_port(port, intel_dsi->ports) {
> > +		tmp = I915_READ(MIPI_CTRL(port));
> > +		I915_WRITE(MIPI_CTRL(port), tmp | GLK_MIPIIO_ENABLE);
> > +	}
> > +
> > +	/* Wait for Pwr ACK */
> > +	for_each_dsi_port(port, intel_dsi->ports) {
> > +		if (intel_wait_for_register(dev_priv,
> > +				MIPI_CTRL(port),
> GLK_MIPIIO_PORT_POWERED,
> > +				GLK_MIPIIO_PORT_POWERED, 20))
> > +			DRM_ERROR("Power ACK not received\n");
> > +	}
> > +
> > +	/* Wait for MIPI PHY status bit to set */
> > +	for_each_dsi_port(port, intel_dsi->ports) {
> > +		if (intel_wait_for_register(dev_priv,
> > +				MIPI_CTRL(port),
> GLK_MIPIIO_PORT_POWERED,
> > +				GLK_MIPIIO_PORT_POWERED, 20))
> > +			DRM_ERROR("PHY is not ON\n");
> > +	}
> > +
> > +	/* Get IO out of reset */
> > +	tmp = I915_READ(MIPI_CTRL(PORT_A));
> > +	I915_WRITE(MIPI_CTRL(PORT_A), tmp |
> GLK_MIPIIO_RESET_RELEASED);
> > +
> > +	/* Get IO out of Low power state*/
> > +	for_each_dsi_port(port, intel_dsi->ports) {
> > +		if (!(I915_READ(MIPI_DEVICE_READY(port)) &
> DEVICE_READY)) {
> > +			val = I915_READ(MIPI_DEVICE_READY(port));
> > +			val &= ~ULPS_STATE_MASK;
> > +			val |= DEVICE_READY;
> > +			I915_WRITE(MIPI_DEVICE_READY(port), val);
> > +			usleep_range(10, 15);
> > +		}
> > +
> > +		/* Enter ULPS */
> > +		val = I915_READ(MIPI_DEVICE_READY(port));
> > +		val &= ~ULPS_STATE_MASK;
> > +		val |= (ULPS_STATE_ENTER | DEVICE_READY);
> > +		I915_WRITE(MIPI_DEVICE_READY(port), val);
> > +
> > +		/* Wait for ULPS Not active */
> > +		if (intel_wait_for_register(dev_priv,
> > +				MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE,
> > +				GLK_ULPS_NOT_ACTIVE, 20))
> > +
> > +		/* Exit ULPS */
> > +		val = I915_READ(MIPI_DEVICE_READY(port));
> > +		val &= ~ULPS_STATE_MASK;
> > +		val |= (ULPS_STATE_EXIT | DEVICE_READY);
> > +		I915_WRITE(MIPI_DEVICE_READY(port), val);
> > +
> > +		/* Enter Normal Mode */
> > +		val = I915_READ(MIPI_DEVICE_READY(port));
> > +		val &= ~ULPS_STATE_MASK;
> > +		val |= (ULPS_STATE_NORMAL_OPERATION | DEVICE_READY);
> > +		I915_WRITE(MIPI_DEVICE_READY(port), val);
> > +
> > +		tmp = I915_READ(MIPI_CTRL(port));
> > +		tmp &= ~GLK_LP_WAKE;
> > +		I915_WRITE(MIPI_CTRL(port), tmp);
> > +	}
> > +
> > +	/* Wait for Stop state */
> > +	for_each_dsi_port(port, intel_dsi->ports) {
> > +		if (intel_wait_for_register(dev_priv,
> > +				MIPI_CTRL(port),
> GLK_DATA_LANE_STOP_STATE,
> > +				GLK_DATA_LANE_STOP_STATE, 20))
> > +			DRM_ERROR("Date lane not in STOP state\n");
> > +	}
> > +
> > +	/* Wait for AFE LATCH */
> > +	for_each_dsi_port(port, intel_dsi->ports) {
> > +		if (intel_wait_for_register(dev_priv,
> > +				BXT_MIPI_PORT_CTRL(port), AFE_LATCHOUT,
> > +				AFE_LATCHOUT, 20))
> > +			DRM_ERROR("D-PHY not entering LP-11 state\n");
> > +	}
> > +}
> > +
> 
> I'm wondering if these should just be GLK versions of intel_dsi_device_ready
> and intel_dsi_clear_device_ready. It seems totally wrong that you're doing
> device ready stuff twice on GLK...

Agree. Don't need to call intel_dsi_device_ready for GLK, as device ready is already done inside enable_io.
Will do following :
If(!IS_GEMINILAKE(dev_priv)
intel_dsi_device_ready(encoder);

> 
> 
> >  static void bxt_dsi_device_ready(struct intel_encoder *encoder)  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> @@
> > -559,6 +687,9 @@ static void intel_dsi_pre_enable(struct intel_encoder
> > *encoder,
> >
> >  	intel_dsi_prepare(encoder, pipe_config);
> >
> > +	if (IS_GEMINILAKE(dev_priv))
> > +		intel_dsi_enable_mipiio(encoder);
> > +
> >  	/* Panel Enable over CRC PMIC */
> >  	if (intel_dsi->gpio_panel)
> >  		gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1); @@ -
> 699,6
> > +830,9 @@ static void intel_dsi_clear_device_ready(struct intel_encoder
> *encoder)
> >  		usleep_range(2000, 2500);
> >  	}
> >
> > +	if (IS_GEMINILAKE(dev_priv))
> > +		intel_dsi_disable_mipiio(encoder);
> > +
> 
> When you're doing enable/disable of something, they should be called from
> the corresponding functions in the enable/disable paths. But this is just a
> general remark, if we conclude that these should be alternative device
> ready/unready calls instead.

Yes agree, intel_dsi_disable_mipiio should be called from intel_dsi_post_disable after intel_dsi_clear_device_ready and before intel_disable_dsi_pll
Will do these changes in next series.
> 
> BR,
> Jani.
> 
> 
> >  	intel_disable_dsi_pll(encoder);
> >  }
> 
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [GLK MIPI DSI V2 2/9] drm/i915/glk: Program new MIPI DSI PHY registers for GLK

[Chauhan, Madhav]

> -----Original Message-----
> From: Chauhan, Madhav
> Sent: Saturday, December 24, 2016 12:53 AM
> To: Nikula, Jani <jani.nikula@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>;
> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra
> <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit
> <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com>
> Subject: RE: [GLK MIPI DSI V2 2/9] drm/i915/glk: Program new MIPI DSI PHY
> registers for GLK
> 
> > -----Original Message-----
> > From: Nikula, Jani
> > Sent: Friday, December 23, 2016 7:27 PM
> > To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
> > gfx@lists.freedesktop.org
> > Cc: Conselvan De Oliveira, Ander
> > <ander.conselvan.de.oliveira@intel.com>;
> > Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra
> > <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
> > Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit
> > <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com>; Chauhan,
> > Madhav <madhav.chauhan@intel.com>
> > Subject: Re: [GLK MIPI DSI V2 2/9] drm/i915/glk: Program new MIPI DSI
> > PHY registers for GLK
> >
> > On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com>
> > wrote:
> > > From: Deepak M <m.deepak@intel.com>
> > >
> > > Program the clk lane and tlpx time count registers to configure DSI
> > > PHY.
> > >
> > > v2: Addressed Jani's Review comments(renamed bit field macros)
> > >
> > > Signed-off-by: Deepak M <m.deepak@intel.com>
> > > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h  | 18 ++++++++++++++++++
> > > drivers/gpu/drm/i915/intel_dsi.c | 15 +++++++++++++++
> > >  2 files changed, 33 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h index 8e47b59..03858f9 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -8550,6 +8550,24 @@ enum {
> > >  #define  LP_BYTECLK_SHIFT				0
> > >  #define  LP_BYTECLK_MASK				(0xffff << 0)
> > >
> > > +#define _MIPIA_TLPX_TIME_COUNT		(dev_priv-
> >mipi_mmio_base
> > + 0xb0a4)
> > > +#define _MIPIC_TLPX_TIME_COUNT		(dev_priv-
> >mipi_mmio_base
> > + 0xb8a4)
> > > +#define MIPI_TLPX_TIME_COUNT(port)	 _MMIO_MIPI(port,
> > _MIPIA_TLPX_TIME_COUNT, _MIPIC_TLPX_TIME_COUNT)
> > > +#define  GLK_DPHY_TLPX_TIME_CNT_SHIFT			0
> > > +#define  GLK_DPHY_TLPX_TIME_CNT_MASK			(0xff
> << 0)
> > > +
> > > +#define _MIPIA_CLK_LANE_TIMING		(dev_priv-
> >mipi_mmio_base
> > + 0xb098)
> > > +#define _MIPIC_CLK_LANE_TIMING		(dev_priv-
> >mipi_mmio_base
> > + 0xb898)
> > > +#define MIPI_CLK_LANE_TIMING(port)	 _MMIO_MIPI(port,
> > _MIPIA_CLK_LANE_TIMING, _MIPIC_CLK_LANE_TIMING)
> > > +#define  GLK_MIPI_CLK_LANE_HS_PREP_SHIFT			0
> > > +#define  GLK_MIPI_CLK_LANE_HS_PREP_MASK			(0xff
> > << 0)
> > > +#define  GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT			8
> > > +#define  GLK_MIPI_CLK_LANE_HS_ZERO_MASK
> > 	(0xff00 << 0)
> >
> > 0xff << 8
> >
> > > +#define  GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT			16
> > > +#define  GLK_MIPI_CLK_LANE_HS_TRAIL_MASK
> > 	(0xff0000 << 0)
> >
> > 0xff << 16
> >
> > > +#define  GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT			24
> > > +#define  GLK_MIPI_CLK_LANE_HS_EXIT_MASK
> > 	(0xff000000 << 0)
> >
> > 0xff << 24
> >
> > > +
> > >  /* bits 31:0 */
> > >  #define _MIPIA_LP_GEN_DATA		(dev_priv->mipi_mmio_base
> > + 0xb064)
> > >  #define _MIPIC_LP_GEN_DATA		(dev_priv->mipi_mmio_base
> > + 0xb864)
> > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> > > b/drivers/gpu/drm/i915/intel_dsi.c
> > > index 6b63355..b78c686 100644
> > > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > > @@ -1123,6 +1123,7 @@ static void intel_dsi_prepare(struct
> > > intel_encoder
> > *intel_encoder,
> > >  	struct drm_i915_private *dev_priv = to_i915(dev);
> > >  	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
> > >  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> > > +	struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
> > >  	const struct drm_display_mode *adjusted_mode = &pipe_config-
> > >base.adjusted_mode;
> > >  	enum port port;
> > >  	unsigned int bpp =
> > > mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
> > > @@ -1278,6 +1279,20 @@ static void intel_dsi_prepare(struct
> > intel_encoder *intel_encoder,
> > >  		 */
> > >  		I915_WRITE(MIPI_LP_BYTECLK(port), intel_dsi->lp_byte_clk);
> > >
> > > +		if (IS_GEMINILAKE(dev_priv)) {
> > > +			I915_WRITE(MIPI_TLPX_TIME_COUNT(port),
> > > +					intel_dsi->lp_byte_clk);
> > > +			val = ((mipi_config->ths_prepare <<
> > > +				GLK_MIPI_CLK_LANE_HS_PREP_SHIFT) |
> > > +				(mipi_config->ths_prepare_hszero <<
> > > +				GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT) |
> > > +				(mipi_config->ths_trail <<
> > > +				GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT) |
> > > +				(mipi_config->ths_exit <<
> > > +				GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT));
> > > +			I915_WRITE(MIPI_CLK_LANE_TIMING(port), val);
> > > +		}
> >
> > Please fix this as Ville suggested, i.e. don't look at mipi_config
> > directly in intel_dsi.c. See what gets done with dphy_reg.
> >
> > We may change this later, but for now I think this is the right thing to do.
> 
> Thanks. Will send the patch in next series, early next week.

For GLK, DPHY_PARAM_REG parameters have to be programmed in terms of HS byte clock count.
Till BXT it used to be programmed in HS DDR Clock count, BSPEC is getting updated for this.
Due to above, MIPI_CLK_LANE_TIMING is shadow register of DPHY_PARAM_REG and both have to programmed same value.
Check this by printing the values inside driver for these registers. Hence we can use intel_dsi->dphy_reg for both.

> 
> >
> > BR,
> > Jani.
> >
> > > +
> > >  		/* 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
> >
> > --
> > Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO Enable/disable sequence

[Jani Nikula]

On Mon, 26 Dec 2016, "Chauhan, Madhav" <madhav.chauhan@intel.com> wrote:
>> -----Original Message-----
>> From: Nikula, Jani
>> Sent: Friday, December 23, 2016 7:40 PM
>> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>;
>> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra
>> <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
>> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit
>> <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com>; Chauhan,
>> Madhav <madhav.chauhan@intel.com>
>> Subject: Re: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO Enable/disable
>> sequence
>> 
>> On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com>
>> wrote:
>> > From: Deepak M <m.deepak@intel.com>
>> >
>> > v2: Addressed Jani's Review comments(renamed bit field macros)
>> >
>> > Signed-off-by: Deepak M <m.deepak@intel.com>
>> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_dsi.c | 134
>> > +++++++++++++++++++++++++++++++++++++++
>> >  1 file changed, 134 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
>> > b/drivers/gpu/drm/i915/intel_dsi.c
>> > index b78c686..c0697e9 100644
>> > --- a/drivers/gpu/drm/i915/intel_dsi.c
>> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> > @@ -357,6 +357,134 @@ static bool intel_dsi_compute_config(struct
>> intel_encoder *encoder,
>> >  	return true;
>> >  }
>> >
>> > +static void intel_dsi_disable_mipiio(struct intel_encoder *encoder) {
>> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> > +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>> > +	enum port port;
>> > +	u32 tmp;
>> > +
>> > +	/* Put the IO into reset */
>> > +	tmp = I915_READ(MIPI_CTRL(PORT_A));
>> > +	tmp &= ~GLK_MIPIIO_RESET_RELEASED;
>> > +	I915_WRITE(MIPI_CTRL(PORT_A), tmp);
>> > +
>> > +	/* Wait for MIPI PHY status bit to unset */
>> > +	for_each_dsi_port(port, intel_dsi->ports) {
>> > +		if (intel_wait_for_register(dev_priv,
>> > +				MIPI_CTRL(port),
>> > +				GLK_PHY_STATUS_PORT_READY, 0, 20))
>> > +			DRM_ERROR("PHY is not turning OFF\n");
>> > +	}
>> > +
>> > +	/* Clear MIPI mode */
>> > +	for_each_dsi_port(port, intel_dsi->ports) {
>> > +		tmp = I915_READ(MIPI_CTRL(port));
>> > +		tmp &= ~GLK_MIPIIO_ENABLE;
>> > +		I915_WRITE(MIPI_CTRL(port), tmp);
>> > +	}
>> > +}
>> > +
>> > +static void intel_dsi_enable_mipiio(struct intel_encoder *encoder) {
>> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> > +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>> > +	enum port port;
>> > +	u32 tmp, val;
>> > +
>> > +	/* Put the IO into reset */
>> > +	tmp = I915_READ(MIPI_CTRL(PORT_A));
>> > +	tmp &= ~GLK_MIPIIO_RESET_RELEASED;
>> > +	I915_WRITE(MIPI_CTRL(PORT_A), tmp);
>> > +
>> > +	/* Program LP Wake */
>> > +	for_each_dsi_port(port, intel_dsi->ports) {
>> > +		tmp = I915_READ(MIPI_CTRL(port));
>> > +		tmp &= ~GLK_LP_WAKE;
>> > +		I915_WRITE(MIPI_CTRL(port), tmp);
>> > +	}
>> > +
>> > +	/* Set the MIPI mode */
>> > +	for_each_dsi_port(port, intel_dsi->ports) {
>> > +		tmp = I915_READ(MIPI_CTRL(port));
>> > +		I915_WRITE(MIPI_CTRL(port), tmp | GLK_MIPIIO_ENABLE);
>> > +	}
>> > +
>> > +	/* Wait for Pwr ACK */
>> > +	for_each_dsi_port(port, intel_dsi->ports) {
>> > +		if (intel_wait_for_register(dev_priv,
>> > +				MIPI_CTRL(port),
>> GLK_MIPIIO_PORT_POWERED,
>> > +				GLK_MIPIIO_PORT_POWERED, 20))
>> > +			DRM_ERROR("Power ACK not received\n");
>> > +	}
>> > +
>> > +	/* Wait for MIPI PHY status bit to set */
>> > +	for_each_dsi_port(port, intel_dsi->ports) {
>> > +		if (intel_wait_for_register(dev_priv,
>> > +				MIPI_CTRL(port),
>> GLK_MIPIIO_PORT_POWERED,
>> > +				GLK_MIPIIO_PORT_POWERED, 20))
>> > +			DRM_ERROR("PHY is not ON\n");
>> > +	}
>> > +
>> > +	/* Get IO out of reset */
>> > +	tmp = I915_READ(MIPI_CTRL(PORT_A));
>> > +	I915_WRITE(MIPI_CTRL(PORT_A), tmp |
>> GLK_MIPIIO_RESET_RELEASED);
>> > +
>> > +	/* Get IO out of Low power state*/
>> > +	for_each_dsi_port(port, intel_dsi->ports) {
>> > +		if (!(I915_READ(MIPI_DEVICE_READY(port)) &
>> DEVICE_READY)) {
>> > +			val = I915_READ(MIPI_DEVICE_READY(port));
>> > +			val &= ~ULPS_STATE_MASK;
>> > +			val |= DEVICE_READY;
>> > +			I915_WRITE(MIPI_DEVICE_READY(port), val);
>> > +			usleep_range(10, 15);
>> > +		}
>> > +
>> > +		/* Enter ULPS */
>> > +		val = I915_READ(MIPI_DEVICE_READY(port));
>> > +		val &= ~ULPS_STATE_MASK;
>> > +		val |= (ULPS_STATE_ENTER | DEVICE_READY);
>> > +		I915_WRITE(MIPI_DEVICE_READY(port), val);
>> > +
>> > +		/* Wait for ULPS Not active */
>> > +		if (intel_wait_for_register(dev_priv,
>> > +				MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE,
>> > +				GLK_ULPS_NOT_ACTIVE, 20))
>> > +
>> > +		/* Exit ULPS */
>> > +		val = I915_READ(MIPI_DEVICE_READY(port));
>> > +		val &= ~ULPS_STATE_MASK;
>> > +		val |= (ULPS_STATE_EXIT | DEVICE_READY);
>> > +		I915_WRITE(MIPI_DEVICE_READY(port), val);
>> > +
>> > +		/* Enter Normal Mode */
>> > +		val = I915_READ(MIPI_DEVICE_READY(port));
>> > +		val &= ~ULPS_STATE_MASK;
>> > +		val |= (ULPS_STATE_NORMAL_OPERATION | DEVICE_READY);
>> > +		I915_WRITE(MIPI_DEVICE_READY(port), val);
>> > +
>> > +		tmp = I915_READ(MIPI_CTRL(port));
>> > +		tmp &= ~GLK_LP_WAKE;
>> > +		I915_WRITE(MIPI_CTRL(port), tmp);
>> > +	}
>> > +
>> > +	/* Wait for Stop state */
>> > +	for_each_dsi_port(port, intel_dsi->ports) {
>> > +		if (intel_wait_for_register(dev_priv,
>> > +				MIPI_CTRL(port),
>> GLK_DATA_LANE_STOP_STATE,
>> > +				GLK_DATA_LANE_STOP_STATE, 20))
>> > +			DRM_ERROR("Date lane not in STOP state\n");
>> > +	}
>> > +
>> > +	/* Wait for AFE LATCH */
>> > +	for_each_dsi_port(port, intel_dsi->ports) {
>> > +		if (intel_wait_for_register(dev_priv,
>> > +				BXT_MIPI_PORT_CTRL(port), AFE_LATCHOUT,
>> > +				AFE_LATCHOUT, 20))
>> > +			DRM_ERROR("D-PHY not entering LP-11 state\n");
>> > +	}
>> > +}
>> > +
>> 
>> I'm wondering if these should just be GLK versions of intel_dsi_device_ready
>> and intel_dsi_clear_device_ready. It seems totally wrong that you're doing
>> device ready stuff twice on GLK...
>
> Agree. Don't need to call intel_dsi_device_ready for GLK, as device ready is already done inside enable_io.
> Will do following :
> If(!IS_GEMINILAKE(dev_priv)
> intel_dsi_device_ready(encoder);
>
>> 
>> 
>> >  static void bxt_dsi_device_ready(struct intel_encoder *encoder)  {
>> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> @@
>> > -559,6 +687,9 @@ static void intel_dsi_pre_enable(struct intel_encoder
>> > *encoder,
>> >
>> >  	intel_dsi_prepare(encoder, pipe_config);
>> >
>> > +	if (IS_GEMINILAKE(dev_priv))
>> > +		intel_dsi_enable_mipiio(encoder);
>> > +
>> >  	/* Panel Enable over CRC PMIC */
>> >  	if (intel_dsi->gpio_panel)
>> >  		gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1); @@ -
>> 699,6
>> > +830,9 @@ static void intel_dsi_clear_device_ready(struct intel_encoder
>> *encoder)
>> >  		usleep_range(2000, 2500);
>> >  	}
>> >
>> > +	if (IS_GEMINILAKE(dev_priv))
>> > +		intel_dsi_disable_mipiio(encoder);
>> > +
>> 
>> When you're doing enable/disable of something, they should be called from
>> the corresponding functions in the enable/disable paths. But this is just a
>> general remark, if we conclude that these should be alternative device
>> ready/unready calls instead.
>
> Yes agree, intel_dsi_disable_mipiio should be called from intel_dsi_post_disable after intel_dsi_clear_device_ready and before intel_disable_dsi_pll
> Will do these changes in next series.

I meant, how about renaming intel_dsi_enable_mipiio to
glk_dsi_device_ready, and making intel_dsi_device_ready call
glk_dsi_device_ready for GLK.

Then rename intel_dsi_clear_device_ready to vlv_dsi_clear_device_ready
and intel_dsi_disable_mipiio to glk_dsi_clear_device_ready, and add a
new intel_dsi_clear_device_ready wrapper to call
vlv_dsi_clear_device_ready for VLV/CHV/BXT, and
glk_dsi_clear_device_ready for GLK.

How does that sound?


BR,
Jani.

>> 
>> BR,
>> Jani.
>> 
>> 
>> >  	intel_disable_dsi_pll(encoder);
>> >  }
>> 
>> --
>> Jani Nikula, Intel Open Source Technology Center

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO Enable/disable sequence

[Chauhan, Madhav]

> -----Original Message-----
> From: Nikula, Jani
> Sent: Tuesday, December 27, 2016 6:04 PM
> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>;
> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra
> <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit
> <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com>
> Subject: RE: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO Enable/disable
> sequence
> 
> On Mon, 26 Dec 2016, "Chauhan, Madhav" <madhav.chauhan@intel.com>
> wrote:
> >> -----Original Message-----
> >> From: Nikula, Jani
> >> Sent: Friday, December 23, 2016 7:40 PM
> >> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
> >> gfx@lists.freedesktop.org
> >> Cc: Conselvan De Oliveira, Ander
> >> <ander.conselvan.de.oliveira@intel.com>;
> >> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra
> >> <chandra.konduru@intel.com>; Shankar, Uma
> <uma.shankar@intel.com>;
> >> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit
> >> <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com>;
> Chauhan,
> >> Madhav <madhav.chauhan@intel.com>
> >> Subject: Re: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO
> >> Enable/disable sequence
> >>
> >> On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com>
> >> wrote:
> >> > From: Deepak M <m.deepak@intel.com>
> >> >
> >> > v2: Addressed Jani's Review comments(renamed bit field macros)
> >> >
> >> > Signed-off-by: Deepak M <m.deepak@intel.com>
> >> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/intel_dsi.c | 134
> >> > +++++++++++++++++++++++++++++++++++++++
> >> >  1 file changed, 134 insertions(+)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> >> > b/drivers/gpu/drm/i915/intel_dsi.c
> >> > index b78c686..c0697e9 100644
> >> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> >> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> >> > @@ -357,6 +357,134 @@ static bool intel_dsi_compute_config(struct
> >> intel_encoder *encoder,
> >> >  	return true;
> >> >  }
> >> >
> >> > +static void intel_dsi_disable_mipiio(struct intel_encoder *encoder) {
> >> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >> > +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> >> > +	enum port port;
> >> > +	u32 tmp;
> >> > +
> >> > +	/* Put the IO into reset */
> >> > +	tmp = I915_READ(MIPI_CTRL(PORT_A));
> >> > +	tmp &= ~GLK_MIPIIO_RESET_RELEASED;
> >> > +	I915_WRITE(MIPI_CTRL(PORT_A), tmp);
> >> > +
> >> > +	/* Wait for MIPI PHY status bit to unset */
> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
> >> > +		if (intel_wait_for_register(dev_priv,
> >> > +				MIPI_CTRL(port),
> >> > +				GLK_PHY_STATUS_PORT_READY, 0, 20))
> >> > +			DRM_ERROR("PHY is not turning OFF\n");
> >> > +	}
> >> > +
> >> > +	/* Clear MIPI mode */
> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
> >> > +		tmp = I915_READ(MIPI_CTRL(port));
> >> > +		tmp &= ~GLK_MIPIIO_ENABLE;
> >> > +		I915_WRITE(MIPI_CTRL(port), tmp);
> >> > +	}
> >> > +}
> >> > +
> >> > +static void intel_dsi_enable_mipiio(struct intel_encoder *encoder) {
> >> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >> > +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> >> > +	enum port port;
> >> > +	u32 tmp, val;
> >> > +
> >> > +	/* Put the IO into reset */
> >> > +	tmp = I915_READ(MIPI_CTRL(PORT_A));
> >> > +	tmp &= ~GLK_MIPIIO_RESET_RELEASED;
> >> > +	I915_WRITE(MIPI_CTRL(PORT_A), tmp);
> >> > +
> >> > +	/* Program LP Wake */
> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
> >> > +		tmp = I915_READ(MIPI_CTRL(port));
> >> > +		tmp &= ~GLK_LP_WAKE;
> >> > +		I915_WRITE(MIPI_CTRL(port), tmp);
> >> > +	}
> >> > +
> >> > +	/* Set the MIPI mode */
> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
> >> > +		tmp = I915_READ(MIPI_CTRL(port));
> >> > +		I915_WRITE(MIPI_CTRL(port), tmp | GLK_MIPIIO_ENABLE);
> >> > +	}
> >> > +
> >> > +	/* Wait for Pwr ACK */
> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
> >> > +		if (intel_wait_for_register(dev_priv,
> >> > +				MIPI_CTRL(port),
> >> GLK_MIPIIO_PORT_POWERED,
> >> > +				GLK_MIPIIO_PORT_POWERED, 20))
> >> > +			DRM_ERROR("Power ACK not received\n");
> >> > +	}
> >> > +
> >> > +	/* Wait for MIPI PHY status bit to set */
> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
> >> > +		if (intel_wait_for_register(dev_priv,
> >> > +				MIPI_CTRL(port),
> >> GLK_MIPIIO_PORT_POWERED,
> >> > +				GLK_MIPIIO_PORT_POWERED, 20))
> >> > +			DRM_ERROR("PHY is not ON\n");
> >> > +	}
> >> > +
> >> > +	/* Get IO out of reset */
> >> > +	tmp = I915_READ(MIPI_CTRL(PORT_A));
> >> > +	I915_WRITE(MIPI_CTRL(PORT_A), tmp |
> >> GLK_MIPIIO_RESET_RELEASED);
> >> > +
> >> > +	/* Get IO out of Low power state*/
> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
> >> > +		if (!(I915_READ(MIPI_DEVICE_READY(port)) &
> >> DEVICE_READY)) {
> >> > +			val = I915_READ(MIPI_DEVICE_READY(port));
> >> > +			val &= ~ULPS_STATE_MASK;
> >> > +			val |= DEVICE_READY;
> >> > +			I915_WRITE(MIPI_DEVICE_READY(port), val);
> >> > +			usleep_range(10, 15);
> >> > +		}
> >> > +
> >> > +		/* Enter ULPS */
> >> > +		val = I915_READ(MIPI_DEVICE_READY(port));
> >> > +		val &= ~ULPS_STATE_MASK;
> >> > +		val |= (ULPS_STATE_ENTER | DEVICE_READY);
> >> > +		I915_WRITE(MIPI_DEVICE_READY(port), val);
> >> > +
> >> > +		/* Wait for ULPS Not active */
> >> > +		if (intel_wait_for_register(dev_priv,
> >> > +				MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE,
> >> > +				GLK_ULPS_NOT_ACTIVE, 20))
> >> > +
> >> > +		/* Exit ULPS */
> >> > +		val = I915_READ(MIPI_DEVICE_READY(port));
> >> > +		val &= ~ULPS_STATE_MASK;
> >> > +		val |= (ULPS_STATE_EXIT | DEVICE_READY);
> >> > +		I915_WRITE(MIPI_DEVICE_READY(port), val);
> >> > +
> >> > +		/* Enter Normal Mode */
> >> > +		val = I915_READ(MIPI_DEVICE_READY(port));
> >> > +		val &= ~ULPS_STATE_MASK;
> >> > +		val |= (ULPS_STATE_NORMAL_OPERATION | DEVICE_READY);
> >> > +		I915_WRITE(MIPI_DEVICE_READY(port), val);
> >> > +
> >> > +		tmp = I915_READ(MIPI_CTRL(port));
> >> > +		tmp &= ~GLK_LP_WAKE;
> >> > +		I915_WRITE(MIPI_CTRL(port), tmp);
> >> > +	}
> >> > +
> >> > +	/* Wait for Stop state */
> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
> >> > +		if (intel_wait_for_register(dev_priv,
> >> > +				MIPI_CTRL(port),
> >> GLK_DATA_LANE_STOP_STATE,
> >> > +				GLK_DATA_LANE_STOP_STATE, 20))
> >> > +			DRM_ERROR("Date lane not in STOP state\n");
> >> > +	}
> >> > +
> >> > +	/* Wait for AFE LATCH */
> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
> >> > +		if (intel_wait_for_register(dev_priv,
> >> > +				BXT_MIPI_PORT_CTRL(port), AFE_LATCHOUT,
> >> > +				AFE_LATCHOUT, 20))
> >> > +			DRM_ERROR("D-PHY not entering LP-11 state\n");
> >> > +	}
> >> > +}
> >> > +
> >>
> >> I'm wondering if these should just be GLK versions of
> >> intel_dsi_device_ready and intel_dsi_clear_device_ready. It seems
> >> totally wrong that you're doing device ready stuff twice on GLK...
> >
> > Agree. Don't need to call intel_dsi_device_ready for GLK, as device ready is
> already done inside enable_io.
> > Will do following :
> > If(!IS_GEMINILAKE(dev_priv)
> > intel_dsi_device_ready(encoder);
> >
> >>
> >>
> >> >  static void bxt_dsi_device_ready(struct intel_encoder *encoder)  {
> >> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >> @@
> >> > -559,6 +687,9 @@ static void intel_dsi_pre_enable(struct
> >> > intel_encoder *encoder,
> >> >
> >> >  	intel_dsi_prepare(encoder, pipe_config);
> >> >
> >> > +	if (IS_GEMINILAKE(dev_priv))
> >> > +		intel_dsi_enable_mipiio(encoder);
> >> > +
> >> >  	/* Panel Enable over CRC PMIC */
> >> >  	if (intel_dsi->gpio_panel)
> >> >  		gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1); @@ -
> >> 699,6
> >> > +830,9 @@ static void intel_dsi_clear_device_ready(struct
> >> > +intel_encoder
> >> *encoder)
> >> >  		usleep_range(2000, 2500);
> >> >  	}
> >> >
> >> > +	if (IS_GEMINILAKE(dev_priv))
> >> > +		intel_dsi_disable_mipiio(encoder);
> >> > +
> >>
> >> When you're doing enable/disable of something, they should be called
> >> from the corresponding functions in the enable/disable paths. But
> >> this is just a general remark, if we conclude that these should be
> >> alternative device ready/unready calls instead.
> >
> > Yes agree, intel_dsi_disable_mipiio should be called from
> > intel_dsi_post_disable after intel_dsi_clear_device_ready and before
> intel_disable_dsi_pll Will do these changes in next series.
> 
> I meant, how about renaming intel_dsi_enable_mipiio to
> glk_dsi_device_ready, and making intel_dsi_device_ready call
> glk_dsi_device_ready for GLK.
> 
> Then rename intel_dsi_clear_device_ready to vlv_dsi_clear_device_ready
> and intel_dsi_disable_mipiio to glk_dsi_clear_device_ready, and add a new
> intel_dsi_clear_device_ready wrapper to call vlv_dsi_clear_device_ready for
> VLV/CHV/BXT, and glk_dsi_clear_device_ready for GLK.
> 
> How does that sound?

That's good input. It will align the code to the platforms.
For glk_dsi_clear_device_ready, we need to 1. Disable MIPI IO, 2. Enter LP
Shouldn't we add 2 separate functions for 1,2 and call them inside glk_dsi_clear_device_ready, it will make code readable/modularize as per BSPEC??

> 
> 
> BR,
> Jani.
> 
> >>
> >> BR,
> >> Jani.
> >>
> >>
> >> >  	intel_disable_dsi_pll(encoder);
> >> >  }
> >>
> >> --
> >> Jani Nikula, Intel Open Source Technology Center
> 
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO Enable/disable sequence

[Jani Nikula]

On Tue, 27 Dec 2016, "Chauhan, Madhav" <madhav.chauhan@intel.com> wrote:
>> -----Original Message-----
>> From: Nikula, Jani
>> Sent: Tuesday, December 27, 2016 6:04 PM
>> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>;
>> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra
>> <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
>> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit
>> <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com>
>> Subject: RE: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO Enable/disable
>> sequence
>> 
>> On Mon, 26 Dec 2016, "Chauhan, Madhav" <madhav.chauhan@intel.com>
>> wrote:
>> >> -----Original Message-----
>> >> From: Nikula, Jani
>> >> Sent: Friday, December 23, 2016 7:40 PM
>> >> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
>> >> gfx@lists.freedesktop.org
>> >> Cc: Conselvan De Oliveira, Ander
>> >> <ander.conselvan.de.oliveira@intel.com>;
>> >> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra
>> >> <chandra.konduru@intel.com>; Shankar, Uma
>> <uma.shankar@intel.com>;
>> >> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit
>> >> <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com>;
>> Chauhan,
>> >> Madhav <madhav.chauhan@intel.com>
>> >> Subject: Re: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO
>> >> Enable/disable sequence
>> >>
>> >> On Thu, 15 Dec 2016, Madhav Chauhan <madhav.chauhan@intel.com>
>> >> wrote:
>> >> > From: Deepak M <m.deepak@intel.com>
>> >> >
>> >> > v2: Addressed Jani's Review comments(renamed bit field macros)
>> >> >
>> >> > Signed-off-by: Deepak M <m.deepak@intel.com>
>> >> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
>> >> > ---
>> >> >  drivers/gpu/drm/i915/intel_dsi.c | 134
>> >> > +++++++++++++++++++++++++++++++++++++++
>> >> >  1 file changed, 134 insertions(+)
>> >> >
>> >> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
>> >> > b/drivers/gpu/drm/i915/intel_dsi.c
>> >> > index b78c686..c0697e9 100644
>> >> > --- a/drivers/gpu/drm/i915/intel_dsi.c
>> >> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> >> > @@ -357,6 +357,134 @@ static bool intel_dsi_compute_config(struct
>> >> intel_encoder *encoder,
>> >> >  	return true;
>> >> >  }
>> >> >
>> >> > +static void intel_dsi_disable_mipiio(struct intel_encoder *encoder) {
>> >> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> >> > +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>> >> > +	enum port port;
>> >> > +	u32 tmp;
>> >> > +
>> >> > +	/* Put the IO into reset */
>> >> > +	tmp = I915_READ(MIPI_CTRL(PORT_A));
>> >> > +	tmp &= ~GLK_MIPIIO_RESET_RELEASED;
>> >> > +	I915_WRITE(MIPI_CTRL(PORT_A), tmp);
>> >> > +
>> >> > +	/* Wait for MIPI PHY status bit to unset */
>> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
>> >> > +		if (intel_wait_for_register(dev_priv,
>> >> > +				MIPI_CTRL(port),
>> >> > +				GLK_PHY_STATUS_PORT_READY, 0, 20))
>> >> > +			DRM_ERROR("PHY is not turning OFF\n");
>> >> > +	}
>> >> > +
>> >> > +	/* Clear MIPI mode */
>> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
>> >> > +		tmp = I915_READ(MIPI_CTRL(port));
>> >> > +		tmp &= ~GLK_MIPIIO_ENABLE;
>> >> > +		I915_WRITE(MIPI_CTRL(port), tmp);
>> >> > +	}
>> >> > +}
>> >> > +
>> >> > +static void intel_dsi_enable_mipiio(struct intel_encoder *encoder) {
>> >> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> >> > +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>> >> > +	enum port port;
>> >> > +	u32 tmp, val;
>> >> > +
>> >> > +	/* Put the IO into reset */
>> >> > +	tmp = I915_READ(MIPI_CTRL(PORT_A));
>> >> > +	tmp &= ~GLK_MIPIIO_RESET_RELEASED;
>> >> > +	I915_WRITE(MIPI_CTRL(PORT_A), tmp);
>> >> > +
>> >> > +	/* Program LP Wake */
>> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
>> >> > +		tmp = I915_READ(MIPI_CTRL(port));
>> >> > +		tmp &= ~GLK_LP_WAKE;
>> >> > +		I915_WRITE(MIPI_CTRL(port), tmp);
>> >> > +	}
>> >> > +
>> >> > +	/* Set the MIPI mode */
>> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
>> >> > +		tmp = I915_READ(MIPI_CTRL(port));
>> >> > +		I915_WRITE(MIPI_CTRL(port), tmp | GLK_MIPIIO_ENABLE);
>> >> > +	}
>> >> > +
>> >> > +	/* Wait for Pwr ACK */
>> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
>> >> > +		if (intel_wait_for_register(dev_priv,
>> >> > +				MIPI_CTRL(port),
>> >> GLK_MIPIIO_PORT_POWERED,
>> >> > +				GLK_MIPIIO_PORT_POWERED, 20))
>> >> > +			DRM_ERROR("Power ACK not received\n");
>> >> > +	}
>> >> > +
>> >> > +	/* Wait for MIPI PHY status bit to set */
>> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
>> >> > +		if (intel_wait_for_register(dev_priv,
>> >> > +				MIPI_CTRL(port),
>> >> GLK_MIPIIO_PORT_POWERED,
>> >> > +				GLK_MIPIIO_PORT_POWERED, 20))
>> >> > +			DRM_ERROR("PHY is not ON\n");
>> >> > +	}
>> >> > +
>> >> > +	/* Get IO out of reset */
>> >> > +	tmp = I915_READ(MIPI_CTRL(PORT_A));
>> >> > +	I915_WRITE(MIPI_CTRL(PORT_A), tmp |
>> >> GLK_MIPIIO_RESET_RELEASED);
>> >> > +
>> >> > +	/* Get IO out of Low power state*/
>> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
>> >> > +		if (!(I915_READ(MIPI_DEVICE_READY(port)) &
>> >> DEVICE_READY)) {
>> >> > +			val = I915_READ(MIPI_DEVICE_READY(port));
>> >> > +			val &= ~ULPS_STATE_MASK;
>> >> > +			val |= DEVICE_READY;
>> >> > +			I915_WRITE(MIPI_DEVICE_READY(port), val);
>> >> > +			usleep_range(10, 15);
>> >> > +		}
>> >> > +
>> >> > +		/* Enter ULPS */
>> >> > +		val = I915_READ(MIPI_DEVICE_READY(port));
>> >> > +		val &= ~ULPS_STATE_MASK;
>> >> > +		val |= (ULPS_STATE_ENTER | DEVICE_READY);
>> >> > +		I915_WRITE(MIPI_DEVICE_READY(port), val);
>> >> > +
>> >> > +		/* Wait for ULPS Not active */
>> >> > +		if (intel_wait_for_register(dev_priv,
>> >> > +				MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE,
>> >> > +				GLK_ULPS_NOT_ACTIVE, 20))
>> >> > +
>> >> > +		/* Exit ULPS */
>> >> > +		val = I915_READ(MIPI_DEVICE_READY(port));
>> >> > +		val &= ~ULPS_STATE_MASK;
>> >> > +		val |= (ULPS_STATE_EXIT | DEVICE_READY);
>> >> > +		I915_WRITE(MIPI_DEVICE_READY(port), val);
>> >> > +
>> >> > +		/* Enter Normal Mode */
>> >> > +		val = I915_READ(MIPI_DEVICE_READY(port));
>> >> > +		val &= ~ULPS_STATE_MASK;
>> >> > +		val |= (ULPS_STATE_NORMAL_OPERATION | DEVICE_READY);
>> >> > +		I915_WRITE(MIPI_DEVICE_READY(port), val);
>> >> > +
>> >> > +		tmp = I915_READ(MIPI_CTRL(port));
>> >> > +		tmp &= ~GLK_LP_WAKE;
>> >> > +		I915_WRITE(MIPI_CTRL(port), tmp);
>> >> > +	}
>> >> > +
>> >> > +	/* Wait for Stop state */
>> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
>> >> > +		if (intel_wait_for_register(dev_priv,
>> >> > +				MIPI_CTRL(port),
>> >> GLK_DATA_LANE_STOP_STATE,
>> >> > +				GLK_DATA_LANE_STOP_STATE, 20))
>> >> > +			DRM_ERROR("Date lane not in STOP state\n");
>> >> > +	}
>> >> > +
>> >> > +	/* Wait for AFE LATCH */
>> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
>> >> > +		if (intel_wait_for_register(dev_priv,
>> >> > +				BXT_MIPI_PORT_CTRL(port), AFE_LATCHOUT,
>> >> > +				AFE_LATCHOUT, 20))
>> >> > +			DRM_ERROR("D-PHY not entering LP-11 state\n");
>> >> > +	}
>> >> > +}
>> >> > +
>> >>
>> >> I'm wondering if these should just be GLK versions of
>> >> intel_dsi_device_ready and intel_dsi_clear_device_ready. It seems
>> >> totally wrong that you're doing device ready stuff twice on GLK...
>> >
>> > Agree. Don't need to call intel_dsi_device_ready for GLK, as device ready is
>> already done inside enable_io.
>> > Will do following :
>> > If(!IS_GEMINILAKE(dev_priv)
>> > intel_dsi_device_ready(encoder);
>> >
>> >>
>> >>
>> >> >  static void bxt_dsi_device_ready(struct intel_encoder *encoder)  {
>> >> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> >> @@
>> >> > -559,6 +687,9 @@ static void intel_dsi_pre_enable(struct
>> >> > intel_encoder *encoder,
>> >> >
>> >> >  	intel_dsi_prepare(encoder, pipe_config);
>> >> >
>> >> > +	if (IS_GEMINILAKE(dev_priv))
>> >> > +		intel_dsi_enable_mipiio(encoder);
>> >> > +
>> >> >  	/* Panel Enable over CRC PMIC */
>> >> >  	if (intel_dsi->gpio_panel)
>> >> >  		gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1); @@ -
>> >> 699,6
>> >> > +830,9 @@ static void intel_dsi_clear_device_ready(struct
>> >> > +intel_encoder
>> >> *encoder)
>> >> >  		usleep_range(2000, 2500);
>> >> >  	}
>> >> >
>> >> > +	if (IS_GEMINILAKE(dev_priv))
>> >> > +		intel_dsi_disable_mipiio(encoder);
>> >> > +
>> >>
>> >> When you're doing enable/disable of something, they should be called
>> >> from the corresponding functions in the enable/disable paths. But
>> >> this is just a general remark, if we conclude that these should be
>> >> alternative device ready/unready calls instead.
>> >
>> > Yes agree, intel_dsi_disable_mipiio should be called from
>> > intel_dsi_post_disable after intel_dsi_clear_device_ready and before
>> intel_disable_dsi_pll Will do these changes in next series.
>> 
>> I meant, how about renaming intel_dsi_enable_mipiio to
>> glk_dsi_device_ready, and making intel_dsi_device_ready call
>> glk_dsi_device_ready for GLK.
>> 
>> Then rename intel_dsi_clear_device_ready to vlv_dsi_clear_device_ready
>> and intel_dsi_disable_mipiio to glk_dsi_clear_device_ready, and add a new
>> intel_dsi_clear_device_ready wrapper to call vlv_dsi_clear_device_ready for
>> VLV/CHV/BXT, and glk_dsi_clear_device_ready for GLK.
>> 
>> How does that sound?
>
> That's good input. It will align the code to the platforms.
> For glk_dsi_clear_device_ready, we need to 1. Disable MIPI IO, 2. Enter LP
> Shouldn't we add 2 separate functions for 1,2 and call them inside glk_dsi_clear_device_ready, it will make code readable/modularize as per BSPEC??

Sure, abstractions like that are fine.

BR,
Jani.

>
>> 
>> 
>> BR,
>> Jani.
>> 
>> >>
>> >> BR,
>> >> Jani.
>> >>
>> >>
>> >> >  	intel_disable_dsi_pll(encoder);
>> >> >  }
>> >>
>> >> --
>> >> Jani Nikula, Intel Open Source Technology Center
>> 
>> --
>> Jani Nikula, Intel Open Source Technology Center

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO Enable/disable sequence

[Chauhan, Madhav]

> -----Original Message-----
> From: Nikula, Jani
> Sent: Tuesday, December 27, 2016 8:18 PM
> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>;
> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra
> <chandra.konduru@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit
> <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com>
> Subject: RE: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO Enable/disable
> sequence
> 
> On Tue, 27 Dec 2016, "Chauhan, Madhav" <madhav.chauhan@intel.com>
> wrote:
> >> -----Original Message-----
> >> From: Nikula, Jani
> >> Sent: Tuesday, December 27, 2016 6:04 PM
> >> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
> >> gfx@lists.freedesktop.org
> >> Cc: Conselvan De Oliveira, Ander
> >> <ander.conselvan.de.oliveira@intel.com>;
> >> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra
> >> <chandra.konduru@intel.com>; Shankar, Uma
> <uma.shankar@intel.com>;
> >> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit
> >> <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com>
> >> Subject: RE: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO
> >> Enable/disable sequence
> >>
> >> On Mon, 26 Dec 2016, "Chauhan, Madhav"
> <madhav.chauhan@intel.com>
> >> wrote:
> >> >> -----Original Message-----
> >> >> From: Nikula, Jani
> >> >> Sent: Friday, December 23, 2016 7:40 PM
> >> >> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
> >> >> gfx@lists.freedesktop.org
> >> >> Cc: Conselvan De Oliveira, Ander
> >> >> <ander.conselvan.de.oliveira@intel.com>;
> >> >> Saarinen, Jani <jani.saarinen@intel.com>; Konduru, Chandra
> >> >> <chandra.konduru@intel.com>; Shankar, Uma
> >> <uma.shankar@intel.com>;
> >> >> Mukherjee, Indranil <indranil.mukherjee@intel.com>; Kumar, Shobhit
> >> >> <shobhit.kumar@intel.com>; Deepak M <m.deepak@intel.com>;
> >> Chauhan,
> >> >> Madhav <madhav.chauhan@intel.com>
> >> >> Subject: Re: [GLK MIPI DSI V2 3/9] drm/i915/glk: Add MIPIIO
> >> >> Enable/disable sequence
> >> >>
> >> >> On Thu, 15 Dec 2016, Madhav Chauhan
> <madhav.chauhan@intel.com>
> >> >> wrote:
> >> >> > From: Deepak M <m.deepak@intel.com>
> >> >> >
> >> >> > v2: Addressed Jani's Review comments(renamed bit field macros)
> >> >> >
> >> >> > Signed-off-by: Deepak M <m.deepak@intel.com>
> >> >> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> >> >> > ---
> >> >> >  drivers/gpu/drm/i915/intel_dsi.c | 134
> >> >> > +++++++++++++++++++++++++++++++++++++++
> >> >> >  1 file changed, 134 insertions(+)
> >> >> >
> >> >> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> >> >> > b/drivers/gpu/drm/i915/intel_dsi.c
> >> >> > index b78c686..c0697e9 100644
> >> >> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> >> >> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> >> >> > @@ -357,6 +357,134 @@ static bool
> >> >> > intel_dsi_compute_config(struct
> >> >> intel_encoder *encoder,
> >> >> >  	return true;
> >> >> >  }
> >> >> >
> >> >> > +static void intel_dsi_disable_mipiio(struct intel_encoder *encoder) {
> >> >> > +	struct drm_i915_private *dev_priv = to_i915(encoder-
> >base.dev);
> >> >> > +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder-
> >base);
> >> >> > +	enum port port;
> >> >> > +	u32 tmp;
> >> >> > +
> >> >> > +	/* Put the IO into reset */
> >> >> > +	tmp = I915_READ(MIPI_CTRL(PORT_A));
> >> >> > +	tmp &= ~GLK_MIPIIO_RESET_RELEASED;
> >> >> > +	I915_WRITE(MIPI_CTRL(PORT_A), tmp);
> >> >> > +
> >> >> > +	/* Wait for MIPI PHY status bit to unset */
> >> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
> >> >> > +		if (intel_wait_for_register(dev_priv,
> >> >> > +				MIPI_CTRL(port),
> >> >> > +				GLK_PHY_STATUS_PORT_READY, 0,
> 20))
> >> >> > +			DRM_ERROR("PHY is not turning OFF\n");
> >> >> > +	}
> >> >> > +
> >> >> > +	/* Clear MIPI mode */
> >> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
> >> >> > +		tmp = I915_READ(MIPI_CTRL(port));
> >> >> > +		tmp &= ~GLK_MIPIIO_ENABLE;
> >> >> > +		I915_WRITE(MIPI_CTRL(port), tmp);
> >> >> > +	}
> >> >> > +}
> >> >> > +
> >> >> > +static void intel_dsi_enable_mipiio(struct intel_encoder *encoder) {
> >> >> > +	struct drm_i915_private *dev_priv = to_i915(encoder-
> >base.dev);
> >> >> > +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder-
> >base);
> >> >> > +	enum port port;
> >> >> > +	u32 tmp, val;
> >> >> > +
> >> >> > +	/* Put the IO into reset */
> >> >> > +	tmp = I915_READ(MIPI_CTRL(PORT_A));
> >> >> > +	tmp &= ~GLK_MIPIIO_RESET_RELEASED;
> >> >> > +	I915_WRITE(MIPI_CTRL(PORT_A), tmp);
> >> >> > +
> >> >> > +	/* Program LP Wake */
> >> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
> >> >> > +		tmp = I915_READ(MIPI_CTRL(port));
> >> >> > +		tmp &= ~GLK_LP_WAKE;
> >> >> > +		I915_WRITE(MIPI_CTRL(port), tmp);
> >> >> > +	}
> >> >> > +
> >> >> > +	/* Set the MIPI mode */
> >> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
> >> >> > +		tmp = I915_READ(MIPI_CTRL(port));
> >> >> > +		I915_WRITE(MIPI_CTRL(port), tmp |
> GLK_MIPIIO_ENABLE);
> >> >> > +	}
> >> >> > +
> >> >> > +	/* Wait for Pwr ACK */
> >> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
> >> >> > +		if (intel_wait_for_register(dev_priv,
> >> >> > +				MIPI_CTRL(port),
> >> >> GLK_MIPIIO_PORT_POWERED,
> >> >> > +				GLK_MIPIIO_PORT_POWERED, 20))
> >> >> > +			DRM_ERROR("Power ACK not received\n");
> >> >> > +	}
> >> >> > +
> >> >> > +	/* Wait for MIPI PHY status bit to set */
> >> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
> >> >> > +		if (intel_wait_for_register(dev_priv,
> >> >> > +				MIPI_CTRL(port),
> >> >> GLK_MIPIIO_PORT_POWERED,
> >> >> > +				GLK_MIPIIO_PORT_POWERED, 20))
> >> >> > +			DRM_ERROR("PHY is not ON\n");
> >> >> > +	}
> >> >> > +
> >> >> > +	/* Get IO out of reset */
> >> >> > +	tmp = I915_READ(MIPI_CTRL(PORT_A));
> >> >> > +	I915_WRITE(MIPI_CTRL(PORT_A), tmp |
> >> >> GLK_MIPIIO_RESET_RELEASED);
> >> >> > +
> >> >> > +	/* Get IO out of Low power state*/
> >> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
> >> >> > +		if (!(I915_READ(MIPI_DEVICE_READY(port)) &
> >> >> DEVICE_READY)) {
> >> >> > +			val = I915_READ(MIPI_DEVICE_READY(port));
> >> >> > +			val &= ~ULPS_STATE_MASK;
> >> >> > +			val |= DEVICE_READY;
> >> >> > +			I915_WRITE(MIPI_DEVICE_READY(port), val);
> >> >> > +			usleep_range(10, 15);
> >> >> > +		}
> >> >> > +
> >> >> > +		/* Enter ULPS */
> >> >> > +		val = I915_READ(MIPI_DEVICE_READY(port));
> >> >> > +		val &= ~ULPS_STATE_MASK;
> >> >> > +		val |= (ULPS_STATE_ENTER | DEVICE_READY);
> >> >> > +		I915_WRITE(MIPI_DEVICE_READY(port), val);
> >> >> > +
> >> >> > +		/* Wait for ULPS Not active */
> >> >> > +		if (intel_wait_for_register(dev_priv,
> >> >> > +				MIPI_CTRL(port),
> GLK_ULPS_NOT_ACTIVE,
> >> >> > +				GLK_ULPS_NOT_ACTIVE, 20))
> >> >> > +
> >> >> > +		/* Exit ULPS */
> >> >> > +		val = I915_READ(MIPI_DEVICE_READY(port));
> >> >> > +		val &= ~ULPS_STATE_MASK;
> >> >> > +		val |= (ULPS_STATE_EXIT | DEVICE_READY);
> >> >> > +		I915_WRITE(MIPI_DEVICE_READY(port), val);
> >> >> > +
> >> >> > +		/* Enter Normal Mode */
> >> >> > +		val = I915_READ(MIPI_DEVICE_READY(port));
> >> >> > +		val &= ~ULPS_STATE_MASK;
> >> >> > +		val |= (ULPS_STATE_NORMAL_OPERATION |
> DEVICE_READY);
> >> >> > +		I915_WRITE(MIPI_DEVICE_READY(port), val);
> >> >> > +
> >> >> > +		tmp = I915_READ(MIPI_CTRL(port));
> >> >> > +		tmp &= ~GLK_LP_WAKE;
> >> >> > +		I915_WRITE(MIPI_CTRL(port), tmp);
> >> >> > +	}
> >> >> > +
> >> >> > +	/* Wait for Stop state */
> >> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
> >> >> > +		if (intel_wait_for_register(dev_priv,
> >> >> > +				MIPI_CTRL(port),
> >> >> GLK_DATA_LANE_STOP_STATE,
> >> >> > +				GLK_DATA_LANE_STOP_STATE, 20))
> >> >> > +			DRM_ERROR("Date lane not in STOP
> state\n");
> >> >> > +	}
> >> >> > +
> >> >> > +	/* Wait for AFE LATCH */
> >> >> > +	for_each_dsi_port(port, intel_dsi->ports) {
> >> >> > +		if (intel_wait_for_register(dev_priv,
> >> >> > +				BXT_MIPI_PORT_CTRL(port),
> AFE_LATCHOUT,
> >> >> > +				AFE_LATCHOUT, 20))
> >> >> > +			DRM_ERROR("D-PHY not entering LP-11
> state\n");
> >> >> > +	}
> >> >> > +}
> >> >> > +
> >> >>
> >> >> I'm wondering if these should just be GLK versions of
> >> >> intel_dsi_device_ready and intel_dsi_clear_device_ready. It seems
> >> >> totally wrong that you're doing device ready stuff twice on GLK...
> >> >
> >> > Agree. Don't need to call intel_dsi_device_ready for GLK, as device
> >> > ready is
> >> already done inside enable_io.
> >> > Will do following :
> >> > If(!IS_GEMINILAKE(dev_priv)
> >> > intel_dsi_device_ready(encoder);
> >> >
> >> >>
> >> >>
> >> >> >  static void bxt_dsi_device_ready(struct intel_encoder *encoder)  {
> >> >> >  	struct drm_i915_private *dev_priv =
> >> >> > to_i915(encoder->base.dev);
> >> >> @@
> >> >> > -559,6 +687,9 @@ static void intel_dsi_pre_enable(struct
> >> >> > intel_encoder *encoder,
> >> >> >
> >> >> >  	intel_dsi_prepare(encoder, pipe_config);
> >> >> >
> >> >> > +	if (IS_GEMINILAKE(dev_priv))
> >> >> > +		intel_dsi_enable_mipiio(encoder);
> >> >> > +
> >> >> >  	/* Panel Enable over CRC PMIC */
> >> >> >  	if (intel_dsi->gpio_panel)
> >> >> >  		gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1);
> @@ -
> >> >> 699,6
> >> >> > +830,9 @@ static void intel_dsi_clear_device_ready(struct
> >> >> > +intel_encoder
> >> >> *encoder)
> >> >> >  		usleep_range(2000, 2500);
> >> >> >  	}
> >> >> >
> >> >> > +	if (IS_GEMINILAKE(dev_priv))
> >> >> > +		intel_dsi_disable_mipiio(encoder);
> >> >> > +
> >> >>
> >> >> When you're doing enable/disable of something, they should be
> >> >> called from the corresponding functions in the enable/disable
> >> >> paths. But this is just a general remark, if we conclude that
> >> >> these should be alternative device ready/unready calls instead.
> >> >
> >> > Yes agree, intel_dsi_disable_mipiio should be called from
> >> > intel_dsi_post_disable after intel_dsi_clear_device_ready and
> >> > before
> >> intel_disable_dsi_pll Will do these changes in next series.
> >>
> >> I meant, how about renaming intel_dsi_enable_mipiio to
> >> glk_dsi_device_ready, and making intel_dsi_device_ready call
> >> glk_dsi_device_ready for GLK.
> >>
> >> Then rename intel_dsi_clear_device_ready to
> >> vlv_dsi_clear_device_ready and intel_dsi_disable_mipiio to
> >> glk_dsi_clear_device_ready, and add a new
> >> intel_dsi_clear_device_ready wrapper to call vlv_dsi_clear_device_ready
> for VLV/CHV/BXT, and glk_dsi_clear_device_ready for GLK.
> >>
> >> How does that sound?
> >
> > That's good input. It will align the code to the platforms.
> > For glk_dsi_clear_device_ready, we need to 1. Disable MIPI IO, 2.
> > Enter LP Shouldn't we add 2 separate functions for 1,2 and call them inside
> glk_dsi_clear_device_ready, it will make code readable/modularize as per
> BSPEC??
> 
> Sure, abstractions like that are fine.

Thanks. Will send the patch addressing all these review comments in next series.

> 
> BR,
> Jani.
> 
> >
> >>
> >>
> >> BR,
> >> Jani.
> >>
> >> >>
> >> >> BR,
> >> >> Jani.
> >> >>
> >> >>
> >> >> >  	intel_disable_dsi_pll(encoder);  }
> >> >>
> >> >> --
> >> >> Jani Nikula, Intel Open Source Technology Center
> >>
> >> --
> >> Jani Nikula, Intel Open Source Technology Center
> 
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx