[igt-dev] [PATCH i-g-t 1/4] tools/intel_reg: Simplify negative ports

[igt-dev] [PATCH i-g-t 1/4] tools/intel_reg: Simplify negative ports

[Ville Syrjala]

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

Let's avoid having to type in the explicit negative value for
all the port enums.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tools/intel_reg_spec.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/intel_reg_spec.h b/tools/intel_reg_spec.h
index c94c61bab3f8..a6e0514387df 100644
--- a/tools/intel_reg_spec.h
+++ b/tools/intel_reg_spec.h
@@ -25,10 +25,11 @@
 #define __INTEL_REG_SPEC_H__
 
 enum port_addr {
+	PORT_MMIO = -127,
+	PORT_PORTIO_VGA,	/* see vga reg read/write */
+	PORT_MMIO_VGA,		/* see vga reg read/write */
+
 	PORT_NONE = 0,
-	PORT_MMIO = -1,
-	PORT_PORTIO_VGA = -2,	/* see vga reg read/write */
-	PORT_MMIO_VGA = -3,	/* see vga reg read/write */
 
 	/* vlv */
 	PORT_BUNIT = 0x03,
-- 
2.24.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 2/4] tools/intel_reg: Support 8 and 16 bit mmio registers

[Ville Syrjala]

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

Add new "ports" for doing 8 and 16 bit mmio accesses.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tools/intel_reg.c      | 53 ++++++++++++++++++++++++++++++------------
 tools/intel_reg_spec.c | 19 +++++++++------
 tools/intel_reg_spec.h |  8 ++++---
 3 files changed, 55 insertions(+), 25 deletions(-)

diff --git a/tools/intel_reg.c b/tools/intel_reg.c
index 5f1beba4a269..7f8ddbd8e927 100644
--- a/tools/intel_reg.c
+++ b/tools/intel_reg.c
@@ -179,6 +179,18 @@ static void to_binary(char *buf, size_t buflen, uint32_t val)
 	snprintf(buf, buflen, "\n");
 }
 
+static bool port_is_mmio(enum port_addr port)
+{
+	switch (port) {
+	case PORT_MMIO_32:
+	case PORT_MMIO_16:
+	case PORT_MMIO_8:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static void dump_decode(struct config *config, struct reg *reg, uint32_t val)
 {
 	char decode[1300];
@@ -207,7 +219,7 @@ static void dump_decode(struct config *config, struct reg *reg, uint32_t val)
 		snprintf(decode, sizeof(decode), "\n");
 	}
 
-	if (reg->port_desc.port == PORT_MMIO) {
+	if (port_is_mmio(reg->port_desc.port)) {
 		/* Omit port name for MMIO, optionally include MMIO offset. */
 		if (reg->mmio_offset)
 			printf("%24s (0x%08x:0x%08x): 0x%08x%s",
@@ -359,20 +371,23 @@ static int read_register(struct config *config, struct reg *reg, uint32_t *valp)
 	uint32_t val = 0;
 
 	switch (reg->port_desc.port) {
-	case PORT_MMIO:
+	case PORT_MMIO_32:
 		if (reg->engine)
 			val = register_srm(config, reg, NULL);
 		else
 			val = INREG(reg->mmio_offset + reg->addr);
 		break;
-	case PORT_PORTIO_VGA:
+	case PORT_MMIO_16:
+		val = INREG16(reg->mmio_offset + reg->addr);
+		break;
+	case PORT_MMIO_8:
+		val = INREG8(reg->mmio_offset + reg->addr);
+		break;
+	case PORT_PORTIO:
 		iopl(3);
 		val = inb(reg->addr);
 		iopl(0);
 		break;
-	case PORT_MMIO_VGA:
-		val = INREG8(reg->addr);
-		break;
 	case PORT_BUNIT:
 	case PORT_PUNIT:
 	case PORT_NC:
@@ -419,30 +434,38 @@ static int write_register(struct config *config, struct reg *reg, uint32_t val)
 	}
 
 	switch (reg->port_desc.port) {
-	case PORT_MMIO:
+	case PORT_MMIO_32:
 		if (reg->engine) {
 			register_srm(config, reg, &val);
 		} else {
 			OUTREG(reg->mmio_offset + reg->addr, val);
 		}
 		break;
-	case PORT_PORTIO_VGA:
+	case PORT_MMIO_16:
+		if (val > 0xffff) {
+			fprintf(stderr, "value 0x%08x out of range for port %s\n",
+				val, reg->port_desc.name);
+			return -1;
+		}
+		OUTREG16(reg->mmio_offset + reg->addr, val);
+		break;
+	case PORT_MMIO_8:
 		if (val > 0xff) {
 			fprintf(stderr, "value 0x%08x out of range for port %s\n",
 				val, reg->port_desc.name);
 			return -1;
 		}
-		iopl(3);
-		outb(val, reg->addr);
-		iopl(0);
+		OUTREG8(reg->mmio_offset + reg->addr, val);
 		break;
-	case PORT_MMIO_VGA:
+	case PORT_PORTIO:
 		if (val > 0xff) {
 			fprintf(stderr, "value 0x%08x out of range for port %s\n",
 				val, reg->port_desc.name);
 			return -1;
 		}
-		OUTREG8(reg->addr, val);
+		iopl(3);
+		outb(val, reg->addr);
+		iopl(0);
 		break;
 	case PORT_BUNIT:
 	case PORT_PUNIT:
@@ -482,7 +505,7 @@ static int parse_engine(struct reg *reg, const char *s)
 
 	e = find_engine(s);
 	if (e) {
-		reg->port_desc.port = PORT_MMIO;
+		reg->port_desc.port = PORT_MMIO_32;
 		reg->port_desc.name = strdup(s);
 		reg->port_desc.stride = 4;
 		reg->engine = strdup(s);
@@ -629,7 +652,7 @@ static int intel_reg_dump(struct config *config, int argc, char *argv[])
 		reg = &config->regs[i];
 
 		/* can't dump sideband with mmiofile */
-		if (config->mmiofile && reg->port_desc.port != PORT_MMIO)
+		if (config->mmiofile && !port_is_mmio(reg->port_desc.port))
 			continue;
 
 		dump_register(config, &config->regs[i]);
diff --git a/tools/intel_reg_spec.c b/tools/intel_reg_spec.c
index 5ab56ec1a31f..7c1c6ed34a9e 100644
--- a/tools/intel_reg_spec.c
+++ b/tools/intel_reg_spec.c
@@ -35,17 +35,22 @@
 static const struct port_desc port_descs[] = {
 	{
 		.name = "mmio",
-		.port = PORT_MMIO,
+		.port = PORT_MMIO_32,
 		.stride = 4,
 	},
 	{
-		.name = "portio-vga",
-		.port = PORT_PORTIO_VGA,
+		.name = "mmio16",
+		.port = PORT_MMIO_16,
+		.stride = 2,
+	},
+	{
+		.name = "mmio8",
+		.port = PORT_MMIO_8,
 		.stride = 1,
 	},
 	{
-		.name = "mmio-vga",
-		.port = PORT_MMIO_VGA,
+		.name = "portio",
+		.port = PORT_PORTIO,
 		.stride = 1,
 	},
 	{
@@ -116,7 +121,7 @@ int parse_port_desc(struct reg *reg, const char *s)
 		if (endp > s && *endp == 0) {
 			if (n > PORT_MAX) {
 				/* Not a sideband port, assume MMIO offset. */
-				port = PORT_MMIO;
+				port = PORT_MMIO_32;
 				reg->mmio_offset = n;
 			} else {
 				port = n;
@@ -127,7 +132,7 @@ int parse_port_desc(struct reg *reg, const char *s)
 		}
 	} else {
 		/* No port, default to searching for MMIO. */
-		port = PORT_MMIO;
+		port = PORT_MMIO_32;
 		reg->mmio_offset = 0;
 	}
 
diff --git a/tools/intel_reg_spec.h b/tools/intel_reg_spec.h
index a6e0514387df..29f4d9ae2a47 100644
--- a/tools/intel_reg_spec.h
+++ b/tools/intel_reg_spec.h
@@ -25,9 +25,11 @@
 #define __INTEL_REG_SPEC_H__
 
 enum port_addr {
-	PORT_MMIO = -127,
-	PORT_PORTIO_VGA,	/* see vga reg read/write */
-	PORT_MMIO_VGA,		/* see vga reg read/write */
+	PORT_MMIO_32 = -127,
+	PORT_MMIO_16,
+	PORT_MMIO_8,
+
+	PORT_PORTIO,
 
 	PORT_NONE = 0,
 
-- 
2.24.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 3/4] tools/intel_reg: Add support for register access via the MCHBAR mirror

[Ville Syrjala]

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

Remembering the MCHBAR mirror offset is too much work. Make the tool
remember it for me.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tools/intel_reg.c      | 17 +++++++++++++++++
 tools/intel_reg_spec.c | 15 +++++++++++++++
 tools/intel_reg_spec.h |  4 ++++
 3 files changed, 36 insertions(+)

diff --git a/tools/intel_reg.c b/tools/intel_reg.c
index 7f8ddbd8e927..69af0f5a19ff 100644
--- a/tools/intel_reg.c
+++ b/tools/intel_reg.c
@@ -185,6 +185,9 @@ static bool port_is_mmio(enum port_addr port)
 	case PORT_MMIO_32:
 	case PORT_MMIO_16:
 	case PORT_MMIO_8:
+	case PORT_MCHBAR_32:
+	case PORT_MCHBAR_16:
+	case PORT_MCHBAR_8:
 		return true;
 	default:
 		return false;
@@ -366,20 +369,34 @@ static int register_srm(struct config *config, struct reg *reg,
 	return val;
 }
 
+static uint32_t mcbar_offset(uint32_t devid)
+{
+	return intel_gen(devid) >= 6 ? 0x140000 : 0x10000;
+}
+
 static int read_register(struct config *config, struct reg *reg, uint32_t *valp)
 {
 	uint32_t val = 0;
 
 	switch (reg->port_desc.port) {
+	case PORT_MCHBAR_32:
+		reg->mmio_offset = mcbar_offset(config->devid);
+		/* fall through */
 	case PORT_MMIO_32:
 		if (reg->engine)
 			val = register_srm(config, reg, NULL);
 		else
 			val = INREG(reg->mmio_offset + reg->addr);
 		break;
+	case PORT_MCHBAR_16:
+		reg->mmio_offset = mcbar_offset(config->devid);
+		/* fall through */
 	case PORT_MMIO_16:
 		val = INREG16(reg->mmio_offset + reg->addr);
 		break;
+	case PORT_MCHBAR_8:
+		reg->mmio_offset = mcbar_offset(config->devid);
+		/* fall through */
 	case PORT_MMIO_8:
 		val = INREG8(reg->mmio_offset + reg->addr);
 		break;
diff --git a/tools/intel_reg_spec.c b/tools/intel_reg_spec.c
index 7c1c6ed34a9e..dadd844730f0 100644
--- a/tools/intel_reg_spec.c
+++ b/tools/intel_reg_spec.c
@@ -48,6 +48,21 @@ static const struct port_desc port_descs[] = {
 		.port = PORT_MMIO_8,
 		.stride = 1,
 	},
+	{
+		.name = "mchbar",
+		.port = PORT_MCHBAR_32,
+		.stride = 4,
+	},
+	{
+		.name = "mchbar16",
+		.port = PORT_MCHBAR_16,
+		.stride = 2,
+	},
+	{
+		.name = "mchbar8",
+		.port = PORT_MCHBAR_8,
+		.stride = 1,
+	},
 	{
 		.name = "portio",
 		.port = PORT_PORTIO,
diff --git a/tools/intel_reg_spec.h b/tools/intel_reg_spec.h
index 29f4d9ae2a47..fc27a9a62f6f 100644
--- a/tools/intel_reg_spec.h
+++ b/tools/intel_reg_spec.h
@@ -29,6 +29,10 @@ enum port_addr {
 	PORT_MMIO_16,
 	PORT_MMIO_8,
 
+	PORT_MCHBAR_32,
+	PORT_MCHBAR_16,
+	PORT_MCHBAR_8,
+
 	PORT_PORTIO,
 
 	PORT_NONE = 0,
-- 
2.24.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 4/4] tools/intel_reg: Add support for reading indexed VGA registers

[Ville Syrjala]

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

Add convenient "ports" for accessing the indexed VGA registers.
SAdly we still have to poke these at times.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 lib/intel_reg.h        |  15 ++-
 tools/intel_reg.c      | 201 +++++++++++++++++++++++++++++++++++++++++
 tools/intel_reg_spec.c |  40 ++++++++
 tools/intel_reg_spec.h |  10 ++
 4 files changed, 263 insertions(+), 3 deletions(-)

diff --git a/lib/intel_reg.h b/lib/intel_reg.h
index 0a9ee34cd90d..036dfec51e7d 100644
--- a/lib/intel_reg.h
+++ b/lib/intel_reg.h
@@ -46,9 +46,18 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 /* I/O register offsets
  */
-#define SRX 0x3C4		/* p208 */
-#define GRX 0x3CE		/* p213 */
-#define ARX 0x3C0		/* p224 */
+#define CRX_MDA		0x3B4
+#define CRD_MDA		0x3B5
+#define ST01_MDA	0x3BA
+#define ARX		0x3C0
+#define ARD_R		0x3C1
+#define SRX		0x3C4
+#define SRD		0x3C5
+#define GRX		0x3CE
+#define GRD		0x3CF
+#define CRX_CGA		0x3D4
+#define CRD_CGA		0x3D5
+#define ST01_CGA	0x3DA
 
 /* VGA Color Palette Registers */
 #define DACMASK  0x3C6		/* p232 */
diff --git a/tools/intel_reg.c b/tools/intel_reg.c
index 69af0f5a19ff..f46b4849ddd8 100644
--- a/tools/intel_reg.c
+++ b/tools/intel_reg.c
@@ -374,6 +374,119 @@ static uint32_t mcbar_offset(uint32_t devid)
 	return intel_gen(devid) >= 6 ? 0x140000 : 0x10000;
 }
 
+static bool vga_is_cga_mode(bool mmio)
+{
+	if (mmio)
+		return INREG8(MSR_R) & IO_ADDR_SELECT;
+	else
+		return inb(MSR_R) & IO_ADDR_SELECT;
+}
+
+static uint16_t vga_st01(bool mmio)
+{
+	if (vga_is_cga_mode(mmio))
+		return ST01_CGA;
+	else
+		return ST01_MDA;
+}
+
+static uint16_t vga_crx(bool mmio)
+{
+	if (vga_is_cga_mode(mmio))
+		return CRX_CGA;
+	else
+		return CRX_MDA;
+}
+
+static uint16_t vga_crd(bool mmio)
+{
+	if (vga_is_cga_mode(mmio))
+		return CRD_CGA;
+	else
+		return CRD_MDA;
+}
+
+static uint8_t vga_read(uint16_t index_reg, uint16_t data_reg,
+			uint8_t index, bool mmio)
+{
+	uint8_t val;
+
+	if (mmio) {
+		if (index_reg == ARX)
+			INREG(vga_st01(mmio)); /* reset flip-flop */
+		OUTREG8(index_reg, index);
+		val = INREG8(data_reg);
+	} else {
+		iopl(3);
+		if (index_reg == ARX)
+			inb(vga_st01(mmio)); /* reset flip-flop */
+		outb(index, index_reg);
+		val = inb(data_reg);
+		iopl(0);
+	}
+
+	return val;
+}
+
+static void vga_write(uint16_t index_reg, uint16_t data_reg,
+		      uint8_t index, uint8_t value, bool mmio)
+{
+	if (mmio) {
+		if (index_reg == ARX)
+			INREG(vga_st01(mmio)); /* reset flip-flop */
+		OUTREG8(index_reg, index);
+		OUTREG(data_reg, value);
+	} else {
+		iopl(3);
+		if (index_reg == ARX)
+			inb(vga_st01(mmio)); /* reset flip-flop */
+		outb(index, index_reg);
+		outb(value, data_reg);
+		iopl(0);
+	}
+}
+
+static uint8_t vga_ar_read(uint8_t index, bool mmio)
+{
+	return vga_read(ARX, ARD_R, index, mmio);
+}
+
+static void vga_ar_write(uint8_t index, uint8_t value, bool mmio)
+{
+	/* Yes, ARX is where you write the data! */
+	vga_write(ARX, ARX, index, value, mmio);
+}
+
+static uint8_t vga_sr_read(uint8_t index, bool mmio)
+{
+	return vga_read(SRX, SRD, index, mmio);
+}
+
+static void vga_sr_write(uint8_t index, uint8_t value, bool mmio)
+{
+	vga_write(SRX, SRD, index, value, mmio);
+}
+
+static uint8_t vga_gr_read(uint8_t index, bool mmio)
+{
+	return vga_read(GRX, GRD, index, mmio);
+}
+
+static void vga_gr_write(uint8_t index, uint8_t value, bool mmio)
+{
+	vga_write(GRX, GRD, index, value, mmio);
+}
+
+static uint8_t vga_cr_read(uint8_t index, bool mmio)
+{
+	return vga_read(vga_crx(mmio), vga_crd(mmio), index, mmio);
+}
+
+static void vga_cr_write(uint8_t index, uint8_t value, bool mmio)
+{
+	vga_write(vga_crx(mmio), vga_crd(mmio), index, value, mmio);
+}
+
 static int read_register(struct config *config, struct reg *reg, uint32_t *valp)
 {
 	uint32_t val = 0;
@@ -400,11 +513,35 @@ static int read_register(struct config *config, struct reg *reg, uint32_t *valp)
 	case PORT_MMIO_8:
 		val = INREG8(reg->mmio_offset + reg->addr);
 		break;
+	case PORT_MMIO_VGA_AR:
+		val = vga_ar_read(reg->addr, true);
+		break;
+	case PORT_MMIO_VGA_SR:
+		val = vga_sr_read(reg->addr, true);
+		break;
+	case PORT_MMIO_VGA_GR:
+		val = vga_gr_read(reg->addr, true);
+		break;
+	case PORT_MMIO_VGA_CR:
+		val = vga_cr_read(reg->addr, true);
+		break;
 	case PORT_PORTIO:
 		iopl(3);
 		val = inb(reg->addr);
 		iopl(0);
 		break;
+	case PORT_PORTIO_VGA_AR:
+		val = vga_ar_read(reg->addr, false);
+		break;
+	case PORT_PORTIO_VGA_SR:
+		val = vga_sr_read(reg->addr, false);
+		break;
+	case PORT_PORTIO_VGA_GR:
+		val = vga_gr_read(reg->addr, false);
+		break;
+	case PORT_PORTIO_VGA_CR:
+		val = vga_cr_read(reg->addr, false);
+		break;
 	case PORT_BUNIT:
 	case PORT_PUNIT:
 	case PORT_NC:
@@ -474,6 +611,38 @@ static int write_register(struct config *config, struct reg *reg, uint32_t val)
 		}
 		OUTREG8(reg->mmio_offset + reg->addr, val);
 		break;
+	case PORT_MMIO_VGA_AR:
+		if (val > 0xff) {
+			fprintf(stderr, "value 0x%08x out of range for port %s\n",
+				val, reg->port_desc.name);
+			return -1;
+		}
+		vga_ar_write(reg->addr, val, true);
+		break;
+	case PORT_MMIO_VGA_SR:
+		if (val > 0xff) {
+			fprintf(stderr, "value 0x%08x out of range for port %s\n",
+				val, reg->port_desc.name);
+			return -1;
+		}
+		vga_sr_write(reg->addr, val, true);
+		break;
+	case PORT_MMIO_VGA_GR:
+		if (val > 0xff) {
+			fprintf(stderr, "value 0x%08x out of range for port %s\n",
+				val, reg->port_desc.name);
+			return -1;
+		}
+		vga_gr_write(reg->addr, val, true);
+		break;
+	case PORT_MMIO_VGA_CR:
+		if (val > 0xff) {
+			fprintf(stderr, "value 0x%08x out of range for port %s\n",
+				val, reg->port_desc.name);
+			return -1;
+		}
+		vga_cr_write(reg->addr, val, true);
+		break;
 	case PORT_PORTIO:
 		if (val > 0xff) {
 			fprintf(stderr, "value 0x%08x out of range for port %s\n",
@@ -484,6 +653,38 @@ static int write_register(struct config *config, struct reg *reg, uint32_t val)
 		outb(val, reg->addr);
 		iopl(0);
 		break;
+	case PORT_PORTIO_VGA_AR:
+		if (val > 0xff) {
+			fprintf(stderr, "value 0x%08x out of range for port %s\n",
+				val, reg->port_desc.name);
+			return -1;
+		}
+		vga_ar_write(reg->addr, val, false);
+		break;
+	case PORT_PORTIO_VGA_SR:
+		if (val > 0xff) {
+			fprintf(stderr, "value 0x%08x out of range for port %s\n",
+				val, reg->port_desc.name);
+			return -1;
+		}
+		vga_sr_write(reg->addr, val, false);
+		break;
+	case PORT_PORTIO_VGA_GR:
+		if (val > 0xff) {
+			fprintf(stderr, "value 0x%08x out of range for port %s\n",
+				val, reg->port_desc.name);
+			return -1;
+		}
+		vga_gr_write(reg->addr, val, false);
+		break;
+	case PORT_PORTIO_VGA_CR:
+		if (val > 0xff) {
+			fprintf(stderr, "value 0x%08x out of range for port %s\n",
+				val, reg->port_desc.name);
+			return -1;
+		}
+		vga_cr_write(reg->addr, val, false);
+		break;
 	case PORT_BUNIT:
 	case PORT_PUNIT:
 	case PORT_NC:
diff --git a/tools/intel_reg_spec.c b/tools/intel_reg_spec.c
index dadd844730f0..632db6b9f045 100644
--- a/tools/intel_reg_spec.c
+++ b/tools/intel_reg_spec.c
@@ -63,11 +63,51 @@ static const struct port_desc port_descs[] = {
 		.port = PORT_MCHBAR_8,
 		.stride = 1,
 	},
+	{
+		.name = "mmio-ar",
+		.port = PORT_MMIO_VGA_AR,
+		.stride = 1,
+	},
+	{
+		.name = "mmio-sr",
+		.port = PORT_MMIO_VGA_SR,
+		.stride = 1,
+	},
+	{
+		.name = "mmio-gr",
+		.port = PORT_MMIO_VGA_GR,
+		.stride = 1,
+	},
+	{
+		.name = "mmio-cr",
+		.port = PORT_MMIO_VGA_CR,
+		.stride = 1,
+	},
 	{
 		.name = "portio",
 		.port = PORT_PORTIO,
 		.stride = 1,
 	},
+	{
+		.name = "portio-ar",
+		.port = PORT_PORTIO_VGA_AR,
+		.stride = 1,
+	},
+	{
+		.name = "portio-sr",
+		.port = PORT_PORTIO_VGA_SR,
+		.stride = 1,
+	},
+	{
+		.name = "portio-gr",
+		.port = PORT_PORTIO_VGA_GR,
+		.stride = 1,
+	},
+	{
+		.name = "portio-cr",
+		.port = PORT_PORTIO_VGA_CR,
+		.stride = 1,
+	},
 	{
 		.name = "bunit",
 		.port = PORT_BUNIT,
diff --git a/tools/intel_reg_spec.h b/tools/intel_reg_spec.h
index fc27a9a62f6f..06d9bc257558 100644
--- a/tools/intel_reg_spec.h
+++ b/tools/intel_reg_spec.h
@@ -33,8 +33,18 @@ enum port_addr {
 	PORT_MCHBAR_16,
 	PORT_MCHBAR_8,
 
+	PORT_MMIO_VGA_AR,
+	PORT_MMIO_VGA_SR,
+	PORT_MMIO_VGA_GR,
+	PORT_MMIO_VGA_CR,
+
 	PORT_PORTIO,
 
+	PORT_PORTIO_VGA_AR,
+	PORT_PORTIO_VGA_SR,
+	PORT_PORTIO_VGA_GR,
+	PORT_PORTIO_VGA_CR,
+
 	PORT_NONE = 0,
 
 	/* vlv */
-- 
2.24.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,1/4] tools/intel_reg: Simplify negative ports

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/4] tools/intel_reg: Simplify negative ports
URL   : https://patchwork.freedesktop.org/series/74389/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8080 -> IGTPW_4272
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_4272 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_4272, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4272/index.html

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@runner@aborted:
    - fi-hsw-peppy:       NOTRUN -> [FAIL][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4272/fi-hsw-peppy/igt@runner@aborted.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@i915_selftest@live@ring_submission}:
    - fi-hsw-peppy:       [PASS][2] -> [INCOMPLETE][3]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8080/fi-hsw-peppy/igt@i915_selftest@live@ring_submission.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4272/fi-hsw-peppy/igt@i915_selftest@live@ring_submission.html

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [PASS][4] -> [DMESG-WARN][5] ([i915#44])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8080/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4272/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  * igt@prime_vgem@basic-read:
    - fi-tgl-y:           [PASS][6] -> [DMESG-WARN][7] ([CI#94] / [i915#402]) +2 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8080/fi-tgl-y/igt@prime_vgem@basic-read.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4272/fi-tgl-y/igt@prime_vgem@basic-read.html

  
#### Possible fixes ####

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][8] ([fdo#111096] / [i915#323]) -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8080/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4272/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@prime_self_import@basic-llseek-size:
    - fi-tgl-y:           [DMESG-WARN][10] ([CI#94] / [i915#402]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8080/fi-tgl-y/igt@prime_self_import@basic-llseek-size.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4272/fi-tgl-y/igt@prime_self_import@basic-llseek-size.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [CI#94]: https://gitlab.freedesktop.org/gfx-ci/i915-infra/issues/94
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [i915#323]: https://gitlab.freedesktop.org/drm/intel/issues/323
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#44]: https://gitlab.freedesktop.org/drm/intel/issues/44


Participating hosts (47 -> 44)
------------------------------

  Additional (3): fi-gdg-551 fi-ivb-3770 fi-snb-2520m 
  Missing    (6): fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_5495 -> IGTPW_4272

  CI-20190529: 20190529
  CI_DRM_8080: d122b6eaf8f3999f69ddd9cfde05cc0775e0e68e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_4272: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4272/index.html
  IGT_5495: 22df72de8affcec22d9f354bb6148d77f60cc580 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4272/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/4] tools/intel_reg: Simplify negative ports

[Jani Nikula]

On Fri, 06 Mar 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Let's avoid having to type in the explicit negative value for
> all the port enums.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  tools/intel_reg_spec.h | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/tools/intel_reg_spec.h b/tools/intel_reg_spec.h
> index c94c61bab3f8..a6e0514387df 100644
> --- a/tools/intel_reg_spec.h
> +++ b/tools/intel_reg_spec.h
> @@ -25,10 +25,11 @@
>  #define __INTEL_REG_SPEC_H__
>  

Might copy-paste this comment

/* Negative port numbers are not real sideband ports. */

from intel_reg.c somewhere here.

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

>  enum port_addr {
> +	PORT_MMIO = -127,
> +	PORT_PORTIO_VGA,	/* see vga reg read/write */
> +	PORT_MMIO_VGA,		/* see vga reg read/write */
> +
>  	PORT_NONE = 0,
> -	PORT_MMIO = -1,
> -	PORT_PORTIO_VGA = -2,	/* see vga reg read/write */
> -	PORT_MMIO_VGA = -3,	/* see vga reg read/write */
>  
>  	/* vlv */
>  	PORT_BUNIT = 0x03,

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/4] tools/intel_reg: Support 8 and 16 bit mmio registers

[Jani Nikula]

On Fri, 06 Mar 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Add new "ports" for doing 8 and 16 bit mmio accesses.

Might mention the mmio-vga -> mmio8 and portio-vga -> portio port name
changes here.

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


>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  tools/intel_reg.c      | 53 ++++++++++++++++++++++++++++++------------
>  tools/intel_reg_spec.c | 19 +++++++++------
>  tools/intel_reg_spec.h |  8 ++++---
>  3 files changed, 55 insertions(+), 25 deletions(-)
>
> diff --git a/tools/intel_reg.c b/tools/intel_reg.c
> index 5f1beba4a269..7f8ddbd8e927 100644
> --- a/tools/intel_reg.c
> +++ b/tools/intel_reg.c
> @@ -179,6 +179,18 @@ static void to_binary(char *buf, size_t buflen, uint32_t val)
>  	snprintf(buf, buflen, "\n");
>  }
>  
> +static bool port_is_mmio(enum port_addr port)
> +{
> +	switch (port) {
> +	case PORT_MMIO_32:
> +	case PORT_MMIO_16:
> +	case PORT_MMIO_8:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
>  static void dump_decode(struct config *config, struct reg *reg, uint32_t val)
>  {
>  	char decode[1300];
> @@ -207,7 +219,7 @@ static void dump_decode(struct config *config, struct reg *reg, uint32_t val)
>  		snprintf(decode, sizeof(decode), "\n");
>  	}
>  
> -	if (reg->port_desc.port == PORT_MMIO) {
> +	if (port_is_mmio(reg->port_desc.port)) {
>  		/* Omit port name for MMIO, optionally include MMIO offset. */
>  		if (reg->mmio_offset)
>  			printf("%24s (0x%08x:0x%08x): 0x%08x%s",
> @@ -359,20 +371,23 @@ static int read_register(struct config *config, struct reg *reg, uint32_t *valp)
>  	uint32_t val = 0;
>  
>  	switch (reg->port_desc.port) {
> -	case PORT_MMIO:
> +	case PORT_MMIO_32:
>  		if (reg->engine)
>  			val = register_srm(config, reg, NULL);
>  		else
>  			val = INREG(reg->mmio_offset + reg->addr);
>  		break;
> -	case PORT_PORTIO_VGA:
> +	case PORT_MMIO_16:
> +		val = INREG16(reg->mmio_offset + reg->addr);
> +		break;
> +	case PORT_MMIO_8:
> +		val = INREG8(reg->mmio_offset + reg->addr);
> +		break;
> +	case PORT_PORTIO:
>  		iopl(3);
>  		val = inb(reg->addr);
>  		iopl(0);
>  		break;
> -	case PORT_MMIO_VGA:
> -		val = INREG8(reg->addr);
> -		break;
>  	case PORT_BUNIT:
>  	case PORT_PUNIT:
>  	case PORT_NC:
> @@ -419,30 +434,38 @@ static int write_register(struct config *config, struct reg *reg, uint32_t val)
>  	}
>  
>  	switch (reg->port_desc.port) {
> -	case PORT_MMIO:
> +	case PORT_MMIO_32:
>  		if (reg->engine) {
>  			register_srm(config, reg, &val);
>  		} else {
>  			OUTREG(reg->mmio_offset + reg->addr, val);
>  		}
>  		break;
> -	case PORT_PORTIO_VGA:
> +	case PORT_MMIO_16:
> +		if (val > 0xffff) {
> +			fprintf(stderr, "value 0x%08x out of range for port %s\n",
> +				val, reg->port_desc.name);
> +			return -1;
> +		}
> +		OUTREG16(reg->mmio_offset + reg->addr, val);
> +		break;
> +	case PORT_MMIO_8:
>  		if (val > 0xff) {
>  			fprintf(stderr, "value 0x%08x out of range for port %s\n",
>  				val, reg->port_desc.name);
>  			return -1;
>  		}
> -		iopl(3);
> -		outb(val, reg->addr);
> -		iopl(0);
> +		OUTREG8(reg->mmio_offset + reg->addr, val);
>  		break;
> -	case PORT_MMIO_VGA:
> +	case PORT_PORTIO:
>  		if (val > 0xff) {
>  			fprintf(stderr, "value 0x%08x out of range for port %s\n",
>  				val, reg->port_desc.name);
>  			return -1;
>  		}
> -		OUTREG8(reg->addr, val);
> +		iopl(3);
> +		outb(val, reg->addr);
> +		iopl(0);
>  		break;
>  	case PORT_BUNIT:
>  	case PORT_PUNIT:
> @@ -482,7 +505,7 @@ static int parse_engine(struct reg *reg, const char *s)
>  
>  	e = find_engine(s);
>  	if (e) {
> -		reg->port_desc.port = PORT_MMIO;
> +		reg->port_desc.port = PORT_MMIO_32;
>  		reg->port_desc.name = strdup(s);
>  		reg->port_desc.stride = 4;
>  		reg->engine = strdup(s);
> @@ -629,7 +652,7 @@ static int intel_reg_dump(struct config *config, int argc, char *argv[])
>  		reg = &config->regs[i];
>  
>  		/* can't dump sideband with mmiofile */
> -		if (config->mmiofile && reg->port_desc.port != PORT_MMIO)
> +		if (config->mmiofile && !port_is_mmio(reg->port_desc.port))
>  			continue;
>  
>  		dump_register(config, &config->regs[i]);
> diff --git a/tools/intel_reg_spec.c b/tools/intel_reg_spec.c
> index 5ab56ec1a31f..7c1c6ed34a9e 100644
> --- a/tools/intel_reg_spec.c
> +++ b/tools/intel_reg_spec.c
> @@ -35,17 +35,22 @@
>  static const struct port_desc port_descs[] = {
>  	{
>  		.name = "mmio",
> -		.port = PORT_MMIO,
> +		.port = PORT_MMIO_32,
>  		.stride = 4,
>  	},
>  	{
> -		.name = "portio-vga",
> -		.port = PORT_PORTIO_VGA,
> +		.name = "mmio16",
> +		.port = PORT_MMIO_16,
> +		.stride = 2,
> +	},
> +	{
> +		.name = "mmio8",
> +		.port = PORT_MMIO_8,
>  		.stride = 1,
>  	},
>  	{
> -		.name = "mmio-vga",
> -		.port = PORT_MMIO_VGA,
> +		.name = "portio",
> +		.port = PORT_PORTIO,
>  		.stride = 1,
>  	},
>  	{
> @@ -116,7 +121,7 @@ int parse_port_desc(struct reg *reg, const char *s)
>  		if (endp > s && *endp == 0) {
>  			if (n > PORT_MAX) {
>  				/* Not a sideband port, assume MMIO offset. */
> -				port = PORT_MMIO;
> +				port = PORT_MMIO_32;
>  				reg->mmio_offset = n;
>  			} else {
>  				port = n;
> @@ -127,7 +132,7 @@ int parse_port_desc(struct reg *reg, const char *s)
>  		}
>  	} else {
>  		/* No port, default to searching for MMIO. */
> -		port = PORT_MMIO;
> +		port = PORT_MMIO_32;
>  		reg->mmio_offset = 0;
>  	}
>  
> diff --git a/tools/intel_reg_spec.h b/tools/intel_reg_spec.h
> index a6e0514387df..29f4d9ae2a47 100644
> --- a/tools/intel_reg_spec.h
> +++ b/tools/intel_reg_spec.h
> @@ -25,9 +25,11 @@
>  #define __INTEL_REG_SPEC_H__
>  
>  enum port_addr {
> -	PORT_MMIO = -127,
> -	PORT_PORTIO_VGA,	/* see vga reg read/write */
> -	PORT_MMIO_VGA,		/* see vga reg read/write */
> +	PORT_MMIO_32 = -127,
> +	PORT_MMIO_16,
> +	PORT_MMIO_8,
> +
> +	PORT_PORTIO,
>  
>  	PORT_NONE = 0,

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 3/4] tools/intel_reg: Add support for register access via the MCHBAR mirror

[Jani Nikula]

On Fri, 06 Mar 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Remembering the MCHBAR mirror offset is too much work. Make the tool
> remember it for me.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  tools/intel_reg.c      | 17 +++++++++++++++++
>  tools/intel_reg_spec.c | 15 +++++++++++++++
>  tools/intel_reg_spec.h |  4 ++++
>  3 files changed, 36 insertions(+)
>
> diff --git a/tools/intel_reg.c b/tools/intel_reg.c
> index 7f8ddbd8e927..69af0f5a19ff 100644
> --- a/tools/intel_reg.c
> +++ b/tools/intel_reg.c
> @@ -185,6 +185,9 @@ static bool port_is_mmio(enum port_addr port)
>  	case PORT_MMIO_32:
>  	case PORT_MMIO_16:
>  	case PORT_MMIO_8:
> +	case PORT_MCHBAR_32:
> +	case PORT_MCHBAR_16:
> +	case PORT_MCHBAR_8:
>  		return true;
>  	default:
>  		return false;
> @@ -366,20 +369,34 @@ static int register_srm(struct config *config, struct reg *reg,
>  	return val;
>  }
>  
> +static uint32_t mcbar_offset(uint32_t devid)
> +{
> +	return intel_gen(devid) >= 6 ? 0x140000 : 0x10000;
> +}
> +
>  static int read_register(struct config *config, struct reg *reg, uint32_t *valp)
>  {
>  	uint32_t val = 0;
>  
>  	switch (reg->port_desc.port) {
> +	case PORT_MCHBAR_32:
> +		reg->mmio_offset = mcbar_offset(config->devid);
> +		/* fall through */

I think it would be better to handle this earlier, at parse_port_desc()
or parse_reg() level. I think having this here might screw with the
ability to handle the mchbar "ports" nicely in the reg spec. Or decode
them.

This being devid specific makes it a bit harder I guess.

BR,
Jani.


>  	case PORT_MMIO_32:
>  		if (reg->engine)
>  			val = register_srm(config, reg, NULL);
>  		else
>  			val = INREG(reg->mmio_offset + reg->addr);
>  		break;
> +	case PORT_MCHBAR_16:
> +		reg->mmio_offset = mcbar_offset(config->devid);
> +		/* fall through */
>  	case PORT_MMIO_16:
>  		val = INREG16(reg->mmio_offset + reg->addr);
>  		break;
> +	case PORT_MCHBAR_8:
> +		reg->mmio_offset = mcbar_offset(config->devid);
> +		/* fall through */
>  	case PORT_MMIO_8:
>  		val = INREG8(reg->mmio_offset + reg->addr);
>  		break;
> diff --git a/tools/intel_reg_spec.c b/tools/intel_reg_spec.c
> index 7c1c6ed34a9e..dadd844730f0 100644
> --- a/tools/intel_reg_spec.c
> +++ b/tools/intel_reg_spec.c
> @@ -48,6 +48,21 @@ static const struct port_desc port_descs[] = {
>  		.port = PORT_MMIO_8,
>  		.stride = 1,
>  	},
> +	{
> +		.name = "mchbar",
> +		.port = PORT_MCHBAR_32,
> +		.stride = 4,
> +	},
> +	{
> +		.name = "mchbar16",
> +		.port = PORT_MCHBAR_16,
> +		.stride = 2,
> +	},
> +	{
> +		.name = "mchbar8",
> +		.port = PORT_MCHBAR_8,
> +		.stride = 1,
> +	},
>  	{
>  		.name = "portio",
>  		.port = PORT_PORTIO,
> diff --git a/tools/intel_reg_spec.h b/tools/intel_reg_spec.h
> index 29f4d9ae2a47..fc27a9a62f6f 100644
> --- a/tools/intel_reg_spec.h
> +++ b/tools/intel_reg_spec.h
> @@ -29,6 +29,10 @@ enum port_addr {
>  	PORT_MMIO_16,
>  	PORT_MMIO_8,
>  
> +	PORT_MCHBAR_32,
> +	PORT_MCHBAR_16,
> +	PORT_MCHBAR_8,
> +
>  	PORT_PORTIO,
>  
>  	PORT_NONE = 0,

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 2/4] tools/intel_reg: Support 8 and 16 bit mmio registers

[Ville Syrjala]

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

Add new "ports" for doing 8 and 16 bit mmio accesses.

Note that we rename a few of the existing "ports" for
consistency:
- mmio-vga -> mmio8
- portio-vga -> portio

v2: Mention the renames (Jani)

Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tools/intel_reg.c      | 59 +++++++++++++++++++++++++++++-------------
 tools/intel_reg_spec.c | 19 +++++++++-----
 tools/intel_reg_spec.h |  8 +++---
 3 files changed, 58 insertions(+), 28 deletions(-)

diff --git a/tools/intel_reg.c b/tools/intel_reg.c
index 584a2ccd23f6..c59fa0c34034 100644
--- a/tools/intel_reg.c
+++ b/tools/intel_reg.c
@@ -180,6 +180,18 @@ static void to_binary(char *buf, size_t buflen, uint32_t val)
 	snprintf(buf, buflen, "\n");
 }
 
+static bool port_is_mmio(enum port_addr port)
+{
+	switch (port) {
+	case PORT_MMIO_32:
+	case PORT_MMIO_16:
+	case PORT_MMIO_8:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static void dump_decode(struct config *config, struct reg *reg, uint32_t val)
 {
 	char decode[1300];
@@ -208,7 +220,7 @@ static void dump_decode(struct config *config, struct reg *reg, uint32_t val)
 		snprintf(decode, sizeof(decode), "\n");
 	}
 
-	if (reg->port_desc.port == PORT_MMIO) {
+	if (port_is_mmio(reg->port_desc.port)) {
 		/* Omit port name for MMIO, optionally include MMIO offset. */
 		if (reg->mmio_offset)
 			printf("%24s (0x%08x:0x%08x): 0x%08x%s",
@@ -360,20 +372,23 @@ static int read_register(struct config *config, struct reg *reg, uint32_t *valp)
 	uint32_t val = 0;
 
 	switch (reg->port_desc.port) {
-	case PORT_MMIO:
+	case PORT_MMIO_32:
 		if (reg->engine)
 			val = register_srm(config, reg, NULL);
 		else
 			val = INREG(reg->mmio_offset + reg->addr);
 		break;
-	case PORT_PORTIO_VGA:
+	case PORT_MMIO_16:
+		val = INREG16(reg->mmio_offset + reg->addr);
+		break;
+	case PORT_MMIO_8:
+		val = INREG8(reg->mmio_offset + reg->addr);
+		break;
+	case PORT_PORTIO:
 		iopl(3);
 		val = inb(reg->addr);
 		iopl(0);
 		break;
-	case PORT_MMIO_VGA:
-		val = INREG8(reg->addr);
-		break;
 	case PORT_BUNIT:
 	case PORT_PUNIT:
 	case PORT_NC:
@@ -420,14 +435,30 @@ static int write_register(struct config *config, struct reg *reg, uint32_t val)
 	}
 
 	switch (reg->port_desc.port) {
-	case PORT_MMIO:
+	case PORT_MMIO_32:
 		if (reg->engine) {
 			register_srm(config, reg, &val);
 		} else {
 			OUTREG(reg->mmio_offset + reg->addr, val);
 		}
 		break;
-	case PORT_PORTIO_VGA:
+	case PORT_MMIO_16:
+		if (val > 0xffff) {
+			fprintf(stderr, "value 0x%08x out of range for port %s\n",
+				val, reg->port_desc.name);
+			return -1;
+		}
+		OUTREG16(reg->mmio_offset + reg->addr, val);
+		break;
+	case PORT_MMIO_8:
+		if (val > 0xff) {
+			fprintf(stderr, "value 0x%08x out of range for port %s\n",
+				val, reg->port_desc.name);
+			return -1;
+		}
+		OUTREG8(reg->mmio_offset + reg->addr, val);
+		break;
+	case PORT_PORTIO:
 		if (val > 0xff) {
 			fprintf(stderr, "value 0x%08x out of range for port %s\n",
 				val, reg->port_desc.name);
@@ -437,14 +468,6 @@ static int write_register(struct config *config, struct reg *reg, uint32_t val)
 		outb(val, reg->addr);
 		iopl(0);
 		break;
-	case PORT_MMIO_VGA:
-		if (val > 0xff) {
-			fprintf(stderr, "value 0x%08x out of range for port %s\n",
-				val, reg->port_desc.name);
-			return -1;
-		}
-		OUTREG8(reg->addr, val);
-		break;
 	case PORT_BUNIT:
 	case PORT_PUNIT:
 	case PORT_NC:
@@ -483,7 +506,7 @@ static int parse_engine(struct reg *reg, const char *s)
 
 	e = find_engine(s);
 	if (e) {
-		reg->port_desc.port = PORT_MMIO;
+		reg->port_desc.port = PORT_MMIO_32;
 		reg->port_desc.name = strdup(s);
 		reg->port_desc.stride = 4;
 		reg->engine = strdup(s);
@@ -630,7 +653,7 @@ static int intel_reg_dump(struct config *config, int argc, char *argv[])
 		reg = &config->regs[i];
 
 		/* can't dump sideband with mmiofile */
-		if (config->mmiofile && reg->port_desc.port != PORT_MMIO)
+		if (config->mmiofile && !port_is_mmio(reg->port_desc.port))
 			continue;
 
 		dump_register(config, &config->regs[i]);
diff --git a/tools/intel_reg_spec.c b/tools/intel_reg_spec.c
index 5ab56ec1a31f..7c1c6ed34a9e 100644
--- a/tools/intel_reg_spec.c
+++ b/tools/intel_reg_spec.c
@@ -35,17 +35,22 @@
 static const struct port_desc port_descs[] = {
 	{
 		.name = "mmio",
-		.port = PORT_MMIO,
+		.port = PORT_MMIO_32,
 		.stride = 4,
 	},
 	{
-		.name = "portio-vga",
-		.port = PORT_PORTIO_VGA,
+		.name = "mmio16",
+		.port = PORT_MMIO_16,
+		.stride = 2,
+	},
+	{
+		.name = "mmio8",
+		.port = PORT_MMIO_8,
 		.stride = 1,
 	},
 	{
-		.name = "mmio-vga",
-		.port = PORT_MMIO_VGA,
+		.name = "portio",
+		.port = PORT_PORTIO,
 		.stride = 1,
 	},
 	{
@@ -116,7 +121,7 @@ int parse_port_desc(struct reg *reg, const char *s)
 		if (endp > s && *endp == 0) {
 			if (n > PORT_MAX) {
 				/* Not a sideband port, assume MMIO offset. */
-				port = PORT_MMIO;
+				port = PORT_MMIO_32;
 				reg->mmio_offset = n;
 			} else {
 				port = n;
@@ -127,7 +132,7 @@ int parse_port_desc(struct reg *reg, const char *s)
 		}
 	} else {
 		/* No port, default to searching for MMIO. */
-		port = PORT_MMIO;
+		port = PORT_MMIO_32;
 		reg->mmio_offset = 0;
 	}
 
diff --git a/tools/intel_reg_spec.h b/tools/intel_reg_spec.h
index 1a2c5ebd9a58..e8bd763adb3f 100644
--- a/tools/intel_reg_spec.h
+++ b/tools/intel_reg_spec.h
@@ -26,9 +26,11 @@
 
 enum port_addr {
 	/* Negative port numbers are not real sideband ports. */
-	PORT_MMIO = -127,
-	PORT_PORTIO_VGA,	/* see vga reg read/write */
-	PORT_MMIO_VGA,		/* see vga reg read/write */
+	PORT_MMIO_32 = -127,
+	PORT_MMIO_16,
+	PORT_MMIO_8,
+
+	PORT_PORTIO,
 
 	PORT_NONE = 0,
 
-- 
2.39.1