Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] amd/lib: refactor function is_reset_enable
@ 2025-01-28 19:24 vitaly.prosyak
  2025-01-28 20:43 ` Michael Strawbridge
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: vitaly.prosyak @ 2025-01-28 19:24 UTC (permalink / raw)
  To: igt-dev
  Cc: Vitaly Prosyak, Christian Koenig, Alexander Deucher, Jesse Zhang,
	Michael Strawbridge

From: Vitaly Prosyak <vitaly.prosyak@amd.com>

This function takes a reset type mask (`AMDGPU_RESET_TYPE_FULL`,
`AMDGPU_RESET_TYPE_SOFT_RESET`, AMDGPU_RESET_TYPE_PER_PIPE
or `AMDGPU_RESET_TYPE_PER_QUEUE`) and attempts to open and
read the corresponding sysfs file, e.g.,
`/sys/bus/pci/devices/0000:08:00.0/gfx_reset_mask`.
It then iterates through predefined keywords such as `full`,
`soft`, and `queue`, validating whether AMDGPU exposes the
requested operation. If the requested operation is not available,
the test is skipped.

The previous implementation included redundant steps and failed
in certain cases. This refactored version simplifies the logic,
improves reliability, and ensures consistent behavior across various
scenarios.

Cc: Christian Koenig <christian.koenig@amd.com>
Cc: Alexander Deucher <alexander.deucher@amd.com>
Cc: Jesse Zhang <jesse.zhang@amd.com>
Cc: Michael Strawbridge <michael.strawbridge@amd.com>

Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
---
 lib/amdgpu/amd_ip_blocks.c | 79 +++++++++++++++++++++++++-------------
 1 file changed, 53 insertions(+), 26 deletions(-)

diff --git a/lib/amdgpu/amd_ip_blocks.c b/lib/amdgpu/amd_ip_blocks.c
index 2b1b84414..f063ad0ab 100644
--- a/lib/amdgpu/amd_ip_blocks.c
+++ b/lib/amdgpu/amd_ip_blocks.c
@@ -1006,11 +1006,27 @@ asic_rings_readness(amdgpu_device_handle device_handle, uint32_t mask,
 bool
 is_reset_enable(enum amd_ip_block_type ip_type, uint32_t reset_type, const struct pci_addr *pci)
 {
-	char cmd[256];
-	FILE *fp, *fp2;
-	char buffer[100];
+	const struct reset_arr {
+		const char *name;
+		unsigned int reset;
+	} reset_arr[] = {
+		{"full",	AMDGPU_RESET_TYPE_FULL		},
+		{"soft",	AMDGPU_RESET_TYPE_SOFT_RESET},
+		{"queue",	AMDGPU_RESET_TYPE_PER_QUEUE },
+		{"pipe",	AMDGPU_RESET_TYPE_PER_PIPE },
+		{NULL, 0}
+	};
+
 	bool enable = false;
-	char reset_mask[100];
+	char cmd[256];
+	FILE *fp;
+	char buffer[128];
+
+	char *token, *buf;
+	char reset_mask[32];
+	uint32_t mask = 0;
+	int found = 0;
+	int i;
 
 	if (ip_type == AMD_IP_GFX)
 		snprintf(reset_mask, sizeof(reset_mask) - 1, "gfx_reset_mask");
@@ -1019,33 +1035,44 @@ is_reset_enable(enum amd_ip_block_type ip_type, uint32_t reset_type, const struc
 	else
 		snprintf(reset_mask, sizeof(reset_mask) - 1, "sdma_reset_mask");
 
-	if( pci)
-		snprintf(cmd, sizeof(cmd) - 1, "sudo cat /sys/kernel/debug/dri/%04x:%02x:%02x.%01x/name |grep -oP '(?<=dev=)[0-9:.]+'",
-			pci->domain, pci->bus, pci->device, pci->function);
-	else
-		snprintf(cmd, sizeof(cmd) - 1, "sudo cat /sys/kernel/debug/dri/0/name |grep -oP '(?<=dev=)[0-9:.]+'");
+	snprintf(cmd, sizeof(cmd) - 1, "sudo cat /sys/bus/pci/devices/%04x:%02x:%02x.%01x/%s",
+			pci->domain, pci->bus, pci->device, pci->function, reset_mask);
 
 	fp = popen(cmd, "r");
-	if (fp == NULL)
+	if (fp == NULL) {
+		igt_kmsg("***FAILURE popen %s LINE %d FILE %s\n", cmd, __LINE__, __FILE__);
 		return false;
+	}
 
-	if (fgets(buffer, 13, fp) != NULL) {
-		snprintf(cmd, sizeof(cmd) - 1, "sudo cat /sys/bus/pci/devices/%s/%s | grep -oP '%s'",
-			buffer, reset_mask,
-			reset_type & AMDGPU_RESET_TYPE_FULL ? "full" :
-			reset_type & AMDGPU_RESET_TYPE_SOFT_RESET ? "soft" :
-			reset_type & AMDGPU_RESET_TYPE_PER_QUEUE ? "queue" : "pipe");
-
-		fp2 = popen(cmd, "r");
-		if (fp2 == NULL) {
-			pclose(fp);
-			return false;
+	buf = fgets(buffer, sizeof(buffer)-1, fp);
+
+	//igt_kmsg("cmd: %s buf: %s LINE %d FILE %s\n",
+	//		cmd, buf != NULL ? buf : "NULL", __LINE__, __FILE__);
+
+	if (buf != NULL) {
+		token = strtok(buf, " \n");
+		while (token != NULL) {
+			for (i = 0; reset_arr[i].name != NULL; i++) {
+				if (reset_type == reset_arr[i].reset &&
+					strcmp(token, reset_arr[i].name) == 0) {
+					mask |= reset_arr[i].reset;
+					found = 1;
+					break;
+				}
+			}
+			if (!found)
+				token = strtok(NULL, " \n");
 		}
-
-		if (fgets(buffer, 13, fp2) != NULL)
-			enable = true;
-		pclose(fp2);
+	} else {
+		igt_kmsg("***FAILURE fgets %s LINE %d FILE %s\n",
+				buffer, __LINE__, __FILE__);
 	}
+	if (mask == reset_type)
+		enable = true;
+	else
+		igt_kmsg("***FAILURE mask found 0x%x requested 0x%x operation is not supported LINE %d FILE %s\n",
+				mask, reset_type, __LINE__, __FILE__);
+
 	pclose(fp);
 
 	return enable;
@@ -1105,7 +1132,7 @@ int get_pci_addr_from_fd(int fd, struct pci_addr *pci)
 
 	if (ret != 4) {
 		igt_info("error %s Unable to extract PCI device address from '%s\n",
-				__FUNCTION__, buf);
+				__func__, buf);
 		return -ENOENT;
 	}
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH] amd/lib: refactor function is_reset_enable
@ 2025-01-28  1:00 vitaly.prosyak
  0 siblings, 0 replies; 8+ messages in thread
From: vitaly.prosyak @ 2025-01-28  1:00 UTC (permalink / raw)
  To: igt-dev; +Cc: Vitaly Prosyak, Christian Koenig, Alexander Deucher, Jesse Zhang

From: Vitaly Prosyak <vitaly.prosyak@amd.com>

This function takes a reset type mask (`AMDGPU_RESET_TYPE_FULL`,
`AMDGPU_RESET_TYPE_SOFT_RESET`, or `AMDGPU_RESET_TYPE_PER_QUEUE`)
and attempts to open and read the corresponding sysfs file,
e.g., `/sys/bus/pci/devices/0000:08:00.0/gfx_reset_mask`.
It then iterates through predefined keywords such as `full`,
`soft`, and `queue`, validating whether AMDGPU exposes the
requested operation. If the requested operation is not available,
the test is skipped.

The previous implementation included redundant steps and failed
in certain cases. This refactored version simplifies the logic,
improves reliability, and ensures consistent behavior across various
scenarios.

Cc: Christian Koenig <christian.koenig@amd.com>
Cc: Alexander Deucher <alexander.deucher@amd.com>
Cc: Jesse Zhang <jesse.zhang@amd.com>

Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
---
 lib/amdgpu/amd_ip_blocks.c | 71 ++++++++++++++++++++++++--------------
 1 file changed, 45 insertions(+), 26 deletions(-)

diff --git a/lib/amdgpu/amd_ip_blocks.c b/lib/amdgpu/amd_ip_blocks.c
index 2b1b84414..8df37df37 100644
--- a/lib/amdgpu/amd_ip_blocks.c
+++ b/lib/amdgpu/amd_ip_blocks.c
@@ -1006,11 +1006,25 @@ asic_rings_readness(amdgpu_device_handle device_handle, uint32_t mask,
 bool
 is_reset_enable(enum amd_ip_block_type ip_type, uint32_t reset_type, const struct pci_addr *pci)
 {
-	char cmd[256];
-	FILE *fp, *fp2;
-	char buffer[100];
+	const struct reset_arr {
+		const char *name;
+		unsigned int reset;
+	} reset_arr[] = {
+		{"full",	AMDGPU_RESET_TYPE_FULL		},
+		{"soft",	AMDGPU_RESET_TYPE_SOFT_RESET},
+		{"queue",	AMDGPU_RESET_TYPE_PER_QUEUE },
+		{NULL, 0}
+	};
+
 	bool enable = false;
-	char reset_mask[100];
+	char cmd[256];
+	FILE *fp;
+	char buffer[128];
+
+	char *token, *buf;
+	char reset_mask[32];
+	uint32_t mask = 0;
+	int i;
 
 	if (ip_type == AMD_IP_GFX)
 		snprintf(reset_mask, sizeof(reset_mask) - 1, "gfx_reset_mask");
@@ -1019,33 +1033,38 @@ is_reset_enable(enum amd_ip_block_type ip_type, uint32_t reset_type, const struc
 	else
 		snprintf(reset_mask, sizeof(reset_mask) - 1, "sdma_reset_mask");
 
-	if( pci)
-		snprintf(cmd, sizeof(cmd) - 1, "sudo cat /sys/kernel/debug/dri/%04x:%02x:%02x.%01x/name |grep -oP '(?<=dev=)[0-9:.]+'",
-			pci->domain, pci->bus, pci->device, pci->function);
-	else
-		snprintf(cmd, sizeof(cmd) - 1, "sudo cat /sys/kernel/debug/dri/0/name |grep -oP '(?<=dev=)[0-9:.]+'");
+	snprintf(cmd, sizeof(cmd) - 1, "sudo cat /sys/bus/pci/devices/%04x:%02x:%02x.%01x/%s",
+			pci->domain, pci->bus, pci->device, pci->function, reset_mask);
 
 	fp = popen(cmd, "r");
-	if (fp == NULL)
+	if (fp == NULL) {
+		igt_kmsg("***FAILURE popen %s LINE %d FILE %s\n", cmd, __LINE__, __FILE__);
 		return false;
+	}
 
-	if (fgets(buffer, 13, fp) != NULL) {
-		snprintf(cmd, sizeof(cmd) - 1, "sudo cat /sys/bus/pci/devices/%s/%s | grep -oP '%s'",
-			buffer, reset_mask,
-			reset_type & AMDGPU_RESET_TYPE_FULL ? "full" :
-			reset_type & AMDGPU_RESET_TYPE_SOFT_RESET ? "soft" :
-			reset_type & AMDGPU_RESET_TYPE_PER_QUEUE ? "queue" : "pipe");
-
-		fp2 = popen(cmd, "r");
-		if (fp2 == NULL) {
-			pclose(fp);
-			return false;
+	buf = fgets(buffer, sizeof(buffer)-1, fp);
+	if (buf != NULL) {
+		token = strtok(buf, " \n");
+		while (token != NULL) {
+			for (i = 0; reset_arr[i].name != NULL; i++) {
+				if (reset_type == reset_arr[i].reset &&
+					strcmp(token, reset_arr[i].name) == 0) {
+					mask |= reset_arr[i].reset;
+					break;
+				}
+			}
+			token = strtok(NULL, " \n");
 		}
-
-		if (fgets(buffer, 13, fp2) != NULL)
-			enable = true;
-		pclose(fp2);
+	} else {
+		igt_kmsg("***FAILURE fgets %s LINE %d FILE %s\n",
+				buffer, __LINE__, __FILE__);
 	}
+	if (mask == reset_type)
+		enable = true;
+	else
+		igt_kmsg("***FAILURE mask found 0x%x requested 0x%x LINE %d FILE %s\n",
+				mask, reset_type, __LINE__, __FILE__);
+
 	pclose(fp);
 
 	return enable;
@@ -1105,7 +1124,7 @@ int get_pci_addr_from_fd(int fd, struct pci_addr *pci)
 
 	if (ret != 4) {
 		igt_info("error %s Unable to extract PCI device address from '%s\n",
-				__FUNCTION__, buf);
+				__func__, buf);
 		return -ENOENT;
 	}
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-01-29 12:11 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-28 19:24 [PATCH] amd/lib: refactor function is_reset_enable vitaly.prosyak
2025-01-28 20:43 ` Michael Strawbridge
2025-01-28 20:48   ` vitaly prosyak
2025-01-28 21:39 ` ✓ Xe.CI.BAT: success for amd/lib: refactor function is_reset_enable (rev2) Patchwork
2025-01-28 21:42 ` ✓ i915.CI.BAT: " Patchwork
2025-01-29 12:10 ` ✗ i915.CI.Full: failure " Patchwork
2025-01-29 12:11 ` ✗ Xe.CI.Full: " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2025-01-28  1:00 [PATCH] amd/lib: refactor function is_reset_enable vitaly.prosyak

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