[PATCH umr] Allow * for ipname in --read/--write/--write-bit (v2)

Tom St Denis tstdenis82 at gmail.com
Thu Feb 23 18:51:43 UTC 2017


To simplify scripting you can now use * for ipnames as well.

(v2) Continue matching other IP blocks after first register hit.

Signed-off-by: Tom St Denis <tom.stdenis at amd.com>
---
 doc/umr.1         |   6 ++-
 src/app/main.c    |   5 ++-
 src/app/scan.c    |  32 ++++++++-------
 src/app/set_bit.c | 117 +++++++++++++++++++++++++++---------------------------
 src/app/set_reg.c |  91 +++++++++++++++++++++---------------------
 5 files changed, 128 insertions(+), 123 deletions(-)

diff --git a/doc/umr.1 b/doc/umr.1
index 5ed2c3bb93ae..a12dfb0a00ff 100644
--- a/doc/umr.1
+++ b/doc/umr.1
@@ -34,7 +34,7 @@ Look up an MMIO register by address and bitfield decode the value specified.
 Write a value specified in hex to a register specified with a complete
 register path in the form <
 .B asicname.ipname.regname
->.  For example, fiji.uvd6.mmUVD_CGC_GATE.  The value of asicname can be
+>.  For example, fiji.uvd6.mmUVD_CGC_GATE.  The value of asicname and/or ipname can be
 .B *
 to simplify scripting.  This command can be used multiple times to
 write to multiple registers in a single invocation.
@@ -52,7 +52,9 @@ command but also allows
 for the regname field.
 .IP "--scan, -s <string>"
 Scan and print an IP block by name, for example,
-.B uvd5.
+.B uvd6
+or
+.B carrizo.uvd6.
 Can be used multiple times in a single invocation.
 .IP "--ring, -R <string>(from:to)"
 Read the contents of a ring named by the string without the
diff --git a/src/app/main.c b/src/app/main.c
index 8364d54c5227..99e014888571 100644
--- a/src/app/main.c
+++ b/src/app/main.c
@@ -364,12 +364,13 @@ int main(int argc, char **argv)
 "\n\t--write, -w <address> <number>\n\t\tWrite a value in hex to a register specified as a register path in the"
 	"\n\t\tform <asicname.ipname.regname>.  For instance \"tonga.uvd5.mmUVD_SOFT_RESET\"."
 	"\n\t\tCan be used multiple times to set multiple registers.  You can"
-	"\n\t\tspecify * for asicname to simplify scripts.\n"
+	"\n\t\tspecify * for asicname and/or ipname to simplify scripts.\n"
 "\n\t--writebit, -wb <string> <number>\n\t\tWrite a value in hex to a register bitfield specified as in --write but"
 	"\n\t\tthe addition of the bitfield name.  For instance: \"*.gfx80.mmRLC_PG_CNTL.PG_OVERRIDE\"\n"
 "\n\t--read, -r <string>\n\t\tRead a value from a register and print it to stdout.  This command"
 	"\n\t\tuses the same path notation as --write.  It also accepts * for regname.\n"
-"\n\t--scan, -s <string>\n\t\tScan and print an ip block by name, e.g. \"uvd5\".  Can be used multiple times.\n"
+"\n\t--scan, -s <string>\n\t\tScan and print an ip block by name, e.g. \"uvd6\" or \"carrizo.uvd6\"."
+	"\n\t\tCan be used multiple times.\n"
 "\n\t--ring, -R <string>([from:to])\n\t\tRead the contents of a ring named by the string without the amdgpu_ring_ prefix. "
 	"\n\t\tBy default it will read and display the entire ring.  A starting and ending "
 	"\n\t\taddress can be specified in decimal or a '.' can be used to indicate relative "
diff --git a/src/app/scan.c b/src/app/scan.c
index 19c97fe1499d..e91fbd4649c8 100644
--- a/src/app/scan.c
+++ b/src/app/scan.c
@@ -27,27 +27,30 @@
 int umr_scan_asic(struct umr_asic *asic, char *asicname, char *ipname, char *regname)
 {
 	int r, fd;
-	int i, j, k;
+	int first, i, j, k;
 	uint64_t addr, scale;
 	char buf[256];
 
 	/* scan them all in order */
 	if (!asicname[0] || !strcmp(asicname, "*") || !strcmp(asicname, asic->asicname)) {
 		for (i = 0; i < asic->no_blocks; i++) {
-			if (!ipname[0] || !strcmp(ipname, asic->blocks[i]->ipname)) {
-				if (asic->blocks[i]->grant) {
-					r = asic->blocks[i]->grant(asic);
-					if (r) {
-						if (ipname[0]) {
-							fprintf(stderr, "[ERROR] Must specify at least one 'risky' option before scanning specific blocks.\n");
-							exit(EXIT_FAILURE);
-						}
-						continue;
-					}
-				}
+			if (!ipname[0] || ipname[0] == '*' || !strcmp(ipname, asic->blocks[i]->ipname)) {
+				first = 1;
 				for (j = 0; j < asic->blocks[i]->no_regs; j++) {
 					if (!regname[0] || !strcmp(regname, "*") || !strcmp(regname, asic->blocks[i]->regs[j].regname) ||
 					(options.many && strstr(asic->blocks[i]->regs[j].regname, regname))) {
+						// only grant if any regspec matches otherwise it's a waste
+						if (first && asic->blocks[i]->grant) {
+							first = 0;
+							r = asic->blocks[i]->grant(asic);
+							if (r) {
+								if (ipname[0]) {
+									fprintf(stderr, "[ERROR] Must specify at least one 'risky' option before scanning specific blocks.\n");
+									exit(EXIT_FAILURE);
+								}
+								continue;
+							}
+						}
 						if (asic->pci.mem == NULL) {
 							switch(asic->blocks[i]->regs[j].type) {
 							case REG_MMIO: fd = asic->fd.mmio; scale = 4; break;
@@ -89,7 +92,7 @@ int umr_scan_asic(struct umr_asic *asic, char *asicname, char *ipname, char *reg
 						}
 						if (regname[0]) {
 							if (options.named)
-								printf("%s%s%s => ", CYAN, asic->blocks[i]->regs[j].regname, RST);
+								printf("%s%s.%s%s => ", CYAN, asic->blocks[i]->ipname,  asic->blocks[i]->regs[j].regname, RST);
 							printf("%s0x%08lx%s\n", YELLOW, (unsigned long)asic->blocks[i]->regs[j].value, RST);
 							if (options.bitfields)
 								for (k = 0; k < asic->blocks[i]->regs[j].no_bits; k++) {
@@ -101,7 +104,8 @@ int umr_scan_asic(struct umr_asic *asic, char *asicname, char *ipname, char *reg
 						}
 					}
 				}
-				if (asic->blocks[i]->release) {
+				// only release if granted
+				if (!first && asic->blocks[i]->release) {
 					r = asic->blocks[i]->release(asic);
 					if (r)
 						goto error;
diff --git a/src/app/set_bit.c b/src/app/set_bit.c
index d9ee7d8f3a55..bed7ee858b6a 100644
--- a/src/app/set_bit.c
+++ b/src/app/set_bit.c
@@ -31,79 +31,78 @@
  */
 int umr_set_register_bit(struct umr_asic *asic, char *regpath, char *regvalue)
 {
-	char rpath[512];
+	char asicname[128], ipname[128], regname[128], bitname[128];
 	int i, j, k, fd;
 	uint32_t value, mask, copy;
 	uint64_t addr, scale;
 
-	/* scan until we compare with regpath... */
-	for (i = 0; i < asic->no_blocks; i++) {
-		if (!memcmp(regpath, "*.", 2))
-			snprintf(rpath, sizeof(rpath)-1, "*.%s.", asic->blocks[i]->ipname);
-		else
-			snprintf(rpath, sizeof(rpath)-1, "%s.%s.", asic->asicname, asic->blocks[i]->ipname);
-		if (memcmp(regpath, rpath, strlen(rpath)) == 0) {
-			for (j = 0; j < asic->blocks[i]->no_regs; j++) {
-				if (asic->blocks[i]->regs[j].bits) {
-					for (k = 0; k < asic->blocks[i]->regs[j].no_bits; k++) {
-						if (!memcmp(regpath, "*.", 2))
-							snprintf(rpath, sizeof(rpath)-1, "*.%s.%s.%s", asic->blocks[i]->ipname, asic->blocks[i]->regs[j].regname, asic->blocks[i]->regs[j].bits[k].regname);
-						else
-							snprintf(rpath, sizeof(rpath)-1, "%s.%s.%s.%s", asic->asicname, asic->blocks[i]->ipname, asic->blocks[i]->regs[j].regname, asic->blocks[i]->regs[j].bits[k].regname);
-						if (!strcmp(regpath, rpath)) {
-							sscanf(regvalue, "%"SCNx32, &value);
+	if (sscanf(regpath, "%[^.].%[^.].%[^.].%[^.]", asicname, ipname, regname, bitname) != 4) {
+		fprintf(stderr, "[ERROR]: Invalid regpath for bit write\n");
+		return -1;
+	}
 
-							mask = (1UL<<((1+asic->blocks[i]->regs[j].bits[k].stop)-asic->blocks[i]->regs[j].bits[k].start))-1;
-							mask <<= asic->blocks[i]->regs[j].bits[k].start;
-							if (asic->pci.mem == NULL) {
-								// set this register
-								switch (asic->blocks[i]->regs[j].type){
-								case REG_MMIO: fd = asic->fd.mmio; scale = 4; break;
-								case REG_DIDT: fd = asic->fd.didt; scale = 1; break;
-								case REG_PCIE: fd = asic->fd.pcie; scale = 1; break;
-								case REG_SMC:  fd = asic->fd.smc;  scale = 1; break;
-								default: return -1;
-								}
-								if (asic->blocks[i]->grant) {
-									if (asic->blocks[i]->grant(asic)) {
-										fprintf(stderr, "[ERROR] Must specify at least one 'risky' before writing to this block.\n");
-										return -1;
+	if (asicname[0] == '*' || !strcmp(asicname, asic->asicname)) {
+		/* scan until we compare with regpath... */
+		for (i = 0; i < asic->no_blocks; i++) {
+			if (ipname[0] == '*' || !strcmp(ipname, asic->blocks[i]->ipname)) {
+				for (j = 0; j < asic->blocks[i]->no_regs; j++) {
+					if (!strcmp(regname, asic->blocks[i]->regs[j].regname) && asic->blocks[i]->regs[j].bits) {
+						for (k = 0; k < asic->blocks[i]->regs[j].no_bits; k++) {
+							if (!strcmp(bitname, asic->blocks[i]->regs[j].bits[k].regname)) {
+								sscanf(regvalue, "%"SCNx32, &value);
+
+								mask = (1UL<<((1+asic->blocks[i]->regs[j].bits[k].stop)-asic->blocks[i]->regs[j].bits[k].start))-1;
+								mask <<= asic->blocks[i]->regs[j].bits[k].start;
+								if (asic->pci.mem == NULL) {
+									// set this register
+									switch (asic->blocks[i]->regs[j].type){
+									case REG_MMIO: fd = asic->fd.mmio; scale = 4; break;
+									case REG_DIDT: fd = asic->fd.didt; scale = 1; break;
+									case REG_PCIE: fd = asic->fd.pcie; scale = 1; break;
+									case REG_SMC:  fd = asic->fd.smc;  scale = 1; break;
+									default: return -1;
+									}
+									if (asic->blocks[i]->grant) {
+										if (asic->blocks[i]->grant(asic)) {
+											fprintf(stderr, "[ERROR] Must specify at least one 'risky' before writing to this block.\n");
+											return -1;
+										}
 									}
-								}
 
-								if (options.use_bank && asic->blocks[i]->regs[j].type == REG_MMIO)
-									addr =
-										(1ULL << 62) |
-										(((uint64_t)options.se_bank) << 24) |
-										(((uint64_t)options.sh_bank) << 34) |
-										(((uint64_t)options.instance_bank) << 44);
-								else
-									addr = 0;
+									if (options.use_bank && asic->blocks[i]->regs[j].type == REG_MMIO)
+										addr =
+											(1ULL << 62) |
+											(((uint64_t)options.se_bank) << 24) |
+											(((uint64_t)options.sh_bank) << 34) |
+											(((uint64_t)options.instance_bank) << 44);
+									else
+										addr = 0;
 
-								lseek(fd, addr | (asic->blocks[i]->regs[j].addr*scale), SEEK_SET);
-								read(fd, &copy, 4);
+									lseek(fd, addr | (asic->blocks[i]->regs[j].addr*scale), SEEK_SET);
+									read(fd, &copy, 4);
 
-								// read-modify-write value back
-								copy &= ~mask;
-								value = (value << asic->blocks[i]->regs[j].bits[k].start) & mask;
-								copy |= value;
+									// read-modify-write value back
+									copy &= ~mask;
+									value = (value << asic->blocks[i]->regs[j].bits[k].start) & mask;
+									copy |= value;
 
-								lseek(fd, addr | (asic->blocks[i]->regs[j].addr<<2), SEEK_SET);
-								write(fd, &copy, 4);
-								if (!options.quiet) printf("%s <= 0x%08lx\n", regpath, (unsigned long)copy);
+									lseek(fd, addr | (asic->blocks[i]->regs[j].addr<<2), SEEK_SET);
+									write(fd, &copy, 4);
+									if (!options.quiet) printf("%s <= 0x%08lx\n", regpath, (unsigned long)copy);
 
-								if (asic->blocks[i]->release) {
-									if (asic->blocks[i]->release(asic)) {
-										return -1;
+									if (asic->blocks[i]->release) {
+										if (asic->blocks[i]->release(asic)) {
+											return -1;
+										}
 									}
+								} else if (asic->blocks[i]->regs[j].type == REG_MMIO) {
+									copy = asic->pci.mem[asic->blocks[i]->regs[j].addr] & ~mask;
+									copy |= (value << asic->blocks[i]->regs[j].bits[k].start) & mask;
+									asic->pci.mem[asic->blocks[i]->regs[j].addr] = copy;
+									if (!options.quiet) printf("%s <= 0x%08lx\n", regpath, (unsigned long)copy);
 								}
-							} else if (asic->blocks[i]->regs[j].type == REG_MMIO) {
-								copy = asic->pci.mem[asic->blocks[i]->regs[j].addr] & ~mask;
-								copy |= (value << asic->blocks[i]->regs[j].bits[k].start) & mask;
-								asic->pci.mem[asic->blocks[i]->regs[j].addr] = copy;
-								if (!options.quiet) printf("%s <= 0x%08lx\n", regpath, (unsigned long)copy);
+								return 0;
 							}
-							return 0;
 						}
 					}
 				}
diff --git a/src/app/set_reg.c b/src/app/set_reg.c
index 9861170d55c3..ed8f708e977f 100644
--- a/src/app/set_reg.c
+++ b/src/app/set_reg.c
@@ -31,64 +31,63 @@
  */
 int umr_set_register(struct umr_asic *asic, char *regpath, char *regvalue)
 {
-	char rpath[512];
+	char asicname[128], ipname[128], regname[128];
 	int i, j, fd;
 	uint32_t value;
 	uint64_t addr, scale;
 
-	/* scan until we compare with regpath... */
-	for (i = 0; i < asic->no_blocks; i++) {
-		if (!memcmp(regpath, "*.", 2))
-			snprintf(rpath, sizeof(rpath)-1, "*.%s.", asic->blocks[i]->ipname);
-		else
-			snprintf(rpath, sizeof(rpath)-1, "%s.%s.", asic->asicname, asic->blocks[i]->ipname);
-		if (memcmp(regpath, rpath, strlen(rpath)) == 0) {
-			for (j = 0; j < asic->blocks[i]->no_regs; j++) {
-				if (!memcmp(regpath, "*.", 2))
-					snprintf(rpath, sizeof(rpath)-1, "*.%s.%s", asic->blocks[i]->ipname, asic->blocks[i]->regs[j].regname);
-				else
-					snprintf(rpath, sizeof(rpath)-1, "%s.%s.%s", asic->asicname, asic->blocks[i]->ipname, asic->blocks[i]->regs[j].regname);
-				if (!strcmp(regpath, rpath)) {
-					sscanf(regvalue, "%"SCNx32, &value);
+	if (sscanf(regpath, "%[^.].%[^.].%[^.]", asicname, ipname, regname) != 3) {
+		fprintf(stderr, "[ERROR]: Invalid regpath for write\n");
+		return -1;
+	}
 
-					if (asic->pci.mem == NULL) {
-						// set this register
-						switch (asic->blocks[i]->regs[j].type){
-						case REG_MMIO: fd = asic->fd.mmio; scale = 4; break;
-						case REG_DIDT: fd = asic->fd.didt; scale = 1; break;
-						case REG_PCIE: fd = asic->fd.pcie; scale = 1; break;
-						case REG_SMC:  fd = asic->fd.smc; scale = 1; break;
-						default: return -1;
-						}
-						
-						if (asic->blocks[i]->grant) {
-							if (asic->blocks[i]->grant(asic)) {
-								fprintf(stderr, "[ERROR] Must specify at least one 'risky' before writing to this block.\n");
-								return -1;
+	if (asicname[0] == '*' || !strcmp(asicname, asic->asicname)) {
+		// scan all ip blocks for matching entry
+		for (i = 0; i < asic->no_blocks; i++) {
+			if (ipname[0] == '*' || !strcmp(ipname, asic->blocks[i]->ipname)) {
+				for (j = 0; j < asic->blocks[i]->no_regs; j++) {
+					if (!strcmp(regname, asic->blocks[i]->regs[j].regname)) {
+						sscanf(regvalue, "%"SCNx32, &value);
+
+						if (asic->pci.mem == NULL) {
+							// set this register
+							switch (asic->blocks[i]->regs[j].type){
+							case REG_MMIO: fd = asic->fd.mmio; scale = 4; break;
+							case REG_DIDT: fd = asic->fd.didt; scale = 1; break;
+							case REG_PCIE: fd = asic->fd.pcie; scale = 1; break;
+							case REG_SMC:  fd = asic->fd.smc; scale = 1; break;
+							default: return -1;
+							}
+
+							if (asic->blocks[i]->grant) {
+								if (asic->blocks[i]->grant(asic)) {
+									fprintf(stderr, "[ERROR] Must specify at least one 'risky' before writing to this block.\n");
+									return -1;
+								}
 							}
-						}
 
-						if (options.use_bank && asic->blocks[i]->regs[j].type == REG_MMIO)
-							addr =
-								(1ULL << 62) |
-								(((uint64_t)options.se_bank) << 24) |
-								(((uint64_t)options.sh_bank) << 34) |
-								(((uint64_t)options.instance_bank) << 44);
-						else
-							addr = 0;
+							if (options.use_bank && asic->blocks[i]->regs[j].type == REG_MMIO)
+								addr =
+									(1ULL << 62) |
+									(((uint64_t)options.se_bank) << 24) |
+									(((uint64_t)options.sh_bank) << 34) |
+									(((uint64_t)options.instance_bank) << 44);
+							else
+								addr = 0;
 
-						lseek(fd, addr | (asic->blocks[i]->regs[j].addr*scale), SEEK_SET);
-						write(fd, &value, 4);
+							lseek(fd, addr | (asic->blocks[i]->regs[j].addr*scale), SEEK_SET);
+							write(fd, &value, 4);
 
-						if (asic->blocks[i]->release) {
-							if (asic->blocks[i]->release(asic)) {
-								return -1;
+							if (asic->blocks[i]->release) {
+								if (asic->blocks[i]->release(asic)) {
+									return -1;
+								}
 							}
+						} else if (asic->blocks[i]->regs[j].type == REG_MMIO) {
+							asic->pci.mem[asic->blocks[i]->regs[j].addr] = value;
 						}
-					} else if (asic->blocks[i]->regs[j].type == REG_MMIO) {
-						asic->pci.mem[asic->blocks[i]->regs[j].addr] = value;
+						return 0;
 					}
-					return 0;
 				}
 			}
 		}
-- 
2.11.0



More information about the amd-gfx mailing list