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

[Axel Davy]

On 11/10/2016 20:31, Emil Velikov wrote:
>   
> -   udev = udev_new();
> -   if (!udev)
> -      goto prime_clean;
> +   if (drmGetDevice(default_fd, &device) != 0)
> +      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;
> -
> -   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;
> +      /* Hmm... detection for 2-7 seems to be broken. Nicely done. */
For DRI2, DRI_PRIME takes a number corresponding to the device number as 
configured in xorg (and you have xrandr commands to configure the 
behaviour). This doesn't work with the DRI3 scheme. However for 
retrocompatibility, after several discussions, we decided to let DRI2's 
most used command 'DRI_PRIME=1' to still sorta work on DRI3, but the 
meaning changed: DRI_PRIME=1 => 'give me any device that is not the 
compositor/xorg device'
> +      is_different_device = 1;
> +   } else {
> +      default_tag = drm_construct_id_path_tag(device);
> +      if (default_tag == NULL)
> +         goto err;
>   
> -   device_name = get_render_node_from_id_path_tag(udev,
> -                                                  prime,
> -                                                  another_tag);
> -   if (device_name == NULL) {
> +      if (strcmp(default_tag, prime) != 0) {
> +         free(default_tag);
> +         goto err;
> +      }
> +
> +      free(default_tag);
> +      /* we are to get a new fd (render-node) of the same device */
>         is_different_device = 0;
> -      goto default_device_clean;
> +      // XXX: WTF ^^ so one uses the new model only to point to the exact same
> +      // device and not the other more/less powerful GPU ?
This case if when one has in his configuration file that the app should 
run on device XXX, which happens to be the one in use by the compositor.
This case needed to be handled, and while it may not very seem useful 
now, it could be in the future an interesting case (On wayland, you 
could have the compositor use a card on some screens, and another card 
on the other, etc. In this case it makes sense for the user to specify 
the card, and it may or may not be the card used for the given screen).

Axel
>      }
> +   device_name = device->nodes[DRM_NODE_RENDER];
> +
> +   /* The device/node name provided by libdrm may be an empty string */
> +   if (device_name == NULL || device_name[0] == '\0')
> +      goto err;
>   
>      fd = loader_open_device(device_name);
> -   if (fd >= 0) {
> -      close(default_fd);
> -   } else {
> -      fd = default_fd;
> -      is_different_device = 0;
> -   }
> -   free(device_name);
> +   if (fd < 0)
> +      goto err;
>   
> - default_device_clean:
> -   free(default_device_id_path_tag);
> - udev_clean:
> -   udev_unref(udev);
> - prime_clean:
> +   close(default_fd);
> +
> +   drmFreeDevice(&device);
>      free(prime);
>   
>      *different_device = is_different_device;
>      return fd;
> +
> + err:
> +   drmFreeDevice(&device);
> +   free(prime);
> +
> +   *different_device = 0;
> +   return default_fd;
>   }
>   #else
>   int loader_get_user_preferred_fd(int default_fd, int *different_device)