All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH libdrm 1/3] amdgpu: verify the tested device
@ 2017-01-24 22:29 Alex Xie
       [not found] ` <1485296992-2719-1-git-send-email-AlexBin.Xie-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Alex Xie @ 2017-01-24 22:29 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Alex Xie

Verify the vender ID and driver name.
Open all AMDGPU devices.
Provide an option to open render node.

Tested as root: PASS
Tested as non-privileged user:
All tests failed as expected

v2: Return value in the ene of function amdgpu_open_devices.
    Check the return value of amdgpu_open_devices.
    amdgpu_test is not for USB device for the time being.
    Get the name of node from function drmGetDevices2.
    Drop the legacy drmAvailable() from the test.

Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
---
 tests/amdgpu/amdgpu_test.c | 145 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 115 insertions(+), 30 deletions(-)

diff --git a/tests/amdgpu/amdgpu_test.c b/tests/amdgpu/amdgpu_test.c
index 71f357c..d2b00d4 100644
--- a/tests/amdgpu/amdgpu_test.c
+++ b/tests/amdgpu/amdgpu_test.c
@@ -115,6 +115,111 @@ static const char usage[] = "Usage: %s [-hl] [<-s <suite id>> [-t <test id>]]\n"
 /** Specified options strings for getopt */
 static const char options[]   = "hls:t:";
 
+/* Open AMD devices.
+ * Return the number of AMD device openned.
+ */
+static int amdgpu_open_devices(int open_render_node)
+{
+	drmDevicePtr devices[MAX_CARDS_SUPPORTED];
+	int ret;
+	int i;
+	int drm_node;
+	int amd_index = 0;
+	int drm_count;
+	int fd;
+	drmVersionPtr version;
+
+	drm_count = drmGetDevices2(0, devices, MAX_CARDS_SUPPORTED);
+
+	if (drm_count < 0) {
+		fprintf(stderr,
+			"drmGetDevices2() returned an error %d\n",
+			drm_count);
+		return 0;
+	}
+
+	for (i = 0; i < drm_count; i++) {
+		/* If this is not PCI device, skip*/
+		if (devices[i]->bustype != DRM_BUS_PCI)
+			continue;
+
+		/* If this is not AMD GPU vender ID, skip*/
+		if (devices[i]->deviceinfo.pci->vendor_id != 0x1002)
+			continue;
+
+		if (open_render_node)
+			drm_node = DRM_NODE_RENDER;
+		else
+			drm_node = DRM_NODE_PRIMARY;
+
+		fd = -1;
+		if (devices[i]->available_nodes & 1 << drm_node)
+			fd = open(
+				devices[i]->nodes[drm_node],
+				O_RDWR | O_CLOEXEC);
+
+		/* This node is not available. */
+		if (fd < 0) continue;
+
+		version = drmGetVersion(fd);
+		if (!version) {
+			fprintf(stderr,
+				"Warning: Cannot get version for %s."
+				"Error is %s\n",
+				devices[i]->nodes[drm_node],
+				strerror(errno));
+			close(fd);
+			continue;
+		}
+
+		if (strcmp(version->name, "amdgpu")) {
+			/* This is not AMDGPU driver, skip.*/
+			drmFreeVersion(version);
+			close(fd);
+			continue;
+		}
+
+		drmFreeVersion(version);
+
+		drm_amdgpu[amd_index] = fd;
+		amd_index++;
+	}
+
+	drmFreeDevices(devices, drm_count);
+	return amd_index;
+}
+
+/* Close AMD devices.
+ */
+static void amdgpu_close_devices()
+{
+	int i;
+	for (i = 0; i < MAX_CARDS_SUPPORTED; i++)
+		if (drm_amdgpu[i] >=0)
+			close(drm_amdgpu[i]);
+}
+
+/* Print AMD devices information */
+static void amdgpu_print_devices()
+{
+	int i;
+	for (i = 0; i < MAX_CARDS_SUPPORTED; i++)
+		if (drm_amdgpu[i] >=0) {
+			/** Display version of DRM driver */
+			drmVersionPtr retval = drmGetVersion(drm_amdgpu[0]);
+
+			if (retval == NULL) {
+				perror("Cannot get version for AMDGPU device");
+				exit(EXIT_FAILURE);
+			}
+
+			printf("AMDGPU device #%d: "
+				"Name: [%s] : Date [%s] : Description [%s]\n",
+				i, retval->name, retval->date, retval->desc);
+			drmFreeVersion(retval);
+		}
+}
+
 /* The main() function for setting up and running the tests.
  * Returns a CUE_SUCCESS on successful running, another
  * CUnit error code on failure.
@@ -128,14 +233,6 @@ int main(int argc, char **argv)
 	CU_pSuite pSuite = NULL;
 	CU_pTest  pTest  = NULL;
 
-	int aval = drmAvailable();
-
-	if (aval == 0) {
-		fprintf(stderr, "DRM driver is not available\n");
-		exit(EXIT_FAILURE);
-	}
-
-
 	for (i = 0; i < MAX_CARDS_SUPPORTED; i++)
 		drm_amdgpu[i] = -1;
 
@@ -163,35 +260,23 @@ int main(int argc, char **argv)
 		}
 	}
 
-	/* Try to open all possible radeon connections
-	 * Right now: Open only the 0.
-	 */
-	printf("Try to open the card 0..\n");
-	drm_amdgpu[0] = open("/dev/dri/card0", O_RDWR | O_CLOEXEC);
-
-	if (drm_amdgpu[0] < 0) {
-		perror("Cannot open /dev/dri/card0\n");
+	if (amdgpu_open_devices(0) <= 0) {
+		perror("Cannot open AMDGPU device");
 		exit(EXIT_FAILURE);
 	}
 
-	/** Display version of DRM driver */
-	drmVersionPtr retval = drmGetVersion(drm_amdgpu[0]);
-
-	if (retval == NULL) {
-		perror("Could not get information about DRM driver");
+	if (drm_amdgpu[0] < 0) {
+		perror("Cannot open AMDGPU device");
 		exit(EXIT_FAILURE);
 	}
 
-	printf("DRM Driver: Name: [%s] : Date [%s] : Description [%s]\n",
-	       retval->name, retval->date, retval->desc);
-
-	drmFreeVersion(retval);
+	amdgpu_print_devices();
 
 	/* Initialize test suites to run */
 
 	/* initialize the CUnit test registry */
 	if (CUE_SUCCESS != CU_initialize_registry()) {
-		close(drm_amdgpu[0]);
+		amdgpu_close_devices();
 		return CU_get_error();
 	}
 
@@ -200,7 +285,7 @@ int main(int argc, char **argv)
 		fprintf(stderr, "suite registration failed - %s\n",
 				CU_get_error_msg());
 		CU_cleanup_registry();
-		close(drm_amdgpu[0]);
+		amdgpu_close_devices();
 		exit(EXIT_FAILURE);
 	}
 
@@ -222,7 +307,7 @@ int main(int argc, char **argv)
 					fprintf(stderr, "Invalid test id: %d\n",
 								test_id);
 					CU_cleanup_registry();
-					close(drm_amdgpu[0]);
+					amdgpu_close_devices();
 					exit(EXIT_FAILURE);
 				}
 			} else
@@ -231,13 +316,13 @@ int main(int argc, char **argv)
 			fprintf(stderr, "Invalid suite id : %d\n",
 					suite_id);
 			CU_cleanup_registry();
-			close(drm_amdgpu[0]);
+			amdgpu_close_devices();
 			exit(EXIT_FAILURE);
 		}
 	} else
 		CU_basic_run_tests();
 
 	CU_cleanup_registry();
-	close(drm_amdgpu[0]);
+	amdgpu_close_devices();
 	return CU_get_error();
 }
-- 
2.7.4

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

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

* [PATCH libdrm 2/3] amdgpu: A new option to choose which device to run most tests
       [not found] ` <1485296992-2719-1-git-send-email-AlexBin.Xie-5C7GfCeVMHo@public.gmane.org>
@ 2017-01-24 22:29   ` Alex Xie
  2017-01-24 22:29   ` [PATCH libdrm 3/3] amdgpu: A new option to run tests on render node Alex Xie
  2017-01-27  0:08   ` [PATCH libdrm 1/3] amdgpu: verify the tested device Emil Velikov
  2 siblings, 0 replies; 11+ messages in thread
From: Alex Xie @ 2017-01-24 22:29 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Alex Xie

This can be used to test multiple GPUs

v2: Use PCI bus ID and optional PCI device ID to choose device
    Add an option to display information of AMDGPU devices

Tested:
   ./amdgpu_test -p
   ./amdgpu_test
   ./amdgpu_test -b 1 #fail as expected
   ./amdgpu_test -b 6 #pass
   ./amdgpu_test -b -d 1 #fail as expected
   ./amdgpu_test -b -d 0 #pass

Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
---
 tests/amdgpu/amdgpu_test.c | 133 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 116 insertions(+), 17 deletions(-)

diff --git a/tests/amdgpu/amdgpu_test.c b/tests/amdgpu/amdgpu_test.c
index d2b00d4..c01ee54 100644
--- a/tests/amdgpu/amdgpu_test.c
+++ b/tests/amdgpu/amdgpu_test.c
@@ -108,12 +108,17 @@ static void display_test_suites(void)
 
 
 /** Help string for command line parameters */
-static const char usage[] = "Usage: %s [-hl] [<-s <suite id>> [-t <test id>]]\n"
-				"where:\n"
-				"       l - Display all suites and their tests\n"
-				"       h - Display this help\n";
+static const char usage[] =
+	"Usage: %s [-hlp] [<-s <suite id>> [-t <test id>]] "
+	"[-b <pci_bus_id> [-d <pci_device_id>]]\n"
+	"where:\n"
+	"       l - Display all suites and their tests\n"
+	"       b - Specify device's PCI bus id to run tests\n"
+	"       d - Specify device's PCI device id to run tests (optional)\n"
+	"       p - Display information of AMDGPU devices in system\n"
+	"       h - Display this help\n";
 /** Specified options strings for getopt */
-static const char options[]   = "hls:t:";
+static const char options[]   = "hlps:t:b:d:";
 
 /* Open AMD devices.
  * Return the number of AMD device openned.
@@ -203,21 +208,79 @@ static void amdgpu_close_devices()
 static void amdgpu_print_devices()
 {
 	int i;
-	for (i = 0; i < MAX_CARDS_SUPPORTED; i++)
-		if (drm_amdgpu[i] >=0) {
-			/** Display version of DRM driver */
-			drmVersionPtr retval = drmGetVersion(drm_amdgpu[0]);
+	drmDevicePtr device;
+
+	/* Open the first AMD devcie to print driver information. */
+	if (drm_amdgpu[0] >=0) {
+		/* Display AMD driver version information.*/
+		drmVersionPtr retval = drmGetVersion(drm_amdgpu[0]);
+
+		if (retval == NULL) {
+			perror("Cannot get version for AMDGPU device");
+			return;
+		}
 
-			if (retval == NULL) {
-				perror("Cannot get version for AMDGPU device");
-				exit(EXIT_FAILURE);
+		printf("Driver name: %s, Date: %s, Description: %s.\n",
+			retval->name, retval->date, retval->desc);
+		drmFreeVersion(retval);
+	}
+
+	/* Display information of AMD devices */
+	printf("Devices:\n");
+	for (i = 0; i < MAX_CARDS_SUPPORTED && drm_amdgpu[i] >=0; i++)
+		if (drmGetDevice2(drm_amdgpu[i],
+			DRM_DEVICE_GET_PCI_REVISION,
+			&device) == 0) {
+			if (device->bustype == DRM_BUS_PCI) {
+				printf("PCI ");
+				printf(" domain:%04x",
+					device->businfo.pci->domain);
+				printf(" bus:%02x",
+					device->businfo.pci->bus);
+				printf(" device:%02x",
+					device->businfo.pci->dev);
+				printf(" function:%01x",
+					device->businfo.pci->func);
+				printf(" vendor_id:%04x",
+					device->deviceinfo.pci->vendor_id);
+				printf(" device_id:%04x",
+					device->deviceinfo.pci->device_id);
+				printf(" subvendor_id:%04x",
+					device->deviceinfo.pci->subvendor_id);
+				printf(" subdevice_id:%04x",
+					device->deviceinfo.pci->subdevice_id);
+				printf(" revision_id:%02x",
+					device->deviceinfo.pci->revision_id);
+				printf("\n");
 			}
+			drmFreeDevice(&device);
+		}
+}
+
+/* Find a match AMD device in PCI bus
+ * Return the index of the device or -1 if not found
+ */
+static int amdgpu_find_device(uint8_t bus, uint8_t dev)
+{
+	int i;
+	drmDevicePtr device;
+
+	for (i = 0; i < MAX_CARDS_SUPPORTED && drm_amdgpu[i] >=0; i++)
+		if (drmGetDevice2(drm_amdgpu[i],
+			DRM_DEVICE_GET_PCI_REVISION,
+			&device) == 0) {
+			if (device->bustype == DRM_BUS_PCI)
+				if (device->businfo.pci->bus == bus &&
+					device->businfo.pci->dev == dev) {
+
+					drmFreeDevice(&device);
+					return i;
+				}
 
-			printf("AMDGPU device #%d: "
-				"Name: [%s] : Date [%s] : Description [%s]\n",
-				i, retval->name, retval->date, retval->desc);
-			drmFreeVersion(retval);
+			drmFreeDevice(&device);
 		}
+
+	return -1;
 }
 
 /* The main() function for setting up and running the tests.
@@ -230,8 +293,12 @@ int main(int argc, char **argv)
 	int i = 0;
 	int suite_id = -1;	/* By default run everything */
 	int test_id  = -1;	/* By default run all tests in the suite */
+	int pci_bus_id = -1;    /* By default PC bus ID is not specified */
+	int pci_device_id = 0;  /* By default PC device ID is zero */
+	int display_devices = 0;/* By default not to display devices' info */
 	CU_pSuite pSuite = NULL;
 	CU_pTest  pTest  = NULL;
+	int test_device_index;
 
 	for (i = 0; i < MAX_CARDS_SUPPORTED; i++)
 		drm_amdgpu[i] = -1;
@@ -250,6 +317,15 @@ int main(int argc, char **argv)
 		case 't':
 			test_id = atoi(optarg);
 			break;
+		case 'b':
+			pci_bus_id = atoi(optarg);
+			break;
+		case 'd':
+			pci_device_id = atoi(optarg);
+			break;
+		case 'p':
+			display_devices = 1;
+			break;
 		case '?':
 		case 'h':
 			fprintf(stderr, usage, argv[0]);
@@ -270,7 +346,30 @@ int main(int argc, char **argv)
 		exit(EXIT_FAILURE);
 	}
 
-	amdgpu_print_devices();
+	if (display_devices) {
+		amdgpu_print_devices();
+		amdgpu_close_devices();
+		exit(EXIT_SUCCESS);
+	}
+
+	if (pci_bus_id > 0) {
+		/* A device was specified to run the test */
+		test_device_index = amdgpu_find_device((uint8_t)pci_bus_id,
+							(uint8_t)pci_device_id);
+
+		if (test_device_index >= 0) {
+			/* Most tests run on device of drm_amdgpu[0].
+			 * Swap the chosen device to drm_amdgpu[0].
+			 */
+			i = drm_amdgpu[0];
+			drm_amdgpu[0] = drm_amdgpu[test_device_index];
+			drm_amdgpu[test_device_index] = i;
+		} else {
+			fprintf(stderr,
+				"The specified GPU device does not exist.\n");
+			exit(EXIT_FAILURE);
+		}
+	}
 
 	/* Initialize test suites to run */
 
-- 
2.7.4

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

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

* [PATCH libdrm 3/3] amdgpu: A new option to run tests on render node
       [not found] ` <1485296992-2719-1-git-send-email-AlexBin.Xie-5C7GfCeVMHo@public.gmane.org>
  2017-01-24 22:29   ` [PATCH libdrm 2/3] amdgpu: A new option to choose which device to run most tests Alex Xie
@ 2017-01-24 22:29   ` Alex Xie
       [not found]     ` <1485296992-2719-3-git-send-email-AlexBin.Xie-5C7GfCeVMHo@public.gmane.org>
  2017-01-27  0:08   ` [PATCH libdrm 1/3] amdgpu: verify the tested device Emil Velikov
  2 siblings, 1 reply; 11+ messages in thread
From: Alex Xie @ 2017-01-24 22:29 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Alex Xie

Tested:
1. As root, tests passed on primary.
2. As root, tests passed on render node.
   BO export/import test was skipped
3. As non-privileged user, tests failed on primary as expected.
4. As non-privileged user, tests passed on render node.
   BO export/import test was skipped

Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
---
 tests/amdgpu/amdgpu_test.c | 13 ++++++++++---
 tests/amdgpu/amdgpu_test.h |  3 +++
 tests/amdgpu/bo_tests.c    |  5 +++++
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/tests/amdgpu/amdgpu_test.c b/tests/amdgpu/amdgpu_test.c
index c01ee54..3fd6820 100644
--- a/tests/amdgpu/amdgpu_test.c
+++ b/tests/amdgpu/amdgpu_test.c
@@ -56,6 +56,9 @@
  */
 int drm_amdgpu[MAX_CARDS_SUPPORTED];
 
+/** Open render node to test */
+int open_render_node = 0;	/* By default run most tests on primary node */
+
 /** The table of all known test suites to run */
 static CU_SuiteInfo suites[] = {
 	{
@@ -109,16 +112,17 @@ static void display_test_suites(void)
 
 /** Help string for command line parameters */
 static const char usage[] =
-	"Usage: %s [-hlp] [<-s <suite id>> [-t <test id>]] "
+	"Usage: %s [-hlpr] [<-s <suite id>> [-t <test id>]] "
 	"[-b <pci_bus_id> [-d <pci_device_id>]]\n"
 	"where:\n"
 	"       l - Display all suites and their tests\n"
+	"       r - Run the tests on render node\n"
 	"       b - Specify device's PCI bus id to run tests\n"
 	"       d - Specify device's PCI device id to run tests (optional)\n"
 	"       p - Display information of AMDGPU devices in system\n"
 	"       h - Display this help\n";
 /** Specified options strings for getopt */
-static const char options[]   = "hlps:t:b:d:";
+static const char options[]   = "hlrps:t:b:d:";
 
 /* Open AMD devices.
  * Return the number of AMD device openned.
@@ -326,6 +330,9 @@ int main(int argc, char **argv)
 		case 'p':
 			display_devices = 1;
 			break;
+		case 'r':
+			open_render_node = 1;
+			break;
 		case '?':
 		case 'h':
 			fprintf(stderr, usage, argv[0]);
@@ -336,7 +343,7 @@ int main(int argc, char **argv)
 		}
 	}
 
-	if (amdgpu_open_devices(0) <= 0) {
+	if (amdgpu_open_devices(open_render_node) <= 0) {
 		perror("Cannot open AMDGPU device");
 		exit(EXIT_FAILURE);
 	}
diff --git a/tests/amdgpu/amdgpu_test.h b/tests/amdgpu/amdgpu_test.h
index fca92ad..e30e231 100644
--- a/tests/amdgpu/amdgpu_test.h
+++ b/tests/amdgpu/amdgpu_test.h
@@ -35,6 +35,9 @@
 /* Forward reference for array to keep "drm" handles */
 extern int drm_amdgpu[MAX_CARDS_SUPPORTED];
 
+/* Global variables */
+extern int open_render_node;
+
 /*************************  Basic test suite ********************************/
 
 /*
diff --git a/tests/amdgpu/bo_tests.c b/tests/amdgpu/bo_tests.c
index 25df767..74b5e77 100644
--- a/tests/amdgpu/bo_tests.c
+++ b/tests/amdgpu/bo_tests.c
@@ -152,6 +152,11 @@ static void amdgpu_bo_export_import_do_type(enum amdgpu_bo_handle_type type)
 
 static void amdgpu_bo_export_import(void)
 {
+	if (open_render_node) {
+		printf("(DRM render node is used. Skip export/Import test) ");
+		return;
+	}
+
 	amdgpu_bo_export_import_do_type(amdgpu_bo_handle_type_gem_flink_name);
 	amdgpu_bo_export_import_do_type(amdgpu_bo_handle_type_dma_buf_fd);
 }
-- 
2.7.4

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

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

* Re: [PATCH libdrm 1/3] amdgpu: verify the tested device
       [not found] ` <1485296992-2719-1-git-send-email-AlexBin.Xie-5C7GfCeVMHo@public.gmane.org>
  2017-01-24 22:29   ` [PATCH libdrm 2/3] amdgpu: A new option to choose which device to run most tests Alex Xie
  2017-01-24 22:29   ` [PATCH libdrm 3/3] amdgpu: A new option to run tests on render node Alex Xie
@ 2017-01-27  0:08   ` Emil Velikov
  2 siblings, 0 replies; 11+ messages in thread
From: Emil Velikov @ 2017-01-27  0:08 UTC (permalink / raw)
  To: Alex Xie; +Cc: amd-gfx mailing list

Hi Alex,

Things look a lot better imho. There's some ideas below, for the
future if you/others see value in them. Please do not worry about
those now.

On 24 January 2017 at 22:29, Alex Xie <AlexBin.Xie@amd.com> wrote:
> Verify the vender ID and driver name.
> Open all AMDGPU devices.
> Provide an option to open render node.
>
> Tested as root: PASS
> Tested as non-privileged user:
> All tests failed as expected
>
> v2: Return value in the ene of function amdgpu_open_devices.
>     Check the return value of amdgpu_open_devices.
>     amdgpu_test is not for USB device for the time being.
>     Get the name of node from function drmGetDevices2.
>     Drop the legacy drmAvailable() from the test.
>
> Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
> ---
>  tests/amdgpu/amdgpu_test.c | 145 +++++++++++++++++++++++++++++++++++----------
>  1 file changed, 115 insertions(+), 30 deletions(-)
>
> diff --git a/tests/amdgpu/amdgpu_test.c b/tests/amdgpu/amdgpu_test.c
> index 71f357c..d2b00d4 100644
> --- a/tests/amdgpu/amdgpu_test.c
> +++ b/tests/amdgpu/amdgpu_test.c
> @@ -115,6 +115,111 @@ static const char usage[] = "Usage: %s [-hl] [<-s <suite id>> [-t <test id>]]\n"
>  /** Specified options strings for getopt */
>  static const char options[]   = "hls:t:";
>
> +/* Open AMD devices.
> + * Return the number of AMD device openned.
> + */
> +static int amdgpu_open_devices(int open_render_node)
> +{
> +       drmDevicePtr devices[MAX_CARDS_SUPPORTED];
> +       int ret;
> +       int i;
> +       int drm_node;
> +       int amd_index = 0;
> +       int drm_count;
> +       int fd;
> +       drmVersionPtr version;
> +
> +       drm_count = drmGetDevices2(0, devices, MAX_CARDS_SUPPORTED);
> +

> +       for (i = 0; i < drm_count; i++) {
> +               /* If this is not PCI device, skip*/
> +               if (devices[i]->bustype != DRM_BUS_PCI)
> +                       continue;
> +
> +               /* If this is not AMD GPU vender ID, skip*/
> +               if (devices[i]->deviceinfo.pci->vendor_id != 0x1002)
> +                       continue;
> +
Once the filtering is done, it may be that only 2 of the
MAX_CARDS_SUPPORTED acquired fit the criteria.
One could fetch arbitrary large (all?) devices and then cap if/as needed.


> +
> +               /* This node is not available. */
> +               if (fd < 0) continue;
Normally continue should be on the next line.


> +/* Print AMD devices information */
> +static void amdgpu_print_devices()
> +{
> +       int i;
> +       for (i = 0; i < MAX_CARDS_SUPPORTED; i++)
> +               if (drm_amdgpu[i] >=0) {
> +                       /** Display version of DRM driver */
> +                       drmVersionPtr retval = drmGetVersion(drm_amdgpu[0]);
Since we've done this above one might as well store it and cleanups at
amdgpu_close_devices() time.
It comes more applicable as you use drmGetDevice2 to get the device
info with later patch(es). Might as well store all the info that we
fetch during amdgpu_open_devices() ?

Thanks
Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* Re: [PATCH libdrm 3/3] amdgpu: A new option to run tests on render node
       [not found]     ` <1485296992-2719-3-git-send-email-AlexBin.Xie-5C7GfCeVMHo@public.gmane.org>
@ 2017-01-27  0:11       ` Emil Velikov
  0 siblings, 0 replies; 11+ messages in thread
From: Emil Velikov @ 2017-01-27  0:11 UTC (permalink / raw)
  To: Alex Xie; +Cc: amd-gfx mailing list

On 24 January 2017 at 22:29, Alex Xie <AlexBin.Xie@amd.com> wrote:
> Tested:
> 1. As root, tests passed on primary.
> 2. As root, tests passed on render node.
>    BO export/import test was skipped
> 3. As non-privileged user, tests failed on primary as expected.
> 4. As non-privileged user, tests passed on render node.
>    BO export/import test was skipped
>
> Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
I haven't done too in-depth review by the series looks a lot better.
Thanks again for addressing my suggestions !

Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>

-Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* Re: [PATCH libdrm 1/3] amdgpu: verify the tested device
       [not found]         ` <CY4PR12MB1640D78EBE9F998A00F5323FF2710-rpdhrqHFk06q//3LJcutlgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2017-01-20 19:45           ` Emil Velikov
  0 siblings, 0 replies; 11+ messages in thread
From: Emil Velikov @ 2017-01-20 19:45 UTC (permalink / raw)
  To: Xie, AlexBin; +Cc: amd-gfx mailing list

On 20 January 2017 at 19:14, Xie, AlexBin <AlexBin.Xie@amd.com> wrote:
> Hi Emil,
>
>
> Thanks for the comments.
>
>
> Please see below.
>
>
> Regards,
>
> Alex Bin Xie
>
>
>
> ________________________________
> From: Emil Velikov <emil.l.velikov@gmail.com>
> Sent: Friday, January 20, 2017 8:18 AM
> To: Xie, AlexBin
> Cc: amd-gfx mailing list
> Subject: Re: [PATCH libdrm 1/3] amdgpu: verify the tested device
>
> Hi Alex,
>
> Thanks for doing this. There's a few nitpicks on top of what David and
> Christian has spotted.
>
> On 19 January 2017 at 22:53, Alex Xie <AlexBin.Xie@amd.com> wrote:
>> Verify the vender ID and driver name.
>> Open all AMDGPU devices.
>> Provide an option to open render node.
>>
>> Tested as root: PASS
>> Tested as non-privileged user:
>> All tests failed as expected
>>
>> Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
>> ---
>>  tests/amdgpu/amdgpu_test.c | 144
>> +++++++++++++++++++++++++++++++++++++--------
>>  1 file changed, 121 insertions(+), 23 deletions(-)
>>
>> diff --git a/tests/amdgpu/amdgpu_test.c b/tests/amdgpu/amdgpu_test.c
>> index 71f357c..e42ef9d 100644
>> --- a/tests/amdgpu/amdgpu_test.c
>> +++ b/tests/amdgpu/amdgpu_test.c
>> @@ -115,6 +115,119 @@ static const char usage[] = "Usage: %s [-hl] [<-s
>> <suite id>> [-t <test id>]]\n"
>>  /** Specified options strings for getopt */
>>  static const char options[]   = "hls:t:";
>>
>> +/* Open AMD devices.
>> + * Return the number of AMD device openned.
>> + */
>> +static int amdgpu_open_devices(int open_render_node)
>> +{
>> +       drmDevicePtr devices[MAX_CARDS_SUPPORTED];
>> +       int ret;
>> +       int i;
>> +       int j;
>> +       int amd_index = 0;
>> +       int drm_count;
>> +       int fd;
>> +       char *device_name;
>> +       drmVersionPtr version;
>> +
>> +       drm_count = drmGetDevices2(0, devices, MAX_CARDS_SUPPORTED);
>> +
>> +       if (drm_count < 0) {
>> +               fprintf(stderr,
>> +                       "drmGetDevices2() returned an error %d\n",
>> +                       drm_count);
>> +               return 0;
>> +       }
>> +
>> +       for (i = 0; i < drm_count; i++) {
>> +               /* If this is not AMD GPU vender ID, skip*/
>> +               if (devices[i]->bustype == DRM_BUS_PCI)
>> +                       if (devices[i]->deviceinfo.pci->vendor_id !=
>> 0x1002)
>> +                               continue;
>> +
>> +               for (j = 0; j < DRM_NODE_MAX; j++) {
>> +                       if (devices[i]->available_nodes & 1 << j) {
>> +                               fd = open(
>> +                                       devices[i]->nodes[j],
>> +                                       O_RDONLY | O_CLOEXEC,
>> +                                       0);
>> +                               if (fd < 0) continue;
>> +                       }
> You don't need to iterate over all the available nodes. Just fetch the
> PRIMARY or RENDER based on open_render_node.
> Note that a device can be missing some node types (say RENDER) so make
> sure the available_nodes bitmask is set.
>
> [Alex Bin Xie]:
> That was my original design, but later I changed.
> I knew this work for the current xf86drm.c. But is the nodes[0]  always
> primary?
> I was afraid that this may be changed in future. I searched all drm tests. I
> did not see an example.
>
> So amdgpu_test will be the first to access nodes like:
> open(devices[i]->nodes[DRM_NODE_PRIMARY]), for example.
>
Please avoid nodes[0] and use the symbolic name - nodes[DRM_NODE_PRIMARY].
Comment applies for any DRM_NODE_*.

>> +                       if (open_render_node)
>> +                               device_name =
>> drmGetRenderDeviceNameFromFd(fd);
>> +                       else
>> +                               device_name =
>> drmGetPrimaryDeviceNameFromFd(fd);
>> +
>> +                       close(fd);
>> +
>> +                       drm_amdgpu[amd_index] = open(device_name,
>> +                                                       O_RDWR |
>> O_CLOEXEC);
>> +
>> +                       if (drm_amdgpu[amd_index] >= 0)
>> +                               amd_index++;
>> +
>> +                       free(device_name);
>> +
> With the above comment this becomes redundant.
>
>> +                       /* We have open this device. Go to next device.*/
>> +                       break;
>> +               }
>> +       }
>> +
> Here you want to initialise the remainder of drm_amdgpu[] (since
> drm_count can be less than MAX_CARDS_SUPPORTED) with -1.
> Otherwise you'll have fun experiences during close/print (below).
>
> [Alex Bin Xie]: This initialization is done in existing main() function (not
> introduced this patch).
>
Indeed that's correct. It looks a bit strange to have it separate, but
not my call.

Unrelated:
Fwiw please can drop the legacy drmAvailable() from the test, if you
have a minute.

Thanks !
Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* Re: [PATCH libdrm 1/3] amdgpu: verify the tested device
       [not found]     ` <CACvgo50HCafUSw7zDjkS+_1xs67=QgHcOWcO1DDqzBL4RcRBJA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-01-20 19:14       ` Xie, AlexBin
       [not found]         ` <CY4PR12MB1640D78EBE9F998A00F5323FF2710-rpdhrqHFk06q//3LJcutlgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Xie, AlexBin @ 2017-01-20 19:14 UTC (permalink / raw)
  To: Emil Velikov; +Cc: amd-gfx mailing list


[-- Attachment #1.1: Type: text/plain, Size: 4514 bytes --]

Hi Emil,


Thanks for the comments.


Please see below.


Regards,

Alex Bin Xie


________________________________
From: Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Sent: Friday, January 20, 2017 8:18 AM
To: Xie, AlexBin
Cc: amd-gfx mailing list
Subject: Re: [PATCH libdrm 1/3] amdgpu: verify the tested device

Hi Alex,

Thanks for doing this. There's a few nitpicks on top of what David and
Christian has spotted.

On 19 January 2017 at 22:53, Alex Xie <AlexBin.Xie-5C7GfCeVMHo@public.gmane.org> wrote:
> Verify the vender ID and driver name.
> Open all AMDGPU devices.
> Provide an option to open render node.
>
> Tested as root: PASS
> Tested as non-privileged user:
> All tests failed as expected
>
> Signed-off-by: Alex Xie <AlexBin.Xie-5C7GfCeVMHo@public.gmane.org>
> ---
>  tests/amdgpu/amdgpu_test.c | 144 +++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 121 insertions(+), 23 deletions(-)
>
> diff --git a/tests/amdgpu/amdgpu_test.c b/tests/amdgpu/amdgpu_test.c
> index 71f357c..e42ef9d 100644
> --- a/tests/amdgpu/amdgpu_test.c
> +++ b/tests/amdgpu/amdgpu_test.c
> @@ -115,6 +115,119 @@ static const char usage[] = "Usage: %s [-hl] [<-s <suite id>> [-t <test id>]]\n"
>  /** Specified options strings for getopt */
>  static const char options[]   = "hls:t:";
>
> +/* Open AMD devices.
> + * Return the number of AMD device openned.
> + */
> +static int amdgpu_open_devices(int open_render_node)
> +{
> +       drmDevicePtr devices[MAX_CARDS_SUPPORTED];
> +       int ret;
> +       int i;
> +       int j;
> +       int amd_index = 0;
> +       int drm_count;
> +       int fd;
> +       char *device_name;
> +       drmVersionPtr version;
> +
> +       drm_count = drmGetDevices2(0, devices, MAX_CARDS_SUPPORTED);
> +
> +       if (drm_count < 0) {
> +               fprintf(stderr,
> +                       "drmGetDevices2() returned an error %d\n",
> +                       drm_count);
> +               return 0;
> +       }
> +
> +       for (i = 0; i < drm_count; i++) {
> +               /* If this is not AMD GPU vender ID, skip*/
> +               if (devices[i]->bustype == DRM_BUS_PCI)
> +                       if (devices[i]->deviceinfo.pci->vendor_id != 0x1002)
> +                               continue;
> +
> +               for (j = 0; j < DRM_NODE_MAX; j++) {
> +                       if (devices[i]->available_nodes & 1 << j) {
> +                               fd = open(
> +                                       devices[i]->nodes[j],
> +                                       O_RDONLY | O_CLOEXEC,
> +                                       0);
> +                               if (fd < 0) continue;
> +                       }
You don't need to iterate over all the available nodes. Just fetch the
PRIMARY or RENDER based on open_render_node.
Note that a device can be missing some node types (say RENDER) so make
sure the available_nodes bitmask is set.

[Alex Bin Xie]:
That was my original design, but later I changed.
I knew this work for the current xf86drm.c. But is the nodes[0]  always primary?
I was afraid that this may be changed in future. I searched all drm tests. I did not see an example.

So amdgpu_test will be the first to access nodes like: open(devices[i]->nodes[DRM_NODE_PRIMARY]), for example.

> +                       if (open_render_node)
> +                               device_name = drmGetRenderDeviceNameFromFd(fd);
> +                       else
> +                               device_name = drmGetPrimaryDeviceNameFromFd(fd);
> +
> +                       close(fd);
> +
> +                       drm_amdgpu[amd_index] = open(device_name,
> +                                                       O_RDWR | O_CLOEXEC);
> +
> +                       if (drm_amdgpu[amd_index] >= 0)
> +                               amd_index++;
> +
> +                       free(device_name);
> +
With the above comment this becomes redundant.

> +                       /* We have open this device. Go to next device.*/
> +                       break;
> +               }
> +       }
> +
Here you want to initialise the remainder of drm_amdgpu[] (since
drm_count can be less than MAX_CARDS_SUPPORTED) with -1.
Otherwise you'll have fun experiences during close/print (below).

[Alex Bin Xie]: This initialization is done in existing main() function (not introduced this patch).

-Emil

[-- Attachment #1.2: Type: text/html, Size: 10811 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

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

* Re: [PATCH libdrm 1/3] amdgpu: verify the tested device
       [not found] ` <1484866391-17175-1-git-send-email-AlexBin.Xie-5C7GfCeVMHo@public.gmane.org>
  2017-01-20  3:45   ` zhoucm1
@ 2017-01-20 13:18   ` Emil Velikov
       [not found]     ` <CACvgo50HCafUSw7zDjkS+_1xs67=QgHcOWcO1DDqzBL4RcRBJA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  1 sibling, 1 reply; 11+ messages in thread
From: Emil Velikov @ 2017-01-20 13:18 UTC (permalink / raw)
  To: Alex Xie; +Cc: amd-gfx mailing list

Hi Alex,

Thanks for doing this. There's a few nitpicks on top of what David and
Christian has spotted.

On 19 January 2017 at 22:53, Alex Xie <AlexBin.Xie@amd.com> wrote:
> Verify the vender ID and driver name.
> Open all AMDGPU devices.
> Provide an option to open render node.
>
> Tested as root: PASS
> Tested as non-privileged user:
> All tests failed as expected
>
> Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
> ---
>  tests/amdgpu/amdgpu_test.c | 144 +++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 121 insertions(+), 23 deletions(-)
>
> diff --git a/tests/amdgpu/amdgpu_test.c b/tests/amdgpu/amdgpu_test.c
> index 71f357c..e42ef9d 100644
> --- a/tests/amdgpu/amdgpu_test.c
> +++ b/tests/amdgpu/amdgpu_test.c
> @@ -115,6 +115,119 @@ static const char usage[] = "Usage: %s [-hl] [<-s <suite id>> [-t <test id>]]\n"
>  /** Specified options strings for getopt */
>  static const char options[]   = "hls:t:";
>
> +/* Open AMD devices.
> + * Return the number of AMD device openned.
> + */
> +static int amdgpu_open_devices(int open_render_node)
> +{
> +       drmDevicePtr devices[MAX_CARDS_SUPPORTED];
> +       int ret;
> +       int i;
> +       int j;
> +       int amd_index = 0;
> +       int drm_count;
> +       int fd;
> +       char *device_name;
> +       drmVersionPtr version;
> +
> +       drm_count = drmGetDevices2(0, devices, MAX_CARDS_SUPPORTED);
> +
> +       if (drm_count < 0) {
> +               fprintf(stderr,
> +                       "drmGetDevices2() returned an error %d\n",
> +                       drm_count);
> +               return 0;
> +       }
> +
> +       for (i = 0; i < drm_count; i++) {
> +               /* If this is not AMD GPU vender ID, skip*/
> +               if (devices[i]->bustype == DRM_BUS_PCI)
> +                       if (devices[i]->deviceinfo.pci->vendor_id != 0x1002)
> +                               continue;
> +
> +               for (j = 0; j < DRM_NODE_MAX; j++) {
> +                       if (devices[i]->available_nodes & 1 << j) {
> +                               fd = open(
> +                                       devices[i]->nodes[j],
> +                                       O_RDONLY | O_CLOEXEC,
> +                                       0);
> +                               if (fd < 0) continue;
> +                       }
You don't need to iterate over all the available nodes. Just fetch the
PRIMARY or RENDER based on open_render_node.
Note that a device can be missing some node types (say RENDER) so make
sure the available_nodes bitmask is set.


> +                       if (open_render_node)
> +                               device_name = drmGetRenderDeviceNameFromFd(fd);
> +                       else
> +                               device_name = drmGetPrimaryDeviceNameFromFd(fd);
> +
> +                       close(fd);
> +
> +                       drm_amdgpu[amd_index] = open(device_name,
> +                                                       O_RDWR | O_CLOEXEC);
> +
> +                       if (drm_amdgpu[amd_index] >= 0)
> +                               amd_index++;
> +
> +                       free(device_name);
> +
With the above comment this becomes redundant.

> +                       /* We have open this device. Go to next device.*/
> +                       break;
> +               }
> +       }
> +
Here you want to initialise the remainder of drm_amdgpu[] (since
drm_count can be less than MAX_CARDS_SUPPORTED) with -1.
Otherwise you'll have fun experiences during close/print (below).

-Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* Re: [PATCH libdrm 1/3] amdgpu: verify the tested device
       [not found]     ` <588187BC.9070806-5C7GfCeVMHo@public.gmane.org>
@ 2017-01-20  8:40       ` Christian König
  0 siblings, 0 replies; 11+ messages in thread
From: Christian König @ 2017-01-20  8:40 UTC (permalink / raw)
  To: zhoucm1, Alex Xie, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Am 20.01.2017 um 04:45 schrieb zhoucm1:
> some small nitpick...
>
> On 2017年01月20日 06:53, Alex Xie wrote:
>> Verify the vender ID and driver name.
>> Open all AMDGPU devices.
>> Provide an option to open render node.
>>
>> Tested as root: PASS
>> Tested as non-privileged user:
>> All tests failed as expected
>>
>> Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
>> ---
>>   tests/amdgpu/amdgpu_test.c | 144 
>> +++++++++++++++++++++++++++++++++++++--------
>>   1 file changed, 121 insertions(+), 23 deletions(-)
>>
>> diff --git a/tests/amdgpu/amdgpu_test.c b/tests/amdgpu/amdgpu_test.c
>> index 71f357c..e42ef9d 100644
>> --- a/tests/amdgpu/amdgpu_test.c
>> +++ b/tests/amdgpu/amdgpu_test.c
>> @@ -115,6 +115,119 @@ static const char usage[] = "Usage: %s [-hl] 
>> [<-s <suite id>> [-t <test id>]]\n"
>>   /** Specified options strings for getopt */
>>   static const char options[]   = "hls:t:";
>>   +/* Open AMD devices.
>> + * Return the number of AMD device openned.
>> + */
>> +static int amdgpu_open_devices(int open_render_node)
>> +{
>> +    drmDevicePtr devices[MAX_CARDS_SUPPORTED];
>> +    int ret;
>> +    int i;
>> +    int j;
>> +    int amd_index = 0;
>> +    int drm_count;
>> +    int fd;
>> +    char *device_name;
>> +    drmVersionPtr version;
>> +
>> +    drm_count = drmGetDevices2(0, devices, MAX_CARDS_SUPPORTED);
>> +
>> +    if (drm_count < 0) {
>> +        fprintf(stderr,
>> +            "drmGetDevices2() returned an error %d\n",
>> +            drm_count);
>> +        return 0;
>> +    }
>> +
>> +    for (i = 0; i < drm_count; i++) {
>> +        /* If this is not AMD GPU vender ID, skip*/
>> +        if (devices[i]->bustype == DRM_BUS_PCI)
>> +            if (devices[i]->deviceinfo.pci->vendor_id != 0x1002)
>> +                continue;

Additional to what David noted that check probably needs to be:

if (devices[i]->bustype != DRM_BUS_PCI ||
     devices[i]->deviceinfo.pci->vendor_id != 0x1002)
         continue;

Or do we sell GPUs connected to USB as well?

Apart from that looks good to me and it is really good that somebody 
finally looks into this cause trouble with that on my A+A laptop.

Regards,
Christian.

>> +
>> +        for (j = 0; j < DRM_NODE_MAX; j++) {
>> +            if (devices[i]->available_nodes & 1 << j) {
>> +                fd = open(
>> +                    devices[i]->nodes[j],
>> +                    O_RDONLY | O_CLOEXEC,
>> +                    0);
>> +                if (fd < 0) continue;
>> +            }
>> +
>> +            version = drmGetVersion(fd);
>> +            if (!version) {
>> +                fprintf(stderr,
>> +                    "Warning: Cannot get version for %s."
>> +                    "Error is %s\n",
>> +                    devices[i]->nodes[j],
>> +                    strerror(errno));
>> +                close(fd);
>> +                break;
>> +            }
>> +
>> +            if (strcmp(version->name, "amdgpu")) {
>> +                /* This is not AMDGPU driver, skip.*/
>> +                drmFreeVersion(version);
>> +                close(fd);
>> +                break;
>> +            }
>> +
>> +            drmFreeVersion(version);
>> +
>> +            if (open_render_node)
>> +                device_name = drmGetRenderDeviceNameFromFd(fd);
>> +            else
>> +                device_name = drmGetPrimaryDeviceNameFromFd(fd);
>> +
>> +            close(fd);
>> +
>> +            drm_amdgpu[amd_index] = open(device_name,
>> +                            O_RDWR | O_CLOEXEC);
>> +
>> +            if (drm_amdgpu[amd_index] >= 0)
>> +                amd_index++;
>> +
>> +            free(device_name);
>> +
>> +            /* We have open this device. Go to next device.*/
>> +            break;
>> +        }
>> +    }
>> +
>> +    drmFreeDevices(devices, drm_count);
> here needs return value;
>> +}
>> +
>> +/* Close AMD devices.
>> + */
>> +static void amdgpu_close_devices()
>> +{
>> +    int i;
>> +    for (i = 0; i < MAX_CARDS_SUPPORTED; i++)
>> +        if (drm_amdgpu[i] >=0)
>> +            close(drm_amdgpu[i]);
>> +}
>> +
>> +/* Print AMD devices information */
>> +static void amdgpu_print_devices()
>> +{
>> +    int i;
>> +    for (i = 0; i < MAX_CARDS_SUPPORTED; i++)
>> +        if (drm_amdgpu[i] >=0) {
>> +            /** Display version of DRM driver */
>> +            drmVersionPtr retval = drmGetVersion(drm_amdgpu[0]);
>> +
>> +            if (retval == NULL) {
>> +                perror("Cannot get version for AMDGPU device");
>> +                exit(EXIT_FAILURE);
>> +            }
>> +
>> +            printf("AMDGPU device #%d: "
>> +                "Name: [%s] : Date [%s] : Description [%s]\n",
>> +                i, retval->name, retval->date, retval->desc);
>> +            drmFreeVersion(retval);
>> +        }
>> +}
>> +
>>   /* The main() function for setting up and running the tests.
>>    * Returns a CUE_SUCCESS on successful running, another
>>    * CUnit error code on failure.
>> @@ -163,35 +276,20 @@ int main(int argc, char **argv)
>>           }
>>       }
>>   -    /* Try to open all possible radeon connections
>> -     * Right now: Open only the 0.
>> -     */
>> -    printf("Try to open the card 0..\n");
>> -    drm_amdgpu[0] = open("/dev/dri/card0", O_RDWR | O_CLOEXEC);
>> +    amdgpu_open_devices(0);
> we'd better to check this function return value.
>
> Regards,
> David Zhou
>>         if (drm_amdgpu[0] < 0) {
>> -        perror("Cannot open /dev/dri/card0\n");
>> -        exit(EXIT_FAILURE);
>> -    }
>> -
>> -    /** Display version of DRM driver */
>> -    drmVersionPtr retval = drmGetVersion(drm_amdgpu[0]);
>> -
>> -    if (retval == NULL) {
>> -        perror("Could not get information about DRM driver");
>> +        perror("Cannot open AMDGPU device.\n");
>>           exit(EXIT_FAILURE);
>>       }
>>   -    printf("DRM Driver: Name: [%s] : Date [%s] : Description [%s]\n",
>> -           retval->name, retval->date, retval->desc);
>> -
>> -    drmFreeVersion(retval);
>> +    amdgpu_print_devices();
>>         /* Initialize test suites to run */
>>         /* initialize the CUnit test registry */
>>       if (CUE_SUCCESS != CU_initialize_registry()) {
>> -        close(drm_amdgpu[0]);
>> +        amdgpu_close_devices();
>>           return CU_get_error();
>>       }
>>   @@ -200,7 +298,7 @@ int main(int argc, char **argv)
>>           fprintf(stderr, "suite registration failed - %s\n",
>>                   CU_get_error_msg());
>>           CU_cleanup_registry();
>> -        close(drm_amdgpu[0]);
>> +        amdgpu_close_devices();
>>           exit(EXIT_FAILURE);
>>       }
>>   @@ -222,7 +320,7 @@ int main(int argc, char **argv)
>>                       fprintf(stderr, "Invalid test id: %d\n",
>>                                   test_id);
>>                       CU_cleanup_registry();
>> -                    close(drm_amdgpu[0]);
>> +                    amdgpu_close_devices();
>>                       exit(EXIT_FAILURE);
>>                   }
>>               } else
>> @@ -231,13 +329,13 @@ int main(int argc, char **argv)
>>               fprintf(stderr, "Invalid suite id : %d\n",
>>                       suite_id);
>>               CU_cleanup_registry();
>> -            close(drm_amdgpu[0]);
>> +            amdgpu_close_devices();
>>               exit(EXIT_FAILURE);
>>           }
>>       } else
>>           CU_basic_run_tests();
>>         CU_cleanup_registry();
>> -    close(drm_amdgpu[0]);
>> +    amdgpu_close_devices();
>>       return CU_get_error();
>>   }
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

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

* Re: [PATCH libdrm 1/3] amdgpu: verify the tested device
       [not found] ` <1484866391-17175-1-git-send-email-AlexBin.Xie-5C7GfCeVMHo@public.gmane.org>
@ 2017-01-20  3:45   ` zhoucm1
       [not found]     ` <588187BC.9070806-5C7GfCeVMHo@public.gmane.org>
  2017-01-20 13:18   ` Emil Velikov
  1 sibling, 1 reply; 11+ messages in thread
From: zhoucm1 @ 2017-01-20  3:45 UTC (permalink / raw)
  To: Alex Xie, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

some small nitpick...

On 2017年01月20日 06:53, Alex Xie wrote:
> Verify the vender ID and driver name.
> Open all AMDGPU devices.
> Provide an option to open render node.
>
> Tested as root: PASS
> Tested as non-privileged user:
> All tests failed as expected
>
> Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
> ---
>   tests/amdgpu/amdgpu_test.c | 144 +++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 121 insertions(+), 23 deletions(-)
>
> diff --git a/tests/amdgpu/amdgpu_test.c b/tests/amdgpu/amdgpu_test.c
> index 71f357c..e42ef9d 100644
> --- a/tests/amdgpu/amdgpu_test.c
> +++ b/tests/amdgpu/amdgpu_test.c
> @@ -115,6 +115,119 @@ static const char usage[] = "Usage: %s [-hl] [<-s <suite id>> [-t <test id>]]\n"
>   /** Specified options strings for getopt */
>   static const char options[]   = "hls:t:";
>   
> +/* Open AMD devices.
> + * Return the number of AMD device openned.
> + */
> +static int amdgpu_open_devices(int open_render_node)
> +{
> +	drmDevicePtr devices[MAX_CARDS_SUPPORTED];
> +	int ret;
> +	int i;
> +	int j;
> +	int amd_index = 0;
> +	int drm_count;
> +	int fd;
> +	char *device_name;
> +	drmVersionPtr version;
> +
> +	drm_count = drmGetDevices2(0, devices, MAX_CARDS_SUPPORTED);
> +
> +	if (drm_count < 0) {
> +		fprintf(stderr,
> +			"drmGetDevices2() returned an error %d\n",
> +			drm_count);
> +		return 0;
> +	}
> +
> +	for (i = 0; i < drm_count; i++) {
> +		/* If this is not AMD GPU vender ID, skip*/
> +		if (devices[i]->bustype == DRM_BUS_PCI)
> +			if (devices[i]->deviceinfo.pci->vendor_id != 0x1002)
> +				continue;
> +
> +		for (j = 0; j < DRM_NODE_MAX; j++) {
> +			if (devices[i]->available_nodes & 1 << j) {
> +				fd = open(
> +					devices[i]->nodes[j],
> +					O_RDONLY | O_CLOEXEC,
> +					0);
> +				if (fd < 0) continue;
> +			}
> +
> +			version = drmGetVersion(fd);
> +			if (!version) {
> +				fprintf(stderr,
> +					"Warning: Cannot get version for %s."
> +					"Error is %s\n",
> +					devices[i]->nodes[j],
> +					strerror(errno));
> +				close(fd);
> +				break;
> +			}
> +
> +			if (strcmp(version->name, "amdgpu")) {
> +				/* This is not AMDGPU driver, skip.*/
> +				drmFreeVersion(version);
> +				close(fd);
> +				break;
> +			}
> +
> +			drmFreeVersion(version);
> +
> +			if (open_render_node)
> +				device_name = drmGetRenderDeviceNameFromFd(fd);
> +			else
> +				device_name = drmGetPrimaryDeviceNameFromFd(fd);
> +
> +			close(fd);
> +
> +			drm_amdgpu[amd_index] = open(device_name,
> +							O_RDWR | O_CLOEXEC);
> +
> +			if (drm_amdgpu[amd_index] >= 0)
> +				amd_index++;
> +
> +			free(device_name);
> +
> +			/* We have open this device. Go to next device.*/
> +			break;
> +		}
> +	}
> +
> +	drmFreeDevices(devices, drm_count);
here needs return value;
> +}
> +
> +/* Close AMD devices.
> + */
> +static void amdgpu_close_devices()
> +{
> +	int i;
> +	for (i = 0; i < MAX_CARDS_SUPPORTED; i++)
> +		if (drm_amdgpu[i] >=0)
> +			close(drm_amdgpu[i]);
> +}
> +
> +/* Print AMD devices information */
> +static void amdgpu_print_devices()
> +{
> +	int i;
> +	for (i = 0; i < MAX_CARDS_SUPPORTED; i++)
> +		if (drm_amdgpu[i] >=0) {
> +			/** Display version of DRM driver */
> +			drmVersionPtr retval = drmGetVersion(drm_amdgpu[0]);
> +
> +			if (retval == NULL) {
> +				perror("Cannot get version for AMDGPU device");
> +				exit(EXIT_FAILURE);
> +			}
> +
> +			printf("AMDGPU device #%d: "
> +				"Name: [%s] : Date [%s] : Description [%s]\n",
> +				i, retval->name, retval->date, retval->desc);
> +			drmFreeVersion(retval);
> +		}
> +}
> +
>   /* The main() function for setting up and running the tests.
>    * Returns a CUE_SUCCESS on successful running, another
>    * CUnit error code on failure.
> @@ -163,35 +276,20 @@ int main(int argc, char **argv)
>   		}
>   	}
>   
> -	/* Try to open all possible radeon connections
> -	 * Right now: Open only the 0.
> -	 */
> -	printf("Try to open the card 0..\n");
> -	drm_amdgpu[0] = open("/dev/dri/card0", O_RDWR | O_CLOEXEC);
> +	amdgpu_open_devices(0);
we'd better to check this function return value.

Regards,
David Zhou
>   
>   	if (drm_amdgpu[0] < 0) {
> -		perror("Cannot open /dev/dri/card0\n");
> -		exit(EXIT_FAILURE);
> -	}
> -
> -	/** Display version of DRM driver */
> -	drmVersionPtr retval = drmGetVersion(drm_amdgpu[0]);
> -
> -	if (retval == NULL) {
> -		perror("Could not get information about DRM driver");
> +		perror("Cannot open AMDGPU device.\n");
>   		exit(EXIT_FAILURE);
>   	}
>   
> -	printf("DRM Driver: Name: [%s] : Date [%s] : Description [%s]\n",
> -	       retval->name, retval->date, retval->desc);
> -
> -	drmFreeVersion(retval);
> +	amdgpu_print_devices();
>   
>   	/* Initialize test suites to run */
>   
>   	/* initialize the CUnit test registry */
>   	if (CUE_SUCCESS != CU_initialize_registry()) {
> -		close(drm_amdgpu[0]);
> +		amdgpu_close_devices();
>   		return CU_get_error();
>   	}
>   
> @@ -200,7 +298,7 @@ int main(int argc, char **argv)
>   		fprintf(stderr, "suite registration failed - %s\n",
>   				CU_get_error_msg());
>   		CU_cleanup_registry();
> -		close(drm_amdgpu[0]);
> +		amdgpu_close_devices();
>   		exit(EXIT_FAILURE);
>   	}
>   
> @@ -222,7 +320,7 @@ int main(int argc, char **argv)
>   					fprintf(stderr, "Invalid test id: %d\n",
>   								test_id);
>   					CU_cleanup_registry();
> -					close(drm_amdgpu[0]);
> +					amdgpu_close_devices();
>   					exit(EXIT_FAILURE);
>   				}
>   			} else
> @@ -231,13 +329,13 @@ int main(int argc, char **argv)
>   			fprintf(stderr, "Invalid suite id : %d\n",
>   					suite_id);
>   			CU_cleanup_registry();
> -			close(drm_amdgpu[0]);
> +			amdgpu_close_devices();
>   			exit(EXIT_FAILURE);
>   		}
>   	} else
>   		CU_basic_run_tests();
>   
>   	CU_cleanup_registry();
> -	close(drm_amdgpu[0]);
> +	amdgpu_close_devices();
>   	return CU_get_error();
>   }

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

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

* [PATCH libdrm 1/3] amdgpu: verify the tested device
@ 2017-01-19 22:53 Alex Xie
       [not found] ` <1484866391-17175-1-git-send-email-AlexBin.Xie-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Alex Xie @ 2017-01-19 22:53 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Alex Xie

Verify the vender ID and driver name.
Open all AMDGPU devices.
Provide an option to open render node.

Tested as root: PASS
Tested as non-privileged user:
All tests failed as expected

Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
---
 tests/amdgpu/amdgpu_test.c | 144 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 121 insertions(+), 23 deletions(-)

diff --git a/tests/amdgpu/amdgpu_test.c b/tests/amdgpu/amdgpu_test.c
index 71f357c..e42ef9d 100644
--- a/tests/amdgpu/amdgpu_test.c
+++ b/tests/amdgpu/amdgpu_test.c
@@ -115,6 +115,119 @@ static const char usage[] = "Usage: %s [-hl] [<-s <suite id>> [-t <test id>]]\n"
 /** Specified options strings for getopt */
 static const char options[]   = "hls:t:";
 
+/* Open AMD devices.
+ * Return the number of AMD device openned.
+ */
+static int amdgpu_open_devices(int open_render_node)
+{
+	drmDevicePtr devices[MAX_CARDS_SUPPORTED];
+	int ret;
+	int i;
+	int j;
+	int amd_index = 0;
+	int drm_count;
+	int fd;
+	char *device_name;
+	drmVersionPtr version;
+
+	drm_count = drmGetDevices2(0, devices, MAX_CARDS_SUPPORTED);
+
+	if (drm_count < 0) {
+		fprintf(stderr,
+			"drmGetDevices2() returned an error %d\n",
+			drm_count);
+		return 0;
+	}
+
+	for (i = 0; i < drm_count; i++) {
+		/* If this is not AMD GPU vender ID, skip*/
+		if (devices[i]->bustype == DRM_BUS_PCI)
+			if (devices[i]->deviceinfo.pci->vendor_id != 0x1002)
+				continue;
+
+		for (j = 0; j < DRM_NODE_MAX; j++) {
+			if (devices[i]->available_nodes & 1 << j) {
+				fd = open(
+					devices[i]->nodes[j],
+					O_RDONLY | O_CLOEXEC,
+					0);
+				if (fd < 0) continue;
+			}
+
+			version = drmGetVersion(fd);
+			if (!version) {
+				fprintf(stderr,
+					"Warning: Cannot get version for %s."
+					"Error is %s\n",
+					devices[i]->nodes[j],
+					strerror(errno));
+				close(fd);
+				break;
+			}
+
+			if (strcmp(version->name, "amdgpu")) {
+				/* This is not AMDGPU driver, skip.*/
+				drmFreeVersion(version);
+				close(fd);
+				break;
+			}
+
+			drmFreeVersion(version);
+
+			if (open_render_node)
+				device_name = drmGetRenderDeviceNameFromFd(fd);
+			else
+				device_name = drmGetPrimaryDeviceNameFromFd(fd);
+
+			close(fd);
+
+			drm_amdgpu[amd_index] = open(device_name,
+							O_RDWR | O_CLOEXEC);
+
+			if (drm_amdgpu[amd_index] >= 0)
+				amd_index++;
+
+			free(device_name);
+
+			/* We have open this device. Go to next device.*/
+			break;
+		}
+	}
+
+	drmFreeDevices(devices, drm_count);
+}
+
+/* Close AMD devices.
+ */
+static void amdgpu_close_devices()
+{
+	int i;
+	for (i = 0; i < MAX_CARDS_SUPPORTED; i++)
+		if (drm_amdgpu[i] >=0)
+			close(drm_amdgpu[i]);
+}
+
+/* Print AMD devices information */
+static void amdgpu_print_devices()
+{
+	int i;
+	for (i = 0; i < MAX_CARDS_SUPPORTED; i++)
+		if (drm_amdgpu[i] >=0) {
+			/** Display version of DRM driver */
+			drmVersionPtr retval = drmGetVersion(drm_amdgpu[0]);
+
+			if (retval == NULL) {
+				perror("Cannot get version for AMDGPU device");
+				exit(EXIT_FAILURE);
+			}
+
+			printf("AMDGPU device #%d: "
+				"Name: [%s] : Date [%s] : Description [%s]\n",
+				i, retval->name, retval->date, retval->desc);
+			drmFreeVersion(retval);
+		}
+}
+
 /* The main() function for setting up and running the tests.
  * Returns a CUE_SUCCESS on successful running, another
  * CUnit error code on failure.
@@ -163,35 +276,20 @@ int main(int argc, char **argv)
 		}
 	}
 
-	/* Try to open all possible radeon connections
-	 * Right now: Open only the 0.
-	 */
-	printf("Try to open the card 0..\n");
-	drm_amdgpu[0] = open("/dev/dri/card0", O_RDWR | O_CLOEXEC);
+	amdgpu_open_devices(0);
 
 	if (drm_amdgpu[0] < 0) {
-		perror("Cannot open /dev/dri/card0\n");
-		exit(EXIT_FAILURE);
-	}
-
-	/** Display version of DRM driver */
-	drmVersionPtr retval = drmGetVersion(drm_amdgpu[0]);
-
-	if (retval == NULL) {
-		perror("Could not get information about DRM driver");
+		perror("Cannot open AMDGPU device.\n");
 		exit(EXIT_FAILURE);
 	}
 
-	printf("DRM Driver: Name: [%s] : Date [%s] : Description [%s]\n",
-	       retval->name, retval->date, retval->desc);
-
-	drmFreeVersion(retval);
+	amdgpu_print_devices();
 
 	/* Initialize test suites to run */
 
 	/* initialize the CUnit test registry */
 	if (CUE_SUCCESS != CU_initialize_registry()) {
-		close(drm_amdgpu[0]);
+		amdgpu_close_devices();
 		return CU_get_error();
 	}
 
@@ -200,7 +298,7 @@ int main(int argc, char **argv)
 		fprintf(stderr, "suite registration failed - %s\n",
 				CU_get_error_msg());
 		CU_cleanup_registry();
-		close(drm_amdgpu[0]);
+		amdgpu_close_devices();
 		exit(EXIT_FAILURE);
 	}
 
@@ -222,7 +320,7 @@ int main(int argc, char **argv)
 					fprintf(stderr, "Invalid test id: %d\n",
 								test_id);
 					CU_cleanup_registry();
-					close(drm_amdgpu[0]);
+					amdgpu_close_devices();
 					exit(EXIT_FAILURE);
 				}
 			} else
@@ -231,13 +329,13 @@ int main(int argc, char **argv)
 			fprintf(stderr, "Invalid suite id : %d\n",
 					suite_id);
 			CU_cleanup_registry();
-			close(drm_amdgpu[0]);
+			amdgpu_close_devices();
 			exit(EXIT_FAILURE);
 		}
 	} else
 		CU_basic_run_tests();
 
 	CU_cleanup_registry();
-	close(drm_amdgpu[0]);
+	amdgpu_close_devices();
 	return CU_get_error();
 }
-- 
2.7.4

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

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

end of thread, other threads:[~2017-01-27  0:11 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-24 22:29 [PATCH libdrm 1/3] amdgpu: verify the tested device Alex Xie
     [not found] ` <1485296992-2719-1-git-send-email-AlexBin.Xie-5C7GfCeVMHo@public.gmane.org>
2017-01-24 22:29   ` [PATCH libdrm 2/3] amdgpu: A new option to choose which device to run most tests Alex Xie
2017-01-24 22:29   ` [PATCH libdrm 3/3] amdgpu: A new option to run tests on render node Alex Xie
     [not found]     ` <1485296992-2719-3-git-send-email-AlexBin.Xie-5C7GfCeVMHo@public.gmane.org>
2017-01-27  0:11       ` Emil Velikov
2017-01-27  0:08   ` [PATCH libdrm 1/3] amdgpu: verify the tested device Emil Velikov
  -- strict thread matches above, loose matches on Subject: below --
2017-01-19 22:53 Alex Xie
     [not found] ` <1484866391-17175-1-git-send-email-AlexBin.Xie-5C7GfCeVMHo@public.gmane.org>
2017-01-20  3:45   ` zhoucm1
     [not found]     ` <588187BC.9070806-5C7GfCeVMHo@public.gmane.org>
2017-01-20  8:40       ` Christian König
2017-01-20 13:18   ` Emil Velikov
     [not found]     ` <CACvgo50HCafUSw7zDjkS+_1xs67=QgHcOWcO1DDqzBL4RcRBJA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-01-20 19:14       ` Xie, AlexBin
     [not found]         ` <CY4PR12MB1640D78EBE9F998A00F5323FF2710-rpdhrqHFk06q//3LJcutlgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2017-01-20 19:45           ` Emil Velikov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.