Skip to contentSkip to conversation

drm/i915/chv dsi pll stuff

PATCH Superseded Jani Nikula 3 participants 14 messages May 12, 2015 – May 15, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover drm/i915/chv dsi pll stuff

I picked up Gaurav's chv dsi pll patches, added some prep stuff and rebased.

This is based on http://patchwork.freedesktop.org/patch/49102 which Daniel picked up already but apparently didn't push yet...

BR,
Jani.

Gaurav K Singh (2):
drm/i915: Support for higher DSI clk
drm/i915: Changes required to enable DSI Video Mode on CHT

Jani Nikula (2):
drm/i915/dsi: abstract dsi bpp derivation from pixel format drm/i915/dsi: add support for DSI PLL N1 divisor values

 drivers/gpu/drm/i915/intel_dsi_pll.c | 110 +++++++++++++++++------------------
 1 file changed, 55 insertions(+), 55 deletions(-)
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#1 lore raw

PATCH 1/4 drm/i915/dsi: abstract dsi bpp derivation from pixel format

Nuke three copies of the same switch case.

Hopefully we can switch to a drm generic function later on, but that will require us to swich to enum mipi_dsi_pixel_format first.

---
 drivers/gpu/drm/i915/intel_dsi_pll.c | 67 +++++++++++++-----------------------
 1 file changed, 24 insertions(+), 43 deletions(-)
drivers/gpu/drm/i915/intel_dsi_pll.c +24 -43
index cfd527765156..9ada06ec88e5 100644
@@ -38,6 +38,27 @@
 #define DSI_HFP_PACKET_EXTRA_SIZE	6
 #define DSI_EOTP_PACKET_SIZE		4
 
+static int dsi_pixel_format_bpp(int pixel_format)
+{
+	int bpp;
+
+	switch (pixel_format) {
+	default:
+	case VID_MODE_FORMAT_RGB888:
+	case VID_MODE_FORMAT_RGB666_LOOSE:
+		bpp = 24;
+		break;
+	case VID_MODE_FORMAT_RGB666:
+		bpp = 18;
+		break;
+	case VID_MODE_FORMAT_RGB565:
+		bpp = 16;
+		break;
+	}
+
+	return bpp;
+}
+
 struct dsi_mnp {
 	u32 dsi_pll_ctrl;
 	u32 dsi_pll_div;
@@ -65,19 +86,7 @@ static u32 dsi_rr_formula(const struct drm_display_mode *mode,
 	u32 dsi_bit_clock_hz;
 	u32 dsi_clk;
 
-	switch (pixel_format) {
-	default:
-	case VID_MODE_FORMAT_RGB888:
-	case VID_MODE_FORMAT_RGB666_LOOSE:
-		bpp = 24;
-		break;
-	case VID_MODE_FORMAT_RGB666:
-		bpp = 18;
-		break;
-	case VID_MODE_FORMAT_RGB565:
-		bpp = 16;
-		break;
-	}
+	bpp = dsi_pixel_format_bpp(pixel_format);
 
 	hactive = mode->hdisplay;
 	vactive = mode->vdisplay;
@@ -137,21 +146,7 @@ static u32 dsi_rr_formula(const struct drm_display_mode *mode,
 static u32 dsi_clk_from_pclk(u32 pclk, int pixel_format, int lane_count)
 {
 	u32 dsi_clk_khz;
-	u32 bpp;
-
-	switch (pixel_format) {
-	default:
-	case VID_MODE_FORMAT_RGB888:
-	case VID_MODE_FORMAT_RGB666_LOOSE:
-		bpp = 24;
-		break;
-	case VID_MODE_FORMAT_RGB666:
-		bpp = 18;
-		break;
-	case VID_MODE_FORMAT_RGB565:
-		bpp = 16;
-		break;
-	}
+	u32 bpp = dsi_pixel_format_bpp(pixel_format);
 
 	/* DSI data rate = pixel clock * bits per pixel / lane count
 	   pixel clock is converted from KHz to Hz */
@@ -285,21 +280,7 @@ void vlv_disable_dsi_pll(struct intel_encoder *encoder)
 
 static void assert_bpp_mismatch(int pixel_format, int pipe_bpp)
 {
-	int bpp;
-
-	switch (pixel_format) {
-	default:
-	case VID_MODE_FORMAT_RGB888:
-	case VID_MODE_FORMAT_RGB666_LOOSE:
-		bpp = 24;
-		break;
-	case VID_MODE_FORMAT_RGB666:
-		bpp = 18;
-		break;
-	case VID_MODE_FORMAT_RGB565:
-		bpp = 16;
-		break;
-	}
+	int bpp = dsi_pixel_format_bpp(pixel_format);
 
 	WARN(bpp != pipe_bpp,
 	     "bpp match assertion failure (expected %d, current %d)\n",
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#4 lore raw

PATCH 2/4 drm/i915/dsi: add support for DSI PLL N1 divisor values

In reply to Jani Nikula

Currently DSI PLL N1 is hardcoded off. Make it possible to use it later. This should have no functional changes for now.

---
 drivers/gpu/drm/i915/intel_dsi_pll.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/intel_dsi_pll.c +11 -6
index 9ada06ec88e5..effb561e00a0 100644
@@ -160,7 +160,7 @@ static u32 dsi_clk_from_pclk(u32 pclk, int pixel_format, int lane_count)
 static int dsi_calc_mnp(int target_dsi_clk, struct dsi_mnp *dsi_mnp)
 {
 	unsigned int calc_m = 0, calc_p = 0;
-	unsigned int m, n, p;
+	unsigned int m, n = 1, p;
 	int ref_clk = 25000;
 	int delta = target_dsi_clk;
 	u32 m_seed;
@@ -177,7 +177,7 @@ static int dsi_calc_mnp(int target_dsi_clk, struct dsi_mnp *dsi_mnp)
 			 * Find the optimal m and p divisors with minimal delta
 			 * +/- the required clock
 			 */
-			int calc_dsi_clk = (m * ref_clk) / p;
+			int calc_dsi_clk = (m * ref_clk) / (p * n);
 			int d = abs(target_dsi_clk - calc_dsi_clk);
 			if (d < delta) {
 				delta = d;
@@ -187,10 +187,11 @@ static int dsi_calc_mnp(int target_dsi_clk, struct dsi_mnp *dsi_mnp)
 		}
 	}
 
+	/* register has log2(N1), this works fine for powers of two */
+	n = ffz(~(n));
 	m_seed = lfsr_converts[calc_m - 62];
-	n = 1;
 	dsi_mnp->dsi_pll_ctrl = 1 << (DSI_PLL_P1_POST_DIV_SHIFT + calc_p - 2);
-	dsi_mnp->dsi_pll_div = (n - 1) << DSI_PLL_N1_DIV_SHIFT |
+	dsi_mnp->dsi_pll_div = n << DSI_PLL_N1_DIV_SHIFT |
 		m_seed << DSI_PLL_M1_DIV_SHIFT;
 
 	return 0;
@@ -293,7 +294,7 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
 	u32 dsi_clock, pclk;
 	u32 pll_ctl, pll_div;
-	u32 m = 0, p = 0;
+	u32 m = 0, p = 0, n;
 	int refclk = 25000;
 	int i;
 
@@ -308,6 +309,10 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
 	pll_ctl &= DSI_PLL_P1_POST_DIV_MASK;
 	pll_ctl = pll_ctl >> (DSI_PLL_P1_POST_DIV_SHIFT - 2);
 
+	/* N1 divisor */
+	n = (pll_div & DSI_PLL_N1_DIV_MASK) >> DSI_PLL_N1_DIV_SHIFT;
+	n = 1 << n; /* register has log2(N1) */
+
 	/* mask out the other bits and extract the M1 divisor */
 	pll_div &= DSI_PLL_M1_DIV_MASK;
 	pll_div = pll_div >> DSI_PLL_M1_DIV_SHIFT;
@@ -335,7 +340,7 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
 
 	m = i + 62;
 
-	dsi_clock = (m * refclk) / p;
+	dsi_clock = (m * refclk) / (p * n);
 
 	/* pixel_format and pipe_bpp should agree */
 	assert_bpp_mismatch(intel_dsi->pixel_format, pipe_bpp);
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#9 lore raw

PATCH 3/4 drm/i915: Support for higher DSI clk

In reply to Jani Nikula

From: Gaurav K Singh <[email protected]>

For MIPI panels requiring higher DSI clk, values needs to be added in lfsr_converts table for getting the correct values of pll ctrl and dividor values which gets programmed in cck regs, otherwise DSI PLL does not get locked leading to no display on the MIPI panel.

---
 drivers/gpu/drm/i915/intel_dsi_pll.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/intel_dsi_pll.c +2 -2
index effb561e00a0..d1aefc7a0629 100644
@@ -67,8 +67,8 @@ struct dsi_mnp {
 static const u32 lfsr_converts[] = {
 	426, 469, 234, 373, 442, 221, 110, 311, 411,		/* 62 - 70 */
 	461, 486, 243, 377, 188, 350, 175, 343, 427, 213,	/* 71 - 80 */
-	106, 53, 282, 397, 354, 227, 113, 56, 284, 142,		/* 81 - 90 */
-	71, 35							/* 91 - 92 */
+	106, 53, 282, 397, 454, 227, 113, 56, 284, 142,		/* 81 - 90 */
+	71, 35, 273, 136, 324, 418, 465, 488, 500, 506		/* 91 - 100 */
 };
 
 #ifdef DSI_CLK_FROM_RR
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#11 lore raw

PATCH 4/4 drm/i915: Changes required to enable DSI Video Mode on CHT

In reply to Jani Nikula

From: Gaurav K Singh <[email protected]>

On CHT, changes are required for calculating the correct m,n & p with minimal error +/- for the required DSI clock, so that the correct dividor & ctrl values are written in cck regs for DSI. This patch has been tested on CHT RVP with 1200 x 1920 panel.

v2 by Jani, rebased on earlier refactoring, original at [1].

[1] http://mid.gmane.org/[email protected]

---
 drivers/gpu/drm/i915/intel_dsi_pll.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/intel_dsi_pll.c +20 -6
index d1aefc7a0629..686802b49b83 100644
@@ -157,11 +157,13 @@ static u32 dsi_clk_from_pclk(u32 pclk, int pixel_format, int lane_count)
 
 #endif
 
-static int dsi_calc_mnp(int target_dsi_clk, struct dsi_mnp *dsi_mnp)
+static int dsi_calc_mnp(struct drm_i915_private *dev_priv,
+			struct dsi_mnp *dsi_mnp, int target_dsi_clk)
 {
 	unsigned int calc_m = 0, calc_p = 0;
-	unsigned int m, n = 1, p;
-	int ref_clk = 25000;
+	unsigned int m_min, m_max, p_min = 2, p_max = 6;
+	unsigned int m, n, p;
+	int ref_clk;
 	int delta = target_dsi_clk;
 	u32 m_seed;
 
@@ -171,8 +173,20 @@ static int dsi_calc_mnp(int target_dsi_clk, struct dsi_mnp *dsi_mnp)
 		return -ECHRNG;
 	}
 
-	for (m = 62; m <= 92 && delta; m++) {
-		for (p = 2; p <= 6 && delta; p++) {
+	if (IS_CHERRYVIEW(dev_priv)) {
+		ref_clk = 100000;
+		n = 4;
+		m_min = 70;
+		m_max = 96;
+	} else {
+		ref_clk = 25000;
+		n = 1;
+		m_min = 62;
+		m_max = 92;
+	}
+
+	for (m = m_min; m <= m_max && delta; m++) {
+		for (p = p_min; p <= p_max && delta; p++) {
 			/*
 			 * Find the optimal m and p divisors with minimal delta
 			 * +/- the required clock
@@ -212,7 +226,7 @@ static void vlv_configure_dsi_pll(struct intel_encoder *encoder)
 	dsi_clk = dsi_clk_from_pclk(intel_dsi->pclk, intel_dsi->pixel_format,
 				    intel_dsi->lane_count);
 
-	ret = dsi_calc_mnp(dsi_clk, &dsi_mnp);
+	ret = dsi_calc_mnp(dev_priv, &dsi_mnp, dsi_clk);
 	if (ret) {
 		DRM_DEBUG_KMS("dsi_calc_mnp failed\n");
 		return;
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Reviewed-by on v1 1/4
#2 lore raw

Re: [PATCH 1/4] drm/i915/dsi: abstract dsi bpp derivation from pixel format

In reply to Jani Nikula
Quoted 28 lines · Jani Nikula

Nuke three copies of the same switch case.

Hopefully we can switch to a drm generic function later on, but that will require us to swich to enum mipi_dsi_pixel_format first.

---
 drivers/gpu/drm/i915/intel_dsi_pll.c | 67 +++++++++++++-----------------------
 1 file changed, 24 insertions(+), 43 deletions(-)
drivers/gpu/drm/i915/intel_dsi_pll.c +10 -0
index cfd527765156..9ada06ec88e5 100644
@@ -38,6 +38,27 @@
 #define DSI_HFP_PACKET_EXTRA_SIZE	6
 #define DSI_EOTP_PACKET_SIZE		4
 
+static int dsi_pixel_format_bpp(int pixel_format)
+{
+	int bpp;
+
+	switch (pixel_format) {
+	default:
+	case VID_MODE_FORMAT_RGB888:
+	case VID_MODE_FORMAT_RGB666_LOOSE:
+		bpp = 24;
+		break;

drivers/gpu/drm/i915/intel_dsi_pll.c line 60

Quoted 10 lines
drivers/gpu/drm/i915/intel_dsi_pll.c +10 -0
+	case VID_MODE_FORMAT_RGB666:
+		bpp = 18;
+		break;
+	case VID_MODE_FORMAT_RGB565:
+		bpp = 16;
+		break;
+	}
+
+	return bpp;
+}

Optional bikeshed: Could return directly from the switch cases and avoid the local variable.

But anyway:

Quoted 73 lines
drivers/gpu/drm/i915/intel_dsi_pll.c +4 -43
+
 struct dsi_mnp {
 	u32 dsi_pll_ctrl;
 	u32 dsi_pll_div;
@@ -65,19 +86,7 @@ static u32 dsi_rr_formula(const struct drm_display_mode *mode,
 	u32 dsi_bit_clock_hz;
 	u32 dsi_clk;
 
-	switch (pixel_format) {
-	default:
-	case VID_MODE_FORMAT_RGB888:
-	case VID_MODE_FORMAT_RGB666_LOOSE:
-		bpp = 24;
-		break;
-	case VID_MODE_FORMAT_RGB666:
-		bpp = 18;
-		break;
-	case VID_MODE_FORMAT_RGB565:
-		bpp = 16;
-		break;
-	}
+	bpp = dsi_pixel_format_bpp(pixel_format);
 
 	hactive = mode->hdisplay;
 	vactive = mode->vdisplay;
@@ -137,21 +146,7 @@ static u32 dsi_rr_formula(const struct drm_display_mode *mode,
 static u32 dsi_clk_from_pclk(u32 pclk, int pixel_format, int lane_count)
 {
 	u32 dsi_clk_khz;
-	u32 bpp;
-
-	switch (pixel_format) {
-	default:
-	case VID_MODE_FORMAT_RGB888:
-	case VID_MODE_FORMAT_RGB666_LOOSE:
-		bpp = 24;
-		break;
-	case VID_MODE_FORMAT_RGB666:
-		bpp = 18;
-		break;
-	case VID_MODE_FORMAT_RGB565:
-		bpp = 16;
-		break;
-	}
+	u32 bpp = dsi_pixel_format_bpp(pixel_format);
 
 	/* DSI data rate = pixel clock * bits per pixel / lane count
 	   pixel clock is converted from KHz to Hz */
@@ -285,21 +280,7 @@ void vlv_disable_dsi_pll(struct intel_encoder *encoder)
 
 static void assert_bpp_mismatch(int pixel_format, int pipe_bpp)
 {
-	int bpp;
-
-	switch (pixel_format) {
-	default:
-	case VID_MODE_FORMAT_RGB888:
-	case VID_MODE_FORMAT_RGB666_LOOSE:
-		bpp = 24;
-		break;
-	case VID_MODE_FORMAT_RGB666:
-		bpp = 18;
-		break;
-	case VID_MODE_FORMAT_RGB565:
-		bpp = 16;
-		break;
-	}
+	int bpp = dsi_pixel_format_bpp(pixel_format);
 
 	WARN(bpp != pipe_bpp,
 	     "bpp match assertion failure (expected %d, current %d)\n",
Signature · 1 line
2.1.4
Signature · 6 lines
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#5 lore raw

Re: [PATCH 2/4] drm/i915/dsi: add support for DSI PLL N1 divisor values

In reply to Jani Nikula
Quoted 31 lines · Jani Nikula

Currently DSI PLL N1 is hardcoded off. Make it possible to use it later. This should have no functional changes for now.

---
 drivers/gpu/drm/i915/intel_dsi_pll.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/intel_dsi_pll.c +2 -2
index 9ada06ec88e5..effb561e00a0 100644
@@ -160,7 +160,7 @@ static u32 dsi_clk_from_pclk(u32 pclk, int pixel_format, int lane_count)
 static int dsi_calc_mnp(int target_dsi_clk, struct dsi_mnp *dsi_mnp)
 {
 	unsigned int calc_m = 0, calc_p = 0;
-	unsigned int m, n, p;
+	unsigned int m, n = 1, p;
 	int ref_clk = 25000;
 	int delta = target_dsi_clk;
 	u32 m_seed;
@@ -177,7 +177,7 @@ static int dsi_calc_mnp(int target_dsi_clk, struct dsi_mnp *dsi_mnp)
 			 * Find the optimal m and p divisors with minimal delta
 			 * +/- the required clock
 			 */
-			int calc_dsi_clk = (m * ref_clk) / p;
+			int calc_dsi_clk = (m * ref_clk) / (p * n);
 			int d = abs(target_dsi_clk - calc_dsi_clk);
 			if (d < delta) {
 				delta = d;

drivers/gpu/drm/i915/intel_dsi_pll.c static int dsi_calc_mnp(int target_dsi_clk, struct dsi_mnp *dsi_mnp) · line 191

Quoted 5 lines
drivers/gpu/drm/i915/intel_dsi_pll.c +2 -0
@@ -187,10 +187,11 @@ static int dsi_calc_mnp(int target_dsi_clk, struct dsi_mnp *dsi_mnp)
 		}
 	}
 
+	/* register has log2(N1), this works fine for powers of two */
+	n = ffz(~(n));

I think 'ffs() - 1' is what's been used elsewhere for this. Although in this case we could avoid it entirely by keeping n as the log2 value always. Either way would be fine by me.

Quoted 39 lines
drivers/gpu/drm/i915/intel_dsi_pll.c +7 -4
 	m_seed = lfsr_converts[calc_m - 62];
-	n = 1;
 	dsi_mnp->dsi_pll_ctrl = 1 << (DSI_PLL_P1_POST_DIV_SHIFT + calc_p - 2);
-	dsi_mnp->dsi_pll_div = (n - 1) << DSI_PLL_N1_DIV_SHIFT |
+	dsi_mnp->dsi_pll_div = n << DSI_PLL_N1_DIV_SHIFT |
 		m_seed << DSI_PLL_M1_DIV_SHIFT;
 
 	return 0;
@@ -293,7 +294,7 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
 	u32 dsi_clock, pclk;
 	u32 pll_ctl, pll_div;
-	u32 m = 0, p = 0;
+	u32 m = 0, p = 0, n;
 	int refclk = 25000;
 	int i;
 
@@ -308,6 +309,10 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
 	pll_ctl &= DSI_PLL_P1_POST_DIV_MASK;
 	pll_ctl = pll_ctl >> (DSI_PLL_P1_POST_DIV_SHIFT - 2);
 
+	/* N1 divisor */
+	n = (pll_div & DSI_PLL_N1_DIV_MASK) >> DSI_PLL_N1_DIV_SHIFT;
+	n = 1 << n; /* register has log2(N1) */
+
 	/* mask out the other bits and extract the M1 divisor */
 	pll_div &= DSI_PLL_M1_DIV_MASK;
 	pll_div = pll_div >> DSI_PLL_M1_DIV_SHIFT;
@@ -335,7 +340,7 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
 
 	m = i + 62;
 
-	dsi_clock = (m * refclk) / p;
+	dsi_clock = (m * refclk) / (p * n);
 
 	/* pixel_format and pipe_bpp should agree */
 	assert_bpp_mismatch(intel_dsi->pixel_format, pipe_bpp);
Signature · 1 line
2.1.4
Signature · 6 lines
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Reviewed-by on v1 4/4
#12 lore raw

Re: [PATCH 4/4] drm/i915: Changes required to enable DSI Video Mode on CHT

In reply to Jani Nikula
Quoted 46 lines · Jani Nikula

From: Gaurav K Singh <[email protected]>

On CHT, changes are required for calculating the correct m,n & p with minimal error +/- for the required DSI clock, so that the correct dividor & ctrl values are written in cck regs for DSI. This patch has been tested on CHT RVP with 1200 x 1920 panel.

v2 by Jani, rebased on earlier refactoring, original at [1].

[1] http://mid.gmane.org/[email protected]

---
 drivers/gpu/drm/i915/intel_dsi_pll.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/intel_dsi_pll.c +6 -5
index d1aefc7a0629..686802b49b83 100644
@@ -157,11 +157,13 @@ static u32 dsi_clk_from_pclk(u32 pclk, int pixel_format, int lane_count)
 
 #endif
 
-static int dsi_calc_mnp(int target_dsi_clk, struct dsi_mnp *dsi_mnp)
+static int dsi_calc_mnp(struct drm_i915_private *dev_priv,
+			struct dsi_mnp *dsi_mnp, int target_dsi_clk)
 {
 	unsigned int calc_m = 0, calc_p = 0;
-	unsigned int m, n = 1, p;
-	int ref_clk = 25000;
+	unsigned int m_min, m_max, p_min = 2, p_max = 6;
+	unsigned int m, n, p;
+	int ref_clk;
 	int delta = target_dsi_clk;
 	u32 m_seed;
 
@@ -171,8 +173,20 @@ static int dsi_calc_mnp(int target_dsi_clk, struct dsi_mnp *dsi_mnp)
 		return -ECHRNG;
 	}
 
-	for (m = 62; m <= 92 && delta; m++) {
-		for (p = 2; p <= 6 && delta; p++) {
+	if (IS_CHERRYVIEW(dev_priv)) {

drivers/gpu/drm/i915/intel_dsi_pll.c static int dsi_calc_mnp(int target_dsi_clk, struct dsi_mnp *dsi_mnp) · line 186

Quoted 10 lines
drivers/gpu/drm/i915/intel_dsi_pll.c +10 -0
+		ref_clk = 100000;
+		n = 4;
+		m_min = 70;
+		m_max = 96;
+	} else {
+		ref_clk = 25000;
+		n = 1;
+		m_min = 62;
+		m_max = 92;
+	}

About the only thing I can actually verify from the specs is the refclk change. As for the rest, well, it looks sane enough.

Quoted 17 lines
drivers/gpu/drm/i915/intel_dsi_pll.c +4 -1
+
+	for (m = m_min; m <= m_max && delta; m++) {
+		for (p = p_min; p <= p_max && delta; p++) {
 			/*
 			 * Find the optimal m and p divisors with minimal delta
 			 * +/- the required clock
@@ -212,7 +226,7 @@ static void vlv_configure_dsi_pll(struct intel_encoder *encoder)
 	dsi_clk = dsi_clk_from_pclk(intel_dsi->pclk, intel_dsi->pixel_format,
 				    intel_dsi->lane_count);
 
-	ret = dsi_calc_mnp(dsi_clk, &dsi_mnp);
+	ret = dsi_calc_mnp(dev_priv, &dsi_mnp, dsi_clk);
 	if (ret) {
 		DRM_DEBUG_KMS("dsi_calc_mnp failed\n");
 		return;
Signature · 1 line
2.1.4
Signature · 6 lines
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Reviewed-by on v1 3/4
#10 lore raw

Re: [PATCH 3/4] drm/i915: Support for higher DSI clk

In reply to Jani Nikula
Quoted 19 lines · Jani Nikula

From: Gaurav K Singh <[email protected]>

For MIPI panels requiring higher DSI clk, values needs to be added in lfsr_converts table for getting the correct values of pll ctrl and dividor values which gets programmed in cck regs, otherwise DSI PLL does not get locked leading to no display on the MIPI panel.

---
 drivers/gpu/drm/i915/intel_dsi_pll.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/intel_dsi_pll.c +0 -0
index effb561e00a0..d1aefc7a0629 100644

drivers/gpu/drm/i915/intel_dsi_pll.c struct dsi_mnp { · line 71

Quoted 7 lines
drivers/gpu/drm/i915/intel_dsi_pll.c +2 -2
@@ -67,8 +67,8 @@ struct dsi_mnp {
 static const u32 lfsr_converts[] = {
 	426, 469, 234, 373, 442, 221, 110, 311, 411,		/* 62 - 70 */
 	461, 486, 243, 377, 188, 350, 175, 343, 427, 213,	/* 71 - 80 */
-	106, 53, 282, 397, 354, 227, 113, 56, 284, 142,		/* 81 - 90 */
-	71, 35							/* 91 - 92 */
+	106, 53, 282, 397, 454, 227, 113, 56, 284, 142,		/* 81 - 90 */
+	71, 35, 273, 136, 324, 418, 465, 488, 500, 506		/* 91 - 100 */

I've never seen any details about this lfsr in any docs. However this is actually a subset of mdfld_m_converts[] in gma500, so based on that:

Quoted 5 lines
drivers/gpu/drm/i915/intel_dsi_pll.c +0 -0
 };
 
 #ifdef DSI_CLK_FROM_RR
Signature · 1 line
2.1.4
Signature · 6 lines
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#3 lore raw

Re: [PATCH 1/4] drm/i915/dsi: abstract dsi bpp derivation from pixel format

In reply to Ville Syrjälä
Quoted 34 lines · Ville Syrjälä
Quoted 33 lines · Jani Nikula

Nuke three copies of the same switch case.

Hopefully we can switch to a drm generic function later on, but that will require us to swich to enum mipi_dsi_pixel_format first.

---
 drivers/gpu/drm/i915/intel_dsi_pll.c | 67 +++++++++++++-----------------------
 1 file changed, 24 insertions(+), 43 deletions(-)
drivers/gpu/drm/i915/intel_dsi_pll.c +15 -0
index cfd527765156..9ada06ec88e5 100644
@@ -38,6 +38,27 @@
 #define DSI_HFP_PACKET_EXTRA_SIZE	6
 #define DSI_EOTP_PACKET_SIZE		4
 
+static int dsi_pixel_format_bpp(int pixel_format)
+{
+	int bpp;
+
+	switch (pixel_format) {
+	default:
+	case VID_MODE_FORMAT_RGB888:
+	case VID_MODE_FORMAT_RGB666_LOOSE:
+		bpp = 24;
+		break;
+	case VID_MODE_FORMAT_RGB666:
+		bpp = 18;
+		break;
+	case VID_MODE_FORMAT_RGB565:
+		bpp = 16;
Quoted 8 lines

drivers/gpu/drm/i915/intel_dsi_pll.c line 60

Quoted 5 lines
drivers/gpu/drm/i915/intel_dsi_pll.c +5 -0
+		break;
+	}
+
+	return bpp;
+}

Optional bikeshed: Could return directly from the switch cases and avoid the local variable.

The compiler appears to automatically inline the function (*and* two functions it is called from), so does not make a difference.

Thanks for the review.

BR,
Jani.

Quoted 81 lines

But anyway:

Quoted 73 lines
drivers/gpu/drm/i915/intel_dsi_pll.c +4 -43
+
 struct dsi_mnp {
 	u32 dsi_pll_ctrl;
 	u32 dsi_pll_div;
@@ -65,19 +86,7 @@ static u32 dsi_rr_formula(const struct drm_display_mode *mode,
 	u32 dsi_bit_clock_hz;
 	u32 dsi_clk;
 
-	switch (pixel_format) {
-	default:
-	case VID_MODE_FORMAT_RGB888:
-	case VID_MODE_FORMAT_RGB666_LOOSE:
-		bpp = 24;
-		break;
-	case VID_MODE_FORMAT_RGB666:
-		bpp = 18;
-		break;
-	case VID_MODE_FORMAT_RGB565:
-		bpp = 16;
-		break;
-	}
+	bpp = dsi_pixel_format_bpp(pixel_format);
 
 	hactive = mode->hdisplay;
 	vactive = mode->vdisplay;
@@ -137,21 +146,7 @@ static u32 dsi_rr_formula(const struct drm_display_mode *mode,
 static u32 dsi_clk_from_pclk(u32 pclk, int pixel_format, int lane_count)
 {
 	u32 dsi_clk_khz;
-	u32 bpp;
-
-	switch (pixel_format) {
-	default:
-	case VID_MODE_FORMAT_RGB888:
-	case VID_MODE_FORMAT_RGB666_LOOSE:
-		bpp = 24;
-		break;
-	case VID_MODE_FORMAT_RGB666:
-		bpp = 18;
-		break;
-	case VID_MODE_FORMAT_RGB565:
-		bpp = 16;
-		break;
-	}
+	u32 bpp = dsi_pixel_format_bpp(pixel_format);
 
 	/* DSI data rate = pixel clock * bits per pixel / lane count
 	   pixel clock is converted from KHz to Hz */
@@ -285,21 +280,7 @@ void vlv_disable_dsi_pll(struct intel_encoder *encoder)
 
 static void assert_bpp_mismatch(int pixel_format, int pipe_bpp)
 {
-	int bpp;
-
-	switch (pixel_format) {
-	default:
-	case VID_MODE_FORMAT_RGB888:
-	case VID_MODE_FORMAT_RGB666_LOOSE:
-		bpp = 24;
-		break;
-	case VID_MODE_FORMAT_RGB666:
-		bpp = 18;
-		break;
-	case VID_MODE_FORMAT_RGB565:
-		bpp = 16;
-		break;
-	}
+	int bpp = dsi_pixel_format_bpp(pixel_format);
 
 	WARN(bpp != pipe_bpp,
 	     "bpp match assertion failure (expected %d, current %d)\n",
Signature · 1 line
2.1.4
Signature · 2 lines
Ville Syrjälä
Intel OTC
Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#6 lore raw

PATCH v2 1/1 drm/i915/dsi: add support for DSI PLL N1 divisor values

In reply to Ville Syrjälä

Currently DSI PLL N1 is hardcoded off. Make it possible to use it later. This should have no functional changes for now.

v2: s/ffz(~(n))/ffs(n) - 1/ (Ville)

---
 drivers/gpu/drm/i915/intel_dsi_pll.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/intel_dsi_pll.c +11 -6
index 9ada06ec88e5..3a6c2335991c 100644
@@ -160,7 +160,7 @@ static u32 dsi_clk_from_pclk(u32 pclk, int pixel_format, int lane_count)
 static int dsi_calc_mnp(int target_dsi_clk, struct dsi_mnp *dsi_mnp)
 {
 	unsigned int calc_m = 0, calc_p = 0;
-	unsigned int m, n, p;
+	unsigned int m, n = 1, p;
 	int ref_clk = 25000;
 	int delta = target_dsi_clk;
 	u32 m_seed;
@@ -177,7 +177,7 @@ static int dsi_calc_mnp(int target_dsi_clk, struct dsi_mnp *dsi_mnp)
 			 * Find the optimal m and p divisors with minimal delta
 			 * +/- the required clock
 			 */
-			int calc_dsi_clk = (m * ref_clk) / p;
+			int calc_dsi_clk = (m * ref_clk) / (p * n);
 			int d = abs(target_dsi_clk - calc_dsi_clk);
 			if (d < delta) {
 				delta = d;
@@ -187,10 +187,11 @@ static int dsi_calc_mnp(int target_dsi_clk, struct dsi_mnp *dsi_mnp)
 		}
 	}
 
+	/* register has log2(N1), this works fine for powers of two */
+	n = ffs(n) - 1;
 	m_seed = lfsr_converts[calc_m - 62];
-	n = 1;
 	dsi_mnp->dsi_pll_ctrl = 1 << (DSI_PLL_P1_POST_DIV_SHIFT + calc_p - 2);
-	dsi_mnp->dsi_pll_div = (n - 1) << DSI_PLL_N1_DIV_SHIFT |
+	dsi_mnp->dsi_pll_div = n << DSI_PLL_N1_DIV_SHIFT |
 		m_seed << DSI_PLL_M1_DIV_SHIFT;
 
 	return 0;
@@ -293,7 +294,7 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
 	u32 dsi_clock, pclk;
 	u32 pll_ctl, pll_div;
-	u32 m = 0, p = 0;
+	u32 m = 0, p = 0, n;
 	int refclk = 25000;
 	int i;
 
@@ -308,6 +309,10 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
 	pll_ctl &= DSI_PLL_P1_POST_DIV_MASK;
 	pll_ctl = pll_ctl >> (DSI_PLL_P1_POST_DIV_SHIFT - 2);
 
+	/* N1 divisor */
+	n = (pll_div & DSI_PLL_N1_DIV_MASK) >> DSI_PLL_N1_DIV_SHIFT;
+	n = 1 << n; /* register has log2(N1) */
+
 	/* mask out the other bits and extract the M1 divisor */
 	pll_div &= DSI_PLL_M1_DIV_MASK;
 	pll_div = pll_div >> DSI_PLL_M1_DIV_SHIFT;
@@ -335,7 +340,7 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
 
 	m = i + 62;
 
-	dsi_clock = (m * refclk) / p;
+	dsi_clock = (m * refclk) / (p * n);
 
 	/* pixel_format and pipe_bpp should agree */
 	assert_bpp_mismatch(intel_dsi->pixel_format, pipe_bpp);
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Reviewed-by on v2 1/1
#7 lore raw

Re: [PATCH v2] drm/i915/dsi: add support for DSI PLL N1 divisor values

Quoted 6 lines · Jani Nikula

Currently DSI PLL N1 is hardcoded off. Make it possible to use it later. This should have no functional changes for now.

v2: s/ffz(~(n))/ffs(n) - 1/ (Ville)

Quoted 71 lines
---
 drivers/gpu/drm/i915/intel_dsi_pll.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/intel_dsi_pll.c +11 -6
index 9ada06ec88e5..3a6c2335991c 100644
@@ -160,7 +160,7 @@ static u32 dsi_clk_from_pclk(u32 pclk, int pixel_format, int lane_count)
 static int dsi_calc_mnp(int target_dsi_clk, struct dsi_mnp *dsi_mnp)
 {
 	unsigned int calc_m = 0, calc_p = 0;
-	unsigned int m, n, p;
+	unsigned int m, n = 1, p;
 	int ref_clk = 25000;
 	int delta = target_dsi_clk;
 	u32 m_seed;
@@ -177,7 +177,7 @@ static int dsi_calc_mnp(int target_dsi_clk, struct dsi_mnp *dsi_mnp)
 			 * Find the optimal m and p divisors with minimal delta
 			 * +/- the required clock
 			 */
-			int calc_dsi_clk = (m * ref_clk) / p;
+			int calc_dsi_clk = (m * ref_clk) / (p * n);
 			int d = abs(target_dsi_clk - calc_dsi_clk);
 			if (d < delta) {
 				delta = d;
@@ -187,10 +187,11 @@ static int dsi_calc_mnp(int target_dsi_clk, struct dsi_mnp *dsi_mnp)
 		}
 	}
 
+	/* register has log2(N1), this works fine for powers of two */
+	n = ffs(n) - 1;
 	m_seed = lfsr_converts[calc_m - 62];
-	n = 1;
 	dsi_mnp->dsi_pll_ctrl = 1 << (DSI_PLL_P1_POST_DIV_SHIFT + calc_p - 2);
-	dsi_mnp->dsi_pll_div = (n - 1) << DSI_PLL_N1_DIV_SHIFT |
+	dsi_mnp->dsi_pll_div = n << DSI_PLL_N1_DIV_SHIFT |
 		m_seed << DSI_PLL_M1_DIV_SHIFT;
 
 	return 0;
@@ -293,7 +294,7 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
 	u32 dsi_clock, pclk;
 	u32 pll_ctl, pll_div;
-	u32 m = 0, p = 0;
+	u32 m = 0, p = 0, n;
 	int refclk = 25000;
 	int i;
 
@@ -308,6 +309,10 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
 	pll_ctl &= DSI_PLL_P1_POST_DIV_MASK;
 	pll_ctl = pll_ctl >> (DSI_PLL_P1_POST_DIV_SHIFT - 2);
 
+	/* N1 divisor */
+	n = (pll_div & DSI_PLL_N1_DIV_MASK) >> DSI_PLL_N1_DIV_SHIFT;
+	n = 1 << n; /* register has log2(N1) */
+
 	/* mask out the other bits and extract the M1 divisor */
 	pll_div &= DSI_PLL_M1_DIV_MASK;
 	pll_div = pll_div >> DSI_PLL_M1_DIV_SHIFT;
@@ -335,7 +340,7 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
 
 	m = i + 62;
 
-	dsi_clock = (m * refclk) / p;
+	dsi_clock = (m * refclk) / (p * n);
 
 	/* pixel_format and pipe_bpp should agree */
 	assert_bpp_mismatch(intel_dsi->pixel_format, pipe_bpp);
Signature · 1 line
2.1.4
Signature · 6 lines
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Tested-by on v1 4/4
#13 lore raw

Re: [PATCH 4/4] drm/i915: Changes required to enable DSI Video Mode on CHT

In reply to Jani Nikula

Task id: 6391 -------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied

PNV                 -3              272/272              269/272
ILK                 -1              302/302              301/302
SNB                 -1              315/315              314/315
IVB                                  343/343              343/343
BYT                                  287/287              287/287
BDW                                  317/317              317/317
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*PNV  igt@gem_tiled_pread_pwrite      PASS(3)      FAIL(1)
*PNV  igt@gem_userptr_blits@coherency-sync      PASS(3)      CRASH(1)
*PNV  igt@gem_userptr_blits@coherency-unsync      PASS(3)      CRASH(1)
*ILK  igt@kms_flip@flip-vs-dpms-interruptible      PASS(2)      DMESG_WARN(1)

(dmesg patch applied)drm:intel_pch_fifo_underrun_irq_handler[i915]]*ERROR*PCH_transcoder_A_FIFO_underrun@PCH transcoder A FIFO underrun
SNB igt@pm_rpm@dpms-mode-unset-non-lpsp DMESG_WARN(7)PASS(1) DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_uncore.c:#assert_device_not_suspended[i915]()@WARNING:.* at .* assert_device_not_suspended+0x
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Tested-by on v2 1/1
#8 lore raw

Re: [PATCH v2] drm/i915/dsi: add support for DSI PLL N1 divisor values

In reply to Jani Nikula

Task id: 6395 -------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied

PNV                                  276/276              276/276
ILK                                  302/302              302/302
SNB                 -1              315/315              314/315
IVB                                  343/343              343/343
BYT                                  287/287              287/287

BDW -1 321/321 320/321
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*SNB igt@pm_rpm@dpms-mode-unset-non-lpsp PASS(1) DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_uncore.c:#assert_device_not_suspended[i915]()@WARNING:.* at .* assert_device_not_suspended+0x
*BDW igt@gem_exec_params@secure-non-master PASS(1) DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_display.c:#assert_plane[i915]()@WARNING:.* at .* assert_plane assertion_failure@assertion failure
WARNING:at_drivers/gpu/drm/drm_irq.c:#drm_wait_one_vblank[drm]()@WARNING:.* at .* drm_wait_one_vblank+0x
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx