* [PATCH libdrm 02/10] xf86drm: introduce drm_device_has_rdev() helper
2018-06-25 17:36 [PATCH libdrm 01/10] xf86drm: drmGetDevice2: error out if the fd has unknown subsys Emil Velikov
@ 2018-06-25 17:36 ` Emil Velikov
2018-06-28 10:14 ` Robert Foss
2018-06-25 17:36 ` [PATCH libdrm 03/10] xf86drm: Fold drmDevice processing into process_device() helper Emil Velikov
` (9 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Emil Velikov @ 2018-06-25 17:36 UTC (permalink / raw)
To: dri-devel; +Cc: emil.l.velikov
From: Emil Velikov <emil.velikov@collabora.com>
Currently we match the opened drmDevice fd with each drmDevice we
process.
Move that after all the devices are processed and folded, via the
drm_device_has_rdev(). This makes the code easier to follow and allows
us to unify the massive process loop across drmGetDevice2 and
drmGetDevices2. That in itself is coming with a later commit.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
xf86drm.c | 34 +++++++++++++++++++++++++---------
1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/xf86drm.c b/xf86drm.c
index e1bbbe99..cbc0a408 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -3705,6 +3705,21 @@ drm_device_validate_flags(uint32_t flags)
return (flags & ~DRM_DEVICE_GET_PCI_REVISION);
}
+static bool
+drm_device_has_rdev(drmDevicePtr device, dev_t find_rdev)
+{
+ struct stat sbuf;
+
+ for (int i = 0; i < DRM_NODE_MAX; i++) {
+ if (device->available_nodes & 1 << i) {
+ if (stat(device->nodes[i], &sbuf) == 0 &&
+ sbuf.st_rdev == find_rdev)
+ return true;
+ }
+ }
+ return false;
+}
+
/**
* Get information about the opened drm device
*
@@ -3889,21 +3904,22 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
local_devices = temp;
}
- /* store target at local_devices[0] for ease to use below */
- if (find_rdev == sbuf.st_rdev && i) {
- local_devices[i] = local_devices[0];
- local_devices[0] = d;
- }
- else
- local_devices[i] = d;
+ local_devices[i] = d;
i++;
}
node_count = i;
drmFoldDuplicatedDevices(local_devices, node_count);
- *device = local_devices[0];
- drmFreeDevices(&local_devices[1], node_count - 1);
+ for (i = 0; i < node_count; i++) {
+ if (!local_devices[i])
+ continue;
+
+ if (drm_device_has_rdev(local_devices[i], find_rdev))
+ *device = local_devices[i];
+ else
+ drmFreeDevice(&local_devices[i]);
+ }
closedir(sysdir);
free(local_devices);
--
2.18.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH libdrm 02/10] xf86drm: introduce drm_device_has_rdev() helper
2018-06-25 17:36 ` [PATCH libdrm 02/10] xf86drm: introduce drm_device_has_rdev() helper Emil Velikov
@ 2018-06-28 10:14 ` Robert Foss
0 siblings, 0 replies; 31+ messages in thread
From: Robert Foss @ 2018-06-28 10:14 UTC (permalink / raw)
To: Emil Velikov, dri-devel
Feel free to add my r-b to this patch.
On 2018-06-25 19:36, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> Currently we match the opened drmDevice fd with each drmDevice we
> process.
>
> Move that after all the devices are processed and folded, via the
> drm_device_has_rdev(). This makes the code easier to follow and allows
> us to unify the massive process loop across drmGetDevice2 and
> drmGetDevices2. That in itself is coming with a later commit.
>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> xf86drm.c | 34 +++++++++++++++++++++++++---------
> 1 file changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index e1bbbe99..cbc0a408 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -3705,6 +3705,21 @@ drm_device_validate_flags(uint32_t flags)
> return (flags & ~DRM_DEVICE_GET_PCI_REVISION);
> }
>
> +static bool
> +drm_device_has_rdev(drmDevicePtr device, dev_t find_rdev)
> +{
> + struct stat sbuf;
> +
> + for (int i = 0; i < DRM_NODE_MAX; i++) {
> + if (device->available_nodes & 1 << i) {
> + if (stat(device->nodes[i], &sbuf) == 0 &&
> + sbuf.st_rdev == find_rdev)
> + return true;
> + }
> + }
> + return false;
> +}
> +
> /**
> * Get information about the opened drm device
> *
> @@ -3889,21 +3904,22 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
> local_devices = temp;
> }
>
> - /* store target at local_devices[0] for ease to use below */
> - if (find_rdev == sbuf.st_rdev && i) {
> - local_devices[i] = local_devices[0];
> - local_devices[0] = d;
> - }
> - else
> - local_devices[i] = d;
> + local_devices[i] = d;
> i++;
> }
> node_count = i;
>
> drmFoldDuplicatedDevices(local_devices, node_count);
>
> - *device = local_devices[0];
> - drmFreeDevices(&local_devices[1], node_count - 1);
> + for (i = 0; i < node_count; i++) {
> + if (!local_devices[i])
> + continue;
> +
> + if (drm_device_has_rdev(local_devices[i], find_rdev))
> + *device = local_devices[i];
> + else
> + drmFreeDevice(&local_devices[i]);
> + }
>
> closedir(sysdir);
> free(local_devices);
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH libdrm 03/10] xf86drm: Fold drmDevice processing into process_device() helper
2018-06-25 17:36 [PATCH libdrm 01/10] xf86drm: drmGetDevice2: error out if the fd has unknown subsys Emil Velikov
2018-06-25 17:36 ` [PATCH libdrm 02/10] xf86drm: introduce drm_device_has_rdev() helper Emil Velikov
@ 2018-06-25 17:36 ` Emil Velikov
2018-06-28 11:50 ` Robert Foss
2018-06-25 17:36 ` [PATCH libdrm 04/10] xf86drm: Allocate drmDevicePtr's on stack Emil Velikov
` (8 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Emil Velikov @ 2018-06-25 17:36 UTC (permalink / raw)
To: dri-devel; +Cc: emil.l.velikov
From: Emil Velikov <emil.velikov@collabora.com>
Don't the duplicate (nearly) identical code across the two call sites.
It improves legibility and the diff stat seems nice.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
xf86drm.c | 159 ++++++++++++++++++------------------------------------
1 file changed, 51 insertions(+), 108 deletions(-)
diff --git a/xf86drm.c b/xf86drm.c
index cbc0a408..114cf855 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -3676,6 +3676,52 @@ free_device:
return ret;
}
+static int
+process_device(drmDevicePtr *device, const char *d_name,
+ int req_subsystem_type,
+ bool fetch_deviceinfo, uint32_t flags)
+{
+ struct stat sbuf;
+ char node[PATH_MAX + 1];
+ int node_type, subsystem_type;
+ unsigned int maj, min;
+
+ node_type = drmGetNodeType(d_name);
+ if (node_type < 0)
+ return -1;
+
+ snprintf(node, PATH_MAX, "%s/%s", DRM_DIR_NAME, d_name);
+ if (stat(node, &sbuf))
+ return -1;
+
+ maj = major(sbuf.st_rdev);
+ min = minor(sbuf.st_rdev);
+
+ if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode))
+ return -1;
+
+ subsystem_type = drmParseSubsystemType(maj, min);
+ if (req_subsystem_type != -1 && req_subsystem_type != subsystem_type)
+ return -1;
+
+ switch (subsystem_type) {
+ case DRM_BUS_PCI:
+ return drmProcessPciDevice(device, node, node_type, maj, min,
+ fetch_deviceinfo, flags);
+ case DRM_BUS_USB:
+ return drmProcessUsbDevice(device, node, node_type, maj, min,
+ fetch_deviceinfo, flags);
+ case DRM_BUS_PLATFORM:
+ return drmProcessPlatformDevice(device, node, node_type, maj, min,
+ fetch_deviceinfo, flags);
+ case DRM_BUS_HOST1X:
+ return drmProcessHost1xDevice(device, node, node_type, maj, min,
+ fetch_deviceinfo, flags);
+ default:
+ return -1;
+ }
+}
+
/* Consider devices located on the same bus as duplicate and fold the respective
* entries into a single one.
*
@@ -3805,8 +3851,7 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
DIR *sysdir;
struct dirent *dent;
struct stat sbuf;
- char node[PATH_MAX + 1];
- int node_type, subsystem_type;
+ int subsystem_type;
int maj, min;
int ret, i, node_count;
int max_count = 16;
@@ -3844,55 +3889,9 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
i = 0;
while ((dent = readdir(sysdir))) {
- node_type = drmGetNodeType(dent->d_name);
- if (node_type < 0)
- continue;
-
- snprintf(node, PATH_MAX, "%s/%s", DRM_DIR_NAME, dent->d_name);
- if (stat(node, &sbuf))
- continue;
-
- maj = major(sbuf.st_rdev);
- min = minor(sbuf.st_rdev);
-
- if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode))
- continue;
-
- if (drmParseSubsystemType(maj, min) != subsystem_type)
- continue;
-
- switch (subsystem_type) {
- case DRM_BUS_PCI:
- ret = drmProcessPciDevice(&d, node, node_type, maj, min, true, flags);
- if (ret)
- continue;
-
- break;
-
- case DRM_BUS_USB:
- ret = drmProcessUsbDevice(&d, node, node_type, maj, min, true, flags);
- if (ret)
- continue;
-
- break;
-
- case DRM_BUS_PLATFORM:
- ret = drmProcessPlatformDevice(&d, node, node_type, maj, min, true, flags);
- if (ret)
- continue;
-
- break;
-
- case DRM_BUS_HOST1X:
- ret = drmProcessHost1xDevice(&d, node, node_type, maj, min, true, flags);
- if (ret)
- continue;
-
- break;
-
- default:
+ ret = process_device(&d, dent->d_name, subsystem_type, true, flags);
+ if (ret)
continue;
- }
if (i >= max_count) {
drmDevicePtr *temp;
@@ -3973,10 +3972,6 @@ int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
drmDevicePtr device;
DIR *sysdir;
struct dirent *dent;
- struct stat sbuf;
- char node[PATH_MAX + 1];
- int node_type, subsystem_type;
- int maj, min;
int ret, i, node_count, device_count;
int max_count = 16;
@@ -3995,61 +3990,9 @@ int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
i = 0;
while ((dent = readdir(sysdir))) {
- node_type = drmGetNodeType(dent->d_name);
- if (node_type < 0)
- continue;
-
- snprintf(node, PATH_MAX, "%s/%s", DRM_DIR_NAME, dent->d_name);
- if (stat(node, &sbuf))
- continue;
-
- maj = major(sbuf.st_rdev);
- min = minor(sbuf.st_rdev);
-
- if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode))
- continue;
-
- subsystem_type = drmParseSubsystemType(maj, min);
-
- if (subsystem_type < 0)
- continue;
-
- switch (subsystem_type) {
- case DRM_BUS_PCI:
- ret = drmProcessPciDevice(&device, node, node_type,
- maj, min, devices != NULL, flags);
- if (ret)
- continue;
-
- break;
-
- case DRM_BUS_USB:
- ret = drmProcessUsbDevice(&device, node, node_type, maj, min,
- devices != NULL, flags);
- if (ret)
- continue;
-
- break;
-
- case DRM_BUS_PLATFORM:
- ret = drmProcessPlatformDevice(&device, node, node_type, maj, min,
- devices != NULL, flags);
- if (ret)
- continue;
-
- break;
-
- case DRM_BUS_HOST1X:
- ret = drmProcessHost1xDevice(&device, node, node_type, maj, min,
- devices != NULL, flags);
- if (ret)
- continue;
-
- break;
-
- default:
+ ret = process_device(&device, dent->d_name, -1, devices != NULL, flags);
+ if (ret)
continue;
- }
if (i >= max_count) {
drmDevicePtr *temp;
--
2.18.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH libdrm 03/10] xf86drm: Fold drmDevice processing into process_device() helper
2018-06-25 17:36 ` [PATCH libdrm 03/10] xf86drm: Fold drmDevice processing into process_device() helper Emil Velikov
@ 2018-06-28 11:50 ` Robert Foss
0 siblings, 0 replies; 31+ messages in thread
From: Robert Foss @ 2018-06-28 11:50 UTC (permalink / raw)
To: Emil Velikov, dri-devel
Feel free to add my r-b to this patch.
On 2018-06-25 19:36, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> Don't the duplicate (nearly) identical code across the two call sites.
> It improves legibility and the diff stat seems nice.
>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> xf86drm.c | 159 ++++++++++++++++++------------------------------------
> 1 file changed, 51 insertions(+), 108 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index cbc0a408..114cf855 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -3676,6 +3676,52 @@ free_device:
> return ret;
> }
>
> +static int
> +process_device(drmDevicePtr *device, const char *d_name,
> + int req_subsystem_type,
> + bool fetch_deviceinfo, uint32_t flags)
> +{
> + struct stat sbuf;
> + char node[PATH_MAX + 1];
> + int node_type, subsystem_type;
> + unsigned int maj, min;
> +
> + node_type = drmGetNodeType(d_name);
> + if (node_type < 0)
> + return -1;
> +
> + snprintf(node, PATH_MAX, "%s/%s", DRM_DIR_NAME, d_name);
> + if (stat(node, &sbuf))
> + return -1;
> +
> + maj = major(sbuf.st_rdev);
> + min = minor(sbuf.st_rdev);
> +
> + if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode))
> + return -1;
> +
> + subsystem_type = drmParseSubsystemType(maj, min);
> + if (req_subsystem_type != -1 && req_subsystem_type != subsystem_type)
> + return -1;
> +
> + switch (subsystem_type) {
> + case DRM_BUS_PCI:
> + return drmProcessPciDevice(device, node, node_type, maj, min,
> + fetch_deviceinfo, flags);
> + case DRM_BUS_USB:
> + return drmProcessUsbDevice(device, node, node_type, maj, min,
> + fetch_deviceinfo, flags);
> + case DRM_BUS_PLATFORM:
> + return drmProcessPlatformDevice(device, node, node_type, maj, min,
> + fetch_deviceinfo, flags);
> + case DRM_BUS_HOST1X:
> + return drmProcessHost1xDevice(device, node, node_type, maj, min,
> + fetch_deviceinfo, flags);
> + default:
> + return -1;
> + }
> +}
> +
> /* Consider devices located on the same bus as duplicate and fold the respective
> * entries into a single one.
> *
> @@ -3805,8 +3851,7 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
> DIR *sysdir;
> struct dirent *dent;
> struct stat sbuf;
> - char node[PATH_MAX + 1];
> - int node_type, subsystem_type;
> + int subsystem_type;
> int maj, min;
> int ret, i, node_count;
> int max_count = 16;
> @@ -3844,55 +3889,9 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
>
> i = 0;
> while ((dent = readdir(sysdir))) {
> - node_type = drmGetNodeType(dent->d_name);
> - if (node_type < 0)
> - continue;
> -
> - snprintf(node, PATH_MAX, "%s/%s", DRM_DIR_NAME, dent->d_name);
> - if (stat(node, &sbuf))
> - continue;
> -
> - maj = major(sbuf.st_rdev);
> - min = minor(sbuf.st_rdev);
> -
> - if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode))
> - continue;
> -
> - if (drmParseSubsystemType(maj, min) != subsystem_type)
> - continue;
> -
> - switch (subsystem_type) {
> - case DRM_BUS_PCI:
> - ret = drmProcessPciDevice(&d, node, node_type, maj, min, true, flags);
> - if (ret)
> - continue;
> -
> - break;
> -
> - case DRM_BUS_USB:
> - ret = drmProcessUsbDevice(&d, node, node_type, maj, min, true, flags);
> - if (ret)
> - continue;
> -
> - break;
> -
> - case DRM_BUS_PLATFORM:
> - ret = drmProcessPlatformDevice(&d, node, node_type, maj, min, true, flags);
> - if (ret)
> - continue;
> -
> - break;
> -
> - case DRM_BUS_HOST1X:
> - ret = drmProcessHost1xDevice(&d, node, node_type, maj, min, true, flags);
> - if (ret)
> - continue;
> -
> - break;
> -
> - default:
> + ret = process_device(&d, dent->d_name, subsystem_type, true, flags);
> + if (ret)
> continue;
> - }
>
> if (i >= max_count) {
> drmDevicePtr *temp;
> @@ -3973,10 +3972,6 @@ int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
> drmDevicePtr device;
> DIR *sysdir;
> struct dirent *dent;
> - struct stat sbuf;
> - char node[PATH_MAX + 1];
> - int node_type, subsystem_type;
> - int maj, min;
> int ret, i, node_count, device_count;
> int max_count = 16;
>
> @@ -3995,61 +3990,9 @@ int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
>
> i = 0;
> while ((dent = readdir(sysdir))) {
> - node_type = drmGetNodeType(dent->d_name);
> - if (node_type < 0)
> - continue;
> -
> - snprintf(node, PATH_MAX, "%s/%s", DRM_DIR_NAME, dent->d_name);
> - if (stat(node, &sbuf))
> - continue;
> -
> - maj = major(sbuf.st_rdev);
> - min = minor(sbuf.st_rdev);
> -
> - if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode))
> - continue;
> -
> - subsystem_type = drmParseSubsystemType(maj, min);
> -
> - if (subsystem_type < 0)
> - continue;
> -
> - switch (subsystem_type) {
> - case DRM_BUS_PCI:
> - ret = drmProcessPciDevice(&device, node, node_type,
> - maj, min, devices != NULL, flags);
> - if (ret)
> - continue;
> -
> - break;
> -
> - case DRM_BUS_USB:
> - ret = drmProcessUsbDevice(&device, node, node_type, maj, min,
> - devices != NULL, flags);
> - if (ret)
> - continue;
> -
> - break;
> -
> - case DRM_BUS_PLATFORM:
> - ret = drmProcessPlatformDevice(&device, node, node_type, maj, min,
> - devices != NULL, flags);
> - if (ret)
> - continue;
> -
> - break;
> -
> - case DRM_BUS_HOST1X:
> - ret = drmProcessHost1xDevice(&device, node, node_type, maj, min,
> - devices != NULL, flags);
> - if (ret)
> - continue;
> -
> - break;
> -
> - default:
> + ret = process_device(&device, dent->d_name, -1, devices != NULL, flags);
> + if (ret)
> continue;
> - }
>
> if (i >= max_count) {
> drmDevicePtr *temp;
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH libdrm 04/10] xf86drm: Allocate drmDevicePtr's on stack
2018-06-25 17:36 [PATCH libdrm 01/10] xf86drm: drmGetDevice2: error out if the fd has unknown subsys Emil Velikov
2018-06-25 17:36 ` [PATCH libdrm 02/10] xf86drm: introduce drm_device_has_rdev() helper Emil Velikov
2018-06-25 17:36 ` [PATCH libdrm 03/10] xf86drm: Fold drmDevice processing into process_device() helper Emil Velikov
@ 2018-06-25 17:36 ` Emil Velikov
2018-06-28 12:52 ` Robert Foss
` (2 more replies)
2018-06-25 17:36 ` [PATCH libdrm " Emil Velikov
` (7 subsequent siblings)
10 siblings, 3 replies; 31+ messages in thread
From: Emil Velikov @ 2018-06-25 17:36 UTC (permalink / raw)
To: dri-devel; +Cc: emil.l.velikov
From: Emil Velikov <emil.velikov@collabora.com>
Currently we dynamically allocate 16 pointers and reallocate more as
needed.
Instead, allocate the maximum number (256) on stack - the number is
small enough and is unlikely to change in the foreseeable future.
This allows us to simplify the error handling and even shed a few bytes
off the final binary.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
xf86drm.c | 64 ++++++-------------------------------------------------
1 file changed, 6 insertions(+), 58 deletions(-)
diff --git a/xf86drm.c b/xf86drm.c
index 114cf855..d4810740 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -3846,7 +3846,7 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
return 0;
#else
- drmDevicePtr *local_devices;
+ drmDevicePtr local_devices[256];
drmDevicePtr d;
DIR *sysdir;
struct dirent *dent;
@@ -3854,7 +3854,6 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
int subsystem_type;
int maj, min;
int ret, i, node_count;
- int max_count = 16;
dev_t find_rdev;
if (drm_device_validate_flags(flags))
@@ -3877,15 +3876,9 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
if (subsystem_type < 0)
return subsystem_type;
- local_devices = calloc(max_count, sizeof(drmDevicePtr));
- if (local_devices == NULL)
- return -ENOMEM;
-
sysdir = opendir(DRM_DIR_NAME);
- if (!sysdir) {
- ret = -errno;
- goto free_locals;
- }
+ if (!sysdir)
+ return -errno;
i = 0;
while ((dent = readdir(sysdir))) {
@@ -3893,16 +3886,6 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
if (ret)
continue;
- if (i >= max_count) {
- drmDevicePtr *temp;
-
- max_count += 16;
- temp = realloc(local_devices, max_count * sizeof(drmDevicePtr));
- if (!temp)
- goto free_devices;
- local_devices = temp;
- }
-
local_devices[i] = d;
i++;
}
@@ -3921,18 +3904,9 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
}
closedir(sysdir);
- free(local_devices);
if (*device == NULL)
return -ENODEV;
return 0;
-
-free_devices:
- drmFreeDevices(local_devices, i);
- closedir(sysdir);
-
-free_locals:
- free(local_devices);
- return ret;
#endif
}
@@ -3968,25 +3942,18 @@ int drmGetDevice(int fd, drmDevicePtr *device)
*/
int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
{
- drmDevicePtr *local_devices;
+ drmDevicePtr local_devices[256];
drmDevicePtr device;
DIR *sysdir;
struct dirent *dent;
int ret, i, node_count, device_count;
- int max_count = 16;
if (drm_device_validate_flags(flags))
return -EINVAL;
- local_devices = calloc(max_count, sizeof(drmDevicePtr));
- if (local_devices == NULL)
- return -ENOMEM;
-
sysdir = opendir(DRM_DIR_NAME);
- if (!sysdir) {
- ret = -errno;
- goto free_locals;
- }
+ if (!sysdir)
+ return -errno;
i = 0;
while ((dent = readdir(sysdir))) {
@@ -3994,16 +3961,6 @@ int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
if (ret)
continue;
- if (i >= max_count) {
- drmDevicePtr *temp;
-
- max_count += 16;
- temp = realloc(local_devices, max_count * sizeof(drmDevicePtr));
- if (!temp)
- goto free_devices;
- local_devices = temp;
- }
-
local_devices[i] = device;
i++;
}
@@ -4025,16 +3982,7 @@ int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
}
closedir(sysdir);
- free(local_devices);
return device_count;
-
-free_devices:
- drmFreeDevices(local_devices, i);
- closedir(sysdir);
-
-free_locals:
- free(local_devices);
- return ret;
}
/**
--
2.18.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH libdrm 04/10] xf86drm: Allocate drmDevicePtr's on stack
2018-06-25 17:36 ` [PATCH libdrm 04/10] xf86drm: Allocate drmDevicePtr's on stack Emil Velikov
@ 2018-06-28 12:52 ` Robert Foss
2018-06-28 17:07 ` Emil Velikov
2018-06-29 15:20 ` [PATCH libdrm v2 " Emil Velikov
2018-06-29 15:22 ` [PATCH libdrm v2 05/10] xf86drm: introduce a get_real_pci_path() helper Emil Velikov
2 siblings, 1 reply; 31+ messages in thread
From: Robert Foss @ 2018-06-28 12:52 UTC (permalink / raw)
To: Emil Velikov, dri-devel
Hey Emil,
On 2018-06-25 19:36, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> Currently we dynamically allocate 16 pointers and reallocate more as
> needed.
>
> Instead, allocate the maximum number (256) on stack - the number is
> small enough and is unlikely to change in the foreseeable future.
>
> This allows us to simplify the error handling and even shed a few bytes
> off the final binary.
>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> xf86drm.c | 64 ++++++-------------------------------------------------
> 1 file changed, 6 insertions(+), 58 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 114cf855..d4810740 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -3846,7 +3846,7 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
>
> return 0;
> #else
> - drmDevicePtr *local_devices;
> + drmDevicePtr local_devices[256];
This number is seen later on in this patch, maybe it should be broken out into a
define, since it's reused later on too at [1].
> drmDevicePtr d;
> DIR *sysdir;
> struct dirent *dent;
> @@ -3854,7 +3854,6 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
> int subsystem_type;
> int maj, min;
> int ret, i, node_count;
> - int max_count = 16;
> dev_t find_rdev;
>
> if (drm_device_validate_flags(flags))
> @@ -3877,15 +3876,9 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
> if (subsystem_type < 0)
> return subsystem_type;
>
> - local_devices = calloc(max_count, sizeof(drmDevicePtr));
> - if (local_devices == NULL)
> - return -ENOMEM;
> -
> sysdir = opendir(DRM_DIR_NAME);
> - if (!sysdir) {
> - ret = -errno;
> - goto free_locals;
> - }
> + if (!sysdir)
> + return -errno;
>
> i = 0;
> while ((dent = readdir(sysdir))) {
> @@ -3893,16 +3886,6 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
> if (ret)
> continue;
>
> - if (i >= max_count) {
Is this check really not needded what exactly is it that defines 256 as the maximum?
From what I understand readdir(sysdir) is the call that defines how many
devices will be looked through, and as far as I understand it can return an
arbitrary number of files.
> - drmDevicePtr *temp;
> -
> - max_count += 16;
> - temp = realloc(local_devices, max_count * sizeof(drmDevicePtr));
> - if (!temp)
> - goto free_devices;
> - local_devices = temp;
> - }
> -
> local_devices[i] = d;
> i++;
> }
> @@ -3921,18 +3904,9 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
> }
>
> closedir(sysdir);
> - free(local_devices);
> if (*device == NULL)
> return -ENODEV;
> return 0;
> -
> -free_devices:
> - drmFreeDevices(local_devices, i);
> - closedir(sysdir);
> -
> -free_locals:
> - free(local_devices);
> - return ret;
> #endif
> }
>
> @@ -3968,25 +3942,18 @@ int drmGetDevice(int fd, drmDevicePtr *device)
> */
> int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
> {
> - drmDevicePtr *local_devices;
> + drmDevicePtr local_devices[256];'
[1]
> drmDevicePtr device;
> DIR *sysdir;
> struct dirent *dent;
> int ret, i, node_count, device_count;
> - int max_count = 16;
>
> if (drm_device_validate_flags(flags))
> return -EINVAL;
>
> - local_devices = calloc(max_count, sizeof(drmDevicePtr));
> - if (local_devices == NULL)
> - return -ENOMEM;
> -
> sysdir = opendir(DRM_DIR_NAME);
> - if (!sysdir) {
> - ret = -errno;
> - goto free_locals;
> - }
> + if (!sysdir)
> + return -errno;
>
> i = 0;
> while ((dent = readdir(sysdir))) {
> @@ -3994,16 +3961,6 @@ int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
> if (ret)
> continue;
>
> - if (i >= max_count) {
This out of bounds check should be dealt with too, if the above bounds check
should be.
> - drmDevicePtr *temp;
> -
> - max_count += 16;
> - temp = realloc(local_devices, max_count * sizeof(drmDevicePtr));
> - if (!temp)
> - goto free_devices;
> - local_devices = temp;
> - }
> -
> local_devices[i] = device;
> i++;
> }
> @@ -4025,16 +3982,7 @@ int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
> }
>
> closedir(sysdir);
> - free(local_devices);
> return device_count;
> -
> -free_devices:
> - drmFreeDevices(local_devices, i);
> - closedir(sysdir);
> -
> -free_locals:
> - free(local_devices);
> - return ret;
> }
>
> /**
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH libdrm 04/10] xf86drm: Allocate drmDevicePtr's on stack
2018-06-28 12:52 ` Robert Foss
@ 2018-06-28 17:07 ` Emil Velikov
0 siblings, 0 replies; 31+ messages in thread
From: Emil Velikov @ 2018-06-28 17:07 UTC (permalink / raw)
To: Robert Foss; +Cc: ML dri-devel
Hi Rob,
On 28 June 2018 at 13:52, Robert Foss <robert.foss@collabora.com> wrote:
> Hey Emil,
>
> On 2018-06-25 19:36, Emil Velikov wrote:
>>
>> From: Emil Velikov <emil.velikov@collabora.com>
>>
>> Currently we dynamically allocate 16 pointers and reallocate more as
>> needed.
>>
>> Instead, allocate the maximum number (256) on stack - the number is
>> small enough and is unlikely to change in the foreseeable future.
>>
>> This allows us to simplify the error handling and even shed a few bytes
>> off the final binary.
>>
>> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
>> ---
>> xf86drm.c | 64 ++++++-------------------------------------------------
>> 1 file changed, 6 insertions(+), 58 deletions(-)
>>
>> diff --git a/xf86drm.c b/xf86drm.c
>> index 114cf855..d4810740 100644
>> --- a/xf86drm.c
>> +++ b/xf86drm.c
>> @@ -3846,7 +3846,7 @@ int drmGetDevice2(int fd, uint32_t flags,
>> drmDevicePtr *device)
>> return 0;
>> #else
>> - drmDevicePtr *local_devices;
>> + drmDevicePtr local_devices[256];
>
>
> This number is seen later on in this patch, maybe it should be broken out
> into a
> define, since it's reused later on too at [1].
>
Sure thing. The lame MAX_DRM_NODES comes to mind, so alternatives are welcome.
>
>> drmDevicePtr d;
>> DIR *sysdir;
>> struct dirent *dent;
>> @@ -3854,7 +3854,6 @@ int drmGetDevice2(int fd, uint32_t flags,
>> drmDevicePtr *device)
>> int subsystem_type;
>> int maj, min;
>> int ret, i, node_count;
>> - int max_count = 16;
>> dev_t find_rdev;
>> if (drm_device_validate_flags(flags))
>> @@ -3877,15 +3876,9 @@ int drmGetDevice2(int fd, uint32_t flags,
>> drmDevicePtr *device)
>> if (subsystem_type < 0)
>> return subsystem_type;
>> - local_devices = calloc(max_count, sizeof(drmDevicePtr));
>> - if (local_devices == NULL)
>> - return -ENOMEM;
>> -
>> sysdir = opendir(DRM_DIR_NAME);
>> - if (!sysdir) {
>> - ret = -errno;
>> - goto free_locals;
>> - }
>> + if (!sysdir)
>> + return -errno;
>> i = 0;
>> while ((dent = readdir(sysdir))) {
>> @@ -3893,16 +3886,6 @@ int drmGetDevice2(int fd, uint32_t flags,
>> drmDevicePtr *device)
>> if (ret)
>> continue;
>> - if (i >= max_count) {
>
>
> Is this check really not needded what exactly is it that defines 256 as the
> maximum?
>
> From what I understand readdir(sysdir) is the call that defines how many
> devices will be looked through, and as far as I understand it can return an
> arbitrary number of files.
>
The kernel drm core has a number of places that assume maximum of 3x64
devices nodes.
That's 64 for each of primary, control and render nodes. I've rounded
it up to 256 for simplicity.
I assume we'll be able to see if anyone changes the kernel, although
bailing out just in case is a good idea.
The is one exciting error message - let me know if anything else comes to mind.
if (i >= MAX_DRM_NODES) {
fprintf(stderr, "More than %d drm nodes detected. Please report a
bug - that should not happen.\nSkipping extra nodes\n",
MAX_DRM_NODES);
break;
}
I'll send out v2 for the patches that need work some time
tomorrow/over the weekend..
Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH libdrm v2 04/10] xf86drm: Allocate drmDevicePtr's on stack
2018-06-25 17:36 ` [PATCH libdrm 04/10] xf86drm: Allocate drmDevicePtr's on stack Emil Velikov
2018-06-28 12:52 ` Robert Foss
@ 2018-06-29 15:20 ` Emil Velikov
2018-06-29 15:49 ` Robert Foss
2018-06-29 15:22 ` [PATCH libdrm v2 05/10] xf86drm: introduce a get_real_pci_path() helper Emil Velikov
2 siblings, 1 reply; 31+ messages in thread
From: Emil Velikov @ 2018-06-29 15:20 UTC (permalink / raw)
To: dri-devel; +Cc: Robert Foss, emil.l.velikov
From: Emil Velikov <emil.velikov@collabora.com>
Currently we dynamically allocate 16 pointers and reallocate more as
needed.
Instead, allocate the maximum number (256) on stack - the number is
small enough and is unlikely to change in the foreseeable future.
This allows us to simplify the error handling and even shed a few bytes
off the final binary.
v2:
- add a define & description behind the magic 256 (Rob)
- report error to strerr and skip when over 256 device nodes (Rob)
Cc: Robert Foss <robert.foss@collabora.com>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Tested-by: Robert Foss <robert.foss@collabora.com> (v1)
Reviewed-by: Robert Foss <robert.foss@collabora.com> (v1)
Reviewed-by: Eric Engestrom <eric@engestrom.ch> (v1)
---
xf86drm.c | 79 ++++++++++++++++---------------------------------------
1 file changed, 23 insertions(+), 56 deletions(-)
diff --git a/xf86drm.c b/xf86drm.c
index 114cf855..02da3e1f 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -3766,6 +3766,13 @@ drm_device_has_rdev(drmDevicePtr device, dev_t find_rdev)
return false;
}
+/*
+ * The kernel drm core has a number of places that assume maximum of
+ * 3x64 devices nodes. That's 64 for each of primary, control and
+ * render nodes. Rounded it up to 256 for simplicity.
+ */
+#define MAX_DRM_NODES 256
+
/**
* Get information about the opened drm device
*
@@ -3846,7 +3853,7 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
return 0;
#else
- drmDevicePtr *local_devices;
+ drmDevicePtr local_devices[MAX_DRM_NODES];
drmDevicePtr d;
DIR *sysdir;
struct dirent *dent;
@@ -3854,7 +3861,6 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
int subsystem_type;
int maj, min;
int ret, i, node_count;
- int max_count = 16;
dev_t find_rdev;
if (drm_device_validate_flags(flags))
@@ -3877,15 +3883,9 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
if (subsystem_type < 0)
return subsystem_type;
- local_devices = calloc(max_count, sizeof(drmDevicePtr));
- if (local_devices == NULL)
- return -ENOMEM;
-
sysdir = opendir(DRM_DIR_NAME);
- if (!sysdir) {
- ret = -errno;
- goto free_locals;
- }
+ if (!sysdir)
+ return -errno;
i = 0;
while ((dent = readdir(sysdir))) {
@@ -3893,16 +3893,12 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
if (ret)
continue;
- if (i >= max_count) {
- drmDevicePtr *temp;
-
- max_count += 16;
- temp = realloc(local_devices, max_count * sizeof(drmDevicePtr));
- if (!temp)
- goto free_devices;
- local_devices = temp;
+ if (i >= MAX_DRM_NODES) {
+ fprintf(stderr, "More than %d drm nodes detected. "
+ "Please report a bug - that should not happen.\n"
+ "Skipping extra nodes\n", MAX_DRM_NODES);
+ break;
}
-
local_devices[i] = d;
i++;
}
@@ -3921,18 +3917,9 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
}
closedir(sysdir);
- free(local_devices);
if (*device == NULL)
return -ENODEV;
return 0;
-
-free_devices:
- drmFreeDevices(local_devices, i);
- closedir(sysdir);
-
-free_locals:
- free(local_devices);
- return ret;
#endif
}
@@ -3968,25 +3955,18 @@ int drmGetDevice(int fd, drmDevicePtr *device)
*/
int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
{
- drmDevicePtr *local_devices;
+ drmDevicePtr local_devices[MAX_DRM_NODES];
drmDevicePtr device;
DIR *sysdir;
struct dirent *dent;
int ret, i, node_count, device_count;
- int max_count = 16;
if (drm_device_validate_flags(flags))
return -EINVAL;
- local_devices = calloc(max_count, sizeof(drmDevicePtr));
- if (local_devices == NULL)
- return -ENOMEM;
-
sysdir = opendir(DRM_DIR_NAME);
- if (!sysdir) {
- ret = -errno;
- goto free_locals;
- }
+ if (!sysdir)
+ return -errno;
i = 0;
while ((dent = readdir(sysdir))) {
@@ -3994,16 +3974,12 @@ int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
if (ret)
continue;
- if (i >= max_count) {
- drmDevicePtr *temp;
-
- max_count += 16;
- temp = realloc(local_devices, max_count * sizeof(drmDevicePtr));
- if (!temp)
- goto free_devices;
- local_devices = temp;
+ if (i >= MAX_DRM_NODES) {
+ fprintf(stderr, "More than %d drm nodes detected. "
+ "Please report a bug - that should not happen.\n"
+ "Skipping extra nodes\n", MAX_DRM_NODES);
+ break;
}
-
local_devices[i] = device;
i++;
}
@@ -4025,16 +4001,7 @@ int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
}
closedir(sysdir);
- free(local_devices);
return device_count;
-
-free_devices:
- drmFreeDevices(local_devices, i);
- closedir(sysdir);
-
-free_locals:
- free(local_devices);
- return ret;
}
/**
--
2.18.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH libdrm v2 04/10] xf86drm: Allocate drmDevicePtr's on stack
2018-06-29 15:20 ` [PATCH libdrm v2 " Emil Velikov
@ 2018-06-29 15:49 ` Robert Foss
0 siblings, 0 replies; 31+ messages in thread
From: Robert Foss @ 2018-06-29 15:49 UTC (permalink / raw)
To: Emil Velikov, dri-devel
LGTM
On 2018-06-29 17:20, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> Currently we dynamically allocate 16 pointers and reallocate more as
> needed.
>
> Instead, allocate the maximum number (256) on stack - the number is
> small enough and is unlikely to change in the foreseeable future.
>
> This allows us to simplify the error handling and even shed a few bytes
> off the final binary.
>
> v2:
> - add a define & description behind the magic 256 (Rob)
> - report error to strerr and skip when over 256 device nodes (Rob)
>
> Cc: Robert Foss <robert.foss@collabora.com>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> Tested-by: Robert Foss <robert.foss@collabora.com> (v1)
> Reviewed-by: Robert Foss <robert.foss@collabora.com> (v1)
> Reviewed-by: Eric Engestrom <eric@engestrom.ch> (v1)
> ---
> xf86drm.c | 79 ++++++++++++++++---------------------------------------
> 1 file changed, 23 insertions(+), 56 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 114cf855..02da3e1f 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -3766,6 +3766,13 @@ drm_device_has_rdev(drmDevicePtr device, dev_t find_rdev)
> return false;
> }
>
> +/*
> + * The kernel drm core has a number of places that assume maximum of
> + * 3x64 devices nodes. That's 64 for each of primary, control and
> + * render nodes. Rounded it up to 256 for simplicity.
> + */
> +#define MAX_DRM_NODES 256
> +
> /**
> * Get information about the opened drm device
> *
> @@ -3846,7 +3853,7 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
>
> return 0;
> #else
> - drmDevicePtr *local_devices;
> + drmDevicePtr local_devices[MAX_DRM_NODES];
> drmDevicePtr d;
> DIR *sysdir;
> struct dirent *dent;
> @@ -3854,7 +3861,6 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
> int subsystem_type;
> int maj, min;
> int ret, i, node_count;
> - int max_count = 16;
> dev_t find_rdev;
>
> if (drm_device_validate_flags(flags))
> @@ -3877,15 +3883,9 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
> if (subsystem_type < 0)
> return subsystem_type;
>
> - local_devices = calloc(max_count, sizeof(drmDevicePtr));
> - if (local_devices == NULL)
> - return -ENOMEM;
> -
> sysdir = opendir(DRM_DIR_NAME);
> - if (!sysdir) {
> - ret = -errno;
> - goto free_locals;
> - }
> + if (!sysdir)
> + return -errno;
>
> i = 0;
> while ((dent = readdir(sysdir))) {
> @@ -3893,16 +3893,12 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
> if (ret)
> continue;
>
> - if (i >= max_count) {
> - drmDevicePtr *temp;
> -
> - max_count += 16;
> - temp = realloc(local_devices, max_count * sizeof(drmDevicePtr));
> - if (!temp)
> - goto free_devices;
> - local_devices = temp;
> + if (i >= MAX_DRM_NODES) {
> + fprintf(stderr, "More than %d drm nodes detected. "
> + "Please report a bug - that should not happen.\n"
> + "Skipping extra nodes\n", MAX_DRM_NODES);
> + break;
> }
> -
> local_devices[i] = d;
> i++;
> }
> @@ -3921,18 +3917,9 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
> }
>
> closedir(sysdir);
> - free(local_devices);
> if (*device == NULL)
> return -ENODEV;
> return 0;
> -
> -free_devices:
> - drmFreeDevices(local_devices, i);
> - closedir(sysdir);
> -
> -free_locals:
> - free(local_devices);
> - return ret;
> #endif
> }
>
> @@ -3968,25 +3955,18 @@ int drmGetDevice(int fd, drmDevicePtr *device)
> */
> int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
> {
> - drmDevicePtr *local_devices;
> + drmDevicePtr local_devices[MAX_DRM_NODES];
> drmDevicePtr device;
> DIR *sysdir;
> struct dirent *dent;
> int ret, i, node_count, device_count;
> - int max_count = 16;
>
> if (drm_device_validate_flags(flags))
> return -EINVAL;
>
> - local_devices = calloc(max_count, sizeof(drmDevicePtr));
> - if (local_devices == NULL)
> - return -ENOMEM;
> -
> sysdir = opendir(DRM_DIR_NAME);
> - if (!sysdir) {
> - ret = -errno;
> - goto free_locals;
> - }
> + if (!sysdir)
> + return -errno;
>
> i = 0;
> while ((dent = readdir(sysdir))) {
> @@ -3994,16 +3974,12 @@ int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
> if (ret)
> continue;
>
> - if (i >= max_count) {
> - drmDevicePtr *temp;
> -
> - max_count += 16;
> - temp = realloc(local_devices, max_count * sizeof(drmDevicePtr));
> - if (!temp)
> - goto free_devices;
> - local_devices = temp;
> + if (i >= MAX_DRM_NODES) {
> + fprintf(stderr, "More than %d drm nodes detected. "
> + "Please report a bug - that should not happen.\n"
> + "Skipping extra nodes\n", MAX_DRM_NODES);
> + break;
> }
> -
> local_devices[i] = device;
> i++;
> }
> @@ -4025,16 +4001,7 @@ int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_devices)
> }
>
> closedir(sysdir);
> - free(local_devices);
> return device_count;
> -
> -free_devices:
> - drmFreeDevices(local_devices, i);
> - closedir(sysdir);
> -
> -free_locals:
> - free(local_devices);
> - return ret;
> }
>
> /**
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH libdrm v2 05/10] xf86drm: introduce a get_real_pci_path() helper
2018-06-25 17:36 ` [PATCH libdrm 04/10] xf86drm: Allocate drmDevicePtr's on stack Emil Velikov
2018-06-28 12:52 ` Robert Foss
2018-06-29 15:20 ` [PATCH libdrm v2 " Emil Velikov
@ 2018-06-29 15:22 ` Emil Velikov
2 siblings, 0 replies; 31+ messages in thread
From: Emil Velikov @ 2018-06-29 15:22 UTC (permalink / raw)
To: dri-devel; +Cc: Eric Engestrom, emil.l.velikov
From: Emil Velikov <emil.velikov@collabora.com>
Introduce a helper which gets the real sysfs path for the given pci
device.
In other words, instead opening the /sys/dev/char/*/device symlink, we
opt for the actual /sys/devices/pci*/*/
It folds three (nearly identical) snprintf's and paves the way of adding
extra devices (see next patch) a piece of pie.
v2: use a caller (on stack) provided real_path (Eric)
Cc: Eric Engestrom <eric@engestrom.ch>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Tested-by: Robert Foss <robert.foss@collabora.com> (v1)
Reviewed-by: Robert Foss <robert.foss@collabora.com> (v1)
Reviewed-by: Eric Engestrom <eric@engestrom.ch> (v1)
---
Eric, I couldn't quite remove all the error handling since on realpath()
failure the output buffer contents are undefined :-\
xf86drm.c | 34 ++++++++++++++++++++++++++--------
1 file changed, 26 insertions(+), 8 deletions(-)
diff --git a/xf86drm.c b/xf86drm.c
index 02da3e1f..51e00d23 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -2992,16 +2992,29 @@ static int drmParseSubsystemType(int maj, int min)
#endif
}
+static char *
+get_real_pci_path(int maj, int min, char *real_path)
+{
+ char path[PATH_MAX + 1];
+
+ snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
+ if (!realpath(path, real_path))
+ return NULL;
+
+ return real_path;
+}
+
static int drmParsePciBusInfo(int maj, int min, drmPciBusInfoPtr info)
{
#ifdef __linux__
unsigned int domain, bus, dev, func;
- char path[PATH_MAX + 1], *value;
+ char real_path[PATH_MAX + 1], *value;
int num;
- snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
+ if (get_real_pci_path(maj, min, real_path) == NULL)
+ return -ENOENT;
- value = sysfs_uevent_get(path, "PCI_SLOT_NAME");
+ value = sysfs_uevent_get(real_path, "PCI_SLOT_NAME");
if (!value)
return -ENOENT;
@@ -3114,14 +3127,16 @@ static int parse_separate_sysfs_files(int maj, int min,
"subsystem_vendor",
"subsystem_device",
};
- char path[PATH_MAX + 1];
+ char path[PATH_MAX + 1], real_path[PATH_MAX + 1];
unsigned int data[ARRAY_SIZE(attrs)];
FILE *fp;
int ret;
+ if (get_real_pci_path(maj, min, real_path) == NULL)
+ return -ENOENT;
+
for (unsigned i = ignore_revision ? 1 : 0; i < ARRAY_SIZE(attrs); i++) {
- snprintf(path, PATH_MAX, "/sys/dev/char/%d:%d/device/%s", maj, min,
- attrs[i]);
+ snprintf(path, PATH_MAX, "%s/%s", real_path, attrs[i]);
fp = fopen(path, "r");
if (!fp)
return -errno;
@@ -3145,11 +3160,14 @@ static int parse_separate_sysfs_files(int maj, int min,
static int parse_config_sysfs_file(int maj, int min,
drmPciDeviceInfoPtr device)
{
- char path[PATH_MAX + 1];
+ char path[PATH_MAX + 1], real_path[PATH_MAX + 1];
unsigned char config[64];
int fd, ret;
- snprintf(path, PATH_MAX, "/sys/dev/char/%d:%d/device/config", maj, min);
+ if (get_real_pci_path(maj, min, real_path) == NULL)
+ return -ENOENT;
+
+ snprintf(path, PATH_MAX, "%s/config", real_path);
fd = open(path, O_RDONLY);
if (fd < 0)
return -errno;
--
2.18.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH libdrm 05/10] xf86drm: introduce a get_real_pci_path() helper
2018-06-25 17:36 [PATCH libdrm 01/10] xf86drm: drmGetDevice2: error out if the fd has unknown subsys Emil Velikov
` (2 preceding siblings ...)
2018-06-25 17:36 ` [PATCH libdrm 04/10] xf86drm: Allocate drmDevicePtr's on stack Emil Velikov
@ 2018-06-25 17:36 ` Emil Velikov
2018-06-28 10:21 ` Eric Engestrom
2018-06-28 16:06 ` Robert Foss
2018-06-25 17:36 ` [PATCH libdrm 06/10] xf86drm: Add drmDevice support for virtio_gpu Emil Velikov
` (6 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Emil Velikov @ 2018-06-25 17:36 UTC (permalink / raw)
To: dri-devel; +Cc: emil.l.velikov
From: Emil Velikov <emil.velikov@collabora.com>
Introduce a helper which gets the real sysfs path for the given pci
device.
In other words, instead opening the /sys/dev/char/*/device symlink, we
opt for the actual /sys/devices/pci*/*/
It folds three (nearly identical) snprintf's and paves the way of adding
extra devices (see next patch) a piece of pie.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
xf86drm.c | 62 ++++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 50 insertions(+), 12 deletions(-)
diff --git a/xf86drm.c b/xf86drm.c
index d4810740..8ccd528f 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -2992,16 +2992,37 @@ static int drmParseSubsystemType(int maj, int min)
#endif
}
+static char *
+get_real_pci_path(int maj, int min)
+{
+ char path[PATH_MAX + 1];
+ char *real_path = malloc(PATH_MAX);
+
+ if (!real_path)
+ return NULL;
+
+ snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
+ if (!realpath(path, real_path)) {
+ free(real_path);
+ return NULL;
+ }
+
+ return real_path;
+}
+
static int drmParsePciBusInfo(int maj, int min, drmPciBusInfoPtr info)
{
#ifdef __linux__
unsigned int domain, bus, dev, func;
- char path[PATH_MAX + 1], *value;
+ char *real_path, *value;
int num;
- snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
+ real_path = get_real_pci_path(maj, min);
+ if (!real_path)
+ return -ENOENT;
- value = sysfs_uevent_get(path, "PCI_SLOT_NAME");
+ value = sysfs_uevent_get(real_path, "PCI_SLOT_NAME");
+ free(real_path);
if (!value)
return -ENOENT;
@@ -3114,24 +3135,32 @@ static int parse_separate_sysfs_files(int maj, int min,
"subsystem_vendor",
"subsystem_device",
};
- char path[PATH_MAX + 1];
+ char path[PATH_MAX + 1], *real_path;
unsigned int data[ARRAY_SIZE(attrs)];
FILE *fp;
int ret;
+ real_path = get_real_pci_path(maj, min);
+ if (!real_path)
+ return -ENOENT;
+
for (unsigned i = ignore_revision ? 1 : 0; i < ARRAY_SIZE(attrs); i++) {
- snprintf(path, PATH_MAX, "/sys/dev/char/%d:%d/device/%s", maj, min,
- attrs[i]);
+ snprintf(path, PATH_MAX, "%s/%s", real_path, attrs[i]);
fp = fopen(path, "r");
- if (!fp)
+ if (!fp) {
+ free(real_path);
return -errno;
+ }
ret = fscanf(fp, "%x", &data[i]);
fclose(fp);
- if (ret != 1)
+ if (ret != 1) {
+ free(real_path);
return -errno;
+ }
}
+ free(real_path);
device->revision_id = ignore_revision ? 0xff : data[0] & 0xff;
device->vendor_id = data[1] & 0xffff;
@@ -3145,19 +3174,28 @@ static int parse_separate_sysfs_files(int maj, int min,
static int parse_config_sysfs_file(int maj, int min,
drmPciDeviceInfoPtr device)
{
- char path[PATH_MAX + 1];
+ char path[PATH_MAX + 1], *real_path;
unsigned char config[64];
int fd, ret;
- snprintf(path, PATH_MAX, "/sys/dev/char/%d:%d/device/config", maj, min);
+ real_path = get_real_pci_path(maj, min);
+ if (!real_path)
+ return -ENOENT;
+
+ snprintf(path, PATH_MAX, "%s/config", real_path);
fd = open(path, O_RDONLY);
- if (fd < 0)
+ if (fd < 0) {
+ free(real_path);
return -errno;
+ }
ret = read(fd, config, sizeof(config));
close(fd);
- if (ret < 0)
+ if (ret < 0) {
+ free(real_path);
return -errno;
+ }
+ free(real_path);
device->vendor_id = config[0] | (config[1] << 8);
device->device_id = config[2] | (config[3] << 8);
--
2.18.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH libdrm 05/10] xf86drm: introduce a get_real_pci_path() helper
2018-06-25 17:36 ` [PATCH libdrm " Emil Velikov
@ 2018-06-28 10:21 ` Eric Engestrom
2018-06-28 10:23 ` Eric Engestrom
2018-06-28 16:42 ` Emil Velikov
2018-06-28 16:06 ` Robert Foss
1 sibling, 2 replies; 31+ messages in thread
From: Eric Engestrom @ 2018-06-28 10:21 UTC (permalink / raw)
To: Emil Velikov; +Cc: dri-devel
On Monday, 2018-06-25 17:39:14 +0000, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> Introduce a helper which gets the real sysfs path for the given pci
> device.
>
> In other words, instead opening the /sys/dev/char/*/device symlink, we
> opt for the actual /sys/devices/pci*/*/
>
> It folds three (nearly identical) snprintf's and paves the way of adding
> extra devices (see next patch) a piece of pie.
>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> xf86drm.c | 62 ++++++++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 50 insertions(+), 12 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index d4810740..8ccd528f 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2992,16 +2992,37 @@ static int drmParseSubsystemType(int maj, int min)
> #endif
> }
>
> +static char *
> +get_real_pci_path(int maj, int min)
> +{
> + char path[PATH_MAX + 1];
> + char *real_path = malloc(PATH_MAX);
Why not allocate this on the stack and pass it in?
It would avoid the error handling you had to add in this patch :)
> +
> + if (!real_path)
> + return NULL;
> +
> + snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
> + if (!realpath(path, real_path)) {
> + free(real_path);
> + return NULL;
> + }
> +
> + return real_path;
> +}
> +
> static int drmParsePciBusInfo(int maj, int min, drmPciBusInfoPtr info)
> {
> #ifdef __linux__
> unsigned int domain, bus, dev, func;
> - char path[PATH_MAX + 1], *value;
> + char *real_path, *value;
> int num;
>
> - snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
> + real_path = get_real_pci_path(maj, min);
> + if (!real_path)
> + return -ENOENT;
>
> - value = sysfs_uevent_get(path, "PCI_SLOT_NAME");
> + value = sysfs_uevent_get(real_path, "PCI_SLOT_NAME");
> + free(real_path);
> if (!value)
> return -ENOENT;
>
> @@ -3114,24 +3135,32 @@ static int parse_separate_sysfs_files(int maj, int min,
> "subsystem_vendor",
> "subsystem_device",
> };
> - char path[PATH_MAX + 1];
> + char path[PATH_MAX + 1], *real_path;
> unsigned int data[ARRAY_SIZE(attrs)];
> FILE *fp;
> int ret;
>
> + real_path = get_real_pci_path(maj, min);
> + if (!real_path)
> + return -ENOENT;
> +
> for (unsigned i = ignore_revision ? 1 : 0; i < ARRAY_SIZE(attrs); i++) {
> - snprintf(path, PATH_MAX, "/sys/dev/char/%d:%d/device/%s", maj, min,
> - attrs[i]);
> + snprintf(path, PATH_MAX, "%s/%s", real_path, attrs[i]);
> fp = fopen(path, "r");
> - if (!fp)
> + if (!fp) {
> + free(real_path);
> return -errno;
> + }
>
> ret = fscanf(fp, "%x", &data[i]);
> fclose(fp);
> - if (ret != 1)
> + if (ret != 1) {
> + free(real_path);
> return -errno;
> + }
>
> }
> + free(real_path);
>
> device->revision_id = ignore_revision ? 0xff : data[0] & 0xff;
> device->vendor_id = data[1] & 0xffff;
> @@ -3145,19 +3174,28 @@ static int parse_separate_sysfs_files(int maj, int min,
> static int parse_config_sysfs_file(int maj, int min,
> drmPciDeviceInfoPtr device)
> {
> - char path[PATH_MAX + 1];
> + char path[PATH_MAX + 1], *real_path;
> unsigned char config[64];
> int fd, ret;
>
> - snprintf(path, PATH_MAX, "/sys/dev/char/%d:%d/device/config", maj, min);
> + real_path = get_real_pci_path(maj, min);
> + if (!real_path)
> + return -ENOENT;
> +
> + snprintf(path, PATH_MAX, "%s/config", real_path);
> fd = open(path, O_RDONLY);
> - if (fd < 0)
> + if (fd < 0) {
> + free(real_path);
> return -errno;
> + }
>
> ret = read(fd, config, sizeof(config));
> close(fd);
> - if (ret < 0)
> + if (ret < 0) {
> + free(real_path);
> return -errno;
> + }
> + free(real_path);
>
> device->vendor_id = config[0] | (config[1] << 8);
> device->device_id = config[2] | (config[3] << 8);
> --
> 2.18.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH libdrm 05/10] xf86drm: introduce a get_real_pci_path() helper
2018-06-28 10:21 ` Eric Engestrom
@ 2018-06-28 10:23 ` Eric Engestrom
2018-06-28 16:42 ` Emil Velikov
1 sibling, 0 replies; 31+ messages in thread
From: Eric Engestrom @ 2018-06-28 10:23 UTC (permalink / raw)
To: Emil Velikov; +Cc: dri-devel
On Thursday, 2018-06-28 11:21:26 +0100, Eric Engestrom wrote:
> On Monday, 2018-06-25 17:39:14 +0000, Emil Velikov wrote:
> > From: Emil Velikov <emil.velikov@collabora.com>
> >
> > Introduce a helper which gets the real sysfs path for the given pci
> > device.
> >
> > In other words, instead opening the /sys/dev/char/*/device symlink, we
> > opt for the actual /sys/devices/pci*/*/
> >
> > It folds three (nearly identical) snprintf's and paves the way of adding
> > extra devices (see next patch) a piece of pie.
> >
> > Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> > ---
> > xf86drm.c | 62 ++++++++++++++++++++++++++++++++++++++++++++-----------
> > 1 file changed, 50 insertions(+), 12 deletions(-)
> >
> > diff --git a/xf86drm.c b/xf86drm.c
> > index d4810740..8ccd528f 100644
> > --- a/xf86drm.c
> > +++ b/xf86drm.c
> > @@ -2992,16 +2992,37 @@ static int drmParseSubsystemType(int maj, int min)
> > #endif
> > }
> >
> > +static char *
> > +get_real_pci_path(int maj, int min)
> > +{
> > + char path[PATH_MAX + 1];
> > + char *real_path = malloc(PATH_MAX);
>
> Why not allocate this on the stack and pass it in?
> It would avoid the error handling you had to add in this patch :)
Sorry, I forgot to add: with that amend, the series is
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
>
> > +
> > + if (!real_path)
> > + return NULL;
> > +
> > + snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
> > + if (!realpath(path, real_path)) {
> > + free(real_path);
> > + return NULL;
> > + }
> > +
> > + return real_path;
> > +}
> > +
> > static int drmParsePciBusInfo(int maj, int min, drmPciBusInfoPtr info)
> > {
> > #ifdef __linux__
> > unsigned int domain, bus, dev, func;
> > - char path[PATH_MAX + 1], *value;
> > + char *real_path, *value;
> > int num;
> >
> > - snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
> > + real_path = get_real_pci_path(maj, min);
> > + if (!real_path)
> > + return -ENOENT;
> >
> > - value = sysfs_uevent_get(path, "PCI_SLOT_NAME");
> > + value = sysfs_uevent_get(real_path, "PCI_SLOT_NAME");
> > + free(real_path);
> > if (!value)
> > return -ENOENT;
> >
> > @@ -3114,24 +3135,32 @@ static int parse_separate_sysfs_files(int maj, int min,
> > "subsystem_vendor",
> > "subsystem_device",
> > };
> > - char path[PATH_MAX + 1];
> > + char path[PATH_MAX + 1], *real_path;
> > unsigned int data[ARRAY_SIZE(attrs)];
> > FILE *fp;
> > int ret;
> >
> > + real_path = get_real_pci_path(maj, min);
> > + if (!real_path)
> > + return -ENOENT;
> > +
> > for (unsigned i = ignore_revision ? 1 : 0; i < ARRAY_SIZE(attrs); i++) {
> > - snprintf(path, PATH_MAX, "/sys/dev/char/%d:%d/device/%s", maj, min,
> > - attrs[i]);
> > + snprintf(path, PATH_MAX, "%s/%s", real_path, attrs[i]);
> > fp = fopen(path, "r");
> > - if (!fp)
> > + if (!fp) {
> > + free(real_path);
> > return -errno;
> > + }
> >
> > ret = fscanf(fp, "%x", &data[i]);
> > fclose(fp);
> > - if (ret != 1)
> > + if (ret != 1) {
> > + free(real_path);
> > return -errno;
> > + }
> >
> > }
> > + free(real_path);
> >
> > device->revision_id = ignore_revision ? 0xff : data[0] & 0xff;
> > device->vendor_id = data[1] & 0xffff;
> > @@ -3145,19 +3174,28 @@ static int parse_separate_sysfs_files(int maj, int min,
> > static int parse_config_sysfs_file(int maj, int min,
> > drmPciDeviceInfoPtr device)
> > {
> > - char path[PATH_MAX + 1];
> > + char path[PATH_MAX + 1], *real_path;
> > unsigned char config[64];
> > int fd, ret;
> >
> > - snprintf(path, PATH_MAX, "/sys/dev/char/%d:%d/device/config", maj, min);
> > + real_path = get_real_pci_path(maj, min);
> > + if (!real_path)
> > + return -ENOENT;
> > +
> > + snprintf(path, PATH_MAX, "%s/config", real_path);
> > fd = open(path, O_RDONLY);
> > - if (fd < 0)
> > + if (fd < 0) {
> > + free(real_path);
> > return -errno;
> > + }
> >
> > ret = read(fd, config, sizeof(config));
> > close(fd);
> > - if (ret < 0)
> > + if (ret < 0) {
> > + free(real_path);
> > return -errno;
> > + }
> > + free(real_path);
> >
> > device->vendor_id = config[0] | (config[1] << 8);
> > device->device_id = config[2] | (config[3] << 8);
> > --
> > 2.18.0
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH libdrm 05/10] xf86drm: introduce a get_real_pci_path() helper
2018-06-28 10:21 ` Eric Engestrom
2018-06-28 10:23 ` Eric Engestrom
@ 2018-06-28 16:42 ` Emil Velikov
1 sibling, 0 replies; 31+ messages in thread
From: Emil Velikov @ 2018-06-28 16:42 UTC (permalink / raw)
To: Eric Engestrom; +Cc: ML dri-devel
Hi Eric,
On 28 June 2018 at 11:21, Eric Engestrom <eric@engestrom.ch> wrote:
>> @@ -2992,16 +2992,37 @@ static int drmParseSubsystemType(int maj, int min)
>> #endif
>> }
>>
>> +static char *
>> +get_real_pci_path(int maj, int min)
>> +{
>> + char path[PATH_MAX + 1];
>> + char *real_path = malloc(PATH_MAX);
>
> Why not allocate this on the stack and pass it in?
> It would avoid the error handling you had to add in this patch :)
>
As always, thanks for having a look.
I was mostly hesitant since some callers already have a char foo[PATH_MAX +1].
Thinking about it - the default stack size should be enough to
accommodate an extra ~4k bytes.
Will fix.
Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH libdrm 05/10] xf86drm: introduce a get_real_pci_path() helper
2018-06-25 17:36 ` [PATCH libdrm " Emil Velikov
2018-06-28 10:21 ` Eric Engestrom
@ 2018-06-28 16:06 ` Robert Foss
1 sibling, 0 replies; 31+ messages in thread
From: Robert Foss @ 2018-06-28 16:06 UTC (permalink / raw)
To: Emil Velikov, dri-devel
Feel free to add my r-b to this patch.
On 2018-06-25 19:36, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> Introduce a helper which gets the real sysfs path for the given pci
> device.
>
> In other words, instead opening the /sys/dev/char/*/device symlink, we
> opt for the actual /sys/devices/pci*/*/
>
> It folds three (nearly identical) snprintf's and paves the way of adding
> extra devices (see next patch) a piece of pie.
>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> xf86drm.c | 62 ++++++++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 50 insertions(+), 12 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index d4810740..8ccd528f 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2992,16 +2992,37 @@ static int drmParseSubsystemType(int maj, int min)
> #endif
> }
>
> +static char *
> +get_real_pci_path(int maj, int min)
> +{
> + char path[PATH_MAX + 1];
> + char *real_path = malloc(PATH_MAX);
> +
> + if (!real_path)
> + return NULL;
> +
> + snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
> + if (!realpath(path, real_path)) {
> + free(real_path);
> + return NULL;
> + }
> +
> + return real_path;
> +}
> +
> static int drmParsePciBusInfo(int maj, int min, drmPciBusInfoPtr info)
> {
> #ifdef __linux__
> unsigned int domain, bus, dev, func;
> - char path[PATH_MAX + 1], *value;
> + char *real_path, *value;
> int num;
>
> - snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
> + real_path = get_real_pci_path(maj, min);
> + if (!real_path)
> + return -ENOENT;
>
> - value = sysfs_uevent_get(path, "PCI_SLOT_NAME");
> + value = sysfs_uevent_get(real_path, "PCI_SLOT_NAME");
> + free(real_path);
> if (!value)
> return -ENOENT;
>
> @@ -3114,24 +3135,32 @@ static int parse_separate_sysfs_files(int maj, int min,
> "subsystem_vendor",
> "subsystem_device",
> };
> - char path[PATH_MAX + 1];
> + char path[PATH_MAX + 1], *real_path;
> unsigned int data[ARRAY_SIZE(attrs)];
> FILE *fp;
> int ret;
>
> + real_path = get_real_pci_path(maj, min);
> + if (!real_path)
> + return -ENOENT;
> +
> for (unsigned i = ignore_revision ? 1 : 0; i < ARRAY_SIZE(attrs); i++) {
> - snprintf(path, PATH_MAX, "/sys/dev/char/%d:%d/device/%s", maj, min,
> - attrs[i]);
> + snprintf(path, PATH_MAX, "%s/%s", real_path, attrs[i]);
> fp = fopen(path, "r");
> - if (!fp)
> + if (!fp) {
> + free(real_path);
> return -errno;
> + }
>
> ret = fscanf(fp, "%x", &data[i]);
> fclose(fp);
> - if (ret != 1)
> + if (ret != 1) {
> + free(real_path);
> return -errno;
> + }
>
> }
> + free(real_path);
>
> device->revision_id = ignore_revision ? 0xff : data[0] & 0xff;
> device->vendor_id = data[1] & 0xffff;
> @@ -3145,19 +3174,28 @@ static int parse_separate_sysfs_files(int maj, int min,
> static int parse_config_sysfs_file(int maj, int min,
> drmPciDeviceInfoPtr device)
> {
> - char path[PATH_MAX + 1];
> + char path[PATH_MAX + 1], *real_path;
> unsigned char config[64];
> int fd, ret;
>
> - snprintf(path, PATH_MAX, "/sys/dev/char/%d:%d/device/config", maj, min);
> + real_path = get_real_pci_path(maj, min);
> + if (!real_path)
> + return -ENOENT;
> +
> + snprintf(path, PATH_MAX, "%s/config", real_path);
> fd = open(path, O_RDONLY);
> - if (fd < 0)
> + if (fd < 0) {
> + free(real_path);
> return -errno;
> + }
>
> ret = read(fd, config, sizeof(config));
> close(fd);
> - if (ret < 0)
> + if (ret < 0) {
> + free(real_path);
> return -errno;
> + }
> + free(real_path);
>
> device->vendor_id = config[0] | (config[1] << 8);
> device->device_id = config[2] | (config[3] << 8);
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH libdrm 06/10] xf86drm: Add drmDevice support for virtio_gpu
2018-06-25 17:36 [PATCH libdrm 01/10] xf86drm: drmGetDevice2: error out if the fd has unknown subsys Emil Velikov
` (3 preceding siblings ...)
2018-06-25 17:36 ` [PATCH libdrm " Emil Velikov
@ 2018-06-25 17:36 ` Emil Velikov
2018-06-28 16:08 ` Robert Foss
2018-06-25 17:36 ` [PATCH libdrm 07/10] tests/drmdevices: install alongside other utilities Emil Velikov
` (5 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Emil Velikov @ 2018-06-25 17:36 UTC (permalink / raw)
To: dri-devel; +Cc: emil.l.velikov
From: Emil Velikov <emil.velikov@collabora.com>
The GPU almost exclusively lives on the PCI bus, so we expose it as a
normal PCI one.
This allows any existing drmDevice users to work without any changes.
One could wonder why a separate typeset is not introduced, alike say
host1x. Unlike host1x the PCI/platform distinction for virtio provides
no extra information. Plus needed we can add the separate set at a later
stage.
Here are a couple of 'features' that virtio seems to be missing:
- provides extra information on top the plaform devices
- supports a range of GPU devices
- is considered hardware description (DT)
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
xf86drm.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/xf86drm.c b/xf86drm.c
index 8ccd528f..b847ea26 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -2954,6 +2954,9 @@ sysfs_uevent_get(const char *path, const char *fmt, ...)
}
#endif
+/* Little white lie to avoid major rework of the existing code */
+#define DRM_BUS_VIRTIO 0x10
+
static int drmParseSubsystemType(int maj, int min)
{
#ifdef __linux__
@@ -2983,6 +2986,9 @@ static int drmParseSubsystemType(int maj, int min)
if (strncmp(name, "/host1x", 7) == 0)
return DRM_BUS_HOST1X;
+ if (strncmp(name, "/virtio", 7) == 0)
+ return DRM_BUS_VIRTIO;
+
return -EINVAL;
#elif defined(__OpenBSD__)
return DRM_BUS_PCI;
@@ -2996,7 +3002,7 @@ static char *
get_real_pci_path(int maj, int min)
{
char path[PATH_MAX + 1];
- char *real_path = malloc(PATH_MAX);
+ char *term, *real_path = malloc(PATH_MAX);
if (!real_path)
return NULL;
@@ -3007,6 +3013,10 @@ get_real_pci_path(int maj, int min)
return NULL;
}
+ term = strrchr(real_path, '/');
+ if (term && strncmp(term, "/virtio", 7) == 0)
+ *term = 0;
+
return real_path;
}
@@ -3744,6 +3754,7 @@ process_device(drmDevicePtr *device, const char *d_name,
switch (subsystem_type) {
case DRM_BUS_PCI:
+ case DRM_BUS_VIRTIO:
return drmProcessPciDevice(device, node, node_type, maj, min,
fetch_deviceinfo, flags);
case DRM_BUS_USB:
--
2.18.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH libdrm 06/10] xf86drm: Add drmDevice support for virtio_gpu
2018-06-25 17:36 ` [PATCH libdrm 06/10] xf86drm: Add drmDevice support for virtio_gpu Emil Velikov
@ 2018-06-28 16:08 ` Robert Foss
0 siblings, 0 replies; 31+ messages in thread
From: Robert Foss @ 2018-06-28 16:08 UTC (permalink / raw)
To: Emil Velikov, dri-devel
Feel free to add my r-b to this patch.
On 2018-06-25 19:36, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> The GPU almost exclusively lives on the PCI bus, so we expose it as a
> normal PCI one.
>
> This allows any existing drmDevice users to work without any changes.
>
> One could wonder why a separate typeset is not introduced, alike say
> host1x. Unlike host1x the PCI/platform distinction for virtio provides
> no extra information. Plus needed we can add the separate set at a later
> stage.
>
> Here are a couple of 'features' that virtio seems to be missing:
> - provides extra information on top the plaform devices
> - supports a range of GPU devices
> - is considered hardware description (DT)
>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> xf86drm.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 8ccd528f..b847ea26 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2954,6 +2954,9 @@ sysfs_uevent_get(const char *path, const char *fmt, ...)
> }
> #endif
>
> +/* Little white lie to avoid major rework of the existing code */
> +#define DRM_BUS_VIRTIO 0x10
> +
> static int drmParseSubsystemType(int maj, int min)
> {
> #ifdef __linux__
> @@ -2983,6 +2986,9 @@ static int drmParseSubsystemType(int maj, int min)
> if (strncmp(name, "/host1x", 7) == 0)
> return DRM_BUS_HOST1X;
>
> + if (strncmp(name, "/virtio", 7) == 0)
> + return DRM_BUS_VIRTIO;
> +
> return -EINVAL;
> #elif defined(__OpenBSD__)
> return DRM_BUS_PCI;
> @@ -2996,7 +3002,7 @@ static char *
> get_real_pci_path(int maj, int min)
> {
> char path[PATH_MAX + 1];
> - char *real_path = malloc(PATH_MAX);
> + char *term, *real_path = malloc(PATH_MAX);
>
> if (!real_path)
> return NULL;
> @@ -3007,6 +3013,10 @@ get_real_pci_path(int maj, int min)
> return NULL;
> }
>
> + term = strrchr(real_path, '/');
> + if (term && strncmp(term, "/virtio", 7) == 0)
> + *term = 0;
> +
> return real_path;
> }
>
> @@ -3744,6 +3754,7 @@ process_device(drmDevicePtr *device, const char *d_name,
>
> switch (subsystem_type) {
> case DRM_BUS_PCI:
> + case DRM_BUS_VIRTIO:
> return drmProcessPciDevice(device, node, node_type, maj, min,
> fetch_deviceinfo, flags);
> case DRM_BUS_USB:
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH libdrm 07/10] tests/drmdevices: install alongside other utilities
2018-06-25 17:36 [PATCH libdrm 01/10] xf86drm: drmGetDevice2: error out if the fd has unknown subsys Emil Velikov
` (4 preceding siblings ...)
2018-06-25 17:36 ` [PATCH libdrm 06/10] xf86drm: Add drmDevice support for virtio_gpu Emil Velikov
@ 2018-06-25 17:36 ` Emil Velikov
2018-06-28 16:09 ` Robert Foss
2018-06-25 17:36 ` [PATCH libdrm 08/10] tests/drmdevice: add a couple of printf headers Emil Velikov
` (4 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Emil Velikov @ 2018-06-25 17:36 UTC (permalink / raw)
To: dri-devel; +Cc: emil.l.velikov
From: Emil Velikov <emil.velikov@collabora.com>
It's mildly useful program, to ship it when the user wants the "tests"
installed. Obviously the "tests" in the name is a misnomer.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
tests/Makefile.am | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 0355a925..b72c24f9 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -43,5 +43,10 @@ TESTS = \
random
check_PROGRAMS = \
- $(TESTS) \
- drmdevice
+ $(TESTS)
+
+if HAVE_INSTALL_TESTS
+bin_PROGRAMS = drmdevice
+else
+check_PROGRAMS += drmdevice
+endif
--
2.18.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH libdrm 07/10] tests/drmdevices: install alongside other utilities
2018-06-25 17:36 ` [PATCH libdrm 07/10] tests/drmdevices: install alongside other utilities Emil Velikov
@ 2018-06-28 16:09 ` Robert Foss
0 siblings, 0 replies; 31+ messages in thread
From: Robert Foss @ 2018-06-28 16:09 UTC (permalink / raw)
To: Emil Velikov, dri-devel
Feel free to add my r-b to this patch.
On 2018-06-25 19:36, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> It's mildly useful program, to ship it when the user wants the "tests"
> installed. Obviously the "tests" in the name is a misnomer.
>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> tests/Makefile.am | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 0355a925..b72c24f9 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -43,5 +43,10 @@ TESTS = \
> random
>
> check_PROGRAMS = \
> - $(TESTS) \
> - drmdevice
> + $(TESTS)
> +
> +if HAVE_INSTALL_TESTS
> +bin_PROGRAMS = drmdevice
> +else
> +check_PROGRAMS += drmdevice
> +endif
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH libdrm 08/10] tests/drmdevice: add a couple of printf headers
2018-06-25 17:36 [PATCH libdrm 01/10] xf86drm: drmGetDevice2: error out if the fd has unknown subsys Emil Velikov
` (5 preceding siblings ...)
2018-06-25 17:36 ` [PATCH libdrm 07/10] tests/drmdevices: install alongside other utilities Emil Velikov
@ 2018-06-25 17:36 ` Emil Velikov
2018-06-28 16:09 ` Robert Foss
2018-06-25 17:36 ` [PATCH libdrm 09/10] drmdevice: convert the tabbed output into a tree Emil Velikov
` (3 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Emil Velikov @ 2018-06-25 17:36 UTC (permalink / raw)
To: dri-devel; +Cc: emil.l.velikov
From: Emil Velikov <emil.velikov@collabora.com>
Add a few printf statements, which should make the output easier to
parse.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
tests/drmdevice.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tests/drmdevice.c b/tests/drmdevice.c
index 9dd5098a..0d75836f 100644
--- a/tests/drmdevice.c
+++ b/tests/drmdevice.c
@@ -112,12 +112,15 @@ main(void)
drmDevicePtr device;
int fd, ret, max_devices;
+ printf("--- Checking the number of DRM device available ---\n");
max_devices = drmGetDevices2(0, NULL, 0);
if (max_devices <= 0) {
printf("drmGetDevices2() has returned %d\n", max_devices);
return -1;
}
+ printf("--- Devices reported %d ---\n", max_devices);
+
devices = calloc(max_devices, sizeof(drmDevicePtr));
if (devices == NULL) {
@@ -125,6 +128,7 @@ main(void)
return -1;
}
+ printf("--- Retrieving devices information (PCI device revision is ignored) ---\n");
ret = drmGetDevices2(0, devices, max_devices);
if (ret < 0) {
printf("drmGetDevices2() returned an error %d\n", ret);
@@ -137,13 +141,14 @@ main(void)
for (int j = 0; j < DRM_NODE_MAX; j++) {
if (devices[i]->available_nodes & 1 << j) {
- printf("Opening device %d node %s\n", i, devices[i]->nodes[j]);
+ printf("--- Opening device node %s ---\n", devices[i]->nodes[j]);
fd = open(devices[i]->nodes[j], O_RDONLY | O_CLOEXEC, 0);
if (fd < 0) {
printf("Failed - %s (%d)\n", strerror(errno), errno);
continue;
}
+ printf("--- Retrieving device info, for node %s ---\n", devices[i]->nodes[j]);
if (drmGetDevice2(fd, DRM_DEVICE_GET_PCI_REVISION, &device) == 0) {
print_device_info(device, i, true);
drmFreeDevice(&device);
--
2.18.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH libdrm 08/10] tests/drmdevice: add a couple of printf headers
2018-06-25 17:36 ` [PATCH libdrm 08/10] tests/drmdevice: add a couple of printf headers Emil Velikov
@ 2018-06-28 16:09 ` Robert Foss
0 siblings, 0 replies; 31+ messages in thread
From: Robert Foss @ 2018-06-28 16:09 UTC (permalink / raw)
To: Emil Velikov, dri-devel
Feel free to add my r-b to this patch.
On 2018-06-25 19:36, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> Add a few printf statements, which should make the output easier to
> parse.
>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> tests/drmdevice.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/tests/drmdevice.c b/tests/drmdevice.c
> index 9dd5098a..0d75836f 100644
> --- a/tests/drmdevice.c
> +++ b/tests/drmdevice.c
> @@ -112,12 +112,15 @@ main(void)
> drmDevicePtr device;
> int fd, ret, max_devices;
>
> + printf("--- Checking the number of DRM device available ---\n");
> max_devices = drmGetDevices2(0, NULL, 0);
>
> if (max_devices <= 0) {
> printf("drmGetDevices2() has returned %d\n", max_devices);
> return -1;
> }
> + printf("--- Devices reported %d ---\n", max_devices);
> +
>
> devices = calloc(max_devices, sizeof(drmDevicePtr));
> if (devices == NULL) {
> @@ -125,6 +128,7 @@ main(void)
> return -1;
> }
>
> + printf("--- Retrieving devices information (PCI device revision is ignored) ---\n");
> ret = drmGetDevices2(0, devices, max_devices);
> if (ret < 0) {
> printf("drmGetDevices2() returned an error %d\n", ret);
> @@ -137,13 +141,14 @@ main(void)
>
> for (int j = 0; j < DRM_NODE_MAX; j++) {
> if (devices[i]->available_nodes & 1 << j) {
> - printf("Opening device %d node %s\n", i, devices[i]->nodes[j]);
> + printf("--- Opening device node %s ---\n", devices[i]->nodes[j]);
> fd = open(devices[i]->nodes[j], O_RDONLY | O_CLOEXEC, 0);
> if (fd < 0) {
> printf("Failed - %s (%d)\n", strerror(errno), errno);
> continue;
> }
>
> + printf("--- Retrieving device info, for node %s ---\n", devices[i]->nodes[j]);
> if (drmGetDevice2(fd, DRM_DEVICE_GET_PCI_REVISION, &device) == 0) {
> print_device_info(device, i, true);
> drmFreeDevice(&device);
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH libdrm 09/10] drmdevice: convert the tabbed output into a tree
2018-06-25 17:36 [PATCH libdrm 01/10] xf86drm: drmGetDevice2: error out if the fd has unknown subsys Emil Velikov
` (6 preceding siblings ...)
2018-06-25 17:36 ` [PATCH libdrm 08/10] tests/drmdevice: add a couple of printf headers Emil Velikov
@ 2018-06-25 17:36 ` Emil Velikov
2018-06-28 10:19 ` Eric Engestrom
` (2 more replies)
2018-06-25 17:36 ` [PATCH libdrm 10/10] drmdevice: print the correct host1x information Emil Velikov
` (2 subsequent siblings)
10 siblings, 3 replies; 31+ messages in thread
From: Emil Velikov @ 2018-06-25 17:36 UTC (permalink / raw)
To: dri-devel; +Cc: emil.l.velikov
From: Emil Velikov <emil.velikov@collabora.com>
Making the output a little bit easier to parse by human beings.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
tests/drmdevice.c | 78 +++++++++++++++++++++++------------------------
1 file changed, 39 insertions(+), 39 deletions(-)
diff --git a/tests/drmdevice.c b/tests/drmdevice.c
index 0d75836f..e9e9d7f1 100644
--- a/tests/drmdevice.c
+++ b/tests/drmdevice.c
@@ -36,67 +36,67 @@ static void
print_device_info(drmDevicePtr device, int i, bool print_revision)
{
printf("device[%i]\n", i);
- printf("\tavailable_nodes %04x\n", device->available_nodes);
- printf("\tnodes\n");
+ printf("+->available_nodes %#04x\n", device->available_nodes);
+ printf("+->nodes\n");
for (int j = 0; j < DRM_NODE_MAX; j++)
if (device->available_nodes & 1 << j)
- printf("\t\tnodes[%d] %s\n", j, device->nodes[j]);
+ printf("| +->nodes[%d] %s\n", j, device->nodes[j]);
- printf("\tbustype %04x\n", device->bustype);
- printf("\tbusinfo\n");
+ printf("+->bustype %04x\n", device->bustype);
+ printf("+->businfo\n");
if (device->bustype == DRM_BUS_PCI) {
- printf("\t\tpci\n");
- printf("\t\t\tdomain\t%04x\n",device->businfo.pci->domain);
- printf("\t\t\tbus\t%02x\n", device->businfo.pci->bus);
- printf("\t\t\tdev\t%02x\n", device->businfo.pci->dev);
- printf("\t\t\tfunc\t%1u\n", device->businfo.pci->func);
-
- printf("\tdeviceinfo\n");
- printf("\t\tpci\n");
- printf("\t\t\tvendor_id\t%04x\n", device->deviceinfo.pci->vendor_id);
- printf("\t\t\tdevice_id\t%04x\n", device->deviceinfo.pci->device_id);
- printf("\t\t\tsubvendor_id\t%04x\n", device->deviceinfo.pci->subvendor_id);
- printf("\t\t\tsubdevice_id\t%04x\n", device->deviceinfo.pci->subdevice_id);
+ printf("| +->pci\n");
+ printf("| +->domain %04x\n",device->businfo.pci->domain);
+ printf("| +->bus %02x\n", device->businfo.pci->bus);
+ printf("| +->dev %02x\n", device->businfo.pci->dev);
+ printf("| +->func %1u\n", device->businfo.pci->func);
+
+ printf("+->deviceinfo\n");
+ printf(" +->pci\n");
+ printf(" +->vendor_id %04x\n", device->deviceinfo.pci->vendor_id);
+ printf(" +->device_id %04x\n", device->deviceinfo.pci->device_id);
+ printf(" +->subvendor_id %04x\n", device->deviceinfo.pci->subvendor_id);
+ printf(" +->subdevice_id %04x\n", device->deviceinfo.pci->subdevice_id);
if (print_revision)
- printf("\t\t\trevision_id\t%02x\n", device->deviceinfo.pci->revision_id);
+ printf(" +->revision_id %02x\n", device->deviceinfo.pci->revision_id);
else
- printf("\t\t\trevision_id\tIGNORED\n");
+ printf(" +->revision_id IGNORED\n");
} else if (device->bustype == DRM_BUS_USB) {
- printf("\t\tusb\n");
- printf("\t\t\tbus\t%03u\n", device->businfo.usb->bus);
- printf("\t\t\tdev\t%03u\n", device->businfo.usb->dev);
-
- printf("\tdeviceinfo\n");
- printf("\t\tusb\n");
- printf("\t\t\tvendor\t%04x\n", device->deviceinfo.usb->vendor);
- printf("\t\t\tproduct\t%04x\n", device->deviceinfo.usb->product);
+ printf("| +->usb\n");
+ printf("| +->bus %03u\n", device->businfo.usb->bus);
+ printf("| +->dev %03u\n", device->businfo.usb->dev);
+
+ printf("+->deviceinfo\n");
+ printf(" +->usb\n");
+ printf(" +->vendor %04x\n", device->deviceinfo.usb->vendor);
+ printf(" +->product %04x\n", device->deviceinfo.usb->product);
} else if (device->bustype == DRM_BUS_PLATFORM) {
char **compatible = device->deviceinfo.platform->compatible;
- printf("\t\tplatform\n");
- printf("\t\t\tfullname\t%s\n", device->businfo.platform->fullname);
+ printf("| +->platform\n");
+ printf("| +->fullname\t%s\n", device->businfo.platform->fullname);
- printf("\tdeviceinfo\n");
- printf("\t\tplatform\n");
- printf("\t\t\tcompatible\n");
+ printf("+->deviceinfo\n");
+ printf(" +->platform\n");
+ printf(" +->compatible\n");
while (*compatible) {
- printf("\t\t\t\t%s\n", *compatible);
+ printf(" %s\n", *compatible);
compatible++;
}
} else if (device->bustype == DRM_BUS_HOST1X) {
char **compatible = device->deviceinfo.platform->compatible;
- printf("\t\thost1x\n");
- printf("\t\t\tfullname\t%s\n", device->businfo.host1x->fullname);
+ printf("| +->host1x\n");
+ printf("| +->fullname\t%s\n", device->businfo.host1x->fullname);
- printf("\tdeviceinfo\n");
- printf("\t\tplatform\n");
- printf("\t\t\tcompatible\n");
+ printf("+->deviceinfo\n");
+ printf(" +->platform\n");
+ printf(" +->compatible\n");
while (*compatible) {
- printf("\t\t\t\t%s\n", *compatible);
+ printf(" %s\n", *compatible);
compatible++;
}
} else {
--
2.18.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH libdrm 09/10] drmdevice: convert the tabbed output into a tree
2018-06-25 17:36 ` [PATCH libdrm 09/10] drmdevice: convert the tabbed output into a tree Emil Velikov
@ 2018-06-28 10:19 ` Eric Engestrom
2018-06-28 16:43 ` Emil Velikov
2018-06-28 16:09 ` Robert Foss
2018-06-29 15:24 ` [PATCH libdrm v2 " Emil Velikov
2 siblings, 1 reply; 31+ messages in thread
From: Eric Engestrom @ 2018-06-28 10:19 UTC (permalink / raw)
To: Emil Velikov; +Cc: dri-devel
On Monday, 2018-06-25 17:40:02 +0000, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> Making the output a little bit easier to parse by human beings.
>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> tests/drmdevice.c | 78 +++++++++++++++++++++++------------------------
> 1 file changed, 39 insertions(+), 39 deletions(-)
>
> diff --git a/tests/drmdevice.c b/tests/drmdevice.c
> index 0d75836f..e9e9d7f1 100644
> --- a/tests/drmdevice.c
> +++ b/tests/drmdevice.c
> @@ -36,67 +36,67 @@ static void
> print_device_info(drmDevicePtr device, int i, bool print_revision)
> {
> printf("device[%i]\n", i);
> - printf("\tavailable_nodes %04x\n", device->available_nodes);
> - printf("\tnodes\n");
> + printf("+->available_nodes %#04x\n", device->available_nodes);
> + printf("+->nodes\n");
Nit: I'd put a space between `>` and the text, for readability
> for (int j = 0; j < DRM_NODE_MAX; j++)
> if (device->available_nodes & 1 << j)
> - printf("\t\tnodes[%d] %s\n", j, device->nodes[j]);
> + printf("| +->nodes[%d] %s\n", j, device->nodes[j]);
>
> - printf("\tbustype %04x\n", device->bustype);
> - printf("\tbusinfo\n");
> + printf("+->bustype %04x\n", device->bustype);
> + printf("+->businfo\n");
> if (device->bustype == DRM_BUS_PCI) {
> - printf("\t\tpci\n");
> - printf("\t\t\tdomain\t%04x\n",device->businfo.pci->domain);
> - printf("\t\t\tbus\t%02x\n", device->businfo.pci->bus);
> - printf("\t\t\tdev\t%02x\n", device->businfo.pci->dev);
> - printf("\t\t\tfunc\t%1u\n", device->businfo.pci->func);
> -
> - printf("\tdeviceinfo\n");
> - printf("\t\tpci\n");
> - printf("\t\t\tvendor_id\t%04x\n", device->deviceinfo.pci->vendor_id);
> - printf("\t\t\tdevice_id\t%04x\n", device->deviceinfo.pci->device_id);
> - printf("\t\t\tsubvendor_id\t%04x\n", device->deviceinfo.pci->subvendor_id);
> - printf("\t\t\tsubdevice_id\t%04x\n", device->deviceinfo.pci->subdevice_id);
> + printf("| +->pci\n");
> + printf("| +->domain %04x\n",device->businfo.pci->domain);
> + printf("| +->bus %02x\n", device->businfo.pci->bus);
> + printf("| +->dev %02x\n", device->businfo.pci->dev);
> + printf("| +->func %1u\n", device->businfo.pci->func);
> +
> + printf("+->deviceinfo\n");
> + printf(" +->pci\n");
> + printf(" +->vendor_id %04x\n", device->deviceinfo.pci->vendor_id);
> + printf(" +->device_id %04x\n", device->deviceinfo.pci->device_id);
> + printf(" +->subvendor_id %04x\n", device->deviceinfo.pci->subvendor_id);
> + printf(" +->subdevice_id %04x\n", device->deviceinfo.pci->subdevice_id);
> if (print_revision)
> - printf("\t\t\trevision_id\t%02x\n", device->deviceinfo.pci->revision_id);
> + printf(" +->revision_id %02x\n", device->deviceinfo.pci->revision_id);
> else
> - printf("\t\t\trevision_id\tIGNORED\n");
> + printf(" +->revision_id IGNORED\n");
>
> } else if (device->bustype == DRM_BUS_USB) {
> - printf("\t\tusb\n");
> - printf("\t\t\tbus\t%03u\n", device->businfo.usb->bus);
> - printf("\t\t\tdev\t%03u\n", device->businfo.usb->dev);
> -
> - printf("\tdeviceinfo\n");
> - printf("\t\tusb\n");
> - printf("\t\t\tvendor\t%04x\n", device->deviceinfo.usb->vendor);
> - printf("\t\t\tproduct\t%04x\n", device->deviceinfo.usb->product);
> + printf("| +->usb\n");
> + printf("| +->bus %03u\n", device->businfo.usb->bus);
> + printf("| +->dev %03u\n", device->businfo.usb->dev);
> +
> + printf("+->deviceinfo\n");
> + printf(" +->usb\n");
> + printf(" +->vendor %04x\n", device->deviceinfo.usb->vendor);
> + printf(" +->product %04x\n", device->deviceinfo.usb->product);
> } else if (device->bustype == DRM_BUS_PLATFORM) {
> char **compatible = device->deviceinfo.platform->compatible;
>
> - printf("\t\tplatform\n");
> - printf("\t\t\tfullname\t%s\n", device->businfo.platform->fullname);
> + printf("| +->platform\n");
> + printf("| +->fullname\t%s\n", device->businfo.platform->fullname);
>
> - printf("\tdeviceinfo\n");
> - printf("\t\tplatform\n");
> - printf("\t\t\tcompatible\n");
> + printf("+->deviceinfo\n");
> + printf(" +->platform\n");
> + printf(" +->compatible\n");
>
> while (*compatible) {
> - printf("\t\t\t\t%s\n", *compatible);
> + printf(" %s\n", *compatible);
> compatible++;
> }
> } else if (device->bustype == DRM_BUS_HOST1X) {
> char **compatible = device->deviceinfo.platform->compatible;
>
> - printf("\t\thost1x\n");
> - printf("\t\t\tfullname\t%s\n", device->businfo.host1x->fullname);
> + printf("| +->host1x\n");
> + printf("| +->fullname\t%s\n", device->businfo.host1x->fullname);
>
> - printf("\tdeviceinfo\n");
> - printf("\t\tplatform\n");
> - printf("\t\t\tcompatible\n");
> + printf("+->deviceinfo\n");
> + printf(" +->platform\n");
> + printf(" +->compatible\n");
>
> while (*compatible) {
> - printf("\t\t\t\t%s\n", *compatible);
> + printf(" %s\n", *compatible);
> compatible++;
> }
> } else {
> --
> 2.18.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH libdrm 09/10] drmdevice: convert the tabbed output into a tree
2018-06-28 10:19 ` Eric Engestrom
@ 2018-06-28 16:43 ` Emil Velikov
0 siblings, 0 replies; 31+ messages in thread
From: Emil Velikov @ 2018-06-28 16:43 UTC (permalink / raw)
To: Eric Engestrom; +Cc: ML dri-devel
On 28 June 2018 at 11:19, Eric Engestrom <eric@engestrom.ch> wrote:
> On Monday, 2018-06-25 17:40:02 +0000, Emil Velikov wrote:
>> From: Emil Velikov <emil.velikov@collabora.com>
>>
>> Making the output a little bit easier to parse by human beings.
>>
>> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
>> ---
>> tests/drmdevice.c | 78 +++++++++++++++++++++++------------------------
>> 1 file changed, 39 insertions(+), 39 deletions(-)
>>
>> diff --git a/tests/drmdevice.c b/tests/drmdevice.c
>> index 0d75836f..e9e9d7f1 100644
>> --- a/tests/drmdevice.c
>> +++ b/tests/drmdevice.c
>> @@ -36,67 +36,67 @@ static void
>> print_device_info(drmDevicePtr device, int i, bool print_revision)
>> {
>> printf("device[%i]\n", i);
>> - printf("\tavailable_nodes %04x\n", device->available_nodes);
>> - printf("\tnodes\n");
>> + printf("+->available_nodes %#04x\n", device->available_nodes);
>> + printf("+->nodes\n");
>
> Nit: I'd put a space between `>` and the text, for readability
>
Ack, will do.
Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH libdrm 09/10] drmdevice: convert the tabbed output into a tree
2018-06-25 17:36 ` [PATCH libdrm 09/10] drmdevice: convert the tabbed output into a tree Emil Velikov
2018-06-28 10:19 ` Eric Engestrom
@ 2018-06-28 16:09 ` Robert Foss
2018-06-29 15:24 ` [PATCH libdrm v2 " Emil Velikov
2 siblings, 0 replies; 31+ messages in thread
From: Robert Foss @ 2018-06-28 16:09 UTC (permalink / raw)
To: Emil Velikov, dri-devel
Feel free to add my r-b to this patch.
On 2018-06-25 19:36, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> Making the output a little bit easier to parse by human beings.
>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> tests/drmdevice.c | 78 +++++++++++++++++++++++------------------------
> 1 file changed, 39 insertions(+), 39 deletions(-)
>
> diff --git a/tests/drmdevice.c b/tests/drmdevice.c
> index 0d75836f..e9e9d7f1 100644
> --- a/tests/drmdevice.c
> +++ b/tests/drmdevice.c
> @@ -36,67 +36,67 @@ static void
> print_device_info(drmDevicePtr device, int i, bool print_revision)
> {
> printf("device[%i]\n", i);
> - printf("\tavailable_nodes %04x\n", device->available_nodes);
> - printf("\tnodes\n");
> + printf("+->available_nodes %#04x\n", device->available_nodes);
> + printf("+->nodes\n");
> for (int j = 0; j < DRM_NODE_MAX; j++)
> if (device->available_nodes & 1 << j)
> - printf("\t\tnodes[%d] %s\n", j, device->nodes[j]);
> + printf("| +->nodes[%d] %s\n", j, device->nodes[j]);
>
> - printf("\tbustype %04x\n", device->bustype);
> - printf("\tbusinfo\n");
> + printf("+->bustype %04x\n", device->bustype);
> + printf("+->businfo\n");
> if (device->bustype == DRM_BUS_PCI) {
> - printf("\t\tpci\n");
> - printf("\t\t\tdomain\t%04x\n",device->businfo.pci->domain);
> - printf("\t\t\tbus\t%02x\n", device->businfo.pci->bus);
> - printf("\t\t\tdev\t%02x\n", device->businfo.pci->dev);
> - printf("\t\t\tfunc\t%1u\n", device->businfo.pci->func);
> -
> - printf("\tdeviceinfo\n");
> - printf("\t\tpci\n");
> - printf("\t\t\tvendor_id\t%04x\n", device->deviceinfo.pci->vendor_id);
> - printf("\t\t\tdevice_id\t%04x\n", device->deviceinfo.pci->device_id);
> - printf("\t\t\tsubvendor_id\t%04x\n", device->deviceinfo.pci->subvendor_id);
> - printf("\t\t\tsubdevice_id\t%04x\n", device->deviceinfo.pci->subdevice_id);
> + printf("| +->pci\n");
> + printf("| +->domain %04x\n",device->businfo.pci->domain);
> + printf("| +->bus %02x\n", device->businfo.pci->bus);
> + printf("| +->dev %02x\n", device->businfo.pci->dev);
> + printf("| +->func %1u\n", device->businfo.pci->func);
> +
> + printf("+->deviceinfo\n");
> + printf(" +->pci\n");
> + printf(" +->vendor_id %04x\n", device->deviceinfo.pci->vendor_id);
> + printf(" +->device_id %04x\n", device->deviceinfo.pci->device_id);
> + printf(" +->subvendor_id %04x\n", device->deviceinfo.pci->subvendor_id);
> + printf(" +->subdevice_id %04x\n", device->deviceinfo.pci->subdevice_id);
> if (print_revision)
> - printf("\t\t\trevision_id\t%02x\n", device->deviceinfo.pci->revision_id);
> + printf(" +->revision_id %02x\n", device->deviceinfo.pci->revision_id);
> else
> - printf("\t\t\trevision_id\tIGNORED\n");
> + printf(" +->revision_id IGNORED\n");
>
> } else if (device->bustype == DRM_BUS_USB) {
> - printf("\t\tusb\n");
> - printf("\t\t\tbus\t%03u\n", device->businfo.usb->bus);
> - printf("\t\t\tdev\t%03u\n", device->businfo.usb->dev);
> -
> - printf("\tdeviceinfo\n");
> - printf("\t\tusb\n");
> - printf("\t\t\tvendor\t%04x\n", device->deviceinfo.usb->vendor);
> - printf("\t\t\tproduct\t%04x\n", device->deviceinfo.usb->product);
> + printf("| +->usb\n");
> + printf("| +->bus %03u\n", device->businfo.usb->bus);
> + printf("| +->dev %03u\n", device->businfo.usb->dev);
> +
> + printf("+->deviceinfo\n");
> + printf(" +->usb\n");
> + printf(" +->vendor %04x\n", device->deviceinfo.usb->vendor);
> + printf(" +->product %04x\n", device->deviceinfo.usb->product);
> } else if (device->bustype == DRM_BUS_PLATFORM) {
> char **compatible = device->deviceinfo.platform->compatible;
>
> - printf("\t\tplatform\n");
> - printf("\t\t\tfullname\t%s\n", device->businfo.platform->fullname);
> + printf("| +->platform\n");
> + printf("| +->fullname\t%s\n", device->businfo.platform->fullname);
>
> - printf("\tdeviceinfo\n");
> - printf("\t\tplatform\n");
> - printf("\t\t\tcompatible\n");
> + printf("+->deviceinfo\n");
> + printf(" +->platform\n");
> + printf(" +->compatible\n");
>
> while (*compatible) {
> - printf("\t\t\t\t%s\n", *compatible);
> + printf(" %s\n", *compatible);
> compatible++;
> }
> } else if (device->bustype == DRM_BUS_HOST1X) {
> char **compatible = device->deviceinfo.platform->compatible;
>
> - printf("\t\thost1x\n");
> - printf("\t\t\tfullname\t%s\n", device->businfo.host1x->fullname);
> + printf("| +->host1x\n");
> + printf("| +->fullname\t%s\n", device->businfo.host1x->fullname);
>
> - printf("\tdeviceinfo\n");
> - printf("\t\tplatform\n");
> - printf("\t\t\tcompatible\n");
> + printf("+->deviceinfo\n");
> + printf(" +->platform\n");
> + printf(" +->compatible\n");
>
> while (*compatible) {
> - printf("\t\t\t\t%s\n", *compatible);
> + printf(" %s\n", *compatible);
> compatible++;
> }
> } else {
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH libdrm v2 09/10] drmdevice: convert the tabbed output into a tree
2018-06-25 17:36 ` [PATCH libdrm 09/10] drmdevice: convert the tabbed output into a tree Emil Velikov
2018-06-28 10:19 ` Eric Engestrom
2018-06-28 16:09 ` Robert Foss
@ 2018-06-29 15:24 ` Emil Velikov
2 siblings, 0 replies; 31+ messages in thread
From: Emil Velikov @ 2018-06-29 15:24 UTC (permalink / raw)
To: dri-devel; +Cc: Eric Engestrom, emil.l.velikov
From: Emil Velikov <emil.velikov@collabora.com>
Making the output a little bit easier to parse by human beings.
v2: Add extra whitespace (Eric)
Cc: Eric Engestrom <eric@engestrom.ch>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Tested-by: Robert Foss <robert.foss@collabora.com> (v1)
Reviewed-by: Robert Foss <robert.foss@collabora.com> (v1)
Reviewed-by: Eric Engestrom <eric@engestrom.ch> (v1)
---
tests/drmdevice.c | 77 +++++++++++++++++++++++------------------------
1 file changed, 38 insertions(+), 39 deletions(-)
diff --git a/tests/drmdevice.c b/tests/drmdevice.c
index 0d75836f..049df3cf 100644
--- a/tests/drmdevice.c
+++ b/tests/drmdevice.c
@@ -36,67 +36,66 @@ static void
print_device_info(drmDevicePtr device, int i, bool print_revision)
{
printf("device[%i]\n", i);
- printf("\tavailable_nodes %04x\n", device->available_nodes);
- printf("\tnodes\n");
+ printf("+-> available_nodes %#04x\n", device->available_nodes);
+ printf("+-> nodes\n");
for (int j = 0; j < DRM_NODE_MAX; j++)
if (device->available_nodes & 1 << j)
- printf("\t\tnodes[%d] %s\n", j, device->nodes[j]);
+ printf("| +-> nodes[%d] %s\n", j, device->nodes[j]);
- printf("\tbustype %04x\n", device->bustype);
- printf("\tbusinfo\n");
+ printf("+-> bustype %04x\n", device->bustype);
if (device->bustype == DRM_BUS_PCI) {
- printf("\t\tpci\n");
- printf("\t\t\tdomain\t%04x\n",device->businfo.pci->domain);
- printf("\t\t\tbus\t%02x\n", device->businfo.pci->bus);
- printf("\t\t\tdev\t%02x\n", device->businfo.pci->dev);
- printf("\t\t\tfunc\t%1u\n", device->businfo.pci->func);
-
- printf("\tdeviceinfo\n");
- printf("\t\tpci\n");
- printf("\t\t\tvendor_id\t%04x\n", device->deviceinfo.pci->vendor_id);
- printf("\t\t\tdevice_id\t%04x\n", device->deviceinfo.pci->device_id);
- printf("\t\t\tsubvendor_id\t%04x\n", device->deviceinfo.pci->subvendor_id);
- printf("\t\t\tsubdevice_id\t%04x\n", device->deviceinfo.pci->subdevice_id);
+ printf("| +-> pci\n");
+ printf("| +-> domain %04x\n",device->businfo.pci->domain);
+ printf("| +-> bus %02x\n", device->businfo.pci->bus);
+ printf("| +-> dev %02x\n", device->businfo.pci->dev);
+ printf("| +-> func %1u\n", device->businfo.pci->func);
+
+ printf("+-> deviceinfo\n");
+ printf(" +-> pci\n");
+ printf(" +-> vendor_id %04x\n", device->deviceinfo.pci->vendor_id);
+ printf(" +-> device_id %04x\n", device->deviceinfo.pci->device_id);
+ printf(" +-> subvendor_id %04x\n", device->deviceinfo.pci->subvendor_id);
+ printf(" +-> subdevice_id %04x\n", device->deviceinfo.pci->subdevice_id);
if (print_revision)
- printf("\t\t\trevision_id\t%02x\n", device->deviceinfo.pci->revision_id);
+ printf(" +-> revision_id %02x\n", device->deviceinfo.pci->revision_id);
else
- printf("\t\t\trevision_id\tIGNORED\n");
+ printf(" +-> revision_id IGNORED\n");
} else if (device->bustype == DRM_BUS_USB) {
- printf("\t\tusb\n");
- printf("\t\t\tbus\t%03u\n", device->businfo.usb->bus);
- printf("\t\t\tdev\t%03u\n", device->businfo.usb->dev);
-
- printf("\tdeviceinfo\n");
- printf("\t\tusb\n");
- printf("\t\t\tvendor\t%04x\n", device->deviceinfo.usb->vendor);
- printf("\t\t\tproduct\t%04x\n", device->deviceinfo.usb->product);
+ printf("| +-> usb\n");
+ printf("| +-> bus %03u\n", device->businfo.usb->bus);
+ printf("| +-> dev %03u\n", device->businfo.usb->dev);
+
+ printf("+-> deviceinfo\n");
+ printf(" +-> usb\n");
+ printf(" +-> vendor %04x\n", device->deviceinfo.usb->vendor);
+ printf(" +-> product %04x\n", device->deviceinfo.usb->product);
} else if (device->bustype == DRM_BUS_PLATFORM) {
char **compatible = device->deviceinfo.platform->compatible;
- printf("\t\tplatform\n");
- printf("\t\t\tfullname\t%s\n", device->businfo.platform->fullname);
+ printf("| +-> platform\n");
+ printf("| +-> fullname\t%s\n", device->businfo.platform->fullname);
- printf("\tdeviceinfo\n");
- printf("\t\tplatform\n");
- printf("\t\t\tcompatible\n");
+ printf("+-> deviceinfo\n");
+ printf(" +-> platform\n");
+ printf(" +-> compatible\n");
while (*compatible) {
- printf("\t\t\t\t%s\n", *compatible);
+ printf(" %s\n", *compatible);
compatible++;
}
} else if (device->bustype == DRM_BUS_HOST1X) {
char **compatible = device->deviceinfo.platform->compatible;
- printf("\t\thost1x\n");
- printf("\t\t\tfullname\t%s\n", device->businfo.host1x->fullname);
+ printf("| +-> host1x\n");
+ printf("| +-> fullname\t%s\n", device->businfo.host1x->fullname);
- printf("\tdeviceinfo\n");
- printf("\t\tplatform\n");
- printf("\t\t\tcompatible\n");
+ printf("+-> deviceinfo\n");
+ printf(" +-> platform\n");
+ printf(" +-> compatible\n");
while (*compatible) {
- printf("\t\t\t\t%s\n", *compatible);
+ printf(" %s\n", *compatible);
compatible++;
}
} else {
--
2.18.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH libdrm 10/10] drmdevice: print the correct host1x information
2018-06-25 17:36 [PATCH libdrm 01/10] xf86drm: drmGetDevice2: error out if the fd has unknown subsys Emil Velikov
` (7 preceding siblings ...)
2018-06-25 17:36 ` [PATCH libdrm 09/10] drmdevice: convert the tabbed output into a tree Emil Velikov
@ 2018-06-25 17:36 ` Emil Velikov
2018-06-28 16:09 ` Robert Foss
2018-06-28 10:11 ` [PATCH libdrm 01/10] xf86drm: drmGetDevice2: error out if the fd has unknown subsys Robert Foss
2018-06-28 10:11 ` Robert Foss
10 siblings, 1 reply; 31+ messages in thread
From: Emil Velikov @ 2018-06-25 17:36 UTC (permalink / raw)
To: dri-devel; +Cc: emil.l.velikov
From: Emil Velikov <emil.velikov@collabora.com>
While fairly close, the host1x and platform are two separate things.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
tests/drmdevice.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/drmdevice.c b/tests/drmdevice.c
index e9e9d7f1..97ce8ff9 100644
--- a/tests/drmdevice.c
+++ b/tests/drmdevice.c
@@ -86,13 +86,13 @@ print_device_info(drmDevicePtr device, int i, bool print_revision)
compatible++;
}
} else if (device->bustype == DRM_BUS_HOST1X) {
- char **compatible = device->deviceinfo.platform->compatible;
+ char **compatible = device->deviceinfo.host1x->compatible;
printf("| +->host1x\n");
printf("| +->fullname\t%s\n", device->businfo.host1x->fullname);
printf("+->deviceinfo\n");
- printf(" +->platform\n");
+ printf(" +->host1x\n");
printf(" +->compatible\n");
while (*compatible) {
--
2.18.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH libdrm 10/10] drmdevice: print the correct host1x information
2018-06-25 17:36 ` [PATCH libdrm 10/10] drmdevice: print the correct host1x information Emil Velikov
@ 2018-06-28 16:09 ` Robert Foss
0 siblings, 0 replies; 31+ messages in thread
From: Robert Foss @ 2018-06-28 16:09 UTC (permalink / raw)
To: dri-devel
Nice catch!
Feel free to add my r-b to this patch.
On 2018-06-25 19:36, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> While fairly close, the host1x and platform are two separate things.
>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> tests/drmdevice.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/drmdevice.c b/tests/drmdevice.c
> index e9e9d7f1..97ce8ff9 100644
> --- a/tests/drmdevice.c
> +++ b/tests/drmdevice.c
> @@ -86,13 +86,13 @@ print_device_info(drmDevicePtr device, int i, bool print_revision)
> compatible++;
> }
> } else if (device->bustype == DRM_BUS_HOST1X) {
> - char **compatible = device->deviceinfo.platform->compatible;
> + char **compatible = device->deviceinfo.host1x->compatible;
>
> printf("| +->host1x\n");
> printf("| +->fullname\t%s\n", device->businfo.host1x->fullname);
>
> printf("+->deviceinfo\n");
> - printf(" +->platform\n");
> + printf(" +->host1x\n");
> printf(" +->compatible\n");
>
> while (*compatible) {
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH libdrm 01/10] xf86drm: drmGetDevice2: error out if the fd has unknown subsys
2018-06-25 17:36 [PATCH libdrm 01/10] xf86drm: drmGetDevice2: error out if the fd has unknown subsys Emil Velikov
` (8 preceding siblings ...)
2018-06-25 17:36 ` [PATCH libdrm 10/10] drmdevice: print the correct host1x information Emil Velikov
@ 2018-06-28 10:11 ` Robert Foss
2018-06-28 10:11 ` Robert Foss
10 siblings, 0 replies; 31+ messages in thread
From: Robert Foss @ 2018-06-28 10:11 UTC (permalink / raw)
To: Emil Velikov, dri-devel
This series has been:
Tested-by: Robert Foss <robert.foss@collabora.com>
On 2018-06-25 19:36, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> Currently one can open() any /dev node. If it's unknown
> drmParseSubsystemType() will return an error.
>
> Track that and bail as needed.
>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> xf86drm.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 87c216cf..e1bbbe99 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -3814,6 +3814,8 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
> return -EINVAL;
>
> subsystem_type = drmParseSubsystemType(maj, min);
> + if (subsystem_type < 0)
> + return subsystem_type;
>
> local_devices = calloc(max_count, sizeof(drmDevicePtr));
> if (local_devices == NULL)
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH libdrm 01/10] xf86drm: drmGetDevice2: error out if the fd has unknown subsys
2018-06-25 17:36 [PATCH libdrm 01/10] xf86drm: drmGetDevice2: error out if the fd has unknown subsys Emil Velikov
` (9 preceding siblings ...)
2018-06-28 10:11 ` [PATCH libdrm 01/10] xf86drm: drmGetDevice2: error out if the fd has unknown subsys Robert Foss
@ 2018-06-28 10:11 ` Robert Foss
10 siblings, 0 replies; 31+ messages in thread
From: Robert Foss @ 2018-06-28 10:11 UTC (permalink / raw)
To: Emil Velikov, dri-devel
Feel free to add my r-b to this patch.
On 2018-06-25 19:36, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> Currently one can open() any /dev node. If it's unknown
> drmParseSubsystemType() will return an error.
>
> Track that and bail as needed.
>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> xf86drm.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 87c216cf..e1bbbe99 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -3814,6 +3814,8 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device)
> return -EINVAL;
>
> subsystem_type = drmParseSubsystemType(maj, min);
> + if (subsystem_type < 0)
> + return subsystem_type;
>
> local_devices = calloc(max_count, sizeof(drmDevicePtr));
> if (local_devices == NULL)
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread