public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH i-g-t 0/8] tools/intel_vbt_decode: refactoring and cleanups
@ 2017-10-19 15:22 Jani Nikula
  2017-10-19 15:22 ` [PATCH i-g-t 1/8] tools/intel_vbt_decode: make a copy of child devices before dumping Jani Nikula
                   ` (8 more replies)
  0 siblings, 9 replies; 15+ messages in thread
From: Jani Nikula @ 2017-10-19 15:22 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Flush some old branches to the list...

BR,
Jani.


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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH i-g-t 1/8] tools/intel_vbt_decode: make a copy of child devices before dumping
  2017-10-19 15:22 [PATCH i-g-t 0/8] tools/intel_vbt_decode: refactoring and cleanups Jani Nikula
@ 2017-10-19 15:22 ` Jani Nikula
  2017-10-19 15:22 ` [PATCH i-g-t 2/8] tools/intel_vbt_decode: update dvo port name dumping Jani Nikula
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Jani Nikula @ 2017-10-19 15:22 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Take child device size into account, avoid reading past the actual child
device.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 tools/intel_vbt_decode.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/tools/intel_vbt_decode.c b/tools/intel_vbt_decode.c
index 948dc29dd114..499dcb065745 100644
--- a/tools/intel_vbt_decode.c
+++ b/tools/intel_vbt_decode.c
@@ -36,6 +36,7 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 
+#include "igt_aux.h"
 #include "intel_io.h"
 #include "intel_chipset.h"
 #include "drmtest.h"
@@ -475,6 +476,7 @@ static void dump_general_definitions(struct context *context,
 				     const struct bdb_block *block)
 {
 	const struct bdb_general_definitions *defs = block->data;
+	struct child_device_config *child;
 	int i;
 	int child_device_num;
 
@@ -489,8 +491,22 @@ static void dump_general_definitions(struct context *context,
 	printf("\tChild device size: %d\n", defs->child_dev_size);
 	child_device_num = (block->size - sizeof(*defs)) /
 		defs->child_dev_size;
-	for (i = 0; i < child_device_num; i++)
-		dump_child_device(context, (const void*)&defs->devices[i * defs->child_dev_size]);
+
+	/*
+	 * Use a temp buffer so dump_child_device() doesn't have to worry about
+	 * accessing the struct beyond child_dev_size. The tail, if any, remains
+	 * initialized to zero.
+	 */
+	child = calloc(1, sizeof(*child));
+
+	for (i = 0; i < child_device_num; i++) {
+		memcpy(child, &defs->devices[i * defs->child_dev_size],
+		       min(sizeof(*child), defs->child_dev_size));
+
+		dump_child_device(context, child);
+	}
+
+	free(child);
 }
 
 static void dump_legacy_child_devices(struct context *context,
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH i-g-t 2/8] tools/intel_vbt_decode: update dvo port name dumping
  2017-10-19 15:22 [PATCH i-g-t 0/8] tools/intel_vbt_decode: refactoring and cleanups Jani Nikula
  2017-10-19 15:22 ` [PATCH i-g-t 1/8] tools/intel_vbt_decode: make a copy of child devices before dumping Jani Nikula
@ 2017-10-19 15:22 ` Jani Nikula
  2017-10-19 15:22 ` [PATCH i-g-t 3/8] tools/intel_vbt_decode: use %.*s instead of duplicating a string Jani Nikula
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Jani Nikula @ 2017-10-19 15:22 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Add names for new ports, throw out unused macros.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 tools/intel_bios.h       | 12 ------------
 tools/intel_vbt_decode.c | 47 +++++++++++++++++++++++++----------------------
 2 files changed, 25 insertions(+), 34 deletions(-)

diff --git a/tools/intel_bios.h b/tools/intel_bios.h
index 4e06ef74e459..f0475b5cfcc3 100644
--- a/tools/intel_bios.h
+++ b/tools/intel_bios.h
@@ -42,18 +42,6 @@
 #define DEVICE_TYPE_DVI			0x68d2
 #define DEVICE_TYPE_MIPI		0x7cc2
 
-#define DEVICE_PORT_DVOA	0x00	/* none on 845+ */
-#define DEVICE_PORT_DVOB	0x01
-#define DEVICE_PORT_DVOC	0x02
-
-#define DEVICE_PORT_NONE	0
-#define DEVICE_PORT_HDMIB	1
-#define DEVICE_PORT_HDMIC	2
-#define DEVICE_PORT_HDMID	3
-#define DEVICE_PORT_DPB		7
-#define DEVICE_PORT_DPC		8
-#define DEVICE_PORT_DPD		9
-
 struct legacy_child_device_config {
 	uint16_t handle;
 	uint16_t device_type;	/* See DEVICE_TYPE_* above */
diff --git a/tools/intel_vbt_decode.c b/tools/intel_vbt_decode.c
index 499dcb065745..4865f0bab25f 100644
--- a/tools/intel_vbt_decode.c
+++ b/tools/intel_vbt_decode.c
@@ -356,29 +356,32 @@ static const char *child_device_handle(unsigned char handle)
 	return "unknown";
 }
 
-static const struct {
-	unsigned short type;
-	const char *name;
-} efp_ports[] = {
-	{ DEVICE_PORT_NONE, "N/A" },
-	{ DEVICE_PORT_HDMIB, "HDMI-B" },
-	{ DEVICE_PORT_HDMIC, "HDMI-C" },
-	{ DEVICE_PORT_HDMID, "HDMI-D" },
-	{ DEVICE_PORT_DPB, "DP-B" },
-	{ DEVICE_PORT_DPC, "DP-C" },
-	{ DEVICE_PORT_DPD, "DP-D" },
+static const char *dvo_port_names[] = {
+	[DVO_PORT_HDMIA] = "HDMI-A",
+	[DVO_PORT_HDMIB] = "HDMI-B",
+	[DVO_PORT_HDMIC] = "HDMI-C",
+	[DVO_PORT_HDMID] = "HDMI-D",
+	[DVO_PORT_LVDS] = "LVDS",
+	[DVO_PORT_TV] = "TV",
+	[DVO_PORT_CRT] = "CRT",
+	[DVO_PORT_DPB] = "DP-B",
+	[DVO_PORT_DPC] = "DP-C",
+	[DVO_PORT_DPD] = "DP-D",
+	[DVO_PORT_DPA] = "DP-A",
+	[DVO_PORT_DPE] = "DP-E",
+	[DVO_PORT_HDMIE] = "HDMI-E",
+	[DVO_PORT_MIPIA] = "MIPI-A",
+	[DVO_PORT_MIPIB] = "MIPI-B",
+	[DVO_PORT_MIPIC] = "MIPI-C",
+	[DVO_PORT_MIPID] = "MIPI-D",
 };
-static const int num_efp_ports = sizeof(efp_ports) / sizeof(efp_ports[0]);
 
-static const char *efp_port(uint8_t type)
+static const char *dvo_port(uint8_t type)
 {
-	int i;
-
-	for (i = 0; i < num_efp_ports; i++)
-		if (efp_ports[i].type == type)
-			return efp_ports[i].name;
-
-	return "unknown";
+	if (type < ARRAY_SIZE(dvo_port_names) && dvo_port_names[type])
+		return dvo_port_names[type];
+	else
+		return "unknown";
 }
 
 static void dump_child_device(struct context *context,
@@ -418,9 +421,9 @@ static void dump_child_device(struct context *context,
 		printf("\t\tCompression enable: %s\n", YESNO(efp->compression_enable));
 		printf("\t\tEdidless EFP: %s\n", YESNO(efp->edidless_efp));
 		printf("\t\tCompression structure index: 0x%02x)\n", efp->compression_structure_index);
-		printf("\t\tSlave DDI port: 0x%02x (%s)\n", efp->slave_port, efp_port(efp->slave_port));
+		printf("\t\tSlave DDI port: 0x%02x (%s)\n", efp->slave_port, dvo_port(efp->slave_port));
 		printf("\t\tAIM offset: %d\n", child->addin_offset);
-		printf("\t\tPort: 0x%02x (%s)\n", efp->dvo_port, efp_port(efp->dvo_port));
+		printf("\t\tPort: 0x%02x (%s)\n", efp->dvo_port, dvo_port(efp->dvo_port));
 		printf("\t\tAIM I2C pin: 0x%02x\n", efp->i2c_pin);
 		printf("\t\tAIM Slave address: 0x%02x\n", efp->slave_addr);
 		printf("\t\tDDC pin: 0x%02x\n", efp->ddc_pin);
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH i-g-t 3/8] tools/intel_vbt_decode: use %.*s instead of duplicating a string
  2017-10-19 15:22 [PATCH i-g-t 0/8] tools/intel_vbt_decode: refactoring and cleanups Jani Nikula
  2017-10-19 15:22 ` [PATCH i-g-t 1/8] tools/intel_vbt_decode: make a copy of child devices before dumping Jani Nikula
  2017-10-19 15:22 ` [PATCH i-g-t 2/8] tools/intel_vbt_decode: update dvo port name dumping Jani Nikula
@ 2017-10-19 15:22 ` Jani Nikula
  2017-10-19 15:22 ` [PATCH i-g-t 4/8] tools/intel_vbt_decode: abstract DSI bridge type dump Jani Nikula
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Jani Nikula @ 2017-10-19 15:22 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

child->device_id may not be terminated, but we can use %.*s format
specifier to define the max length to print. No need to make a copy.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 tools/intel_vbt_decode.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/tools/intel_vbt_decode.c b/tools/intel_vbt_decode.c
index 4865f0bab25f..0f7e2dafc762 100644
--- a/tools/intel_vbt_decode.c
+++ b/tools/intel_vbt_decode.c
@@ -388,19 +388,15 @@ static void dump_child_device(struct context *context,
 			      const struct child_device_config *child)
 {
 	const struct child_device_config *efp = child;
-	char child_id[11];
 
 	if (!child->device_type)
 		return;
 
 	if (context->bdb->version < 152) {
-		strncpy(child_id, (const char *)child->device_id, 10);
-		child_id[10] = 0;
-
 		printf("\tChild device info:\n");
 		printf("\t\tDevice type: %04x (%s)\n", child->device_type,
 		       child_device_type(child->device_type));
-		printf("\t\tSignature: %s\n", child_id);
+		printf("\t\tSignature: %.*s\n", (int)sizeof(child->device_id), child->device_id);
 		printf("\t\tAIM offset: %d\n", child->addin_offset);
 		printf("\t\tDVO port: 0x%02x\n", child->dvo_port);
 	} else { /* 152+ have EFP blocks here */
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH i-g-t 4/8] tools/intel_vbt_decode: abstract DSI bridge type dump
  2017-10-19 15:22 [PATCH i-g-t 0/8] tools/intel_vbt_decode: refactoring and cleanups Jani Nikula
                   ` (2 preceding siblings ...)
  2017-10-19 15:22 ` [PATCH i-g-t 3/8] tools/intel_vbt_decode: use %.*s instead of duplicating a string Jani Nikula
@ 2017-10-19 15:22 ` Jani Nikula
  2017-10-19 15:22 ` [PATCH i-g-t 5/8] tools/intel_vbt_decode: unify child device printing across versions Jani Nikula
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Jani Nikula @ 2017-10-19 15:22 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Cleaner than having it inline.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 tools/intel_vbt_decode.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/tools/intel_vbt_decode.c b/tools/intel_vbt_decode.c
index 0f7e2dafc762..69cf1e4c6cc6 100644
--- a/tools/intel_vbt_decode.c
+++ b/tools/intel_vbt_decode.c
@@ -384,6 +384,20 @@ static const char *dvo_port(uint8_t type)
 		return "unknown";
 }
 
+static const char *mipi_bridge_type(uint8_t type)
+{
+	switch (type) {
+	case 1:
+		return "ASUS";
+	case 2:
+		return "Toshiba";
+	case 3:
+		return "Renesas";
+	default:
+		return "unknown";
+	}
+}
+
 static void dump_child_device(struct context *context,
 			      const struct child_device_config *child)
 {
@@ -440,21 +454,8 @@ static void dump_child_device(struct context *context,
 		printf("\t\tSDVO stall signal available: %s\n", YESNO(efp->sdvo_stall));
 		printf("\t\tPipe capabilities: 0x%02x\n", efp->pipe_cap);
 		printf("\t\tDVO wiring: 0x%02x\n", efp->dvo_wiring);
-		printf("\t\tMIPI bridge type:");
-		switch (efp->mipi_bridge_type) {
-		case 1:
-			printf("ASUS\n");
-			break;
-		case 2:
-			printf("Toshiba\n");
-			break;
-		case 3:
-			printf("Renesas\n");
-			break;
-		default:
-			printf("(unknown value %d)\n", efp->mipi_bridge_type);
-			break;
-		}
+		printf("\t\tMIPI bridge type: %02x (%s)\n", efp->mipi_bridge_type,
+		       mipi_bridge_type(efp->mipi_bridge_type));
 		printf("\t\tDevice class extendsion: 0x%02x\n", efp->extended_type);
 		printf("\t\tDVO function: 0x%02x\n", efp->dvo_function);
 	}
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH i-g-t 5/8] tools/intel_vbt_decode: unify child device printing across versions
  2017-10-19 15:22 [PATCH i-g-t 0/8] tools/intel_vbt_decode: refactoring and cleanups Jani Nikula
                   ` (3 preceding siblings ...)
  2017-10-19 15:22 ` [PATCH i-g-t 4/8] tools/intel_vbt_decode: abstract DSI bridge type dump Jani Nikula
@ 2017-10-19 15:22 ` Jani Nikula
  2017-10-19 17:24   ` Ville Syrjälä
  2017-10-19 15:22 ` [PATCH i-g-t 6/8] tools/intel_vbt_decode: unify legacy child device block dumping Jani Nikula
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Jani Nikula @ 2017-10-19 15:22 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Make it easier to compare dumping against the struct definition.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 tools/intel_vbt_decode.c | 128 ++++++++++++++++++++++++++---------------------
 1 file changed, 71 insertions(+), 57 deletions(-)

diff --git a/tools/intel_vbt_decode.c b/tools/intel_vbt_decode.c
index 69cf1e4c6cc6..bc5f38112619 100644
--- a/tools/intel_vbt_decode.c
+++ b/tools/intel_vbt_decode.c
@@ -401,74 +401,88 @@ static const char *mipi_bridge_type(uint8_t type)
 static void dump_child_device(struct context *context,
 			      const struct child_device_config *child)
 {
-	const struct child_device_config *efp = child;
-
 	if (!child->device_type)
 		return;
 
+	printf("\tChild device info:\n");
+	printf("\t\tDevice handle: 0x%04x (%s)\n", child->handle,
+	       child_device_handle(child->handle));
+	printf("\t\tDevice type: 0x%04x (%s)\n", child->device_type,
+	       child_device_type(child->device_type));
+	dump_child_device_type_bits(child->device_type);
+
 	if (context->bdb->version < 152) {
-		printf("\tChild device info:\n");
-		printf("\t\tDevice type: %04x (%s)\n", child->device_type,
-		       child_device_type(child->device_type));
 		printf("\t\tSignature: %.*s\n", (int)sizeof(child->device_id), child->device_id);
-		printf("\t\tAIM offset: %d\n", child->addin_offset);
-		printf("\t\tDVO port: 0x%02x\n", child->dvo_port);
-	} else { /* 152+ have EFP blocks here */
-		printf("\tEFP device info:\n");
-		printf("\t\tDevice handle: 0x%04x (%s)\n", efp->handle,
-		       child_device_handle(efp->handle));
-		printf("\t\tDevice type: 0x%04x (%s)\n", efp->device_type,
-		       child_device_type(efp->device_type));
-		dump_child_device_type_bits(efp->device_type);
-		printf("\t\tI2C speed: 0x%02x\n", efp->i2c_speed);
-		printf("\t\tDP onboard redriver: 0x%02x\n", efp->dp_onboard_redriver);
-		printf("\t\tDP ondock redriver: 0x%02x\n", efp->dp_ondock_redriver);
-		printf("\t\tHDMI max data rate: 0x%02x\n", efp->hdmi_max_data_rate);
-		printf("\t\tHDMI level shifter value: 0x%02x\n", efp->hdmi_level_shifter_value);
-		printf("\t\tOffset to DTD buffer for edidless EFP: 0x%02x\n", efp->dtd_buf_ptr);
-		printf("\t\tDual pipe ganged eDP: %s\n", YESNO(efp->ganged_edp));
-		printf("\t\tCompression method CPS: %s\n", YESNO(efp->compression_method));
-		printf("\t\tCompression enable: %s\n", YESNO(efp->compression_enable));
-		printf("\t\tEdidless EFP: %s\n", YESNO(efp->edidless_efp));
-		printf("\t\tCompression structure index: 0x%02x)\n", efp->compression_structure_index);
-		printf("\t\tSlave DDI port: 0x%02x (%s)\n", efp->slave_port, dvo_port(efp->slave_port));
-		printf("\t\tAIM offset: %d\n", child->addin_offset);
-		printf("\t\tPort: 0x%02x (%s)\n", efp->dvo_port, dvo_port(efp->dvo_port));
-		printf("\t\tAIM I2C pin: 0x%02x\n", efp->i2c_pin);
-		printf("\t\tAIM Slave address: 0x%02x\n", efp->slave_addr);
-		printf("\t\tDDC pin: 0x%02x\n", efp->ddc_pin);
-		printf("\t\tEDID buffer ptr: 0x%02x\n", efp->edid_ptr);
-		printf("\t\tDVO config: 0x%02x\n", efp->dvo_cfg);
-		printf("\t\tHPD sense invert: %s\n", YESNO(efp->hpd_invert));
-		printf("\t\tIboost enable: %s\n", YESNO(efp->iboost));
-		printf("\t\tOnboard LSPCON: %s\n", YESNO(efp->lspcon));
-		printf("\t\tLane reversal: %s\n", YESNO(efp->lane_reversal));
-		printf("\t\tEFP routed through dock: %s\n", YESNO(efp->efp_routed));
-		printf("\t\tHDMI compatible? %s\n", YESNO(efp->hdmi_support));
-		printf("\t\tDP compatible? %s\n", YESNO(efp->dp_support));
-		printf("\t\tTMDS compatible? %s\n", YESNO(efp->tmds_support));
-		printf("\t\tAux channel: 0x%02x\n", efp->aux_channel);
-		printf("\t\tDongle detect: 0x%02x\n", efp->dongle_detect);
-		printf("\t\tIntegrated encoder instead of SDVO: %s\n", YESNO(efp->integrated_encoder));
-		printf("\t\tHotplu connect status: 0x%02x\n", efp->hpd_status);
-		printf("\t\tSDVO stall signal available: %s\n", YESNO(efp->sdvo_stall));
-		printf("\t\tPipe capabilities: 0x%02x\n", efp->pipe_cap);
-		printf("\t\tDVO wiring: 0x%02x\n", efp->dvo_wiring);
-		printf("\t\tMIPI bridge type: %02x (%s)\n", efp->mipi_bridge_type,
-		       mipi_bridge_type(efp->mipi_bridge_type));
-		printf("\t\tDevice class extendsion: 0x%02x\n", efp->extended_type);
-		printf("\t\tDVO function: 0x%02x\n", efp->dvo_function);
+	} else {
+		printf("\t\tI2C speed: 0x%02x\n", child->i2c_speed);
+		printf("\t\tDP onboard redriver: 0x%02x\n", child->dp_onboard_redriver);
+		printf("\t\tDP ondock redriver: 0x%02x\n", child->dp_ondock_redriver);
+		printf("\t\tHDMI level shifter value: 0x%02x\n", child->hdmi_level_shifter_value);
+		printf("\t\tHDMI max data rate: 0x%02x\n", child->hdmi_max_data_rate);
+		printf("\t\tOffset to DTD buffer for edidless CHILD: 0x%02x\n", child->dtd_buf_ptr);
+		printf("\t\tEdidless EFP: %s\n", YESNO(child->edidless_efp));
+		printf("\t\tCompression enable: %s\n", YESNO(child->compression_enable));
+		printf("\t\tCompression method CPS: %s\n", YESNO(child->compression_method));
+		printf("\t\tDual pipe ganged eDP: %s\n", YESNO(child->ganged_edp));
+		printf("\t\tCompression structure index: 0x%02x)\n", child->compression_structure_index);
+		printf("\t\tSlave DDI port: 0x%02x (%s)\n", child->slave_port, dvo_port(child->slave_port));
+	}
+
+	printf("\t\tAIM offset: %d\n", child->addin_offset);
+	printf("\t\tDVO Port: 0x%02x (%s)\n", child->dvo_port, dvo_port(child->dvo_port));
+
+	if (context->bdb->version < 152)
+		return;
+
+	printf("\t\tAIM I2C pin: 0x%02x\n", child->i2c_pin);
+	printf("\t\tAIM Slave address: 0x%02x\n", child->slave_addr);
+	printf("\t\tDDC pin: 0x%02x\n", child->ddc_pin);
+	printf("\t\tEDID buffer ptr: 0x%02x\n", child->edid_ptr);
+	printf("\t\tDVO config: 0x%02x\n", child->dvo_cfg);
+
+	if (context->bdb->version < 158) {
+		printf("\t\tDVO2 Port: 0x%02x (%s)\n", child->dvo2_port, dvo_port(child->dvo2_port));
+		printf("\t\tI2C2 pin: 0x%02x\n", child->i2c2_pin);
+		printf("\t\tSlave2 address: 0x%02x\n", child->slave2_addr);
+		printf("\t\tDDC2 pin: 0x%02x\n", child->ddc2_pin);
+	} else {
+		printf("\t\tEFP routed through dock: %s\n", YESNO(child->efp_routed));
+		printf("\t\tLane reversal: %s\n", YESNO(child->lane_reversal));
+		printf("\t\tOnboard LSPCON: %s\n", YESNO(child->lspcon));
+		printf("\t\tIboost enable: %s\n", YESNO(child->iboost));
+		printf("\t\tHPD sense invert: %s\n", YESNO(child->hpd_invert));
+		printf("\t\tHDMI compatible? %s\n", YESNO(child->hdmi_support));
+		printf("\t\tDP compatible? %s\n", YESNO(child->dp_support));
+		printf("\t\tTMDS compatible? %s\n", YESNO(child->tmds_support));
+		printf("\t\tAux channel: 0x%02x\n", child->aux_channel);
+		printf("\t\tDongle detect: 0x%02x\n", child->dongle_detect);
+	}
+
+	printf("\t\tPipe capabilities: 0x%02x\n", child->pipe_cap);
+	printf("\t\tSDVO stall signal available: %s\n", YESNO(child->sdvo_stall));
+	printf("\t\tHotplu connect status: 0x%02x\n", child->hpd_status);
+	printf("\t\tIntegrated encoder instead of SDVO: %s\n", YESNO(child->integrated_encoder));
+	printf("\t\tDVO wiring: 0x%02x\n", child->dvo_wiring);
+
+	if (context->bdb->version < 171) {
+		printf("\t\tDVO2 wiring: 0x%02x\n", child->dvo2_wiring);
+	} else {
+		printf("\t\tMIPI bridge type: %02x (%s)\n", child->mipi_bridge_type,
+		       mipi_bridge_type(child->mipi_bridge_type));
 	}
 
+	printf("\t\tDevice class extendsion: 0x%02x\n", child->extended_type);
+	printf("\t\tDVO function: 0x%02x\n", child->dvo_function);
+
 	if (context->bdb->version >= 195) {
-		printf("\t\tDP USB type C support: %s\n", YESNO(efp->dp_usb_type_c));
-		printf("\t\t2X DP GPIO index: 0x%02x\n", efp->dp_gpio_index);
-		printf("\t\t2X DP GPIO pin number: 0x%02x\n", efp->dp_gpio_pin_num);
+		printf("\t\tDP USB type C support: %s\n", YESNO(child->dp_usb_type_c));
+		printf("\t\t2X DP GPIO index: 0x%02x\n", child->dp_gpio_index);
+		printf("\t\t2X DP GPIO pin number: 0x%02x\n", child->dp_gpio_pin_num);
 	}
 
 	if (context->bdb->version >= 196) {
-		printf("\t\tIBoost level for HDMI: 0x%02x\n", efp->hdmi_iboost_level);
-		printf("\t\tIBoost level for DP/eDP: 0x%02x\n", efp->dp_iboost_level);
+		printf("\t\tIBoost level for HDMI: 0x%02x\n", child->hdmi_iboost_level);
+		printf("\t\tIBoost level for DP/eDP: 0x%02x\n", child->dp_iboost_level);
 	}
 }
 
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH i-g-t 6/8] tools/intel_vbt_decode: unify legacy child device block dumping
  2017-10-19 15:22 [PATCH i-g-t 0/8] tools/intel_vbt_decode: refactoring and cleanups Jani Nikula
                   ` (4 preceding siblings ...)
  2017-10-19 15:22 ` [PATCH i-g-t 5/8] tools/intel_vbt_decode: unify child device printing across versions Jani Nikula
@ 2017-10-19 15:22 ` Jani Nikula
  2017-10-19 15:22 ` [PATCH i-g-t 7/8] tools/intel_vbt_decode: dump more child device data for version < 152 Jani Nikula
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Jani Nikula @ 2017-10-19 15:22 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

It's the same stuff as in the new child devices.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 tools/intel_bios.h       | 30 +++---------------------------
 tools/intel_vbt_decode.c | 37 +++++++++++++++++++++----------------
 2 files changed, 24 insertions(+), 43 deletions(-)

diff --git a/tools/intel_bios.h b/tools/intel_bios.h
index f0475b5cfcc3..78a96d977536 100644
--- a/tools/intel_bios.h
+++ b/tools/intel_bios.h
@@ -42,33 +42,9 @@
 #define DEVICE_TYPE_DVI			0x68d2
 #define DEVICE_TYPE_MIPI		0x7cc2
 
-struct legacy_child_device_config {
-	uint16_t handle;
-	uint16_t device_type;	/* See DEVICE_TYPE_* above */
-	uint8_t device_id[10];
-	uint16_t addin_offset;
-	uint8_t dvo_port;	/* See DEVICE_PORT_* above */
-	uint8_t i2c_pin;
-	uint8_t slave_addr;
-	uint8_t ddc_pin;
-	uint16_t edid_ptr;
-	uint8_t dvo_cfg;	/* See DEVICE_CFG_* above */
-	uint8_t dvo2_port;
-	uint8_t i2c2_pin;
-	uint8_t slave2_addr;
-	uint8_t ddc2_pin;
-	uint8_t capabilities;
-	uint8_t dvo_wiring;	/* See DEVICE_WIRE_* above */
-	uint8_t dvo2_wiring;
-	uint16_t extended_type;
-	uint8_t dvo_function;
-} __attribute__ ((packed));
-
-#define DEVICE_CHILD_SIZE 7
-
-struct bdb_child_devices {
-	uint8_t child_structure_size;
-	struct legacy_child_device_config children[DEVICE_CHILD_SIZE];
+struct bdb_legacy_child_devices {
+	uint8_t child_dev_size;
+	uint8_t devices[0]; /* presumably 7 * 33 */
 } __attribute__ ((packed));
 
 #define BDB_DRIVER_NO_LVDS	0
diff --git a/tools/intel_vbt_decode.c b/tools/intel_vbt_decode.c
index bc5f38112619..3cce60bf2ee2 100644
--- a/tools/intel_vbt_decode.c
+++ b/tools/intel_vbt_decode.c
@@ -526,25 +526,30 @@ static void dump_general_definitions(struct context *context,
 static void dump_legacy_child_devices(struct context *context,
 				      const struct bdb_block *block)
 {
-	const struct bdb_child_devices *child_devs = block->data;
-	const struct legacy_child_device_config *child;
+	const struct bdb_legacy_child_devices *defs = block->data;
+	struct child_device_config *child;
 	int i;
+	int child_device_num;
 
-	for (i = 0; i < DEVICE_CHILD_SIZE; i++) {
-		child = &child_devs->children[i];
-		/* Skip nonexistent children */
-		if (!child->device_type)
-			continue;
-		printf("\tChild device %d\n", i);
-		printf("\t\tType: 0x%04x (%s)\n", child->device_type,
-		       child_device_type(child->device_type));
-		printf("\t\tDVO port: 0x%02x\n", child->dvo_port);
-		printf("\t\tI2C pin: 0x%02x\n", child->i2c_pin);
-		printf("\t\tSlave addr: 0x%02x\n", child->slave_addr);
-		printf("\t\tDDC pin: 0x%02x\n", child->ddc_pin);
-		printf("\t\tDVO config: 0x%02x\n", child->dvo_cfg);
-		printf("\t\tDVO wiring: 0x%02x\n", child->dvo_wiring);
+	printf("\tChild device size: %d\n", defs->child_dev_size);
+	child_device_num = (block->size - sizeof(*defs)) /
+		defs->child_dev_size;
+
+	/*
+	 * Use a temp buffer so dump_child_device() doesn't have to worry about
+	 * accessing the struct beyond child_dev_size. The tail, if any, remains
+	 * initialized to zero.
+	 */
+	child = calloc(1, sizeof(*child));
+
+	for (i = 0; i < child_device_num; i++) {
+		memcpy(child, &defs->devices[i * defs->child_dev_size],
+		       min(sizeof(*child), defs->child_dev_size));
+
+		dump_child_device(context, child);
 	}
+
+	free(child);
 }
 
 static void dump_lvds_options(struct context *context,
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH i-g-t 7/8] tools/intel_vbt_decode: dump more child device data for version < 152
  2017-10-19 15:22 [PATCH i-g-t 0/8] tools/intel_vbt_decode: refactoring and cleanups Jani Nikula
                   ` (5 preceding siblings ...)
  2017-10-19 15:22 ` [PATCH i-g-t 6/8] tools/intel_vbt_decode: unify legacy child device block dumping Jani Nikula
@ 2017-10-19 15:22 ` Jani Nikula
  2017-10-19 15:22 ` [PATCH i-g-t 8/8] tools/intel_vbt_decode: abstract child devices printing more Jani Nikula
  2017-10-19 16:52 ` ✗ Fi.CI.BAT: failure for tools/intel_vbt_decode: refactoring and cleanups Patchwork
  8 siblings, 0 replies; 15+ messages in thread
From: Jani Nikula @ 2017-10-19 15:22 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

There's no evidence that this is the limit.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 tools/intel_vbt_decode.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tools/intel_vbt_decode.c b/tools/intel_vbt_decode.c
index 3cce60bf2ee2..ebc65b82bbde 100644
--- a/tools/intel_vbt_decode.c
+++ b/tools/intel_vbt_decode.c
@@ -431,9 +431,6 @@ static void dump_child_device(struct context *context,
 	printf("\t\tAIM offset: %d\n", child->addin_offset);
 	printf("\t\tDVO Port: 0x%02x (%s)\n", child->dvo_port, dvo_port(child->dvo_port));
 
-	if (context->bdb->version < 152)
-		return;
-
 	printf("\t\tAIM I2C pin: 0x%02x\n", child->i2c_pin);
 	printf("\t\tAIM Slave address: 0x%02x\n", child->slave_addr);
 	printf("\t\tDDC pin: 0x%02x\n", child->ddc_pin);
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH i-g-t 8/8] tools/intel_vbt_decode: abstract child devices printing more
  2017-10-19 15:22 [PATCH i-g-t 0/8] tools/intel_vbt_decode: refactoring and cleanups Jani Nikula
                   ` (6 preceding siblings ...)
  2017-10-19 15:22 ` [PATCH i-g-t 7/8] tools/intel_vbt_decode: dump more child device data for version < 152 Jani Nikula
@ 2017-10-19 15:22 ` Jani Nikula
  2017-10-19 16:52 ` ✗ Fi.CI.BAT: failure for tools/intel_vbt_decode: refactoring and cleanups Patchwork
  8 siblings, 0 replies; 15+ messages in thread
From: Jani Nikula @ 2017-10-19 15:22 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Unify the common code for current and legacy blocks.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 tools/intel_vbt_decode.c | 70 +++++++++++++++++++++---------------------------
 1 file changed, 31 insertions(+), 39 deletions(-)

diff --git a/tools/intel_vbt_decode.c b/tools/intel_vbt_decode.c
index ebc65b82bbde..04f733c8cb06 100644
--- a/tools/intel_vbt_decode.c
+++ b/tools/intel_vbt_decode.c
@@ -483,25 +483,12 @@ static void dump_child_device(struct context *context,
 	}
 }
 
-static void dump_general_definitions(struct context *context,
-				     const struct bdb_block *block)
+
+static void dump_child_devices(struct context *context, const uint8_t *devices,
+			       uint8_t child_dev_num, uint8_t child_dev_size)
 {
-	const struct bdb_general_definitions *defs = block->data;
 	struct child_device_config *child;
 	int i;
-	int child_device_num;
-
-	printf("\tCRT DDC GMBUS addr: 0x%02x\n", defs->crt_ddc_gmbus_pin);
-	printf("\tUse ACPI DPMS CRT power states: %s\n",
-	       YESNO(defs->dpms_acpi));
-	printf("\tSkip CRT detect at boot: %s\n",
-	       YESNO(defs->skip_boot_crt_detect));
-	printf("\tUse DPMS on AIM devices: %s\n", YESNO(defs->dpms_aim));
-	printf("\tBoot display type: 0x%02x%02x\n", defs->boot_display[1],
-	       defs->boot_display[0]);
-	printf("\tChild device size: %d\n", defs->child_dev_size);
-	child_device_num = (block->size - sizeof(*defs)) /
-		defs->child_dev_size;
 
 	/*
 	 * Use a temp buffer so dump_child_device() doesn't have to worry about
@@ -510,9 +497,9 @@ static void dump_general_definitions(struct context *context,
 	 */
 	child = calloc(1, sizeof(*child));
 
-	for (i = 0; i < child_device_num; i++) {
-		memcpy(child, &defs->devices[i * defs->child_dev_size],
-		       min(sizeof(*child), defs->child_dev_size));
+	for (i = 0; i < child_dev_num; i++) {
+		memcpy(child, devices + i * child_dev_size,
+		       min(sizeof(*child), child_dev_size));
 
 		dump_child_device(context, child);
 	}
@@ -520,33 +507,38 @@ static void dump_general_definitions(struct context *context,
 	free(child);
 }
 
-static void dump_legacy_child_devices(struct context *context,
-				      const struct bdb_block *block)
+static void dump_general_definitions(struct context *context,
+				     const struct bdb_block *block)
 {
-	const struct bdb_legacy_child_devices *defs = block->data;
-	struct child_device_config *child;
-	int i;
-	int child_device_num;
+	const struct bdb_general_definitions *defs = block->data;
+	int child_dev_num;
 
+	printf("\tCRT DDC GMBUS addr: 0x%02x\n", defs->crt_ddc_gmbus_pin);
+	printf("\tUse ACPI DPMS CRT power states: %s\n",
+	       YESNO(defs->dpms_acpi));
+	printf("\tSkip CRT detect at boot: %s\n",
+	       YESNO(defs->skip_boot_crt_detect));
+	printf("\tUse DPMS on AIM devices: %s\n", YESNO(defs->dpms_aim));
+	printf("\tBoot display type: 0x%02x%02x\n", defs->boot_display[1],
+	       defs->boot_display[0]);
 	printf("\tChild device size: %d\n", defs->child_dev_size);
-	child_device_num = (block->size - sizeof(*defs)) /
-		defs->child_dev_size;
 
-	/*
-	 * Use a temp buffer so dump_child_device() doesn't have to worry about
-	 * accessing the struct beyond child_dev_size. The tail, if any, remains
-	 * initialized to zero.
-	 */
-	child = calloc(1, sizeof(*child));
+	child_dev_num = (block->size - sizeof(*defs)) / defs->child_dev_size;
+	dump_child_devices(context, defs->devices,
+			   child_dev_num, defs->child_dev_size);
+}
 
-	for (i = 0; i < child_device_num; i++) {
-		memcpy(child, &defs->devices[i * defs->child_dev_size],
-		       min(sizeof(*child), defs->child_dev_size));
+static void dump_legacy_child_devices(struct context *context,
+				      const struct bdb_block *block)
+{
+	const struct bdb_legacy_child_devices *defs = block->data;
+	int child_dev_num;
 
-		dump_child_device(context, child);
-	}
+	printf("\tChild device size: %d\n", defs->child_dev_size);
 
-	free(child);
+	child_dev_num = (block->size - sizeof(*defs)) / defs->child_dev_size;
+	dump_child_devices(context, defs->devices,
+			   child_dev_num, defs->child_dev_size);
 }
 
 static void dump_lvds_options(struct context *context,
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* ✗ Fi.CI.BAT: failure for tools/intel_vbt_decode: refactoring and cleanups
  2017-10-19 15:22 [PATCH i-g-t 0/8] tools/intel_vbt_decode: refactoring and cleanups Jani Nikula
                   ` (7 preceding siblings ...)
  2017-10-19 15:22 ` [PATCH i-g-t 8/8] tools/intel_vbt_decode: abstract child devices printing more Jani Nikula
@ 2017-10-19 16:52 ` Patchwork
  8 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2017-10-19 16:52 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: tools/intel_vbt_decode: refactoring and cleanups
URL   : https://patchwork.freedesktop.org/series/32302/
State : failure

== Summary ==

IGT patchset tested on top of latest successful build
abc08cba366a64a07f7f4deb167ae7d6ae059958 lib: Free all internal buffers before measuring available memory

with latest DRM-Tip kernel build CI_DRM_3266
9024f1a2827a drm-tip: 2017y-10m-19d-12h-57m-03s UTC integration manifest

No testlist changes.

Test chamelium:
        Subgroup dp-crc-fast:
                pass       -> FAIL       (fi-kbl-7500u) fdo#102514
Test kms_busy:
        Subgroup basic-flip-b:
                fail       -> PASS       (fi-gdg-551) fdo#102654
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-legacy:
                fail       -> PASS       (fi-gdg-551) fdo#102618
        Subgroup basic-flip-after-cursor-varying-size:
                incomplete -> PASS       (fi-skl-6260u)
Test kms_flip:
        Subgroup basic-plain-flip:
                pass       -> INCOMPLETE (fi-skl-6700hq)

fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#102654 https://bugs.freedesktop.org/show_bug.cgi?id=102654
fdo#102618 https://bugs.freedesktop.org/show_bug.cgi?id=102618

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:444s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:458s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:375s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:539s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:267s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:502s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:500s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:495s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:489s
fi-cfl-s         total:289  pass:253  dwarn:4   dfail:0   fail:0   skip:32  time:546s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:412s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:253s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:579s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:451s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:428s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:437s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:482s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:461s
fi-kbl-7500u     total:289  pass:263  dwarn:1   dfail:0   fail:1   skip:24  time:483s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:572s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:477s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:585s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:545s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:453s
fi-skl-6700hq    total:220  pass:199  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:515s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:506s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:454s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:569s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:425s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_389/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH i-g-t 5/8] tools/intel_vbt_decode: unify child device printing across versions
  2017-10-19 15:22 ` [PATCH i-g-t 5/8] tools/intel_vbt_decode: unify child device printing across versions Jani Nikula
@ 2017-10-19 17:24   ` Ville Syrjälä
  2017-10-20 10:56     ` Jani Nikula
  0 siblings, 1 reply; 15+ messages in thread
From: Ville Syrjälä @ 2017-10-19 17:24 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Thu, Oct 19, 2017 at 06:22:56PM +0300, Jani Nikula wrote:
> Make it easier to compare dumping against the struct definition.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  tools/intel_vbt_decode.c | 128 ++++++++++++++++++++++++++---------------------
>  1 file changed, 71 insertions(+), 57 deletions(-)
> 
> diff --git a/tools/intel_vbt_decode.c b/tools/intel_vbt_decode.c
> index 69cf1e4c6cc6..bc5f38112619 100644
> --- a/tools/intel_vbt_decode.c
> +++ b/tools/intel_vbt_decode.c
> @@ -401,74 +401,88 @@ static const char *mipi_bridge_type(uint8_t type)
>  static void dump_child_device(struct context *context,
>  			      const struct child_device_config *child)
>  {
> -	const struct child_device_config *efp = child;
> -
>  	if (!child->device_type)
>  		return;
>  
> +	printf("\tChild device info:\n");
> +	printf("\t\tDevice handle: 0x%04x (%s)\n", child->handle,
> +	       child_device_handle(child->handle));
> +	printf("\t\tDevice type: 0x%04x (%s)\n", child->device_type,
> +	       child_device_type(child->device_type));
> +	dump_child_device_type_bits(child->device_type);
> +
>  	if (context->bdb->version < 152) {
> -		printf("\tChild device info:\n");
> -		printf("\t\tDevice type: %04x (%s)\n", child->device_type,
> -		       child_device_type(child->device_type));
>  		printf("\t\tSignature: %.*s\n", (int)sizeof(child->device_id), child->device_id);
> -		printf("\t\tAIM offset: %d\n", child->addin_offset);
> -		printf("\t\tDVO port: 0x%02x\n", child->dvo_port);
> -	} else { /* 152+ have EFP blocks here */
> -		printf("\tEFP device info:\n");
> -		printf("\t\tDevice handle: 0x%04x (%s)\n", efp->handle,
> -		       child_device_handle(efp->handle));
> -		printf("\t\tDevice type: 0x%04x (%s)\n", efp->device_type,
> -		       child_device_type(efp->device_type));
> -		dump_child_device_type_bits(efp->device_type);
> -		printf("\t\tI2C speed: 0x%02x\n", efp->i2c_speed);
> -		printf("\t\tDP onboard redriver: 0x%02x\n", efp->dp_onboard_redriver);
> -		printf("\t\tDP ondock redriver: 0x%02x\n", efp->dp_ondock_redriver);
> -		printf("\t\tHDMI max data rate: 0x%02x\n", efp->hdmi_max_data_rate);
> -		printf("\t\tHDMI level shifter value: 0x%02x\n", efp->hdmi_level_shifter_value);
> -		printf("\t\tOffset to DTD buffer for edidless EFP: 0x%02x\n", efp->dtd_buf_ptr);
> -		printf("\t\tDual pipe ganged eDP: %s\n", YESNO(efp->ganged_edp));
> -		printf("\t\tCompression method CPS: %s\n", YESNO(efp->compression_method));
> -		printf("\t\tCompression enable: %s\n", YESNO(efp->compression_enable));
> -		printf("\t\tEdidless EFP: %s\n", YESNO(efp->edidless_efp));
> -		printf("\t\tCompression structure index: 0x%02x)\n", efp->compression_structure_index);
> -		printf("\t\tSlave DDI port: 0x%02x (%s)\n", efp->slave_port, dvo_port(efp->slave_port));
> -		printf("\t\tAIM offset: %d\n", child->addin_offset);
> -		printf("\t\tPort: 0x%02x (%s)\n", efp->dvo_port, dvo_port(efp->dvo_port));
> -		printf("\t\tAIM I2C pin: 0x%02x\n", efp->i2c_pin);
> -		printf("\t\tAIM Slave address: 0x%02x\n", efp->slave_addr);
> -		printf("\t\tDDC pin: 0x%02x\n", efp->ddc_pin);
> -		printf("\t\tEDID buffer ptr: 0x%02x\n", efp->edid_ptr);
> -		printf("\t\tDVO config: 0x%02x\n", efp->dvo_cfg);
> -		printf("\t\tHPD sense invert: %s\n", YESNO(efp->hpd_invert));
> -		printf("\t\tIboost enable: %s\n", YESNO(efp->iboost));
> -		printf("\t\tOnboard LSPCON: %s\n", YESNO(efp->lspcon));
> -		printf("\t\tLane reversal: %s\n", YESNO(efp->lane_reversal));
> -		printf("\t\tEFP routed through dock: %s\n", YESNO(efp->efp_routed));
> -		printf("\t\tHDMI compatible? %s\n", YESNO(efp->hdmi_support));
> -		printf("\t\tDP compatible? %s\n", YESNO(efp->dp_support));
> -		printf("\t\tTMDS compatible? %s\n", YESNO(efp->tmds_support));
> -		printf("\t\tAux channel: 0x%02x\n", efp->aux_channel);
> -		printf("\t\tDongle detect: 0x%02x\n", efp->dongle_detect);
> -		printf("\t\tIntegrated encoder instead of SDVO: %s\n", YESNO(efp->integrated_encoder));
> -		printf("\t\tHotplu connect status: 0x%02x\n", efp->hpd_status);
> -		printf("\t\tSDVO stall signal available: %s\n", YESNO(efp->sdvo_stall));
> -		printf("\t\tPipe capabilities: 0x%02x\n", efp->pipe_cap);
> -		printf("\t\tDVO wiring: 0x%02x\n", efp->dvo_wiring);
> -		printf("\t\tMIPI bridge type: %02x (%s)\n", efp->mipi_bridge_type,
> -		       mipi_bridge_type(efp->mipi_bridge_type));
> -		printf("\t\tDevice class extendsion: 0x%02x\n", efp->extended_type);
> -		printf("\t\tDVO function: 0x%02x\n", efp->dvo_function);
> +	} else {
> +		printf("\t\tI2C speed: 0x%02x\n", child->i2c_speed);
> +		printf("\t\tDP onboard redriver: 0x%02x\n", child->dp_onboard_redriver);
> +		printf("\t\tDP ondock redriver: 0x%02x\n", child->dp_ondock_redriver);
> +		printf("\t\tHDMI level shifter value: 0x%02x\n", child->hdmi_level_shifter_value);
> +		printf("\t\tHDMI max data rate: 0x%02x\n", child->hdmi_max_data_rate);
> +		printf("\t\tOffset to DTD buffer for edidless CHILD: 0x%02x\n", child->dtd_buf_ptr);
> +		printf("\t\tEdidless EFP: %s\n", YESNO(child->edidless_efp));
> +		printf("\t\tCompression enable: %s\n", YESNO(child->compression_enable));
> +		printf("\t\tCompression method CPS: %s\n", YESNO(child->compression_method));
> +		printf("\t\tDual pipe ganged eDP: %s\n", YESNO(child->ganged_edp));
> +		printf("\t\tCompression structure index: 0x%02x)\n", child->compression_structure_index);
> +		printf("\t\tSlave DDI port: 0x%02x (%s)\n", child->slave_port, dvo_port(child->slave_port));
> +	}
> +
> +	printf("\t\tAIM offset: %d\n", child->addin_offset);
> +	printf("\t\tDVO Port: 0x%02x (%s)\n", child->dvo_port, dvo_port(child->dvo_port));
> +
> +	if (context->bdb->version < 152)
> +		return;
> +
> +	printf("\t\tAIM I2C pin: 0x%02x\n", child->i2c_pin);
> +	printf("\t\tAIM Slave address: 0x%02x\n", child->slave_addr);
> +	printf("\t\tDDC pin: 0x%02x\n", child->ddc_pin);
> +	printf("\t\tEDID buffer ptr: 0x%02x\n", child->edid_ptr);
> +	printf("\t\tDVO config: 0x%02x\n", child->dvo_cfg);
> +
> +	if (context->bdb->version < 158) {
> +		printf("\t\tDVO2 Port: 0x%02x (%s)\n", child->dvo2_port, dvo_port(child->dvo2_port));
> +		printf("\t\tI2C2 pin: 0x%02x\n", child->i2c2_pin);
> +		printf("\t\tSlave2 address: 0x%02x\n", child->slave2_addr);
> +		printf("\t\tDDC2 pin: 0x%02x\n", child->ddc2_pin);
> +	} else {
> +		printf("\t\tEFP routed through dock: %s\n", YESNO(child->efp_routed));
> +		printf("\t\tLane reversal: %s\n", YESNO(child->lane_reversal));
> +		printf("\t\tOnboard LSPCON: %s\n", YESNO(child->lspcon));
> +		printf("\t\tIboost enable: %s\n", YESNO(child->iboost));
> +		printf("\t\tHPD sense invert: %s\n", YESNO(child->hpd_invert));
> +		printf("\t\tHDMI compatible? %s\n", YESNO(child->hdmi_support));
> +		printf("\t\tDP compatible? %s\n", YESNO(child->dp_support));
> +		printf("\t\tTMDS compatible? %s\n", YESNO(child->tmds_support));
> +		printf("\t\tAux channel: 0x%02x\n", child->aux_channel);
> +		printf("\t\tDongle detect: 0x%02x\n", child->dongle_detect);

The version of the spec I have at hand says aux and dongle are
already part of version 155.

> +	}
> +
> +	printf("\t\tPipe capabilities: 0x%02x\n", child->pipe_cap);
> +	printf("\t\tSDVO stall signal available: %s\n", YESNO(child->sdvo_stall));
> +	printf("\t\tHotplu connect status: 0x%02x\n", child->hpd_status);
                          ^
Pre-existing typo

> +	printf("\t\tIntegrated encoder instead of SDVO: %s\n", YESNO(child->integrated_encoder));
> +	printf("\t\tDVO wiring: 0x%02x\n", child->dvo_wiring);
> +
> +	if (context->bdb->version < 171) {
> +		printf("\t\tDVO2 wiring: 0x%02x\n", child->dvo2_wiring);
> +	} else {
> +		printf("\t\tMIPI bridge type: %02x (%s)\n", child->mipi_bridge_type,
> +		       mipi_bridge_type(child->mipi_bridge_type));
>  	}
>  
> +	printf("\t\tDevice class extendsion: 0x%02x\n", child->extended_type);
                                      ^
ditto

> +	printf("\t\tDVO function: 0x%02x\n", child->dvo_function);
> +
>  	if (context->bdb->version >= 195) {
> -		printf("\t\tDP USB type C support: %s\n", YESNO(efp->dp_usb_type_c));
> -		printf("\t\t2X DP GPIO index: 0x%02x\n", efp->dp_gpio_index);
> -		printf("\t\t2X DP GPIO pin number: 0x%02x\n", efp->dp_gpio_pin_num);
> +		printf("\t\tDP USB type C support: %s\n", YESNO(child->dp_usb_type_c));
> +		printf("\t\t2X DP GPIO index: 0x%02x\n", child->dp_gpio_index);
> +		printf("\t\t2X DP GPIO pin number: 0x%02x\n", child->dp_gpio_pin_num);
>  	}
>  
>  	if (context->bdb->version >= 196) {
> -		printf("\t\tIBoost level for HDMI: 0x%02x\n", efp->hdmi_iboost_level);
> -		printf("\t\tIBoost level for DP/eDP: 0x%02x\n", efp->dp_iboost_level);
> +		printf("\t\tIBoost level for HDMI: 0x%02x\n", child->hdmi_iboost_level);
> +		printf("\t\tIBoost level for DP/eDP: 0x%02x\n", child->dp_iboost_level);
>  	}
>  }
>  
> -- 
> 2.11.0

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH i-g-t 5/8] tools/intel_vbt_decode: unify child device printing across versions
  2017-10-19 17:24   ` Ville Syrjälä
@ 2017-10-20 10:56     ` Jani Nikula
  2017-10-20 11:33       ` Ville Syrjälä
  0 siblings, 1 reply; 15+ messages in thread
From: Jani Nikula @ 2017-10-20 10:56 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Thu, 19 Oct 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Oct 19, 2017 at 06:22:56PM +0300, Jani Nikula wrote:
>> Make it easier to compare dumping against the struct definition.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  tools/intel_vbt_decode.c | 128 ++++++++++++++++++++++++++---------------------
>>  1 file changed, 71 insertions(+), 57 deletions(-)
>> 
>> diff --git a/tools/intel_vbt_decode.c b/tools/intel_vbt_decode.c
>> index 69cf1e4c6cc6..bc5f38112619 100644
>> --- a/tools/intel_vbt_decode.c
>> +++ b/tools/intel_vbt_decode.c
>> @@ -401,74 +401,88 @@ static const char *mipi_bridge_type(uint8_t type)
>>  static void dump_child_device(struct context *context,
>>  			      const struct child_device_config *child)
>>  {
>> -	const struct child_device_config *efp = child;
>> -
>>  	if (!child->device_type)
>>  		return;
>>  
>> +	printf("\tChild device info:\n");
>> +	printf("\t\tDevice handle: 0x%04x (%s)\n", child->handle,
>> +	       child_device_handle(child->handle));
>> +	printf("\t\tDevice type: 0x%04x (%s)\n", child->device_type,
>> +	       child_device_type(child->device_type));
>> +	dump_child_device_type_bits(child->device_type);
>> +
>>  	if (context->bdb->version < 152) {
>> -		printf("\tChild device info:\n");
>> -		printf("\t\tDevice type: %04x (%s)\n", child->device_type,
>> -		       child_device_type(child->device_type));
>>  		printf("\t\tSignature: %.*s\n", (int)sizeof(child->device_id), child->device_id);
>> -		printf("\t\tAIM offset: %d\n", child->addin_offset);
>> -		printf("\t\tDVO port: 0x%02x\n", child->dvo_port);
>> -	} else { /* 152+ have EFP blocks here */
>> -		printf("\tEFP device info:\n");
>> -		printf("\t\tDevice handle: 0x%04x (%s)\n", efp->handle,
>> -		       child_device_handle(efp->handle));
>> -		printf("\t\tDevice type: 0x%04x (%s)\n", efp->device_type,
>> -		       child_device_type(efp->device_type));
>> -		dump_child_device_type_bits(efp->device_type);
>> -		printf("\t\tI2C speed: 0x%02x\n", efp->i2c_speed);
>> -		printf("\t\tDP onboard redriver: 0x%02x\n", efp->dp_onboard_redriver);
>> -		printf("\t\tDP ondock redriver: 0x%02x\n", efp->dp_ondock_redriver);
>> -		printf("\t\tHDMI max data rate: 0x%02x\n", efp->hdmi_max_data_rate);
>> -		printf("\t\tHDMI level shifter value: 0x%02x\n", efp->hdmi_level_shifter_value);
>> -		printf("\t\tOffset to DTD buffer for edidless EFP: 0x%02x\n", efp->dtd_buf_ptr);
>> -		printf("\t\tDual pipe ganged eDP: %s\n", YESNO(efp->ganged_edp));
>> -		printf("\t\tCompression method CPS: %s\n", YESNO(efp->compression_method));
>> -		printf("\t\tCompression enable: %s\n", YESNO(efp->compression_enable));
>> -		printf("\t\tEdidless EFP: %s\n", YESNO(efp->edidless_efp));
>> -		printf("\t\tCompression structure index: 0x%02x)\n", efp->compression_structure_index);
>> -		printf("\t\tSlave DDI port: 0x%02x (%s)\n", efp->slave_port, dvo_port(efp->slave_port));
>> -		printf("\t\tAIM offset: %d\n", child->addin_offset);
>> -		printf("\t\tPort: 0x%02x (%s)\n", efp->dvo_port, dvo_port(efp->dvo_port));
>> -		printf("\t\tAIM I2C pin: 0x%02x\n", efp->i2c_pin);
>> -		printf("\t\tAIM Slave address: 0x%02x\n", efp->slave_addr);
>> -		printf("\t\tDDC pin: 0x%02x\n", efp->ddc_pin);
>> -		printf("\t\tEDID buffer ptr: 0x%02x\n", efp->edid_ptr);
>> -		printf("\t\tDVO config: 0x%02x\n", efp->dvo_cfg);
>> -		printf("\t\tHPD sense invert: %s\n", YESNO(efp->hpd_invert));
>> -		printf("\t\tIboost enable: %s\n", YESNO(efp->iboost));
>> -		printf("\t\tOnboard LSPCON: %s\n", YESNO(efp->lspcon));
>> -		printf("\t\tLane reversal: %s\n", YESNO(efp->lane_reversal));
>> -		printf("\t\tEFP routed through dock: %s\n", YESNO(efp->efp_routed));
>> -		printf("\t\tHDMI compatible? %s\n", YESNO(efp->hdmi_support));
>> -		printf("\t\tDP compatible? %s\n", YESNO(efp->dp_support));
>> -		printf("\t\tTMDS compatible? %s\n", YESNO(efp->tmds_support));
>> -		printf("\t\tAux channel: 0x%02x\n", efp->aux_channel);
>> -		printf("\t\tDongle detect: 0x%02x\n", efp->dongle_detect);
>> -		printf("\t\tIntegrated encoder instead of SDVO: %s\n", YESNO(efp->integrated_encoder));
>> -		printf("\t\tHotplu connect status: 0x%02x\n", efp->hpd_status);
>> -		printf("\t\tSDVO stall signal available: %s\n", YESNO(efp->sdvo_stall));
>> -		printf("\t\tPipe capabilities: 0x%02x\n", efp->pipe_cap);
>> -		printf("\t\tDVO wiring: 0x%02x\n", efp->dvo_wiring);
>> -		printf("\t\tMIPI bridge type: %02x (%s)\n", efp->mipi_bridge_type,
>> -		       mipi_bridge_type(efp->mipi_bridge_type));
>> -		printf("\t\tDevice class extendsion: 0x%02x\n", efp->extended_type);
>> -		printf("\t\tDVO function: 0x%02x\n", efp->dvo_function);
>> +	} else {
>> +		printf("\t\tI2C speed: 0x%02x\n", child->i2c_speed);
>> +		printf("\t\tDP onboard redriver: 0x%02x\n", child->dp_onboard_redriver);
>> +		printf("\t\tDP ondock redriver: 0x%02x\n", child->dp_ondock_redriver);
>> +		printf("\t\tHDMI level shifter value: 0x%02x\n", child->hdmi_level_shifter_value);
>> +		printf("\t\tHDMI max data rate: 0x%02x\n", child->hdmi_max_data_rate);
>> +		printf("\t\tOffset to DTD buffer for edidless CHILD: 0x%02x\n", child->dtd_buf_ptr);
>> +		printf("\t\tEdidless EFP: %s\n", YESNO(child->edidless_efp));
>> +		printf("\t\tCompression enable: %s\n", YESNO(child->compression_enable));
>> +		printf("\t\tCompression method CPS: %s\n", YESNO(child->compression_method));
>> +		printf("\t\tDual pipe ganged eDP: %s\n", YESNO(child->ganged_edp));
>> +		printf("\t\tCompression structure index: 0x%02x)\n", child->compression_structure_index);
>> +		printf("\t\tSlave DDI port: 0x%02x (%s)\n", child->slave_port, dvo_port(child->slave_port));
>> +	}
>> +
>> +	printf("\t\tAIM offset: %d\n", child->addin_offset);
>> +	printf("\t\tDVO Port: 0x%02x (%s)\n", child->dvo_port, dvo_port(child->dvo_port));
>> +
>> +	if (context->bdb->version < 152)
>> +		return;
>> +
>> +	printf("\t\tAIM I2C pin: 0x%02x\n", child->i2c_pin);
>> +	printf("\t\tAIM Slave address: 0x%02x\n", child->slave_addr);
>> +	printf("\t\tDDC pin: 0x%02x\n", child->ddc_pin);
>> +	printf("\t\tEDID buffer ptr: 0x%02x\n", child->edid_ptr);
>> +	printf("\t\tDVO config: 0x%02x\n", child->dvo_cfg);
>> +
>> +	if (context->bdb->version < 158) {
>> +		printf("\t\tDVO2 Port: 0x%02x (%s)\n", child->dvo2_port, dvo_port(child->dvo2_port));
>> +		printf("\t\tI2C2 pin: 0x%02x\n", child->i2c2_pin);
>> +		printf("\t\tSlave2 address: 0x%02x\n", child->slave2_addr);
>> +		printf("\t\tDDC2 pin: 0x%02x\n", child->ddc2_pin);
>> +	} else {
>> +		printf("\t\tEFP routed through dock: %s\n", YESNO(child->efp_routed));
>> +		printf("\t\tLane reversal: %s\n", YESNO(child->lane_reversal));
>> +		printf("\t\tOnboard LSPCON: %s\n", YESNO(child->lspcon));
>> +		printf("\t\tIboost enable: %s\n", YESNO(child->iboost));
>> +		printf("\t\tHPD sense invert: %s\n", YESNO(child->hpd_invert));
>> +		printf("\t\tHDMI compatible? %s\n", YESNO(child->hdmi_support));
>> +		printf("\t\tDP compatible? %s\n", YESNO(child->dp_support));
>> +		printf("\t\tTMDS compatible? %s\n", YESNO(child->tmds_support));
>> +		printf("\t\tAux channel: 0x%02x\n", child->aux_channel);
>> +		printf("\t\tDongle detect: 0x%02x\n", child->dongle_detect);
>
> The version of the spec I have at hand says aux and dongle are
> already part of version 155.

Seems like dvo2/i2c2/slave2/ddc2 were converted to flags at 155. Okay if
I just change the condition above from < 158 to < 155?

BR,
Jani.


>
>> +	}
>> +
>> +	printf("\t\tPipe capabilities: 0x%02x\n", child->pipe_cap);
>> +	printf("\t\tSDVO stall signal available: %s\n", YESNO(child->sdvo_stall));
>> +	printf("\t\tHotplu connect status: 0x%02x\n", child->hpd_status);
>                           ^
> Pre-existing typo
>
>> +	printf("\t\tIntegrated encoder instead of SDVO: %s\n", YESNO(child->integrated_encoder));
>> +	printf("\t\tDVO wiring: 0x%02x\n", child->dvo_wiring);
>> +
>> +	if (context->bdb->version < 171) {
>> +		printf("\t\tDVO2 wiring: 0x%02x\n", child->dvo2_wiring);
>> +	} else {
>> +		printf("\t\tMIPI bridge type: %02x (%s)\n", child->mipi_bridge_type,
>> +		       mipi_bridge_type(child->mipi_bridge_type));
>>  	}
>>  
>> +	printf("\t\tDevice class extendsion: 0x%02x\n", child->extended_type);
>                                       ^
> ditto
>
>> +	printf("\t\tDVO function: 0x%02x\n", child->dvo_function);
>> +
>>  	if (context->bdb->version >= 195) {
>> -		printf("\t\tDP USB type C support: %s\n", YESNO(efp->dp_usb_type_c));
>> -		printf("\t\t2X DP GPIO index: 0x%02x\n", efp->dp_gpio_index);
>> -		printf("\t\t2X DP GPIO pin number: 0x%02x\n", efp->dp_gpio_pin_num);
>> +		printf("\t\tDP USB type C support: %s\n", YESNO(child->dp_usb_type_c));
>> +		printf("\t\t2X DP GPIO index: 0x%02x\n", child->dp_gpio_index);
>> +		printf("\t\t2X DP GPIO pin number: 0x%02x\n", child->dp_gpio_pin_num);
>>  	}
>>  
>>  	if (context->bdb->version >= 196) {
>> -		printf("\t\tIBoost level for HDMI: 0x%02x\n", efp->hdmi_iboost_level);
>> -		printf("\t\tIBoost level for DP/eDP: 0x%02x\n", efp->dp_iboost_level);
>> +		printf("\t\tIBoost level for HDMI: 0x%02x\n", child->hdmi_iboost_level);
>> +		printf("\t\tIBoost level for DP/eDP: 0x%02x\n", child->dp_iboost_level);
>>  	}
>>  }
>>  
>> -- 
>> 2.11.0

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH i-g-t 5/8] tools/intel_vbt_decode: unify child device printing across versions
  2017-10-20 10:56     ` Jani Nikula
@ 2017-10-20 11:33       ` Ville Syrjälä
  2017-10-20 13:17         ` Jani Nikula
  0 siblings, 1 reply; 15+ messages in thread
From: Ville Syrjälä @ 2017-10-20 11:33 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Fri, Oct 20, 2017 at 01:56:11PM +0300, Jani Nikula wrote:
> On Thu, 19 Oct 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Thu, Oct 19, 2017 at 06:22:56PM +0300, Jani Nikula wrote:
> >> Make it easier to compare dumping against the struct definition.
> >> 
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >>  tools/intel_vbt_decode.c | 128 ++++++++++++++++++++++++++---------------------
> >>  1 file changed, 71 insertions(+), 57 deletions(-)
> >> 
> >> diff --git a/tools/intel_vbt_decode.c b/tools/intel_vbt_decode.c
> >> index 69cf1e4c6cc6..bc5f38112619 100644
> >> --- a/tools/intel_vbt_decode.c
> >> +++ b/tools/intel_vbt_decode.c
> >> @@ -401,74 +401,88 @@ static const char *mipi_bridge_type(uint8_t type)
> >>  static void dump_child_device(struct context *context,
> >>  			      const struct child_device_config *child)
> >>  {
> >> -	const struct child_device_config *efp = child;
> >> -
> >>  	if (!child->device_type)
> >>  		return;
> >>  
> >> +	printf("\tChild device info:\n");
> >> +	printf("\t\tDevice handle: 0x%04x (%s)\n", child->handle,
> >> +	       child_device_handle(child->handle));
> >> +	printf("\t\tDevice type: 0x%04x (%s)\n", child->device_type,
> >> +	       child_device_type(child->device_type));
> >> +	dump_child_device_type_bits(child->device_type);
> >> +
> >>  	if (context->bdb->version < 152) {
> >> -		printf("\tChild device info:\n");
> >> -		printf("\t\tDevice type: %04x (%s)\n", child->device_type,
> >> -		       child_device_type(child->device_type));
> >>  		printf("\t\tSignature: %.*s\n", (int)sizeof(child->device_id), child->device_id);
> >> -		printf("\t\tAIM offset: %d\n", child->addin_offset);
> >> -		printf("\t\tDVO port: 0x%02x\n", child->dvo_port);
> >> -	} else { /* 152+ have EFP blocks here */
> >> -		printf("\tEFP device info:\n");
> >> -		printf("\t\tDevice handle: 0x%04x (%s)\n", efp->handle,
> >> -		       child_device_handle(efp->handle));
> >> -		printf("\t\tDevice type: 0x%04x (%s)\n", efp->device_type,
> >> -		       child_device_type(efp->device_type));
> >> -		dump_child_device_type_bits(efp->device_type);
> >> -		printf("\t\tI2C speed: 0x%02x\n", efp->i2c_speed);
> >> -		printf("\t\tDP onboard redriver: 0x%02x\n", efp->dp_onboard_redriver);
> >> -		printf("\t\tDP ondock redriver: 0x%02x\n", efp->dp_ondock_redriver);
> >> -		printf("\t\tHDMI max data rate: 0x%02x\n", efp->hdmi_max_data_rate);
> >> -		printf("\t\tHDMI level shifter value: 0x%02x\n", efp->hdmi_level_shifter_value);
> >> -		printf("\t\tOffset to DTD buffer for edidless EFP: 0x%02x\n", efp->dtd_buf_ptr);
> >> -		printf("\t\tDual pipe ganged eDP: %s\n", YESNO(efp->ganged_edp));
> >> -		printf("\t\tCompression method CPS: %s\n", YESNO(efp->compression_method));
> >> -		printf("\t\tCompression enable: %s\n", YESNO(efp->compression_enable));
> >> -		printf("\t\tEdidless EFP: %s\n", YESNO(efp->edidless_efp));
> >> -		printf("\t\tCompression structure index: 0x%02x)\n", efp->compression_structure_index);
> >> -		printf("\t\tSlave DDI port: 0x%02x (%s)\n", efp->slave_port, dvo_port(efp->slave_port));
> >> -		printf("\t\tAIM offset: %d\n", child->addin_offset);
> >> -		printf("\t\tPort: 0x%02x (%s)\n", efp->dvo_port, dvo_port(efp->dvo_port));
> >> -		printf("\t\tAIM I2C pin: 0x%02x\n", efp->i2c_pin);
> >> -		printf("\t\tAIM Slave address: 0x%02x\n", efp->slave_addr);
> >> -		printf("\t\tDDC pin: 0x%02x\n", efp->ddc_pin);
> >> -		printf("\t\tEDID buffer ptr: 0x%02x\n", efp->edid_ptr);
> >> -		printf("\t\tDVO config: 0x%02x\n", efp->dvo_cfg);
> >> -		printf("\t\tHPD sense invert: %s\n", YESNO(efp->hpd_invert));
> >> -		printf("\t\tIboost enable: %s\n", YESNO(efp->iboost));
> >> -		printf("\t\tOnboard LSPCON: %s\n", YESNO(efp->lspcon));
> >> -		printf("\t\tLane reversal: %s\n", YESNO(efp->lane_reversal));
> >> -		printf("\t\tEFP routed through dock: %s\n", YESNO(efp->efp_routed));
> >> -		printf("\t\tHDMI compatible? %s\n", YESNO(efp->hdmi_support));
> >> -		printf("\t\tDP compatible? %s\n", YESNO(efp->dp_support));
> >> -		printf("\t\tTMDS compatible? %s\n", YESNO(efp->tmds_support));
> >> -		printf("\t\tAux channel: 0x%02x\n", efp->aux_channel);
> >> -		printf("\t\tDongle detect: 0x%02x\n", efp->dongle_detect);
> >> -		printf("\t\tIntegrated encoder instead of SDVO: %s\n", YESNO(efp->integrated_encoder));
> >> -		printf("\t\tHotplu connect status: 0x%02x\n", efp->hpd_status);
> >> -		printf("\t\tSDVO stall signal available: %s\n", YESNO(efp->sdvo_stall));
> >> -		printf("\t\tPipe capabilities: 0x%02x\n", efp->pipe_cap);
> >> -		printf("\t\tDVO wiring: 0x%02x\n", efp->dvo_wiring);
> >> -		printf("\t\tMIPI bridge type: %02x (%s)\n", efp->mipi_bridge_type,
> >> -		       mipi_bridge_type(efp->mipi_bridge_type));
> >> -		printf("\t\tDevice class extendsion: 0x%02x\n", efp->extended_type);
> >> -		printf("\t\tDVO function: 0x%02x\n", efp->dvo_function);
> >> +	} else {
> >> +		printf("\t\tI2C speed: 0x%02x\n", child->i2c_speed);
> >> +		printf("\t\tDP onboard redriver: 0x%02x\n", child->dp_onboard_redriver);
> >> +		printf("\t\tDP ondock redriver: 0x%02x\n", child->dp_ondock_redriver);
> >> +		printf("\t\tHDMI level shifter value: 0x%02x\n", child->hdmi_level_shifter_value);
> >> +		printf("\t\tHDMI max data rate: 0x%02x\n", child->hdmi_max_data_rate);
> >> +		printf("\t\tOffset to DTD buffer for edidless CHILD: 0x%02x\n", child->dtd_buf_ptr);
> >> +		printf("\t\tEdidless EFP: %s\n", YESNO(child->edidless_efp));
> >> +		printf("\t\tCompression enable: %s\n", YESNO(child->compression_enable));
> >> +		printf("\t\tCompression method CPS: %s\n", YESNO(child->compression_method));
> >> +		printf("\t\tDual pipe ganged eDP: %s\n", YESNO(child->ganged_edp));
> >> +		printf("\t\tCompression structure index: 0x%02x)\n", child->compression_structure_index);
> >> +		printf("\t\tSlave DDI port: 0x%02x (%s)\n", child->slave_port, dvo_port(child->slave_port));
> >> +	}
> >> +
> >> +	printf("\t\tAIM offset: %d\n", child->addin_offset);
> >> +	printf("\t\tDVO Port: 0x%02x (%s)\n", child->dvo_port, dvo_port(child->dvo_port));
> >> +
> >> +	if (context->bdb->version < 152)
> >> +		return;
> >> +
> >> +	printf("\t\tAIM I2C pin: 0x%02x\n", child->i2c_pin);
> >> +	printf("\t\tAIM Slave address: 0x%02x\n", child->slave_addr);
> >> +	printf("\t\tDDC pin: 0x%02x\n", child->ddc_pin);
> >> +	printf("\t\tEDID buffer ptr: 0x%02x\n", child->edid_ptr);
> >> +	printf("\t\tDVO config: 0x%02x\n", child->dvo_cfg);
> >> +
> >> +	if (context->bdb->version < 158) {
> >> +		printf("\t\tDVO2 Port: 0x%02x (%s)\n", child->dvo2_port, dvo_port(child->dvo2_port));
> >> +		printf("\t\tI2C2 pin: 0x%02x\n", child->i2c2_pin);
> >> +		printf("\t\tSlave2 address: 0x%02x\n", child->slave2_addr);
> >> +		printf("\t\tDDC2 pin: 0x%02x\n", child->ddc2_pin);
> >> +	} else {
> >> +		printf("\t\tEFP routed through dock: %s\n", YESNO(child->efp_routed));
> >> +		printf("\t\tLane reversal: %s\n", YESNO(child->lane_reversal));
> >> +		printf("\t\tOnboard LSPCON: %s\n", YESNO(child->lspcon));
> >> +		printf("\t\tIboost enable: %s\n", YESNO(child->iboost));
> >> +		printf("\t\tHPD sense invert: %s\n", YESNO(child->hpd_invert));
> >> +		printf("\t\tHDMI compatible? %s\n", YESNO(child->hdmi_support));
> >> +		printf("\t\tDP compatible? %s\n", YESNO(child->dp_support));
> >> +		printf("\t\tTMDS compatible? %s\n", YESNO(child->tmds_support));
> >> +		printf("\t\tAux channel: 0x%02x\n", child->aux_channel);
> >> +		printf("\t\tDongle detect: 0x%02x\n", child->dongle_detect);
> >
> > The version of the spec I have at hand says aux and dongle are
> > already part of version 155.
> 
> Seems like dvo2/i2c2/slave2/ddc2 were converted to flags at 155. Okay if
> I just change the condition above from < 158 to < 155?

Aye.

I didn't trawl through all the gritty details of the entire series, but
what I do see I like, so for the series
Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I wonder if we should add more version checks so that we wouldn't dump
things that aren't actually specified? One crazy idea I had is that we
could define a parallel structure which would have a matching u8 member
to everything in the child_device_config, and we would initialize
each member with the correct version number. Then we could have some
kind of nice dump macro/function that checks the current VBT version
against the number from said parallel structure.

> 
> BR,
> Jani.
> 
> 
> >
> >> +	}
> >> +
> >> +	printf("\t\tPipe capabilities: 0x%02x\n", child->pipe_cap);
> >> +	printf("\t\tSDVO stall signal available: %s\n", YESNO(child->sdvo_stall));
> >> +	printf("\t\tHotplu connect status: 0x%02x\n", child->hpd_status);
> >                           ^
> > Pre-existing typo
> >
> >> +	printf("\t\tIntegrated encoder instead of SDVO: %s\n", YESNO(child->integrated_encoder));
> >> +	printf("\t\tDVO wiring: 0x%02x\n", child->dvo_wiring);
> >> +
> >> +	if (context->bdb->version < 171) {
> >> +		printf("\t\tDVO2 wiring: 0x%02x\n", child->dvo2_wiring);
> >> +	} else {
> >> +		printf("\t\tMIPI bridge type: %02x (%s)\n", child->mipi_bridge_type,
> >> +		       mipi_bridge_type(child->mipi_bridge_type));
> >>  	}
> >>  
> >> +	printf("\t\tDevice class extendsion: 0x%02x\n", child->extended_type);
> >                                       ^
> > ditto
> >
> >> +	printf("\t\tDVO function: 0x%02x\n", child->dvo_function);
> >> +
> >>  	if (context->bdb->version >= 195) {
> >> -		printf("\t\tDP USB type C support: %s\n", YESNO(efp->dp_usb_type_c));
> >> -		printf("\t\t2X DP GPIO index: 0x%02x\n", efp->dp_gpio_index);
> >> -		printf("\t\t2X DP GPIO pin number: 0x%02x\n", efp->dp_gpio_pin_num);
> >> +		printf("\t\tDP USB type C support: %s\n", YESNO(child->dp_usb_type_c));
> >> +		printf("\t\t2X DP GPIO index: 0x%02x\n", child->dp_gpio_index);
> >> +		printf("\t\t2X DP GPIO pin number: 0x%02x\n", child->dp_gpio_pin_num);
> >>  	}
> >>  
> >>  	if (context->bdb->version >= 196) {
> >> -		printf("\t\tIBoost level for HDMI: 0x%02x\n", efp->hdmi_iboost_level);
> >> -		printf("\t\tIBoost level for DP/eDP: 0x%02x\n", efp->dp_iboost_level);
> >> +		printf("\t\tIBoost level for HDMI: 0x%02x\n", child->hdmi_iboost_level);
> >> +		printf("\t\tIBoost level for DP/eDP: 0x%02x\n", child->dp_iboost_level);
> >>  	}
> >>  }
> >>  
> >> -- 
> >> 2.11.0
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH i-g-t 5/8] tools/intel_vbt_decode: unify child device printing across versions
  2017-10-20 11:33       ` Ville Syrjälä
@ 2017-10-20 13:17         ` Jani Nikula
  2017-10-20 13:49           ` Ville Syrjälä
  0 siblings, 1 reply; 15+ messages in thread
From: Jani Nikula @ 2017-10-20 13:17 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Fri, 20 Oct 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Oct 20, 2017 at 01:56:11PM +0300, Jani Nikula wrote:
>> On Thu, 19 Oct 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> > On Thu, Oct 19, 2017 at 06:22:56PM +0300, Jani Nikula wrote:
>> >> Make it easier to compare dumping against the struct definition.
>> >> 
>> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> >> ---
>> >>  tools/intel_vbt_decode.c | 128 ++++++++++++++++++++++++++---------------------
>> >>  1 file changed, 71 insertions(+), 57 deletions(-)
>> >> 
>> >> diff --git a/tools/intel_vbt_decode.c b/tools/intel_vbt_decode.c
>> >> index 69cf1e4c6cc6..bc5f38112619 100644
>> >> --- a/tools/intel_vbt_decode.c
>> >> +++ b/tools/intel_vbt_decode.c
>> >> @@ -401,74 +401,88 @@ static const char *mipi_bridge_type(uint8_t type)
>> >>  static void dump_child_device(struct context *context,
>> >>  			      const struct child_device_config *child)
>> >>  {
>> >> -	const struct child_device_config *efp = child;
>> >> -
>> >>  	if (!child->device_type)
>> >>  		return;
>> >>  
>> >> +	printf("\tChild device info:\n");
>> >> +	printf("\t\tDevice handle: 0x%04x (%s)\n", child->handle,
>> >> +	       child_device_handle(child->handle));
>> >> +	printf("\t\tDevice type: 0x%04x (%s)\n", child->device_type,
>> >> +	       child_device_type(child->device_type));
>> >> +	dump_child_device_type_bits(child->device_type);
>> >> +
>> >>  	if (context->bdb->version < 152) {
>> >> -		printf("\tChild device info:\n");
>> >> -		printf("\t\tDevice type: %04x (%s)\n", child->device_type,
>> >> -		       child_device_type(child->device_type));
>> >>  		printf("\t\tSignature: %.*s\n", (int)sizeof(child->device_id), child->device_id);
>> >> -		printf("\t\tAIM offset: %d\n", child->addin_offset);
>> >> -		printf("\t\tDVO port: 0x%02x\n", child->dvo_port);
>> >> -	} else { /* 152+ have EFP blocks here */
>> >> -		printf("\tEFP device info:\n");
>> >> -		printf("\t\tDevice handle: 0x%04x (%s)\n", efp->handle,
>> >> -		       child_device_handle(efp->handle));
>> >> -		printf("\t\tDevice type: 0x%04x (%s)\n", efp->device_type,
>> >> -		       child_device_type(efp->device_type));
>> >> -		dump_child_device_type_bits(efp->device_type);
>> >> -		printf("\t\tI2C speed: 0x%02x\n", efp->i2c_speed);
>> >> -		printf("\t\tDP onboard redriver: 0x%02x\n", efp->dp_onboard_redriver);
>> >> -		printf("\t\tDP ondock redriver: 0x%02x\n", efp->dp_ondock_redriver);
>> >> -		printf("\t\tHDMI max data rate: 0x%02x\n", efp->hdmi_max_data_rate);
>> >> -		printf("\t\tHDMI level shifter value: 0x%02x\n", efp->hdmi_level_shifter_value);
>> >> -		printf("\t\tOffset to DTD buffer for edidless EFP: 0x%02x\n", efp->dtd_buf_ptr);
>> >> -		printf("\t\tDual pipe ganged eDP: %s\n", YESNO(efp->ganged_edp));
>> >> -		printf("\t\tCompression method CPS: %s\n", YESNO(efp->compression_method));
>> >> -		printf("\t\tCompression enable: %s\n", YESNO(efp->compression_enable));
>> >> -		printf("\t\tEdidless EFP: %s\n", YESNO(efp->edidless_efp));
>> >> -		printf("\t\tCompression structure index: 0x%02x)\n", efp->compression_structure_index);
>> >> -		printf("\t\tSlave DDI port: 0x%02x (%s)\n", efp->slave_port, dvo_port(efp->slave_port));
>> >> -		printf("\t\tAIM offset: %d\n", child->addin_offset);
>> >> -		printf("\t\tPort: 0x%02x (%s)\n", efp->dvo_port, dvo_port(efp->dvo_port));
>> >> -		printf("\t\tAIM I2C pin: 0x%02x\n", efp->i2c_pin);
>> >> -		printf("\t\tAIM Slave address: 0x%02x\n", efp->slave_addr);
>> >> -		printf("\t\tDDC pin: 0x%02x\n", efp->ddc_pin);
>> >> -		printf("\t\tEDID buffer ptr: 0x%02x\n", efp->edid_ptr);
>> >> -		printf("\t\tDVO config: 0x%02x\n", efp->dvo_cfg);
>> >> -		printf("\t\tHPD sense invert: %s\n", YESNO(efp->hpd_invert));
>> >> -		printf("\t\tIboost enable: %s\n", YESNO(efp->iboost));
>> >> -		printf("\t\tOnboard LSPCON: %s\n", YESNO(efp->lspcon));
>> >> -		printf("\t\tLane reversal: %s\n", YESNO(efp->lane_reversal));
>> >> -		printf("\t\tEFP routed through dock: %s\n", YESNO(efp->efp_routed));
>> >> -		printf("\t\tHDMI compatible? %s\n", YESNO(efp->hdmi_support));
>> >> -		printf("\t\tDP compatible? %s\n", YESNO(efp->dp_support));
>> >> -		printf("\t\tTMDS compatible? %s\n", YESNO(efp->tmds_support));
>> >> -		printf("\t\tAux channel: 0x%02x\n", efp->aux_channel);
>> >> -		printf("\t\tDongle detect: 0x%02x\n", efp->dongle_detect);
>> >> -		printf("\t\tIntegrated encoder instead of SDVO: %s\n", YESNO(efp->integrated_encoder));
>> >> -		printf("\t\tHotplu connect status: 0x%02x\n", efp->hpd_status);
>> >> -		printf("\t\tSDVO stall signal available: %s\n", YESNO(efp->sdvo_stall));
>> >> -		printf("\t\tPipe capabilities: 0x%02x\n", efp->pipe_cap);
>> >> -		printf("\t\tDVO wiring: 0x%02x\n", efp->dvo_wiring);
>> >> -		printf("\t\tMIPI bridge type: %02x (%s)\n", efp->mipi_bridge_type,
>> >> -		       mipi_bridge_type(efp->mipi_bridge_type));
>> >> -		printf("\t\tDevice class extendsion: 0x%02x\n", efp->extended_type);
>> >> -		printf("\t\tDVO function: 0x%02x\n", efp->dvo_function);
>> >> +	} else {
>> >> +		printf("\t\tI2C speed: 0x%02x\n", child->i2c_speed);
>> >> +		printf("\t\tDP onboard redriver: 0x%02x\n", child->dp_onboard_redriver);
>> >> +		printf("\t\tDP ondock redriver: 0x%02x\n", child->dp_ondock_redriver);
>> >> +		printf("\t\tHDMI level shifter value: 0x%02x\n", child->hdmi_level_shifter_value);
>> >> +		printf("\t\tHDMI max data rate: 0x%02x\n", child->hdmi_max_data_rate);
>> >> +		printf("\t\tOffset to DTD buffer for edidless CHILD: 0x%02x\n", child->dtd_buf_ptr);
>> >> +		printf("\t\tEdidless EFP: %s\n", YESNO(child->edidless_efp));
>> >> +		printf("\t\tCompression enable: %s\n", YESNO(child->compression_enable));
>> >> +		printf("\t\tCompression method CPS: %s\n", YESNO(child->compression_method));
>> >> +		printf("\t\tDual pipe ganged eDP: %s\n", YESNO(child->ganged_edp));
>> >> +		printf("\t\tCompression structure index: 0x%02x)\n", child->compression_structure_index);
>> >> +		printf("\t\tSlave DDI port: 0x%02x (%s)\n", child->slave_port, dvo_port(child->slave_port));
>> >> +	}
>> >> +
>> >> +	printf("\t\tAIM offset: %d\n", child->addin_offset);
>> >> +	printf("\t\tDVO Port: 0x%02x (%s)\n", child->dvo_port, dvo_port(child->dvo_port));
>> >> +
>> >> +	if (context->bdb->version < 152)
>> >> +		return;
>> >> +
>> >> +	printf("\t\tAIM I2C pin: 0x%02x\n", child->i2c_pin);
>> >> +	printf("\t\tAIM Slave address: 0x%02x\n", child->slave_addr);
>> >> +	printf("\t\tDDC pin: 0x%02x\n", child->ddc_pin);
>> >> +	printf("\t\tEDID buffer ptr: 0x%02x\n", child->edid_ptr);
>> >> +	printf("\t\tDVO config: 0x%02x\n", child->dvo_cfg);
>> >> +
>> >> +	if (context->bdb->version < 158) {
>> >> +		printf("\t\tDVO2 Port: 0x%02x (%s)\n", child->dvo2_port, dvo_port(child->dvo2_port));
>> >> +		printf("\t\tI2C2 pin: 0x%02x\n", child->i2c2_pin);
>> >> +		printf("\t\tSlave2 address: 0x%02x\n", child->slave2_addr);
>> >> +		printf("\t\tDDC2 pin: 0x%02x\n", child->ddc2_pin);
>> >> +	} else {
>> >> +		printf("\t\tEFP routed through dock: %s\n", YESNO(child->efp_routed));
>> >> +		printf("\t\tLane reversal: %s\n", YESNO(child->lane_reversal));
>> >> +		printf("\t\tOnboard LSPCON: %s\n", YESNO(child->lspcon));
>> >> +		printf("\t\tIboost enable: %s\n", YESNO(child->iboost));
>> >> +		printf("\t\tHPD sense invert: %s\n", YESNO(child->hpd_invert));
>> >> +		printf("\t\tHDMI compatible? %s\n", YESNO(child->hdmi_support));
>> >> +		printf("\t\tDP compatible? %s\n", YESNO(child->dp_support));
>> >> +		printf("\t\tTMDS compatible? %s\n", YESNO(child->tmds_support));
>> >> +		printf("\t\tAux channel: 0x%02x\n", child->aux_channel);
>> >> +		printf("\t\tDongle detect: 0x%02x\n", child->dongle_detect);
>> >
>> > The version of the spec I have at hand says aux and dongle are
>> > already part of version 155.
>> 
>> Seems like dvo2/i2c2/slave2/ddc2 were converted to flags at 155. Okay if
>> I just change the condition above from < 158 to < 155?
>
> Aye.
>
> I didn't trawl through all the gritty details of the entire series, but
> what I do see I like, so for the series
> Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks, pushed the lot.

> I wonder if we should add more version checks so that we wouldn't dump
> things that aren't actually specified? One crazy idea I had is that we
> could define a parallel structure which would have a matching u8 member
> to everything in the child_device_config, and we would initialize
> each member with the correct version number. Then we could have some
> kind of nice dump macro/function that checks the current VBT version
> against the number from said parallel structure.

Overengineering? ;)

BR,
Jani.


>
>> 
>> BR,
>> Jani.
>> 
>> 
>> >
>> >> +	}
>> >> +
>> >> +	printf("\t\tPipe capabilities: 0x%02x\n", child->pipe_cap);
>> >> +	printf("\t\tSDVO stall signal available: %s\n", YESNO(child->sdvo_stall));
>> >> +	printf("\t\tHotplu connect status: 0x%02x\n", child->hpd_status);
>> >                           ^
>> > Pre-existing typo
>> >
>> >> +	printf("\t\tIntegrated encoder instead of SDVO: %s\n", YESNO(child->integrated_encoder));
>> >> +	printf("\t\tDVO wiring: 0x%02x\n", child->dvo_wiring);
>> >> +
>> >> +	if (context->bdb->version < 171) {
>> >> +		printf("\t\tDVO2 wiring: 0x%02x\n", child->dvo2_wiring);
>> >> +	} else {
>> >> +		printf("\t\tMIPI bridge type: %02x (%s)\n", child->mipi_bridge_type,
>> >> +		       mipi_bridge_type(child->mipi_bridge_type));
>> >>  	}
>> >>  
>> >> +	printf("\t\tDevice class extendsion: 0x%02x\n", child->extended_type);
>> >                                       ^
>> > ditto
>> >
>> >> +	printf("\t\tDVO function: 0x%02x\n", child->dvo_function);
>> >> +
>> >>  	if (context->bdb->version >= 195) {
>> >> -		printf("\t\tDP USB type C support: %s\n", YESNO(efp->dp_usb_type_c));
>> >> -		printf("\t\t2X DP GPIO index: 0x%02x\n", efp->dp_gpio_index);
>> >> -		printf("\t\t2X DP GPIO pin number: 0x%02x\n", efp->dp_gpio_pin_num);
>> >> +		printf("\t\tDP USB type C support: %s\n", YESNO(child->dp_usb_type_c));
>> >> +		printf("\t\t2X DP GPIO index: 0x%02x\n", child->dp_gpio_index);
>> >> +		printf("\t\t2X DP GPIO pin number: 0x%02x\n", child->dp_gpio_pin_num);
>> >>  	}
>> >>  
>> >>  	if (context->bdb->version >= 196) {
>> >> -		printf("\t\tIBoost level for HDMI: 0x%02x\n", efp->hdmi_iboost_level);
>> >> -		printf("\t\tIBoost level for DP/eDP: 0x%02x\n", efp->dp_iboost_level);
>> >> +		printf("\t\tIBoost level for HDMI: 0x%02x\n", child->hdmi_iboost_level);
>> >> +		printf("\t\tIBoost level for DP/eDP: 0x%02x\n", child->dp_iboost_level);
>> >>  	}
>> >>  }
>> >>  
>> >> -- 
>> >> 2.11.0
>> 
>> -- 
>> Jani Nikula, Intel Open Source Technology Center

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH i-g-t 5/8] tools/intel_vbt_decode: unify child device printing across versions
  2017-10-20 13:17         ` Jani Nikula
@ 2017-10-20 13:49           ` Ville Syrjälä
  0 siblings, 0 replies; 15+ messages in thread
From: Ville Syrjälä @ 2017-10-20 13:49 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Fri, Oct 20, 2017 at 04:17:06PM +0300, Jani Nikula wrote:
> On Fri, 20 Oct 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Fri, Oct 20, 2017 at 01:56:11PM +0300, Jani Nikula wrote:
> >> On Thu, 19 Oct 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >> > On Thu, Oct 19, 2017 at 06:22:56PM +0300, Jani Nikula wrote:
> >> >> Make it easier to compare dumping against the struct definition.
> >> >> 
> >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> >> ---
> >> >>  tools/intel_vbt_decode.c | 128 ++++++++++++++++++++++++++---------------------
> >> >>  1 file changed, 71 insertions(+), 57 deletions(-)
> >> >> 
> >> >> diff --git a/tools/intel_vbt_decode.c b/tools/intel_vbt_decode.c
> >> >> index 69cf1e4c6cc6..bc5f38112619 100644
> >> >> --- a/tools/intel_vbt_decode.c
> >> >> +++ b/tools/intel_vbt_decode.c
> >> >> @@ -401,74 +401,88 @@ static const char *mipi_bridge_type(uint8_t type)
> >> >>  static void dump_child_device(struct context *context,
> >> >>  			      const struct child_device_config *child)
> >> >>  {
> >> >> -	const struct child_device_config *efp = child;
> >> >> -
> >> >>  	if (!child->device_type)
> >> >>  		return;
> >> >>  
> >> >> +	printf("\tChild device info:\n");
> >> >> +	printf("\t\tDevice handle: 0x%04x (%s)\n", child->handle,
> >> >> +	       child_device_handle(child->handle));
> >> >> +	printf("\t\tDevice type: 0x%04x (%s)\n", child->device_type,
> >> >> +	       child_device_type(child->device_type));
> >> >> +	dump_child_device_type_bits(child->device_type);
> >> >> +
> >> >>  	if (context->bdb->version < 152) {
> >> >> -		printf("\tChild device info:\n");
> >> >> -		printf("\t\tDevice type: %04x (%s)\n", child->device_type,
> >> >> -		       child_device_type(child->device_type));
> >> >>  		printf("\t\tSignature: %.*s\n", (int)sizeof(child->device_id), child->device_id);
> >> >> -		printf("\t\tAIM offset: %d\n", child->addin_offset);
> >> >> -		printf("\t\tDVO port: 0x%02x\n", child->dvo_port);
> >> >> -	} else { /* 152+ have EFP blocks here */
> >> >> -		printf("\tEFP device info:\n");
> >> >> -		printf("\t\tDevice handle: 0x%04x (%s)\n", efp->handle,
> >> >> -		       child_device_handle(efp->handle));
> >> >> -		printf("\t\tDevice type: 0x%04x (%s)\n", efp->device_type,
> >> >> -		       child_device_type(efp->device_type));
> >> >> -		dump_child_device_type_bits(efp->device_type);
> >> >> -		printf("\t\tI2C speed: 0x%02x\n", efp->i2c_speed);
> >> >> -		printf("\t\tDP onboard redriver: 0x%02x\n", efp->dp_onboard_redriver);
> >> >> -		printf("\t\tDP ondock redriver: 0x%02x\n", efp->dp_ondock_redriver);
> >> >> -		printf("\t\tHDMI max data rate: 0x%02x\n", efp->hdmi_max_data_rate);
> >> >> -		printf("\t\tHDMI level shifter value: 0x%02x\n", efp->hdmi_level_shifter_value);
> >> >> -		printf("\t\tOffset to DTD buffer for edidless EFP: 0x%02x\n", efp->dtd_buf_ptr);
> >> >> -		printf("\t\tDual pipe ganged eDP: %s\n", YESNO(efp->ganged_edp));
> >> >> -		printf("\t\tCompression method CPS: %s\n", YESNO(efp->compression_method));
> >> >> -		printf("\t\tCompression enable: %s\n", YESNO(efp->compression_enable));
> >> >> -		printf("\t\tEdidless EFP: %s\n", YESNO(efp->edidless_efp));
> >> >> -		printf("\t\tCompression structure index: 0x%02x)\n", efp->compression_structure_index);
> >> >> -		printf("\t\tSlave DDI port: 0x%02x (%s)\n", efp->slave_port, dvo_port(efp->slave_port));
> >> >> -		printf("\t\tAIM offset: %d\n", child->addin_offset);
> >> >> -		printf("\t\tPort: 0x%02x (%s)\n", efp->dvo_port, dvo_port(efp->dvo_port));
> >> >> -		printf("\t\tAIM I2C pin: 0x%02x\n", efp->i2c_pin);
> >> >> -		printf("\t\tAIM Slave address: 0x%02x\n", efp->slave_addr);
> >> >> -		printf("\t\tDDC pin: 0x%02x\n", efp->ddc_pin);
> >> >> -		printf("\t\tEDID buffer ptr: 0x%02x\n", efp->edid_ptr);
> >> >> -		printf("\t\tDVO config: 0x%02x\n", efp->dvo_cfg);
> >> >> -		printf("\t\tHPD sense invert: %s\n", YESNO(efp->hpd_invert));
> >> >> -		printf("\t\tIboost enable: %s\n", YESNO(efp->iboost));
> >> >> -		printf("\t\tOnboard LSPCON: %s\n", YESNO(efp->lspcon));
> >> >> -		printf("\t\tLane reversal: %s\n", YESNO(efp->lane_reversal));
> >> >> -		printf("\t\tEFP routed through dock: %s\n", YESNO(efp->efp_routed));
> >> >> -		printf("\t\tHDMI compatible? %s\n", YESNO(efp->hdmi_support));
> >> >> -		printf("\t\tDP compatible? %s\n", YESNO(efp->dp_support));
> >> >> -		printf("\t\tTMDS compatible? %s\n", YESNO(efp->tmds_support));
> >> >> -		printf("\t\tAux channel: 0x%02x\n", efp->aux_channel);
> >> >> -		printf("\t\tDongle detect: 0x%02x\n", efp->dongle_detect);
> >> >> -		printf("\t\tIntegrated encoder instead of SDVO: %s\n", YESNO(efp->integrated_encoder));
> >> >> -		printf("\t\tHotplu connect status: 0x%02x\n", efp->hpd_status);
> >> >> -		printf("\t\tSDVO stall signal available: %s\n", YESNO(efp->sdvo_stall));
> >> >> -		printf("\t\tPipe capabilities: 0x%02x\n", efp->pipe_cap);
> >> >> -		printf("\t\tDVO wiring: 0x%02x\n", efp->dvo_wiring);
> >> >> -		printf("\t\tMIPI bridge type: %02x (%s)\n", efp->mipi_bridge_type,
> >> >> -		       mipi_bridge_type(efp->mipi_bridge_type));
> >> >> -		printf("\t\tDevice class extendsion: 0x%02x\n", efp->extended_type);
> >> >> -		printf("\t\tDVO function: 0x%02x\n", efp->dvo_function);
> >> >> +	} else {
> >> >> +		printf("\t\tI2C speed: 0x%02x\n", child->i2c_speed);
> >> >> +		printf("\t\tDP onboard redriver: 0x%02x\n", child->dp_onboard_redriver);
> >> >> +		printf("\t\tDP ondock redriver: 0x%02x\n", child->dp_ondock_redriver);
> >> >> +		printf("\t\tHDMI level shifter value: 0x%02x\n", child->hdmi_level_shifter_value);
> >> >> +		printf("\t\tHDMI max data rate: 0x%02x\n", child->hdmi_max_data_rate);
> >> >> +		printf("\t\tOffset to DTD buffer for edidless CHILD: 0x%02x\n", child->dtd_buf_ptr);
> >> >> +		printf("\t\tEdidless EFP: %s\n", YESNO(child->edidless_efp));
> >> >> +		printf("\t\tCompression enable: %s\n", YESNO(child->compression_enable));
> >> >> +		printf("\t\tCompression method CPS: %s\n", YESNO(child->compression_method));
> >> >> +		printf("\t\tDual pipe ganged eDP: %s\n", YESNO(child->ganged_edp));
> >> >> +		printf("\t\tCompression structure index: 0x%02x)\n", child->compression_structure_index);
> >> >> +		printf("\t\tSlave DDI port: 0x%02x (%s)\n", child->slave_port, dvo_port(child->slave_port));
> >> >> +	}
> >> >> +
> >> >> +	printf("\t\tAIM offset: %d\n", child->addin_offset);
> >> >> +	printf("\t\tDVO Port: 0x%02x (%s)\n", child->dvo_port, dvo_port(child->dvo_port));
> >> >> +
> >> >> +	if (context->bdb->version < 152)
> >> >> +		return;
> >> >> +
> >> >> +	printf("\t\tAIM I2C pin: 0x%02x\n", child->i2c_pin);
> >> >> +	printf("\t\tAIM Slave address: 0x%02x\n", child->slave_addr);
> >> >> +	printf("\t\tDDC pin: 0x%02x\n", child->ddc_pin);
> >> >> +	printf("\t\tEDID buffer ptr: 0x%02x\n", child->edid_ptr);
> >> >> +	printf("\t\tDVO config: 0x%02x\n", child->dvo_cfg);
> >> >> +
> >> >> +	if (context->bdb->version < 158) {
> >> >> +		printf("\t\tDVO2 Port: 0x%02x (%s)\n", child->dvo2_port, dvo_port(child->dvo2_port));
> >> >> +		printf("\t\tI2C2 pin: 0x%02x\n", child->i2c2_pin);
> >> >> +		printf("\t\tSlave2 address: 0x%02x\n", child->slave2_addr);
> >> >> +		printf("\t\tDDC2 pin: 0x%02x\n", child->ddc2_pin);
> >> >> +	} else {
> >> >> +		printf("\t\tEFP routed through dock: %s\n", YESNO(child->efp_routed));
> >> >> +		printf("\t\tLane reversal: %s\n", YESNO(child->lane_reversal));
> >> >> +		printf("\t\tOnboard LSPCON: %s\n", YESNO(child->lspcon));
> >> >> +		printf("\t\tIboost enable: %s\n", YESNO(child->iboost));
> >> >> +		printf("\t\tHPD sense invert: %s\n", YESNO(child->hpd_invert));
> >> >> +		printf("\t\tHDMI compatible? %s\n", YESNO(child->hdmi_support));
> >> >> +		printf("\t\tDP compatible? %s\n", YESNO(child->dp_support));
> >> >> +		printf("\t\tTMDS compatible? %s\n", YESNO(child->tmds_support));
> >> >> +		printf("\t\tAux channel: 0x%02x\n", child->aux_channel);
> >> >> +		printf("\t\tDongle detect: 0x%02x\n", child->dongle_detect);
> >> >
> >> > The version of the spec I have at hand says aux and dongle are
> >> > already part of version 155.
> >> 
> >> Seems like dvo2/i2c2/slave2/ddc2 were converted to flags at 155. Okay if
> >> I just change the condition above from < 158 to < 155?
> >
> > Aye.
> >
> > I didn't trawl through all the gritty details of the entire series, but
> > what I do see I like, so for the series
> > Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Thanks, pushed the lot.
> 
> > I wonder if we should add more version checks so that we wouldn't dump
> > things that aren't actually specified? One crazy idea I had is that we
> > could define a parallel structure which would have a matching u8 member
> > to everything in the child_device_config, and we would initialize
> > each member with the correct version number. Then we could have some
> > kind of nice dump macro/function that checks the current VBT version
> > against the number from said parallel structure.
> 
> Overengineering? ;)

Possibly. But it seems like a much nicer way to go about it that
sprinking 'if (version >= ...)' checks all over. Although I guess
we could also just pass the hardcoded minimum version to that
macro/function I was thinking we'd add.

> 
> BR,
> Jani.
> 
> 
> >
> >> 
> >> BR,
> >> Jani.
> >> 
> >> 
> >> >
> >> >> +	}
> >> >> +
> >> >> +	printf("\t\tPipe capabilities: 0x%02x\n", child->pipe_cap);
> >> >> +	printf("\t\tSDVO stall signal available: %s\n", YESNO(child->sdvo_stall));
> >> >> +	printf("\t\tHotplu connect status: 0x%02x\n", child->hpd_status);
> >> >                           ^
> >> > Pre-existing typo
> >> >
> >> >> +	printf("\t\tIntegrated encoder instead of SDVO: %s\n", YESNO(child->integrated_encoder));
> >> >> +	printf("\t\tDVO wiring: 0x%02x\n", child->dvo_wiring);
> >> >> +
> >> >> +	if (context->bdb->version < 171) {
> >> >> +		printf("\t\tDVO2 wiring: 0x%02x\n", child->dvo2_wiring);
> >> >> +	} else {
> >> >> +		printf("\t\tMIPI bridge type: %02x (%s)\n", child->mipi_bridge_type,
> >> >> +		       mipi_bridge_type(child->mipi_bridge_type));
> >> >>  	}
> >> >>  
> >> >> +	printf("\t\tDevice class extendsion: 0x%02x\n", child->extended_type);
> >> >                                       ^
> >> > ditto
> >> >
> >> >> +	printf("\t\tDVO function: 0x%02x\n", child->dvo_function);
> >> >> +
> >> >>  	if (context->bdb->version >= 195) {
> >> >> -		printf("\t\tDP USB type C support: %s\n", YESNO(efp->dp_usb_type_c));
> >> >> -		printf("\t\t2X DP GPIO index: 0x%02x\n", efp->dp_gpio_index);
> >> >> -		printf("\t\t2X DP GPIO pin number: 0x%02x\n", efp->dp_gpio_pin_num);
> >> >> +		printf("\t\tDP USB type C support: %s\n", YESNO(child->dp_usb_type_c));
> >> >> +		printf("\t\t2X DP GPIO index: 0x%02x\n", child->dp_gpio_index);
> >> >> +		printf("\t\t2X DP GPIO pin number: 0x%02x\n", child->dp_gpio_pin_num);
> >> >>  	}
> >> >>  
> >> >>  	if (context->bdb->version >= 196) {
> >> >> -		printf("\t\tIBoost level for HDMI: 0x%02x\n", efp->hdmi_iboost_level);
> >> >> -		printf("\t\tIBoost level for DP/eDP: 0x%02x\n", efp->dp_iboost_level);
> >> >> +		printf("\t\tIBoost level for HDMI: 0x%02x\n", child->hdmi_iboost_level);
> >> >> +		printf("\t\tIBoost level for DP/eDP: 0x%02x\n", child->dp_iboost_level);
> >> >>  	}
> >> >>  }
> >> >>  
> >> >> -- 
> >> >> 2.11.0
> >> 
> >> -- 
> >> Jani Nikula, Intel Open Source Technology Center
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-10-20 13:49 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-19 15:22 [PATCH i-g-t 0/8] tools/intel_vbt_decode: refactoring and cleanups Jani Nikula
2017-10-19 15:22 ` [PATCH i-g-t 1/8] tools/intel_vbt_decode: make a copy of child devices before dumping Jani Nikula
2017-10-19 15:22 ` [PATCH i-g-t 2/8] tools/intel_vbt_decode: update dvo port name dumping Jani Nikula
2017-10-19 15:22 ` [PATCH i-g-t 3/8] tools/intel_vbt_decode: use %.*s instead of duplicating a string Jani Nikula
2017-10-19 15:22 ` [PATCH i-g-t 4/8] tools/intel_vbt_decode: abstract DSI bridge type dump Jani Nikula
2017-10-19 15:22 ` [PATCH i-g-t 5/8] tools/intel_vbt_decode: unify child device printing across versions Jani Nikula
2017-10-19 17:24   ` Ville Syrjälä
2017-10-20 10:56     ` Jani Nikula
2017-10-20 11:33       ` Ville Syrjälä
2017-10-20 13:17         ` Jani Nikula
2017-10-20 13:49           ` Ville Syrjälä
2017-10-19 15:22 ` [PATCH i-g-t 6/8] tools/intel_vbt_decode: unify legacy child device block dumping Jani Nikula
2017-10-19 15:22 ` [PATCH i-g-t 7/8] tools/intel_vbt_decode: dump more child device data for version < 152 Jani Nikula
2017-10-19 15:22 ` [PATCH i-g-t 8/8] tools/intel_vbt_decode: abstract child devices printing more Jani Nikula
2017-10-19 16:52 ` ✗ Fi.CI.BAT: failure for tools/intel_vbt_decode: refactoring and cleanups Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox