[PATCH i-g-t 1/3] tools/intel_reg: Don't reuse stale decoded results for later registers

[PATCH i-g-t 1/3] tools/intel_reg: Don't reuse stale decoded results for later registers

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

In case the ->debug_output() function skips decoding the register it
just returns, which means the caller will reuse whatever it already has
in the tmp buffer as the decoded result for this result. What it usually
has in there is the decoded result of some previous register.

Showing incorrect decoded results is no good, so let's allow
->debug_output() to actually return how many bytes it wrote, and the
caller can then skip showing the decoded results if zero bytes
were produced.

We'll make a variant of snprintf() that's safe to call without having to
check the return value for the case when it didn't have enough space to
do its work, that is, make it return 0 in case no bytes were written.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tools/intel_reg_decode.c | 195 ++++++++++++++++++++++++++---------------------
 1 file changed, 109 insertions(+), 86 deletions(-)

diff --git a/tools/intel_reg_decode.c b/tools/intel_reg_decode.c
index 71f3ead6177f..0dbabe81ca3d 100644
--- a/tools/intel_reg_decode.c
+++ b/tools/intel_reg_decode.c
@@ -26,6 +26,7 @@
  */
 
 #include <errno.h>
+#include <stdarg.h>
 #include <stdbool.h>
 #include <stdint.h>
 #include <stdio.h>
@@ -39,12 +40,32 @@
 #include "intel_reg_spec.h"
 
 #define _DEBUGSTRING(func) \
-	void func(char *result, int len, int reg, uint32_t val, uint32_t devid)
+	int func(char *result, int len, int reg, uint32_t val, uint32_t devid)
 #define DEBUGSTRING(func) static _DEBUGSTRING(func)
 
+/* snprintf that returns 0 when the thing didn't fit */
+static int __attribute__((format(printf, 3, 4)))
+z_snprintf(char *str, size_t size, const char *fmt, ...)
+{
+	va_list ap;
+	int ret;
+
+	va_start(ap, fmt);
+	ret = vsnprintf(str, size, fmt, ap);
+	va_end(ap);
+
+	if (ret >= size) {
+		fprintf(stderr, "snprintf() returned %d, expected < %zu\n",
+			ret, size);
+		return 0;
+	}
+
+	return ret;
+}
+
 DEBUGSTRING(i830_16bit_func)
 {
-	snprintf(result, len, "0x%04x", (uint16_t) val);
+	return z_snprintf(result, len, "0x%04x", (uint16_t) val);
 }
 
 DEBUGSTRING(i830_debug_dcc)
@@ -52,7 +73,7 @@ DEBUGSTRING(i830_debug_dcc)
 	const char *addressing = NULL;
 
 	if (!IS_MOBILE(devid))
-		return;
+		return 0;
 
 	if (IS_GEN4(devid)) {
 		if (val & (1 << 1))
@@ -76,7 +97,7 @@ DEBUGSTRING(i830_debug_dcc)
 		}
 	}
 
-	snprintf(result, len, "%s, XOR randomization: %sabled, XOR bit: %d",
+	return z_snprintf(result, len, "%s, XOR randomization: %sabled, XOR bit: %d",
 		 addressing,
 		 (val & (1 << 10)) ? "dis" : "en",
 		 (val & (1 << 9)) ? 17 : 11);
@@ -101,7 +122,7 @@ DEBUGSTRING(i830_debug_chdecmisc)
 		break;
 	}
 
-	snprintf(result, len,
+	return z_snprintf(result, len,
 		 "%s, ch2 enh %sabled, ch1 enh %sabled, "
 		 "ch0 enh %sabled, "
 		 "flex %sabled, ep %spresent", enhmodesel,
@@ -114,24 +135,24 @@ DEBUGSTRING(i830_debug_chdecmisc)
 
 DEBUGSTRING(i830_debug_xyminus1)
 {
-	snprintf(result, len, "%d, %d", (val & 0xffff) + 1,
+	return z_snprintf(result, len, "%d, %d", (val & 0xffff) + 1,
 		 ((val & 0xffff0000) >> 16) + 1);
 }
 
 DEBUGSTRING(i830_debug_yxminus1)
 {
-	snprintf(result, len, "%d, %d", ((val & 0xffff0000) >> 16) + 1,
+	return z_snprintf(result, len, "%d, %d", ((val & 0xffff0000) >> 16) + 1,
 		 (val & 0xffff) + 1);
 }
 
 DEBUGSTRING(i830_debug_xy)
 {
-	snprintf(result, len, "%d, %d", (val & 0xffff), ((val & 0xffff0000) >> 16));
+	return z_snprintf(result, len, "%d, %d", (val & 0xffff), ((val & 0xffff0000) >> 16));
 }
 
 DEBUGSTRING(i830_debug_dspstride)
 {
-	snprintf(result, len, "%d bytes", val);
+	return z_snprintf(result, len, "%d bytes", val);
 }
 
 DEBUGSTRING(i830_debug_dspcntr)
@@ -139,9 +160,9 @@ DEBUGSTRING(i830_debug_dspcntr)
 	const char *enabled = val & DISPLAY_PLANE_ENABLE ? "enabled" : "disabled";
 	char plane = val & DISPPLANE_SEL_PIPE_B ? 'B' : 'A';
 	if (HAS_PCH_SPLIT(devid) || IS_BROXTON(devid))
-		snprintf(result, len, "%s", enabled);
+		return z_snprintf(result, len, "%s", enabled);
 	else
-		snprintf(result, len, "%s, pipe %c", enabled, plane);
+		return z_snprintf(result, len, "%s, pipe %c", enabled, plane);
 }
 
 DEBUGSTRING(i830_debug_pipeconf)
@@ -157,7 +178,7 @@ DEBUGSTRING(i830_debug_pipeconf)
 	else
 		bit30 = val & I965_PIPECONF_ACTIVE ? "active" : "inactive";
 
-	buf_len = snprintf(buf, sizeof(buf), "%s, %s", enabled, bit30);
+	buf_len = z_snprintf(buf, sizeof(buf), "%s, %s", enabled, bit30);
 
 	if (HAS_PCH_SPLIT(devid) || IS_BROXTON(devid)) {
 		const char *interlace;
@@ -190,7 +211,7 @@ DEBUGSTRING(i830_debug_pipeconf)
 			break;
 		}
 		if (buf_len < sizeof(buf))
-			buf_len += snprintf(&buf[buf_len], sizeof(buf) - buf_len,
+			buf_len += z_snprintf(&buf[buf_len], sizeof(buf) - buf_len,
 					    ", %s", interlace);
 	} else if (IS_GEN4(devid) || IS_VALLEYVIEW(devid) ||
 		   IS_CHERRYVIEW(devid)) {
@@ -217,7 +238,7 @@ DEBUGSTRING(i830_debug_pipeconf)
 			break;
 		}
 		if (buf_len < sizeof(buf))
-			buf_len += snprintf(&buf[buf_len], sizeof(buf) - buf_len,
+			buf_len += z_snprintf(&buf[buf_len], sizeof(buf) - buf_len,
 					    ", %s", interlace);
 	}
 
@@ -240,7 +261,7 @@ DEBUGSTRING(i830_debug_pipeconf)
 			break;
 		}
 		if (buf_len < sizeof(buf))
-			buf_len += snprintf(&buf[buf_len], sizeof(buf) - buf_len,
+			buf_len += z_snprintf(&buf[buf_len], sizeof(buf) - buf_len,
 					    ", %s", rotation);
 	}
 
@@ -265,11 +286,11 @@ DEBUGSTRING(i830_debug_pipeconf)
 			break;
 		}
 		if (buf_len < sizeof(buf))
-			buf_len += snprintf(&buf[buf_len], sizeof(buf) - buf_len,
+			buf_len += z_snprintf(&buf[buf_len], sizeof(buf) - buf_len,
 					    ", %s", bpc);
 	}
 
-	snprintf(result, len, "%s", buf);
+	return z_snprintf(result, len, "%s", buf);
 }
 
 DEBUGSTRING(i830_debug_pipestat)
@@ -323,7 +344,7 @@ DEBUGSTRING(i830_debug_pipestat)
 	    val & VBLANK_INT_STATUS ? " VBLANK_INT_STATUS" : "";
 	const char *_OREG_UPDATE_STATUS =
 	    val & OREG_UPDATE_STATUS ? " OREG_UPDATE_STATUS" : "";
-	snprintf(result, len,
+	return z_snprintf(result, len,
 		 "status:%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s",
 		 _FIFO_UNDERRUN,
 		 _CRC_ERROR_ENABLE,
@@ -366,41 +387,41 @@ DEBUGSTRING(ivb_debug_port)
 			drrs = "high";
 			break;
 	}
-	snprintf(result, len, "HW DRRS %s",
+	return z_snprintf(result, len, "HW DRRS %s",
 			drrs);
 }
 
 DEBUGSTRING(i830_debug_hvtotal)
 {
-	snprintf(result, len, "%d active, %d total",
+	return z_snprintf(result, len, "%d active, %d total",
 		 (val & 0xffff) + 1,
 		 ((val & 0xffff0000) >> 16) + 1);
 }
 
 DEBUGSTRING(i830_debug_hvsyncblank)
 {
-	snprintf(result, len, "%d start, %d end",
+	return z_snprintf(result, len, "%d start, %d end",
 		 (val & 0xffff) + 1,
 		 ((val & 0xffff0000) >> 16) + 1);
 }
 
 DEBUGSTRING(i830_debug_vgacntrl)
 {
-	snprintf(result, len, "%s",
+	return z_snprintf(result, len, "%s",
 		 val & VGA_DISP_DISABLE ? "disabled" : "enabled");
 }
 
 DEBUGSTRING(i830_debug_fp)
 {
 	if (IS_PINEVIEW(devid)) {
-		snprintf(result, len, "n = %d, m1 = %d, m2 = %d",
+		return z_snprintf(result, len, "n = %d, m1 = %d, m2 = %d",
 			 ffs((val & FP_N_IGD_DIV_MASK) >>
 			     FP_N_DIV_SHIFT) - 1,
 			 ((val & FP_M1_DIV_MASK) >> FP_M1_DIV_SHIFT),
 			 ((val & FP_M2_IGD_DIV_MASK) >>
 			  FP_M2_DIV_SHIFT));
 	}
-	snprintf(result, len, "n = %d, m1 = %d, m2 = %d",
+	return z_snprintf(result, len, "n = %d, m1 = %d, m2 = %d",
 		 ((val & FP_N_DIV_MASK) >> FP_N_DIV_SHIFT),
 		 ((val & FP_M1_DIV_MASK) >> FP_M1_DIV_SHIFT),
 		 ((val & FP_M2_DIV_MASK) >> FP_M2_DIV_SHIFT));
@@ -424,7 +445,7 @@ DEBUGSTRING(i830_debug_vga_pd)
 		vga1_p1 = ((val & VGA1_PD_P1_MASK) >> VGA1_PD_P1_SHIFT) + 2;
 	vga1_p2 = (val & VGA1_PD_P2_DIV_4) ? 4 : 2;
 
-	snprintf(result, len, "vga0 p1 = %d, p2 = %d, vga1 p1 = %d, p2 = %d",
+	return z_snprintf(result, len, "vga0 p1 = %d, p2 = %d, vga1 p1 = %d, p2 = %d",
 			 vga0_p1, vga0_p2, vga1_p1, vga1_p2);
 }
 
@@ -446,12 +467,12 @@ DEBUGSTRING(i830_debug_pp_status)
 		break;
 	}
 
-	snprintf(result, len, "%s, %s, sequencing %s", status, ready, seq);
+	return z_snprintf(result, len, "%s, %s, sequencing %s", status, ready, seq);
 }
 
 DEBUGSTRING(i830_debug_pp_control)
 {
-	snprintf(result, len, "power target: %s",
+	return z_snprintf(result, len, "power target: %s",
 			 val & POWER_TARGET_ON ? "on" : "off");
 }
 
@@ -539,7 +560,7 @@ DEBUGSTRING(i830_debug_dpll)
 		sdvoextra[0] = '\0';
 	}
 
-	snprintf(result, len, "%s, %s%s, %s clock, %s mode, p1 = %d, "
+	return z_snprintf(result, len, "%s, %s%s, %s clock, %s mode, p1 = %d, "
 			 "p2 = %d%s%s",
 			 enabled, dvomode, vgamode, clock, mode, p1, p2,
 			 fpextra, sdvoextra);
@@ -556,7 +577,7 @@ DEBUGSTRING(i830_debug_dpll_test)
 	const char *dpllbinput = val & DPLLB_INPUT_BUFFER_ENABLE ?
 	    "" : ", DPLLB input buffer disabled";
 
-	snprintf(result, len, "%s%s%s%s%s%s",
+	return z_snprintf(result, len, "%s%s%s%s%s%s",
 			 dpllandiv, dpllamdiv, dpllainput,
 			 dpllbndiv, dpllbmdiv, dpllbinput);
 }
@@ -572,10 +593,10 @@ DEBUGSTRING(i830_debug_adpa)
 		disp_pipe = val & (1<<29) ? 'B' : 'A';
 
 	if (HAS_PCH_SPLIT(devid))
-		snprintf(result, len, "%s, transcoder %c, %chsync, %cvsync",
+		return z_snprintf(result, len, "%s, transcoder %c, %chsync, %cvsync",
 				 enable, disp_pipe, hsync, vsync);
 	else
-		snprintf(result, len, "%s, pipe %c, %chsync, %cvsync",
+		return z_snprintf(result, len, "%s, pipe %c, %chsync, %cvsync",
 				 enable, disp_pipe, hsync, vsync);
 }
 
@@ -598,7 +619,7 @@ DEBUGSTRING(i830_debug_lvds)
 	if (HAS_CPT)
 		disp_pipe = val & (1<<29) ? 'B' : 'A';
 
-	snprintf(result, len, "%s, pipe %c, %d bit, %s",
+	return z_snprintf(result, len, "%s, pipe %c, %d bit, %s",
 			 enable, disp_pipe, depth, channels);
 }
 
@@ -625,7 +646,7 @@ DEBUGSTRING(i830_debug_dvo)
 		break;
 	}
 
-	snprintf(result, len, "%s, pipe %c, %s, %chsync, %cvsync",
+	return z_snprintf(result, len, "%s, pipe %c, %s, %chsync, %cvsync",
 			 enable, disp_pipe, stall, hsync, vsync);
 }
 
@@ -646,7 +667,7 @@ DEBUGSTRING(i830_debug_sdvo)
 		sdvoextra[0] = '\0';
 	}
 
-	snprintf(result, len, "%s, pipe %c, stall %s, %sdetected%s%s",
+	return z_snprintf(result, len, "%s, pipe %c, stall %s, %sdetected%s%s",
 			 enable, disp_pipe, stall, detected, sdvoextra, gang);
 }
 
@@ -685,7 +706,7 @@ DEBUGSTRING(i830_debug_dspclk_gate_d)
 	const char *OVUUNIT = val & OVUUNIT_CLOCK_GATE_DISABLE ? " OVUUNIT" : "";
 	const char *OVLUNIT = val & OVLUNIT_CLOCK_GATE_DISABLE ? " OVLUNIT" : "";
 
-	snprintf(result, len,
+	return z_snprintf(result, len,
 		 "clock gates disabled:%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s",
 		 DPUNIT_B, VSUNIT, VRHUNIT, VRDUNIT, AUDUNIT, DPUNIT_A, DPCUNIT,
 		 TVRUNIT, TVCUNIT, TVFUNIT, TVEUNIT, DVSUNIT, DSSUNIT, DDBUNIT,
@@ -702,16 +723,16 @@ DEBUGSTRING(i810_debug_915_fence)
 	int size = (1024 * 1024) << ((val & 0x700) >> 8);
 
 	if (IS_GEN4(devid) || (IS_915(devid) && reg >= FENCE_NEW))
-		return;
+		return 0;
 
 	if (format == 'X')
 		pitch *= 4;
 	if (val & 1) {
-		snprintf(result, len, "enabled, %c tiled, %4d pitch, 0x%08x - 0x%08x (%dkb)",
+		return z_snprintf(result, len, "enabled, %c tiled, %4d pitch, 0x%08x - 0x%08x (%dkb)",
 			 format, pitch, offset, offset + size,
 			 size / 1024);
 	} else {
-		snprintf(result, len, "disabled");
+		return z_snprintf(result, len, "disabled");
 	}
 }
 
@@ -723,9 +744,9 @@ DEBUGSTRING(i810_debug_965_fence_start)
 	unsigned int offset = val & 0xfffff000;
 
 	if (!IS_GEN4(devid))
-		return;
+		return 0;
 
-	snprintf(result, len, "%s, %c tile walk, %4d pitch, 0x%08x start",
+	return z_snprintf(result, len, "%s, %c tile walk, %4d pitch, 0x%08x start",
 		 enable, format, pitch, offset);
 }
 
@@ -734,9 +755,9 @@ DEBUGSTRING(i810_debug_965_fence_end)
 	unsigned int end = val & 0xfffff000;
 
 	if (!IS_GEN4(devid))
-		return;
+		return 0;
 
-	snprintf(result, len, "                                   0x%08x end", end);
+	return z_snprintf(result, len, "                                   0x%08x end", end);
 }
 
 #define DEFINEREG(reg) \
@@ -1017,19 +1038,19 @@ static const struct reg_debug intel_debug_regs[] = {
 
 DEBUGSTRING(ironlake_debug_rr_hw_ctl)
 {
-	snprintf(result, len, "low %d, high %d", val & RR_HW_LOW_POWER_FRAMES_MASK,
+	return z_snprintf(result, len, "low %d, high %d", val & RR_HW_LOW_POWER_FRAMES_MASK,
 		 (val & RR_HW_HIGH_POWER_FRAMES_MASK) >> 8);
 }
 
 DEBUGSTRING(ironlake_debug_m_tu)
 {
-	snprintf(result, len, "TU %d, val 0x%x %d", (val >> 25) + 1, val & 0xffffff,
+	return z_snprintf(result, len, "TU %d, val 0x%x %d", (val >> 25) + 1, val & 0xffffff,
 		 val & 0xffffff);
 }
 
 DEBUGSTRING(ironlake_debug_n)
 {
-	snprintf(result, len, "val 0x%x %d", val & 0xffffff, val & 0xffffff);
+	return z_snprintf(result, len, "val 0x%x %d", val & 0xffffff, val & 0xffffff);
 }
 
 DEBUGSTRING(ironlake_debug_fdi_tx_ctl)
@@ -1126,7 +1147,7 @@ DEBUGSTRING(ironlake_debug_fdi_tx_ctl)
 		break;
 	}
 
-	snprintf(result, len, "%s, train pattern %s, voltage swing %s,"
+	return z_snprintf(result, len, "%s, train pattern %s, voltage swing %s,"
 		 "pre-emphasis %s, port width %s, enhanced framing %s, FDI PLL %s, scrambing %s, master mode %s",
 		 val & FDI_TX_ENABLE ? "enable" : "disable",
 		 train, voltage, pre_emphasis, portw,
@@ -1203,7 +1224,7 @@ DEBUGSTRING(ironlake_debug_fdi_rx_ctl)
 		break;
 	}
 
-	snprintf(result, len, "%s, train pattern %s, port width %s, %s,"
+	return z_snprintf(result, len, "%s, train pattern %s, port width %s, %s,"
 		 "link_reverse_strap_overwrite %s, dmi_link_reverse %s, FDI PLL %s,"
 		 "FS ecc %s, FE ecc %s, FS err report %s, FE err report %s,"
 		 "scrambing %s, enhanced framing %s, %s",
@@ -1223,7 +1244,7 @@ DEBUGSTRING(ironlake_debug_fdi_rx_ctl)
 
 DEBUGSTRING(ironlake_debug_dspstride)
 {
-	snprintf(result, len, "%d", val >> 6);
+	return z_snprintf(result, len, "%d", val >> 6);
 }
 
 DEBUGSTRING(ironlake_debug_pch_dpll)
@@ -1272,7 +1293,7 @@ DEBUGSTRING(ironlake_debug_pch_dpll)
 
 	sdvo_mul = ((val & PLL_REF_SDVO_HDMI_MULTIPLIER_MASK) >> 9) + 1;
 
-	snprintf(result, len, "%s, sdvo high speed %s, mode %s, p2 %s, "
+	return z_snprintf(result, len, "%s, sdvo high speed %s, mode %s, p2 %s, "
 		 "FPA0 P1 %d, FPA1 P1 %d, refclk %s, sdvo/hdmi mul %d",
 		 enable, highspeed, mode, p2, fpa0_p1, fpa1_p1, refclk,
 		 sdvo_mul);
@@ -1304,7 +1325,7 @@ DEBUGSTRING(ironlake_debug_dref_ctl)
 	default:
 		cpu_source = "reserved";
 	}
-	snprintf(result, len, "cpu source %s, ssc_source %s, nonspread_source %s, "
+	return z_snprintf(result, len, "cpu source %s, ssc_source %s, nonspread_source %s, "
 		 "superspread_source %s, ssc4_mode %s, ssc1 %s, ssc4 %s",
 		 cpu_source, ssc_source, nonspread_source,
 		 superspread_source, ssc4_mode, ssc1, ssc4);
@@ -1342,14 +1363,14 @@ DEBUGSTRING(ironlake_debug_rawclk_freq)
 		tp2 = "12.0us";
 		break;
 	}
-	snprintf(result, len, "FDL_TP1 timer %s, FDL_TP2 timer %s, freq %d",
+	return z_snprintf(result, len, "FDL_TP1 timer %s, FDL_TP2 timer %s, freq %d",
 		 tp1, tp2, val & RAWCLK_FREQ_MASK);
 
 }
 
 DEBUGSTRING(ironlake_debug_fdi_rx_misc)
 {
-	snprintf(result, len, "FDI Delay %d", val & ((1 << 13) - 1));
+	return z_snprintf(result, len, "FDI Delay %d", val & ((1 << 13) - 1));
 }
 
 DEBUGSTRING(ironlake_debug_transconf)
@@ -1375,7 +1396,7 @@ DEBUGSTRING(ironlake_debug_transconf)
 		interlace = "rsvd";
 	}
 
-	snprintf(result, len, "%s, %s, %s", enable, state, interlace);
+	return z_snprintf(result, len, "%s, %s, %s", enable, state, interlace);
 }
 
 DEBUGSTRING(ironlake_debug_panel_fitting)
@@ -1412,7 +1433,7 @@ DEBUGSTRING(ironlake_debug_panel_fitting)
 		break;
 	}
 
-	snprintf(result, len,
+	return z_snprintf(result, len,
 		 "%s, auto_scale %s, auto_scale_cal %s, v_filter %s, vadapt %s, mode %s, filter_sel %s,"
 		 "chroma pre-filter %s, vert3tap %s, v_inter_invert %s",
 		 val & PF_ENABLE ? "enable" : "disable",
@@ -1429,21 +1450,21 @@ DEBUGSTRING(ironlake_debug_panel_fitting)
 
 DEBUGSTRING(ironlake_debug_panel_fitting_2)
 {
-	snprintf(result, len,
+	return z_snprintf(result, len,
 		 "vscale %f",
 		 val / (float) (1<<15));
 }
 
 DEBUGSTRING(ironlake_debug_panel_fitting_3)
 {
-	snprintf(result, len,
+	return z_snprintf(result, len,
 		 "vscale initial phase %f",
 		 val / (float) (1<<15));
 }
 
 DEBUGSTRING(ironlake_debug_panel_fitting_4)
 {
-	snprintf(result, len,
+	return z_snprintf(result, len,
 		 "hscale %f",
 		 val / (float) (1<<15));
 }
@@ -1455,7 +1476,7 @@ DEBUGSTRING(ironlake_debug_pf_win)
 	a = (val >> 16) & 0x1fff;
 	b = val & 0xfff;
 
-	snprintf(result, len, "%d, %d", a, b);
+	return z_snprintf(result, len, "%d, %d", a, b);
 }
 
 DEBUGSTRING(ironlake_debug_hdmi)
@@ -1513,7 +1534,7 @@ DEBUGSTRING(ironlake_debug_hdmi)
 	else
 		detect = "non-detected";
 
-	snprintf(result, len, "%s pipe %c %s %s %s audio %s %s %s %s",
+	return z_snprintf(result, len, "%s pipe %c %s %s %s audio %s %s %s %s",
 		 enable, disp_pipe + 'A', bpc, encoding, mode, audio, vsync, hsync, detect);
 }
 
@@ -1523,7 +1544,7 @@ DEBUGSTRING(snb_debug_dpll_sel)
 	const char *dplla = NULL, *dpllb = NULL;
 
 	if (!HAS_CPT)
-		return;
+		return 0;
 
 	if (val & TRANSA_DPLL_ENABLE) {
 		transa = "enable";
@@ -1543,7 +1564,7 @@ DEBUGSTRING(snb_debug_dpll_sel)
 	} else
 		transb = "disable";
 
-	snprintf(result, len, "TransA DPLL %s (DPLL %s), TransB DPLL %s (DPLL %s)",
+	return z_snprintf(result, len, "TransA DPLL %s (DPLL %s), TransB DPLL %s (DPLL %s)",
 		 transa, dplla, transb, dpllb);
 }
 
@@ -1552,7 +1573,7 @@ DEBUGSTRING(snb_debug_trans_dp_ctl)
 	const char *enable, *port = NULL, *bpc = NULL, *vsync, *hsync;
 
 	if (!HAS_CPT)
-		return;
+		return 0;
 
 	if (val & TRANS_DP_OUTPUT_ENABLE)
 		enable = "enable";
@@ -1599,13 +1620,13 @@ DEBUGSTRING(snb_debug_trans_dp_ctl)
 	else
 		hsync = "-hsync";
 
-	snprintf(result, len, "%s port %s %s %s %s",
+	return z_snprintf(result, len, "%s port %s %s %s %s",
 		 enable, port, bpc, vsync, hsync);
 }
 
 DEBUGSTRING(ilk_debug_pp_control)
 {
-	snprintf(result, len, "blacklight %s, %spower down on reset, panel %s",
+	return z_snprintf(result, len, "blacklight %s, %spower down on reset, panel %s",
 		 (val & (1 << 2)) ? "enabled" : "disabled",
 		 (val & (1 << 1)) ? "" : "do not ",
 		 (val & (1 << 0)) ? "on" : "off");
@@ -1642,7 +1663,7 @@ DEBUGSTRING(hsw_debug_port_clk_sel)
 		break;
 	}
 
-	snprintf(result, len, "%s", clock);
+	return z_snprintf(result, len, "%s", clock);
 }
 
 DEBUGSTRING(hsw_debug_pipe_clk_sel)
@@ -1670,7 +1691,7 @@ DEBUGSTRING(hsw_debug_pipe_clk_sel)
 		break;
 	}
 
-	snprintf(result, len, "%s", clock);
+	return z_snprintf(result, len, "%s", clock);
 }
 
 DEBUGSTRING(hsw_debug_ddi_buf_ctl)
@@ -1697,7 +1718,7 @@ DEBUGSTRING(hsw_debug_ddi_buf_ctl)
 
 	detected = (val & 1) ? "detected" : "not detected";
 
-	snprintf(result, len, "%s %s %s %s", enable, reversal, width, detected);
+	return z_snprintf(result, len, "%s %s %s %s", enable, reversal, width, detected);
 }
 
 DEBUGSTRING(hsw_debug_sfuse_strap)
@@ -1711,7 +1732,7 @@ DEBUGSTRING(hsw_debug_sfuse_strap)
 	portc = (val & (1<<1)) ? "yes" : "no";
 	portd = (val & (1<<0)) ? "yes" : "no";
 
-	snprintf(result, len, "display %s, crt %s, lane reversal %s, "
+	return z_snprintf(result, len, "display %s, crt %s, lane reversal %s, "
 		 "port b %s, port c %s, port d %s", display, crt, lane_reversal,
 		 portb, portc, portd);
 }
@@ -1820,7 +1841,7 @@ DEBUGSTRING(hsw_debug_pipe_ddi_func_ctl)
 		break;
 	}
 
-	snprintf(result, len, "%s, %s, %s, %s, %s, %s, %s, %s", enable,
+	return z_snprintf(result, len, "%s, %s, %s, %s, %s, %s, %s, %s", enable,
 		 port, mode, bpc, vsync, hsync, edp_input, width);
 }
 
@@ -1832,7 +1853,7 @@ DEBUGSTRING(hsw_debug_wm_pipe)
 	sprite = (val >> 8) & 0x7F;
 	cursor = val & 0x3F;
 
-	snprintf(result, len, "primary %d, sprite %d, pipe %d", primary,
+	return z_snprintf(result, len, "primary %d, sprite %d, pipe %d", primary,
 		 sprite, cursor);
 }
 
@@ -1847,7 +1868,7 @@ DEBUGSTRING(hsw_debug_lp_wm)
 	pri = (val >> 8) & 0x3FF;
 	cur = val & 0xFF;
 
-	snprintf(result, len, "%s, latency %d, fbc %d, pri %d, cur %d",
+	return z_snprintf(result, len, "%s, latency %d, fbc %d, pri %d, cur %d",
 		 enable, latency, fbc, pri, cur);
 }
 
@@ -1860,7 +1881,7 @@ DEBUGSTRING(hsw_debug_sinterrupt)
 	portb = (val >> 21) & 1;
 	crt = (val >> 19) & 1;
 
-	snprintf(result, len, "port d:%d, port c:%d, port b:%d, crt:%d",
+	return z_snprintf(result, len, "port d:%d, port c:%d, port b:%d, crt:%d",
 		 portd, portc, portb, crt);
 }
 
@@ -1894,12 +1915,12 @@ DEBUGSTRING(ilk_debug_blc_pwm_cpu_ctl2)
 	}
 
 	if (IS_GEN5(devid) || IS_GEN6(devid) || IS_IVYBRIDGE(devid)) {
-		snprintf(result, len, "enable %d, pipe %s", enable, pipe);
+		return z_snprintf(result, len, "enable %d, pipe %s", enable, pipe);
 	} else {
 		blinking = (val >> 28) & 1;
 		granularity = ((val >> 27) & 1) ? 8 : 128;
 
-		snprintf(result, len, "enable %d, pipe %s, blinking %d, "
+		return z_snprintf(result, len, "enable %d, pipe %s, blinking %d, "
 			 "granularity %d", enable, pipe, blinking,
 			 granularity);
 	}
@@ -1912,11 +1933,11 @@ DEBUGSTRING(ilk_debug_blc_pwm_cpu_ctl)
 	cycle = (val & 0xFFFF);
 
 	if (IS_GEN5(devid) || IS_GEN6(devid) || IS_IVYBRIDGE(devid)) {
-		snprintf(result, len, "cycle %d", cycle);
+		return z_snprintf(result, len, "cycle %d", cycle);
 	} else {
 		freq = (val >> 16) & 0xFFFF;
 
-		snprintf(result, len, "cycle %d, freq %d", cycle, freq);
+		return z_snprintf(result, len, "cycle %d, freq %d", cycle, freq);
 	}
 }
 
@@ -1928,7 +1949,7 @@ DEBUGSTRING(ibx_debug_blc_pwm_ctl1)
 	override = (val >> 30) & 1;
 	inverted_polarity = (val >> 29) & 1;
 
-	snprintf(result, len, "enable %d, override %d, inverted polarity %d",
+	return z_snprintf(result, len, "enable %d, override %d, inverted polarity %d",
 		 enable, override, inverted_polarity);
 }
 
@@ -1939,7 +1960,7 @@ DEBUGSTRING(ibx_debug_blc_pwm_ctl2)
 	freq = (val >> 16) & 0xFFFF;
 	cycle = val & 0xFFFF;
 
-	snprintf(result, len, "freq %d, cycle %d", freq, cycle);
+	return z_snprintf(result, len, "freq %d, cycle %d", freq, cycle);
 }
 
 DEBUGSTRING(hsw_debug_blc_misc_ctl)
@@ -1948,7 +1969,7 @@ DEBUGSTRING(hsw_debug_blc_misc_ctl)
 
 	sel = (val & 1) ? "PWM1-CPU PWM2-PCH" : "PWM1-PCH PWM2-CPU";
 
-	snprintf(result, len, "%s", sel);
+	return z_snprintf(result, len, "%s", sel);
 }
 
 DEBUGSTRING(hsw_debug_util_pin_ctl)
@@ -1994,7 +2015,7 @@ DEBUGSTRING(hsw_debug_util_pin_ctl)
 	data = (val >> 23) & 1;
 	inverted_polarity = (val >> 22) & 1;
 
-	snprintf(result, len, "enable %d, transcoder %s, mode %s, data %d "
+	return z_snprintf(result, len, "enable %d, transcoder %s, mode %s, data %d "
 		 "inverted polarity %d", enable, transcoder, mode, data,
 		 inverted_polarity);
 }
@@ -2530,7 +2551,7 @@ static const struct reg_debug i945gm_mi_regs[] = {
 
 DEBUGSTRING(gen6_rp_control)
 {
-	snprintf(result, len, "%s",
+	return z_snprintf(result, len, "%s",
 		 (val & (1 << 7)) ? "enabled" : "disabled");
 }
 
@@ -2631,13 +2652,15 @@ int intel_reg_spec_decode(char *buf, size_t bufsize, const struct reg *reg,
 			if (reg->addr != r->reg)
 				continue;
 
-			if (r->debug_output)
-				r->debug_output(tmp, sizeof(tmp), r->reg,
-						val, devid);
-			else if (devid)
+			if (r->debug_output) {
+				if (r->debug_output(tmp, sizeof(tmp), r->reg,
+						    val, devid) == 0)
+					continue;
+			} else if (devid) {
 				return 0;
-			else
+			} else {
 				continue;
+			}
 
 			if (devid) {
 				strncpy(buf, tmp, bufsize);
-- 
2.7.4

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

[PATCH i-g-t 2/3] tools/intel_reg: Dump fence registers on ILK

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently ILK doesn't get its fences dumped. Let's fix that.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tools/intel_reg_decode.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/tools/intel_reg_decode.c b/tools/intel_reg_decode.c
index 0dbabe81ca3d..2d79290a4b58 100644
--- a/tools/intel_reg_decode.c
+++ b/tools/intel_reg_decode.c
@@ -743,7 +743,7 @@ DEBUGSTRING(i810_debug_965_fence_start)
 	int pitch = ((val & 0xffc) >> 2) * 128 + 128;
 	unsigned int offset = val & 0xfffff000;
 
-	if (!IS_GEN4(devid))
+	if (!IS_GEN4(devid) && !IS_GEN5(devid))
 		return 0;
 
 	return z_snprintf(result, len, "%s, %c tile walk, %4d pitch, 0x%08x start",
@@ -1012,6 +1012,10 @@ static const struct reg_debug intel_debug_regs[] = {
 	DEFINEFENCE_945(14),
 	DEFINEFENCE_945(15),
 
+	DEFINEREG(INST_PM),
+};
+
+static const struct reg_debug i965_fences[] = {
 #define DEFINEFENCE_965(i) \
 	{ FENCE_NEW+i*8, "FENCE START " #i, i810_debug_965_fence_start }, \
 	{ FENCE_NEW+i*8+4, "FENCE END " #i, i810_debug_965_fence_end }
@@ -1032,8 +1036,6 @@ static const struct reg_debug intel_debug_regs[] = {
 	DEFINEFENCE_965(13),
 	DEFINEFENCE_965(14),
 	DEFINEFENCE_965(15),
-
-	DEFINEREG(INST_PM),
 };
 
 DEBUGSTRING(ironlake_debug_rr_hw_ctl)
@@ -2594,6 +2596,11 @@ static bool is_gen56ivb(uint32_t devid, uint32_t pch)
 	return IS_GEN5(devid) || IS_GEN6(devid) || IS_IVYBRIDGE(devid);
 }
 
+static bool is_gen45(uint32_t devid, uint32_t pch)
+{
+	return IS_GEN4(devid) || IS_GEN5(devid);
+}
+
 static bool is_945gm(uint32_t devid, uint32_t pch)
 {
 	return IS_945GM(devid);
@@ -2614,6 +2621,7 @@ static struct {
 } known_registers[] = {
 	DECLARE_REGS("Gen2",	intel_debug_regs,	is_gen234),
 	DECLARE_REGS("i945GM",	i945gm_mi_regs,		is_945gm),
+	DECLARE_REGS("Gen4",	i965_fences,		is_gen45),
 	DECLARE_REGS("Gen5",	ironlake_debug_regs,	is_gen56ivb),
 	DECLARE_REGS("Gen6",	gen6_rp_debug_regs,	is_gen6_plus),
 	DECLARE_REGS("Gen6+",	gen6_fences,		is_gen6_plus),
-- 
2.7.4

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

[PATCH i-g-t 3/3] tools/intel_reg: Dump DP_BUFTRANS registers on ILK-IVB

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 lib/intel_reg.h          |  2 ++
 tools/intel_reg_decode.c | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/lib/intel_reg.h b/lib/intel_reg.h
index 0ffa803dab8d..3a28c08c3595 100644
--- a/lib/intel_reg.h
+++ b/lib/intel_reg.h
@@ -3099,6 +3099,8 @@ typedef enum {
 #define TRANSC_DP_LINK_M2	0xe2048
 #define TRANSC_DP_LINK_N2	0xe204c
 
+#define DP_BUFTRANS(x)		(0xe4f00 + 4 * (x))
+
 #define TRANSACONF		0xf0008
 #define TRANSBCONF		0xf1008
 #define TRANSCCONF		0xf2008
diff --git a/tools/intel_reg_decode.c b/tools/intel_reg_decode.c
index 2d79290a4b58..98415fe8ae31 100644
--- a/tools/intel_reg_decode.c
+++ b/tools/intel_reg_decode.c
@@ -1540,6 +1540,26 @@ DEBUGSTRING(ironlake_debug_hdmi)
 		 enable, disp_pipe + 'A', bpc, encoding, mode, audio, vsync, hsync, detect);
 }
 
+DEBUGSTRING(ironlake_debug_dp_buftrans)
+{
+	static const char * const reg_names[] = {
+		"400mV, 0.0dB",
+		"400mV, 3.5dB",
+		"400mV, 6.0dB",
+		"400mV, 9.5dB",
+		"600mV, 0.0dB",
+		"600mV, 3.5dB",
+		"600mV, 6.0dB",
+		"800mV, 0.0dB",
+		"800mV, 3.5dB",
+		"1200mV, 0.0dB",
+	};
+	int idx = (reg - DP_BUFTRANS(0)) / 4;
+
+	return z_snprintf(result, len, "%s: OE=%3d, pre-emphasis=%2d, P current drive=%2d, N current drive=%2d",
+			  reg_names[idx], (val >> 19) & 0x1ff, (val >> 12) & 0x1f, (val >> 6) & 0xf, val & 0xf);
+}
+
 DEBUGSTRING(snb_debug_dpll_sel)
 {
 	const char *transa, *transb;
@@ -2304,6 +2324,18 @@ static const struct reg_debug ironlake_debug_regs[] = {
 	DEFINEREG(PCH_DP_B),
 	DEFINEREG(PCH_DP_C),
 	DEFINEREG(PCH_DP_D),
+
+	DEFINEREG2(DP_BUFTRANS(0), ironlake_debug_dp_buftrans),
+	DEFINEREG2(DP_BUFTRANS(1), ironlake_debug_dp_buftrans),
+	DEFINEREG2(DP_BUFTRANS(2), ironlake_debug_dp_buftrans),
+	DEFINEREG2(DP_BUFTRANS(3), ironlake_debug_dp_buftrans),
+	DEFINEREG2(DP_BUFTRANS(4), ironlake_debug_dp_buftrans),
+	DEFINEREG2(DP_BUFTRANS(5), ironlake_debug_dp_buftrans),
+	DEFINEREG2(DP_BUFTRANS(6), ironlake_debug_dp_buftrans),
+	DEFINEREG2(DP_BUFTRANS(7), ironlake_debug_dp_buftrans),
+	DEFINEREG2(DP_BUFTRANS(8), ironlake_debug_dp_buftrans),
+	DEFINEREG2(DP_BUFTRANS(9), ironlake_debug_dp_buftrans),
+
 	DEFINEREG2(TRANS_DP_CTL_A, snb_debug_trans_dp_ctl),
 	DEFINEREG2(TRANS_DP_CTL_B, snb_debug_trans_dp_ctl),
 	DEFINEREG2(TRANS_DP_CTL_C, snb_debug_trans_dp_ctl),
-- 
2.7.4

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

Re: [PATCH i-g-t 1/3] tools/intel_reg: Don't reuse stale decoded results for later registers

[Jani Nikula]

On Fri, 05 Aug 2016, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> In case the ->debug_output() function skips decoding the register it
> just returns, which means the caller will reuse whatever it already has
> in the tmp buffer as the decoded result for this result. What it usually
> has in there is the decoded result of some previous register.
>
> Showing incorrect decoded results is no good, so let's allow
> ->debug_output() to actually return how many bytes it wrote, and the
> caller can then skip showing the decoded results if zero bytes
> were produced.
>
> We'll make a variant of snprintf() that's safe to call without having to
> check the return value for the case when it didn't have enough space to
> do its work, that is, make it return 0 in case no bytes were written.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  tools/intel_reg_decode.c | 195 ++++++++++++++++++++++++++---------------------
>  1 file changed, 109 insertions(+), 86 deletions(-)
>
> diff --git a/tools/intel_reg_decode.c b/tools/intel_reg_decode.c
> index 71f3ead6177f..0dbabe81ca3d 100644
> --- a/tools/intel_reg_decode.c
> +++ b/tools/intel_reg_decode.c
> @@ -26,6 +26,7 @@
>   */
>  
>  #include <errno.h>
> +#include <stdarg.h>
>  #include <stdbool.h>
>  #include <stdint.h>
>  #include <stdio.h>
> @@ -39,12 +40,32 @@
>  #include "intel_reg_spec.h"
>  
>  #define _DEBUGSTRING(func) \
> -	void func(char *result, int len, int reg, uint32_t val, uint32_t devid)
> +	int func(char *result, int len, int reg, uint32_t val, uint32_t devid)
>  #define DEBUGSTRING(func) static _DEBUGSTRING(func)

This could use a brief comment describing the return value.

[snip]

> @@ -2631,13 +2652,15 @@ int intel_reg_spec_decode(char *buf, size_t bufsize, const struct reg *reg,
>  			if (reg->addr != r->reg)
>  				continue;
>  
> -			if (r->debug_output)
> -				r->debug_output(tmp, sizeof(tmp), r->reg,
> -						val, devid);
> -			else if (devid)
> +			if (r->debug_output) {
> +				if (r->debug_output(tmp, sizeof(tmp), r->reg,
> +						    val, devid) == 0)
> +					continue;
> +			} else if (devid) {
>  				return 0;
> -			else
> +			} else {
>  				continue;
> +			}

Bah, I've included too much trickery between the devid == 0 vs != 0
cases to make this clear. No fault of this patch.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

No need to resend, please just add the comment on the return value.


>  
>  			if (devid) {
>  				strncpy(buf, tmp, bufsize);

-- 
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: [PATCH i-g-t 1/3] tools/intel_reg: Don't reuse stale decoded results for later registers

[Ville Syrjälä]

On Fri, Aug 05, 2016 at 11:54:19AM +0300, Jani Nikula wrote:
> On Fri, 05 Aug 2016, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > In case the ->debug_output() function skips decoding the register it
> > just returns, which means the caller will reuse whatever it already has
> > in the tmp buffer as the decoded result for this result. What it usually
> > has in there is the decoded result of some previous register.
> >
> > Showing incorrect decoded results is no good, so let's allow
> > ->debug_output() to actually return how many bytes it wrote, and the
> > caller can then skip showing the decoded results if zero bytes
> > were produced.
> >
> > We'll make a variant of snprintf() that's safe to call without having to
> > check the return value for the case when it didn't have enough space to
> > do its work, that is, make it return 0 in case no bytes were written.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  tools/intel_reg_decode.c | 195 ++++++++++++++++++++++++++---------------------
> >  1 file changed, 109 insertions(+), 86 deletions(-)
> >
> > diff --git a/tools/intel_reg_decode.c b/tools/intel_reg_decode.c
> > index 71f3ead6177f..0dbabe81ca3d 100644
> > --- a/tools/intel_reg_decode.c
> > +++ b/tools/intel_reg_decode.c
> > @@ -26,6 +26,7 @@
> >   */
> >  
> >  #include <errno.h>
> > +#include <stdarg.h>
> >  #include <stdbool.h>
> >  #include <stdint.h>
> >  #include <stdio.h>
> > @@ -39,12 +40,32 @@
> >  #include "intel_reg_spec.h"
> >  
> >  #define _DEBUGSTRING(func) \
> > -	void func(char *result, int len, int reg, uint32_t val, uint32_t devid)
> > +	int func(char *result, int len, int reg, uint32_t val, uint32_t devid)
> >  #define DEBUGSTRING(func) static _DEBUGSTRING(func)
> 
> This could use a brief comment describing the return value.
> 
> [snip]
> 
> > @@ -2631,13 +2652,15 @@ int intel_reg_spec_decode(char *buf, size_t bufsize, const struct reg *reg,
> >  			if (reg->addr != r->reg)
> >  				continue;
> >  
> > -			if (r->debug_output)
> > -				r->debug_output(tmp, sizeof(tmp), r->reg,
> > -						val, devid);
> > -			else if (devid)
> > +			if (r->debug_output) {
> > +				if (r->debug_output(tmp, sizeof(tmp), r->reg,
> > +						    val, devid) == 0)
> > +					continue;
> > +			} else if (devid) {
> >  				return 0;
> > -			else
> > +			} else {
> >  				continue;
> > +			}
> 
> Bah, I've included too much trickery between the devid == 0 vs != 0
> cases to make this clear. No fault of this patch.
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> No need to resend, please just add the comment on the return value.

Added full blown docs for it, and pushed the lot.

/**
 * DEBUGSTRING:
 * @result: character string where to write
 * @len: maximum number of bytes to write
 * @reg: register offset
 * @val: register value
 * @devid: PCI device ID
 *
 * Print the decoded representation of the register value into @result.
 *
 * Returns: Number of bytes written to @result
 */

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