On 14 October 2016 at 20:21, Axel Davy <axel.d...@ens.fr> wrote:
> On 14/10/2016 20:21, Emil Velikov wrote:
>>
>> From: Emil Velikov <emil.veli...@collabora.com>
>>
>> Currently not everyone has libudev and with follow-up patches we'll
>> completely remove the divergent codepaths.
>>
>> Use the libdrm drm device API to construct the required ID_PATH_TAG-like
>> string, to preserve the current functionality for libudev users and
>> allow others to benefit from it as well.
>>
>> v2: Drop ranty comments, pick the correct device
>>
>> Cc: Axel Davy <axel.d...@ens.fr>
>> Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
>> ---
>> src/loader/loader.c | 247
>> ++++++++++++++++++++++------------------------------
>> 1 file changed, 106 insertions(+), 141 deletions(-)
>>
>> diff --git a/src/loader/loader.c b/src/loader/loader.c
>> index ad4f946..06df05b 100644
>> --- a/src/loader/loader.c
>> +++ b/src/loader/loader.c
>> @@ -69,16 +69,11 @@
>> #include <sys/stat.h>
>> #include <stdarg.h>
>> #include <stdio.h>
>> +#include <stdbool.h>
>> #include <string.h>
>> #ifdef HAVE_LIBUDEV
>> #include <assert.h>
>> #include <dlfcn.h>
>> -#include <unistd.h>
>> -#include <stdlib.h>
>> -#ifdef USE_DRICONF
>> -#include "xmlconfig.h"
>> -#include "xmlpool.h"
>> -#endif
>> #endif
>> #ifdef MAJOR_IN_MKDEV
>> #include <sys/mkdev.h>
>> @@ -89,7 +84,13 @@
>> #include "loader.h"
>> #ifdef HAVE_LIBDRM
>> +#include <stdlib.h>
>> +#include <unistd.h>
>> #include <xf86drm.h>
>> +#ifdef USE_DRICONF
>> +#include "xmlconfig.h"
>> +#include "xmlpool.h"
>> +#endif
>> #endif
>> #define __IS_LOADER
>> @@ -203,99 +204,9 @@ udev_device_new_from_fd(struct udev *udev, int fd)
>> return device;
>> }
>> +#endif
>> -static char *
>> -get_render_node_from_id_path_tag(struct udev *udev,
>> - char *id_path_tag,
>> - char another_tag)
>> -{
>> - struct udev_device *device;
>> - struct udev_enumerate *e;
>> - struct udev_list_entry *entry;
>> - const char *path, *id_path_tag_tmp;
>> - char *path_res;
>> - char found = 0;
>> - UDEV_SYMBOL(struct udev_enumerate *, udev_enumerate_new,
>> - (struct udev *));
>> - UDEV_SYMBOL(int, udev_enumerate_add_match_subsystem,
>> - (struct udev_enumerate *, const char *));
>> - UDEV_SYMBOL(int, udev_enumerate_add_match_sysname,
>> - (struct udev_enumerate *, const char *));
>> - UDEV_SYMBOL(int, udev_enumerate_scan_devices,
>> - (struct udev_enumerate *));
>> - UDEV_SYMBOL(struct udev_list_entry *, udev_enumerate_get_list_entry,
>> - (struct udev_enumerate *));
>> - UDEV_SYMBOL(void, udev_enumerate_unref,
>> - (struct udev_enumerate *));
>> - UDEV_SYMBOL(struct udev_list_entry *, udev_list_entry_get_next,
>> - (struct udev_list_entry *));
>> - UDEV_SYMBOL(const char *, udev_list_entry_get_name,
>> - (struct udev_list_entry *));
>> - UDEV_SYMBOL(struct udev_device *, udev_device_new_from_syspath,
>> - (struct udev *, const char *));
>> - UDEV_SYMBOL(const char *, udev_device_get_property_value,
>> - (struct udev_device *, const char *));
>> - UDEV_SYMBOL(const char *, udev_device_get_devnode,
>> - (struct udev_device *));
>> - UDEV_SYMBOL(struct udev_device *, udev_device_unref,
>> - (struct udev_device *));
>> -
>> - e = udev_enumerate_new(udev);
>> - udev_enumerate_add_match_subsystem(e, "drm");
>> - udev_enumerate_add_match_sysname(e, "render*");
>> -
>> - udev_enumerate_scan_devices(e);
>> - udev_list_entry_foreach(entry, udev_enumerate_get_list_entry(e)) {
>> - path = udev_list_entry_get_name(entry);
>> - device = udev_device_new_from_syspath(udev, path);
>> - if (!device)
>> - continue;
>> - id_path_tag_tmp = udev_device_get_property_value(device,
>> "ID_PATH_TAG");
>> - if (id_path_tag_tmp) {
>> - if ((!another_tag && !strcmp(id_path_tag, id_path_tag_tmp)) ||
>> - (another_tag && strcmp(id_path_tag, id_path_tag_tmp))) {
>> - found = 1;
>> - break;
>> - }
>> - }
>> - udev_device_unref(device);
>> - }
>> -
>> - udev_enumerate_unref(e);
>> -
>> - if (found) {
>> - path_res = strdup(udev_device_get_devnode(device));
>> - udev_device_unref(device);
>> - return path_res;
>> - }
>> - return NULL;
>> -}
>> -
>> -static char *
>> -get_id_path_tag_from_fd(struct udev *udev, int fd)
>> -{
>> - struct udev_device *device;
>> - const char *id_path_tag_tmp;
>> - char *id_path_tag;
>> - UDEV_SYMBOL(const char *, udev_device_get_property_value,
>> - (struct udev_device *, const char *));
>> - UDEV_SYMBOL(struct udev_device *, udev_device_unref,
>> - (struct udev_device *));
>> -
>> - device = udev_device_new_from_fd(udev, fd);
>> - if (!device)
>> - return NULL;
>> -
>> - id_path_tag_tmp = udev_device_get_property_value(device,
>> "ID_PATH_TAG");
>> - if (!id_path_tag_tmp)
>> - return NULL;
>> -
>> - id_path_tag = strdup(id_path_tag_tmp);
>> -
>> - udev_device_unref(device);
>> - return id_path_tag;
>> -}
>> -
>> +#if defined(HAVE_LIBDRM)
>> #ifdef USE_DRICONF
>> static const char __driConfigOptionsLoader[] =
>> DRI_CONF_BEGIN
>> @@ -321,17 +232,60 @@ static char *loader_get_dri_config_device_id(void)
>> }
>> #endif
>> +static char *drm_construct_id_path_tag(drmDevicePtr device)
>> +{
>> +/* Length of "pci-xxxx_xx_xx_x\n" */
>
> I assume you want to say:
>
> "pci-xxxx_xx_xx_x" + terminal byte
>
> Thus it should be
>
> "pci-xxxx_xx_xx_x\0"
>
Yes, using \0 was the goal.
>> +#define PCI_ID_PATH_TAG_LENGTH 17
>> + char *tag = NULL;
>> +
>> + if (device->bustype == DRM_BUS_PCI) {
>> + tag = calloc(PCI_ID_PATH_TAG_LENGTH, sizeof(char));
>> + if (tag == NULL)
>> + return NULL;
>> +
>> + sprintf(tag, "pci-%04x_%02x_%02x_%1u",
>> device->businfo.pci->domain,
>> + device->businfo.pci->bus, device->businfo.pci->dev,
>> + device->businfo.pci->func);
>> + }
>
>
> I haven't tested, but I guess you have compared to what udev does to have
> the equivalence.
> The advantage of the udev path is that usb graphic cards are supposed to
> work with the system,
> but I've no idea what's the status of hotplug support for usb graphic cards
> in the kernel.
>
> I guess support for such systems can be introduced when needed.
>
Pretty much my line of thought as well. If we have (m)any such users
they're more than welcome to send patches, ask for assistance or test
patches.
> The code looks good. With the minor nitpick fixed, this patch is:
> Reviewed-by: Axel Davy <axel.d...@ens.fr>
>
Thanks. If you can skim through any of the other patches that'll be appreciated.
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
