[Mesa-dev] [PATCH v2 05/16] loader: reimplement loader_get_user_preferred_fd via libdrm

[Nicolai Hähnle]

On 14.10.2016 20:21, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov at 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.davy at ens.fr>
> Signed-off-by: Emil Velikov <emil.velikov at 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
[snip]
> @@ -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" */
> +#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);

Defensive programming would suggest to use snprintf.

[snip]
> @@ -345,55 +299,66 @@ int loader_get_user_preferred_fd(int default_fd, int *different_device)
>        return default_fd;
>     }
>
> -   udev = udev_new();
> -   if (!udev)
> -      goto prime_clean;
> +   default_tag = drm_get_id_path_tag_for_fd(default_fd);
> +   if (default_tag == NULL)
> +      goto err;
>
> -   default_device_id_path_tag = get_id_path_tag_from_fd(udev, default_fd);
> -   if (!default_device_id_path_tag)
> -      goto udev_clean;
> +   num_devices = drmGetDevices(devices, MAX_DRM_DEVICES);
> +   if (num_devices < 0)
> +      goto err;
>
> -   is_different_device = 1;
>     /* two format are supported:
>      * "1": choose any other card than the card used by default.
>      * id_path_tag: (for example "pci-0000_02_00_0") choose the card
>      * with this id_path_tag.
>      */
>     if (!strcmp(prime,"1")) {
> -      free(prime);
> -      prime = strdup(default_device_id_path_tag);
> -      /* request a card with a different card than the default card */
> -      another_tag = 1;
> -   } else if (!strcmp(default_device_id_path_tag, prime))
> -      /* we are to get a new fd (render-node) of the same device */
> -      is_different_device = 0;
> -
> -   device_name = get_render_node_from_id_path_tag(udev,
> -                                                  prime,
> -                                                  another_tag);
> -   if (device_name == NULL) {
> -      is_different_device = 0;
> -      goto default_device_clean;
> +      /* Hmm... detection for 2-7 seems to be broken. Oh well ...
> +       * Pick the first render device that is not our own.
> +       */
> +      for (i = 0; i < num_devices; i++) {
> +         if (devices[i]->available_nodes & 1 << DRM_NODE_RENDER &&
> +             !drm_device_matches_tag(devices[i], default_tag)) {
> +
> +            found = true;
> +            break;
> +         }
> +      }
> +   } else {
> +      for (i = 0; i < num_devices; i++) {
> +         if (devices[i]->available_nodes & 1 << DRM_NODE_RENDER &&
> +            drm_device_matches_tag(devices[i], prime)) {
> +
> +            found = true;
> +            break;
> +         }
> +      }

I feel like it would be helpful to have a warning here if the device was 
not found. This could avoid some confusion when people inevitably typo 
their prime setting.

Nicolai