Skip to contentSkip to conversation

drm/i915/dsi: improve dphy param limits logging

PATCH In review Jani Nikula 3 participants 10 messages Apr 19, 2018

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

PATCH 1/2 drm/i915/dsi: improve dphy param limits logging

Move the limit checks near the calculations for each field, and actually log the values that exceed limits.

Cc Ville Syrjälä <[email protected]>
---
 drivers/gpu/drm/i915/intel_dsi_vbt.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/intel_dsi_vbt.c +18 -16
index 91c07b0c8db9..4d6ffa7b3e7b 100644
@@ -647,6 +647,11 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	/* prepare count */
 	prepare_cnt = DIV_ROUND_UP(ths_prepare_ns * ui_den, ui_num * mul);
 
+	if (prepare_cnt > PREPARE_CNT_MAX) {
+		DRM_DEBUG_KMS("prepare count too high %u\n", prepare_cnt);
+		prepare_cnt = PREPARE_CNT_MAX;
+	}
+
 	/* exit zero count */
 	exit_zero_cnt = DIV_ROUND_UP(
 				(ths_prepare_hszero - ths_prepare_ns) * ui_den,
@@ -662,32 +667,29 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	if (exit_zero_cnt < (55 * ui_den / ui_num) && (55 * ui_den) % ui_num)
 		exit_zero_cnt += 1;
 
+	if (exit_zero_cnt > EXIT_ZERO_CNT_MAX) {
+		DRM_DEBUG_KMS("exit zero count too high %u\n", exit_zero_cnt);
+		exit_zero_cnt = EXIT_ZERO_CNT_MAX;
+	}
+
 	/* clk zero count */
 	clk_zero_cnt = DIV_ROUND_UP(
 				(tclk_prepare_clkzero -	ths_prepare_ns)
 				* ui_den, ui_num * mul);
 
+	if (clk_zero_cnt > CLK_ZERO_CNT_MAX) {
+		DRM_DEBUG_KMS("clock zero count too high %u\n", clk_zero_cnt);
+		clk_zero_cnt = CLK_ZERO_CNT_MAX;
+	}
+
 	/* trail count */
 	tclk_trail_ns = max(mipi_config->tclk_trail, mipi_config->ths_trail);
 	trail_cnt = DIV_ROUND_UP(tclk_trail_ns * ui_den, ui_num * mul);
 
-	if (prepare_cnt > PREPARE_CNT_MAX ||
-		exit_zero_cnt > EXIT_ZERO_CNT_MAX ||
-		clk_zero_cnt > CLK_ZERO_CNT_MAX ||
-		trail_cnt > TRAIL_CNT_MAX)
-		DRM_DEBUG_DRIVER("Values crossing maximum limits, restricting to max values\n");
-
-	if (prepare_cnt > PREPARE_CNT_MAX)
-		prepare_cnt = PREPARE_CNT_MAX;
-
-	if (exit_zero_cnt > EXIT_ZERO_CNT_MAX)
-		exit_zero_cnt = EXIT_ZERO_CNT_MAX;
-
-	if (clk_zero_cnt > CLK_ZERO_CNT_MAX)
-		clk_zero_cnt = CLK_ZERO_CNT_MAX;
-
-	if (trail_cnt > TRAIL_CNT_MAX)
+	if (trail_cnt > TRAIL_CNT_MAX) {
+		DRM_DEBUG_KMS("trail count too high %u\n", trail_cnt);
 		trail_cnt = TRAIL_CNT_MAX;
+	}
 
 	/* B080 */
 	intel_dsi->dphy_reg = exit_zero_cnt << 24 | trail_cnt << 16 |
Signature · 6 lines
2.11.0

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

PATCH 2/2 drm/i915/dsi: fix dphy param field widths and range checks for chv+

Current CHV and BXT bspec says the dphy param register has four 8-bit fields instead of the smaller VLV field widths. CHV bspec mentions the register has changed since K0, but there's no indication of what exactly changed. Lacking further details, change the field widths for all CHV and later.

Define the max values based on the platform. Also define them based on the register definitions instead of duplicating information.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106130

---
 drivers/gpu/drm/i915/i915_reg.h      | 11 +++++++----
 drivers/gpu/drm/i915/intel_dsi_vbt.c | 31 +++++++++++++++++++------------
 2 files changed, 26 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/i915_reg.h +7 -4
index fb106026a1f4..f4435a13b757 100644
@@ -9547,14 +9547,17 @@ enum skl_power_gate {
 #define _MIPIA_DPHY_PARAM		(dev_priv->mipi_mmio_base + 0xb080)
 #define _MIPIC_DPHY_PARAM		(dev_priv->mipi_mmio_base + 0xb880)
 #define MIPI_DPHY_PARAM(port)		_MMIO_MIPI(port, _MIPIA_DPHY_PARAM, _MIPIC_DPHY_PARAM)
-#define  EXIT_ZERO_COUNT_SHIFT				24
 #define  EXIT_ZERO_COUNT_MASK				(0x3f << 24)
-#define  TRAIL_COUNT_SHIFT				16
+#define  EXIT_ZERO_COUNT_MASK_CHV			(0xff << 24)
+#define  EXIT_ZERO_COUNT_SHIFT				24
 #define  TRAIL_COUNT_MASK				(0x1f << 16)
-#define  CLK_ZERO_COUNT_SHIFT				8
+#define  TRAIL_COUNT_MASK_CHV				(0xff << 16)
+#define  TRAIL_COUNT_SHIFT				16
 #define  CLK_ZERO_COUNT_MASK				(0xff << 8)
-#define  PREPARE_COUNT_SHIFT				0
+#define  CLK_ZERO_COUNT_SHIFT				8
 #define  PREPARE_COUNT_MASK				(0x3f << 0)
+#define  PREPARE_COUNT_MASK_CHV				(0xff << 0)
+#define  PREPARE_COUNT_SHIFT				0
 
 /* bits 31:0 */
 #define _MIPIA_DBI_BW_CTRL		(dev_priv->mipi_mmio_base + 0xb084)
drivers/gpu/drm/i915/intel_dsi_vbt.c +19 -12
index 4d6ffa7b3e7b..8d3dea693840 100644
@@ -41,10 +41,17 @@
 #define MIPI_VIRTUAL_CHANNEL_SHIFT	1
 #define MIPI_PORT_SHIFT			3
 
-#define PREPARE_CNT_MAX		0x3F
-#define EXIT_ZERO_CNT_MAX	0x3F
-#define CLK_ZERO_CNT_MAX	0xFF
-#define TRAIL_CNT_MAX		0x1F
+#define EXIT_ZERO_CNT_MAX(dev_priv) \
+	((IS_VALLEYVIEW(dev_priv) ? EXIT_ZERO_COUNT_MASK : EXIT_ZERO_COUNT_MASK_CHV) >> EXIT_ZERO_COUNT_SHIFT)
+
+#define TRAIL_CNT_MAX(dev_priv) \
+	((IS_VALLEYVIEW(dev_priv) ? TRAIL_COUNT_MASK : TRAIL_COUNT_MASK_CHV) >> TRAIL_COUNT_SHIFT)
+
+#define CLK_ZERO_CNT_MAX(dev_priv) \
+	(CLK_ZERO_COUNT_MASK >> CLK_ZERO_COUNT_SHIFT)
+
+#define PREPARE_CNT_MAX(dev_priv)					\
+	((IS_VALLEYVIEW(dev_priv) ? PREPARE_COUNT_MASK : PREPARE_COUNT_MASK_CHV) >> PREPARE_COUNT_SHIFT)
 
 #define NS_KHZ_RATIO 1000000
 
@@ -647,9 +654,9 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	/* prepare count */
 	prepare_cnt = DIV_ROUND_UP(ths_prepare_ns * ui_den, ui_num * mul);
 
-	if (prepare_cnt > PREPARE_CNT_MAX) {
+	if (prepare_cnt > PREPARE_CNT_MAX(dev_priv)) {
 		DRM_DEBUG_KMS("prepare count too high %u\n", prepare_cnt);
-		prepare_cnt = PREPARE_CNT_MAX;
+		prepare_cnt = PREPARE_CNT_MAX(dev_priv);
 	}
 
 	/* exit zero count */
@@ -667,9 +674,9 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	if (exit_zero_cnt < (55 * ui_den / ui_num) && (55 * ui_den) % ui_num)
 		exit_zero_cnt += 1;
 
-	if (exit_zero_cnt > EXIT_ZERO_CNT_MAX) {
+	if (exit_zero_cnt > EXIT_ZERO_CNT_MAX(dev_priv)) {
 		DRM_DEBUG_KMS("exit zero count too high %u\n", exit_zero_cnt);
-		exit_zero_cnt = EXIT_ZERO_CNT_MAX;
+		exit_zero_cnt = EXIT_ZERO_CNT_MAX(dev_priv);
 	}
 
 	/* clk zero count */
@@ -677,18 +684,18 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 				(tclk_prepare_clkzero -	ths_prepare_ns)
 				* ui_den, ui_num * mul);
 
-	if (clk_zero_cnt > CLK_ZERO_CNT_MAX) {
+	if (clk_zero_cnt > CLK_ZERO_CNT_MAX(dev_priv)) {
 		DRM_DEBUG_KMS("clock zero count too high %u\n", clk_zero_cnt);
-		clk_zero_cnt = CLK_ZERO_CNT_MAX;
+		clk_zero_cnt = CLK_ZERO_CNT_MAX(dev_priv);
 	}
 
 	/* trail count */
 	tclk_trail_ns = max(mipi_config->tclk_trail, mipi_config->ths_trail);
 	trail_cnt = DIV_ROUND_UP(tclk_trail_ns * ui_den, ui_num * mul);
 
-	if (trail_cnt > TRAIL_CNT_MAX) {
+	if (trail_cnt > TRAIL_CNT_MAX(dev_priv)) {
 		DRM_DEBUG_KMS("trail count too high %u\n", trail_cnt);
-		trail_cnt = TRAIL_CNT_MAX;
+		trail_cnt = TRAIL_CNT_MAX(dev_priv);
 	}
 
 	/* B080 */
Signature · 1 line
2.11.0
#2 lore raw

Re: [PATCH 2/2] drm/i915/dsi: fix dphy param field widths and range checks for chv+

Quoted 5 lines · Jani Nikula

Current CHV and BXT bspec says the dphy param register has four 8-bit fields instead of the smaller VLV field widths. CHV bspec mentions the register has changed since K0, but there's no indication of what exactly changed. Lacking further details, change the field widths for all CHV and later.

K0 didn't happen, and looks like the linked HSD specifically says that this change was meant for K0. So I suspect this patch is wrong.

The BXT HSD says planned for B0, but not sure if that actually happened either.

On my VLV the reserved bits can't be set:
intel_reg write 0x180000:0xb080 0xffffffff
intel_reg write 0x180000:0xb880 0xffffffff
intel_reg read 0x180000:0xb080 0x180000:0xb880
(0x00180000:0x0000b080): 0x3f1fff3f
(0x00180000:0x0000b880): 0x3f1fff3f

So presumably the same rmw trick could be used to check what how many bits we have on CHV/BXT.

Also looks like the CHV spec was forked from VLV before someone fixed the prepare count to be 6 bits, which seems to be what my VLV actually has as well. Not the first time the VLV docs are more up to date than the CHV ones unfortunately.

Quoted 114 lines

Define the max values based on the platform. Also define them based on the register definitions instead of duplicating information.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106130

---
 drivers/gpu/drm/i915/i915_reg.h      | 11 +++++++----
 drivers/gpu/drm/i915/intel_dsi_vbt.c | 31 +++++++++++++++++++------------
 2 files changed, 26 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/i915_reg.h +7 -4
index fb106026a1f4..f4435a13b757 100644
@@ -9547,14 +9547,17 @@ enum skl_power_gate {
 #define _MIPIA_DPHY_PARAM		(dev_priv->mipi_mmio_base + 0xb080)
 #define _MIPIC_DPHY_PARAM		(dev_priv->mipi_mmio_base + 0xb880)
 #define MIPI_DPHY_PARAM(port)		_MMIO_MIPI(port, _MIPIA_DPHY_PARAM, _MIPIC_DPHY_PARAM)
-#define  EXIT_ZERO_COUNT_SHIFT				24
 #define  EXIT_ZERO_COUNT_MASK				(0x3f << 24)
-#define  TRAIL_COUNT_SHIFT				16
+#define  EXIT_ZERO_COUNT_MASK_CHV			(0xff << 24)
+#define  EXIT_ZERO_COUNT_SHIFT				24
 #define  TRAIL_COUNT_MASK				(0x1f << 16)
-#define  CLK_ZERO_COUNT_SHIFT				8
+#define  TRAIL_COUNT_MASK_CHV				(0xff << 16)
+#define  TRAIL_COUNT_SHIFT				16
 #define  CLK_ZERO_COUNT_MASK				(0xff << 8)
-#define  PREPARE_COUNT_SHIFT				0
+#define  CLK_ZERO_COUNT_SHIFT				8
 #define  PREPARE_COUNT_MASK				(0x3f << 0)
+#define  PREPARE_COUNT_MASK_CHV				(0xff << 0)
+#define  PREPARE_COUNT_SHIFT				0
 
 /* bits 31:0 */
 #define _MIPIA_DBI_BW_CTRL		(dev_priv->mipi_mmio_base + 0xb084)
drivers/gpu/drm/i915/intel_dsi_vbt.c +19 -12
index 4d6ffa7b3e7b..8d3dea693840 100644
@@ -41,10 +41,17 @@
 #define MIPI_VIRTUAL_CHANNEL_SHIFT	1
 #define MIPI_PORT_SHIFT			3
 
-#define PREPARE_CNT_MAX		0x3F
-#define EXIT_ZERO_CNT_MAX	0x3F
-#define CLK_ZERO_CNT_MAX	0xFF
-#define TRAIL_CNT_MAX		0x1F
+#define EXIT_ZERO_CNT_MAX(dev_priv) \
+	((IS_VALLEYVIEW(dev_priv) ? EXIT_ZERO_COUNT_MASK : EXIT_ZERO_COUNT_MASK_CHV) >> EXIT_ZERO_COUNT_SHIFT)
+
+#define TRAIL_CNT_MAX(dev_priv) \
+	((IS_VALLEYVIEW(dev_priv) ? TRAIL_COUNT_MASK : TRAIL_COUNT_MASK_CHV) >> TRAIL_COUNT_SHIFT)
+
+#define CLK_ZERO_CNT_MAX(dev_priv) \
+	(CLK_ZERO_COUNT_MASK >> CLK_ZERO_COUNT_SHIFT)
+
+#define PREPARE_CNT_MAX(dev_priv)					\
+	((IS_VALLEYVIEW(dev_priv) ? PREPARE_COUNT_MASK : PREPARE_COUNT_MASK_CHV) >> PREPARE_COUNT_SHIFT)
 
 #define NS_KHZ_RATIO 1000000
 
@@ -647,9 +654,9 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	/* prepare count */
 	prepare_cnt = DIV_ROUND_UP(ths_prepare_ns * ui_den, ui_num * mul);
 
-	if (prepare_cnt > PREPARE_CNT_MAX) {
+	if (prepare_cnt > PREPARE_CNT_MAX(dev_priv)) {
 		DRM_DEBUG_KMS("prepare count too high %u\n", prepare_cnt);
-		prepare_cnt = PREPARE_CNT_MAX;
+		prepare_cnt = PREPARE_CNT_MAX(dev_priv);
 	}
 
 	/* exit zero count */
@@ -667,9 +674,9 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	if (exit_zero_cnt < (55 * ui_den / ui_num) && (55 * ui_den) % ui_num)
 		exit_zero_cnt += 1;
 
-	if (exit_zero_cnt > EXIT_ZERO_CNT_MAX) {
+	if (exit_zero_cnt > EXIT_ZERO_CNT_MAX(dev_priv)) {
 		DRM_DEBUG_KMS("exit zero count too high %u\n", exit_zero_cnt);
-		exit_zero_cnt = EXIT_ZERO_CNT_MAX;
+		exit_zero_cnt = EXIT_ZERO_CNT_MAX(dev_priv);
 	}
 
 	/* clk zero count */
@@ -677,18 +684,18 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 				(tclk_prepare_clkzero -	ths_prepare_ns)
 				* ui_den, ui_num * mul);
 
-	if (clk_zero_cnt > CLK_ZERO_CNT_MAX) {
+	if (clk_zero_cnt > CLK_ZERO_CNT_MAX(dev_priv)) {
 		DRM_DEBUG_KMS("clock zero count too high %u\n", clk_zero_cnt);
-		clk_zero_cnt = CLK_ZERO_CNT_MAX;
+		clk_zero_cnt = CLK_ZERO_CNT_MAX(dev_priv);
 	}
 
 	/* trail count */
 	tclk_trail_ns = max(mipi_config->tclk_trail, mipi_config->ths_trail);
 	trail_cnt = DIV_ROUND_UP(tclk_trail_ns * ui_den, ui_num * mul);
 
-	if (trail_cnt > TRAIL_CNT_MAX) {
+	if (trail_cnt > TRAIL_CNT_MAX(dev_priv)) {
 		DRM_DEBUG_KMS("trail count too high %u\n", trail_cnt);
-		trail_cnt = TRAIL_CNT_MAX;
+		trail_cnt = TRAIL_CNT_MAX(dev_priv);
 	}
 
 	/* B080 */
Signature · 1 line
2.11.0
Signature · 6 lines
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#3 lore raw

Re: [PATCH 2/2] drm/i915/dsi: fix dphy param field widths and range checks for chv+

Quoted 9 lines · Ville Syrjälä
Quoted 5 lines · Jani Nikula

Current CHV and BXT bspec says the dphy param register has four 8-bit fields instead of the smaller VLV field widths. CHV bspec mentions the register has changed since K0, but there's no indication of what exactly changed. Lacking further details, change the field widths for all CHV and later.

K0 didn't happen, and looks like the linked HSD specifically says that this change was meant for K0. So I suspect this patch is wrong.

Oh well. The referenced bug indicated overflow in the logs, so I figured this might be it.

I pushed patch 1 for now.

Quoted 10 lines

The BXT HSD says planned for B0, but not sure if that actually happened either.

On my VLV the reserved bits can't be set:
intel_reg write 0x180000:0xb080 0xffffffff
intel_reg write 0x180000:0xb880 0xffffffff
intel_reg read 0x180000:0xb080 0x180000:0xb880
(0x00180000:0x0000b080): 0x3f1fff3f
(0x00180000:0x0000b880): 0x3f1fff3f

Quoted 2 lines

So presumably the same rmw trick could be used to check what how many bits we have on CHV/BXT.

Asked on the bug, let's see if we get a response.

BR,
Jani.

Quoted 120 lines

Also looks like the CHV spec was forked from VLV before someone fixed the prepare count to be 6 bits, which seems to be what my VLV actually has as well. Not the first time the VLV docs are more up to date than the CHV ones unfortunately.

Quoted 114 lines

Define the max values based on the platform. Also define them based on the register definitions instead of duplicating information.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106130

---
 drivers/gpu/drm/i915/i915_reg.h      | 11 +++++++----
 drivers/gpu/drm/i915/intel_dsi_vbt.c | 31 +++++++++++++++++++------------
 2 files changed, 26 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/i915_reg.h +7 -4
index fb106026a1f4..f4435a13b757 100644
@@ -9547,14 +9547,17 @@ enum skl_power_gate {
 #define _MIPIA_DPHY_PARAM		(dev_priv->mipi_mmio_base + 0xb080)
 #define _MIPIC_DPHY_PARAM		(dev_priv->mipi_mmio_base + 0xb880)
 #define MIPI_DPHY_PARAM(port)		_MMIO_MIPI(port, _MIPIA_DPHY_PARAM, _MIPIC_DPHY_PARAM)
-#define  EXIT_ZERO_COUNT_SHIFT				24
 #define  EXIT_ZERO_COUNT_MASK				(0x3f << 24)
-#define  TRAIL_COUNT_SHIFT				16
+#define  EXIT_ZERO_COUNT_MASK_CHV			(0xff << 24)
+#define  EXIT_ZERO_COUNT_SHIFT				24
 #define  TRAIL_COUNT_MASK				(0x1f << 16)
-#define  CLK_ZERO_COUNT_SHIFT				8
+#define  TRAIL_COUNT_MASK_CHV				(0xff << 16)
+#define  TRAIL_COUNT_SHIFT				16
 #define  CLK_ZERO_COUNT_MASK				(0xff << 8)
-#define  PREPARE_COUNT_SHIFT				0
+#define  CLK_ZERO_COUNT_SHIFT				8
 #define  PREPARE_COUNT_MASK				(0x3f << 0)
+#define  PREPARE_COUNT_MASK_CHV				(0xff << 0)
+#define  PREPARE_COUNT_SHIFT				0
 
 /* bits 31:0 */
 #define _MIPIA_DBI_BW_CTRL		(dev_priv->mipi_mmio_base + 0xb084)
drivers/gpu/drm/i915/intel_dsi_vbt.c +19 -12
index 4d6ffa7b3e7b..8d3dea693840 100644
@@ -41,10 +41,17 @@
 #define MIPI_VIRTUAL_CHANNEL_SHIFT	1
 #define MIPI_PORT_SHIFT			3
 
-#define PREPARE_CNT_MAX		0x3F
-#define EXIT_ZERO_CNT_MAX	0x3F
-#define CLK_ZERO_CNT_MAX	0xFF
-#define TRAIL_CNT_MAX		0x1F
+#define EXIT_ZERO_CNT_MAX(dev_priv) \
+	((IS_VALLEYVIEW(dev_priv) ? EXIT_ZERO_COUNT_MASK : EXIT_ZERO_COUNT_MASK_CHV) >> EXIT_ZERO_COUNT_SHIFT)
+
+#define TRAIL_CNT_MAX(dev_priv) \
+	((IS_VALLEYVIEW(dev_priv) ? TRAIL_COUNT_MASK : TRAIL_COUNT_MASK_CHV) >> TRAIL_COUNT_SHIFT)
+
+#define CLK_ZERO_CNT_MAX(dev_priv) \
+	(CLK_ZERO_COUNT_MASK >> CLK_ZERO_COUNT_SHIFT)
+
+#define PREPARE_CNT_MAX(dev_priv)					\
+	((IS_VALLEYVIEW(dev_priv) ? PREPARE_COUNT_MASK : PREPARE_COUNT_MASK_CHV) >> PREPARE_COUNT_SHIFT)
 
 #define NS_KHZ_RATIO 1000000
 
@@ -647,9 +654,9 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	/* prepare count */
 	prepare_cnt = DIV_ROUND_UP(ths_prepare_ns * ui_den, ui_num * mul);
 
-	if (prepare_cnt > PREPARE_CNT_MAX) {
+	if (prepare_cnt > PREPARE_CNT_MAX(dev_priv)) {
 		DRM_DEBUG_KMS("prepare count too high %u\n", prepare_cnt);
-		prepare_cnt = PREPARE_CNT_MAX;
+		prepare_cnt = PREPARE_CNT_MAX(dev_priv);
 	}
 
 	/* exit zero count */
@@ -667,9 +674,9 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	if (exit_zero_cnt < (55 * ui_den / ui_num) && (55 * ui_den) % ui_num)
 		exit_zero_cnt += 1;
 
-	if (exit_zero_cnt > EXIT_ZERO_CNT_MAX) {
+	if (exit_zero_cnt > EXIT_ZERO_CNT_MAX(dev_priv)) {
 		DRM_DEBUG_KMS("exit zero count too high %u\n", exit_zero_cnt);
-		exit_zero_cnt = EXIT_ZERO_CNT_MAX;
+		exit_zero_cnt = EXIT_ZERO_CNT_MAX(dev_priv);
 	}
 
 	/* clk zero count */
@@ -677,18 +684,18 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 				(tclk_prepare_clkzero -	ths_prepare_ns)
 				* ui_den, ui_num * mul);
 
-	if (clk_zero_cnt > CLK_ZERO_CNT_MAX) {
+	if (clk_zero_cnt > CLK_ZERO_CNT_MAX(dev_priv)) {
 		DRM_DEBUG_KMS("clock zero count too high %u\n", clk_zero_cnt);
-		clk_zero_cnt = CLK_ZERO_CNT_MAX;
+		clk_zero_cnt = CLK_ZERO_CNT_MAX(dev_priv);
 	}
 
 	/* trail count */
 	tclk_trail_ns = max(mipi_config->tclk_trail, mipi_config->ths_trail);
 	trail_cnt = DIV_ROUND_UP(tclk_trail_ns * ui_den, ui_num * mul);
 
-	if (trail_cnt > TRAIL_CNT_MAX) {
+	if (trail_cnt > TRAIL_CNT_MAX(dev_priv)) {
 		DRM_DEBUG_KMS("trail count too high %u\n", trail_cnt);
-		trail_cnt = TRAIL_CNT_MAX;
+		trail_cnt = TRAIL_CNT_MAX(dev_priv);
 	}
 
 	/* B080 */
Signature · 1 line
2.11.0
Signature · 1 line
Jani Nikula, Intel Open Source Technology Center
#4 lore raw

Re: [PATCH 2/2] drm/i915/dsi: fix dphy param field widths and range checks for chv+

Quoted 26 lines · Jani Nikula
Quoted 9 lines · Ville Syrjälä
Quoted 5 lines · Jani Nikula

Current CHV and BXT bspec says the dphy param register has four 8-bit fields instead of the smaller VLV field widths. CHV bspec mentions the register has changed since K0, but there's no indication of what exactly changed. Lacking further details, change the field widths for all CHV and later.

K0 didn't happen, and looks like the linked HSD specifically says that this change was meant for K0. So I suspect this patch is wrong.

Oh well. The referenced bug indicated overflow in the logs, so I figured this might be it.

I pushed patch 1 for now.

Quoted 10 lines

The BXT HSD says planned for B0, but not sure if that actually happened either.

On my VLV the reserved bits can't be set:
intel_reg write 0x180000:0xb080 0xffffffff
intel_reg write 0x180000:0xb880 0xffffffff
intel_reg read 0x180000:0xb080 0x180000:0xb880
(0x00180000:0x0000b080): 0x3f1fff3f
(0x00180000:0x0000b880): 0x3f1fff3f

Quoted 4 lines
Quoted 2 lines

So presumably the same rmw trick could be used to check what how many bits we have on CHV/BXT.

Asked on the bug, let's see if we get a response.

Confirms what you said:

(0x00180000:0x0000b080): 0x3f1fff3f
(0x00180000:0x0000b880): 0x3f1fff3f

BR,
Jani.

Quoted 124 lines

BR,
Jani.

Quoted 120 lines

Also looks like the CHV spec was forked from VLV before someone fixed the prepare count to be 6 bits, which seems to be what my VLV actually has as well. Not the first time the VLV docs are more up to date than the CHV ones unfortunately.

Quoted 114 lines

Define the max values based on the platform. Also define them based on the register definitions instead of duplicating information.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106130

---
 drivers/gpu/drm/i915/i915_reg.h      | 11 +++++++----
 drivers/gpu/drm/i915/intel_dsi_vbt.c | 31 +++++++++++++++++++------------
 2 files changed, 26 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/i915_reg.h +7 -4
index fb106026a1f4..f4435a13b757 100644
@@ -9547,14 +9547,17 @@ enum skl_power_gate {
 #define _MIPIA_DPHY_PARAM		(dev_priv->mipi_mmio_base + 0xb080)
 #define _MIPIC_DPHY_PARAM		(dev_priv->mipi_mmio_base + 0xb880)
 #define MIPI_DPHY_PARAM(port)		_MMIO_MIPI(port, _MIPIA_DPHY_PARAM, _MIPIC_DPHY_PARAM)
-#define  EXIT_ZERO_COUNT_SHIFT				24
 #define  EXIT_ZERO_COUNT_MASK				(0x3f << 24)
-#define  TRAIL_COUNT_SHIFT				16
+#define  EXIT_ZERO_COUNT_MASK_CHV			(0xff << 24)
+#define  EXIT_ZERO_COUNT_SHIFT				24
 #define  TRAIL_COUNT_MASK				(0x1f << 16)
-#define  CLK_ZERO_COUNT_SHIFT				8
+#define  TRAIL_COUNT_MASK_CHV				(0xff << 16)
+#define  TRAIL_COUNT_SHIFT				16
 #define  CLK_ZERO_COUNT_MASK				(0xff << 8)
-#define  PREPARE_COUNT_SHIFT				0
+#define  CLK_ZERO_COUNT_SHIFT				8
 #define  PREPARE_COUNT_MASK				(0x3f << 0)
+#define  PREPARE_COUNT_MASK_CHV				(0xff << 0)
+#define  PREPARE_COUNT_SHIFT				0
 
 /* bits 31:0 */
 #define _MIPIA_DBI_BW_CTRL		(dev_priv->mipi_mmio_base + 0xb084)
drivers/gpu/drm/i915/intel_dsi_vbt.c +19 -12
index 4d6ffa7b3e7b..8d3dea693840 100644
@@ -41,10 +41,17 @@
 #define MIPI_VIRTUAL_CHANNEL_SHIFT	1
 #define MIPI_PORT_SHIFT			3
 
-#define PREPARE_CNT_MAX		0x3F
-#define EXIT_ZERO_CNT_MAX	0x3F
-#define CLK_ZERO_CNT_MAX	0xFF
-#define TRAIL_CNT_MAX		0x1F
+#define EXIT_ZERO_CNT_MAX(dev_priv) \
+	((IS_VALLEYVIEW(dev_priv) ? EXIT_ZERO_COUNT_MASK : EXIT_ZERO_COUNT_MASK_CHV) >> EXIT_ZERO_COUNT_SHIFT)
+
+#define TRAIL_CNT_MAX(dev_priv) \
+	((IS_VALLEYVIEW(dev_priv) ? TRAIL_COUNT_MASK : TRAIL_COUNT_MASK_CHV) >> TRAIL_COUNT_SHIFT)
+
+#define CLK_ZERO_CNT_MAX(dev_priv) \
+	(CLK_ZERO_COUNT_MASK >> CLK_ZERO_COUNT_SHIFT)
+
+#define PREPARE_CNT_MAX(dev_priv)					\
+	((IS_VALLEYVIEW(dev_priv) ? PREPARE_COUNT_MASK : PREPARE_COUNT_MASK_CHV) >> PREPARE_COUNT_SHIFT)
 
 #define NS_KHZ_RATIO 1000000
 
@@ -647,9 +654,9 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	/* prepare count */
 	prepare_cnt = DIV_ROUND_UP(ths_prepare_ns * ui_den, ui_num * mul);
 
-	if (prepare_cnt > PREPARE_CNT_MAX) {
+	if (prepare_cnt > PREPARE_CNT_MAX(dev_priv)) {
 		DRM_DEBUG_KMS("prepare count too high %u\n", prepare_cnt);
-		prepare_cnt = PREPARE_CNT_MAX;
+		prepare_cnt = PREPARE_CNT_MAX(dev_priv);
 	}
 
 	/* exit zero count */
@@ -667,9 +674,9 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	if (exit_zero_cnt < (55 * ui_den / ui_num) && (55 * ui_den) % ui_num)
 		exit_zero_cnt += 1;
 
-	if (exit_zero_cnt > EXIT_ZERO_CNT_MAX) {
+	if (exit_zero_cnt > EXIT_ZERO_CNT_MAX(dev_priv)) {
 		DRM_DEBUG_KMS("exit zero count too high %u\n", exit_zero_cnt);
-		exit_zero_cnt = EXIT_ZERO_CNT_MAX;
+		exit_zero_cnt = EXIT_ZERO_CNT_MAX(dev_priv);
 	}
 
 	/* clk zero count */
@@ -677,18 +684,18 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 				(tclk_prepare_clkzero -	ths_prepare_ns)
 				* ui_den, ui_num * mul);
 
-	if (clk_zero_cnt > CLK_ZERO_CNT_MAX) {
+	if (clk_zero_cnt > CLK_ZERO_CNT_MAX(dev_priv)) {
 		DRM_DEBUG_KMS("clock zero count too high %u\n", clk_zero_cnt);
-		clk_zero_cnt = CLK_ZERO_CNT_MAX;
+		clk_zero_cnt = CLK_ZERO_CNT_MAX(dev_priv);
 	}
 
 	/* trail count */
 	tclk_trail_ns = max(mipi_config->tclk_trail, mipi_config->ths_trail);
 	trail_cnt = DIV_ROUND_UP(tclk_trail_ns * ui_den, ui_num * mul);
 
-	if (trail_cnt > TRAIL_CNT_MAX) {
+	if (trail_cnt > TRAIL_CNT_MAX(dev_priv)) {
 		DRM_DEBUG_KMS("trail count too high %u\n", trail_cnt);
-		trail_cnt = TRAIL_CNT_MAX;
+		trail_cnt = TRAIL_CNT_MAX(dev_priv);
 	}
 
 	/* B080 */
Signature · 1 line
2.11.0
Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#5 lore raw

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/dsi: improve dphy param limits logging

In reply to Jani Nikula
== Series Details ==

== Series Details ==

Series: series starting with [1/2] drm/i915/dsi: improve dphy param limits logging
URL : https://patchwork.freedesktop.org/series/41953/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
469811c2b803 drm/i915/dsi: improve dphy param limits logging
b6472fbf79b8 drm/i915/dsi: fix dphy param field widths and range checks for chv+
-:63: WARNING:LONG_LINE: line over 100 characters
#63: FILE: drivers/gpu/drm/i915/intel_dsi_vbt.c:45:
+	((IS_VALLEYVIEW(dev_priv) ? EXIT_ZERO_COUNT_MASK : EXIT_ZERO_COUNT_MASK_CHV) >> EXIT_ZERO_COUNT_SHIFT)

-:72: WARNING:LONG_LINE: line over 100 characters #72: FILE: drivers/gpu/drm/i915/intel_dsi_vbt.c:54:
+ ((IS_VALLEYVIEW(dev_priv) ? PREPARE_COUNT_MASK : PREPARE_COUNT_MASK_CHV) >> PREPARE_COUNT_SHIFT)

total: 0 errors, 2 warnings, 0 checks, 86 lines checked

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

#6 lore raw

✗ Fi.CI.SPARSE: warning for series starting with [1/2] drm/i915/dsi: improve dphy param limits logging

In reply to Jani Nikula
== Series Details ==

== Series Details ==

Series: series starting with [1/2] drm/i915/dsi: improve dphy param limits logging
URL : https://patchwork.freedesktop.org/series/41953/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915/dsi: improve dphy param limits logging
-O:drivers/gpu/drm/i915/intel_dsi_vbt.c:671:25: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_dsi_vbt.c:671:25: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dsi_vbt.c:686:25: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_dsi_vbt.c:686:25: warning: expression using sizeof(void)

Commit: drm/i915/dsi: fix dphy param field widths and range checks for chv+ -O:drivers/gpu/drm/i915/intel_dsi_vbt.c:686:25: warning: expression using sizeof(void) -O:drivers/gpu/drm/i915/intel_dsi_vbt.c:686:25: warning: expression using sizeof(void) +drivers/gpu/drm/i915/intel_dsi_vbt.c:693:25: warning: expression using sizeof(void) +drivers/gpu/drm/i915/intel_dsi_vbt.c:693:25: warning: expression using sizeof(void)

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

#7 lore raw

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/dsi: improve dphy param limits logging

In reply to Jani Nikula
== Series Details ==

== Series Details ==

Series: series starting with [1/2] drm/i915/dsi: improve dphy param limits logging
URL : https://patchwork.freedesktop.org/series/41953/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4067 -> Patchwork_8748 =

== Summary - SUCCESS ==

No regressions found.

External URL: https://patchwork.freedesktop.org/api/1.0/series/41953/revisions/1/mbox/

== Known issues ==

Here are the changes found in Patchwork_8748 that come from known issues:

=== IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s3:
      fi-ivb-3520m:       PASS -> DMESG-WARN (fdo#106084)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
      fi-elk-e7500:       PASS -> INCOMPLETE (fdo#103989)

    igt@pm_rpm@basic-rte:
      fi-glk-1:           NOTRUN -> INCOMPLETE (k.org#198133, fdo#103359)

    
    ==== Possible fixes ====

    igt@gem_mmap_gtt@basic-small-bo-tiledx:
      fi-gdg-551:         FAIL (fdo#102575) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-ivb-3520m:       DMESG-WARN (fdo#106084) -> PASS
      fi-skl-guc:         FAIL (fdo#103191) -> PASS

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084
k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133

== Participating hosts (34 -> 32) ==

Additional (1): fi-glk-1
Missing (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq

== Build changes ==

* Linux: CI_DRM_4067 -> Patchwork_8748

CI_DRM_4067: 1c7ccdf37b04bedb10e2191d34dfbba62beb79ea @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4441: 83ba5b7d3bde48b383df41792fc9c955a5a23bdb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8748: b6472fbf79b8a753fd55d7ec7a93f484ad61bae1 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4441: e60d247eb359f044caf0c09904da14e39d7adca1 @ git://anongit.freedesktop.org/piglit

== Linux commits ==

b6472fbf79b8 drm/i915/dsi: fix dphy param field widths and range checks for chv+ 469811c2b803 drm/i915/dsi: improve dphy param limits logging

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8748/issues.html
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reviewed-by on v1 1/2
#8 lore raw
In reply to Jani Nikula
Quoted 5 lines · Jani Nikula

Move the limit checks near the calculations for each field, and actually log the values that exceed limits.

Cc Ville Syrjälä <[email protected]>
Quoted 68 lines
---
 drivers/gpu/drm/i915/intel_dsi_vbt.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/intel_dsi_vbt.c +18 -16
index 91c07b0c8db9..4d6ffa7b3e7b 100644
@@ -647,6 +647,11 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	/* prepare count */
 	prepare_cnt = DIV_ROUND_UP(ths_prepare_ns * ui_den, ui_num * mul);
 
+	if (prepare_cnt > PREPARE_CNT_MAX) {
+		DRM_DEBUG_KMS("prepare count too high %u\n", prepare_cnt);
+		prepare_cnt = PREPARE_CNT_MAX;
+	}
+
 	/* exit zero count */
 	exit_zero_cnt = DIV_ROUND_UP(
 				(ths_prepare_hszero - ths_prepare_ns) * ui_den,
@@ -662,32 +667,29 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	if (exit_zero_cnt < (55 * ui_den / ui_num) && (55 * ui_den) % ui_num)
 		exit_zero_cnt += 1;
 
+	if (exit_zero_cnt > EXIT_ZERO_CNT_MAX) {
+		DRM_DEBUG_KMS("exit zero count too high %u\n", exit_zero_cnt);
+		exit_zero_cnt = EXIT_ZERO_CNT_MAX;
+	}
+
 	/* clk zero count */
 	clk_zero_cnt = DIV_ROUND_UP(
 				(tclk_prepare_clkzero -	ths_prepare_ns)
 				* ui_den, ui_num * mul);
 
+	if (clk_zero_cnt > CLK_ZERO_CNT_MAX) {
+		DRM_DEBUG_KMS("clock zero count too high %u\n", clk_zero_cnt);
+		clk_zero_cnt = CLK_ZERO_CNT_MAX;
+	}
+
 	/* trail count */
 	tclk_trail_ns = max(mipi_config->tclk_trail, mipi_config->ths_trail);
 	trail_cnt = DIV_ROUND_UP(tclk_trail_ns * ui_den, ui_num * mul);
 
-	if (prepare_cnt > PREPARE_CNT_MAX ||
-		exit_zero_cnt > EXIT_ZERO_CNT_MAX ||
-		clk_zero_cnt > CLK_ZERO_CNT_MAX ||
-		trail_cnt > TRAIL_CNT_MAX)
-		DRM_DEBUG_DRIVER("Values crossing maximum limits, restricting to max values\n");
-
-	if (prepare_cnt > PREPARE_CNT_MAX)
-		prepare_cnt = PREPARE_CNT_MAX;
-
-	if (exit_zero_cnt > EXIT_ZERO_CNT_MAX)
-		exit_zero_cnt = EXIT_ZERO_CNT_MAX;
-
-	if (clk_zero_cnt > CLK_ZERO_CNT_MAX)
-		clk_zero_cnt = CLK_ZERO_CNT_MAX;
-
-	if (trail_cnt > TRAIL_CNT_MAX)
+	if (trail_cnt > TRAIL_CNT_MAX) {
+		DRM_DEBUG_KMS("trail count too high %u\n", trail_cnt);
 		trail_cnt = TRAIL_CNT_MAX;
+	}
 
 	/* B080 */
 	intel_dsi->dphy_reg = exit_zero_cnt << 24 | trail_cnt << 16 |
Signature · 1 line
2.11.0
Signature · 6 lines
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#9 lore raw

✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915/dsi: improve dphy param limits logging

In reply to Jani Nikula
== Series Details ==

== Series Details ==

Series: series starting with [1/2] drm/i915/dsi: improve dphy param limits logging
URL : https://patchwork.freedesktop.org/series/41953/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4067_full -> Patchwork_8748_full =

== Summary - WARNING ==

Minor unknown changes coming with Patchwork_8748_full need to be verified manually.

If you think the reported changes have nothing to do with the changes introduced in Patchwork_8748_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI.

External URL: https://patchwork.freedesktop.org/api/1.0/series/41953/revisions/1/mbox/

== Possible new issues ==

Here are the unknown changes that may have been introduced in Patchwork_8748_full:

=== IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-bsd2:
      shard-kbl:          SKIP -> PASS +3

== Known issues ==

Here are the changes found in Patchwork_8748_full that come from known issues:

=== IGT changes ===

    ==== Issues hit ====

    igt@drv_hangman@error-state-capture-blt:
      shard-snb:          PASS -> INCOMPLETE (fdo#103880)

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-kbl:          PASS -> INCOMPLETE (fdo#106023, fdo#103665)

    igt@kms_chv_cursor_fail@pipe-b-128x128-bottom-edge:
      shard-apl:          PASS -> FAIL (fdo#104671)

    igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
      shard-hsw:          PASS -> FAIL (fdo#102887)

    
    ==== Possible fixes ====

    igt@kms_flip@wf_vblank-ts-check-interruptible:
      shard-apl:          FAIL (fdo#100368) -> PASS

fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#103880 https://bugs.freedesktop.org/show_bug.cgi?id=103880
fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023

== Participating hosts (6 -> 4) ==

Missing (2): shard-glk shard-glkb

== Build changes ==

* Linux: CI_DRM_4067 -> Patchwork_8748

CI_DRM_4067: 1c7ccdf37b04bedb10e2191d34dfbba62beb79ea @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4441: 83ba5b7d3bde48b383df41792fc9c955a5a23bdb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8748: b6472fbf79b8a753fd55d7ec7a93f484ad61bae1 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4441: e60d247eb359f044caf0c09904da14e39d7adca1 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8748/shards.html
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx